commit: ecd1ef8cb5afa16dba5158e9e278a18c0856ca3e
parent d8924e1a56d51ac67d4cca6f9efb57fae33bbed4
Author: feld <feld@feld.me>
Date: Fri, 20 Nov 2020 18:40:15 +0000
Merge branch 'fix/2302-report-duplicates' into 'develop'
Fix for forwarded reports
Closes #2303 and #2302
See merge request pleroma/pleroma!3146
Diffstat:
10 files changed, 338 insertions(+), 41 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -47,8 +47,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed
-- Fix ability to update Pleroma Chat push notifications with PUT /api/v1/push/subscription and alert type pleroma:chat_mention
-
### Fixed
- Config generation: rename `Pleroma.Upload.Filter.ExifTool` to `Pleroma.Upload.Filter.Exiftool`.
@@ -56,6 +54,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- S3 Uploads with Elixir 1.11.
- Emoji Reaction activity filtering from blocked and muted accounts.
- Mix task pleroma.user delete_activities for source installations.
+- Fix ability to update Pleroma Chat push notifications with PUT /api/v1/push/subscription and alert type pleroma:chat_mention
+- Forwarded reports duplication from Pleroma instances.
+
+<details>
+ <summary>API</summary>
+- Statuses were not displayed for Mastodon forwarded reports.
+
+</details>
## [2.2.0] - 2020-11-12
diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex
@@ -356,4 +356,15 @@ defmodule Pleroma.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
@@ -48,6 +48,9 @@ defmodule Pleroma.Emails.AdminEmail do
status_url = Helpers.o_status_url(Pleroma.Web.Endpoint, :notice, id)
"<li><a href=\"#{status_url}\">#{status_url}</li>"
+ %{"id" => id} when is_binary(id) ->
+ "<li><a href=\"#{id}\">#{id}</li>"
+
id when is_binary(id) ->
"<li><a href=\"#{id}\">#{id}</li>"
end)
diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -334,15 +334,21 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub 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)
@@ -360,7 +366,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
{: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 ->
@@ -370,6 +377,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
end)
{:ok, activity}
+ else
+ {:error, error} -> Repo.rollback(error)
end
end
@@ -793,10 +802,10 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
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
@@ -702,14 +702,30 @@ defmodule Pleroma.Web.ActivityPub.Utils do
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
@@ -720,22 +736,14 @@ defmodule Pleroma.Web.ActivityPub.Utils do
case Activity.get_by_ap_id_with_object(id) do
%Activity{} = activity ->
- activity_actor = User.get_by_ap_id(activity.object.data["actor"])
-
- %{
- "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}
+ build_flag_object(activity)
+
+ 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/fixtures/mastodon/application_actor.json b/test/fixtures/mastodon/application_actor.json
@@ -0,0 +1,67 @@
+{
+ "@context": [
+ "https://www.w3.org/ns/activitystreams",
+ "https://w3id.org/security/v1",
+ {
+ "manuallyApprovesFollowers": "as:manuallyApprovesFollowers",
+ "toot": "http://joinmastodon.org/ns#",
+ "featured": {
+ "@id": "toot:featured",
+ "@type": "@id"
+ },
+ "alsoKnownAs": {
+ "@id": "as:alsoKnownAs",
+ "@type": "@id"
+ },
+ "movedTo": {
+ "@id": "as:movedTo",
+ "@type": "@id"
+ },
+ "schema": "http://schema.org#",
+ "PropertyValue": "schema:PropertyValue",
+ "value": "schema:value",
+ "IdentityProof": "toot:IdentityProof",
+ "discoverable": "toot:discoverable",
+ "Device": "toot:Device",
+ "Ed25519Signature": "toot:Ed25519Signature",
+ "Ed25519Key": "toot:Ed25519Key",
+ "Curve25519Key": "toot:Curve25519Key",
+ "EncryptedMessage": "toot:EncryptedMessage",
+ "publicKeyBase64": "toot:publicKeyBase64",
+ "deviceId": "toot:deviceId",
+ "claim": {
+ "@type": "@id",
+ "@id": "toot:claim"
+ },
+ "fingerprintKey": {
+ "@type": "@id",
+ "@id": "toot:fingerprintKey"
+ },
+ "identityKey": {
+ "@type": "@id",
+ "@id": "toot:identityKey"
+ },
+ "devices": {
+ "@type": "@id",
+ "@id": "toot:devices"
+ },
+ "messageFranking": "toot:messageFranking",
+ "messageType": "toot:messageType",
+ "cipherText": "toot:cipherText"
+ }
+ ],
+ "id": "https://{{DOMAIN}}/actor",
+ "type": "Application",
+ "inbox": "https://{{DOMAIN}}/actor/inbox",
+ "preferredUsername": "{{DOMAIN}}",
+ "url": "https://{{DOMAIN}}/about/more?instance_actor=true",
+ "manuallyApprovesFollowers": true,
+ "publicKey": {
+ "id": "https://{{DOMAIN}}/actor#main-key",
+ "owner": "https://{{DOMAIN}}/actor",
+ "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAA0CA08AMIIBCgKCAQEAyi2T2FFZJgRPY+96YQrn\n6J6eF2P60J+nz+/pRc/acv/Nx+NLxxPyXby0F2s60MV7uALRQbBBnf7oNKCd/T4S\nvbr7UXMCWTdaJBpYubMKWT9uBlaUUkUfqL+WTV+IQnlcKtssQ4+AwrAKAZXza8ws\nZypevOsLHzayyEzztmm1KQC9GCUOITCLf7Q6qEhy8z/HuqLBEC0Own0pD7QsbfcS\no1peuZY7g1E/jJ9HR9GqJccMaR0H28KmJ7tT1Yzlyf5uZMRIdPxsoMR9sGLjR2B8\noegSwaf9SogR3ScP395Tt/9Ud1VVzuhpoS8Uy7jKSs+3CuLJsEGoMrib8VyOwadS\n9wIDAQAB\n-----END PUBLIC KEY-----\n"
+ },
+ "endpoints": {
+ "sharedInbox": "https://{{DOMAIN}}/inbox"
+ }
+}
diff --git a/test/pleroma/activity_test.exs b/test/pleroma/activity_test.exs
@@ -231,4 +231,20 @@ defmodule Pleroma.ActivityTest 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
@@ -799,6 +799,142 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest 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
@@ -1298,6 +1298,31 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
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
@@ -24,7 +24,7 @@ defmodule Pleroma.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 @@ defmodule Pleroma.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
