From d788b41c2e348b968ae72be2a347f186cdc2b70c Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 22 Dec 2020 21:32:11 +0100 Subject: [PATCH] Report fixes. --- CHANGELOG.md | 8 ++ lib/pleroma/activity.ex | 11 ++ lib/pleroma/emails/admin_email.ex | 3 + lib/pleroma/web/activity_pub/activity_pub.ex | 35 +++-- lib/pleroma/web/activity_pub/utils.ex | 42 +++--- test/pleroma/activity_test.exs | 16 +++ .../activity_pub_controller_test.exs | 136 ++++++++++++++++++ .../web/activity_pub/activity_pub_test.exs | 25 ++++ test/support/factory.ex | 32 +++-- 9 files changed, 270 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcc27ce28..5f6d96c0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Search: RUM index search speed has been fixed. - Rich Media Previews sometimes showed the wrong preview due to a bug following redirects. - Fixes for the autolinker. +- Forwarded reports duplication from Pleroma instances. + +
+ API +- Statuses were not displayed for Mastodon forwarded reports. + +
+ ## [2.2.0] - 2020-11-12 diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 17af04257..1dc777e3b 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -343,4 +343,15 @@ def pinned_by_actor?(%Activity{} = activity) do actor = user_actor(activity) activity.id in actor.pinned_activities end + + @spec get_by_object_ap_id_with_object(String.t()) :: t() | nil + def get_by_object_ap_id_with_object(ap_id) when is_binary(ap_id) do + ap_id + |> Queries.by_object_id() + |> with_preloaded_object() + |> first() + |> Repo.one() + end + + def get_by_object_ap_id_with_object(_), do: nil end diff --git a/lib/pleroma/emails/admin_email.ex b/lib/pleroma/emails/admin_email.ex index 8979db2f8..423c294cb 100644 --- a/lib/pleroma/emails/admin_email.ex +++ b/lib/pleroma/emails/admin_email.ex @@ -52,6 +52,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 3543f7f73..99c729473 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/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 713b0ca1f..d580c02a9 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -701,14 +701,30 @@ def make_flag_data(%{actor: actor, context: context, content: content} = params, def make_flag_data(_, _), do: %{} - defp build_flag_object(%{account: account, statuses: statuses} = _) do - [account.ap_id] ++ build_flag_object(%{statuses: statuses}) + defp build_flag_object(%{account: account, statuses: statuses}) do + [account.ap_id | build_flag_object(%{statuses: statuses})] end defp build_flag_object(%{statuses: statuses}) do Enum.map(statuses || [], &build_flag_object/1) end + defp build_flag_object(%Activity{data: %{"id" => id}, object: %{data: data}}) do + activity_actor = User.get_by_ap_id(data["actor"]) + + %{ + "type" => "Note", + "id" => id, + "content" => data["content"], + "published" => data["published"], + "actor" => + AccountView.render( + "show.json", + %{user: activity_actor, skip_visibility_check: true} + ) + } + end + defp build_flag_object(act) when is_map(act) or is_binary(act) do id = case act do @@ -719,22 +735,14 @@ defp build_flag_object(act) when is_map(act) or is_binary(act) do case Activity.get_by_ap_id_with_object(id) do %Activity{} = activity -> - activity_actor = User.get_by_ap_id(activity.object.data["actor"]) + build_flag_object(activity) - %{ - "type" => "Note", - "id" => activity.data["id"], - "content" => activity.object.data["content"], - "published" => activity.object.data["published"], - "actor" => - AccountView.render( - "show.json", - %{user: activity_actor, skip_visibility_check: true} - ) - } - - _ -> - %{"id" => id, "deleted" => true} + nil -> + if activity = Activity.get_by_object_ap_id_with_object(id) do + build_flag_object(activity) + else + %{"id" => id, "deleted" => true} + end end end diff --git a/test/pleroma/activity_test.exs b/test/pleroma/activity_test.exs index ee6a99cc3..3e9fe209e 100644 --- a/test/pleroma/activity_test.exs +++ b/test/pleroma/activity_test.exs @@ -231,4 +231,20 @@ test "all_by_actor_and_id/2" do assert [%Activity{id: ^id1}, %Activity{id: ^id2}] = activities end + + test "get_by_object_ap_id_with_object/1" do + user = insert(:user) + another = insert(:user) + + {:ok, %{id: id, object: %{data: %{"id" => obj_id}}}} = + Pleroma.Web.CommonAPI.post(user, %{status: "cofe"}) + + Pleroma.Web.CommonAPI.favorite(another, id) + + assert obj_id + |> Pleroma.Activity.Queries.by_object_id() + |> Repo.aggregate(:count, :id) == 2 + + assert %{id: ^id} = Activity.get_by_object_ap_id_with_object(obj_id) + end end 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 b696a24f4..fb5911825 100644 --- a/test/pleroma/web/activity_pub/activity_pub_controller_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_controller_test.exs @@ -766,6 +766,142 @@ test "it requires authentication", %{conn: conn} do assert json_response(ret_conn, 200) end + + @tag capture_log: true + 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 + + @tag capture_log: true + test "forwarded report from mastodon", %{conn: conn} do + admin = insert(:user, is_admin: true) + actor = insert(:user, local: false) + remote_domain = URI.parse(actor.ap_id).host + remote_actor = "https://#{remote_domain}/actor" + [reported_user, another] = insert_list(2, :user) + + note = insert(:note_activity, user: reported_user) + + Pleroma.Web.CommonAPI.favorite(another, note.id) + + mock_json_body = + "test/fixtures/mastodon/application_actor.json" + |> File.read!() + |> String.replace("{{DOMAIN}}", remote_domain) + + Tesla.Mock.mock(fn %{url: ^remote_actor} -> + %Tesla.Env{ + status: 200, + body: mock_json_body, + headers: [{"content-type", "application/activity+json"}] + } + end) + + data = %{ + "@context" => "https://www.w3.org/ns/activitystreams", + "actor" => remote_actor, + "content" => "test report", + "id" => "https://#{remote_domain}/e3b12fd1-948c-446e-b93b-a5e67edbe1d8", + "nickname" => reported_user.nickname, + "object" => [ + reported_user.ap_id, + note.data["object"] + ], + "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)) + + flag_activity = "Flag" |> Pleroma.Activity.Queries.by_type() |> Pleroma.Repo.one() + reported_user_ap_id = reported_user.ap_id + + [^reported_user_ap_id, flag_data] = flag_activity.data["object"] + + Enum.each(~w(actor content id published type), &Map.has_key?(flag_data, &1)) + ObanHelpers.perform_all() + + Swoosh.TestAssertions.assert_email_sent( + to: {admin.name, admin.email}, + html_body: ~r/#{note.data["object"]}/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 c6ca37847..ef93b7859 100644 --- a/test/pleroma/web/activity_pub/activity_pub_test.exs +++ b/test/pleroma/web/activity_pub/activity_pub_test.exs @@ -1282,6 +1282,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 fb82be0c4..581c4a2d8 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