From 5367a00257c6f862a4a8080e0176f676ce491e4d Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 11 May 2020 15:06:23 +0200 Subject: [PATCH] Deletion: Handle the case of pruned objects. --- lib/pleroma/user.ex | 19 ++++++++++-- lib/pleroma/web/activity_pub/builder.ex | 10 +++++++ .../object_validators/delete_validator.ex | 1 + .../web/activity_pub/transmogrifier.ex | 15 ++++++++++ lib/pleroma/web/common_api/common_api.ex | 29 +++++++++++++++---- test/tasks/user_test.exs | 25 ++++++++++++++++ test/web/activity_pub/side_effects_test.exs | 29 +++++++++++++++++++ .../transmogrifier/delete_handling_test.exs | 28 ++++++++++++++++++ test/web/common_api/common_api_test.exs | 18 ++++++++++++ 9 files changed, 166 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 2a6a23fec..a86cc3202 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -1554,10 +1554,23 @@ defmodule Pleroma.User do |> Stream.run() end - defp delete_activity(%{data: %{"type" => "Create", "object" => object}}, user) do - {:ok, delete_data, _} = Builder.delete(user, object) + defp delete_activity(%{data: %{"type" => "Create", "object" => object}} = activity, user) do + with {_, %Object{}} <- {:find_object, Object.get_by_ap_id(object)}, + {:ok, delete_data, _} <- Builder.delete(user, object) do + Pipeline.common_pipeline(delete_data, local: user.local) + else + {:find_object, nil} -> + # We have the create activity, but not the object, it was probably pruned. + # Insert a tombstone and try again + with {:ok, tombstone_data, _} <- Builder.tombstone(user.ap_id, object), + {:ok, _tombstone} <- Object.create(tombstone_data) do + delete_activity(activity, user) + end - Pipeline.common_pipeline(delete_data, local: user.local) + e -> + Logger.error("Could not delete #{object} created by #{activity.data["ap_id"]}") + Logger.error("Error: #{inspect(e)}") + end end defp delete_activity(%{data: %{"type" => type}} = activity, user) diff --git a/lib/pleroma/web/activity_pub/builder.ex b/lib/pleroma/web/activity_pub/builder.ex index 922a444a9..4a247ad0c 100644 --- a/lib/pleroma/web/activity_pub/builder.ex +++ b/lib/pleroma/web/activity_pub/builder.ex @@ -62,6 +62,16 @@ defmodule Pleroma.Web.ActivityPub.Builder do }, []} end + @spec tombstone(String.t(), String.t()) :: {:ok, map(), keyword()} + def tombstone(actor, id) do + {:ok, + %{ + "id" => id, + "actor" => actor, + "type" => "Tombstone" + }, []} + end + @spec like(User.t(), Object.t()) :: {:ok, map(), keyword()} def like(actor, object) do with {:ok, data, meta} <- object_action(actor, object) do diff --git a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex index e06de3dff..f42c03510 100644 --- a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -51,6 +51,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do Page Question Video + Tombstone } def validate_data(cng) do cng diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index be7b57f13..921576617 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -14,7 +14,9 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do alias Pleroma.Repo alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub + alias Pleroma.Web.ActivityPub.Builder alias Pleroma.Web.ActivityPub.ObjectValidator + alias Pleroma.Web.ActivityPub.ObjectValidators.Types alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Visibility @@ -720,6 +722,19 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do ) do with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do {:ok, activity} + else + {:error, {:validate_object, _}} = e -> + # Check if we have a create activity for this + with {:ok, object_id} <- Types.ObjectID.cast(data["object"]), + %Activity{data: %{"actor" => actor}} <- + Activity.create_by_object_ap_id(object_id) |> Repo.one(), + # We have one, insert a tombstone and retry + {:ok, tombstone_data, _} <- Builder.tombstone(actor, object_id), + {:ok, _tombstone} <- Object.create(tombstone_data) do + handle_incoming(data) + else + _ -> e + end end end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index c538a634f..fbef05e83 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -83,16 +83,35 @@ defmodule Pleroma.Web.CommonAPI do end def delete(activity_id, user) do - with {_, %Activity{data: %{"object" => _}} = activity} <- - {:find_activity, Activity.get_by_id_with_object(activity_id)}, - %Object{} = object <- Object.normalize(activity), + with {_, %Activity{data: %{"object" => _, "type" => "Create"}} = activity} <- + {:find_activity, Activity.get_by_id(activity_id)}, + {_, %Object{} = object, _} <- + {:find_object, Object.normalize(activity, false), activity}, true <- User.superuser?(user) || user.ap_id == object.data["actor"], {:ok, delete_data, _} <- Builder.delete(user, object.data["id"]), {:ok, delete, _} <- Pipeline.common_pipeline(delete_data, local: true) do {:ok, delete} else - {:find_activity, _} -> {:error, :not_found} - _ -> {:error, dgettext("errors", "Could not delete")} + {:find_activity, _} -> + {:error, :not_found} + + {:find_object, nil, %Activity{data: %{"actor" => actor, "object" => object}}} -> + # We have the create activity, but not the object, it was probably pruned. + # Insert a tombstone and try again + with {:ok, tombstone_data, _} <- Builder.tombstone(actor, object), + {:ok, _tombstone} <- Object.create(tombstone_data) do + delete(activity_id, user) + else + _ -> + Logger.error( + "Could not insert tombstone for missing object on deletion. Object is #{object}." + ) + + {:error, dgettext("errors", "Could not delete")} + end + + _ -> + {:error, dgettext("errors", "Could not delete")} end end diff --git a/test/tasks/user_test.exs b/test/tasks/user_test.exs index e0fee7290..b4f68d494 100644 --- a/test/tasks/user_test.exs +++ b/test/tasks/user_test.exs @@ -3,9 +3,12 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Mix.Tasks.Pleroma.UserTest do + alias Pleroma.Activity + alias Pleroma.Object alias Pleroma.Repo alias Pleroma.Tests.ObanHelpers alias Pleroma.User + alias Pleroma.Web.CommonAPI alias Pleroma.Web.OAuth.Authorization alias Pleroma.Web.OAuth.Token @@ -103,6 +106,28 @@ defmodule Mix.Tasks.Pleroma.UserTest do end end + test "a remote user's create activity is deleted when the object has been pruned" do + user = insert(:user) + + {:ok, post} = CommonAPI.post(user, %{"status" => "uguu"}) + object = Object.normalize(post) + Object.prune(object) + + with_mock Pleroma.Web.Federator, + publish: fn _ -> nil end do + Mix.Tasks.Pleroma.User.run(["rm", user.nickname]) + ObanHelpers.perform_all() + + assert_received {:mix_shell, :info, [message]} + assert message =~ " deleted" + assert %{deactivated: true} = User.get_by_nickname(user.nickname) + + assert called(Pleroma.Web.Federator.publish(:_)) + end + + refute Activity.get_by_id(post.id) + end + test "no user to delete" do Mix.Tasks.Pleroma.User.run(["rm", "nonexistent"]) diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index b29a7a7be..aa3e40be1 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -64,6 +64,35 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do assert object.data["repliesCount"] == 0 end + test "it handles object deletions when the object itself has been pruned", %{ + delete: delete, + post: post, + object: object, + user: user, + op: op + } do + with_mock Pleroma.Web.ActivityPub.ActivityPub, [:passthrough], + stream_out: fn _ -> nil end, + stream_out_participations: fn _, _ -> nil end do + {:ok, delete, _} = SideEffects.handle(delete) + user = User.get_cached_by_ap_id(object.data["actor"]) + + assert called(Pleroma.Web.ActivityPub.ActivityPub.stream_out(delete)) + assert called(Pleroma.Web.ActivityPub.ActivityPub.stream_out_participations(object, user)) + end + + object = Object.get_by_id(object.id) + assert object.data["type"] == "Tombstone" + refute Activity.get_by_id(post.id) + + user = User.get_by_id(user.id) + assert user.note_count == 0 + + object = Object.normalize(op.data["object"], false) + + assert object.data["repliesCount"] == 0 + end + test "it handles user deletions", %{delete_user: delete, user: user} do {:ok, _delete, _} = SideEffects.handle(delete) ObanHelpers.perform_all() diff --git a/test/web/activity_pub/transmogrifier/delete_handling_test.exs b/test/web/activity_pub/transmogrifier/delete_handling_test.exs index f235a8e63..c9a53918c 100644 --- a/test/web/activity_pub/transmogrifier/delete_handling_test.exs +++ b/test/web/activity_pub/transmogrifier/delete_handling_test.exs @@ -44,6 +44,34 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.DeleteHandlingTest do assert object.data["type"] == "Tombstone" end + test "it works for incoming when the object has been pruned" do + activity = insert(:note_activity) + + {:ok, object} = + Object.normalize(activity.data["object"]) + |> Repo.delete() + + Cachex.del(:object_cache, "object:#{object.data["id"]}") + + deleting_user = insert(:user) + + data = + File.read!("test/fixtures/mastodon-delete.json") + |> Poison.decode!() + |> Map.put("actor", deleting_user.ap_id) + |> put_in(["object", "id"], activity.data["object"]) + + {:ok, %Activity{actor: actor, local: false, data: %{"id" => id}}} = + Transmogrifier.handle_incoming(data) + + assert id == data["id"] + + # We delete the Create activity because we base our timelines on it. + # This should be changed after we unify objects and activities + refute Activity.get_by_id(activity.id) + assert actor == deleting_user.ap_id + end + test "it fails for incoming deletes with spoofed origin" do activity = insert(:note_activity) %{ap_id: ap_id} = insert(:user, ap_id: "https://gensokyo.2hu/users/raymoo") diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 2fd17a1b8..c524d1c0c 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -24,6 +24,24 @@ defmodule Pleroma.Web.CommonAPITest do setup do: clear_config([:instance, :max_pinned_statuses]) describe "deletion" do + test "it works with pruned objects" do + user = insert(:user) + + {:ok, post} = CommonAPI.post(user, %{"status" => "namu amida butsu"}) + + Object.normalize(post, false) + |> Object.prune() + + with_mock Pleroma.Web.Federator, + publish: fn _ -> nil end do + assert {:ok, delete} = CommonAPI.delete(post.id, user) + assert delete.local + assert called(Pleroma.Web.Federator.publish(delete)) + end + + refute Activity.get_by_id(post.id) + end + test "it allows users to delete their posts" do user = insert(:user)