Просмотр исходного кода

Create "DeletedUser" model for tracking number of deleted users and deletion reason (#1299)

* add DeletedUser model

* move model to deleteduser.py

* drop old model

* create create_user_deletion function

* add migrations

* fix code

* fix spaces

* drop deleted_on

* chenge magical numbers

* add DeletedUser model

* Revert "change magical numbers"

This reverts commit 0451b29674e20bdb2b755f053f42eefed74a0b8b.

* update migration

* move db_index to deleted_by

* redoing migration

* update from upstream

* move model to deleteduser.py

* drop old model

* create create_user_deletion function

* add migrations

* fix code

* fix spaces

* drop deleted_on

* chenge magical numbers

* Revert "change magical numbers"

This reverts commit 0451b29674e20bdb2b755f053f42eefed74a0b8b.

* update migration

* redoing migration

* Remove migrations

* Redo migration, format code with black

* Cleanup deletion recording utils

* Update backend logic to record user account deletions
Rafał Pitoń 5 лет назад
Родитель
Сommit
2c5e306140

+ 2 - 0
misago/users/admin/tasks.py

@@ -2,6 +2,7 @@ from celery import shared_task
 from django.contrib.auth import get_user_model
 from django.contrib.auth import get_user_model
 
 
 from ...conf.shortcuts import get_dynamic_settings
 from ...conf.shortcuts import get_dynamic_settings
+from ..deletesrecord import record_user_deleted_by_staff
 
 
 User = get_user_model()
 User = get_user_model()
 
 
@@ -15,3 +16,4 @@ def delete_user_with_content(pk):
     else:
     else:
         settings = get_dynamic_settings()
         settings = get_dynamic_settings()
         user.delete(anonymous_username=settings.anonymous_username, delete_content=True)
         user.delete(anonymous_username=settings.anonymous_username, delete_content=True)
+        record_user_deleted_by_staff()

+ 6 - 0
misago/users/admin/tests/test_deleting_user_with_content.py

@@ -1,6 +1,7 @@
 import pytest
 import pytest
 from django.contrib.auth import get_user_model
 from django.contrib.auth import get_user_model
 
 
+from ...models import DeletedUser
 from ..tasks import delete_user_with_content
 from ..tasks import delete_user_with_content
 
 
 User = get_user_model()
 User = get_user_model()
@@ -24,3 +25,8 @@ def test_task_deletes_user(user):
     delete_user_with_content(user.id)
     delete_user_with_content(user.id)
     with pytest.raises(User.DoesNotExist):
     with pytest.raises(User.DoesNotExist):
         user.refresh_from_db()
         user.refresh_from_db()
+
+
+def test_task_records_user_deletion_by_staff(user):
+    delete_user_with_content(user.id)
+    DeletedUser.objects.get(deleted_by=DeletedUser.DELETED_BY_STAFF)

+ 15 - 2
misago/users/admin/tests/test_users_mass_actions.py

@@ -4,11 +4,11 @@ import pytest
 from django.contrib.auth import get_user_model
 from django.contrib.auth import get_user_model
 from django.core import mail
 from django.core import mail
 
 
+from ... import BANS_CACHE
 from ....cache.test import assert_invalidates_cache
 from ....cache.test import assert_invalidates_cache
 from ....test import assert_contains, assert_has_error_message, assert_not_contains
 from ....test import assert_contains, assert_has_error_message, assert_not_contains
-from ... import BANS_CACHE
 from ...datadownloads import request_user_data_download
 from ...datadownloads import request_user_data_download
-from ...models import Ban, DataDownload
+from ...models import Ban, DataDownload, DeletedUser
 from ...test import create_test_user
 from ...test import create_test_user
 
 
 User = get_user_model()
 User = get_user_model()
@@ -185,6 +185,19 @@ def test_multiple_users_can_be_deleted_with_mass_action(admin_client, users_admi
             user.refresh_from_db()
             user.refresh_from_db()
 
 
 
 
+def test_mass_action_records_users_deletion_by_staff(admin_client, users_admin_link):
+    users = create_multiple_users()
+    response = admin_client.post(
+        users_admin_link,
+        data={"action": "delete_accounts", "selected_items": [u.id for u in users]},
+    )
+
+    deleted_count = DeletedUser.objects.filter(
+        deleted_by=DeletedUser.DELETED_BY_STAFF
+    ).count()
+    assert deleted_count == len(users)
+
+
 def test_delete_users_mass_action_fails_if_user_tries_to_delete_themselves(
 def test_delete_users_mass_action_fails_if_user_tries_to_delete_themselves(
     admin_client, users_admin_link, superuser
     admin_client, users_admin_link, superuser
 ):
 ):

+ 2 - 0
misago/users/admin/views/users.py

@@ -14,6 +14,7 @@ from ....core.pgutils import chunk_queryset
 from ....threads.models import Thread
 from ....threads.models import Thread
 from ...avatars.dynamic import set_avatar as set_dynamic_avatar
 from ...avatars.dynamic import set_avatar as set_dynamic_avatar
 from ...datadownloads import request_user_data_download, user_has_data_download_request
 from ...datadownloads import request_user_data_download, user_has_data_download_request
+from ...deletesrecord import record_user_deleted_by_staff
 from ...models import Ban
 from ...models import Ban
 from ...profilefields import profilefields
 from ...profilefields import profilefields
 from ...setupnewuser import setup_new_user
 from ...setupnewuser import setup_new_user
@@ -219,6 +220,7 @@ class UsersList(UserAdmin, generic.ListView):
 
 
         for user in users:
         for user in users:
             user.delete(anonymous_username=request.settings.anonymous_username)
             user.delete(anonymous_username=request.settings.anonymous_username)
+            record_user_deleted_by_staff()
 
 
         messages.success(request, _("Selected users have been deleted."))
         messages.success(request, _("Selected users have been deleted."))
 
 

+ 2 - 0
misago/users/api/users.py

@@ -17,6 +17,7 @@ from ...core.shortcuts import get_int_or_404
 from ...threads.moderation import hide_post, hide_thread
 from ...threads.moderation import hide_post, hide_thread
 from ..bans import get_user_ban
 from ..bans import get_user_ban
 from ..datadownloads import request_user_data_download, user_has_data_download_request
 from ..datadownloads import request_user_data_download, user_has_data_download_request
+from ..deletesrecord import record_user_deleted_by_staff
 from ..online.utils import get_user_status
 from ..online.utils import get_user_status
 from ..permissions import (
 from ..permissions import (
     allow_browse_users_list,
     allow_browse_users_list,
@@ -341,6 +342,7 @@ class UserViewSet(viewsets.GenericViewSet):
                         category.save()
                         category.save()
 
 
                 profile.delete(anonymous_username=request.settings.anonymous_username)
                 profile.delete(anonymous_username=request.settings.anonymous_username)
+                record_user_deleted_by_staff()
 
 
         return Response({})
         return Response({})
 
 

+ 13 - 0
misago/users/deletesrecord.py

@@ -0,0 +1,13 @@
+from .models import DeletedUser
+
+
+def record_user_deleted_by_self():
+    return DeletedUser.objects.create(deleted_by=DeletedUser.DELETED_BY_SELF)
+
+
+def record_user_deleted_by_staff():
+    return DeletedUser.objects.create(deleted_by=DeletedUser.DELETED_BY_STAFF)
+
+
+def record_user_deleted_by_system():
+    return DeletedUser.objects.create(deleted_by=DeletedUser.DELETED_BY_SYSTEM)

+ 2 - 0
misago/users/management/commands/deleteinactiveusers.py

@@ -6,6 +6,7 @@ from django.utils import timezone
 
 
 from ....conf.shortcuts import get_dynamic_settings
 from ....conf.shortcuts import get_dynamic_settings
 from ....core.pgutils import chunk_queryset
 from ....core.pgutils import chunk_queryset
+from ...deletesrecord import record_user_deleted_by_system
 
 
 User = get_user_model()
 User = get_user_model()
 
 
@@ -33,6 +34,7 @@ class Command(BaseCommand):
 
 
         for user in chunk_queryset(queryset):
         for user in chunk_queryset(queryset):
             user.delete(anonymous_username=settings.anonymous_username)
             user.delete(anonymous_username=settings.anonymous_username)
+            record_user_deleted_by_system()
             users_deleted += 1
             users_deleted += 1
 
 
         self.stdout.write("Deleted inactive user accounts: %s" % users_deleted)
         self.stdout.write("Deleted inactive user accounts: %s" % users_deleted)

+ 2 - 0
misago/users/management/commands/deletemarkedusers.py

@@ -3,6 +3,7 @@ from django.core.management.base import BaseCommand
 
 
 from ....conf.shortcuts import get_dynamic_settings
 from ....conf.shortcuts import get_dynamic_settings
 from ....core.pgutils import chunk_queryset
 from ....core.pgutils import chunk_queryset
+from ...deletesrecord import record_user_deleted_by_self
 from ...permissions import can_delete_own_account
 from ...permissions import can_delete_own_account
 
 
 User = get_user_model()
 User = get_user_model()
@@ -23,6 +24,7 @@ class Command(BaseCommand):
         for user in chunk_queryset(queryset):
         for user in chunk_queryset(queryset):
             if can_delete_own_account(settings, user, user):
             if can_delete_own_account(settings, user, user):
                 user.delete(anonymous_username=settings.anonymous_username)
                 user.delete(anonymous_username=settings.anonymous_username)
+                record_user_deleted_by_self()
                 users_deleted += 1
                 users_deleted += 1
 
 
         self.stdout.write("Deleted users: %s" % users_deleted)
         self.stdout.write("Deleted users: %s" % users_deleted)

+ 35 - 0
misago/users/migrations/0022_deleteduser.py

@@ -0,0 +1,35 @@
+# Generated by Django 2.2.3 on 2019-09-28 16:24
+
+from django.db import migrations, models
+import django.utils.timezone
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [("misago_users", "0021_user_sso_id")]
+
+    operations = [
+        migrations.CreateModel(
+            name="DeletedUser",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                ("deleted_on", models.DateTimeField(default=django.utils.timezone.now)),
+                (
+                    "deleted_by",
+                    models.PositiveIntegerField(
+                        choices=[(1, "By self"), (2, "By staff"), (3, "By system")],
+                        db_index=True,
+                    ),
+                ),
+            ],
+            options={"ordering": ["-deleted_on"]},
+        )
+    ]

+ 1 - 0
misago/users/models/__init__.py

@@ -7,3 +7,4 @@ from .audittrail import AuditTrail
 from .avatargallery import AvatarGallery
 from .avatargallery import AvatarGallery
 from .ban import Ban, BanCache
 from .ban import Ban, BanCache
 from .datadownload import DataDownload
 from .datadownload import DataDownload
+from .deleteduser import DeletedUser

+ 21 - 0
misago/users/models/deleteduser.py

@@ -0,0 +1,21 @@
+from django.db import models
+from django.utils import timezone
+from django.utils.translation import ugettext_lazy as _
+
+
+class DeletedUser(models.Model):
+    DELETED_BY_SELF = 1
+    DELETED_BY_STAFF = 2
+    DELETED_BY_SYSTEM = 3
+
+    DELETED_BY_CHOICES = (
+        (DELETED_BY_SELF, _("By self")),
+        (DELETED_BY_STAFF, _("By staff")),
+        (DELETED_BY_SYSTEM, _("By system")),
+    )
+
+    deleted_on = models.DateTimeField(default=timezone.now)
+    deleted_by = models.PositiveIntegerField(choices=DELETED_BY_CHOICES, db_index=True)
+
+    class Meta:
+        ordering = ["-deleted_on"]

+ 15 - 0
misago/users/tests/test_deleteinactiveusers.py

@@ -8,6 +8,7 @@ from django.utils import timezone
 
 
 from ...conf.test import override_dynamic_settings
 from ...conf.test import override_dynamic_settings
 from ..management.commands import deleteinactiveusers
 from ..management.commands import deleteinactiveusers
+from ..models import DeletedUser
 from ..test import create_test_user
 from ..test import create_test_user
 
 
 User = get_user_model()
 User = get_user_model()
@@ -50,6 +51,20 @@ class DeleteInactiveUsersTests(TestCase):
             self.user.refresh_from_db()
             self.user.refresh_from_db()
 
 
     @override_dynamic_settings(new_inactive_accounts_delete=2)
     @override_dynamic_settings(new_inactive_accounts_delete=2)
+    def test_inactive_user_deletion_is_recorded(self):
+        self.user.joined_on = timezone.now() - timedelta(days=2)
+        self.user.requires_activation = User.ACTIVATION_USER
+        self.user.save()
+
+        out = StringIO()
+        call_command(deleteinactiveusers.Command(), stdout=out)
+        command_output = out.getvalue().splitlines()[0].strip()
+
+        self.assertEqual(command_output, "Deleted inactive user accounts: 1")
+
+        DeletedUser.objects.get(deleted_by=DeletedUser.DELETED_BY_SYSTEM)
+
+    @override_dynamic_settings(new_inactive_accounts_delete=2)
     def test_skip_new_user_activation_user(self):
     def test_skip_new_user_activation_user(self):
         """skips inactive user that is too new"""
         """skips inactive user that is too new"""
         self.user.joined_on = timezone.now() - timedelta(days=1)
         self.user.joined_on = timezone.now() - timedelta(days=1)

+ 10 - 0
misago/users/tests/test_deletemarkedusers.py

@@ -6,6 +6,7 @@ from django.test import TestCase
 
 
 from ...conf.test import override_dynamic_settings
 from ...conf.test import override_dynamic_settings
 from ..management.commands import deletemarkedusers
 from ..management.commands import deletemarkedusers
+from ..models import DeletedUser
 from ..test import create_test_user
 from ..test import create_test_user
 
 
 User = get_user_model()
 User = get_user_model()
@@ -27,6 +28,15 @@ class DeleteMarkedUsersTests(TestCase):
         with self.assertRaises(User.DoesNotExist):
         with self.assertRaises(User.DoesNotExist):
             self.user.refresh_from_db()
             self.user.refresh_from_db()
 
 
+    def test_marked_user_deletion_is_recorded(self):
+        out = StringIO()
+        call_command(deletemarkedusers.Command(), stdout=out)
+        command_output = out.getvalue().splitlines()[0].strip()
+
+        self.assertEqual(command_output, "Deleted users: 1")
+
+        DeletedUser.objects.get(deleted_by=DeletedUser.DELETED_BY_SELF)
+
     @override_dynamic_settings(allow_delete_own_account=False)
     @override_dynamic_settings(allow_delete_own_account=False)
     def test_delete_disabled(self):
     def test_delete_disabled(self):
         """deletion respects user decision even if configuration has changed"""
         """deletion respects user decision even if configuration has changed"""

+ 21 - 0
misago/users/tests/test_recording_deletes.py

@@ -0,0 +1,21 @@
+from ..deletesrecord import (
+    record_user_deleted_by_self,
+    record_user_deleted_by_staff,
+    record_user_deleted_by_system,
+)
+from ..models import DeletedUser
+
+
+def test_deletion_by_self_creates_record(db):
+    record_user_deleted_by_self()
+    DeletedUser.objects.get(deleted_by=DeletedUser.DELETED_BY_SELF)
+
+
+def test_deletion_by_staff_creates_record(db):
+    record_user_deleted_by_staff()
+    DeletedUser.objects.get(deleted_by=DeletedUser.DELETED_BY_STAFF)
+
+
+def test_deletion_by_system_creates_record(db):
+    record_user_deleted_by_system()
+    DeletedUser.objects.get(deleted_by=DeletedUser.DELETED_BY_SYSTEM)

+ 14 - 1
misago/users/tests/test_users_api.py

@@ -10,7 +10,7 @@ from ...conf.test import override_dynamic_settings
 from ...threads.models import Post, Thread
 from ...threads.models import Post, Thread
 from ...threads.test import post_thread
 from ...threads.test import post_thread
 from ..activepostersranking import build_active_posters_ranking
 from ..activepostersranking import build_active_posters_ranking
-from ..models import Ban, Rank
+from ..models import Ban, DeletedUser, Rank
 from ..test import AuthenticatedUserTestCase, create_test_user
 from ..test import AuthenticatedUserTestCase, create_test_user
 
 
 User = get_user_model()
 User = get_user_model()
@@ -681,3 +681,16 @@ class UserDeleteTests(AuthenticatedUserTestCase):
 
 
         self.assertEqual(Thread.objects.count(), self.threads + 1)
         self.assertEqual(Thread.objects.count(), self.threads + 1)
         self.assertEqual(Post.objects.count(), self.posts + 2)
         self.assertEqual(Post.objects.count(), self.posts + 2)
+
+    @patch_user_acl(
+        {"can_delete_users_newer_than": 10, "can_delete_users_with_less_posts_than": 10}
+    )
+    def test_deleting_user_using_api_records_deletion_by_staff(self):
+        response = self.client.post(
+            self.link,
+            json.dumps({"with_content": False}),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, 200)
+
+        DeletedUser.objects.get(deleted_by=DeletedUser.DELETED_BY_STAFF)