aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorlain <lain@soykaf.club>2020-04-30 21:23:18 +0200
committerlain <lain@soykaf.club>2020-04-30 21:23:18 +0200
commit32b8386edeec3e9b24123c3ccc81a22f1edd5a1c (patch)
treee815a92b350d3b038cd09d66b4d4cae255b36f8e
parent999d639873b70f75c340dbac3360d25bca27a998 (diff)
downloadpleroma-32b8386edeec3e9b24123c3ccc81a22f1edd5a1c.tar.gz
DeleteValidator: Don't federate local deletions of remote objects.
Closes #1497
-rw-r--r--lib/pleroma/web/activity_pub/object_validator.ex8
-rw-r--r--lib/pleroma/web/activity_pub/object_validators/delete_validator.ex20
-rw-r--r--lib/pleroma/web/activity_pub/pipeline.ex4
-rw-r--r--test/web/activity_pub/object_validator_test.exs17
-rw-r--r--test/web/common_api/common_api_test.exs80
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 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator 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 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator 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 @@ defmodule Pleroma.Web.ActivityPub.Pipeline 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 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidatorTest do
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 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidatorTest do
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)