[#3053] Unauthenticated access control for OStatus-related controllers and ActivityPubController (base actions: :user, :object, :activity). Tests adjustments.

lib/pleroma/web/activity_pub/activity_pub_controller.ex

@@ -32,17 +32,23 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
 
   @federating_only_actions [:internal_fetch, :relay, :relay_following, :relay_followers]
 
+  # Note: :following and :followers must be served even without authentication (as via :api)
+  @auth_only_actions [:read_inbox, :update_outbox, :whoami, :upload_media]
+
+  # Always accessible actions (must perform entity accessibility checks)
+  @no_auth_no_federation_actions [:user, :activity, :object]
+
+  @authenticated_or_federating_actions @federating_only_actions ++
+                                         @auth_only_actions ++ @no_auth_no_federation_actions
+
   plug(FederatingPlug when action in @federating_only_actions)
 
+  plug(EnsureAuthenticatedPlug when action in @auth_only_actions)
+
   plug(
     EnsureAuthenticatedPlug,
-    [unless_func: &FederatingPlug.federating?/1] when action not in @federating_only_actions
+    [unless_func: &FederatingPlug.federating?/1]
-  )
+    when action not in @authenticated_or_federating_actions
-
-  # Note: :following and :followers must be served even without authentication (as via :api)
-  plug(
-    EnsureAuthenticatedPlug
-    when action in [:read_inbox, :update_outbox, :whoami, :upload_media]
   )
 
   plug(
@@ -66,21 +72,22 @@ defp relay_active?(conn, _) do
 
   def user(conn, %{"nickname" => nickname}) do
     with %User{local: true} = user <- User.get_cached_by_nickname(nickname),
+         {_, :visible} <- {:visibility, User.visible_for(user, _reading_user = nil)},
          {:ok, user} <- User.ensure_keys_present(user) do
       conn
       |> put_resp_content_type("application/activity+json")
       |> put_view(UserView)
       |> render("user.json", %{user: user})
     else
-      nil -> {:error, :not_found}
+      _ -> {:error, :not_found}
-      %{local: false} -> {:error, :not_found}
     end
   end
 
   def object(conn, _) do
     with ap_id <- Endpoint.url() <> conn.request_path,
          %Object{} = object <- Object.get_cached_by_ap_id(ap_id),
-         {_, true} <- {:public?, Visibility.is_public?(object)} do
+         {_, true} <- {:public?, Visibility.is_public?(object)},
+         {_, false} <- {:restricted?, Visibility.restrict_unauthenticated_access?(object)} do
       conn
       |> assign(:tracking_fun_data, object.id)
       |> set_cache_ttl_for(object)
@@ -88,25 +95,15 @@ def object(conn, _) do
       |> put_view(ObjectView)
       |> render("object.json", object: object)
     else
-      {:public?, false} ->
+      _ -> {:error, :not_found}
-        {:error, :not_found}
     end
   end
 
-  def track_object_fetch(conn, nil), do: conn
-
-  def track_object_fetch(conn, object_id) do
-    with %{assigns: %{user: %User{id: user_id}}} <- conn do
-      Delivery.create(object_id, user_id)
-    end
-
-    conn
-  end
-
   def activity(conn, _params) do
     with ap_id <- Endpoint.url() <> conn.request_path,
          %Activity{} = activity <- Activity.normalize(ap_id),
-         {_, true} <- {:public?, Visibility.is_public?(activity)} do
+         {_, true} <- {:public?, Visibility.is_public?(activity)},
+         {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)} do
       conn
       |> maybe_set_tracking_data(activity)
       |> set_cache_ttl_for(activity)
@@ -114,8 +111,7 @@ def activity(conn, _params) do
       |> put_view(ObjectView)
       |> render("object.json", object: activity)
     else
-      {:public?, false} -> {:error, :not_found}
+      _ -> {:error, :not_found}
-      nil -> {:error, :not_found}
     end
   end
 
@@ -550,4 +546,14 @@ def upload_media(%{assigns: %{user: %User{} = user}} = conn, %{"file" => file} =
       |> json(object.data)
     end
   end
+
+  def track_object_fetch(conn, nil), do: conn
+
+  def track_object_fetch(conn, object_id) do
+    with %{assigns: %{user: %User{id: user_id}}} <- conn do
+      Delivery.create(object_id, user_id)
+    end
+
+    conn
+  end
 end

lib/pleroma/web/activity_pub/visibility.ex

@@ -44,29 +44,30 @@ def is_direct?(activity) do
   def is_list?(%{data: %{"listMessage" => _}}), do: true
   def is_list?(_), do: false
 
-  @spec visible_for_user?(Activity.t(), User.t() | nil) :: boolean()
+  @spec visible_for_user?(Activity.t() | nil, User.t() | nil) :: boolean()
-  def visible_for_user?(%{actor: ap_id}, %User{ap_id: ap_id}), do: true
+  def visible_for_user?(%Activity{actor: ap_id}, %User{ap_id: ap_id}), do: true
 
   def visible_for_user?(nil, _), do: false
 
-  def visible_for_user?(%{data: %{"listMessage" => _}}, nil), do: false
+  def visible_for_user?(%Activity{data: %{"listMessage" => _}}, nil), do: false
 
-  def visible_for_user?(%{data: %{"listMessage" => list_ap_id}} = activity, %User{} = user) do
+  def visible_for_user?(
+        %Activity{data: %{"listMessage" => list_ap_id}} = activity,
+        %User{} = user
+      ) do
     user.ap_id in activity.data["to"] ||
       list_ap_id
       |> Pleroma.List.get_by_ap_id()
       |> Pleroma.List.member?(user)
   end
 
-  def visible_for_user?(%{local: local} = activity, nil) do
+  def visible_for_user?(%Activity{} = activity, nil) do
-    cfg_key = if local, do: :local, else: :remote
+    if restrict_unauthenticated_access?(activity),
-
-    if Pleroma.Config.restrict_unauthenticated_access?(:activities, cfg_key),
       do: false,
       else: is_public?(activity)
   end
 
-  def visible_for_user?(activity, user) do
+  def visible_for_user?(%Activity{} = activity, user) do
     x = [user.ap_id | User.following(user)]
     y = [activity.actor] ++ activity.data["to"] ++ (activity.data["cc"] || [])
     is_public?(activity) || Enum.any?(x, &(&1 in y))
@@ -82,6 +83,26 @@ def entire_thread_visible_for_user?(%Activity{} = activity, %User{} = user) do
     result
   end
 
+  def restrict_unauthenticated_access?(%Activity{local: local}) do
+    restrict_unauthenticated_access_to_activity?(local)
+  end
+
+  def restrict_unauthenticated_access?(%Object{} = object) do
+    object
+    |> Object.local?()
+    |> restrict_unauthenticated_access_to_activity?()
+  end
+
+  def restrict_unauthenticated_access?(%User{} = user) do
+    User.visible_for(user, _reading_user = nil)
+  end
+
+  defp restrict_unauthenticated_access_to_activity?(local?) when is_boolean(local?) do
+    cfg_key = if local?, do: :local, else: :remote
+
+    Pleroma.Config.restrict_unauthenticated_access?(:activities, cfg_key)
+  end
+
   def get_visibility(object) do
     to = object.data["to"] || []
     cc = object.data["cc"] || []

lib/pleroma/web/feed/tag_controller.ex

@@ -10,7 +10,7 @@ defmodule Pleroma.Web.Feed.TagController do
   alias Pleroma.Web.Feed.FeedView
 
   def feed(conn, params) do
-    unless Config.restrict_unauthenticated_access?(:activities, :local) do
+    if Config.get!([:instance, :public]) do
       render_feed(conn, params)
     else
       render_error(conn, :not_found, "Not found")
@@ -36,12 +36,13 @@ defp render_feed(conn, %{"tag" => raw_tag} = params) do
   end
 
   @spec parse_tag(binary() | any()) :: {format :: String.t(), tag :: String.t()}
-  defp parse_tag(raw_tag) when is_binary(raw_tag) do
+  defp parse_tag(raw_tag) do
-    case Enum.reverse(String.split(raw_tag, ".")) do
+    case is_binary(raw_tag) && Enum.reverse(String.split(raw_tag, ".")) do
-      [format | tag] when format in ["atom", "rss"] -> {format, Enum.join(tag, ".")}
+      [format | tag] when format in ["rss", "atom"] ->
-      _ -> {"rss", raw_tag}
+        {format, Enum.join(tag, ".")}
-    end
-  end
 
-  defp parse_tag(raw_tag), do: {"rss", raw_tag}
+      _ ->
+        {"atom", raw_tag}
+    end
+  end
 end

lib/pleroma/web/feed/user_controller.ex

@@ -6,6 +6,8 @@ defmodule Pleroma.Web.Feed.UserController do
   use Pleroma.Web, :controller
 
   alias Fallback.RedirectController
+
+  alias Pleroma.Config
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
   alias Pleroma.Web.ActivityPub.ActivityPubController
@@ -32,15 +34,7 @@ def feed_redirect(conn, %{"nickname" => nickname}) do
     end
   end
 
-  def feed(conn, params) do
+  def feed(conn, %{"nickname" => nickname} = params) do
-    unless Pleroma.Config.restrict_unauthenticated_access?(:profiles, :local) do
-      render_feed(conn, params)
-    else
-      errors(conn, {:error, :not_found})
-    end
-  end
-
-  defp render_feed(conn, %{"nickname" => nickname} = params) do
     format = get_format(conn)
 
     format =
@@ -50,7 +44,8 @@ defp render_feed(conn, %{"nickname" => nickname} = params) do
         "atom"
       end
 
-    with {_, %User{local: true} = user} <- {:fetch_user, User.get_cached_by_nickname(nickname)} do
+    with {_, %User{local: true} = user} <- {:fetch_user, User.get_cached_by_nickname(nickname)},
+         {_, :visible} <- {:visibility, User.visible_for(user, _reading_user = nil)} do
       activities =
         %{
           type: ["Create"],
@@ -65,7 +60,7 @@ defp render_feed(conn, %{"nickname" => nickname} = params) do
       |> render("user.#{format}",
         user: user,
         activities: activities,
-        feed_config: Pleroma.Config.get([:feed])
+        feed_config: Config.get([:feed])
       )
     end
   end
@@ -77,6 +72,8 @@ def errors(conn, {:error, :not_found}) do
   def errors(conn, {:fetch_user, %User{local: false}}), do: errors(conn, {:error, :not_found})
   def errors(conn, {:fetch_user, nil}), do: errors(conn, {:error, :not_found})
 
+  def errors(conn, {:visibility, _}), do: errors(conn, {:error, :not_found})
+
   def errors(conn, _) do
     render_error(conn, :internal_server_error, "Something went wrong")
   end

lib/pleroma/web/ostatus/ostatus_controller.ex

@@ -33,16 +33,15 @@ def object(%{assigns: %{format: format}} = conn, _params)
     ActivityPubController.call(conn, :object)
   end
 
-  def object(%{assigns: %{format: format}} = conn, _params) do
+  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)} do
+         {_, true} <- {:public?, Visibility.is_public?(activity)},
-      case format do
+         {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)} do
-        _ -> redirect(conn, to: "/notice/#{activity.id}")
+      redirect(conn, to: "/notice/#{activity.id}")
-      end
     else
-      reason when reason in [{:public?, false}, {:activity, nil}] ->
+      reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] ->
         {:error, :not_found}
 
       e ->
@@ -55,15 +54,14 @@ def activity(%{assigns: %{format: format}} = conn, _params)
     ActivityPubController.call(conn, :activity)
   end
 
-  def activity(%{assigns: %{format: format}} = conn, _params) 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)} do
+         {_, true} <- {:public?, Visibility.is_public?(activity)},
-      case format do
+         {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)} do
-        _ -> redirect(conn, to: "/notice/#{activity.id}")
+      redirect(conn, to: "/notice/#{activity.id}")
-      end
     else
-      reason when reason in [{:public?, false}, {:activity, nil}] ->
+      reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] ->
         {:error, :not_found}
 
       e ->
@@ -74,6 +72,7 @@ def activity(%{assigns: %{format: format}} = 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"] ->
@@ -101,7 +100,7 @@ def notice(%{assigns: %{format: format}} = conn, %{"id" => id}) do
           RedirectController.redirector(conn, nil)
       end
     else
-      reason when reason in [{:public?, false}, {:activity, nil}] ->
+      reason when reason in [{:public?, false}, {:visible?, false}, {:activity, nil}] ->
         conn
         |> put_status(404)
         |> RedirectController.redirector(nil, 404)
@@ -115,6 +114,7 @@ def notice(%{assigns: %{format: format}} = conn, %{"id" => id}) do
   def notice_player(conn, %{"id" => id}) do
     with %Activity{data: %{"type" => "Create"}} = activity <- Activity.get_by_id_with_object(id),
          true <- Visibility.is_public?(activity),
+         {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)},
          %Object{} = object <- Object.normalize(activity),
          %{data: %{"attachment" => [%{"url" => [url | _]} | _]}} <- object,
          true <- String.starts_with?(url["mediaType"], ["audio", "video"]) do

lib/pleroma/web/router.ex

@@ -5,6 +5,14 @@
 defmodule Pleroma.Web.Router do
   use Pleroma.Web, :router
 
+  pipeline :accepts_html do
+    plug(:accepts, ["html"])
+  end
+
+  pipeline :accepts_xml_rss_atom do
+    plug(:accepts, ["xml", "rss", "atom"])
+  end
+
   pipeline :browser do
     plug(:accepts, ["html"])
     plug(:fetch_session)
@@ -556,39 +564,55 @@ defmodule Pleroma.Web.Router do
     )
   end
 
-  pipeline :ostatus do
+  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
 
-  pipeline :ostatus_no_html do
-    plug(:accepts, ["xml", "rss", "atom", "activity+json", "json"])
-  end
-
-  pipeline :oembed do
-    plug(:accepts, ["json", "xml"])
-  end
-
   scope "/", Pleroma.Web do
-    # Note: no authentication plugs, all endpoints below should only yield public objects
+    # Note: html format is supported only if static FE is enabled
-    pipe_through(:ostatus)
+    pipe_through(:ostatus_html_json)
 
     get("/objects/:uuid", OStatus.OStatusController, :object)
     get("/activities/:uuid", OStatus.OStatusController, :activity)
     get("/notice/:id", OStatus.OStatusController, :notice)
-    get("/notice/:id/embed_player", OStatus.OStatusController, :notice_player)
 
     # Mastodon compatibility routes
     get("/users/:nickname/statuses/:id", OStatus.OStatusController, :object)
     get("/users/:nickname/statuses/:id/activity", OStatus.OStatusController, :activity)
+  end
 
-    get("/users/:nickname/feed", Feed.UserController, :feed, as: :user_feed)
+  scope "/", Pleroma.Web do
+    # Note: html format is supported only if static FE is enabled
+    pipe_through(:ostatus_html_xml_json)
+
+    # Note: for json format responds with user profile (not user feed)
     get("/users/:nickname", Feed.UserController, :feed_redirect, as: :user_feed)
   end
 
   scope "/", Pleroma.Web do
-    pipe_through(:ostatus_no_html)
+    # Note: html format is supported only if static FE is enabled
+    pipe_through(:ostatus_html_xml)
+    get("/users/:nickname/feed", Feed.UserController, :feed, as: :user_feed)
+  end
 
+  scope "/", Pleroma.Web do
+    pipe_through(:accepts_html)
+    get("/notice/:id/embed_player", OStatus.OStatusController, :notice_player)
+  end
+
+  scope "/", Pleroma.Web do
+    pipe_through(:accepts_xml_rss_atom)
     get("/tags/:tag", Feed.TagController, :feed, as: :tag_feed)
   end
 

lib/pleroma/web/static_fe/static_fe_controller.ex

@@ -24,6 +24,7 @@ def show(%{assigns: %{notice_id: notice_id}} = conn, _params) do
     with %Activity{local: true} = activity <-
            Activity.get_by_id_with_object(notice_id),
          true <- Visibility.is_public?(activity.object),
+         {_, true} <- {:visible?, Visibility.visible_for_user?(activity, _reading_user = nil)},
          %User{} = user <- User.get_by_ap_id(activity.object.data["actor"]) do
       meta = Metadata.build_tags(%{activity_id: notice_id, object: activity.object, user: user})
 
@@ -47,8 +48,9 @@ def show(%{assigns: %{notice_id: notice_id}} = conn, _params) do
 
   @doc "Renders public activities of requested user"
   def show(%{assigns: %{username_or_id: username_or_id}} = conn, params) do
-    case User.get_cached_by_nickname_or_id(username_or_id) do
+    with {_, %User{local: true} = user} <-
-      %User{} = user ->
+           {:fetch_user, User.get_cached_by_nickname_or_id(username_or_id)},
+         {_, :visible} <- {:visibility, User.visible_for(user, _reading_user = nil)} do
       meta = Metadata.build_tags(%{user: user})
 
       params =
@@ -74,7 +76,7 @@ def show(%{assigns: %{username_or_id: username_or_id}} = conn, params) do
         next_page_id: next_page_id,
         meta: meta
       })
-
+    else
       _ ->
         not_found(conn, "User not found.")
     end

test/web/activity_pub/activity_pub_controller_test.exs

@@ -33,25 +33,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
 
   setup do: clear_config([:instance, :federating], true)
 
-  defp ensure_federating_or_authenticated(conn, url, user) do
-    Config.put([:instance, :federating], false)
-
-    conn
-    |> get(url)
-    |> response(403)
-
-    conn
-    |> assign(:user, user)
-    |> get(url)
-    |> response(200)
-
-    Config.put([:instance, :federating], true)
-
-    conn
-    |> get(url)
-    |> response(200)
-  end
-
   describe "/relay" do
     setup do: clear_config([:instance, :allow_relay])
 
@@ -175,21 +156,6 @@ test "it returns error when user is not found", %{conn: conn} do
 
       assert response == "Not found"
     end
-
-    test "it requires authentication if instance is NOT federating", %{
-      conn: conn
-    } do
-      user = insert(:user)
-
-      conn =
-        put_req_header(
-          conn,
-          "accept",
-          "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\""
-        )
-
-      ensure_federating_or_authenticated(conn, "/users/#{user.nickname}.json", user)
-    end
   end
 
   describe "mastodon compatibility routes" do
@@ -357,18 +323,6 @@ test "cached purged after object deletion", %{conn: conn} do
 
       assert "Not found" == json_response(conn2, :not_found)
     end
-
-    test "it requires authentication if instance is NOT federating", %{
-      conn: conn
-    } do
-      user = insert(:user)
-      note = insert(:note)
-      uuid = String.split(note.data["id"], "/") |> List.last()
-
-      conn = put_req_header(conn, "accept", "application/activity+json")
-
-      ensure_federating_or_authenticated(conn, "/objects/#{uuid}", user)
-    end
   end
 
   describe "/activities/:uuid" do
@@ -440,18 +394,6 @@ test "cached purged after activity deletion", %{conn: conn} do
 
       assert "Not found" == json_response(conn2, :not_found)
     end
-
-    test "it requires authentication if instance is NOT federating", %{
-      conn: conn
-    } do
-      user = insert(:user)
-      activity = insert(:note_activity)
-      uuid = String.split(activity.data["id"], "/") |> List.last()
-
-      conn = put_req_header(conn, "accept", "application/activity+json")
-
-      ensure_federating_or_authenticated(conn, "/activities/#{uuid}", user)
-    end
   end
 
   describe "/inbox" do
@@ -912,15 +854,6 @@ test "it returns an announce activity in a collection", %{conn: conn} do
 
       assert response(conn, 200) =~ announce_activity.data["object"]
     end
-
-    test "it requires authentication if instance is NOT federating", %{
-      conn: conn
-    } do
-      user = insert(:user)
-      conn = put_req_header(conn, "accept", "application/activity+json")
-
-      ensure_federating_or_authenticated(conn, "/users/#{user.nickname}/outbox", user)
-    end
   end
 
   describe "POST /users/:nickname/outbox (C2S)" do

test/web/feed/tag_controller_test.exs

@@ -8,6 +8,7 @@ defmodule Pleroma.Web.Feed.TagControllerTest do
   import Pleroma.Factory
   import SweetXml
 
+  alias Pleroma.Config
   alias Pleroma.Object
   alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.Feed.FeedView
@@ -15,7 +16,7 @@ defmodule Pleroma.Web.Feed.TagControllerTest do
   setup do: clear_config([:feed])
 
   test "gets a feed (ATOM)", %{conn: conn} do
-    Pleroma.Config.put(
+    Config.put(
       [:feed, :post_title],
       %{max_length: 25, omission: "..."}
     )
@@ -82,7 +83,7 @@ test "gets a feed (ATOM)", %{conn: conn} do
   end
 
   test "gets a feed (RSS)", %{conn: conn} do
-    Pleroma.Config.put(
+    Config.put(
       [:feed, :post_title],
       %{max_length: 25, omission: "..."}
     )
@@ -157,7 +158,7 @@ test "gets a feed (RSS)", %{conn: conn} do
     response =
       conn
       |> put_req_header("accept", "application/rss+xml")
-      |> get(tag_feed_path(conn, :feed, "pleromaart"))
+      |> get(tag_feed_path(conn, :feed, "pleromaart.rss"))
       |> response(200)
 
     xml = parse(response)
@@ -183,14 +184,12 @@ test "gets a feed (RSS)", %{conn: conn} do
   end
 
   describe "private instance" do
-    setup do: clear_config([:instance, :public])
+    setup do: clear_config([:instance, :public], false)
 
     test "returns 404 for tags feed", %{conn: conn} do
-      Config.put([:instance, :public], false)
-
       conn
       |> put_req_header("accept", "application/rss+xml")
-      |> get(tag_feed_path(conn, :feed, "pleromaart"))
+      |> get(tag_feed_path(conn, :feed, "pleromaart.rss"))
       |> response(404)
     end
   end