diff --git a/bookwyrm/migrations/0183_auto_20231105_1607.py b/bookwyrm/migrations/0183_auto_20231105_1607.py new file mode 100644 index 000000000..0c8376adc --- /dev/null +++ b/bookwyrm/migrations/0183_auto_20231105_1607.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.20 on 2023-11-05 16:07 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0182_auto_20231027_1122"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="is_deleted", + field=models.BooleanField(default=False), + ), + ] diff --git a/bookwyrm/migrations/0184_auto_20231106_0421.py b/bookwyrm/migrations/0184_auto_20231106_0421.py new file mode 100644 index 000000000..e8197dea1 --- /dev/null +++ b/bookwyrm/migrations/0184_auto_20231106_0421.py @@ -0,0 +1,49 @@ +# Generated by Django 3.2.20 on 2023-11-06 04:21 + +from django.db import migrations +from bookwyrm.models import User + + +def update_deleted_users(apps, schema_editor): + """Find all the users who are deleted, not just inactive, and set deleted""" + users = apps.get_model("bookwyrm", "User") + db_alias = schema_editor.connection.alias + users.objects.using(db_alias).filter( + is_active=False, + deactivation_reason__in=[ + "self_deletion", + "moderator_deletion", + ], + ).update(is_deleted=True) + + # differente rules for remote users + users.objects.using(db_alias).filter(is_active=False, local=False,).exclude( + deactivation_reason="moderator_deactivation", + ).update(is_deleted=True) + + +def erase_deleted_user_data(apps, schema_editor): + """Retroactively clear user data""" + for user in User.objects.filter(is_deleted=True): + user.erase_user_data() + user.save( + broadcast=False, + update_fields=["email", "avatar", "preview_image", "summary", "name"], + ) + user.erase_user_statuses(broadcast=False) + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0183_auto_20231105_1607"), + ] + + operations = [ + migrations.RunPython( + update_deleted_users, reverse_code=migrations.RunPython.noop + ), + migrations.RunPython( + erase_deleted_user_data, reverse_code=migrations.RunPython.noop + ), + ] diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 11646431b..cc44fe2bf 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -102,7 +102,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): if hasattr(self, "quotation"): self.quotation = None # pylint: disable=attribute-defined-outside-init self.deleted_date = timezone.now() - self.save() + self.save(*args, **kwargs) @property def recipients(self): diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index c152cf445..75ca1d527 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -1,13 +1,14 @@ """ database schema for user data """ import re from urllib.parse import urlparse +from uuid import uuid4 from django.apps import apps from django.contrib.auth.models import AbstractUser from django.contrib.postgres.fields import ArrayField, CICharField from django.core.exceptions import PermissionDenied, ObjectDoesNotExist from django.dispatch import receiver -from django.db import models, transaction +from django.db import models, transaction, IntegrityError from django.utils import timezone from django.utils.translation import gettext_lazy as _ from model_utils import FieldTracker @@ -53,6 +54,7 @@ class User(OrderedCollectionPageMixin, AbstractUser): username = fields.UsernameField() email = models.EmailField(unique=True, null=True) + is_deleted = models.BooleanField(default=False) key_pair = fields.OneToOneField( "KeyPair", @@ -394,9 +396,44 @@ class User(OrderedCollectionPageMixin, AbstractUser): """We don't actually delete the database entry""" # pylint: disable=attribute-defined-outside-init self.is_active = False - self.avatar = "" + self.allow_reactivation = False + self.is_deleted = True + + self.erase_user_data() + self.erase_user_statuses() + # skip the logic in this class's save() - super().save(*args, **kwargs) + super().save( + *args, + **kwargs, + ) + + def erase_user_data(self): + """Wipe a user's custom data""" + if not self.is_deleted: + raise IntegrityError( + "Trying to erase user data on user that is not deleted" + ) + + # mangle email address + self.email = f"{uuid4()}@deleted.user" + + # erase data fields + self.avatar = "" + self.preview_image = "" + self.summary = None + self.name = None + self.favorites.set([]) + + def erase_user_statuses(self, broadcast=True): + """Wipe the data on all the user's statuses""" + if not self.is_deleted: + raise IntegrityError( + "Trying to erase user data on user that is not deleted" + ) + + for status in self.status_set.all(): + status.delete(broadcast=broadcast) def deactivate(self): """Disable the user but allow them to reactivate""" diff --git a/bookwyrm/templates/settings/users/user_admin.html b/bookwyrm/templates/settings/users/user_admin.html index a1d93ddd0..cc5c51ba7 100644 --- a/bookwyrm/templates/settings/users/user_admin.html +++ b/bookwyrm/templates/settings/users/user_admin.html @@ -74,31 +74,7 @@ {{ user.created_date }} {{ user.last_active_date }} - {% if user.is_active %} - {% if user.moved_to %} - - {% trans "Moved" %} - {% else %} - - {% trans "Active" %} - {% endif %} - {% elif user.deactivation_reason == "moderator_deletion" or user.deactivation_reason == "self_deletion" %} - - {% trans "Deleted" %} - ({{ user.get_deactivation_reason_display }}) - {% else %} - - {% trans "Inactive" %} - ({{ user.get_deactivation_reason_display }}) - {% endif %} + {% include "snippets/user_active_tag.html" with user=user %} {% if status == "federated" %} diff --git a/bookwyrm/templates/settings/users/user_info.html b/bookwyrm/templates/settings/users/user_info.html index 368045a0d..f35c60db9 100644 --- a/bookwyrm/templates/settings/users/user_info.html +++ b/bookwyrm/templates/settings/users/user_info.html @@ -23,24 +23,7 @@

{% trans "Status" %}

- {% if user.is_active %} - {% if user.moved_to %} -

- {% trans "Moved" %} -

- {% else %} -

- {% trans "Active" %} -

- {% endif %} - {% else %} -

- {% trans "Inactive" %} - {% if user.deactivation_reason %} - ({% trans user.get_deactivation_reason_display %}) - {% endif %} -

- {% endif %} + {% include "snippets/user_active_tag.html" with large=True %}

{% if user.local %} {% trans "Local" %} diff --git a/bookwyrm/templates/snippets/user_active_tag.html b/bookwyrm/templates/snippets/user_active_tag.html new file mode 100644 index 000000000..1d85ae68b --- /dev/null +++ b/bookwyrm/templates/snippets/user_active_tag.html @@ -0,0 +1,17 @@ +{% load i18n %} + +{% if user.is_active %} + {% if user.moved_to %} + {% trans "Moved" as text %} + {% include "snippets/user_active_tag_item.html" with icon="x" text=text level="info" %} + {% else %} + {% trans "Active" as text %} + {% include "snippets/user_active_tag_item.html" with icon="check" text=text level="success" %} + {% endif %} +{% elif user.is_deleted %} + {% trans "Deleted" as text %} + {% include "snippets/user_active_tag_item.html" with icon="x" text=text level="danger" deactivation_reason=user.get_deactivation_reason_display %} +{% else %} + {% trans "Inactive" as text %} + {% include "snippets/user_active_tag_item.html" with icon="x" text=text level="warning" deactivation_reason=user.get_deactivation_reason_display %} +{% endif %} diff --git a/bookwyrm/templates/snippets/user_active_tag_item.html b/bookwyrm/templates/snippets/user_active_tag_item.html new file mode 100644 index 000000000..e722150f2 --- /dev/null +++ b/bookwyrm/templates/snippets/user_active_tag_item.html @@ -0,0 +1,19 @@ +{% if large %} + +

+ + {{ text }} + {% if deactivation_reason %} + ({{ deactivation_reason }}) + {% endif %} +

+ +{% else %} + + +{{ text }} + +{% endif %} + diff --git a/bookwyrm/tests/migrations/test_0184.py b/bookwyrm/tests/migrations/test_0184.py new file mode 100644 index 000000000..4bf1b66c9 --- /dev/null +++ b/bookwyrm/tests/migrations/test_0184.py @@ -0,0 +1,121 @@ +""" testing migrations """ +from unittest.mock import patch + +from django.test import TestCase +from django.db.migrations.executor import MigrationExecutor +from django.db import connection + +from bookwyrm import models +from bookwyrm.management.commands import initdb +from bookwyrm.settings import DOMAIN + +# pylint: disable=missing-class-docstring +# pylint: disable=missing-function-docstring +class EraseDeletedUserDataMigration(TestCase): + + migrate_from = "0183_auto_20231105_1607" + migrate_to = "0184_auto_20231106_0421" + + # pylint: disable=invalid-name + def setUp(self): + with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( + "bookwyrm.activitystreams.populate_stream_task.delay" + ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): + self.active_user = models.User.objects.create_user( + f"activeuser@{DOMAIN}", + "activeuser@activeuser.activeuser", + "activeuserword", + local=True, + localname="active", + name="a name", + ) + self.inactive_user = models.User.objects.create_user( + f"inactiveuser@{DOMAIN}", + "inactiveuser@inactiveuser.inactiveuser", + "inactiveuserword", + local=True, + localname="inactive", + is_active=False, + deactivation_reason="self_deactivation", + name="name name", + ) + self.deleted_user = models.User.objects.create_user( + f"deleteduser@{DOMAIN}", + "deleteduser@deleteduser.deleteduser", + "deleteduserword", + local=True, + localname="deleted", + is_active=False, + deactivation_reason="self_deletion", + name="cool name", + ) + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" + ), patch("bookwyrm.activitystreams.add_status_task.delay"): + self.active_status = models.Status.objects.create( + user=self.active_user, content="don't delete me" + ) + self.inactive_status = models.Status.objects.create( + user=self.inactive_user, content="also don't delete me" + ) + self.deleted_status = models.Status.objects.create( + user=self.deleted_user, content="yes, delete me" + ) + + initdb.init_groups() + initdb.init_permissions() + + self.migrate_from = [("bookwyrm", self.migrate_from)] + self.migrate_to = [("bookwyrm", self.migrate_to)] + executor = MigrationExecutor(connection) + old_apps = executor.loader.project_state(self.migrate_from).apps + + # Reverse to the original migration + executor.migrate(self.migrate_from) + + self.setUpBeforeMigration(old_apps) + + # Run the migration to test + executor = MigrationExecutor(connection) + executor.loader.build_graph() # reload. + with patch("bookwyrm.activitystreams.remove_status_task.delay"): + executor.migrate(self.migrate_to) + + self.apps = executor.loader.project_state(self.migrate_to).apps + + def setUpBeforeMigration(self, apps): + pass + + def test_user_data_deleted(self): + """Make sure that only the right data was deleted""" + self.active_user.refresh_from_db() + self.inactive_user.refresh_from_db() + self.deleted_user.refresh_from_db() + self.active_status.refresh_from_db() + self.inactive_status.refresh_from_db() + self.deleted_status.refresh_from_db() + + self.assertTrue(self.active_user.is_active) + self.assertFalse(self.active_user.is_deleted) + self.assertEqual(self.active_user.name, "a name") + self.assertNotEqual(self.deleted_user.email, "activeuser@activeuser.activeuser") + self.assertFalse(self.active_status.deleted) + self.assertEqual(self.active_status.content, "don't delete me") + + self.assertFalse(self.inactive_user.is_active) + self.assertFalse(self.inactive_user.is_deleted) + self.assertEqual(self.inactive_user.name, "name name") + self.assertNotEqual( + self.deleted_user.email, "inactiveuser@inactiveuser.inactiveuser" + ) + self.assertFalse(self.inactive_status.deleted) + self.assertEqual(self.inactive_status.content, "also don't delete me") + + self.assertFalse(self.deleted_user.is_active) + self.assertTrue(self.deleted_user.is_deleted) + self.assertIsNone(self.deleted_user.name) + self.assertNotEqual( + self.deleted_user.email, "deleteduser@deleteduser.deleteduser" + ) + self.assertTrue(self.deleted_status.deleted) + self.assertIsNone(self.deleted_status.content) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index 838dd2e49..30d7918c0 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -1,7 +1,9 @@ """ testing models """ import json + from unittest.mock import patch from django.contrib.auth.models import Group +from django.db import IntegrityError from django.test import TestCase import responses @@ -9,9 +11,11 @@ from bookwyrm import models from bookwyrm.management.commands import initdb from bookwyrm.settings import USE_HTTPS, DOMAIN + # pylint: disable=missing-class-docstring # pylint: disable=missing-function-docstring class User(TestCase): + protocol = "https://" if USE_HTTPS else "http://" # pylint: disable=invalid-name @@ -26,6 +30,7 @@ class User(TestCase): local=True, localname="mouse", name="hi", + summary="a summary", bookwyrm_user=False, ) self.another_user = models.User.objects.create_user( @@ -218,19 +223,71 @@ class User(TestCase): @patch("bookwyrm.suggested_users.remove_user_task.delay") def test_delete_user(self, _): - """deactivate a user""" + """permanently delete a user""" self.assertTrue(self.user.is_active) + self.assertEqual(self.user.name, "hi") + self.assertEqual(self.user.summary, "a summary") + self.assertEqual(self.user.email, "mouse@mouse.mouse") with patch( "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async" - ) as broadcast_mock: + ) as broadcast_mock, patch( + "bookwyrm.models.user.User.erase_user_statuses" + ) as erase_statuses_mock: self.user.delete() + self.assertEqual(erase_statuses_mock.call_count, 1) + + # make sure the deletion is broadcast self.assertEqual(broadcast_mock.call_count, 1) activity = json.loads(broadcast_mock.call_args[1]["args"][1]) self.assertEqual(activity["type"], "Delete") self.assertEqual(activity["object"], self.user.remote_id) + + self.user.refresh_from_db() + + # the user's account data should be deleted + self.assertIsNone(self.user.name) + self.assertIsNone(self.user.summary) + self.assertNotEqual(self.user.email, "mouse@mouse.mouse") self.assertFalse(self.user.is_active) + @patch("bookwyrm.suggested_users.remove_user_task.delay") + @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") + @patch("bookwyrm.activitystreams.add_status_task.delay") + @patch("bookwyrm.activitystreams.remove_status_task.delay") + def test_delete_user_erase_statuses(self, *_): + """erase user statuses when user is deleted""" + status = models.Status.objects.create(user=self.user, content="hello") + self.assertFalse(status.deleted) + self.assertIsNotNone(status.content) + self.assertIsNone(status.deleted_date) + + self.user.delete() + status.refresh_from_db() + + self.assertTrue(status.deleted) + self.assertIsNone(status.content) + self.assertIsNotNone(status.deleted_date) + + @patch("bookwyrm.suggested_users.remove_user_task.delay") + @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") + @patch("bookwyrm.activitystreams.add_status_task.delay") + def test_delete_user_erase_statuses_invalid(self, *_): + """erase user statuses when user is deleted""" + status = models.Status.objects.create(user=self.user, content="hello") + self.assertFalse(status.deleted) + self.assertIsNotNone(status.content) + self.assertIsNone(status.deleted_date) + + self.user.deactivate() + with self.assertRaises(IntegrityError): + self.user.erase_user_statuses() + + status.refresh_from_db() + self.assertFalse(status.deleted) + self.assertIsNotNone(status.content) + self.assertIsNone(status.deleted_date) + def test_admins_no_admins(self): """list of admins""" result = models.User.admins() diff --git a/bookwyrm/tests/views/inbox/test_inbox_delete.py b/bookwyrm/tests/views/inbox/test_inbox_delete.py index 0fb108e22..7b4c12564 100644 --- a/bookwyrm/tests/views/inbox/test_inbox_delete.py +++ b/bookwyrm/tests/views/inbox/test_inbox_delete.py @@ -11,6 +11,7 @@ from bookwyrm import models, views class InboxActivities(TestCase): """inbox tests""" + # pylint: disable=invalid-name def setUp(self): """basic user and book data""" with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch( @@ -97,7 +98,8 @@ class InboxActivities(TestCase): self.assertEqual(models.Notification.objects.get(), notif) @patch("bookwyrm.suggested_users.remove_user_task.delay") - def test_delete_user(self, _): + @patch("bookwyrm.activitystreams.remove_status_task.delay") + def test_delete_user(self, *_): """delete a user""" self.assertTrue(models.User.objects.get(username="rat@example.com").is_active) activity = {