From 5b051631ecf296844b70c669d1cab055d32a7f7b Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 18 Sep 2023 21:21:04 +1000 Subject: [PATCH] Move MVP * update User model to allow for moved_to and also_known_as values * allow users to add aliases (also_known_as) in UI * allow users to move account to another one (moved_to) * redirect webfinger to the new account after a move * present notification to followers inviting to follow at new account Note: unlike Mastodon we're not running any unfollow/autofollow action here: users can decide for themselves This makes undoing moves easier. TODO There is still a bug with incoming Moves, at least from Mastodon. This seems to be something to do with Update activities (rather than Move, strictly). --- bookwyrm/activitypub/person.py | 2 + bookwyrm/activitypub/verbs.py | 21 ++- bookwyrm/forms/edit_user.py | 16 ++ bookwyrm/models/__init__.py | 2 + bookwyrm/models/move.py | 91 ++++++++--- bookwyrm/models/notification.py | 21 ++- bookwyrm/models/user.py | 13 ++ bookwyrm/templates/notifications/item.html | 2 +- .../templates/notifications/items/layout.html | 4 + .../templates/notifications/items/move.html | 28 ---- .../notifications/items/move_user.html | 20 +++ .../templates/preferences/alias_user.html | 56 +++++++ bookwyrm/templates/preferences/layout.html | 8 + bookwyrm/templates/preferences/move_user.html | 41 +++++ .../templates/settings/users/user_admin.html | 15 +- .../templates/settings/users/user_info.html | 12 +- .../templates/snippets/move_user_buttons.html | 13 ++ bookwyrm/templates/user/layout.html | 141 ++++++++++-------- bookwyrm/templates/user/moved.html | 27 ++++ bookwyrm/templatetags/utilities.py | 19 +++ bookwyrm/urls.py | 5 + bookwyrm/views/__init__.py | 1 + bookwyrm/views/preferences/move_user.py | 98 ++++++++++++ bookwyrm/views/wellknown.py | 3 +- 24 files changed, 521 insertions(+), 138 deletions(-) delete mode 100644 bookwyrm/templates/notifications/items/move.html create mode 100644 bookwyrm/templates/notifications/items/move_user.html create mode 100644 bookwyrm/templates/preferences/alias_user.html create mode 100644 bookwyrm/templates/preferences/move_user.html create mode 100644 bookwyrm/templates/snippets/move_user_buttons.html create mode 100644 bookwyrm/templates/user/moved.html create mode 100644 bookwyrm/views/preferences/move_user.py diff --git a/bookwyrm/activitypub/person.py b/bookwyrm/activitypub/person.py index 61c15a579..85cf44409 100644 --- a/bookwyrm/activitypub/person.py +++ b/bookwyrm/activitypub/person.py @@ -40,4 +40,6 @@ class Person(ActivityObject): manuallyApprovesFollowers: str = False discoverable: str = False hideFollows: str = False + movedTo: str = None + alsoKnownAs: dict[str] = None type: str = "Person" diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 3432da4d5..0b04d9eba 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -75,6 +75,9 @@ class Update(Verb): """update a model instance from the dataclass""" if not self.object: return + # BUG: THIS IS THROWING A DUPLIATE USERNAME ERROR WHEN WE GET AN "UPDATE" AFTER/BEFORE A "MOVE" + # FROM MASTODON - BUT ONLY SINCE WE ADDED MOVEUSER + # is it something to do with the updated User model? self.object.to_model( allow_create=False, allow_external_connections=allow_external_connections ) @@ -232,27 +235,23 @@ class Announce(Verb): """boost""" self.to_model(allow_external_connections=allow_external_connections) + @dataclass(init=False) class Move(Verb): """a user moving an object""" - # note the spec example for target and origin is an object but - # Mastodon uses a URI string and TBH this makes more sense - # Is there a way we can account for either format? - object: str type: str = "Move" - target: str - origin: str + origin: str = None + target: str = None def action(self, allow_external_connections=True): """move""" - # we need to work out whether the object is a user or something else. - - object_is_user = True # TODO! + object_is_user = resolve_remote_id(remote_id=self.object, model="User") if object_is_user: - self.to_model(object_is_user=True allow_external_connections=allow_external_connections) + model = apps.get_model("bookwyrm.MoveUser") + self.to_model(model=model) else: - self.to_model(object_is_user=False allow_external_connections=allow_external_connections) \ No newline at end of file + return diff --git a/bookwyrm/forms/edit_user.py b/bookwyrm/forms/edit_user.py index ce7bb6d07..9024972c3 100644 --- a/bookwyrm/forms/edit_user.py +++ b/bookwyrm/forms/edit_user.py @@ -70,6 +70,22 @@ class DeleteUserForm(CustomForm): fields = ["password"] +class MoveUserForm(CustomForm): + target = forms.CharField(widget=forms.TextInput) + + class Meta: + model = models.User + fields = ["password"] + + +class AliasUserForm(CustomForm): + username = forms.CharField(widget=forms.TextInput) + + class Meta: + model = models.User + fields = ["password"] + + class ChangePasswordForm(CustomForm): current_password = forms.CharField(widget=forms.PasswordInput) confirm_password = forms.CharField(widget=forms.PasswordInput) diff --git a/bookwyrm/models/__init__.py b/bookwyrm/models/__init__.py index 7b779190b..9edb89286 100644 --- a/bookwyrm/models/__init__.py +++ b/bookwyrm/models/__init__.py @@ -27,6 +27,8 @@ from .group import Group, GroupMember, GroupMemberInvitation from .import_job import ImportJob, ImportItem +from .move import MoveUser, MoveUserNotification + from .site import SiteSettings, Theme, SiteInvite from .site import PasswordReset, InviteRequest from .announcement import Announcement diff --git a/bookwyrm/models/move.py b/bookwyrm/models/move.py index cb597e84a..5c8044ce3 100644 --- a/bookwyrm/models/move.py +++ b/bookwyrm/models/move.py @@ -1,4 +1,5 @@ """ move an object including migrating a user account """ +from django.core.exceptions import PermissionDenied from django.db import models from bookwyrm import activitypub @@ -6,6 +7,7 @@ from .activitypub_mixin import ActivityMixin from .base_model import BookWyrmModel from . import fields from .status import Status +from bookwyrm.models import User class Move(ActivityMixin, BookWyrmModel): @@ -15,20 +17,21 @@ class Move(ActivityMixin, BookWyrmModel): "User", on_delete=models.PROTECT, activitypub_field="actor" ) - # TODO: can we just use the abstract class here? - activitypub_object = fields.ForeignKey( - "BookWyrmModel", on_delete=models.PROTECT, + object = fields.CharField( + max_length=255, + blank=False, + null=False, + deduplication_field=True, activitypub_field="object", - blank=True, - null=True - ) - - target = fields.CharField( - max_length=255, blank=True, null=True, deduplication_field=True ) origin = fields.CharField( - max_length=255, blank=True, null=True, deduplication_field=True + max_length=255, + blank=True, + null=True, + deduplication_field=True, + default="", + activitypub_field="origin", ) activity_serializer = activitypub.Move @@ -37,14 +40,66 @@ class Move(ActivityMixin, BookWyrmModel): @classmethod def ignore_activity(cls, activity, allow_external_connections=True): """don't bother with incoming moves of unknown objects""" - # TODO how do we check this for any conceivable object? + # TODO pass - def save(self, *args, **kwargs): - """update user active time""" - self.user.update_active_date() - super().save(*args, **kwargs) - # Ok what else? We can trigger a notification for followers of a user who sends a `Move` for themselves - # What about when a book is merged (i.e. moved from one id into another)? We could use that to send out a message - # to other Bookwyrm instances to update their remote_id for the book, but ...how do we trigger any action? \ No newline at end of file +class MoveUser(Move): + """migrating an activitypub user account""" + + target = fields.ForeignKey( + "User", + on_delete=models.PROTECT, + related_name="move_target", + activitypub_field="target", + ) + + # pylint: disable=unused-argument + @classmethod + def ignore_activity(cls, activity, allow_external_connections=True): + """don't bother with incoming moves of unknown users""" + return not User.objects.filter(remote_id=activity.origin).exists() + + def save(self, *args, **kwargs): + """update user info and broadcast it""" + + notify_followers = False + if self.user in self.target.also_known_as.all(): + + self.user.also_known_as.add(self.target.id) + self.user.update_active_date() + self.user.moved_to = self.target.remote_id + self.user.save(update_fields=["moved_to"]) + + if self.user.local: + kwargs[ + "broadcast" + ] = True # Only broadcast if we are initiating the Move + notify_followers = True + + super().save(*args, **kwargs) + + if notify_followers: + for follower in self.user.followers.all(): + MoveUserNotification.objects.create(user=follower, target=self.user) + + else: + raise PermissionDenied() + + +class MoveUserNotification(models.Model): + """notify followers that the user has moved""" + + created_date = models.DateTimeField(auto_now_add=True) + + user = models.ForeignKey( + "User", on_delete=models.CASCADE, related_name="moved_user_notifications" + ) # user we are notifying + + target = models.ForeignKey( + "User", on_delete=models.CASCADE, related_name="moved_user_notification_target" + ) # new account of user who moved + + def save(self, *args, **kwargs): + """send notification""" + super().save(*args, **kwargs) diff --git a/bookwyrm/models/notification.py b/bookwyrm/models/notification.py index 48baacfaf..9026b23df 100644 --- a/bookwyrm/models/notification.py +++ b/bookwyrm/models/notification.py @@ -2,7 +2,14 @@ from django.db import models, transaction from django.dispatch import receiver from .base_model import BookWyrmModel -from . import Boost, Favorite, GroupMemberInvitation, ImportJob, LinkDomain +from . import ( + Boost, + Favorite, + GroupMemberInvitation, + ImportJob, + LinkDomain, + MoveUserNotification, +) from . import ListItem, Report, Status, User, UserFollowRequest @@ -330,13 +337,11 @@ def notify_user_on_follow(sender, instance, created, *args, **kwargs): read=False, ) -@receiver(models.signals.post_save, sender=Move) + +@receiver(models.signals.post_save, sender=MoveUserNotification) # pylint: disable=unused-argument def notify_on_move(sender, instance, *args, **kwargs): - """someone moved something""" + """someone migrated their account""" Notification.notify( - instance.status.user, - instance.user, - related_object=instance.object, - notification_type=Notification.MOVE, - ) \ No newline at end of file + instance.user, instance.target, notification_type=Notification.MOVE + ) diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 6e0912aec..6c87af18c 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -140,6 +140,17 @@ class User(OrderedCollectionPageMixin, AbstractUser): theme = models.ForeignKey("Theme", null=True, blank=True, on_delete=models.SET_NULL) hide_follows = fields.BooleanField(default=False) + # migration fields + + moved_to = fields.RemoteIdField( + null=True, unique=False, activitypub_field="movedTo" + ) + also_known_as = fields.ManyToManyField( + "self", + symmetrical=True, + activitypub_field="alsoKnownAs", + ) + # options to turn features on and off show_goal = models.BooleanField(default=True) show_suggested_users = models.BooleanField(default=True) @@ -314,6 +325,8 @@ class User(OrderedCollectionPageMixin, AbstractUser): "schema": "http://schema.org#", "PropertyValue": "schema:PropertyValue", "value": "schema:value", + "alsoKnownAs": {"@id": "as:alsoKnownAs", "@type": "@id"}, + "movedTo": {"@id": "as:movedTo", "@type": "@id"}, }, ] return activity_object diff --git a/bookwyrm/templates/notifications/item.html b/bookwyrm/templates/notifications/item.html index b2020839a..6c9b295f7 100644 --- a/bookwyrm/templates/notifications/item.html +++ b/bookwyrm/templates/notifications/item.html @@ -36,5 +36,5 @@ {% elif notification.notification_type == 'GROUP_DESCRIPTION' %} {% include 'notifications/items/update.html' %} {% elif notification.notification_type == 'MOVE' %} - {% include 'notifications/items/move.html' %} + {% include 'notifications/items/move_user.html' %} {% endif %} diff --git a/bookwyrm/templates/notifications/items/layout.html b/bookwyrm/templates/notifications/items/layout.html index 8acbb9fec..41353abcf 100644 --- a/bookwyrm/templates/notifications/items/layout.html +++ b/bookwyrm/templates/notifications/items/layout.html @@ -39,6 +39,8 @@ {% with related_user=related_users.0.display_name %} {% with related_user_link=related_users.0.local_path %} + {% with related_user_moved_to=related_users.0.moved_to %} + {% with related_user_username=related_users.0.username %} {% with second_user=related_users.1.display_name %} {% with second_user_link=related_users.1.local_path %} {% with other_user_count=related_user_count|add:"-1" %} @@ -50,6 +52,8 @@ {% endwith %} {% endwith %} {% endwith %} + {% endwith %} + {% endwith %} {% if related_status %} diff --git a/bookwyrm/templates/notifications/items/move.html b/bookwyrm/templates/notifications/items/move.html deleted file mode 100644 index ffa23829f..000000000 --- a/bookwyrm/templates/notifications/items/move.html +++ /dev/null @@ -1,28 +0,0 @@ -{% extends 'notifications/items/layout.html' %} - -{% load i18n %} -{% load utilities %} - -{% block primary_link %}{% spaceless %} - {{ notification.related_object.local_path }} -{% endspaceless %}{% endblock %} - -{% block icon %} - -{% endblock %} - -{% block description %} - - {% blocktrans trimmed with object_name=notification.related_object.name object_path=notification.related_object.local_path %} - {{ related_user }} - moved {{ object_name }} - "{{ object_name }}" - {% endblocktrans %} - - - -{% endblock %} diff --git a/bookwyrm/templates/notifications/items/move_user.html b/bookwyrm/templates/notifications/items/move_user.html new file mode 100644 index 000000000..39d3af83b --- /dev/null +++ b/bookwyrm/templates/notifications/items/move_user.html @@ -0,0 +1,20 @@ +{% extends 'notifications/items/layout.html' %} + +{% load i18n %} +{% load utilities %} +{% load user_page_tags %} + +{% block primary_link %}{% spaceless %} + {{ notification.related_object.local_path }} +{% endspaceless %}{% endblock %} + +{% block icon %} + +{% endblock %} + +{% block description %} + {{ related_user }} {% trans "has moved to" %} {% id_to_username related_user_moved_to %} +
+ {% include 'snippets/move_user_buttons.html' with group=notification.related_group %} +
+{% endblock %} diff --git a/bookwyrm/templates/preferences/alias_user.html b/bookwyrm/templates/preferences/alias_user.html new file mode 100644 index 000000000..493bf83de --- /dev/null +++ b/bookwyrm/templates/preferences/alias_user.html @@ -0,0 +1,56 @@ +{% extends 'preferences/layout.html' %} +{% load i18n %} + +{% block title %}{% trans "Move Account" %}{% endblock %} + +{% block header %} +{% trans "Create Alias" %} +{% endblock %} + +{% block panel %} +
+

{% trans "Add another account as an alias" %}

+
+
+

+ {% trans "Marking another account as an alias is required if you want to move that account to this one. This is a reversable action and will not change this account." %} +

+
+
+ {% csrf_token %} +
+ + + {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %} +
+
+ + + {% include 'snippets/form_errors.html' with errors_list=form.username.errors id="desc_username" %} +
+ +
+
+ {% if user.also_known_as.all.0 %} +
+

{% trans "Aliases" %}

+
+ + {% for alias in user.also_known_as.all %} + + + + + {% endfor %} +
{{ alias.username }} +
+ {% csrf_token %} + + +
+
+
+
+ {% endif %} +
+{% endblock %} diff --git a/bookwyrm/templates/preferences/layout.html b/bookwyrm/templates/preferences/layout.html index ca63ec93d..2ef09a245 100644 --- a/bookwyrm/templates/preferences/layout.html +++ b/bookwyrm/templates/preferences/layout.html @@ -23,6 +23,14 @@ {% url 'prefs-2fa' as url %} {% trans "Two Factor Authentication" %} +
  • + {% url 'prefs-move' as url %} + {% trans "Move Account" %} +
    • +
  • + {% url 'prefs-alias' as url %} + {% trans "Add alias" %} +
  • {% url 'prefs-delete' as url %} {% trans "Delete Account" %} diff --git a/bookwyrm/templates/preferences/move_user.html b/bookwyrm/templates/preferences/move_user.html new file mode 100644 index 000000000..95fa4811f --- /dev/null +++ b/bookwyrm/templates/preferences/move_user.html @@ -0,0 +1,41 @@ +{% extends 'preferences/layout.html' %} +{% load i18n %} + +{% block title %}{% trans "Move Account" %}{% endblock %} + +{% block header %} +{% trans "Move Account" %} +{% endblock %} + +{% block panel %} +
    +

    {% trans "Migrate account to another server" %}

    +
    +
    +

    + {% trans "Moving your account will notify all your followers and redirect them to the new account." %} +

    +

    + {{ user.username }} {% trans "will be marked as moved and will not be discoverable." %} +

    +
    +
    +

    {% trans "Remember to add this user as an alias of the target account before you try to move." %}

    +
    +
    + {% csrf_token %} +
    + + + {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %} +
    +
    + + + {% include 'snippets/form_errors.html' with errors_list=form.target.errors id="desc_target" %} +
    + +
    +
    +
    +{% endblock %} diff --git a/bookwyrm/templates/settings/users/user_admin.html b/bookwyrm/templates/settings/users/user_admin.html index 9bc5805b1..a1d93ddd0 100644 --- a/bookwyrm/templates/settings/users/user_admin.html +++ b/bookwyrm/templates/settings/users/user_admin.html @@ -75,10 +75,17 @@ {{ user.last_active_date }} {% if user.is_active %} - - {% trans "Active" %} + {% if user.moved_to %} + + {% trans "Moved" %} + {% else %} + + {% trans "Active" %} + {% endif %} {% elif user.deactivation_reason == "moderator_deletion" or user.deactivation_reason == "self_deletion" %}