From: Ivan Tashkinov <ivantashkinov@gmail.com>
Date: Sun, 11 Oct 2020 19:34:28 +0000 (+0300)
Subject: [#3053] Removed target accessibility checks for OStatus endpoints delegating to Redir... 
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=89c595b772eaaa8809f5339d708d7dc22e51b662;p=akkoma

[#3053] Removed target accessibility checks for OStatus endpoints delegating to RedirectController. Added tests.
---

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 @@ defmodule Pleroma.Web.OStatus.OStatusController 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 @@ defmodule Pleroma.Web.OStatus.OStatusController do
   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 @@ defmodule Pleroma.Web.OStatus.OStatusController 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 @@ defmodule Pleroma.Web.OStatus.OStatusController 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 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do
 
       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 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do
 
       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