From e2bf6b1f7ee6115a7eafa78272bdbafe7f4789c5 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Mon, 16 Nov 2020 19:22:32 +0300 Subject: [PATCH] fix for forwarded reports --- lib/pleroma/emails/admin_email.ex | 3 + lib/pleroma/web/activity_pub/activity_pub.ex | 35 +++++---- .../activity_pub_controller_test.exs | 76 +++++++++++++++++++ .../web/activity_pub/activity_pub_test.exs | 25 ++++++ test/support/factory.ex | 32 ++++++-- 5 files changed, 150 insertions(+), 21 deletions(-) diff --git a/lib/pleroma/emails/admin_email.ex b/lib/pleroma/emails/admin_email.ex index 02274554f..d5757c12a 100644 --- a/lib/pleroma/emails/admin_email.ex +++ b/lib/pleroma/emails/admin_email.ex @@ -48,6 +48,9 @@ def report(to, reporter, account, statuses, comment) do status_url = Helpers.o_status_url(Pleroma.Web.Endpoint, :notice, id) "
  • #{status_url}
  • " + %{"id" => id} when is_binary(id) -> + "
  • #{id}
  • " + id when is_binary(id) -> "
  • #{id}
  • " end) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 35f71b7ae..8f3ce1343 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -332,15 +332,21 @@ defp do_unfollow(follower, followed, activity_id, local) do end @spec flag(map()) :: {:ok, Activity.t()} | {:error, any()} - def flag( - %{ - actor: actor, - context: _context, - account: account, - statuses: statuses, - content: content - } = params - ) do + def flag(params) do + with {:ok, result} <- Repo.transaction(fn -> do_flag(params) end) do + result + end + end + + defp do_flag( + %{ + actor: actor, + context: _context, + account: account, + statuses: statuses, + content: content + } = params + ) do # only accept false as false value local = !(params[:local] == false) forward = !(params[:forward] == false) @@ -358,7 +364,8 @@ def flag( {:ok, activity} <- insert(flag_data, local), {:ok, stripped_activity} <- strip_report_status_data(activity), _ <- notify_and_stream(activity), - :ok <- maybe_federate(stripped_activity) do + :ok <- + maybe_federate(stripped_activity) do User.all_superusers() |> Enum.filter(fn user -> not is_nil(user.email) end) |> Enum.each(fn superuser -> @@ -368,6 +375,8 @@ def flag( end) {:ok, activity} + else + {:error, error} -> Repo.rollback(error) end end @@ -791,10 +800,10 @@ defp restrict_replies(query, %{ where: fragment( """ - ?->>'type' != 'Create' -- This isn't a Create + ?->>'type' != 'Create' -- This isn't a Create OR ?->>'inReplyTo' is null -- this isn't a reply - OR ? && array_remove(?, ?) -- The recipient is us or one of our friends, - -- unless they are the author (because authors + OR ? && array_remove(?, ?) -- The recipient is us or one of our friends, + -- unless they are the author (because authors -- are also part of the recipients). This leads -- to a bug that self-replies by friends won't -- show up. diff --git a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs index 31e48f87f..49eeaeaff 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -799,6 +799,82 @@ test "it requires authentication", %{conn: conn} do assert json_response(ret_conn, 200) end + + test "forwarded report", %{conn: conn} do + admin = insert(:user, is_admin: true) + actor = insert(:user, local: false) + remote_domain = URI.parse(actor.ap_id).host + reported_user = insert(:user) + + note = insert(:note_activity, user: reported_user) + + data = %{ + "@context" => [ + "https://www.w3.org/ns/activitystreams", + "https://#{remote_domain}/schemas/litepub-0.1.jsonld", + %{ + "@language" => "und" + } + ], + "actor" => actor.ap_id, + "cc" => [ + reported_user.ap_id + ], + "content" => "test", + "context" => "context", + "id" => "http://#{remote_domain}/activities/02be56cf-35e3-46b4-b2c6-47ae08dfee9e", + "nickname" => reported_user.nickname, + "object" => [ + reported_user.ap_id, + %{ + "actor" => %{ + "actor_type" => "Person", + "approval_pending" => false, + "avatar" => "", + "confirmation_pending" => false, + "deactivated" => false, + "display_name" => "test user", + "id" => reported_user.id, + "local" => false, + "nickname" => reported_user.nickname, + "registration_reason" => nil, + "roles" => %{ + "admin" => false, + "moderator" => false + }, + "tags" => [], + "url" => reported_user.ap_id + }, + "content" => "", + "id" => note.data["id"], + "published" => note.data["published"], + "type" => "Note" + } + ], + "published" => note.data["published"], + "state" => "open", + "to" => [], + "type" => "Flag" + } + + conn + |> assign(:valid_signature, true) + |> put_req_header("content-type", "application/activity+json") + |> post("/users/#{reported_user.nickname}/inbox", data) + |> json_response(200) + + ObanHelpers.perform(all_enqueued(worker: ReceiverWorker)) + + assert Pleroma.Repo.aggregate(Activity, :count, :id) == 2 + + ObanHelpers.perform_all() + + Swoosh.TestAssertions.assert_email_sent( + to: {admin.name, admin.email}, + html_body: ~r/Reported Account:/i + ) + + end end describe "GET /users/:nickname/outbox" do diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs index 3eeb0f735..6cc25dd9e 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -1298,6 +1298,31 @@ test "it can create a Flag activity", assert_called(Utils.maybe_federate(%{activity | data: new_data})) end + + test_with_mock "reverts on error", + %{ + reporter: reporter, + context: context, + target_account: target_account, + reported_activity: reported_activity, + content: content + }, + Utils, + [:passthrough], + maybe_federate: fn _ -> {:error, :reverted} end do + assert {:error, :reverted} = + ActivityPub.flag(%{ + actor: reporter, + context: context, + account: target_account, + statuses: [reported_activity], + content: content + }) + + assert Repo.aggregate(Activity, :count, :id) == 1 + assert Repo.aggregate(Object, :count, :id) == 2 + assert Repo.aggregate(Notification, :count, :id) == 0 + end end test "fetch_activities/2 returns activities addressed to a list " do diff --git a/test/support/factory.ex b/test/support/factory.ex index 80b882ee4..8eb07dc3c 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -24,7 +24,7 @@ def conversation_factory do } end - def user_factory do + def user_factory(attrs \\ %{}) do user = %User{ name: sequence(:name, &"Test ใƒ†ใ‚นใƒˆ User #{&1}"), email: sequence(:email, &"user#{&1}@example.com"), @@ -39,13 +39,29 @@ def user_factory do ap_enabled: true } - %{ - user - | ap_id: User.ap_id(user), - follower_address: User.ap_followers(user), - following_address: User.ap_following(user), - raw_bio: user.bio - } + urls = + if attrs[:local] == false do + base_domain = Enum.random(["domain1.com", "domain2.com", "domain3.com"]) + + ap_id = "https://#{base_domain}/users/#{user.nickname}" + + %{ + ap_id: ap_id, + follower_address: ap_id <> "/followers", + following_address: ap_id <> "/following" + } + else + %{ + ap_id: User.ap_id(user), + follower_address: User.ap_followers(user), + following_address: User.ap_following(user) + } + end + + user + |> Map.put(:raw_bio, user.bio) + |> Map.merge(urls) + |> merge_attributes(attrs) end def user_relationship_factory(attrs \\ %{}) do