From 9f3182caf7adfde0ae30fcfe3296126059b47c4d Mon Sep 17 00:00:00 2001 From: Eliot Berriot <contact@eliotberriot.com> Date: Sat, 21 Sep 2019 16:20:49 +0200 Subject: [PATCH] See #852: improved routing logic for federation messages (support multiple objects types for one route) --- api/funkwhale_api/federation/activity.py | 31 ++++- api/funkwhale_api/federation/models.py | 1 + api/funkwhale_api/federation/routes.py | 61 +++++++++ api/funkwhale_api/federation/serializers.py | 7 + api/funkwhale_api/users/models.py | 7 - api/funkwhale_api/users/mutations.py | 40 ++++++ api/funkwhale_api/users/serializers.py | 26 ++++ api/funkwhale_api/users/tasks.py | 58 ++++++++ api/funkwhale_api/users/views.py | 14 +- api/tests/federation/test_routes.py | 144 ++++++++++++++++---- api/tests/users/test_models.py | 10 -- api/tests/users/test_mutations.py | 33 +++++ api/tests/users/test_tasks.py | 32 +++++ api/tests/users/test_views.py | 56 +++++++- changes/changelog.d/852.feature | 1 + changes/notes.rst | 12 ++ docs/users/account.rst | 14 ++ docs/users/index.rst | 1 + front/src/components/auth/Settings.vue | 67 ++++++++- 19 files changed, 561 insertions(+), 54 deletions(-) create mode 100644 api/funkwhale_api/users/mutations.py create mode 100644 api/funkwhale_api/users/tasks.py create mode 100644 api/tests/users/test_mutations.py create mode 100644 api/tests/users/test_tasks.py create mode 100644 changes/changelog.d/852.feature create mode 100644 docs/users/account.rst diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py index 0b34e9141..e3fb7be32 100644 --- a/api/funkwhale_api/federation/activity.py +++ b/api/funkwhale_api/federation/activity.py @@ -385,7 +385,10 @@ class OutboxRouter(Router): def match_route(route, payload): for key, value in route.items(): payload_value = recursive_getattr(payload, key, permissive=True) - if payload_value != value: + if isinstance(value, list): + if payload_value not in value: + return False + elif payload_value != value: return False return True @@ -450,14 +453,32 @@ def prepare_deliveries_and_inbox_items(recipient_list, type, allowed_domains=Non .exclude(actor__domain=None) ) ) + followed_domains = list(follows.values_list("actor__domain_id", flat=True)) actors = models.Actor.objects.filter( - managed_domains__name__in=follows.values_list( - "actor__domain_id", flat=True - ) + managed_domains__name__in=followed_domains ) - values = actors.values("shared_inbox_url", "inbox_url") + values = actors.values("shared_inbox_url", "inbox_url", "domain_id") + handled_domains = set() for v in values: remote_inbox_urls.add(v["shared_inbox_url"] or v["inbox_url"]) + handled_domains.add(v["domain_id"]) + + if len(handled_domains) >= len(followed_domains): + continue + + # for all remaining domains (probably non-funkwhale instances, with no + # service actors), we also pick the latest known actor per domain and send the message + # there instead + remaining_domains = models.Domain.objects.exclude(name__in=handled_domains) + remaining_domains = remaining_domains.filter(name__in=followed_domains) + actors = models.Actor.objects.filter(domain__in=remaining_domains) + actors = ( + actors.order_by("domain_id", "-last_fetch_date") + .distinct("domain_id") + .values("shared_inbox_url", "inbox_url") + ) + for v in actors: + remote_inbox_urls.add(v["shared_inbox_url"] or v["inbox_url"]) deliveries = [ models.Delivery(inbox_url=url) diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 304b94fad..c734939b3 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -21,6 +21,7 @@ from . import utils as federation_utils TYPE_CHOICES = [ ("Person", "Person"), + ("Tombstone", "Tombstone"), ("Application", "Application"), ("Group", "Group"), ("Organization", "Organization"), diff --git a/api/funkwhale_api/federation/routes.py b/api/funkwhale_api/federation/routes.py index bae2812ce..41497e9f2 100644 --- a/api/funkwhale_api/federation/routes.py +++ b/api/funkwhale_api/federation/routes.py @@ -4,6 +4,7 @@ from funkwhale_api.music import models as music_models from . import activity from . import actors +from . import models from . import serializers logger = logging.getLogger(__name__) @@ -380,3 +381,63 @@ def outbox_update_artist(context): to=[activity.PUBLIC_ADDRESS, {"type": "instances_with_followers"}], ), } + + +@outbox.register( + { + "type": "Delete", + "object.type": [ + "Tombstone", + "Actor", + "Person", + "Application", + "Organization", + "Service", + "Group", + ], + } +) +def outbox_delete_actor(context): + actor = context["actor"] + serializer = serializers.ActivitySerializer( + {"type": "Delete", "object": {"type": actor.type, "id": actor.fid}} + ) + yield { + "type": "Delete", + "actor": actor, + "payload": with_recipients( + serializer.data, + to=[activity.PUBLIC_ADDRESS, {"type": "instances_with_followers"}], + ), + } + + +@inbox.register( + { + "type": "Delete", + "object.type": [ + "Tombstone", + "Actor", + "Person", + "Application", + "Organization", + "Service", + "Group", + ], + } +) +def inbox_delete_actor(payload, context): + actor = context["actor"] + serializer = serializers.ActorDeleteSerializer(data=payload) + if not serializer.is_valid(): + logger.info("Skipped actor %s deletion, invalid payload", actor.fid) + return + + deleted_fid = serializer.validated_data["fid"] + try: + # ensure the actor only can delete itself, and is a remote one + actor = models.Actor.objects.local(False).get(fid=deleted_fid, pk=actor.pk) + except models.Actor.DoesNotExist: + logger.warn("Cannot delete actor %s, no matching object found", actor.fid) + return + actor.delete() diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 86e991cea..697c2acc9 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -1138,6 +1138,13 @@ class UploadSerializer(jsonld.JsonLdSerializer): return d +class ActorDeleteSerializer(jsonld.JsonLdSerializer): + fid = serializers.URLField(max_length=500) + + class Meta: + jsonld_mapping = {"fid": jsonld.first_id(contexts.AS.object)} + + class NodeInfoLinkSerializer(serializers.Serializer): href = serializers.URLField() rel = serializers.URLField() diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index dd181f7bf..44bb06d2b 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -400,10 +400,3 @@ def warm_user_avatar(sender, instance, **kwargs): instance_or_queryset=instance, rendition_key_set="square", image_attr="avatar" ) num_created, failed_to_create = user_avatar_warmer.warm() - - -@receiver(models.signals.pre_delete, sender=User) -def delete_actor(sender, instance, **kwargs): - if not instance.actor: - return - instance.actor.delete() diff --git a/api/funkwhale_api/users/mutations.py b/api/funkwhale_api/users/mutations.py new file mode 100644 index 000000000..2c30ffe17 --- /dev/null +++ b/api/funkwhale_api/users/mutations.py @@ -0,0 +1,40 @@ +import uuid + +from django.db import transaction + +from funkwhale_api.common import mutations +from funkwhale_api.common import utils +from funkwhale_api.federation import models + +from . import tasks + + +@mutations.registry.connect("delete_account", models.Actor) +class DeleteAccountMutationSerializer(mutations.MutationSerializer): + @transaction.atomic + def apply(self, obj, validated_data): + if not obj.is_local or not obj.user: + raise mutations.serializers.ValidationError("Cannot delete this account") + + # delete oauth apps / reset all passwords immediatly + obj.user.set_unusable_password() + obj.user.subsonic_api_token = None + # force logout + obj.user.secret_key = uuid.uuid4() + obj.user.users_grant.all().delete() + obj.user.users_accesstoken.all().delete() + obj.user.users_refreshtoken.all().delete() + obj.user.save() + + # since the deletion of related object/message sending can take a long time + # we do that in a separate tasks + utils.on_commit(tasks.delete_account.delay, user_id=obj.user.id) + + def get_previous_state(self, obj, validated_data): + """ + We store usernames and ids for auditability purposes + """ + return { + "user": {"username": obj.user.username, "id": obj.user.pk}, + "actor": {"preferred_username": obj.preferred_username}, + } diff --git a/api/funkwhale_api/users/serializers.py b/api/funkwhale_api/users/serializers.py index 9db081378..67171b49b 100644 --- a/api/funkwhale_api/users/serializers.py +++ b/api/funkwhale_api/users/serializers.py @@ -11,6 +11,7 @@ from versatileimagefield.serializers import VersatileImageFieldSerializer from funkwhale_api.activity import serializers as activity_serializers from funkwhale_api.common import serializers as common_serializers +from funkwhale_api.federation import models as federation_models from . import adapters from . import models @@ -51,6 +52,17 @@ class RegisterSerializer(RS): get_adapter().clean_password(data["password1"], user) return data + def validate_username(self, value): + username = super().validate_username(value) + duplicates = federation_models.Actor.objects.local().filter( + preferred_username__iexact=username + ) + if duplicates.exists(): + raise serializers.ValidationError( + "A user with that username already exists." + ) + return username + def save(self, request): user = super().save(request) if self.validated_data.get("invitation"): @@ -143,3 +155,17 @@ class MeSerializer(UserReadSerializer): class PasswordResetSerializer(PRS): def get_email_options(self): return {"extra_email_context": adapters.get_email_context()} + + +class UserDeleteSerializer(serializers.Serializer): + password = serializers.CharField() + confirm = serializers.BooleanField() + + def validate_password(self, value): + if not self.instance.check_password(value): + raise serializers.ValidationError("Invalid password") + + def validate_confirm(self, value): + if not value: + raise serializers.ValidationError("Please confirm deletion") + return value diff --git a/api/funkwhale_api/users/tasks.py b/api/funkwhale_api/users/tasks.py new file mode 100644 index 000000000..40438ccff --- /dev/null +++ b/api/funkwhale_api/users/tasks.py @@ -0,0 +1,58 @@ +import logging + +from django.db.models.deletion import Collector + +from funkwhale_api.federation import routes +from funkwhale_api.taskapp import celery + +from . import models + +logger = logging.getLogger(__name__) + + +@celery.app.task(name="users.delete_account") +@celery.require_instance(models.User.objects.select_related("actor"), "user") +def delete_account(user): + logger.info("Starting deletion of account %s…", user.username) + actor = user.actor + # we start by deleting the user obj, which will cascade deletion + # to any other object + user.delete() + logger.info("Deleted user object") + + # Then we broadcast the info over federation. We do this *before* deleting objects + # associated with the actor, otherwise follows are removed and we don't know where + # to broadcast + logger.info("Broadcasting deletion to federation…") + routes.outbox.dispatch( + {"type": "Delete", "object": {"type": actor.type}}, context={"actor": actor} + ) + + # then we delete any object associated with the actor object, but *not* the actor + # itself. We keep it for auditability and sending the Delete ActivityPub message + collector = Collector(using="default") + logger.info( + "Prepare deletion of objects associated with account %s…", user.username + ) + collector.collect([actor]) + + for model, instances in collector.data.items(): + if issubclass(model, actor.__class__): + # we skip deletion of the actor itself + continue + + logger.info( + "Deleting %s objects associated with account %s…", + len(instances), + user.username, + ) + to_delete = model.objects.filter(pk__in=[instance.pk for instance in instances]) + to_delete.delete() + + # Finally, we update the actor itself and mark it as removed + logger.info("Marking actor as Tombsone…") + actor.type = "Tombstone" + actor.name = None + actor.summary = None + actor.save(update_fields=["type", "name", "summary"]) + logger.info("Deletion of account done %s!", user.username) diff --git a/api/funkwhale_api/users/views.py b/api/funkwhale_api/users/views.py index 7cdb7cdbb..28189c4bc 100644 --- a/api/funkwhale_api/users/views.py +++ b/api/funkwhale_api/users/views.py @@ -7,7 +7,7 @@ from rest_framework.response import Response from funkwhale_api.common import preferences -from . import models, serializers +from . import models, serializers, tasks class RegisterView(registration_views.RegisterView): @@ -50,9 +50,17 @@ class UserViewSet(mixins.UpdateModelMixin, viewsets.GenericViewSet): lookup_value_regex = r"[a-zA-Z0-9-_.]+" required_scope = "profile" - @action(methods=["get"], detail=False) + @action(methods=["get", "delete"], detail=False) def me(self, request, *args, **kwargs): - """Return information about the current user""" + """Return information about the current user or delete it""" + if request.method.lower() == "delete": + serializer = serializers.UserDeleteSerializer( + request.user, data=request.data + ) + serializer.is_valid(raise_exception=True) + tasks.delete_account.delay(user_id=request.user.pk) + # at this point, password is valid, we launch deletion + return Response(status=204) serializer = serializers.MeSerializer(request.user) return Response(serializer.data) diff --git a/api/tests/federation/test_routes.py b/api/tests/federation/test_routes.py index 56834d55f..3bbdd4868 100644 --- a/api/tests/federation/test_routes.py +++ b/api/tests/federation/test_routes.py @@ -1,6 +1,13 @@ import pytest -from funkwhale_api.federation import actors, contexts, jsonld, routes, serializers +from funkwhale_api.federation import ( + activity, + actors, + contexts, + jsonld, + routes, + serializers, +) @pytest.mark.parametrize( @@ -8,23 +15,29 @@ from funkwhale_api.federation import actors, contexts, jsonld, routes, serialize [ ({"type": "Follow"}, routes.inbox_follow), ({"type": "Accept"}, routes.inbox_accept), - ({"type": "Create", "object.type": "Audio"}, routes.inbox_create_audio), - ({"type": "Update", "object.type": "Library"}, routes.inbox_update_library), - ({"type": "Delete", "object.type": "Library"}, routes.inbox_delete_library), - ({"type": "Delete", "object.type": "Audio"}, routes.inbox_delete_audio), - ({"type": "Undo", "object.type": "Follow"}, routes.inbox_undo_follow), - ({"type": "Update", "object.type": "Artist"}, routes.inbox_update_artist), - ({"type": "Update", "object.type": "Album"}, routes.inbox_update_album), - ({"type": "Update", "object.type": "Track"}, routes.inbox_update_track), + ({"type": "Create", "object": {"type": "Audio"}}, routes.inbox_create_audio), + ( + {"type": "Update", "object": {"type": "Library"}}, + routes.inbox_update_library, + ), + ( + {"type": "Delete", "object": {"type": "Library"}}, + routes.inbox_delete_library, + ), + ({"type": "Delete", "object": {"type": "Audio"}}, routes.inbox_delete_audio), + ({"type": "Undo", "object": {"type": "Follow"}}, routes.inbox_undo_follow), + ({"type": "Update", "object": {"type": "Artist"}}, routes.inbox_update_artist), + ({"type": "Update", "object": {"type": "Album"}}, routes.inbox_update_album), + ({"type": "Update", "object": {"type": "Track"}}, routes.inbox_update_track), + ({"type": "Delete", "object": {"type": "Person"}}, routes.inbox_delete_actor), ], ) def test_inbox_routes(route, handler): - for r, h in routes.inbox.routes: - if r == route: - assert h == handler - return - - assert False, "Inbox route {} not found".format(route) + matching = [ + handler for r, handler in routes.inbox.routes if activity.match_route(r, route) + ] + assert len(matching) == 1, "Inbox route {} not found".format(route) + assert matching[0] == handler @pytest.mark.parametrize( @@ -32,21 +45,41 @@ def test_inbox_routes(route, handler): [ ({"type": "Accept"}, routes.outbox_accept), ({"type": "Follow"}, routes.outbox_follow), - ({"type": "Create", "object.type": "Audio"}, routes.outbox_create_audio), - ({"type": "Update", "object.type": "Library"}, routes.outbox_update_library), - ({"type": "Delete", "object.type": "Library"}, routes.outbox_delete_library), - ({"type": "Delete", "object.type": "Audio"}, routes.outbox_delete_audio), - ({"type": "Undo", "object.type": "Follow"}, routes.outbox_undo_follow), - ({"type": "Update", "object.type": "Track"}, routes.outbox_update_track), + ({"type": "Create", "object": {"type": "Audio"}}, routes.outbox_create_audio), + ( + {"type": "Update", "object": {"type": "Library"}}, + routes.outbox_update_library, + ), + ( + {"type": "Delete", "object": {"type": "Library"}}, + routes.outbox_delete_library, + ), + ({"type": "Delete", "object": {"type": "Audio"}}, routes.outbox_delete_audio), + ({"type": "Undo", "object": {"type": "Follow"}}, routes.outbox_undo_follow), + ({"type": "Update", "object": {"type": "Track"}}, routes.outbox_update_track), + ( + {"type": "Delete", "object": {"type": "Tombstone"}}, + routes.outbox_delete_actor, + ), + ({"type": "Delete", "object": {"type": "Person"}}, routes.outbox_delete_actor), + ({"type": "Delete", "object": {"type": "Service"}}, routes.outbox_delete_actor), + ( + {"type": "Delete", "object": {"type": "Application"}}, + routes.outbox_delete_actor, + ), + ({"type": "Delete", "object": {"type": "Group"}}, routes.outbox_delete_actor), + ( + {"type": "Delete", "object": {"type": "Organization"}}, + routes.outbox_delete_actor, + ), ], ) def test_outbox_routes(route, handler): - for r, h in routes.outbox.routes: - if r == route: - assert h == handler - return - - assert False, "Outbox route {} not found".format(route) + matching = [ + handler for r, handler in routes.outbox.routes if activity.match_route(r, route) + ] + assert len(matching) == 1, "Outbox route {} not found".format(route) + assert matching[0] == handler def test_inbox_follow_library_autoapprove(factories, mocker): @@ -559,3 +592,60 @@ def test_outbox_update_track(factories): assert dict(activity["payload"]) == dict(expected) assert activity["actor"] == actors.get_service_actor() + + +def test_outbox_delete_actor(factories): + user = factories["users.User"]() + actor = user.create_actor() + + activity = list(routes.outbox_delete_actor({"actor": actor}))[0] + expected = serializers.ActivitySerializer( + {"type": "Delete", "object": {"id": actor.fid, "type": actor.type}} + ).data + + expected["to"] = [contexts.AS.Public, {"type": "instances_with_followers"}] + + assert dict(activity["payload"]) == dict(expected) + assert activity["actor"] == actor + + +def test_inbox_delete_actor(factories): + remote_actor = factories["federation.Actor"]() + serializer = serializers.ActivitySerializer( + { + "type": "Delete", + "object": {"type": remote_actor.type, "id": remote_actor.fid}, + } + ) + routes.inbox_delete_actor( + serializer.data, context={"actor": remote_actor, "raise_exception": True} + ) + with pytest.raises(remote_actor.__class__.DoesNotExist): + remote_actor.refresh_from_db() + + +def test_inbox_delete_actor_only_works_on_self(factories): + remote_actor1 = factories["federation.Actor"]() + remote_actor2 = factories["federation.Actor"]() + serializer = serializers.ActivitySerializer( + { + "type": "Delete", + "object": {"type": remote_actor2.type, "id": remote_actor2.fid}, + } + ) + routes.inbox_delete_actor( + serializer.data, context={"actor": remote_actor1, "raise_exception": True} + ) + remote_actor2.refresh_from_db() + + +def test_inbox_delete_actor_doesnt_delete_local_actor(factories): + local_actor = factories["users.User"]().create_actor() + serializer = serializers.ActivitySerializer( + {"type": "Delete", "object": {"type": local_actor.type, "id": local_actor.fid}} + ) + routes.inbox_delete_actor( + serializer.data, context={"actor": local_actor, "raise_exception": True} + ) + # actor should still be here! + local_actor.refresh_from_db() diff --git a/api/tests/users/test_models.py b/api/tests/users/test_models.py index c98472a27..973316424 100644 --- a/api/tests/users/test_models.py +++ b/api/tests/users/test_models.py @@ -220,13 +220,3 @@ def test_user_get_quota_status(factories, preferences, mocker): "errored": 3, "finished": 4, } - - -def test_deleting_users_deletes_associated_actor(factories): - actor = factories["federation.Actor"]() - user = factories["users.User"](actor=actor) - - user.delete() - - with pytest.raises(actor.DoesNotExist): - actor.refresh_from_db() diff --git a/api/tests/users/test_mutations.py b/api/tests/users/test_mutations.py new file mode 100644 index 000000000..33902153f --- /dev/null +++ b/api/tests/users/test_mutations.py @@ -0,0 +1,33 @@ +from funkwhale_api.users import tasks + + +def test_delete_account_mutation(mocker, factories, now): + user = factories["users.User"](subsonic_api_token="test", password="test") + actor = user.create_actor() + on_commit = mocker.patch("funkwhale_api.common.utils.on_commit") + + secret_key = user.secret_key + set_unusable_password = mocker.spy(user, "set_unusable_password") + factories["users.Grant"](user=user) + factories["users.AccessToken"](user=user) + factories["users.RefreshToken"](user=user) + mutation = factories["common.Mutation"]( + type="delete_account", target=actor, payload={} + ) + + mutation.apply() + user.refresh_from_db() + + set_unusable_password.assert_called_once_with() + assert user.has_usable_password() is False + assert user.subsonic_api_token is None + assert user.secret_key is not None and user.secret_key != secret_key + assert user.users_grant.count() == 0 + assert user.users_refreshtoken.count() == 0 + assert user.users_accesstoken.count() == 0 + on_commit.assert_called_once_with(tasks.delete_account.delay, user_id=user.pk) + + assert mutation.previous_state == { + "actor": {"preferred_username": actor.preferred_username}, + "user": {"username": user.username, "id": user.pk}, + } diff --git a/api/tests/users/test_tasks.py b/api/tests/users/test_tasks.py new file mode 100644 index 000000000..243071850 --- /dev/null +++ b/api/tests/users/test_tasks.py @@ -0,0 +1,32 @@ +import pytest + +from funkwhale_api.federation import routes +from funkwhale_api.users import tasks + + +def test_delete_account(factories, mocker): + user = factories["users.User"]() + actor = user.create_actor() + library = factories["music.Library"](actor=actor) + unrelated_library = factories["music.Library"]() + dispatch = mocker.patch.object(routes.outbox, "dispatch") + + tasks.delete_account(user_id=user.pk) + + dispatch.assert_called_once_with( + {"type": "Delete", "object": {"type": actor.type}}, context={"actor": actor} + ) + + with pytest.raises(user.DoesNotExist): + user.refresh_from_db() + + with pytest.raises(library.DoesNotExist): + library.refresh_from_db() + + # this one shouldn't be deleted + unrelated_library.refresh_from_db() + actor.refresh_from_db() + + assert actor.type == "Tombstone" + assert actor.name is None + assert actor.summary is None diff --git a/api/tests/users/test_views.py b/api/tests/users/test_views.py index db8f870f3..8156c84b5 100644 --- a/api/tests/users/test_views.py +++ b/api/tests/users/test_views.py @@ -39,7 +39,7 @@ def test_username_only_accepts_letters_and_underscores( def test_can_restrict_usernames(settings, preferences, db, api_client): url = reverse("rest_register") preferences["users__registration_enabled"] = True - settings.USERNAME_BLACKLIST = ["funkwhale"] + settings.ACCOUNT_USERNAME_BLACKLIST = ["funkwhale"] data = { "username": "funkwhale", "email": "contact@funkwhale.io", @@ -333,3 +333,57 @@ def test_creating_user_sends_confirmation_email( confirmation_message = mailoutbox[-1] assert "Hello world" in confirmation_message.body assert settings.FUNKWHALE_HOSTNAME in confirmation_message.body + + +def test_user_account_deletion_requires_valid_password(logged_in_api_client): + user = logged_in_api_client.user + user.set_password("mypassword") + url = reverse("api:v1:users:users-me") + payload = {"password": "invalid", "confirm": True} + response = logged_in_api_client.delete(url, payload) + + assert response.status_code == 400 + + +def test_user_account_deletion_requires_confirmation(logged_in_api_client): + user = logged_in_api_client.user + user.set_password("mypassword") + url = reverse("api:v1:users:users-me") + payload = {"password": "mypassword", "confirm": False} + response = logged_in_api_client.delete(url, payload) + + assert response.status_code == 400 + + +def test_user_account_deletion_triggers_delete_account(logged_in_api_client, mocker): + user = logged_in_api_client.user + user.set_password("mypassword") + url = reverse("api:v1:users:users-me") + payload = {"password": "mypassword", "confirm": True} + delete_account = mocker.patch("funkwhale_api.users.tasks.delete_account.delay") + response = logged_in_api_client.delete(url, payload) + + assert response.status_code == 204 + delete_account.assert_called_once_with(user_id=user.pk) + + +def test_username_with_existing_local_account_are_invalid( + settings, preferences, factories, api_client +): + actor = factories["users.User"]().create_actor() + user = actor.user + user.delete() + url = reverse("rest_register") + preferences["users__registration_enabled"] = True + settings.ACCOUNT_USERNAME_BLACKLIST = [] + data = { + "username": user.username, + "email": "contact@funkwhale.io", + "password1": "testtest", + "password2": "testtest", + } + + response = api_client.post(url, data) + + assert response.status_code == 400 + assert "username" in response.data diff --git a/changes/changelog.d/852.feature b/changes/changelog.d/852.feature new file mode 100644 index 000000000..3ce6f96be --- /dev/null +++ b/changes/changelog.d/852.feature @@ -0,0 +1 @@ +Users can now delete their account without admin intervention (#852) diff --git a/changes/notes.rst b/changes/notes.rst index d7324a31b..442fc0528 100644 --- a/changes/notes.rst +++ b/changes/notes.rst @@ -49,6 +49,18 @@ For more information about this feature, please check out our documentation: - `User documentation <https://docs.funkwhale.audio/moderator/reports.html>`_ - `Moderator documentation <https://docs.funkwhale.audio/users/reports.html>`_ +Account deletion +^^^^^^^^^^^^^^^^ + +Users can now delete their account themselves, without involving an administrator. + +The deletion process will remove any local data and objects associated with the account, +but the username won't be able to new users to avoid impersonation. Deletion is also broadcasted +to other known servers on the federation. + +For more information about this feature, please check out our documentation: + +- `User documentation <https://docs.funkwhale.audio/users/account.html>`_ Allow-list to restrict federation to trusted domains ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/docs/users/account.rst b/docs/users/account.rst new file mode 100644 index 000000000..6d9891987 --- /dev/null +++ b/docs/users/account.rst @@ -0,0 +1,14 @@ +Manage your account +=================== + +Delete your account +------------------- + +You can delete your Funkwhale account by visiting your settings. The deletion form is found at the bottom of the page. You will need to input your password to confirm the deletion. + +Once the deletion request is submitted, your account and associated data will be removed from the server within a few minutes. This includes, but isn't limited to your avatar, email address, music, favorites, radios, followers and playlists. + +Your server will also broadcast a message to other server on the federation to inform them about the deletion. + +Please note that while these servers are supposed to comply and delete any local copy of your data, this isn't a strong guarantee and some data may remain available, especially on servers +that are offline or unreachable when the deletion message is broadcasted. diff --git a/docs/users/index.rst b/docs/users/index.rst index b802cfb6f..8d46a49f6 100644 --- a/docs/users/index.rst +++ b/docs/users/index.rst @@ -21,6 +21,7 @@ Using Funkwhale .. toctree:: :maxdepth: 2 + account queue managing playlists diff --git a/front/src/components/auth/Settings.vue b/front/src/components/auth/Settings.vue index 5d60d8cd5..989ec68df 100644 --- a/front/src/components/auth/Settings.vue +++ b/front/src/components/auth/Settings.vue @@ -267,6 +267,43 @@ </translate> </empty-state> </section> + <section class="ui text container"> + <div class="ui hidden divider"></div> + <h2 class="ui header"> + <i class="trash icon"></i> + <div class="content"> + <translate translate-context="Content/Settings/Title/Verb">Delete my account</translate> + </div> + </h2> + <p> + <translate translate-context="Content/Settings/Paragraph'">You can permanently and irreversibly delete your account and all the associated data using the form below. You will be asked for confirmation.</translate> + </p> + <div class="ui warning message"> + <translate translate-context="Content/Settings/Paragraph'">Your account will be deleted from our servers within a few minutes. We will also notify other servers who may have a copy of some of your data so they can proceed to deletion. Please note that some of these servers may be offline or unwilling to comply though.</translate> + </div> + <div class="ui form""> + <div v-if="accountDeleteErrors.length > 0" class="ui negative message"> + <div class="header"><translate translate-context="Content/Settings/Error message.Title">We cannot delete your account</translate></div> + <ul class="list"> + <li v-for="error in accountDeleteErrors">{{ error }}</li> + </ul> + </div> + <div class="field"> + <label><translate translate-context="*/*/*">Password</translate></label> + <password-input required v-model="password" /> + </div> + <dangerous-button + :class="['ui', {'loading': isDeletingAccount}, {disabled: !password}, 'button']" + :action="deleteAccount"> + <translate translate-context="*/*/Button.Label">Delete my account…</translate> + <p slot="modal-header"><translate translate-context="Popup/Settings/Title">Do you want to delete your account?</translate></p> + <div slot="modal-content"> + <p><translate translate-context="Popup/Settings/Paragraph">This is irreversible and will permanently remove your data from our servers. You will we immediatly logged out.</translate></p> + </div> + <div slot="modal-confirm"><translate translate-context="Popup/Settings/Button.Label">Delete my account</translate></div> + </dangerous-button> + </div> + </section> </div> </main> </template> @@ -293,8 +330,11 @@ export default { new_password: "", currentAvatar: this.$store.state.auth.profile.avatar, passwordError: "", + password: "", isLoading: false, isLoadingAvatar: false, + isDeletingAccount: false, + accountDeleteErrors: [], avatarErrors: [], avatar: null, apps: [], @@ -471,7 +511,32 @@ export default { self.isLoading = false } ) - } + }, + deleteAccount() { + this.isDeletingAccount = true + this.accountDeleteErrors = [] + let self = this + let payload = { + confirm: true, + password: this.password, + } + axios.delete(`users/users/me/`, {data: payload}) + .then( + response => { + self.isDeletingAccount = false + let msg = self.$pgettext('*/Auth/Message', 'Your deletion request was submitted, your account and content will be deleted shortly') + self.$store.commit('ui/addMessage', { + content: msg, + date: new Date() + }) + self.$store.dispatch('auth/logout') + }, + error => { + self.isDeletingAccount = false + self.accountDeleteErrors = error.backendErrors + } + ) + }, }, computed: { labels() { -- GitLab