Deletion: Handle the case of pruned objects.
authorlain <lain@soykaf.club>
Mon, 11 May 2020 13:06:23 +0000 (15:06 +0200)
committerlain <lain@soykaf.club>
Mon, 11 May 2020 13:06:23 +0000 (15:06 +0200)
lib/pleroma/user.ex
lib/pleroma/web/activity_pub/builder.ex
lib/pleroma/web/activity_pub/object_validators/delete_validator.ex
lib/pleroma/web/activity_pub/transmogrifier.ex
lib/pleroma/web/common_api/common_api.ex
test/tasks/user_test.exs
test/web/activity_pub/side_effects_test.exs
test/web/activity_pub/transmogrifier/delete_handling_test.exs
test/web/common_api/common_api_test.exs

index 2a6a23fecb69c8c1938137b508266a93cb051824..a86cc3202ffca42c5966d3492387eb944691241f 100644 (file)
@@ -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)
index 922a444a9b7704080afcaeca04bdc1c8be5e981d..4a247ad0ca425834e032d8c01df612c6ea3f5fa4 100644 (file)
@@ -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
index e06de3dff02c00a68066e41545345c63aa9b1a34..f42c035105444a8b48eefa53389bdff4c3ab7d0a 100644 (file)
@@ -51,6 +51,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do
     Page
     Question
     Video
+    Tombstone
   }
   def validate_data(cng) do
     cng
index be7b57f13bb20ad9abe2f2c29b2b131a734cf4cc..921576617fb030dc06b569a9f785c9cb4bd6bddf 100644 (file)
@@ -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
 
index c538a634f2e66c7419619e6d7bdd307a9063a763..fbef05e83c7584ffc4aa47d2b42ab8af55ced369 100644 (file)
@@ -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
 
index e0fee729017bdb365fdb0d2fcda27421d12b5788..b4f68d494962298fb670b7906bcd6fe97f3f5523 100644 (file)
@@ -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"])
 
index b29a7a7be8dd4a3f9617d16b10d4b92cbd2749ab..aa3e40be116a269b7ffab8ec5b4adc407c9b87d3 100644 (file)
@@ -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()
index f235a8e633ae7e8b656a67a3654ad3cce90cf2f6..c9a53918c38bd1747cdc4122cdf3ee025b35f7cf 100644 (file)
@@ -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")
index 2fd17a1b896992d785a47a03079dacc8a99096ad..c524d1c0c3fe5ac5459ad5a81f9e9d4e7930bd9a 100644 (file)
@@ -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)