Merge pull request 'MRF ObjectAgePolicy: keep old DMs and followers-only posts private' (#8019) from lambadalambda/pleroma:fix/object-age-non-public into develop

Reviewed-on: https://git.pleroma.social/pleroma/pleroma/pulls/8019

changelog.d/object-age-non-public.security

@@ -0,0 +1 @@
+ObjectAgePolicy no longer makes old DMs and followers-only posts public

lib/pleroma/web/activity_pub/mrf/object_age_policy.ex

@@ -36,16 +36,34 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy do
     end
   end
 
+  @spec delete_and_count(list(), term()) :: {integer(), list()}
+  defp delete_and_count(list, element), do: delete_and_count(list, element, {0, [], list})
+
+  defp delete_and_count([], _element, {0, _nlist, olist}), do: {0, olist}
+  defp delete_and_count([], _element, {count, nlist, _olist}), do: {count, Enum.reverse(nlist)}
+
+  defp delete_and_count([h | r], h, {count, nlist, olist}),
+    do: delete_and_count(r, h, {count + 1, nlist, olist})
+
+  defp delete_and_count([h | r], element, {count, nlist, olist}),
+    do: delete_and_count(r, element, {count, [h | nlist], olist})
+
+  defp insert_if_needed(list, oldcount, element) do
+    if oldcount <= 0 || Enum.member?(list, element) do
+      list
+    else
+      [element | list]
+    end
+  end
+
   defp check_delist(activity, actions) do
     if :delist in actions do
       with %User{} = user <- User.get_cached_by_ap_id(activity["actor"]) do
-        to =
-          List.delete(activity["to"] || [], Pleroma.Constants.as_public()) ++
-            [user.follower_address]
+        {pubcnt, to} = delete_and_count(activity["to"] || [], Pleroma.Constants.as_public())
+        {flwcnt, cc} = delete_and_count(activity["cc"] || [], user.follower_address)
 
-        cc =
-          List.delete(activity["cc"] || [], user.follower_address) ++
-            [Pleroma.Constants.as_public()]
+        cc = insert_if_needed(cc, pubcnt, Pleroma.Constants.as_public())
+        to = insert_if_needed(to, flwcnt, user.follower_address)
 
         activity =
           activity
@@ -67,8 +85,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy do
   defp check_strip_followers(activity, actions) do
     if :strip_followers in actions do
       with %User{} = user <- User.get_cached_by_ap_id(activity["actor"]) do
-        to = List.delete(activity["to"] || [], user.follower_address)
-        cc = List.delete(activity["cc"] || [], user.follower_address)
+        {_, to} = delete_and_count(activity["to"] || [], user.follower_address)
+        {_, cc} = delete_and_count(activity["cc"] || [], user.follower_address)
 
         activity =
           activity

test/pleroma/web/activity_pub/mrf/object_age_policy_test.exs

@@ -8,6 +8,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicyTest do
   alias Pleroma.Web.ActivityPub.MRF.ObjectAgePolicy
   alias Pleroma.Web.ActivityPub.Visibility
 
+  require Pleroma.Constants
+
   setup do:
           clear_config(:mrf_object_age,
             threshold: 172_800,
@@ -79,7 +81,7 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicyTest do
 
       {:ok, data} = ObjectAgePolicy.filter(data)
 
-      assert Visibility.get_visibility(%{data: data}) == "unlisted"
+      assert Visibility.get_visibility(%{data: data}) == "direct"
     end
 
     test "it delists an old post" do
@@ -94,6 +96,90 @@ defmodule Pleroma.Web.ActivityPub.MRF.ObjectAgePolicyTest do
       assert Visibility.get_visibility(%{data: data}) == "unlisted"
     end
 
+    test "it keeps an old followers-only post private" do
+      clear_config([:mrf_object_age, :actions], [:delist])
+
+      data = get_old_message()
+      {:ok, user} = User.get_or_fetch_by_ap_id(data["actor"])
+
+      data =
+        data
+        |> Map.put("to", [user.follower_address])
+        |> Map.put("cc", [])
+        |> put_in(["object", "to"], [user.follower_address])
+        |> put_in(["object", "cc"], [])
+
+      {:ok, data} = ObjectAgePolicy.filter(data)
+
+      assert data["to"] == [user.follower_address]
+      assert data["cc"] == []
+      assert Visibility.get_visibility(%{data: data}) == "private"
+      assert data["object"]["to"] == data["to"]
+      assert data["object"]["cc"] == data["cc"]
+    end
+
+    test "it keeps an old direct message direct" do
+      clear_config([:mrf_object_age, :actions], [:delist])
+
+      data = get_old_message()
+      {:ok, _user} = User.get_or_fetch_by_ap_id(data["actor"])
+      recipient = "https://example.com/users/someone"
+
+      data =
+        data
+        |> Map.put("to", [recipient])
+        |> Map.put("cc", [])
+        |> put_in(["object", "to"], [recipient])
+        |> put_in(["object", "cc"], [])
+
+      {:ok, data} = ObjectAgePolicy.filter(data)
+
+      assert data["to"] == [recipient]
+      assert data["cc"] == []
+      assert Visibility.get_visibility(%{data: data}) == "direct"
+    end
+
+    test "it removes every copy of the follower address" do
+      clear_config([:mrf_object_age, :actions], [:strip_followers])
+
+      data = get_old_message()
+      {:ok, user} = User.get_or_fetch_by_ap_id(data["actor"])
+      public = Pleroma.Constants.as_public()
+      followers = [user.follower_address, user.follower_address]
+
+      data =
+        data
+        |> Map.put("to", [public])
+        |> Map.put("cc", followers)
+        |> put_in(["object", "to"], [public])
+        |> put_in(["object", "cc"], followers)
+
+      {:ok, data} = ObjectAgePolicy.filter(data)
+
+      assert data["to"] == [public]
+      assert data["cc"] == []
+    end
+
+    test "it does not duplicate addresses of an old unlisted post" do
+      clear_config([:mrf_object_age, :actions], [:delist, :strip_followers])
+
+      data = get_old_message()
+      {:ok, user} = User.get_or_fetch_by_ap_id(data["actor"])
+      public = Pleroma.Constants.as_public()
+
+      data =
+        data
+        |> Map.put("to", [user.follower_address])
+        |> Map.put("cc", [public])
+        |> put_in(["object", "to"], [user.follower_address])
+        |> put_in(["object", "cc"], [public])
+
+      {:ok, data} = ObjectAgePolicy.filter(data)
+
+      assert data["to"] == []
+      assert data["cc"] == [public]
+    end
+
     test "it allows a new post" do
       clear_config([:mrf_object_age, :actions], [:delist])