From 3e08e7715126ca1f3bfaf7dddf4806e76d9bd993 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 7 Jul 2020 20:37:11 +0300 Subject: [PATCH 1/4] [#1895] Made hashtag timeline respect `:restrict_unauthenticated` instance setting. --- docs/configuration/cheatsheet.md | 5 +- .../controllers/timeline_controller.ex | 43 +++++++---- .../controllers/timeline_controller_test.exs | 74 +++++++++++++++++++ 3 files changed, 104 insertions(+), 18 deletions(-) diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 6b640cebc..1de51e72d 100644 --- a/docs/configuration/cheatsheet.md +++ b/docs/configuration/cheatsheet.md @@ -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 diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index 4bdd46d7e..4bbb82c23 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -88,21 +88,23 @@ def direct(%{assigns: %{user: user}} = conn, params) do ) end - # GET /api/v1/timelines/public - def public(%{assigns: %{user: user}} = conn, params) do - local_only = params[:local] - - cfg_key = + defp restrict_unauthenticated?(local_only) do + config_key = if local_only do :local else :federated end - restrict? = Pleroma.Config.get([:restrict_unauthenticated, :timelines, cfg_key]) + Pleroma.Config.get([:restrict_unauthenticated, :timelines, config_key]) + end - if restrict? and is_nil(user) do - render_error(conn, :unauthorized, "authorization required for timeline view") + # GET /api/v1/timelines/public + def public(%{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 = params @@ -123,6 +125,10 @@ def public(%{assigns: %{user: user}} = conn, params) 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,15 +163,20 @@ defp hashtag_fetching(params, user, local_only) do # GET /api/v1/timelines/tag/:tag def hashtag(%{assigns: %{user: user}} = conn, params) do local_only = params[:local] - activities = hashtag_fetching(params, user, local_only) - conn - |> add_link_headers(activities, %{"local" => local_only}) - |> render("index.json", - activities: activities, - for: user, - as: :activity - ) + if is_nil(user) and restrict_unauthenticated?(local_only) do + fail_on_bad_auth(conn) + else + activities = hashtag_fetching(params, user, local_only) + + conn + |> add_link_headers(activities, %{"local" => local_only}) + |> render("index.json", + activities: activities, + for: user, + as: :activity + ) + end end # GET /api/v1/timelines/list/:list_id diff --git a/test/web/mastodon_api/controllers/timeline_controller_test.exs b/test/web/mastodon_api/controllers/timeline_controller_test.exs index f069390c1..50e0d783d 100644 --- a/test/web/mastodon_api/controllers/timeline_controller_test.exs +++ b/test/web/mastodon_api/controllers/timeline_controller_test.exs @@ -418,4 +418,78 @@ test "multi-hashtag timeline", %{conn: conn} 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 From 20461137a37968ee4dc8e3a38f7d9c63714702d3 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 7 Jul 2020 20:44:16 +0300 Subject: [PATCH 2/4] [#1895] Documentation hints on private instances and instance/restrict_unauthenticated setting. --- config/description.exs | 5 +++-- docs/configuration/cheatsheet.md | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/description.exs b/config/description.exs index 650610fbe..705ba83d0 100644 --- a/config/description.exs +++ b/config/description.exs @@ -700,8 +700,9 @@ key: :public, type: :boolean, description: - "Makes the client API in authentificated mode-only except for user-profiles." <> - " Useful for disabling the Local Timeline and The Whole Known Network." + "Makes the client API in authenticated mode-only except for user-profiles." <> + " 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, diff --git a/docs/configuration/cheatsheet.md b/docs/configuration/cheatsheet.md index 1de51e72d..f6529b940 100644 --- a/docs/configuration/cheatsheet.md +++ b/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). From 3f8370a285d1ee705e4899656b88c442c50cb99b Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 8 Jul 2020 12:36:44 +0300 Subject: [PATCH 3/4] [#1895] Applied code review suggestion. --- .../mastodon_api/controllers/timeline_controller.ex | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index 4bbb82c23..a23f47e54 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -88,15 +88,12 @@ def direct(%{assigns: %{user: user}} = conn, params) do ) end - defp restrict_unauthenticated?(local_only) do - config_key = - if local_only do - :local - else - :federated - end + defp restrict_unauthenticated?(_local_only = true) do + Pleroma.Config.get([:restrict_unauthenticated, :timelines, :local]) + end - Pleroma.Config.get([:restrict_unauthenticated, :timelines, config_key]) + defp restrict_unauthenticated?(_) do + Pleroma.Config.get([:restrict_unauthenticated, :timelines, :federated]) end # GET /api/v1/timelines/public From a6495f4a686feb3d4728432dd075f425c04c32dd Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 8 Jul 2020 12:54:23 +0300 Subject: [PATCH 4/4] [#1895] credo fix. --- lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex index a23f47e54..ab7b1d6aa 100644 --- a/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/timeline_controller.ex @@ -88,7 +88,7 @@ def direct(%{assigns: %{user: user}} = conn, params) do ) end - defp restrict_unauthenticated?(_local_only = true) do + defp restrict_unauthenticated?(true = _local_only) do Pleroma.Config.get([:restrict_unauthenticated, :timelines, :local]) end