From b8056e69e0a2505fc466dd5742b0986b7c1895ae Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 29 Apr 2020 19:08:08 +0200 Subject: [PATCH 01/34] Object Validator Types: Add Recipients. --- .../object_validators/types/recipients.ex | 34 +++++++++++++++++++ .../types/recipients_test.exs | 27 +++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 lib/pleroma/web/activity_pub/object_validators/types/recipients.ex create mode 100644 test/web/activity_pub/object_validators/types/recipients_test.exs diff --git a/lib/pleroma/web/activity_pub/object_validators/types/recipients.ex b/lib/pleroma/web/activity_pub/object_validators/types/recipients.ex new file mode 100644 index 000000000..48fe61e1a --- /dev/null +++ b/lib/pleroma/web/activity_pub/object_validators/types/recipients.ex @@ -0,0 +1,34 @@ +defmodule Pleroma.Web.ActivityPub.ObjectValidators.Types.Recipients do + use Ecto.Type + + alias Pleroma.Web.ActivityPub.ObjectValidators.Types.ObjectID + + def type, do: {:array, ObjectID} + + def cast(object) when is_binary(object) do + cast([object]) + end + + def cast(data) when is_list(data) do + data + |> Enum.reduce({:ok, []}, fn element, acc -> + case {acc, ObjectID.cast(element)} do + {:error, _} -> :error + {_, :error} -> :error + {{:ok, list}, {:ok, id}} -> {:ok, [id | list]} + end + end) + end + + def cast(_) do + :error + end + + def dump(data) do + {:ok, data} + end + + def load(data) do + {:ok, data} + end +end diff --git a/test/web/activity_pub/object_validators/types/recipients_test.exs b/test/web/activity_pub/object_validators/types/recipients_test.exs new file mode 100644 index 000000000..f278f039b --- /dev/null +++ b/test/web/activity_pub/object_validators/types/recipients_test.exs @@ -0,0 +1,27 @@ +defmodule Pleroma.Web.ObjectValidators.Types.RecipientsTest do + alias Pleroma.Web.ActivityPub.ObjectValidators.Types.Recipients + use Pleroma.DataCase + + test "it asserts that all elements of the list are object ids" do + list = ["https://lain.com/users/lain", "invalid"] + + assert :error == Recipients.cast(list) + end + + test "it works with a list" do + list = ["https://lain.com/users/lain"] + assert {:ok, list} == Recipients.cast(list) + end + + test "it works with a list with whole objects" do + list = ["https://lain.com/users/lain", %{"id" => "https://gensokyo.2hu/users/raymoo"}] + resulting_list = ["https://gensokyo.2hu/users/raymoo", "https://lain.com/users/lain"] + assert {:ok, resulting_list} == Recipients.cast(list) + end + + test "it turns a single string into a list" do + recipient = "https://lain.com/users/lain" + + assert {:ok, [recipient]} == Recipients.cast(recipient) + end +end From 78c864cbeed8fcdbe80e2842377d4fabc9362f3c Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 29 Apr 2020 19:08:36 +0200 Subject: [PATCH 02/34] LikeValidator: Use Recipients Type. --- .../web/activity_pub/object_validators/like_validator.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validators/like_validator.ex b/lib/pleroma/web/activity_pub/object_validators/like_validator.ex index 49546ceaa..eeb0da192 100644 --- a/lib/pleroma/web/activity_pub/object_validators/like_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/like_validator.ex @@ -19,8 +19,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator do field(:object, Types.ObjectID) field(:actor, Types.ObjectID) field(:context, :string) - field(:to, {:array, :string}) - field(:cc, {:array, :string}) + field(:to, Types.Recipients) + field(:cc, Types.Recipients) end def cast_and_validate(data) do From 503de4b8df0bfc34008c3c856edc488633290f0e Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 29 Apr 2020 19:09:51 +0200 Subject: [PATCH 03/34] ObjectValidator: Add validation for `Delete`s. --- lib/pleroma/web/activity_pub/builder.ex | 16 +++++ .../web/activity_pub/object_validator.ex | 17 +++++ .../object_validators/common_validations.ex | 20 ++++++ .../object_validators/delete_validator.ex | 64 ++++++++++++++++++ .../activity_pub/object_validator_test.exs | 67 +++++++++++++++++++ 5 files changed, 184 insertions(+) create mode 100644 lib/pleroma/web/activity_pub/object_validators/delete_validator.ex diff --git a/lib/pleroma/web/activity_pub/builder.ex b/lib/pleroma/web/activity_pub/builder.ex index 429a510b8..5cc46c3ea 100644 --- a/lib/pleroma/web/activity_pub/builder.ex +++ b/lib/pleroma/web/activity_pub/builder.ex @@ -10,6 +10,22 @@ defmodule Pleroma.Web.ActivityPub.Builder do alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Visibility + @spec delete(User.t(), String.t()) :: {:ok, map(), keyword()} + def delete(actor, object_id) do + object = Object.normalize(object_id) + + to = (object.data["to"] || []) ++ (object.data["cc"] || []) + + {:ok, + %{ + "id" => Utils.generate_activity_id(), + "actor" => actor.ap_id, + "object" => object_id, + "to" => to, + "type" => "Delete" + }, []} + end + @spec like(User.t(), Object.t()) :: {:ok, map(), keyword()} def like(actor, object) do object_actor = User.get_cached_by_ap_id(object.data["actor"]) diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex index dc4bce059..f476c6f72 100644 --- a/lib/pleroma/web/activity_pub/object_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validator.ex @@ -12,10 +12,21 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do alias Pleroma.Object alias Pleroma.User alias Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator + alias Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator @spec validate(map(), keyword()) :: {:ok, map(), keyword()} | {:error, any()} def validate(object, meta) + def validate(%{"type" => "Delete"} = object, meta) do + with {:ok, object} <- + object + |> DeleteValidator.cast_and_validate() + |> Ecto.Changeset.apply_action(:insert) do + object = stringify_keys(object) + {:ok, object, meta} + end + end + def validate(%{"type" => "Like"} = object, meta) do with {:ok, object} <- object |> LikeValidator.cast_and_validate() |> Ecto.Changeset.apply_action(:insert) do @@ -24,6 +35,12 @@ def validate(%{"type" => "Like"} = object, meta) do end end + def stringify_keys(%{__struct__: _} = object) do + object + |> Map.from_struct() + |> stringify_keys + end + def stringify_keys(object) do object |> Map.new(fn {key, val} -> {to_string(key), val} end) diff --git a/lib/pleroma/web/activity_pub/object_validators/common_validations.ex b/lib/pleroma/web/activity_pub/object_validators/common_validations.ex index b479c3918..e115d9526 100644 --- a/lib/pleroma/web/activity_pub/object_validators/common_validations.ex +++ b/lib/pleroma/web/activity_pub/object_validators/common_validations.ex @@ -8,6 +8,26 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations do alias Pleroma.Object alias Pleroma.User + def validate_recipients_presence(cng, fields \\ [:to, :cc]) do + non_empty = + fields + |> Enum.map(fn field -> get_field(cng, field) end) + |> Enum.any?(fn + [] -> false + _ -> true + end) + + if non_empty do + cng + else + fields + |> Enum.reduce(cng, fn field, cng -> + cng + |> add_error(field, "no recipients in any field") + end) + end + end + def validate_actor_presence(cng, field_name \\ :actor) do cng |> validate_change(field_name, fn field_name, actor -> diff --git a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex new file mode 100644 index 000000000..8dd5c19ad --- /dev/null +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -0,0 +1,64 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do + use Ecto.Schema + + alias Pleroma.Web.ActivityPub.ObjectValidators.Types + + import Ecto.Changeset + import Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations + + @primary_key false + + embedded_schema do + field(:id, Types.ObjectID, primary_key: true) + field(:type, :string) + field(:actor, Types.ObjectID) + field(:to, Types.Recipients, default: []) + field(:cc, Types.Recipients, default: []) + field(:object, Types.ObjectID) + end + + def cast_data(data) do + %__MODULE__{} + |> cast(data, __schema__(:fields)) + end + + def validate_data(cng) do + cng + |> validate_required([:id, :type, :actor, :to, :cc, :object]) + |> validate_inclusion(:type, ["Delete"]) + |> validate_same_domain() + |> validate_object_presence() + |> validate_recipients_presence() + end + + def validate_same_domain(cng) do + actor_domain = + cng + |> get_field(:actor) + |> URI.parse() + |> (& &1.host).() + + object_domain = + cng + |> get_field(:object) + |> URI.parse() + |> (& &1.host).() + + if object_domain != actor_domain do + cng + |> add_error(:actor, "is not allowed to delete object") + else + cng + end + end + + def cast_and_validate(data) do + data + |> cast_data + |> validate_data + end +end diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index 3c5c3696e..64b9ee1ec 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -1,6 +1,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidatorTest do use Pleroma.DataCase + alias Pleroma.Web.ActivityPub.Builder alias Pleroma.Web.ActivityPub.ObjectValidator alias Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator alias Pleroma.Web.ActivityPub.Utils @@ -8,6 +9,72 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidatorTest do import Pleroma.Factory + describe "deletes" do + setup do + user = insert(:user) + {:ok, post_activity} = CommonAPI.post(user, %{"status" => "cancel me daddy"}) + + {:ok, valid_post_delete, _} = Builder.delete(user, post_activity.data["object"]) + + %{user: user, valid_post_delete: valid_post_delete} + end + + test "it is valid for a post deletion", %{valid_post_delete: valid_post_delete} do + assert match?({:ok, _, _}, ObjectValidator.validate(valid_post_delete, [])) + end + + test "it's invalid if the id is missing", %{valid_post_delete: valid_post_delete} do + no_id = + valid_post_delete + |> Map.delete("id") + + {:error, cng} = ObjectValidator.validate(no_id, []) + + assert {:id, {"can't be blank", [validation: :required]}} in cng.errors + end + + test "it's invalid if the object doesn't exist", %{valid_post_delete: valid_post_delete} do + missing_object = + valid_post_delete + |> Map.put("object", "http://does.not/exist") + + {:error, cng} = ObjectValidator.validate(missing_object, []) + + assert {:object, {"can't find object", []}} in cng.errors + end + + test "it's invalid if the actor of the object and the actor of delete are from different domains", + %{valid_post_delete: valid_post_delete} do + valid_other_actor = + valid_post_delete + |> Map.put("actor", valid_post_delete["actor"] <> "1") + + assert match?({:ok, _, _}, ObjectValidator.validate(valid_other_actor, [])) + + invalid_other_actor = + valid_post_delete + |> Map.put("actor", "https://gensokyo.2hu/users/raymoo") + + {:error, cng} = ObjectValidator.validate(invalid_other_actor, []) + + assert {:actor, {"is not allowed to delete object", []}} in cng.errors + end + + test "it's invalid if all the recipient fields are empty", %{ + valid_post_delete: valid_post_delete + } do + empty_recipients = + valid_post_delete + |> Map.put("to", []) + |> Map.put("cc", []) + + {:error, cng} = ObjectValidator.validate(empty_recipients, []) + + assert {:to, {"no recipients in any field", []}} in cng.errors + assert {:cc, {"no recipients in any field", []}} in cng.errors + end + end + describe "likes" do setup do user = insert(:user) From 64bb72f98a91261158b36e63f6c9634ac9f423a6 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 13:57:47 +0200 Subject: [PATCH 04/34] Typo fix. --- lib/pleroma/web/activity_pub/utils.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 2d685ecc0..1a3b0b3c1 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -512,7 +512,7 @@ def get_latest_reaction(internal_activity_id, %{ap_id: ap_id}, emoji) do #### Announce-related helpers @doc """ - Retruns an existing announce activity if the notice has already been announced + Returns an existing announce activity if the notice has already been announced """ @spec get_existing_announce(String.t(), map()) :: Activity.t() | nil def get_existing_announce(actor, %{data: %{"id" => ap_id}}) do From 42ce7c5164326aa577bc7bd18e98c5d0a9d6fea5 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 14:13:08 +0200 Subject: [PATCH 05/34] ObjectValidator: Add actor fetcher. --- lib/pleroma/web/activity_pub/object_validator.ex | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex index f476c6f72..016f6e7a2 100644 --- a/lib/pleroma/web/activity_pub/object_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validator.ex @@ -46,8 +46,14 @@ def stringify_keys(object) do |> Map.new(fn {key, val} -> {to_string(key), val} end) end + def fetch_actor(object) do + with {:ok, actor} <- Types.ObjectID.cast(object["actor"]) do + User.get_or_fetch_by_ap_id(actor) + end + end + def fetch_actor_and_object(object) do - User.get_or_fetch_by_ap_id(object["actor"]) + fetch_actor(object) Object.normalize(object["object"]) :ok end From bd219ba7e884d694cc1c8747f0b48cd646821222 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 14:14:00 +0200 Subject: [PATCH 06/34] Transmogrifier Tests: Extract deletion tests. --- .../transmogrifier/delete_handling_test.exs | 106 ++++++++++++++++++ test/web/activity_pub/transmogrifier_test.exs | 77 ------------- 2 files changed, 106 insertions(+), 77 deletions(-) create mode 100644 test/web/activity_pub/transmogrifier/delete_handling_test.exs diff --git a/test/web/activity_pub/transmogrifier/delete_handling_test.exs b/test/web/activity_pub/transmogrifier/delete_handling_test.exs new file mode 100644 index 000000000..c15de5a95 --- /dev/null +++ b/test/web/activity_pub/transmogrifier/delete_handling_test.exs @@ -0,0 +1,106 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ActivityPub.Transmogrifier.DeleteHandlingTest do + use Oban.Testing, repo: Pleroma.Repo + use Pleroma.DataCase + + alias Pleroma.Activity + alias Pleroma.Object + alias Pleroma.Tests.ObanHelpers + alias Pleroma.User + alias Pleroma.Web.ActivityPub.Transmogrifier + + import Pleroma.Factory + import ExUnit.CaptureLog + + setup_all do + Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end) + :ok + end + + test "it works for incoming deletes" do + activity = insert(:note_activity) + deleting_user = insert(:user) + + data = + File.read!("test/fixtures/mastodon-delete.json") + |> Poison.decode!() + + object = + data["object"] + |> Map.put("id", activity.data["object"]) + + data = + data + |> Map.put("object", object) + |> Map.put("actor", deleting_user.ap_id) + + {:ok, %Activity{actor: actor, local: false, data: %{"id" => id}}} = + Transmogrifier.handle_incoming(data) + + assert id == data["id"] + + # We delete the Create activity because 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 + + # Objects are replaced by a tombstone object. + object = Object.normalize(activity.data["object"]) + assert object.data["type"] == "Tombstone" + end + + test "it fails for incoming deletes with spoofed origin" do + activity = insert(:note_activity) + + data = + File.read!("test/fixtures/mastodon-delete.json") + |> Poison.decode!() + + object = + data["object"] + |> Map.put("id", activity.data["object"]) + + data = + data + |> Map.put("object", object) + + assert capture_log(fn -> + :error = Transmogrifier.handle_incoming(data) + end) =~ + "[error] Could not decode user at fetch http://mastodon.example.org/users/gargron, {:error, :nxdomain}" + + assert Activity.get_by_id(activity.id) + end + + @tag capture_log: true + test "it works for incoming user deletes" do + %{ap_id: ap_id} = insert(:user, ap_id: "http://mastodon.example.org/users/admin") + + data = + File.read!("test/fixtures/mastodon-delete-user.json") + |> Poison.decode!() + + {:ok, _} = Transmogrifier.handle_incoming(data) + ObanHelpers.perform_all() + + refute User.get_cached_by_ap_id(ap_id) + end + + test "it fails for incoming user deletes with spoofed origin" do + %{ap_id: ap_id} = insert(:user) + + data = + File.read!("test/fixtures/mastodon-delete-user.json") + |> Poison.decode!() + |> Map.put("actor", ap_id) + + assert capture_log(fn -> + assert :error == Transmogrifier.handle_incoming(data) + end) =~ "Object containment failed" + + assert User.get_cached_by_ap_id(ap_id) + end +end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 6057e360a..64e56d378 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -822,83 +822,6 @@ test "it works for incoming update activities which lock the account" do assert user.locked == true end - test "it works for incoming deletes" do - activity = insert(:note_activity) - deleting_user = insert(:user) - - data = - File.read!("test/fixtures/mastodon-delete.json") - |> Poison.decode!() - - object = - data["object"] - |> Map.put("id", activity.data["object"]) - - data = - data - |> Map.put("object", object) - |> Map.put("actor", deleting_user.ap_id) - - {: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 - activity = insert(:note_activity) - - data = - File.read!("test/fixtures/mastodon-delete.json") - |> Poison.decode!() - - object = - data["object"] - |> Map.put("id", activity.data["object"]) - - data = - data - |> Map.put("object", object) - - assert capture_log(fn -> - :error = Transmogrifier.handle_incoming(data) - end) =~ - "[error] Could not decode user at fetch http://mastodon.example.org/users/gargron, {:error, :nxdomain}" - - assert Activity.get_by_id(activity.id) - end - - @tag capture_log: true - test "it works for incoming user deletes" do - %{ap_id: ap_id} = insert(:user, ap_id: "http://mastodon.example.org/users/admin") - - data = - File.read!("test/fixtures/mastodon-delete-user.json") - |> Poison.decode!() - - {:ok, _} = Transmogrifier.handle_incoming(data) - ObanHelpers.perform_all() - - refute User.get_cached_by_ap_id(ap_id) - end - - test "it fails for incoming user deletes with spoofed origin" do - %{ap_id: ap_id} = insert(:user) - - data = - File.read!("test/fixtures/mastodon-delete-user.json") - |> Poison.decode!() - |> Map.put("actor", ap_id) - - assert capture_log(fn -> - assert :error == Transmogrifier.handle_incoming(data) - end) =~ "Object containment failed" - - assert User.get_cached_by_ap_id(ap_id) - end - test "it works for incoming unannounces with an existing notice" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => "hey"}) From db184a8eb495865334f47a24f8c5b1fec65450b6 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 14:37:14 +0200 Subject: [PATCH 07/34] DeleteValidator: Mastodon sends unaddressed deletes. --- .../object_validators/delete_validator.ex | 1 - test/web/activity_pub/object_validator_test.exs | 14 -------------- 2 files changed, 15 deletions(-) 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 8dd5c19ad..0eb31451c 100644 --- a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -32,7 +32,6 @@ def validate_data(cng) do |> validate_inclusion(:type, ["Delete"]) |> validate_same_domain() |> validate_object_presence() - |> validate_recipients_presence() end def validate_same_domain(cng) do diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index 64b9ee1ec..ab26d3501 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -59,20 +59,6 @@ test "it's invalid if the actor of the object and the actor of delete are from d assert {:actor, {"is not allowed to delete object", []}} in cng.errors end - - test "it's invalid if all the recipient fields are empty", %{ - valid_post_delete: valid_post_delete - } do - empty_recipients = - valid_post_delete - |> Map.put("to", []) - |> Map.put("cc", []) - - {:error, cng} = ObjectValidator.validate(empty_recipients, []) - - assert {:to, {"no recipients in any field", []}} in cng.errors - assert {:cc, {"no recipients in any field", []}} in cng.errors - end end describe "likes" do From 4dc5302f455e56d3c2cb669e8a70f52457690a86 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 15:26:23 +0200 Subject: [PATCH 08/34] Transmogrifier: Handle incoming deletes for non-user objects. --- .../web/activity_pub/object_validator.ex | 3 +- lib/pleroma/web/activity_pub/side_effects.ex | 12 ++++++++ .../web/activity_pub/transmogrifier.ex | 29 ++----------------- test/web/activity_pub/side_effects_test.exs | 23 +++++++++++++++ .../transmogrifier/delete_handling_test.exs | 6 ++-- 5 files changed, 42 insertions(+), 31 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex index 016f6e7a2..32f606917 100644 --- a/lib/pleroma/web/activity_pub/object_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validator.ex @@ -11,8 +11,9 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do alias Pleroma.Object alias Pleroma.User - alias Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator alias Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator + alias Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator + alias Pleroma.Web.ActivityPub.ObjectValidators.Types @spec validate(map(), keyword()) :: {:ok, map(), keyword()} | {:error, any()} def validate(object, meta) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 5981e7545..93698a834 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -28,6 +28,18 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do result end + # Tasks this handles: + # - Delete create activity + # - Replace object with Tombstone + # - Set up notification + def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, meta) do + with %Object{} = deleted_object <- Object.normalize(deleted_object), + {:ok, _, _} <- Object.delete(deleted_object) do + Notification.create_notifications(object) + {:ok, object, meta} + end + end + # Nothing to do def handle(object, meta) do {:ok, object, meta} diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 09119137b..855aab8d4 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -729,36 +729,13 @@ def handle_incoming( end end - # TODO: We presently assume that any actor on the same origin domain as the object being - # deleted has the rights to delete that object. A better way to validate whether or not - # the object should be deleted is to refetch the object URI, which should return either - # 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"} = data, _options ) do - object_id = Utils.get_ap_id(object_id) - - with actor <- Containment.get_actor(data), - {: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, actor: actor.ap_id) do + with {:ok, %User{}} <- ObjectValidator.fetch_actor(data), + {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do {:ok, activity} - else - nil -> - case User.get_cached_by_ap_id(object_id) do - %User{ap_id: ^actor} = user -> - User.delete(user) - - nil -> - :error - end - - _e -> - :error end end diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index 0b6b55156..eec9488e7 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -5,6 +5,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do use Pleroma.DataCase + alias Pleroma.Activity alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Repo @@ -15,6 +16,28 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do import Pleroma.Factory + describe "delete objects" do + setup do + user = insert(:user) + {:ok, post} = CommonAPI.post(user, %{"status" => "hey"}) + object = Object.normalize(post) + {:ok, delete_data, _meta} = Builder.delete(user, object.data["id"]) + {:ok, delete, _meta} = ActivityPub.persist(delete_data, local: true) + %{user: user, delete: delete, post: post, object: object} + end + + test "it handles object deletions", %{delete: delete, post: post, object: object} do + # In object deletions, the object is replaced by a tombstone and the + # create activity is deleted + + {:ok, _delete, _} = SideEffects.handle(delete) + + object = Object.get_by_id(object.id) + assert object.data["type"] == "Tombstone" + refute Activity.get_by_id(post.id) + end + end + describe "like objects" do setup do poster = insert(:user) diff --git a/test/web/activity_pub/transmogrifier/delete_handling_test.exs b/test/web/activity_pub/transmogrifier/delete_handling_test.exs index c15de5a95..64c908a05 100644 --- a/test/web/activity_pub/transmogrifier/delete_handling_test.exs +++ b/test/web/activity_pub/transmogrifier/delete_handling_test.exs @@ -68,7 +68,7 @@ test "it fails for incoming deletes with spoofed origin" do |> Map.put("object", object) assert capture_log(fn -> - :error = Transmogrifier.handle_incoming(data) + {:error, _} = Transmogrifier.handle_incoming(data) end) =~ "[error] Could not decode user at fetch http://mastodon.example.org/users/gargron, {:error, :nxdomain}" @@ -97,9 +97,7 @@ test "it fails for incoming user deletes with spoofed origin" do |> Poison.decode!() |> Map.put("actor", ap_id) - assert capture_log(fn -> - assert :error == Transmogrifier.handle_incoming(data) - end) =~ "Object containment failed" + assert match?({:error, _}, Transmogrifier.handle_incoming(data)) assert User.get_cached_by_ap_id(ap_id) end From 1fb383f368b861d7aea77770ba7be6e3dfe3468e Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 15:42:30 +0200 Subject: [PATCH 09/34] DeleteValidator: Deleting a user is valid. --- lib/pleroma/web/activity_pub/builder.ex | 15 +++++++++++++-- .../object_validators/common_validations.ex | 11 +++++++++++ .../object_validators/delete_validator.ex | 2 +- test/web/activity_pub/object_validator_test.exs | 7 ++++++- 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/activity_pub/builder.ex b/lib/pleroma/web/activity_pub/builder.ex index 5cc46c3ea..1345a3a3e 100644 --- a/lib/pleroma/web/activity_pub/builder.ex +++ b/lib/pleroma/web/activity_pub/builder.ex @@ -12,9 +12,20 @@ defmodule Pleroma.Web.ActivityPub.Builder do @spec delete(User.t(), String.t()) :: {:ok, map(), keyword()} def delete(actor, object_id) do - object = Object.normalize(object_id) + object = Object.normalize(object_id, false) - to = (object.data["to"] || []) ++ (object.data["cc"] || []) + user = !object && User.get_cached_by_ap_id(object_id) + + to = + case {object, user} do + {%Object{}, _} -> + # We are deleting an object, address everyone who was originally mentioned + (object.data["to"] || []) ++ (object.data["cc"] || []) + + {_, %User{follower_address: follower_address}} -> + # We are deleting a user, address the followers of that user + [follower_address] + end {:ok, %{ diff --git a/lib/pleroma/web/activity_pub/object_validators/common_validations.ex b/lib/pleroma/web/activity_pub/object_validators/common_validations.ex index e115d9526..d9a629a34 100644 --- a/lib/pleroma/web/activity_pub/object_validators/common_validations.ex +++ b/lib/pleroma/web/activity_pub/object_validators/common_validations.ex @@ -49,4 +49,15 @@ def validate_object_presence(cng, field_name \\ :object) do end end) end + + def validate_object_or_user_presence(cng, field_name \\ :object) do + cng + |> validate_change(field_name, fn field_name, object -> + if Object.get_cached_by_ap_id(object) || User.get_cached_by_ap_id(object) do + [] + else + [{field_name, "can't find object"}] + end + end) + end end 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 0eb31451c..fa1713b50 100644 --- a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -31,7 +31,7 @@ def validate_data(cng) do |> validate_required([:id, :type, :actor, :to, :cc, :object]) |> validate_inclusion(:type, ["Delete"]) |> validate_same_domain() - |> validate_object_presence() + |> validate_object_or_user_presence() end def validate_same_domain(cng) do diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index ab26d3501..83b21a9bc 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -15,14 +15,19 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidatorTest do {:ok, post_activity} = CommonAPI.post(user, %{"status" => "cancel me daddy"}) {:ok, valid_post_delete, _} = Builder.delete(user, post_activity.data["object"]) + {:ok, valid_user_delete, _} = Builder.delete(user, user.ap_id) - %{user: user, valid_post_delete: valid_post_delete} + %{user: user, valid_post_delete: valid_post_delete, valid_user_delete: valid_user_delete} end test "it is valid for a post deletion", %{valid_post_delete: valid_post_delete} do assert match?({:ok, _, _}, ObjectValidator.validate(valid_post_delete, [])) end + test "it is valid for a user deletion", %{valid_user_delete: valid_user_delete} do + assert match?({:ok, _, _}, ObjectValidator.validate(valid_user_delete, [])) + end + test "it's invalid if the id is missing", %{valid_post_delete: valid_post_delete} do no_id = valid_post_delete From 417eed4a2b10b0a1fd916839ddb03d0345966123 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 15:57:27 +0200 Subject: [PATCH 10/34] SideEffects: Handle deletions. --- lib/pleroma/web/activity_pub/side_effects.ex | 22 ++++++++++++++++++-- test/web/activity_pub/side_effects_test.exs | 14 ++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 93698a834..ac1d4c222 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do """ alias Pleroma.Notification alias Pleroma.Object + alias Pleroma.User alias Pleroma.Web.ActivityPub.Utils def handle(object, meta \\ []) @@ -33,10 +34,27 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do # - Replace object with Tombstone # - Set up notification def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, meta) do - with %Object{} = deleted_object <- Object.normalize(deleted_object), - {:ok, _, _} <- Object.delete(deleted_object) do + deleted_object = + Object.normalize(deleted_object, false) || User.get_cached_by_ap_id(deleted_object) + + result = + case deleted_object do + %Object{} -> + with {:ok, _, _} <- Object.delete(deleted_object) do + :ok + end + + %User{} -> + with {:ok, _} <- User.delete(deleted_object) do + :ok + end + end + + if result == :ok do Notification.create_notifications(object) {:ok, object, meta} + else + {:error, result} end end diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index eec9488e7..b3d0addc7 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -3,12 +3,15 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.ActivityPub.SideEffectsTest do + use Oban.Testing, repo: Pleroma.Repo use Pleroma.DataCase alias Pleroma.Activity alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Repo + alias Pleroma.User + alias Pleroma.Tests.ObanHelpers alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Builder alias Pleroma.Web.ActivityPub.SideEffects @@ -22,8 +25,10 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do {:ok, post} = CommonAPI.post(user, %{"status" => "hey"}) object = Object.normalize(post) {:ok, delete_data, _meta} = Builder.delete(user, object.data["id"]) + {:ok, delete_user_data, _meta} = Builder.delete(user, user.ap_id) {:ok, delete, _meta} = ActivityPub.persist(delete_data, local: true) - %{user: user, delete: delete, post: post, object: object} + {:ok, delete_user, _meta} = ActivityPub.persist(delete_user_data, local: true) + %{user: user, delete: delete, post: post, object: object, delete_user: delete_user} end test "it handles object deletions", %{delete: delete, post: post, object: object} do @@ -36,6 +41,13 @@ test "it handles object deletions", %{delete: delete, post: post, object: object assert object.data["type"] == "Tombstone" refute Activity.get_by_id(post.id) end + + test "it handles user deletions", %{delete_user: delete, user: user} do + {:ok, _delete, _} = SideEffects.handle(delete) + ObanHelpers.perform_all() + + refute User.get_cached_by_ap_id(user.ap_id) + end end describe "like objects" do From c9bfa51ea9c0048ffa4c0d3e28c196da2f38e384 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 15:58:37 +0200 Subject: [PATCH 11/34] Credo fixes. --- test/web/activity_pub/side_effects_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index b3d0addc7..fffe0ca38 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -10,8 +10,8 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do alias Pleroma.Notification alias Pleroma.Object alias Pleroma.Repo - alias Pleroma.User alias Pleroma.Tests.ObanHelpers + alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Builder alias Pleroma.Web.ActivityPub.SideEffects From fdd8e7f27697a7128e4e92020cdff6389c999acc Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 16:15:38 +0200 Subject: [PATCH 12/34] CommonAPI: Use common pipeline for deletions. --- lib/pleroma/web/activity_pub/side_effects.ex | 6 ++++-- lib/pleroma/web/common_api/common_api.ex | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index ac1d4c222..ef58fa399 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -30,7 +30,7 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do end # Tasks this handles: - # - Delete create activity + # - Delete and unpins the create activity # - Replace object with Tombstone # - Set up notification def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, meta) do @@ -40,7 +40,9 @@ def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, result = case deleted_object do %Object{} -> - with {:ok, _, _} <- Object.delete(deleted_object) do + with {:ok, _, activity} <- Object.delete(deleted_object), + %User{} = user <- User.get_cached_by_ap_id(deleted_object.data["actor"]) do + User.remove_pinnned_activity(user, activity) :ok end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index d1efe0c36..7cb8e47d0 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -77,8 +77,8 @@ def delete(activity_id, user) do {:find_activity, Activity.get_by_id_with_object(activity_id)}, %Object{} = object <- Object.normalize(activity), true <- User.superuser?(user) || user.ap_id == object.data["actor"], - {:ok, _} <- unpin(activity_id, user), - {:ok, delete} <- ActivityPub.delete(object) do + {: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} From 14c667219334c492ae0549ad0f1e062085d7d412 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 16:49:41 +0200 Subject: [PATCH 13/34] AP C2S: Use common pipelin for deletes. --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index d625530ec..e68d0763e 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -414,7 +414,8 @@ defp handle_user_activity(%User{} = user, %{"type" => "Create"} = params) do defp handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do with %Object{} = object <- Object.normalize(params["object"]), true <- user.is_moderator || user.ap_id == object.data["actor"], - {:ok, delete} <- ActivityPub.delete(object) do + {:ok, delete_data, _} <- Builder.delete(user, object.data["id"]), + {:ok, delete, _} <- Pipeline.common_pipeline(delete_data, local: true) do {:ok, delete} else _ -> {:error, dgettext("errors", "Can't delete object")} From 143353432a562c49f4432e74a549321c5b43650d Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 17:52:29 +0200 Subject: [PATCH 14/34] StreamerTest: Separate deletion test. --- test/web/streamer/streamer_test.exs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/web/streamer/streamer_test.exs b/test/web/streamer/streamer_test.exs index 8b8d8af6c..3c0f240f5 100644 --- a/test/web/streamer/streamer_test.exs +++ b/test/web/streamer/streamer_test.exs @@ -210,6 +210,12 @@ test "it sends to public" do Worker.push_to_socket(topics, "public", activity) Task.await(task) + end + + test "works for deletions" do + user = insert(:user) + other_user = insert(:user) + {:ok, activity} = CommonAPI.post(other_user, %{"status" => "Test"}) task = Task.async(fn -> From 4500fdc04c528331f7289745dc08a34ce18d4da7 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 17:53:02 +0200 Subject: [PATCH 15/34] DeleteValidator: Add internal helper field after validation. --- .../object_validators/delete_validator.ex | 16 ++++++++++++++++ test/web/activity_pub/object_validator_test.exs | 4 +++- 2 files changed, 19 insertions(+), 1 deletion(-) 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 fa1713b50..951cc1414 100644 --- a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -5,6 +5,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do use Ecto.Schema + alias Pleroma.Activity alias Pleroma.Web.ActivityPub.ObjectValidators.Types import Ecto.Changeset @@ -18,6 +19,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do field(:actor, Types.ObjectID) field(:to, Types.Recipients, default: []) field(:cc, Types.Recipients, default: []) + field(:deleted_activity_id) field(:object, Types.ObjectID) end @@ -26,12 +28,26 @@ def cast_data(data) do |> cast(data, __schema__(:fields)) end + def add_deleted_activity_id(cng) do + object = + cng + |> get_field(:object) + + with %Activity{id: id} <- Activity.get_create_by_object_ap_id(object) do + cng + |> put_change(:deleted_activity_id, id) + else + _ -> cng + end + end + def validate_data(cng) do cng |> validate_required([:id, :type, :actor, :to, :cc, :object]) |> validate_inclusion(:type, ["Delete"]) |> validate_same_domain() |> validate_object_or_user_presence() + |> add_deleted_activity_id() end def validate_same_domain(cng) do diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index 83b21a9bc..9e0589722 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -21,7 +21,9 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidatorTest do end test "it is valid for a post deletion", %{valid_post_delete: valid_post_delete} do - assert match?({:ok, _, _}, ObjectValidator.validate(valid_post_delete, [])) + {:ok, valid_post_delete_u, _} = ObjectValidator.validate(valid_post_delete, []) + + assert valid_post_delete_u["deleted_activity_id"] end test "it is valid for a user deletion", %{valid_user_delete: valid_user_delete} do From c832d96fc9fc0b93befdf3a7064a8c9236e96d07 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 17:58:09 +0200 Subject: [PATCH 16/34] SideEffects: Stream out deletes. --- lib/pleroma/web/activity_pub/side_effects.ex | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index ef58fa399..d260e0069 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -9,6 +9,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do alias Pleroma.Object alias Pleroma.User alias Pleroma.Web.ActivityPub.Utils + alias Pleroma.Web.ActivityPub.ActivityPub def handle(object, meta \\ []) @@ -40,9 +41,12 @@ def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, result = case deleted_object do %Object{} -> - with {:ok, _, activity} <- Object.delete(deleted_object), + with {:ok, deleted_object, activity} <- Object.delete(deleted_object), %User{} = user <- User.get_cached_by_ap_id(deleted_object.data["actor"]) do User.remove_pinnned_activity(user, activity) + + ActivityPub.stream_out(object) + ActivityPub.stream_out_participations(deleted_object, user) :ok end From 315b773dd9fa185aef75b115efd90ac92113e6c3 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 17:58:31 +0200 Subject: [PATCH 17/34] ObjectValidator: Refactor. --- test/web/activity_pub/object_validator_test.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index 9e0589722..1d3646487 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -21,9 +21,9 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidatorTest do end test "it is valid for a post deletion", %{valid_post_delete: valid_post_delete} do - {:ok, valid_post_delete_u, _} = ObjectValidator.validate(valid_post_delete, []) + {:ok, valid_post_delete, _} = ObjectValidator.validate(valid_post_delete, []) - assert valid_post_delete_u["deleted_activity_id"] + assert valid_post_delete["deleted_activity_id"] end test "it is valid for a user deletion", %{valid_user_delete: valid_user_delete} do From 3d0dc58e2e0a84cb46df5339596205f7baceb0a4 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 18:10:36 +0200 Subject: [PATCH 18/34] SideEffectsTest: Test streaming. --- test/web/activity_pub/side_effects_test.exs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index fffe0ca38..f5c57d887 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -18,6 +18,7 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do alias Pleroma.Web.CommonAPI import Pleroma.Factory + import Mock describe "delete objects" do setup do @@ -33,9 +34,16 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do test "it handles object deletions", %{delete: delete, post: post, object: object} do # In object deletions, the object is replaced by a tombstone and the - # create activity is deleted + # create activity is deleted. - {:ok, _delete, _} = SideEffects.handle(delete) + with_mock Pleroma.Web.ActivityPub.ActivityPub, + 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" From ab60ee17765ee9d7dcb69cbf9c0630b97d4f5a93 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 18:19:39 +0200 Subject: [PATCH 19/34] SideEffects: On deletion, reduce the User note count. --- lib/pleroma/web/activity_pub/side_effects.ex | 2 ++ test/web/activity_pub/side_effects_test.exs | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index d260e0069..4fec3a797 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -34,6 +34,7 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do # - Delete and unpins the create activity # - Replace object with Tombstone # - Set up notification + # - Reduce the user note count def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, meta) do deleted_object = Object.normalize(deleted_object, false) || User.get_cached_by_ap_id(deleted_object) @@ -45,6 +46,7 @@ def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, %User{} = user <- User.get_cached_by_ap_id(deleted_object.data["actor"]) do User.remove_pinnned_activity(user, activity) + {:ok, user} = ActivityPub.decrease_note_count_if_public(user, deleted_object) ActivityPub.stream_out(object) ActivityPub.stream_out_participations(deleted_object, user) :ok diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index f5c57d887..06b3400d8 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -32,15 +32,16 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do %{user: user, delete: delete, post: post, object: object, delete_user: delete_user} end - test "it handles object deletions", %{delete: delete, post: post, object: object} do + test "it handles object deletions", %{delete: delete, post: post, object: object, user: user} do # In object deletions, the object is replaced by a tombstone and the # create activity is deleted. - with_mock Pleroma.Web.ActivityPub.ActivityPub, + 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 @@ -48,6 +49,9 @@ test "it handles object deletions", %{delete: delete, post: post, object: object 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 end test "it handles user deletions", %{delete_user: delete, user: user} do From 60db58a1c6a2f139960d3db19cba08a496e6ccf4 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 18:38:37 +0200 Subject: [PATCH 20/34] Credo fixes. --- lib/pleroma/web/activity_pub/side_effects.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 4fec3a797..cf31de120 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -8,8 +8,8 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do alias Pleroma.Notification alias Pleroma.Object alias Pleroma.User - alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.ActivityPub + alias Pleroma.Web.ActivityPub.Utils def handle(object, meta \\ []) From 500f5ec14eb02cd1c5a07970a557756b590caab0 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 19:47:13 +0200 Subject: [PATCH 21/34] SideEffects: On deletion, reduce the reply count cache --- lib/pleroma/web/activity_pub/side_effects.ex | 6 ++++++ test/web/activity_pub/side_effects_test.exs | 22 ++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index cf31de120..39b0f384b 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -35,6 +35,7 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do # - Replace object with Tombstone # - Set up notification # - Reduce the user note count + # - TODO: Reduce the reply count def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, meta) do deleted_object = Object.normalize(deleted_object, false) || User.get_cached_by_ap_id(deleted_object) @@ -47,6 +48,11 @@ def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, User.remove_pinnned_activity(user, activity) {:ok, user} = ActivityPub.decrease_note_count_if_public(user, deleted_object) + + if in_reply_to = deleted_object.data["inReplyTo"] do + Object.decrease_replies_count(in_reply_to) + end + ActivityPub.stream_out(object) ActivityPub.stream_out_participations(deleted_object, user) :ok diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index 06b3400d8..ce34eed4c 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -23,19 +23,25 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do describe "delete objects" do setup do user = insert(:user) - {:ok, post} = CommonAPI.post(user, %{"status" => "hey"}) + other_user = insert(:user) + + {:ok, op} = CommonAPI.post(other_user, %{"status" => "big oof"}) + {:ok, post} = CommonAPI.post(user, %{"status" => "hey", "in_reply_to_id" => op}) object = Object.normalize(post) {:ok, delete_data, _meta} = Builder.delete(user, object.data["id"]) {:ok, delete_user_data, _meta} = Builder.delete(user, user.ap_id) {:ok, delete, _meta} = ActivityPub.persist(delete_data, local: true) {:ok, delete_user, _meta} = ActivityPub.persist(delete_user_data, local: true) - %{user: user, delete: delete, post: post, object: object, delete_user: delete_user} + %{user: user, delete: delete, post: post, object: object, delete_user: delete_user, op: op} end - test "it handles object deletions", %{delete: delete, post: post, object: object, user: user} do - # In object deletions, the object is replaced by a tombstone and the - # create activity is deleted. - + test "it handles object deletions", %{ + 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 @@ -52,6 +58,10 @@ test "it handles object deletions", %{delete: delete, post: post, object: object 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 From 5da08c2b73f9ce1f369434fbd2c11092007e4910 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 19:53:30 +0200 Subject: [PATCH 22/34] SideEffects: Fix comment --- lib/pleroma/web/activity_pub/side_effects.ex | 2 +- test/user_test.exs | 28 +------------------- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 39b0f384b..139e609f4 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -35,7 +35,7 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do # - Replace object with Tombstone # - Set up notification # - Reduce the user note count - # - TODO: Reduce the reply count + # - Reduce the reply count def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, meta) do deleted_object = Object.normalize(deleted_object, false) || User.get_cached_by_ap_id(deleted_object) diff --git a/test/user_test.exs b/test/user_test.exs index 347c5be72..23afc605c 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -15,7 +15,6 @@ defmodule Pleroma.UserTest do use Pleroma.DataCase use Oban.Testing, repo: Pleroma.Repo - import Mock import Pleroma.Factory import ExUnit.CaptureLog @@ -1131,7 +1130,7 @@ test ".delete_user_activities deletes all create activities", %{user: user} do User.delete_user_activities(user) - # TODO: Remove favorites, repeats, delete activities. + # TODO: Test removal favorites, repeats, delete activities. refute Activity.get_by_id(activity.id) end @@ -1180,31 +1179,6 @@ test "it deletes a user, all follow relationships and all activities", %{user: u refute Activity.get_by_id(like_two.id) refute Activity.get_by_id(repeat.id) end - - test_with_mock "it sends out User Delete activity", - %{user: user}, - Pleroma.Web.ActivityPub.Publisher, - [:passthrough], - [] do - Pleroma.Config.put([:instance, :federating], true) - - {:ok, follower} = User.get_or_fetch_by_ap_id("http://mastodon.example.org/users/admin") - {:ok, _} = User.follow(follower, user) - - {:ok, job} = User.delete(user) - {:ok, _user} = ObanHelpers.perform(job) - - assert ObanHelpers.member?( - %{ - "op" => "publish_one", - "params" => %{ - "inbox" => "http://mastodon.example.org/inbox", - "id" => "pleroma:fakeid" - } - }, - all_enqueued(worker: Pleroma.Workers.PublisherWorker) - ) - end end test "get_public_key_for_ap_id fetches a user that's not in the db" do From 3b443cbc1dd79b0450e17192aa51a00282b54d2e Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 20:08:25 +0200 Subject: [PATCH 23/34] User: Use common pipeline to delete user activities --- lib/pleroma/user.ex | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index b451202b2..c780f99eb 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -29,7 +29,9 @@ defmodule Pleroma.User do alias Pleroma.UserRelationship alias Pleroma.Web alias Pleroma.Web.ActivityPub.ActivityPub + alias Pleroma.Web.ActivityPub.Builder alias Pleroma.Web.ActivityPub.ObjectValidators.Types + alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI.Utils, as: CommonUtils @@ -1427,8 +1429,6 @@ def perform(:force_password_reset, user), do: force_password_reset(user) @spec perform(atom(), User.t()) :: {:ok, User.t()} def perform(:delete, %User{} = user) do - {:ok, _user} = ActivityPub.delete(user) - # Remove all relationships user |> get_followers() @@ -1531,21 +1531,23 @@ def follow_import(%User{} = follower, followed_identifiers) }) end - def delete_user_activities(%User{ap_id: ap_id}) do + def delete_user_activities(%User{ap_id: ap_id} = user) do ap_id |> Activity.Queries.by_actor() |> RepoStreamer.chunk_stream(50) - |> Stream.each(fn activities -> Enum.each(activities, &delete_activity/1) end) + |> Stream.each(fn activities -> + Enum.each(activities, fn activity -> delete_activity(activity, user) end) + end) |> Stream.run() end - defp delete_activity(%{data: %{"type" => "Create"}} = activity) do - activity - |> Object.normalize() - |> ActivityPub.delete() + defp delete_activity(%{data: %{"type" => "Create", "object" => object}}, user) do + {:ok, delete_data, _} = Builder.delete(user, object) + + Pipeline.common_pipeline(delete_data, local: true) end - defp delete_activity(%{data: %{"type" => "Like"}} = activity) do + defp delete_activity(%{data: %{"type" => "Like"}} = activity, _user) do object = Object.normalize(activity) activity.actor @@ -1553,7 +1555,7 @@ defp delete_activity(%{data: %{"type" => "Like"}} = activity) do |> ActivityPub.unlike(object) end - defp delete_activity(%{data: %{"type" => "Announce"}} = activity) do + defp delete_activity(%{data: %{"type" => "Announce"}} = activity, _user) do object = Object.normalize(activity) activity.actor @@ -1561,7 +1563,7 @@ defp delete_activity(%{data: %{"type" => "Announce"}} = activity) do |> ActivityPub.unannounce(object) end - defp delete_activity(_activity), do: "Doing nothing" + defp delete_activity(_activity, _user), do: "Doing nothing" def html_filter_policy(%User{no_rich_text: true}) do Pleroma.HTML.Scrubber.TwitterText From 999d639873b70f75c340dbac3360d25bca27a998 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 20:13:47 +0200 Subject: [PATCH 24/34] ActivityPub: Remove `delete` function. This is handled by the common pipeline now. --- lib/pleroma/web/activity_pub/activity_pub.ex | 61 --------- test/web/activity_pub/activity_pub_test.exs | 137 ------------------- 2 files changed, 198 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 1f4a09370..51f002129 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -519,67 +519,6 @@ defp do_unfollow(follower, followed, activity_id, local) do end end - @spec delete(User.t() | Object.t(), keyword()) :: {:ok, User.t() | Object.t()} | {:error, any()} - def delete(entity, options \\ []) do - with {:ok, result} <- Repo.transaction(fn -> do_delete(entity, options) end) do - result - end - end - - defp do_delete(%User{ap_id: ap_id, follower_address: follower_address} = user, _) do - with data <- %{ - "to" => [follower_address], - "type" => "Delete", - "actor" => ap_id, - "object" => %{"type" => "Person", "id" => ap_id} - }, - {:ok, activity} <- insert(data, true, true, true), - :ok <- maybe_federate(activity) do - {:ok, user} - end - end - - defp do_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 create_activity <- Activity.get_create_by_object_ap_id(id), - data <- - %{ - "type" => "Delete", - "actor" => actor, - "object" => id, - "to" => to, - "deleted_activity_id" => create_activity && create_activity.id - } - |> maybe_put("id", activity_id), - {:ok, activity} <- insert(data, local, false), - {:ok, object, _create_activity} <- Object.delete(object), - stream_out_participations(object, user), - _ <- decrease_replies_count_if_reply(object), - {:ok, _actor} <- decrease_note_count_if_public(user, object), - :ok <- maybe_federate(activity) do - {:ok, activity} - else - {:error, error} -> - Repo.rollback(error) - end - end - - defp do_delete(%Object{data: %{"type" => "Tombstone", "id" => ap_id}}, _) do - activity = - ap_id - |> Activity.Queries.by_object_id() - |> Activity.Queries.by_type("Delete") - |> Repo.one() - - {:ok, activity} - end - @spec block(User.t(), User.t(), String.t() | nil, boolean()) :: {:ok, Activity.t()} | {:error, any()} def block(blocker, blocked, activity_id \\ nil, local \\ true) do diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index edd7dfb22..b93ee708e 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -1331,143 +1331,6 @@ test "creates an undo activity for the last block" do end end - describe "deletion" do - setup do: clear_config([:instance, :rewrite_policy]) - - test "it reverts deletion on error" do - note = insert(:note_activity) - object = Object.normalize(note) - - with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do - assert {:error, :reverted} = ActivityPub.delete(object) - end - - assert Repo.aggregate(Activity, :count, :id) == 1 - assert Repo.get(Object, object.id) == object - assert Activity.get_by_id(note.id) == note - end - - test "it creates a delete activity and deletes the original object" do - note = insert(:note_activity) - object = Object.normalize(note) - {:ok, delete} = ActivityPub.delete(object) - - assert delete.data["type"] == "Delete" - assert delete.data["actor"] == note.data["actor"] - assert delete.data["object"] == object.data["id"] - - assert Activity.get_by_id(delete.id) != nil - - assert Repo.get(Object, object.id).data["type"] == "Tombstone" - end - - test "it doesn't fail when an activity was already deleted" do - {:ok, delete} = insert(:note_activity) |> Object.normalize() |> ActivityPub.delete() - - assert {:ok, ^delete} = delete |> Object.normalize() |> ActivityPub.delete() - end - - test "decrements user note count only for public activities" do - user = insert(:user, note_count: 10) - - {:ok, a1} = - CommonAPI.post(User.get_cached_by_id(user.id), %{ - "status" => "yeah", - "visibility" => "public" - }) - - {:ok, a2} = - CommonAPI.post(User.get_cached_by_id(user.id), %{ - "status" => "yeah", - "visibility" => "unlisted" - }) - - {:ok, a3} = - CommonAPI.post(User.get_cached_by_id(user.id), %{ - "status" => "yeah", - "visibility" => "private" - }) - - {:ok, a4} = - CommonAPI.post(User.get_cached_by_id(user.id), %{ - "status" => "yeah", - "visibility" => "direct" - }) - - {:ok, _} = Object.normalize(a1) |> ActivityPub.delete() - {:ok, _} = Object.normalize(a2) |> ActivityPub.delete() - {:ok, _} = Object.normalize(a3) |> ActivityPub.delete() - {:ok, _} = Object.normalize(a4) |> ActivityPub.delete() - - user = User.get_cached_by_id(user.id) - assert user.note_count == 10 - end - - test "it creates a delete activity and checks that it is also sent to users mentioned by the deleted object" do - user = insert(:user) - note = insert(:note_activity) - object = Object.normalize(note) - - {:ok, object} = - object - |> Object.change(%{ - data: %{ - "actor" => object.data["actor"], - "id" => object.data["id"], - "to" => [user.ap_id], - "type" => "Note" - } - }) - |> Object.update_and_set_cache() - - {:ok, delete} = ActivityPub.delete(object) - - assert user.ap_id in delete.data["to"] - end - - test "decreases reply count" do - user = insert(:user) - user2 = insert(:user) - - {:ok, activity} = CommonAPI.post(user, %{"status" => "1", "visibility" => "public"}) - reply_data = %{"status" => "1", "in_reply_to_status_id" => activity.id} - ap_id = activity.data["id"] - - {:ok, public_reply} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "public")) - {:ok, unlisted_reply} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "unlisted")) - {:ok, private_reply} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "private")) - {:ok, direct_reply} = CommonAPI.post(user2, Map.put(reply_data, "visibility", "direct")) - - _ = CommonAPI.delete(direct_reply.id, user2) - assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) - assert object.data["repliesCount"] == 2 - - _ = CommonAPI.delete(private_reply.id, user2) - assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) - assert object.data["repliesCount"] == 2 - - _ = CommonAPI.delete(public_reply.id, user2) - assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) - assert object.data["repliesCount"] == 1 - - _ = CommonAPI.delete(unlisted_reply.id, user2) - assert %{data: data, object: object} = Activity.get_by_ap_id_with_object(ap_id) - assert object.data["repliesCount"] == 0 - end - - test "it passes delete activity through MRF before deleting the object" do - Pleroma.Config.put([:instance, :rewrite_policy], Pleroma.Web.ActivityPub.MRF.DropPolicy) - - note = insert(:note_activity) - object = Object.normalize(note) - - {:error, {:reject, _}} = ActivityPub.delete(object) - - assert Activity.get_by_id(note.id) - assert Repo.get(Object, object.id).data["type"] == object.data["type"] - end - end - describe "timeline post-processing" do test "it filters broken threads" do user1 = insert(:user) From 32b8386edeec3e9b24123c3ccc81a22f1edd5a1c Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 30 Apr 2020 21:23:18 +0200 Subject: [PATCH 25/34] DeleteValidator: Don't federate local deletions of remote objects. Closes #1497 --- .../web/activity_pub/object_validator.ex | 8 +- .../object_validators/delete_validator.ex | 20 ++++- lib/pleroma/web/activity_pub/pipeline.ex | 4 +- .../activity_pub/object_validator_test.exs | 17 +++- test/web/common_api/common_api_test.exs | 80 +++++++++++++++++++ 5 files changed, 119 insertions(+), 10 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex index 32f606917..479f922f5 100644 --- a/lib/pleroma/web/activity_pub/object_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validator.ex @@ -19,11 +19,11 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do def validate(object, meta) def validate(%{"type" => "Delete"} = object, meta) do - with {:ok, object} <- - object - |> DeleteValidator.cast_and_validate() - |> Ecto.Changeset.apply_action(:insert) do + with cng <- DeleteValidator.cast_and_validate(object), + do_not_federate <- DeleteValidator.do_not_federate?(cng), + {:ok, object} <- Ecto.Changeset.apply_action(cng, :insert) do object = stringify_keys(object) + meta = Keyword.put(meta, :do_not_federate, do_not_federate) {:ok, object, meta} end end 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 951cc1414..a2eff7b69 100644 --- a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -6,6 +6,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do use Ecto.Schema alias Pleroma.Activity + alias Pleroma.User alias Pleroma.Web.ActivityPub.ObjectValidators.Types import Ecto.Changeset @@ -45,12 +46,17 @@ def validate_data(cng) do cng |> validate_required([:id, :type, :actor, :to, :cc, :object]) |> validate_inclusion(:type, ["Delete"]) - |> validate_same_domain() + |> validate_actor_presence() + |> validate_deletion_rights() |> validate_object_or_user_presence() |> add_deleted_activity_id() end - def validate_same_domain(cng) do + def do_not_federate?(cng) do + !same_domain?(cng) + end + + defp same_domain?(cng) do actor_domain = cng |> get_field(:actor) @@ -63,11 +69,17 @@ def validate_same_domain(cng) do |> URI.parse() |> (& &1.host).() - if object_domain != actor_domain do + object_domain == actor_domain + end + + def validate_deletion_rights(cng) do + actor = User.get_cached_by_ap_id(get_field(cng, :actor)) + + if User.superuser?(actor) || same_domain?(cng) do cng - |> add_error(:actor, "is not allowed to delete object") else cng + |> add_error(:actor, "is not allowed to delete object") end end diff --git a/lib/pleroma/web/activity_pub/pipeline.ex b/lib/pleroma/web/activity_pub/pipeline.ex index 7ccee54c9..017e39abb 100644 --- a/lib/pleroma/web/activity_pub/pipeline.ex +++ b/lib/pleroma/web/activity_pub/pipeline.ex @@ -29,7 +29,9 @@ def common_pipeline(object, meta) do defp maybe_federate(activity, meta) do with {:ok, local} <- Keyword.fetch(meta, :local) do - if local do + do_not_federate = meta[:do_not_federate] + + if !do_not_federate && local do Federator.publish(activity) {:ok, :federated} else diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index 1d3646487..412db09ff 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -52,9 +52,11 @@ test "it's invalid if the object doesn't exist", %{valid_post_delete: valid_post test "it's invalid if the actor of the object and the actor of delete are from different domains", %{valid_post_delete: valid_post_delete} do + valid_user = insert(:user) + valid_other_actor = valid_post_delete - |> Map.put("actor", valid_post_delete["actor"] <> "1") + |> Map.put("actor", valid_user.ap_id) assert match?({:ok, _, _}, ObjectValidator.validate(valid_other_actor, [])) @@ -66,6 +68,19 @@ test "it's invalid if the actor of the object and the actor of delete are from d assert {:actor, {"is not allowed to delete object", []}} in cng.errors end + + test "it's valid if the actor of the object is a local superuser", + %{valid_post_delete: valid_post_delete} do + user = + insert(:user, local: true, is_moderator: true, ap_id: "https://gensokyo.2hu/users/raymoo") + + valid_other_actor = + valid_post_delete + |> Map.put("actor", user.ap_id) + + {:ok, _, meta} = ObjectValidator.validate(valid_other_actor, []) + assert meta[:do_not_federate] + end end describe "likes" do diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 1758662b0..32d91ce02 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -9,11 +9,13 @@ defmodule Pleroma.Web.CommonAPITest do alias Pleroma.Object alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub + alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.AdminAPI.AccountView alias Pleroma.Web.CommonAPI import Pleroma.Factory + import Mock require Pleroma.Constants @@ -21,6 +23,84 @@ defmodule Pleroma.Web.CommonAPITest do setup do: clear_config([:instance, :limit]) setup do: clear_config([:instance, :max_pinned_statuses]) + describe "deletion" do + test "it allows users to delete their posts" do + user = insert(:user) + + {:ok, post} = CommonAPI.post(user, %{"status" => "namu amida butsu"}) + + 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 does not allow a user to delete their posts" do + user = insert(:user) + other_user = insert(:user) + + {:ok, post} = CommonAPI.post(user, %{"status" => "namu amida butsu"}) + + assert {:error, "Could not delete"} = CommonAPI.delete(post.id, other_user) + assert Activity.get_by_id(post.id) + end + + test "it allows moderators to delete other user's posts" do + user = insert(:user) + moderator = insert(:user, is_moderator: true) + + {:ok, post} = CommonAPI.post(user, %{"status" => "namu amida butsu"}) + + assert {:ok, delete} = CommonAPI.delete(post.id, moderator) + assert delete.local + + refute Activity.get_by_id(post.id) + end + + test "it allows admins to delete other user's posts" do + user = insert(:user) + moderator = insert(:user, is_admin: true) + + {:ok, post} = CommonAPI.post(user, %{"status" => "namu amida butsu"}) + + assert {:ok, delete} = CommonAPI.delete(post.id, moderator) + assert delete.local + + refute Activity.get_by_id(post.id) + end + + test "superusers deleting non-local posts won't federate the delete" do + # This is the user of the ingested activity + _user = + insert(:user, + local: false, + ap_id: "http://mastodon.example.org/users/admin", + last_refreshed_at: NaiveDateTime.utc_now() + ) + + moderator = insert(:user, is_admin: true) + + data = + File.read!("test/fixtures/mastodon-post-activity.json") + |> Jason.decode!() + + {:ok, post} = Transmogrifier.handle_incoming(data) + + with_mock Pleroma.Web.Federator, + publish: fn _ -> nil end do + assert {:ok, delete} = CommonAPI.delete(post.id, moderator) + assert delete.local + refute called(Pleroma.Web.Federator.publish(:_)) + end + + refute Activity.get_by_id(post.id) + end + end + test "favoriting race condition" do user = insert(:user) users_serial = insert_list(10, :user) From 5f42e6629d862f0a8dcbbd1527998685b6932d52 Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 1 May 2020 13:34:47 +0200 Subject: [PATCH 26/34] DeleteValidator: Only allow deletion of certain types. --- .../object_validators/common_validations.ex | 48 ++++++++++++------- .../object_validators/delete_validator.ex | 12 ++++- lib/pleroma/web/activity_pub/side_effects.ex | 1 + .../activity_pub/object_validator_test.exs | 19 ++++++++ 4 files changed, 63 insertions(+), 17 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validators/common_validations.ex b/lib/pleroma/web/activity_pub/object_validators/common_validations.ex index d9a629a34..4e6ee2034 100644 --- a/lib/pleroma/web/activity_pub/object_validators/common_validations.ex +++ b/lib/pleroma/web/activity_pub/object_validators/common_validations.ex @@ -28,7 +28,9 @@ def validate_recipients_presence(cng, fields \\ [:to, :cc]) do end end - def validate_actor_presence(cng, field_name \\ :actor) do + def validate_actor_presence(cng, options \\ []) do + field_name = Keyword.get(options, :field_name, :actor) + cng |> validate_change(field_name, fn field_name, actor -> if User.get_cached_by_ap_id(actor) do @@ -39,25 +41,39 @@ def validate_actor_presence(cng, field_name \\ :actor) do end) end - def validate_object_presence(cng, field_name \\ :object) do + def validate_object_presence(cng, options \\ []) do + field_name = Keyword.get(options, :field_name, :object) + allowed_types = Keyword.get(options, :allowed_types, false) + cng - |> validate_change(field_name, fn field_name, object -> - if Object.get_cached_by_ap_id(object) do - [] - else - [{field_name, "can't find object"}] + |> validate_change(field_name, fn field_name, object_id -> + object = Object.get_cached_by_ap_id(object_id) + + cond do + !object -> + [{field_name, "can't find object"}] + + object && allowed_types && object.data["type"] not in allowed_types -> + [{field_name, "object not in allowed types"}] + + true -> + [] end end) end - def validate_object_or_user_presence(cng, field_name \\ :object) do - cng - |> validate_change(field_name, fn field_name, object -> - if Object.get_cached_by_ap_id(object) || User.get_cached_by_ap_id(object) do - [] - else - [{field_name, "can't find object"}] - end - end) + def validate_object_or_user_presence(cng, options \\ []) do + field_name = Keyword.get(options, :field_name, :object) + options = Keyword.put(options, :field_name, field_name) + + actor_cng = + cng + |> validate_actor_presence(options) + + object_cng = + cng + |> validate_object_presence(options) + + if actor_cng.valid?, do: actor_cng, else: object_cng end end 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 a2eff7b69..256ac70b6 100644 --- a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -42,13 +42,23 @@ def add_deleted_activity_id(cng) do end end + @deletable_types ~w{ + Answer + Article + Audio + Event + Note + Page + Question + Video + } def validate_data(cng) do cng |> validate_required([:id, :type, :actor, :to, :cc, :object]) |> validate_inclusion(:type, ["Delete"]) |> validate_actor_presence() |> validate_deletion_rights() - |> validate_object_or_user_presence() + |> validate_object_or_user_presence(allowed_types: @deletable_types) |> add_deleted_activity_id() end diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 139e609f4..52bd5179f 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -36,6 +36,7 @@ def handle(%{data: %{"type" => "Like"}} = object, meta) do # - Set up notification # - Reduce the user note count # - Reduce the reply count + # - Stream out the activity def handle(%{data: %{"type" => "Delete", "object" => deleted_object}} = object, meta) do deleted_object = Object.normalize(deleted_object, false) || User.get_cached_by_ap_id(deleted_object) diff --git a/test/web/activity_pub/object_validator_test.exs b/test/web/activity_pub/object_validator_test.exs index 412db09ff..7ab1c8ffb 100644 --- a/test/web/activity_pub/object_validator_test.exs +++ b/test/web/activity_pub/object_validator_test.exs @@ -1,6 +1,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidatorTest do use Pleroma.DataCase + alias Pleroma.Object alias Pleroma.Web.ActivityPub.Builder alias Pleroma.Web.ActivityPub.ObjectValidator alias Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator @@ -26,6 +27,24 @@ test "it is valid for a post deletion", %{valid_post_delete: valid_post_delete} assert valid_post_delete["deleted_activity_id"] end + test "it is invalid if the object isn't in a list of certain types", %{ + valid_post_delete: valid_post_delete + } do + object = Object.get_by_ap_id(valid_post_delete["object"]) + + data = + object.data + |> Map.put("type", "Like") + + {:ok, _object} = + object + |> Ecto.Changeset.change(%{data: data}) + |> Object.update_and_set_cache() + + {:error, cng} = ObjectValidator.validate(valid_post_delete, []) + assert {:object, {"object not in allowed types", []}} in cng.errors + end + test "it is valid for a user deletion", %{valid_user_delete: valid_user_delete} do assert match?({:ok, _, _}, ObjectValidator.validate(valid_user_delete, [])) end From 51f1dbf0a2bf6b61fdef0be56fd8f20a40827100 Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 1 May 2020 14:05:25 +0200 Subject: [PATCH 27/34] User deletion mix task: Use common pipeline. --- lib/mix/tasks/pleroma/user.ex | 7 +++++-- test/tasks/user_test.exs | 18 +++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/mix/tasks/pleroma/user.ex b/lib/mix/tasks/pleroma/user.ex index 40dd9bdc0..da140ac86 100644 --- a/lib/mix/tasks/pleroma/user.ex +++ b/lib/mix/tasks/pleroma/user.ex @@ -8,6 +8,8 @@ defmodule Mix.Tasks.Pleroma.User do alias Ecto.Changeset alias Pleroma.User alias Pleroma.UserInviteToken + alias Pleroma.Web.ActivityPub.Builder + alias Pleroma.Web.ActivityPub.Pipeline @shortdoc "Manages Pleroma users" @moduledoc File.read!("docs/administration/CLI_tasks/user.md") @@ -96,8 +98,9 @@ def run(["new", nickname, email | rest]) do def run(["rm", nickname]) do start_pleroma() - with %User{local: true} = user <- User.get_cached_by_nickname(nickname) do - User.perform(:delete, user) + with %User{local: true} = user <- User.get_cached_by_nickname(nickname), + {:ok, delete_data, _} <- Builder.delete(user, user.ap_id), + {:ok, _delete, _} <- Pipeline.common_pipeline(delete_data, local: true) do shell_info("User #{nickname} deleted.") else _ -> shell_error("No local user #{nickname}") diff --git a/test/tasks/user_test.exs b/test/tasks/user_test.exs index 8df835b56..ab56f07c1 100644 --- a/test/tasks/user_test.exs +++ b/test/tasks/user_test.exs @@ -4,14 +4,17 @@ defmodule Mix.Tasks.Pleroma.UserTest do alias Pleroma.Repo + alias Pleroma.Tests.ObanHelpers alias Pleroma.User alias Pleroma.Web.OAuth.Authorization alias Pleroma.Web.OAuth.Token use Pleroma.DataCase + use Oban.Testing, repo: Pleroma.Repo - import Pleroma.Factory import ExUnit.CaptureIO + import Mock + import Pleroma.Factory setup_all do Mix.shell(Mix.Shell.Process) @@ -87,12 +90,17 @@ test "user is not created" do test "user is deleted" do user = insert(:user) - Mix.Tasks.Pleroma.User.run(["rm", user.nickname]) + 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_received {:mix_shell, :info, [message]} + assert message =~ " deleted" + refute User.get_by_nickname(user.nickname) - refute User.get_by_nickname(user.nickname) + assert called(Pleroma.Web.Federator.publish(:_)) + end end test "no user to delete" do From ebbd9c7f369f986b7a66f66eddab91537c490c79 Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 1 May 2020 14:22:39 +0200 Subject: [PATCH 28/34] AdminAPIController: Refactor. --- lib/pleroma/web/admin_api/admin_api_controller.ex | 14 ++------------ test/web/admin_api/admin_api_controller_test.exs | 2 +- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 816c11e01..c09584fd1 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -133,18 +133,8 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do action_fallback(:errors) - def user_delete(%{assigns: %{user: admin}} = conn, %{"nickname" => nickname}) do - user = User.get_cached_by_nickname(nickname) - User.delete(user) - - ModerationLog.insert_log(%{ - actor: admin, - subject: [user], - action: "delete" - }) - - conn - |> json(nickname) + def user_delete(conn, %{"nickname" => nickname}) do + user_delete(conn, %{"nicknames" => [nickname]}) end def user_delete(%{assigns: %{user: admin}} = conn, %{"nicknames" => nicknames}) do diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index f80dbf8dd..c92715fab 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -156,7 +156,7 @@ test "single user", %{admin: admin, conn: conn} do assert ModerationLog.get_log_entry_message(log_entry) == "@#{admin.nickname} deleted users: @#{user.nickname}" - assert json_response(conn, 200) == user.nickname + assert json_response(conn, 200) == [user.nickname] end test "multiple users", %{admin: admin, conn: conn} do From 1ead5f49b8da941399fa2afadd40cd8beb8ccf8d Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 1 May 2020 14:30:39 +0200 Subject: [PATCH 29/34] AdminApiController: Use common pipeline for user deletion. --- .../web/admin_api/admin_api_controller.ex | 13 +++++++-- .../admin_api/admin_api_controller_test.exs | 28 +++++++++++++------ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index c09584fd1..9a12da027 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -17,6 +17,8 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do alias Pleroma.User alias Pleroma.UserInviteToken alias Pleroma.Web.ActivityPub.ActivityPub + alias Pleroma.Web.ActivityPub.Builder + alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Relay alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.AdminAPI.AccountView @@ -138,8 +140,15 @@ def user_delete(conn, %{"nickname" => nickname}) do end def user_delete(%{assigns: %{user: admin}} = conn, %{"nicknames" => nicknames}) do - users = nicknames |> Enum.map(&User.get_cached_by_nickname/1) - User.delete(users) + users = + nicknames + |> Enum.map(&User.get_cached_by_nickname/1) + + users + |> Enum.each(fn user -> + {:ok, delete_data, _} = Builder.delete(admin, user.ap_id) + Pipeline.common_pipeline(delete_data, local: true) + end) ModerationLog.insert_log(%{ actor: admin, diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index c92715fab..35001ab4a 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -6,8 +6,9 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do use Pleroma.Web.ConnCase use Oban.Testing, repo: Pleroma.Repo - import Pleroma.Factory import ExUnit.CaptureLog + import Mock + import Pleroma.Factory alias Pleroma.Activity alias Pleroma.Config @@ -146,17 +147,26 @@ test "GET /api/pleroma/admin/users/:nickname requires " <> test "single user", %{admin: admin, conn: conn} do user = insert(:user) - conn = - conn - |> put_req_header("accept", "application/json") - |> delete("/api/pleroma/admin/users?nickname=#{user.nickname}") + with_mock Pleroma.Web.Federator, + publish: fn _ -> nil end do + conn = + conn + |> put_req_header("accept", "application/json") + |> delete("/api/pleroma/admin/users?nickname=#{user.nickname}") - log_entry = Repo.one(ModerationLog) + ObanHelpers.perform_all() - assert ModerationLog.get_log_entry_message(log_entry) == - "@#{admin.nickname} deleted users: @#{user.nickname}" + refute User.get_by_nickname(user.nickname) - assert json_response(conn, 200) == [user.nickname] + log_entry = Repo.one(ModerationLog) + + assert ModerationLog.get_log_entry_message(log_entry) == + "@#{admin.nickname} deleted users: @#{user.nickname}" + + assert json_response(conn, 200) == [user.nickname] + + assert called(Pleroma.Web.Federator.publish(:_)) + end end test "multiple users", %{admin: admin, conn: conn} do From f20a1a27ef93c494e671b67603b320249073e011 Mon Sep 17 00:00:00 2001 From: Lain Soykaf Date: Sun, 3 May 2020 12:19:01 +0200 Subject: [PATCH 30/34] DeleteValidator: Improve code readability --- .../activity_pub/object_validators/delete_validator.ex | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) 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 256ac70b6..68ab08605 100644 --- a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -67,19 +67,17 @@ def do_not_federate?(cng) do end defp same_domain?(cng) do - actor_domain = + actor_uri = cng |> get_field(:actor) |> URI.parse() - |> (& &1.host).() - object_domain = + object_uri = cng |> get_field(:object) |> URI.parse() - |> (& &1.host).() - object_domain == actor_domain + object_uri.host == actor_uri.host end def validate_deletion_rights(cng) do From 4dfc617cdf1c2579f4f941dcd0fa5c728178df06 Mon Sep 17 00:00:00 2001 From: lain Date: Sun, 3 May 2020 12:51:28 +0200 Subject: [PATCH 31/34] Transmogrifier: Don't fetch actor that's guaranteed to be there. --- .../web/activity_pub/transmogrifier.ex | 3 +- .../transmogrifier/delete_handling_test.exs | 30 ++++--------------- 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 855aab8d4..1e031a015 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -733,8 +733,7 @@ def handle_incoming( %{"type" => "Delete"} = data, _options ) do - with {:ok, %User{}} <- ObjectValidator.fetch_actor(data), - {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do + with {:ok, activity, _} <- Pipeline.common_pipeline(data, local: false) do {:ok, activity} end end diff --git a/test/web/activity_pub/transmogrifier/delete_handling_test.exs b/test/web/activity_pub/transmogrifier/delete_handling_test.exs index 64c908a05..c141e25bc 100644 --- a/test/web/activity_pub/transmogrifier/delete_handling_test.exs +++ b/test/web/activity_pub/transmogrifier/delete_handling_test.exs @@ -13,7 +13,6 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.DeleteHandlingTest do alias Pleroma.Web.ActivityPub.Transmogrifier import Pleroma.Factory - import ExUnit.CaptureLog setup_all do Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end) @@ -27,22 +26,15 @@ test "it works for incoming deletes" do data = File.read!("test/fixtures/mastodon-delete.json") |> Poison.decode!() - - object = - data["object"] - |> Map.put("id", activity.data["object"]) - - data = - data - |> Map.put("object", object) |> 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 base our timelines on it. + # 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 @@ -54,25 +46,15 @@ test "it works for incoming deletes" do 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") data = File.read!("test/fixtures/mastodon-delete.json") |> Poison.decode!() + |> Map.put("actor", ap_id) + |> put_in(["object", "id"], activity.data["object"]) - object = - data["object"] - |> Map.put("id", activity.data["object"]) - - data = - data - |> Map.put("object", object) - - assert capture_log(fn -> - {:error, _} = Transmogrifier.handle_incoming(data) - end) =~ - "[error] Could not decode user at fetch http://mastodon.example.org/users/gargron, {:error, :nxdomain}" - - assert Activity.get_by_id(activity.id) + assert match?({:error, _}, Transmogrifier.handle_incoming(data)) end @tag capture_log: true From 6c337489f4db28f78be940bef01ef3a80e279ffc Mon Sep 17 00:00:00 2001 From: lain Date: Sun, 3 May 2020 13:01:19 +0200 Subject: [PATCH 32/34] Various testing fixes in relation to user deletion. --- test/web/activity_pub/side_effects_test.exs | 2 +- test/web/activity_pub/transmogrifier/delete_handling_test.exs | 2 +- test/web/admin_api/admin_api_controller_test.exs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/web/activity_pub/side_effects_test.exs b/test/web/activity_pub/side_effects_test.exs index ce34eed4c..a9598d7b3 100644 --- a/test/web/activity_pub/side_effects_test.exs +++ b/test/web/activity_pub/side_effects_test.exs @@ -68,7 +68,7 @@ test "it handles user deletions", %{delete_user: delete, user: user} do {:ok, _delete, _} = SideEffects.handle(delete) ObanHelpers.perform_all() - refute User.get_cached_by_ap_id(user.ap_id) + assert User.get_cached_by_ap_id(user.ap_id).deactivated end end diff --git a/test/web/activity_pub/transmogrifier/delete_handling_test.exs b/test/web/activity_pub/transmogrifier/delete_handling_test.exs index c141e25bc..f235a8e63 100644 --- a/test/web/activity_pub/transmogrifier/delete_handling_test.exs +++ b/test/web/activity_pub/transmogrifier/delete_handling_test.exs @@ -68,7 +68,7 @@ test "it works for incoming user deletes" do {:ok, _} = Transmogrifier.handle_incoming(data) ObanHelpers.perform_all() - refute User.get_cached_by_ap_id(ap_id) + assert User.get_cached_by_ap_id(ap_id).deactivated end test "it fails for incoming user deletes with spoofed origin" do diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index bf054a12e..0daf29ffb 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -156,7 +156,7 @@ test "single user", %{admin: admin, conn: conn} do ObanHelpers.perform_all() - refute User.get_by_nickname(user.nickname) + assert User.get_by_nickname(user.nickname).deactivated log_entry = Repo.one(ModerationLog) From 1974d0cc423efefcbdadd68442d0fbed8f3ee4ab Mon Sep 17 00:00:00 2001 From: lain Date: Sun, 3 May 2020 13:02:57 +0200 Subject: [PATCH 33/34] DeleteValidator: The deleted activity id is an object id --- .../web/activity_pub/object_validators/delete_validator.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 68ab08605..e06de3dff 100644 --- a/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/delete_validator.ex @@ -20,7 +20,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do field(:actor, Types.ObjectID) field(:to, Types.Recipients, default: []) field(:cc, Types.Recipients, default: []) - field(:deleted_activity_id) + field(:deleted_activity_id, Types.ObjectID) field(:object, Types.ObjectID) end From f21f53829339115e9a6cc9066d09026345047b43 Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 5 May 2020 10:38:59 +0200 Subject: [PATCH 34/34] LikeValidator: Add defaults for recipients back in. --- .../web/activity_pub/object_validators/like_validator.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/object_validators/like_validator.ex b/lib/pleroma/web/activity_pub/object_validators/like_validator.ex index d835b052e..034f25492 100644 --- a/lib/pleroma/web/activity_pub/object_validators/like_validator.ex +++ b/lib/pleroma/web/activity_pub/object_validators/like_validator.ex @@ -20,8 +20,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.LikeValidator do field(:object, Types.ObjectID) field(:actor, Types.ObjectID) field(:context, :string) - field(:to, Types.Recipients) - field(:cc, Types.Recipients) + field(:to, Types.Recipients, default: []) + field(:cc, Types.Recipients, default: []) end def cast_and_validate(data) do