Merge branch 'remote-deletions' into 'develop'

Fix user deletion problems

See merge request pleroma/pleroma!3476
This commit is contained in:
feld 2021-07-06 20:05:05 +00:00
commit 9e1da4bf58
6 changed files with 78 additions and 32 deletions

View File

@ -26,6 +26,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Fixed ### Fixed
- Don't crash so hard when email settings are invalid. - Don't crash so hard when email settings are invalid.
- Checking activated Upload Filters for required commands. - Checking activated Upload Filters for required commands.
- Remote users can no longer reappear after being deleted.
- Deactivated users may now be deleted.
- Mix task `pleroma.database prune_objects` - Mix task `pleroma.database prune_objects`
### Removed ### Removed

View File

@ -1695,8 +1695,6 @@ def purge_user_changeset(user) do
email: nil, email: nil,
name: nil, name: nil,
password_hash: nil, password_hash: nil,
keys: nil,
public_key: nil,
avatar: %{}, avatar: %{},
tags: [], tags: [],
last_refreshed_at: nil, last_refreshed_at: nil,
@ -1707,9 +1705,7 @@ def purge_user_changeset(user) do
follower_count: 0, follower_count: 0,
following_count: 0, following_count: 0,
is_locked: false, is_locked: false,
is_confirmed: true,
password_reset_pending: false, password_reset_pending: false,
is_approved: true,
registration_reason: nil, registration_reason: nil,
confirmation_token: nil, confirmation_token: nil,
domain_blocks: [], domain_blocks: [],
@ -1725,45 +1721,53 @@ def purge_user_changeset(user) do
raw_fields: [], raw_fields: [],
is_discoverable: false, is_discoverable: false,
also_known_as: [] also_known_as: []
# id: preserved
# ap_id: preserved
# nickname: preserved
}) })
end end
# Purge doesn't delete the user from the database.
# It just nulls all its fields and deactivates it.
# See `User.purge_user_changeset/1` above.
defp purge(%User{} = user) do
user
|> purge_user_changeset()
|> update_and_set_cache()
end
def delete(users) when is_list(users) do def delete(users) when is_list(users) do
for user <- users, do: delete(user) for user <- users, do: delete(user)
end end
def delete(%User{} = user) do def delete(%User{} = user) do
# Purge the user immediately
purge(user)
BackgroundWorker.enqueue("delete_user", %{"user_id" => user.id}) BackgroundWorker.enqueue("delete_user", %{"user_id" => user.id})
end end
defp delete_and_invalidate_cache(%User{} = user) do # *Actually* delete the user from the DB
defp delete_from_db(%User{} = user) do
invalidate_cache(user) invalidate_cache(user)
Repo.delete(user) Repo.delete(user)
end end
defp delete_or_deactivate(%User{local: false} = user), do: delete_and_invalidate_cache(user) # If the user never finalized their account, it's safe to delete them.
defp maybe_delete_from_db(%User{local: true, is_confirmed: false} = user),
do: delete_from_db(user)
defp delete_or_deactivate(%User{local: true} = user) do defp maybe_delete_from_db(%User{local: true, is_approved: false} = user),
status = account_status(user) do: delete_from_db(user)
case status do defp maybe_delete_from_db(user), do: {:ok, user}
:confirmation_pending ->
delete_and_invalidate_cache(user)
:approval_pending ->
delete_and_invalidate_cache(user)
_ ->
user
|> purge_user_changeset()
|> update_and_set_cache()
end
end
def perform(:force_password_reset, user), do: force_password_reset(user) def perform(:force_password_reset, user), do: force_password_reset(user)
@spec perform(atom(), User.t()) :: {:ok, User.t()} @spec perform(atom(), User.t()) :: {:ok, User.t()}
def perform(:delete, %User{} = user) do def perform(:delete, %User{} = user) do
# Purge the user again, in case perform/2 is called directly
purge(user)
# Remove all relationships # Remove all relationships
user user
|> get_followers() |> get_followers()
@ -1781,10 +1785,9 @@ def perform(:delete, %User{} = user) do
delete_user_activities(user) delete_user_activities(user)
delete_notifications_from_user_activities(user) delete_notifications_from_user_activities(user)
delete_outgoing_pending_follow_requests(user) delete_outgoing_pending_follow_requests(user)
delete_or_deactivate(user) maybe_delete_from_db(user)
end end
def perform(:set_activation_async, user, status), do: set_activation(user, status) def perform(:set_activation_async, user, status), do: set_activation(user, status)

View File

@ -53,15 +53,18 @@ defp get_recipients(data) do
{recipients, to, cc} {recipients, to, cc}
end end
defp check_actor_is_active(nil), do: true defp check_actor_can_insert(%{"type" => "Delete"}), do: true
defp check_actor_can_insert(%{"type" => "Undo"}), do: true
defp check_actor_is_active(actor) when is_binary(actor) do defp check_actor_can_insert(%{"actor" => actor}) when is_binary(actor) do
case User.get_cached_by_ap_id(actor) do case User.get_cached_by_ap_id(actor) do
%User{is_active: true} -> true %User{is_active: true} -> true
_ -> false _ -> false
end end
end end
defp check_actor_can_insert(_), do: true
defp check_remote_limit(%{"object" => %{"content" => content}}) when not is_nil(content) do defp check_remote_limit(%{"object" => %{"content" => content}}) when not is_nil(content) do
limit = Config.get([:instance, :remote_limit]) limit = Config.get([:instance, :remote_limit])
String.length(content) <= limit String.length(content) <= limit
@ -117,7 +120,7 @@ def persist(object, meta) do
def insert(map, local \\ true, fake \\ false, bypass_actor_check \\ false) when is_map(map) do def insert(map, local \\ true, fake \\ false, bypass_actor_check \\ false) when is_map(map) do
with nil <- Activity.normalize(map), with nil <- Activity.normalize(map),
map <- lazy_put_activity_defaults(map, fake), map <- lazy_put_activity_defaults(map, fake),
{_, true} <- {:actor_check, bypass_actor_check || check_actor_is_active(map["actor"])}, {_, true} <- {:actor_check, bypass_actor_check || check_actor_can_insert(map)},
{_, true} <- {:remote_limit_pass, check_remote_limit(map)}, {_, true} <- {:remote_limit_pass, check_remote_limit(map)},
{:ok, map} <- MRF.filter(map), {:ok, map} <- MRF.filter(map),
{recipients, _, _} = get_recipients(map), {recipients, _, _} = get_recipients(map),

View File

@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.DeleteValidator do
alias Pleroma.Activity alias Pleroma.Activity
alias Pleroma.EctoType.ActivityPub.ObjectValidators alias Pleroma.EctoType.ActivityPub.ObjectValidators
alias Pleroma.User
import Ecto.Changeset import Ecto.Changeset
import Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations import Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations
@ -57,7 +58,7 @@ defp 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_actor_presence() |> validate_delete_actor(:actor)
|> validate_modification_rights() |> validate_modification_rights()
|> validate_object_or_user_presence(allowed_types: @deletable_types) |> validate_object_or_user_presence(allowed_types: @deletable_types)
|> add_deleted_activity_id() |> add_deleted_activity_id()
@ -72,4 +73,13 @@ def cast_and_validate(data) do
|> cast_data |> cast_data
|> validate_data |> validate_data
end end
defp validate_delete_actor(cng, field_name) do
validate_change(cng, field_name, fn field_name, actor ->
case User.get_cached_by_ap_id(actor) do
%User{} -> []
_ -> [{field_name, "can't find user"}]
end
end)
end
end end

View File

@ -7,6 +7,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.UndoValidator do
alias Pleroma.Activity alias Pleroma.Activity
alias Pleroma.EctoType.ActivityPub.ObjectValidators alias Pleroma.EctoType.ActivityPub.ObjectValidators
alias Pleroma.User
import Ecto.Changeset import Ecto.Changeset
import Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations import Pleroma.Web.ActivityPub.ObjectValidators.CommonValidations
@ -42,7 +43,7 @@ defp validate_data(data_cng) do
data_cng data_cng
|> validate_inclusion(:type, ["Undo"]) |> validate_inclusion(:type, ["Undo"])
|> validate_required([:id, :type, :object, :actor, :to, :cc]) |> validate_required([:id, :type, :object, :actor, :to, :cc])
|> validate_actor_presence() |> validate_undo_actor(:actor)
|> validate_object_presence() |> validate_object_presence()
|> validate_undo_rights() |> validate_undo_rights()
end end
@ -59,4 +60,13 @@ def validate_undo_rights(cng) do
_ -> cng _ -> cng
end end
end end
defp validate_undo_actor(cng, field_name) do
validate_change(cng, field_name, fn field_name, actor ->
case User.get_cached_by_ap_id(actor) do
%User{} -> []
_ -> [{field_name, "can't find user"}]
end
end)
end
end end

View File

@ -1639,9 +1639,9 @@ test "delete/1 purges a user when they wouldn't be fully deleted" do
follower_count: 9, follower_count: 9,
following_count: 9001, following_count: 9001,
is_locked: true, is_locked: true,
is_confirmed: false, is_confirmed: true,
password_reset_pending: true, password_reset_pending: true,
is_approved: false, is_approved: true,
registration_reason: "ahhhhh", registration_reason: "ahhhhh",
confirmation_token: "qqqq", confirmation_token: "qqqq",
domain_blocks: ["lain.com"], domain_blocks: ["lain.com"],
@ -1669,8 +1669,8 @@ test "delete/1 purges a user when they wouldn't be fully deleted" do
email: nil, email: nil,
name: nil, name: nil,
password_hash: nil, password_hash: nil,
keys: nil, keys: "RSA begin buplic key",
public_key: nil, public_key: "--PRIVATE KEYE--",
avatar: %{}, avatar: %{},
tags: [], tags: [],
last_refreshed_at: nil, last_refreshed_at: nil,
@ -1702,6 +1702,24 @@ test "delete/1 purges a user when they wouldn't be fully deleted" do
} = user } = user
end end
test "delete/1 purges a remote user" do
user =
insert(:user, %{
name: "qqqqqqq",
avatar: %{"a" => "b"},
banner: %{"a" => "b"},
local: false
})
{:ok, job} = User.delete(user)
{:ok, _} = ObanHelpers.perform(job)
user = User.get_by_id(user.id)
assert user.name == nil
assert user.avatar == %{}
assert user.banner == %{}
end
test "get_public_key_for_ap_id fetches a user that's not in the db" do test "get_public_key_for_ap_id fetches a user that's not in the db" do
assert {:ok, _key} = User.get_public_key_for_ap_id("http://mastodon.example.org/users/admin") assert {:ok, _key} = User.get_public_key_for_ap_id("http://mastodon.example.org/users/admin")
end end