From b6fc98d9cd3a32b39606c65cb4f298d280e2537c Mon Sep 17 00:00:00 2001
From: Ivan Tashkinov <ivantashkinov@gmail.com>
Date: Tue, 3 Mar 2020 22:22:02 +0300
Subject: [PATCH] [#1560] ActivityPubController federation state restrictions
 adjustments. Adjusted tests.

---
 lib/pleroma/plugs/federating_plug.ex          |  4 +-
 .../activity_pub/activity_pub_controller.ex   | 44 ++++++++++++++-----
 lib/pleroma/web/router.ex                     |  3 ++
 .../activity_pub_controller_test.exs          | 29 ++----------
 4 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/lib/pleroma/plugs/federating_plug.ex b/lib/pleroma/plugs/federating_plug.ex
index 4dc4e9279..4c5aca3e9 100644
--- a/lib/pleroma/plugs/federating_plug.ex
+++ b/lib/pleroma/plugs/federating_plug.ex
@@ -10,7 +10,7 @@ def init(options) do
   end
 
   def call(conn, _opts) do
-    if Pleroma.Config.get([:instance, :federating]) do
+    if federating?() do
       conn
     else
       conn
@@ -20,4 +20,6 @@ def call(conn, _opts) do
       |> halt()
     end
   end
+
+  def federating?, do: Pleroma.Config.get([:instance, :federating])
 end
diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
index aee574262..e1984f88f 100644
--- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
@@ -18,19 +18,31 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
   alias Pleroma.Web.ActivityPub.UserView
   alias Pleroma.Web.ActivityPub.Utils
   alias Pleroma.Web.ActivityPub.Visibility
+  alias Pleroma.Web.FederatingPlug
   alias Pleroma.Web.Federator
 
   require Logger
 
   action_fallback(:errors)
 
+  # Note: some of the following actions (like :update_inbox) may be server-to-server as well
+  @client_to_server_actions [
+    :whoami,
+    :read_inbox,
+    :update_outbox,
+    :upload_media,
+    :followers,
+    :following
+  ]
+
+  plug(FederatingPlug when action not in @client_to_server_actions)
+
   plug(
     Pleroma.Plugs.Cache,
     [query_params: false, tracking_fun: &__MODULE__.track_object_fetch/2]
     when action in [:activity, :object]
   )
 
-  plug(Pleroma.Web.FederatingPlug)
   plug(:set_requester_reachable when action in [:inbox])
   plug(:relay_active? when action in [:relay])
 
@@ -255,8 +267,16 @@ def inbox(%{assigns: %{valid_signature: true}} = conn, params) do
     json(conn, "ok")
   end
 
-  # only accept relayed Creates
-  def inbox(conn, %{"type" => "Create"} = params) do
+  # POST /relay/inbox -or- POST /internal/fetch/inbox
+  def inbox(conn, params) do
+    if params["type"] == "Create" && FederatingPlug.federating?() do
+      post_inbox_relayed_create(conn, params)
+    else
+      post_inbox_fallback(conn, params)
+    end
+  end
+
+  defp post_inbox_relayed_create(conn, params) do
     Logger.debug(
       "Signature missing or not from author, relayed Create message, fetching object from source"
     )
@@ -266,7 +286,7 @@ def inbox(conn, %{"type" => "Create"} = params) do
     json(conn, "ok")
   end
 
-  def inbox(conn, params) do
+  defp post_inbox_fallback(conn, params) do
     headers = Enum.into(conn.req_headers, %{})
 
     if String.contains?(headers["signature"], params["actor"]) do
@@ -314,7 +334,7 @@ def whoami(%{assigns: %{user: %User{} = user}} = conn, _params) do
   def whoami(_conn, _params), do: {:error, :not_found}
 
   def read_inbox(
-        %{assigns: %{user: %{nickname: nickname} = user}} = conn,
+        %{assigns: %{user: %User{nickname: nickname} = user}} = conn,
         %{"nickname" => nickname, "page" => page?} = params
       )
       when page? in [true, "true"] do
@@ -337,7 +357,7 @@ def read_inbox(
     })
   end
 
-  def read_inbox(%{assigns: %{user: %{nickname: nickname} = user}} = conn, %{
+  def read_inbox(%{assigns: %{user: %User{nickname: nickname} = user}} = conn, %{
         "nickname" => nickname
       }) do
     with {:ok, user} <- User.ensure_keys_present(user) do
@@ -356,7 +376,7 @@ def read_inbox(%{assigns: %{user: nil}} = conn, %{"nickname" => nickname}) do
     |> json(err)
   end
 
-  def read_inbox(%{assigns: %{user: %{nickname: as_nickname}}} = conn, %{
+  def read_inbox(%{assigns: %{user: %User{nickname: as_nickname}}} = conn, %{
         "nickname" => nickname
       }) do
     err =
@@ -370,7 +390,7 @@ def read_inbox(%{assigns: %{user: %{nickname: as_nickname}}} = conn, %{
     |> json(err)
   end
 
-  def handle_user_activity(user, %{"type" => "Create"} = params) do
+  def handle_user_activity(%User{} = user, %{"type" => "Create"} = params) do
     object =
       params["object"]
       |> Map.merge(Map.take(params, ["to", "cc"]))
@@ -386,7 +406,7 @@ def handle_user_activity(user, %{"type" => "Create"} = params) do
     })
   end
 
-  def handle_user_activity(user, %{"type" => "Delete"} = params) do
+  def handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do
     with %Object{} = object <- Object.normalize(params["object"]),
          true <- user.is_moderator || user.ap_id == object.data["actor"],
          {:ok, delete} <- ActivityPub.delete(object) do
@@ -396,7 +416,7 @@ def handle_user_activity(user, %{"type" => "Delete"} = params) do
     end
   end
 
-  def handle_user_activity(user, %{"type" => "Like"} = params) do
+  def handle_user_activity(%User{} = user, %{"type" => "Like"} = params) do
     with %Object{} = object <- Object.normalize(params["object"]),
          {:ok, activity, _object} <- ActivityPub.like(user, object) do
       {:ok, activity}
@@ -434,7 +454,7 @@ def update_outbox(
     end
   end
 
-  def update_outbox(%{assigns: %{user: user}} = conn, %{"nickname" => nickname} = _) do
+  def update_outbox(%{assigns: %{user: %User{} = user}} = conn, %{"nickname" => nickname}) do
     err =
       dgettext("errors", "can't update outbox of %{nickname} as %{as_nickname}",
         nickname: nickname,
@@ -492,7 +512,7 @@ defp ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user) do
   - HTTP Code: 201 Created
   - HTTP Body: ActivityPub object to be inserted into another's `attachment` field
   """
-  def upload_media(%{assigns: %{user: user}} = conn, %{"file" => file} = data) do
+  def upload_media(%{assigns: %{user: %User{} = user}} = conn, %{"file" => file} = data) do
     with {:ok, object} <-
            ActivityPub.upload(
              file,
diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex
index 980242c68..5f3a06caa 100644
--- a/lib/pleroma/web/router.ex
+++ b/lib/pleroma/web/router.ex
@@ -541,6 +541,7 @@ defmodule Pleroma.Web.Router do
     get("/mailer/unsubscribe/:token", Mailer.SubscriptionController, :unsubscribe)
   end
 
+  # Server to Server (S2S) AP interactions
   pipeline :activitypub do
     plug(:accepts, ["activity+json", "json"])
     plug(Pleroma.Web.Plugs.HTTPSignaturePlug)
@@ -554,6 +555,7 @@ defmodule Pleroma.Web.Router do
     get("/users/:nickname/outbox", ActivityPubController, :outbox)
   end
 
+  # Client to Server (C2S) AP interactions
   pipeline :activitypub_client do
     plug(:accepts, ["activity+json", "json"])
     plug(:fetch_session)
@@ -568,6 +570,7 @@ defmodule Pleroma.Web.Router do
     plug(Pleroma.Plugs.EnsureUserKeyPlug)
   end
 
+  # Note: propagate _any_ updates to `@client_to_server_actions` in `ActivityPubController`
   scope "/", Pleroma.Web.ActivityPub do
     pipe_through([:activitypub_client])
 
diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs
index af0417406..b853474d4 100644
--- a/test/web/activity_pub/activity_pub_controller_test.exs
+++ b/test/web/activity_pub/activity_pub_controller_test.exs
@@ -775,7 +775,7 @@ test "it returns the followers in a collection", %{conn: conn} do
       assert result["first"]["orderedItems"] == [user.ap_id]
     end
 
-    test "it returns returns a uri if the user has 'hide_followers' set", %{conn: conn} do
+    test "it returns a uri if the user has 'hide_followers' set", %{conn: conn} do
       user = insert(:user)
       user_two = insert(:user, hide_followers: true)
       User.follow(user, user_two)
@@ -1060,14 +1060,8 @@ test "returns 404 for GET routes", %{conn: conn} do
       get_uris = [
         "/users/#{user.nickname}",
         "/users/#{user.nickname}/outbox",
-        "/users/#{user.nickname}/inbox?page=true",
-        "/users/#{user.nickname}/followers",
-        "/users/#{user.nickname}/following",
         "/internal/fetch",
-        "/relay",
-        "/relay/following",
-        "/relay/followers",
-        "/api/ap/whoami"
+        "/relay"
       ]
 
       for get_uri <- get_uris do
@@ -1098,8 +1092,7 @@ test "returns 404 for activity-related POST routes", %{conn: conn} do
       post_activity_uris = [
         "/inbox",
         "/relay/inbox",
-        "/users/#{user.nickname}/inbox",
-        "/users/#{user.nickname}/outbox"
+        "/users/#{user.nickname}/inbox"
       ]
 
       for post_activity_uri <- post_activity_uris do
@@ -1113,21 +1106,5 @@ test "returns 404 for activity-related POST routes", %{conn: conn} do
         |> json_response(404)
       end
     end
-
-    test "returns 404 for media upload attempt", %{conn: conn} do
-      user = insert(:user)
-      desc = "Description of the image"
-
-      image = %Plug.Upload{
-        content_type: "image/jpg",
-        path: Path.absname("test/fixtures/image.jpg"),
-        filename: "an_image.jpg"
-      }
-
-      conn
-      |> assign(:user, user)
-      |> post("/api/ap/upload_media", %{"file" => image, "description" => desc})
-      |> json_response(404)
-    end
   end
 end