fix(activitypub): do not require the personal inbox owner to be addressed

Bridgy Fed delivers Follow and Accept to the personal inbox addressed
only to Public. Any server can already address a local user directly,
so the check gave no protection. Ported from Akkoma 2b885288f.

Co-authored-by: Oneric <oneric@oneric.stub>

changelog.d/personal-inbox-addressing.fix

@@ -0,0 +1 @@
+Accept activities delivered to a personal inbox that do not address the inbox owner (fixes Bridgy Fed follows)

lib/pleroma/web/activity_pub/activity_pub_controller.ex

@@ -341,7 +341,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
            {:sender_exists, User.get_or_fetch_by_ap_id(params["actor"])},
          {:recipient_active, true} <- {:recipient_active, recipient.is_active},
          {:sender_active, true} <- {:sender_active, actor.is_active},
-         true <- Utils.recipient_in_message(recipient, actor, params),
          params <- Utils.maybe_splice_recipient(recipient.ap_id, params) do
       Federator.incoming_ap_doc(params)
       json(conn, "ok")

lib/pleroma/web/activity_pub/utils.ex

@@ -78,22 +78,6 @@ defmodule Pleroma.Web.ActivityPub.Utils do
       [params["to"], params["cc"], params["bto"], params["bcc"]]
       |> Enum.any?(&label_in_collection?(label, &1))
 
-  @spec unaddressed_message?(map()) :: boolean()
-  def unaddressed_message?(params),
-    do:
-      [params["to"], params["cc"], params["bto"], params["bcc"]]
-      |> Enum.all?(fn
-        nil -> true
-        [] -> true
-        _ -> false
-      end)
-
-  @spec recipient_in_message(User.t(), User.t(), map()) :: boolean()
-  def recipient_in_message(%User{ap_id: ap_id} = recipient, %User{} = actor, params),
-    do:
-      label_in_message?(ap_id, params) || unaddressed_message?(params) ||
-        User.following?(recipient, actor)
-
   defp extract_list(target) when is_binary(target), do: [target]
   defp extract_list(lst) when is_list(lst), do: lst
   defp extract_list(_), do: []

test/pleroma/web/activity_pub/activity_pub_controller_test.exs

@@ -1872,6 +1872,30 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       assert Activity.get_by_ap_id(data["id"])
     end
 
+    test "it accepts activities that do not address the inbox owner", %{conn: conn} do
+      user = insert(:user)
+      actor = insert(:user, local: false)
+
+      data = %{
+        "@context" => "https://www.w3.org/ns/activitystreams",
+        "type" => "Follow",
+        "id" => "#{actor.ap_id}/follows/1",
+        "actor" => actor.ap_id,
+        "object" => user.ap_id,
+        "to" => ["https://www.w3.org/ns/activitystreams#Public"]
+      }
+
+      conn =
+        conn
+        |> assign_valid_signature_for_actor(data["actor"])
+        |> put_req_header("content-type", "application/activity+json")
+        |> post("/users/#{user.nickname}/inbox", data)
+
+      assert "ok" == json_response(conn, 200)
+      ObanHelpers.perform(all_enqueued(worker: ReceiverWorker))
+      assert Activity.get_by_ap_id(data["id"])
+    end
+
     test "it returns an error when receiving an activity sent to a deactivated user", %{
       conn: conn,
       data: data

test/pleroma/web/activity_pub/utils_test.exs

@@ -406,74 +406,6 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
     end
   end
 
-  describe "recipient_in_message/3" do
-    test "returns true when recipient in `to`" do
-      recipient = insert(:user)
-      actor = insert(:user)
-      assert Utils.recipient_in_message(recipient, actor, %{"to" => recipient.ap_id})
-
-      assert Utils.recipient_in_message(
-               recipient,
-               actor,
-               %{"to" => [recipient.ap_id], "cc" => ""}
-             )
-    end
-
-    test "returns true when recipient in `cc`" do
-      recipient = insert(:user)
-      actor = insert(:user)
-      assert Utils.recipient_in_message(recipient, actor, %{"cc" => recipient.ap_id})
-
-      assert Utils.recipient_in_message(
-               recipient,
-               actor,
-               %{"cc" => [recipient.ap_id], "to" => ""}
-             )
-    end
-
-    test "returns true when recipient in `bto`" do
-      recipient = insert(:user)
-      actor = insert(:user)
-      assert Utils.recipient_in_message(recipient, actor, %{"bto" => recipient.ap_id})
-
-      assert Utils.recipient_in_message(
-               recipient,
-               actor,
-               %{"bcc" => "", "bto" => [recipient.ap_id]}
-             )
-    end
-
-    test "returns true when recipient in `bcc`" do
-      recipient = insert(:user)
-      actor = insert(:user)
-      assert Utils.recipient_in_message(recipient, actor, %{"bcc" => recipient.ap_id})
-
-      assert Utils.recipient_in_message(
-               recipient,
-               actor,
-               %{"bto" => "", "bcc" => [recipient.ap_id]}
-             )
-    end
-
-    test "returns true when message without addresses fields" do
-      recipient = insert(:user)
-      actor = insert(:user)
-      assert Utils.recipient_in_message(recipient, actor, %{"bccc" => recipient.ap_id})
-
-      assert Utils.recipient_in_message(
-               recipient,
-               actor,
-               %{"btod" => "", "bccc" => [recipient.ap_id]}
-             )
-    end
-
-    test "returns false" do
-      recipient = insert(:user)
-      actor = insert(:user)
-      refute Utils.recipient_in_message(recipient, actor, %{"to" => "ap_id"})
-    end
-  end
-
   describe "lazy_put_activity_defaults/2" do
     test "returns map with id and published data" do
       note_activity = insert(:note_activity)