Merge pull request #3086 from bookwyrm-social/user-deletion

Erase user data and statuses on account deletion
This commit is contained in:
Mouse Reeve 2023-11-06 09:49:06 -08:00 committed by GitHub
commit e7a1572450
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 329 additions and 50 deletions

View file

@ -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),
),
]

View file

@ -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
),
]

View file

@ -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):

View file

@ -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"""

View file

@ -74,31 +74,7 @@
<td>{{ user.created_date }}</td>
<td>{{ user.last_active_date }}</td>
<td>
{% if user.is_active %}
{% if user.moved_to %}
<span class="tag is-info" aria-hidden="true">
<span class="icon icon-x"></span>
</span>
{% trans "Moved" %}
{% else %}
<span class="tag is-success" aria-hidden="true">
<span class="icon icon-check"></span>
</span>
{% trans "Active" %}
{% endif %}
{% elif user.deactivation_reason == "moderator_deletion" or user.deactivation_reason == "self_deletion" %}
<span class="tag is-danger" aria-hidden="true">
<span class="icon icon-x"></span>
</span>
{% trans "Deleted" %}
<span class="help">({{ user.get_deactivation_reason_display }})</span>
{% else %}
<span class="tag is-warning" aria-hidden="true">
<span class="icon icon-x"></span>
</span>
{% trans "Inactive" %}
<span class="help">({{ user.get_deactivation_reason_display }})</span>
{% endif %}
{% include "snippets/user_active_tag.html" with user=user %}
</td>
{% if status == "federated" %}
<td>

View file

@ -23,24 +23,7 @@
<div class="column is-flex is-flex-direction-column is-4">
<h4 class="title is-4">{% trans "Status" %}</h4>
<div class="box is-flex-grow-1 has-text-weight-bold">
{% if user.is_active %}
{% if user.moved_to %}
<p class="notification is-info">
{% trans "Moved" %}
</p>
{% else %}
<p class="notification is-success">
{% trans "Active" %}
</p>
{% endif %}
{% else %}
<p class="notification is-warning">
{% trans "Inactive" %}
{% if user.deactivation_reason %}
<span class="help">({% trans user.get_deactivation_reason_display %})</span>
{% endif %}
</p>
{% endif %}
{% include "snippets/user_active_tag.html" with large=True %}
<p class="notification">
{% if user.local %}
{% trans "Local" %}

View file

@ -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 %}

View file

@ -0,0 +1,19 @@
{% if large %}
<p class="notification is-{{ level }}">
<span class="icon icon-{{ icon }}" aria-hidden="true"></span>
{{ text }}
{% if deactivation_reason %}
<span class="help">({{ deactivation_reason }})</span>
{% endif %}
</p>
{% else %}
<span class="tag is-{{ level }}" aria-hidden="true">
<span class="icon icon-{{ icon }}"></span>
</span>
{{ text }}
{% endif %}

View file

@ -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)

View file

@ -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()

View file

@ -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 = {