From b5c97e9ee07545486afb5d5bba3081b427a65334 Mon Sep 17 00:00:00 2001 From: tusooa Date: Tue, 28 Feb 2023 09:01:18 -0500 Subject: [PATCH] Put strip and anonymize process in prepare_outgoing It is not useful to call maybe_federate() with the processed activity, because it will only record the activity id, and put it into the queue. When the job is invoked, it reads from the database for the activity. This means the changes we just made will be discarded. In this commit, I moved the stripping and anonymizing procedures to Transmogrifier.prepare_outgoing, which is called after the federator reads the activity from the database. --- lib/pleroma/web/activity_pub/activity_pub.ex | 5 +- .../web/activity_pub/transmogrifier.ex | 8 +++ lib/pleroma/web/activity_pub/utils.ex | 14 +++-- .../web/activity_pub/activity_pub_test.exs | 63 ------------------- .../web/activity_pub/transmogrifier_test.exs | 63 +++++++++++++++++++ 5 files changed, 82 insertions(+), 71 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 7bf501f77..a25cc1e03 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -414,11 +414,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do with flag_data <- make_flag_data(params, additional), {:ok, activity} <- insert(flag_data, local), - {:ok, stripped_activity} <- strip_report_status_data(activity), - stripped_activity <- maybe_anonymize_reporter(stripped_activity), _ <- notify_and_stream(activity), - :ok <- - maybe_federate(stripped_activity) do + :ok <- maybe_federate(activity) do User.all_users_with_privilege(:reports_manage_reports) |> Enum.filter(fn user -> user.ap_id != actor end) |> Enum.filter(fn user -> not is_nil(user.email) end) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 6517f5eff..0f8e01396 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -891,6 +891,14 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do end end + def prepare_outgoing(%{"type" => "Flag"} = data) do + with {:ok, stripped_activity} <- Utils.strip_report_status_data(data), + stripped_activity <- Utils.maybe_anonymize_reporter(stripped_activity), + stripped_activity <- Map.merge(stripped_activity, Utils.make_json_ld_header()) do + {:ok, stripped_activity} + end + end + def prepare_outgoing(%{"type" => _type} = data) do data = data diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 0b989d02d..807ec8710 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -859,8 +859,14 @@ defmodule Pleroma.Web.ActivityPub.Utils do def update_report_state(_, _), do: {:error, "Unsupported state"} - def strip_report_status_data(activity) do - [actor | reported_activities] = activity.data["object"] + def strip_report_status_data(%Activity{} = activity) do + with {:ok, new_data} <- strip_report_status_data(activity.data) do + {:ok, %{activity | data: new_data}} + end + end + + def strip_report_status_data(data) do + [actor | reported_activities] = data["object"] stripped_activities = Enum.reduce(reported_activities, [], fn act, acc -> @@ -870,9 +876,9 @@ defmodule Pleroma.Web.ActivityPub.Utils do end end) - new_data = put_in(activity.data, ["object"], [actor | stripped_activities]) + new_data = put_in(data, ["object"], [actor | stripped_activities]) - {:ok, %{activity | data: new_data}} + {:ok, new_data} end def get_anonymized_reporter do diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index 0eb262fc6..6da60d64b 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -1681,69 +1681,6 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do } = activity end - test_with_mock "strips status data from Flag, before federating it", - %{ - reporter: reporter, - context: context, - target_account: target_account, - reported_activity: reported_activity, - object_ap_id: object_ap_id, - content: content - }, - Utils, - [:passthrough], - [] do - {:ok, activity} = - ActivityPub.flag(%{ - actor: reporter, - context: context, - account: target_account, - statuses: [reported_activity], - content: content - }) - - new_data = put_in(activity.data, ["object"], [target_account.ap_id, object_ap_id]) - - assert_called(Utils.maybe_federate(%{activity | data: new_data})) - end - - test "anonymize reporter from Flag, before federating it, if configured so", - %{ - reporter: reporter, - context: context, - target_account: target_account, - reported_activity: reported_activity, - object_ap_id: object_ap_id, - content: content - } do - placeholder = insert(:user) - clear_config([:activitypub, :anonymize_reporter], true) - clear_config([:activitypub, :anonymize_reporter_local_nickname], placeholder.nickname) - # Workaround "could not checkout connection" problem - # https://elixirforum.com/t/ecto-timeout-errors-when-wrapping-into-cachex-calls-in-tests/19078/11 - %User{} = User.get_cached_by_nickname(placeholder.nickname) - - with_mock Utils, [:passthrough], [] do - {:ok, activity} = - ActivityPub.flag(%{ - actor: reporter, - context: context, - account: target_account, - statuses: [reported_activity], - content: content - }) - - new_data = - activity.data - |> put_in(["object"], [target_account.ap_id, object_ap_id]) - |> Map.put("actor", placeholder.ap_id) - - assert_called( - Utils.maybe_federate(%{activity | actor: placeholder.ap_id, data: new_data}) - ) - end - end - test_with_mock "reverts on error", %{ reporter: reporter, diff --git a/test/pleroma/web/activity_pub/transmogrifier_test.exs b/test/pleroma/web/activity_pub/transmogrifier_test.exs index e0395d7bb..e34a101b2 100644 --- a/test/pleroma/web/activity_pub/transmogrifier_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier_test.exs @@ -642,6 +642,69 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert [_, _, %{"@language" => "pl"}] = modified["@context"] end + + test "it strips report data" do + reporter = insert(:user) + target_account = insert(:user) + content = "foobar" + {:ok, reported_activity} = CommonAPI.post(target_account, %{status: content}) + context = Utils.generate_context_id() + + object_ap_id = reported_activity.object.data["id"] + + assert {:ok, activity} = + Pleroma.Web.ActivityPub.ActivityPub.flag(%{ + actor: reporter, + context: context, + account: target_account, + statuses: [reported_activity], + content: content + }) + + {:ok, data} = Transmogrifier.prepare_outgoing(activity.data) + + expected_data = + activity.data + |> put_in(["object"], [target_account.ap_id, object_ap_id]) + |> Map.put("actor", reporter.ap_id) + |> Map.merge(Utils.make_json_ld_header()) + + assert data == expected_data + end + + test "it strips report data and anonymize" do + placeholder = insert(:user) + + reporter = insert(:user) + target_account = insert(:user) + content = "foobar" + {:ok, reported_activity} = CommonAPI.post(target_account, %{status: content}) + context = Utils.generate_context_id() + + object_ap_id = reported_activity.object.data["id"] + + assert {:ok, activity} = + Pleroma.Web.ActivityPub.ActivityPub.flag(%{ + actor: reporter, + context: context, + account: target_account, + statuses: [reported_activity], + content: content + }) + + clear_config([:activitypub, :anonymize_reporter], true) + clear_config([:activitypub, :anonymize_reporter_local_nickname], placeholder.nickname) + + {:ok, data} = Transmogrifier.prepare_outgoing(activity.data) + + expected_data = + activity.data + |> put_in(["object"], [target_account.ap_id, object_ap_id]) + |> Map.put("actor", placeholder.ap_id) + |> Map.merge(Utils.make_json_ld_header()) + + assert data == expected_data + end end describe "actor rewriting" do