DeleteValidator: Don't federate local deletions of remote objects.

Closes #1497
This commit is contained in:
lain 2020-04-30 21:23:18 +02:00
parent 999d639873
commit 32b8386ede
5 changed files with 119 additions and 10 deletions

View File

@ -19,11 +19,11 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
def validate(object, meta) def validate(object, meta)
def validate(%{"type" => "Delete"} = object, meta) do def validate(%{"type" => "Delete"} = object, meta) do
with {:ok, object} <- with cng <- DeleteValidator.cast_and_validate(object),
object do_not_federate <- DeleteValidator.do_not_federate?(cng),
|> DeleteValidator.cast_and_validate() {:ok, object} <- Ecto.Changeset.apply_action(cng, :insert) do
|> Ecto.Changeset.apply_action(:insert) do
object = stringify_keys(object) object = stringify_keys(object)
meta = Keyword.put(meta, :do_not_federate, do_not_federate)
{:ok, object, meta} {:ok, object, meta}
end end
end end

View File

@ -6,6 +6,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do
use Ecto.Schema use Ecto.Schema
alias Pleroma.Activity alias Pleroma.Activity
alias Pleroma.User
alias Pleroma.Web.ActivityPub.ObjectValidators.Types alias Pleroma.Web.ActivityPub.ObjectValidators.Types
import Ecto.Changeset import Ecto.Changeset
@ -45,12 +46,17 @@ def validate_data(cng) do
cng cng
|> validate_required([:id, :type, :actor, :to, :cc, :object]) |> validate_required([:id, :type, :actor, :to, :cc, :object])
|> validate_inclusion(:type, ["Delete"]) |> validate_inclusion(:type, ["Delete"])
|> validate_same_domain() |> validate_actor_presence()
|> validate_deletion_rights()
|> validate_object_or_user_presence() |> validate_object_or_user_presence()
|> add_deleted_activity_id() |> add_deleted_activity_id()
end end
def validate_same_domain(cng) do def do_not_federate?(cng) do
!same_domain?(cng)
end
defp same_domain?(cng) do
actor_domain = actor_domain =
cng cng
|> get_field(:actor) |> get_field(:actor)
@ -63,11 +69,17 @@ def validate_same_domain(cng) do
|> URI.parse() |> URI.parse()
|> (& &1.host).() |> (& &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 cng
|> add_error(:actor, "is not allowed to delete object")
else else
cng cng
|> add_error(:actor, "is not allowed to delete object")
end end
end end

View File

@ -29,7 +29,9 @@ def common_pipeline(object, meta) do
defp maybe_federate(activity, meta) do defp maybe_federate(activity, meta) do
with {:ok, local} <- Keyword.fetch(meta, :local) 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) Federator.publish(activity)
{:ok, :federated} {:ok, :federated}
else else

View File

@ -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", 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_post_delete: valid_post_delete} do
valid_user = insert(:user)
valid_other_actor = valid_other_actor =
valid_post_delete 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, [])) 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 assert {:actor, {"is not allowed to delete object", []}} in cng.errors
end 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 end
describe "likes" do describe "likes" do

View File

@ -9,11 +9,13 @@ defmodule Pleroma.Web.CommonAPITest do
alias Pleroma.Object alias Pleroma.Object
alias Pleroma.User alias Pleroma.User
alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.ActivityPub
alias Pleroma.Web.ActivityPub.Transmogrifier
alias Pleroma.Web.ActivityPub.Visibility alias Pleroma.Web.ActivityPub.Visibility
alias Pleroma.Web.AdminAPI.AccountView alias Pleroma.Web.AdminAPI.AccountView
alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI
import Pleroma.Factory import Pleroma.Factory
import Mock
require Pleroma.Constants require Pleroma.Constants
@ -21,6 +23,84 @@ defmodule Pleroma.Web.CommonAPITest do
setup do: clear_config([:instance, :limit]) setup do: clear_config([:instance, :limit])
setup do: clear_config([:instance, :max_pinned_statuses]) 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 test "favoriting race condition" do
user = insert(:user) user = insert(:user)
users_serial = insert_list(10, :user) users_serial = insert_list(10, :user)