Merge branch '1895-hashtag-timeline-restrict-unauthenticated-fix' into 'develop'

[#1895] Made hashtag timeline respect `:restrict_unauthenticated` instance setting

Closes #1895

See merge request pleroma/pleroma!2731

config/description.exs

@@ -700,8 +700,9 @@ config :pleroma, :config_description, [
         key: :public,
         type: :boolean,
         description:
-          "Makes the client API in authentificated mode-only except for user-profiles." <>
+          "Makes the client API in authenticated mode-only except for user-profiles." <>
-            " Useful for disabling the Local Timeline and The Whole Known Network."
+            " Useful for disabling the Local Timeline and The Whole Known Network. " <>
+            " Note: when setting to `false`, please also check `:restrict_unauthenticated` setting."
       },
       %{
         key: :quarantined_instances,

docs/configuration/cheatsheet.md

@@ -37,7 +37,7 @@ To add configuration to your config file, you can copy it from the base config.
 * `federation_incoming_replies_max_depth`: Max. depth of reply-to activities fetching on incoming federation, to prevent out-of-memory situations while fetching very long threads. If set to `nil`, threads of any depth will be fetched. Lower this value if you experience out-of-memory crashes.
 * `federation_reachability_timeout_days`: Timeout (in days) of each external federation target being unreachable prior to pausing federating to it.
 * `allow_relay`: Enable Pleroma’s Relay, which makes it possible to follow a whole instance.
-* `public`: Makes the client API in authenticated mode-only except for user-profiles. Useful for disabling the Local Timeline and The Whole Known Network.
+* `public`: Makes the client API in authenticated mode-only except for user-profiles. Useful for disabling the Local Timeline and The Whole Known Network. See also: `restrict_unauthenticated`.
 * `quarantined_instances`: List of ActivityPub instances where private(DMs, followers-only) activities will not be send.
 * `managed_config`: Whenether the config for pleroma-fe is configured in [:frontend_configurations](#frontend_configurations) or in ``static/config.json``.
 * `allowed_post_formats`: MIME-type list of formats allowed to be posted (transformed into HTML).
@@ -971,11 +971,11 @@ config :pleroma, :database_config_whitelist, [
 
 ### :restrict_unauthenticated
 
-Restrict access for unauthenticated users to timelines (public and federate), user profiles and statuses.
+Restrict access for unauthenticated users to timelines (public and federated), user profiles and statuses.
 
 * `timelines`: public and federated timelines
   * `local`: public timeline
-  * `federated`
+  * `federated`: federated timeline (includes public timeline)
 * `profiles`: user profiles
   * `local`
   * `remote`
@@ -983,6 +983,7 @@ Restrict access for unauthenticated users to timelines (public and federate), us
   * `local`
   * `remote`
 
+Note: setting `restrict_unauthenticated/timelines/local` to `true` has no practical sense if `restrict_unauthenticated/timelines/federated` is set to `false` (since local public activities will still be delivered to unauthenticated users as part of federated timeline). 
 
 ## Pleroma.Web.ApiSpec.CastAndValidate
 

lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex

@@ -88,21 +88,20 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
     )
   end
 
+  defp restrict_unauthenticated?(true = _local_only) do
+    Pleroma.Config.get([:restrict_unauthenticated, :timelines, :local])
+  end
+
+  defp restrict_unauthenticated?(_) do
+    Pleroma.Config.get([:restrict_unauthenticated, :timelines, :federated])
+  end
+
   # GET /api/v1/timelines/public
   def public(%{assigns: %{user: user}} = conn, params) do
     local_only = params[:local]
 
-    cfg_key =
+    if is_nil(user) and restrict_unauthenticated?(local_only) do
-      if local_only do
+      fail_on_bad_auth(conn)
-        :local
-      else
-        :federated
-      end
-
-    restrict? = Pleroma.Config.get([:restrict_unauthenticated, :timelines, cfg_key])
-
-    if restrict? and is_nil(user) do
-      render_error(conn, :unauthorized, "authorization required for timeline view")
     else
       activities =
         params
@@ -123,6 +122,10 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
     end
   end
 
+  defp fail_on_bad_auth(conn) do
+    render_error(conn, :unauthorized, "authorization required for timeline view")
+  end
+
   defp hashtag_fetching(params, user, local_only) do
     tags =
       [params[:tag], params[:any]]
@@ -157,6 +160,10 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
   # GET /api/v1/timelines/tag/:tag
   def hashtag(%{assigns: %{user: user}} = conn, params) do
     local_only = params[:local]
+
+    if is_nil(user) and restrict_unauthenticated?(local_only) do
+      fail_on_bad_auth(conn)
+    else
       activities = hashtag_fetching(params, user, local_only)
 
       conn
@@ -167,6 +174,7 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
         as: :activity
       )
     end
+  end
 
   # GET /api/v1/timelines/list/:list_id
   def list(%{assigns: %{user: user}} = conn, %{list_id: id} = params) do

test/web/mastodon_api/controllers/timeline_controller_test.exs

@@ -418,4 +418,78 @@ defmodule Pleroma.Web.MastodonAPI.TimelineControllerTest do
       assert [status_none] == json_response_and_validate_schema(all_test, :ok)
     end
   end
+
+  describe "hashtag timeline handling of :restrict_unauthenticated setting" do
+    setup do
+      user = insert(:user)
+      {:ok, activity1} = CommonAPI.post(user, %{status: "test #tag1"})
+      {:ok, _activity2} = CommonAPI.post(user, %{status: "test #tag1"})
+
+      activity1
+      |> Ecto.Changeset.change(%{local: false})
+      |> Pleroma.Repo.update()
+
+      base_uri = "/api/v1/timelines/tag/tag1"
+      error_response = %{"error" => "authorization required for timeline view"}
+
+      %{base_uri: base_uri, error_response: error_response}
+    end
+
+    defp ensure_authenticated_access(base_uri) do
+      %{conn: auth_conn} = oauth_access(["read:statuses"])
+
+      res_conn = get(auth_conn, "#{base_uri}?local=true")
+      assert length(json_response(res_conn, 200)) == 1
+
+      res_conn = get(auth_conn, "#{base_uri}?local=false")
+      assert length(json_response(res_conn, 200)) == 2
+    end
+
+    test "with `%{local: true, federated: true}`, returns 403 for unauthenticated users", %{
+      conn: conn,
+      base_uri: base_uri,
+      error_response: error_response
+    } do
+      clear_config([:restrict_unauthenticated, :timelines, :local], true)
+      clear_config([:restrict_unauthenticated, :timelines, :federated], true)
+
+      for local <- [true, false] do
+        res_conn = get(conn, "#{base_uri}?local=#{local}")
+
+        assert json_response(res_conn, :unauthorized) == error_response
+      end
+
+      ensure_authenticated_access(base_uri)
+    end
+
+    test "with `%{local: false, federated: true}`, forbids unauthenticated access to federated timeline",
+         %{conn: conn, base_uri: base_uri, error_response: error_response} do
+      clear_config([:restrict_unauthenticated, :timelines, :local], false)
+      clear_config([:restrict_unauthenticated, :timelines, :federated], true)
+
+      res_conn = get(conn, "#{base_uri}?local=true")
+      assert length(json_response(res_conn, 200)) == 1
+
+      res_conn = get(conn, "#{base_uri}?local=false")
+      assert json_response(res_conn, :unauthorized) == error_response
+
+      ensure_authenticated_access(base_uri)
+    end
+
+    test "with `%{local: true, federated: false}`, forbids unauthenticated access to public timeline" <>
+           "(but not to local public activities which are delivered as part of federated timeline)",
+         %{conn: conn, base_uri: base_uri, error_response: error_response} do
+      clear_config([:restrict_unauthenticated, :timelines, :local], true)
+      clear_config([:restrict_unauthenticated, :timelines, :federated], false)
+
+      res_conn = get(conn, "#{base_uri}?local=true")
+      assert json_response(res_conn, :unauthorized) == error_response
+
+      # Note: local activities get delivered as part of federated timeline
+      res_conn = get(conn, "#{base_uri}?local=false")
+      assert length(json_response(res_conn, 200)) == 2
+
+      ensure_authenticated_access(base_uri)
+    end
+  end
 end