From 29647dfd09a78fa66a605ff97119a21779d15020 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 10 Oct 2019 17:17:33 +0200 Subject: [PATCH 1/5] Transmogrifier: Save correct ids for incoming deletes. --- lib/pleroma/web/activity_pub/activity_pub.ex | 8 ++++++-- lib/pleroma/web/activity_pub/transmogrifier.ex | 4 ++-- test/web/activity_pub/transmogrifier_test.exs | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 9f29087df..3a0c382ca 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -409,7 +409,10 @@ def delete(%User{ap_id: ap_id, follower_address: follower_address} = user) do end end - def delete(%Object{data: %{"id" => id, "actor" => actor}} = object, local \\ true) do + def delete(%Object{data: %{"id" => id, "actor" => actor}} = object, options \\ []) do + local = Keyword.get(options, :local, true) + activity_id = Keyword.get(options, :activity_id, nil) + user = User.get_cached_by_ap_id(actor) to = (object.data["to"] || []) ++ (object.data["cc"] || []) @@ -420,7 +423,8 @@ def delete(%Object{data: %{"id" => id, "actor" => actor}} = object, local \\ tru "object" => id, "to" => to, "deleted_activity_id" => activity && activity.id - }, + } + |> maybe_put("id", activity_id), {:ok, activity} <- insert(data, local, false), stream_out_participations(object, user), _ <- decrease_replies_count_if_reply(object), diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 872ed0eb2..620b54d3b 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -637,7 +637,7 @@ def handle_incoming( # an error or a tombstone. This would allow us to verify that a deletion actually took # place. def handle_incoming( - %{"type" => "Delete", "object" => object_id, "actor" => actor, "id" => _id} = data, + %{"type" => "Delete", "object" => object_id, "actor" => actor, "id" => id} = data, _options ) do object_id = Utils.get_ap_id(object_id) @@ -646,7 +646,7 @@ def handle_incoming( {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor), {:ok, object} <- get_obj_helper(object_id), :ok <- Containment.contain_origin(actor.ap_id, object.data), - {:ok, activity} <- ActivityPub.delete(object, false) do + {:ok, activity} <- ActivityPub.delete(object, local: false, activity_id: id) do {:ok, activity} else nil -> diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 50c0bfb84..4f9c4af33 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -696,8 +696,9 @@ test "it works for incoming deletes" do |> Map.put("object", object) |> Map.put("actor", activity.data["actor"]) - {:ok, %Activity{local: false}} = Transmogrifier.handle_incoming(data) + {:ok, %Activity{local: false, data: %{"id" => id}}} = Transmogrifier.handle_incoming(data) + assert id == data["id"] refute Activity.get_by_id(activity.id) end From 9b963064eb4c8cb740ffa128f491f2ee581fdb8b Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 11 Oct 2019 11:25:45 +0200 Subject: [PATCH 2/5] Transmogrifier: Actually store who deleted a note. --- lib/pleroma/web/activity_pub/activity_pub.ex | 18 ++++++++++-------- lib/pleroma/web/activity_pub/transmogrifier.ex | 3 ++- test/web/activity_pub/transmogrifier_test.exs | 7 +++++-- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 3a0c382ca..f78574455 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -412,19 +412,21 @@ def delete(%User{ap_id: ap_id, follower_address: follower_address} = user) do def delete(%Object{data: %{"id" => id, "actor" => actor}} = object, options \\ []) do local = Keyword.get(options, :local, true) activity_id = Keyword.get(options, :activity_id, nil) + actor = Keyword.get(options, :actor, actor) user = User.get_cached_by_ap_id(actor) to = (object.data["to"] || []) ++ (object.data["cc"] || []) with {:ok, object, activity} <- Object.delete(object), - data <- %{ - "type" => "Delete", - "actor" => actor, - "object" => id, - "to" => to, - "deleted_activity_id" => activity && activity.id - } - |> maybe_put("id", activity_id), + data <- + %{ + "type" => "Delete", + "actor" => actor, + "object" => id, + "to" => to, + "deleted_activity_id" => activity && activity.id + } + |> maybe_put("id", activity_id), {:ok, activity} <- insert(data, local, false), stream_out_participations(object, user), _ <- decrease_replies_count_if_reply(object), diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 620b54d3b..64fbb9fa4 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -646,7 +646,8 @@ def handle_incoming( {:ok, %User{} = actor} <- User.get_or_fetch_by_ap_id(actor), {:ok, object} <- get_obj_helper(object_id), :ok <- Containment.contain_origin(actor.ap_id, object.data), - {:ok, activity} <- ActivityPub.delete(object, local: false, activity_id: id) do + {:ok, activity} <- + ActivityPub.delete(object, local: false, activity_id: id, actor: actor.ap_id) do {:ok, activity} else nil -> diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 4f9c4af33..9f98ec708 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -682,6 +682,7 @@ test "it works for incoming update activities which lock the account" do test "it works for incoming deletes" do activity = insert(:note_activity) + deleting_user = insert(:user) data = File.read!("test/fixtures/mastodon-delete.json") @@ -694,12 +695,14 @@ test "it works for incoming deletes" do data = data |> Map.put("object", object) - |> Map.put("actor", activity.data["actor"]) + |> Map.put("actor", deleting_user.ap_id) - {:ok, %Activity{local: false, data: %{"id" => id}}} = Transmogrifier.handle_incoming(data) + {:ok, %Activity{actor: actor, local: false, data: %{"id" => id}}} = + Transmogrifier.handle_incoming(data) assert id == data["id"] refute Activity.get_by_id(activity.id) + assert actor == deleting_user.ap_id end test "it fails for incoming deletes with spoofed origin" do From 37812740c4c03f0a79d17ff186db644a60414ff7 Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 11 Oct 2019 11:48:58 +0200 Subject: [PATCH 3/5] Transmogrifier: Correctly save incoming ids for Accept/Reject. --- lib/pleroma/web/activity_pub/activity_pub.ex | 25 +++++++++---------- .../web/activity_pub/transmogrifier.ex | 10 +++++--- test/web/activity_pub/transmogrifier_test.exs | 3 +++ 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index f78574455..364452b5d 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -269,22 +269,21 @@ def listen(%{to: to, actor: actor, context: context, object: object} = params) d end end - def accept(%{to: to, actor: actor, object: object} = params) do - # only accept false as false value - local = !(params[:local] == false) - - with data <- %{"to" => to, "type" => "Accept", "actor" => actor.ap_id, "object" => object}, - {:ok, activity} <- insert(data, local), - :ok <- maybe_federate(activity) do - {:ok, activity} - end + def accept(params) do + accept_or_reject("Accept", params) end - def reject(%{to: to, actor: actor, object: object} = params) do - # only accept false as false value - local = !(params[:local] == false) + def reject(params) do + accept_or_reject("Reject", params) + end - with data <- %{"to" => to, "type" => "Reject", "actor" => actor.ap_id, "object" => object}, + def accept_or_reject(type, %{to: to, actor: actor, object: object} = params) do + local = Map.get(params, :local, true) + activity_id = Map.get(params, :activity_id, nil) + + with data <- + %{"to" => to, "type" => type, "actor" => actor.ap_id, "object" => object} + |> Utils.maybe_put("id", activity_id), {:ok, activity} <- insert(data, local), :ok <- maybe_federate(activity) do {:ok, activity} diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 64fbb9fa4..b56343beb 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -514,7 +514,7 @@ def handle_incoming( end def handle_incoming( - %{"type" => "Accept", "object" => follow_object, "actor" => _actor, "id" => _id} = data, + %{"type" => "Accept", "object" => follow_object, "actor" => _actor, "id" => id} = data, _options ) do with actor <- Containment.get_actor(data), @@ -528,7 +528,8 @@ def handle_incoming( type: "Accept", actor: followed, object: follow_activity.data["id"], - local: false + local: false, + activity_id: id }) else _e -> :error @@ -536,7 +537,7 @@ def handle_incoming( end def handle_incoming( - %{"type" => "Reject", "object" => follow_object, "actor" => _actor, "id" => _id} = data, + %{"type" => "Reject", "object" => follow_object, "actor" => _actor, "id" => id} = data, _options ) do with actor <- Containment.get_actor(data), @@ -550,7 +551,8 @@ def handle_incoming( type: "Reject", actor: followed, object: follow_activity.data["id"], - local: false + local: false, + activity_id: id }) do User.unfollow(follower, followed) diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 9f98ec708..6c35a6f4d 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -909,6 +909,8 @@ test "it works for incoming accepts which were pre-accepted" do assert activity.data["object"] == follow_activity.data["id"] + assert activity.data["id"] == accept_data["id"] + follower = User.get_cached_by_id(follower.id) assert User.following?(follower, followed) == true @@ -1013,6 +1015,7 @@ test "it works for incoming rejects which are orphaned" do {:ok, activity} = Transmogrifier.handle_incoming(reject_data) refute activity.local + assert activity.data["id"] == reject_data["id"] follower = User.get_cached_by_id(follower.id) From cb8492962e2a765090dbce82b8fc9f000d49bfa1 Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 11 Oct 2019 12:41:44 +0200 Subject: [PATCH 4/5] SearchController: Fix test. Turns out you can't actually find the user with this. --- .../mastodon_api/controllers/search_controller_test.exs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/web/mastodon_api/controllers/search_controller_test.exs b/test/web/mastodon_api/controllers/search_controller_test.exs index 0ca896e01..ee413eef7 100644 --- a/test/web/mastodon_api/controllers/search_controller_test.exs +++ b/test/web/mastodon_api/controllers/search_controller_test.exs @@ -40,9 +40,9 @@ test "it returns empty result if user or status search return undefined error", test "search", %{conn: conn} do user = insert(:user) user_two = insert(:user, %{nickname: "shp@shitposter.club"}) - user_three = insert(:user, %{nickname: "shp@heldscal.la", name: "I love 2hu 天子"}) + user_three = insert(:user, %{nickname: "shp@heldscal.la", name: "I love 2hu"}) - {:ok, activity} = CommonAPI.post(user, %{"status" => "This is about 2hu private"}) + {:ok, activity} = CommonAPI.post(user, %{"status" => "This is about 2hu private 天子"}) {:ok, _activity} = CommonAPI.post(user, %{ @@ -70,8 +70,8 @@ test "search", %{conn: conn} do get(conn, "/api/v2/search", %{"q" => "天子"}) |> json_response(200) - [account] == results["accounts"] - assert account["id"] == to_string(user_three.id) + [status] = results["statuses"] + assert status["id"] == to_string(activity.id) end end From 422aa6befee0cbcbf71d3cacd96d90c1be3263ba Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 11 Oct 2019 12:53:09 +0200 Subject: [PATCH 5/5] Ostatus DeleteHandler: Fix for new option format. --- lib/pleroma/web/ostatus/handlers/delete_handler.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/ostatus/handlers/delete_handler.ex b/lib/pleroma/web/ostatus/handlers/delete_handler.ex index b2f9f3946..ac2dc115c 100644 --- a/lib/pleroma/web/ostatus/handlers/delete_handler.ex +++ b/lib/pleroma/web/ostatus/handlers/delete_handler.ex @@ -11,7 +11,7 @@ defmodule Pleroma.Web.OStatus.DeleteHandler do def handle_delete(entry, _doc \\ nil) do with id <- XML.string_from_xpath("//id", entry), %Object{} = object <- Object.normalize(id), - {:ok, delete} <- ActivityPub.delete(object, false) do + {:ok, delete} <- ActivityPub.delete(object, local: false) do delete end end