Merge branch 'bugfix/remove-others-follower-collections' into 'develop'

Remove all follower collections but actor's from "cc"

Closes #858

See merge request pleroma/pleroma!1225

3 files changed, 86 insertions, 24 deletions

diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex
index d8fa2728d..f18ffdee6 100644
--- a/lib/pleroma/web/activity_pub/transmogrifier.ex
+++ b/lib/pleroma/web/activity_pub/transmogrifier.ex
@@ -65,7 +65,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     end
   end
 
-  def fix_explicit_addressing(%{"to" => to, "cc" => cc} = object, explicit_mentions) do
+  def fix_explicit_addressing(
+        %{"to" => to, "cc" => cc} = object,
+        explicit_mentions,
+        follower_collection
+      ) do
     explicit_to =
       to
       |> Enum.filter(fn x -> x in explicit_mentions end)
@@ -76,6 +80,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
 
     final_cc =
       (cc ++ explicit_cc)
+      |> Enum.reject(fn x -> String.ends_with?(x, "/followers") and x != follower_collection end)
       |> Enum.uniq()
 
     object
@@ -83,7 +88,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     |> Map.put("cc", final_cc)
   end
 
-  def fix_explicit_addressing(object, _explicit_mentions), do: object
+  def fix_explicit_addressing(object, _explicit_mentions, _followers_collection), do: object
 
   # if directMessage flag is set to true, leave the addressing alone
   def fix_explicit_addressing(%{"directMessage" => true} = object), do: object
@@ -98,8 +103,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     explicit_mentions =
       explicit_mentions ++ ["https://www.w3.org/ns/activitystreams#Public", follower_collection]
 
-    object
-    |> fix_explicit_addressing(explicit_mentions)
+    fix_explicit_addressing(object, explicit_mentions, follower_collection)
   end
 
   # if as:Public is addressed, then make sure the followers collection is also addressed
@@ -136,7 +140,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
     |> fix_addressing_list("cc")
     |> fix_addressing_list("bto")
     |> fix_addressing_list("bcc")
-    |> fix_explicit_addressing
+    |> fix_explicit_addressing()
     |> fix_implicit_addressing(followers_collection)
   end
 
diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs
index 30adfda36..8b3233729 100644
--- a/test/web/activity_pub/activity_pub_controller_test.exs
+++ b/test/web/activity_pub/activity_pub_controller_test.exs
@@ -11,6 +11,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ObjectView
   alias Pleroma.Web.ActivityPub.UserView
+  alias Pleroma.Web.ActivityPub.Utils
 
   setup_all do
     Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
@@ -234,13 +235,17 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
   end
 
   describe "/users/:nickname/inbox" do
-    test "it inserts an incoming activity into the database", %{conn: conn} do
-      user = insert(:user)
-
+    setup do
       data =
         File.read!("test/fixtures/mastodon-post-activity.json")
         |> Poison.decode!()
-        |> Map.put("bcc", [user.ap_id])
+
+      [data: data]
+    end
+
+    test "it inserts an incoming activity into the database", %{conn: conn, data: data} do
+      user = insert(:user)
+      data = Map.put(data, "bcc", [user.ap_id])
 
       conn =
         conn
@@ -253,16 +258,15 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       assert Activity.get_by_ap_id(data["id"])
     end
 
-    test "it accepts messages from actors that are followed by the user", %{conn: conn} do
+    test "it accepts messages from actors that are followed by the user", %{
+      conn: conn,
+      data: data
+    } do
       recipient = insert(:user)
       actor = insert(:user, %{ap_id: "http://mastodon.example.org/users/actor"})
 
       {:ok, recipient} = User.follow(recipient, actor)
 
-      data =
-        File.read!("test/fixtures/mastodon-post-activity.json")
-        |> Poison.decode!()
-
       object =
         data["object"]
         |> Map.put("attributedTo", actor.ap_id)
@@ -309,13 +313,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       assert response(conn, 200) =~ note_activity.data["object"]["content"]
     end
 
-    test "it clears `unreachable` federation status of the sender", %{conn: conn} do
+    test "it clears `unreachable` federation status of the sender", %{conn: conn, data: data} do
       user = insert(:user)
-
-      data =
-        File.read!("test/fixtures/mastodon-post-activity.json")
-        |> Poison.decode!()
-        |> Map.put("bcc", [user.ap_id])
+      data = Map.put(data, "bcc", [user.ap_id])
 
       sender_host = URI.parse(data["actor"]).host
       Instances.set_consistently_unreachable(sender_host)
@@ -330,6 +330,47 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
       assert "ok" == json_response(conn, 200)
       assert Instances.reachable?(sender_host)
     end
+
+    test "it removes all follower collections but actor's", %{conn: conn} do
+      [actor, recipient] = insert_pair(:user)
+
+      data =
+        File.read!("test/fixtures/activitypub-client-post-activity.json")
+        |> Poison.decode!()
+
+      object = Map.put(data["object"], "attributedTo", actor.ap_id)
+
+      data =
+        data
+        |> Map.put("id", Utils.generate_object_id())
+        |> Map.put("actor", actor.ap_id)
+        |> Map.put("object", object)
+        |> Map.put("cc", [
+          recipient.follower_address,
+          actor.follower_address
+        ])
+        |> Map.put("to", [
+          recipient.ap_id,
+          recipient.follower_address,
+          "https://www.w3.org/ns/activitystreams#Public"
+        ])
+
+      conn
+      |> assign(:valid_signature, true)
+      |> put_req_header("content-type", "application/activity+json")
+      |> post("/users/#{recipient.nickname}/inbox", data)
+      |> json_response(200)
+
+      activity = Activity.get_by_ap_id(data["id"])
+
+      assert activity.id
+      assert actor.follower_address in activity.recipients
+      assert actor.follower_address in activity.data["cc"]
+
+      refute recipient.follower_address in activity.recipients
+      refute recipient.follower_address in activity.data["cc"]
+      refute recipient.follower_address in activity.data["to"]
+    end
   end
 
   describe "/users/:nickname/outbox" do
diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs
index ee71de8d0..bcc460f1c 100644
--- a/test/web/activity_pub/transmogrifier_test.exs
+++ b/test/web/activity_pub/transmogrifier_test.exs
@@ -1211,9 +1211,12 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
   end
 
   describe "fix_explicit_addressing" do
-    test "moves non-explicitly mentioned actors to cc" do
+    setup do
       user = insert(:user)
+      [user: user]
+    end
 
+    test "moves non-explicitly mentioned actors to cc", %{user: user} do
       explicitly_mentioned_actors = [
         "https://pleroma.gold/users/user1",
         "https://pleroma.gold/user2"
@@ -1235,9 +1238,7 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
       assert "https://social.beepboop.ga/users/dirb" in fixed_object["cc"]
     end
 
-    test "does not move actor's follower collection to cc" do
-      user = insert(:user)
-
+    test "does not move actor's follower collection to cc", %{user: user} do
       object = %{
         "actor" => user.ap_id,
         "to" => [user.follower_address],
@@ -1248,5 +1249,21 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do
       assert user.follower_address in fixed_object["to"]
       refute user.follower_address in fixed_object["cc"]
     end
+
+    test "removes recipient's follower collection from cc", %{user: user} do
+      recipient = insert(:user)
+
+      object = %{
+        "actor" => user.ap_id,
+        "to" => [recipient.ap_id, "https://www.w3.org/ns/activitystreams#Public"],
+        "cc" => [user.follower_address, recipient.follower_address]
+      }
+
+      fixed_object = Transmogrifier.fix_explicit_addressing(object)
+
+      assert user.follower_address in fixed_object["cc"]
+      refute recipient.follower_address in fixed_object["cc"]
+      refute recipient.follower_address in fixed_object["to"]
+    end
   end
 end