From 89c595b772eaaa8809f5339d708d7dc22e51b662 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sun, 11 Oct 2020 22:34:28 +0300 Subject: [PATCH] [#3053] Removed target accessibility checks for OStatus endpoints delegating to RedirectController. Added tests. --- CHANGELOG.md | 1 + lib/pleroma/web/ostatus/ostatus_controller.ex | 13 +++---- lib/pleroma/web/router.ex | 38 +++++++++---------- .../static_fe/static_fe_controller_test.exs | 23 +++++++++++ 4 files changed, 48 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ae5d0eda..1e7bcca08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ switched to a new configuration mechanism, however it was not officially removed - Add documented-but-missing chat pagination. - Allow sending out emails again. +- OStatus / static FE endpoints: fixed inaccessibility for anonymous users on non-federating instances, switched to handling per `:restrict_unauthenticated` setting. ## Unreleased (Patch) diff --git a/lib/pleroma/web/ostatus/ostatus_controller.ex b/lib/pleroma/web/ostatus/ostatus_controller.ex index b4dc2a87f..e03ca8c0a 100644 --- a/lib/pleroma/web/ostatus/ostatus_controller.ex +++ b/lib/pleroma/web/ostatus/ostatus_controller.ex @@ -37,11 +37,10 @@ def object(conn, _params) do with id <- Endpoint.url() <> conn.request_path, {_, %Activity{} = activity} <- {:activity, Activity.get_create_by_object_ap_id_with_object(id)}, - {_, true} <- {:public?, Visibility.is_public?(activity)}, - {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)} do + {_, true} <- {:public?, Visibility.is_public?(activity)} do redirect(conn, to: "/notice/#{activity.id}") else - reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] -> + reason when reason in [{:public?, false}, {:activity, nil}] -> {:error, :not_found} e -> @@ -57,11 +56,10 @@ def activity(%{assigns: %{format: format}} = conn, _params) def activity(conn, _params) do with id <- Endpoint.url() <> conn.request_path, {_, %Activity{} = activity} <- {:activity, Activity.normalize(id)}, - {_, true} <- {:public?, Visibility.is_public?(activity)}, - {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)} do + {_, true} <- {:public?, Visibility.is_public?(activity)} do redirect(conn, to: "/notice/#{activity.id}") else - reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] -> + reason when reason in [{:public?, false}, {:activity, nil}] -> {:error, :not_found} e -> @@ -72,7 +70,6 @@ def activity(conn, _params) do def notice(%{assigns: %{format: format}} = conn, %{"id" => id}) do with {_, %Activity{} = activity} <- {:activity, Activity.get_by_id_with_object(id)}, {_, true} <- {:public?, Visibility.is_public?(activity)}, - {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)}, %User{} = user <- User.get_cached_by_ap_id(activity.data["actor"]) do cond do format in ["json", "activity+json"] -> @@ -100,7 +97,7 @@ def notice(%{assigns: %{format: format}} = conn, %{"id" => id}) do RedirectController.redirector(conn, nil) end else - reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] -> + reason when reason in [{:public?, false}, {:activity, nil}] -> conn |> put_status(404) |> RedirectController.redirector(nil, 404) diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 97fcaafd5..ef56360ed 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -9,6 +9,18 @@ defmodule Pleroma.Web.Router do plug(:accepts, ["html"]) end + pipeline :accepts_html_xml do + plug(:accepts, ["html", "xml", "rss", "atom"]) + end + + pipeline :accepts_html_json do + plug(:accepts, ["html", "activity+json", "json"]) + end + + pipeline :accepts_html_xml_json do + plug(:accepts, ["html", "xml", "rss", "atom", "activity+json", "json"]) + end + pipeline :accepts_xml_rss_atom do plug(:accepts, ["xml", "rss", "atom"]) end @@ -574,24 +586,10 @@ defmodule Pleroma.Web.Router do ) end - pipeline :ostatus_html_json do - plug(:accepts, ["html", "activity+json", "json"]) - plug(Pleroma.Plugs.StaticFEPlug) - end - - pipeline :ostatus_html_xml do - plug(:accepts, ["html", "xml", "rss", "atom"]) - plug(Pleroma.Plugs.StaticFEPlug) - end - - pipeline :ostatus_html_xml_json do - plug(:accepts, ["html", "xml", "rss", "atom", "activity+json", "json"]) - plug(Pleroma.Plugs.StaticFEPlug) - end - scope "/", Pleroma.Web do # Note: html format is supported only if static FE is enabled - pipe_through(:ostatus_html_json) + # Note: http signature is only considered for json requests (no auth for non-json requests) + pipe_through([:accepts_html_json, :http_signature, Pleroma.Plugs.StaticFEPlug]) get("/objects/:uuid", OStatus.OStatusController, :object) get("/activities/:uuid", OStatus.OStatusController, :activity) @@ -604,15 +602,17 @@ defmodule Pleroma.Web.Router do scope "/", Pleroma.Web do # Note: html format is supported only if static FE is enabled - pipe_through(:ostatus_html_xml_json) + # Note: http signature is only considered for json requests (no auth for non-json requests) + pipe_through([:accepts_html_xml_json, :http_signature, Pleroma.Plugs.StaticFEPlug]) - # Note: for json format responds with user profile (not user feed) + # Note: returns user _profile_ for json requests, redirects to user _feed_ for non-json ones get("/users/:nickname", Feed.UserController, :feed_redirect, as: :user_feed) end scope "/", Pleroma.Web do # Note: html format is supported only if static FE is enabled - pipe_through(:ostatus_html_xml) + pipe_through([:accepts_html_xml, Pleroma.Plugs.StaticFEPlug]) + get("/users/:nickname/feed", Feed.UserController, :feed, as: :user_feed) end diff --git a/test/web/static_fe/static_fe_controller_test.exs b/test/web/static_fe/static_fe_controller_test.exs index bab0b0a7b..8baf5b1ce 100644 --- a/test/web/static_fe/static_fe_controller_test.exs +++ b/test/web/static_fe/static_fe_controller_test.exs @@ -78,6 +78,18 @@ test "does not require authentication on non-federating instances", %{ assert html_response(conn, 200) =~ user.nickname end + + test "returns 404 for local user with `restrict_unauthenticated/profiles/local` setting", %{ + conn: conn + } do + clear_config([:restrict_unauthenticated, :profiles, :local], true) + + local_user = insert(:user, local: true) + + conn + |> get("/users/#{local_user.nickname}") + |> html_response(404) + end end describe "notice html" do @@ -200,5 +212,16 @@ test "does not require authentication on non-federating instances", %{ assert html_response(conn, 200) =~ "testing a thing!" end + + test "returns 404 for local public activity with `restrict_unauthenticated/activities/local` setting", + %{conn: conn, user: user} do + clear_config([:restrict_unauthenticated, :activities, :local], true) + + {:ok, activity} = CommonAPI.post(user, %{status: "testing a thing!"}) + + conn + |> get("/notice/#{activity.id}") + |> html_response(404) + end end end