feat(users): add transactional employee deletion and token revocation

tests/apps/user/test_deletion.py

@@ -0,0 +1,258 @@
+"""Account deletion keeps business history and revokes authentication."""
+
+from datetime import date
+from tempfile import TemporaryDirectory
+from unittest.mock import patch
+
+from apps.core.models import BackgroundJob, ReportUpload
+from apps.exchange.models import ExchangePackageImport
+from apps.registers.models import Register, RegisterUpload
+from apps.user.models import Profile, User
+from apps.user.services import UserService
+from apps.user.views import AdminUserDetailView
+from django.contrib.auth.models import Group
+from django.core.files.base import ContentFile
+from django.db import transaction
+from django.db.models.deletion import ProtectedError
+from django.test import override_settings
+from django.urls import path, reverse
+from drf_yasg import openapi
+from drf_yasg.generators import OpenAPISchemaGenerator
+from rest_framework.test import APITestCase
+from rest_framework_simplejwt.token_blacklist.models import (
+    BlacklistedToken,
+    OutstandingToken,
+)
+from rest_framework_simplejwt.tokens import RefreshToken
+
+from tests.apps.user.factories import UserFactory
+
+
+class UserDeletionTest(APITestCase):
+    """Verify deletion through the public administrative API."""
+
+    def setUp(self):
+        self.admin = UserFactory.create_superuser()
+        self.user = UserFactory.create_user()
+        self.user_id = self.user.pk
+        self.url = reverse("api_v1:user:admin-user-detail", args=[self.user_id])
+        self.client.force_authenticate(self.admin)
+
+    def test_delete_removes_account_and_credentials_but_preserves_history(self):
+        group = Group.objects.create(name="deletion-test")
+        self.user.groups.add(group)
+        refresh = RefreshToken.for_user(self.user)
+        refresh.blacklist()
+        report = ReportUpload.objects.create(
+            form="f1",
+            original_file="reports/keep.xlsx",
+            file_name="keep.xlsx",
+            content_type="application/octet-stream",
+            file_size=1,
+            file_hash="hash",
+            uploaded_by=self.user,
+        )
+        upload = RegisterUpload.objects.create(
+            registry=Register.objects.create(name="History"),
+            actual_date=date.today(),
+            file_name="keep.xlsx",
+            file_hash="hash",
+            uploaded_by=self.user,
+        )
+        imported = ExchangePackageImport.objects.create(
+            package_id="history-package",
+            package_name="keep.zip",
+            package_hash="hash",
+            imported_by=self.user,
+        )
+        job = BackgroundJob.objects.create(
+            task_id="history-task",
+            task_name="test",
+            user_id=self.user_id,
+        )
+
+        response = self.client.delete(self.url)
+
+        self.assertEqual(response.status_code, 204)
+        self.assertEqual(response.content, b"")
+        self.assertFalse(User.objects.filter(pk=self.user_id).exists())
+        self.assertFalse(Profile.objects.filter(user_id=self.user_id).exists())
+        self.assertFalse(OutstandingToken.objects.filter(jti=refresh["jti"]).exists())
+        self.assertEqual(BlacklistedToken.objects.count(), 0)
+        self.assertTrue(Group.objects.filter(pk=group.pk).exists())
+        self.assertEqual(
+            User.groups.through.objects.filter(user_id=self.user_id).count(), 0
+        )
+        for record, field in [
+            (report, "uploaded_by_id"),
+            (upload, "uploaded_by_id"),
+            (imported, "imported_by_id"),
+            (job, "user_id"),
+        ]:
+            record.refresh_from_db()
+            self.assertIsNone(getattr(record, field))
+        self.assertEqual(report.original_file.name, "reports/keep.xlsx")
+        self.assertEqual(job.status, "pending")
+        self.assertEqual(self.client.get(self.url).status_code, 404)
+        self.assertEqual(self.client.delete(self.url).status_code, 404)
+        response = self.client.get(
+            reverse("api_v1:user:admin_users"), {"search": self.user.username}
+        )
+        self.assertEqual(response.data["count"], 0)
+        replacement = UserFactory.create_user(
+            username=self.user.username, email=self.user.email
+        )
+        self.assertNotEqual(replacement.pk, self.user_id)
+
+    def test_deletion_revokes_access_and_refresh_even_without_outstanding_row(self):
+        tokens = UserService.get_tokens_for_user(self.user)
+        self.assertEqual(self.client.delete(self.url).status_code, 204)
+        self.client.force_authenticate(user=None)
+        self.client.credentials(HTTP_AUTHORIZATION=f"Bearer {tokens['access']}")
+        self.assertEqual(
+            self.client.get(reverse("api_v1:user:current_user")).status_code, 401
+        )
+        self.client.credentials()
+        response = self.client.post(
+            reverse("api_v1:user:token_refresh"), {"refresh": tokens["refresh"]}
+        )
+        self.assertEqual(response.status_code, 401)
+
+    def test_inactive_user_cannot_refresh(self):
+        tokens = UserService.get_tokens_for_user(self.user)
+        User.objects.filter(pk=self.user_id).update(is_active=False)
+        self.client.force_authenticate(user=None)
+        response = self.client.post(
+            reverse("api_v1:user:token_refresh"), {"refresh": tokens["refresh"]}
+        )
+        self.assertEqual(response.status_code, 401)
+
+    def test_invalid_user_claim_cannot_refresh(self):
+        refresh = RefreshToken.for_user(self.user)
+        refresh["user_id"] = "invalid-user-id"
+        self.client.force_authenticate(user=None)
+        response = self.client.post(
+            reverse("api_v1:user:token_refresh"), {"refresh": str(refresh)}
+        )
+        self.assertEqual(response.status_code, 401)
+
+    def test_self_delete_is_rejected_and_account_remains(self):
+        url = reverse("api_v1:user:admin-user-detail", args=[self.admin.pk])
+        response = self.client.delete(url)
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.data["errors"][0]["code"], "self_delete_forbidden")
+        self.assertTrue(
+            User.objects.filter(
+                pk=self.admin.pk, is_active=True, is_staff=True
+            ).exists()
+        )
+
+    def test_another_admin_and_inactive_user_can_be_deleted(self):
+        for attributes in ({"is_staff": True}, {"is_active": False}):
+            user = UserFactory.create_user(**attributes)
+            url = reverse("api_v1:user:admin-user-detail", args=[user.pk])
+            self.assertEqual(self.client.delete(url).status_code, 204)
+
+    def test_openapi_describes_delete_without_a_success_body(self):
+        generator = OpenAPISchemaGenerator(
+            info=openapi.Info(title="Account management", default_version="v1"),
+            patterns=[
+                path(
+                    "api/v1/users/admin/users/<int:user_id>/",
+                    AdminUserDetailView.as_view(),
+                )
+            ],
+        )
+        schema = generator.get_schema(request=None, public=True)
+        operation = next(
+            item["delete"] for item in schema.paths.values() if "delete" in item
+        )
+        self.assertEqual(
+            set(operation.responses), {"204", "400", "401", "403", "404", "409"}
+        )
+        self.assertNotIn("schema", operation.responses["204"])
+
+    def test_permission_is_rechecked_for_stale_authenticated_actor(self):
+        User.objects.filter(pk=self.admin.pk).update(is_staff=False)
+        self.assertEqual(self.client.delete(self.url).status_code, 403)
+        self.assertTrue(User.objects.filter(pk=self.user_id).exists())
+
+    def test_non_admin_and_anonymous_cannot_delete(self):
+        self.client.force_authenticate(self.user)
+        self.assertEqual(self.client.delete(self.url).status_code, 403)
+        self.client.force_authenticate(user=None)
+        self.assertEqual(self.client.delete(self.url).status_code, 401)
+
+    def test_transaction_rollback_restores_credentials_and_author_links(self):
+        refresh = RefreshToken.for_user(self.user)
+        job = BackgroundJob.objects.create(
+            task_id="rollback-job", task_name="test", user_id=self.user_id
+        )
+        with patch.object(
+            User, "delete", side_effect=RuntimeError("test rollback")
+        ), self.assertRaises(RuntimeError):
+            UserService.delete_user(self.user_id, actor_id=self.admin.pk)
+        self.assertTrue(User.objects.filter(pk=self.user_id).exists())
+        self.assertTrue(Profile.objects.filter(user_id=self.user_id).exists())
+        self.assertTrue(OutstandingToken.objects.filter(jti=refresh["jti"]).exists())
+        job.refresh_from_db()
+        self.assertEqual(job.user_id, self.user_id)
+
+    def test_avatar_cleanup_waits_for_commit_and_preserves_shared_files(self):
+        with TemporaryDirectory(prefix="statecorp-avatar-") as media, override_settings(
+            MEDIA_ROOT=media
+        ):
+            avatar = self.user.profile.avatar
+            avatar.save("test.png", ContentFile(b"avatar"))
+            storage, name = avatar.storage, avatar.name
+            with self.captureOnCommitCallbacks(execute=True):
+                UserService.delete_user(self.user_id, actor_id=self.admin.pk)
+                self.assertTrue(storage.exists(name))
+            self.assertFalse(storage.exists(name))
+
+            owner = UserFactory.create_user()
+            owner.profile.avatar.save("shared.png", ContentFile(b"shared"))
+            name = owner.profile.avatar.name
+            self.admin.profile.avatar = name
+            self.admin.profile.save()
+            with self.captureOnCommitCallbacks(execute=True):
+                UserService.delete_user(owner.pk, actor_id=self.admin.pk)
+            self.assertTrue(storage.exists(name))
+
+    def test_avatar_survives_rollback(self):
+        with TemporaryDirectory(
+            prefix="statecorp-avatar-rollback-"
+        ) as media, override_settings(MEDIA_ROOT=media):
+            self.user.profile.avatar.save("test.png", ContentFile(b"avatar"))
+            storage, name = (
+                self.user.profile.avatar.storage,
+                self.user.profile.avatar.name,
+            )
+            with self.captureOnCommitCallbacks(execute=True), self.assertRaises(
+                RuntimeError
+            ), transaction.atomic():
+                UserService.delete_user(self.user_id, actor_id=self.admin.pk)
+                raise RuntimeError("rollback outer transaction")
+            self.assertTrue(storage.exists(name))
+            self.assertTrue(User.objects.filter(pk=self.user_id).exists())
+
+    def test_protected_relation_returns_conflict_and_rolls_back(self):
+        refresh = RefreshToken.for_user(self.user)
+        with patch.object(User, "delete", side_effect=ProtectedError("protected", [])):
+            response = self.client.delete(self.url)
+        self.assertEqual(response.status_code, 409)
+        self.assertEqual(response.data["errors"][0]["code"], "user_delete_conflict")
+        self.assertTrue(User.objects.filter(pk=self.user_id).exists())
+        self.assertTrue(OutstandingToken.objects.filter(jti=refresh["jti"]).exists())
+
+    def test_avatar_storage_failure_does_not_undo_account_deletion(self):
+        self.user.profile.avatar = "avatars/unavailable.png"
+        self.user.profile.save()
+        with patch("apps.user.management.logger.error") as log_error, patch.object(
+            self.user.profile.avatar.storage,
+            "delete",
+            side_effect=OSError("storage unavailable"),
+        ), self.captureOnCommitCallbacks(execute=True):
+            self.assertEqual(self.client.delete(self.url).status_code, 204)
+        self.assertFalse(User.objects.filter(pk=self.user_id).exists())
+        log_error.assert_called_once()

tests/apps/user/test_management_admin.py

@@ -0,0 +1,122 @@
+"""Django admin must share the account deletion and access guards."""
+
+from unittest.mock import patch
+
+from apps.core.exceptions import BadRequestError, ConflictError, PermissionDeniedError
+from apps.user.admin import UserAdmin
+from apps.user.management import ensure_admins_remain, lock_managed_users
+from apps.user.models import User
+from django.contrib.admin import AdminSite
+from django.contrib.admin.models import LogEntry
+from django.test import RequestFactory, TestCase
+from django.urls import reverse
+from rest_framework_simplejwt.token_blacklist.models import OutstandingToken
+from rest_framework_simplejwt.tokens import RefreshToken
+
+from tests.apps.user.factories import UserFactory
+
+
+class ManagedUserAdminTest(TestCase):
+    def setUp(self):
+        self.actor = UserFactory.create_superuser()
+        self.user = UserFactory.create_user()
+        self.model_admin = UserAdmin(User, AdminSite())
+        self.request = RequestFactory().post("/admin/user/user/")
+        self.request.user = self.actor
+        self.initial_user_count = User.objects.count()
+
+    def test_admin_single_and_bulk_delete_clean_up_tokens(self):
+        second = UserFactory.create_user()
+        first_id, second_id = self.user.pk, second.pk
+        RefreshToken.for_user(self.user)
+        RefreshToken.for_user(second)
+        self.model_admin.delete_model(self.request, self.user)
+        self.model_admin.delete_queryset(
+            self.request, User.objects.filter(pk=second_id)
+        )
+        self.assertFalse(User.objects.filter(pk__in=[first_id, second_id]).exists())
+        self.assertFalse(
+            OutstandingToken.objects.filter(user_id__in=[first_id, second_id]).exists()
+        )
+
+    def test_bulk_delete_is_atomic_when_selection_includes_actor(self):
+        with self.assertRaises(BadRequestError):
+            self.model_admin.delete_queryset(self.request, User.objects.all())
+        self.assertEqual(User.objects.count(), self.initial_user_count)
+
+    def test_admin_bulk_deactivate_and_change_form_cannot_disable_self(self):
+        with self.assertRaises(BadRequestError):
+            self.model_admin.deactivate_users(self.request, User.objects.all())
+        self.assertTrue(User.objects.get(pk=self.user.pk).is_active)
+        edited_actor = User.objects.get(pk=self.actor.pk)
+        edited_actor.is_staff = False
+        with self.assertRaises(BadRequestError):
+            self.model_admin.save_model(self.request, edited_actor, None, True)
+        self.assertTrue(User.objects.get(pk=self.actor.pk).is_staff)
+
+    def test_admin_rechecks_actor_before_saving_or_bulk_deactivating(self):
+        User.objects.filter(pk=self.actor.pk).update(is_active=False)
+        with self.assertRaises(PermissionDeniedError):
+            self.model_admin.save_model(self.request, self.user, None, True)
+        with self.assertRaises(PermissionDeniedError):
+            self.model_admin.deactivate_users(
+                self.request, User.objects.filter(pk=self.user.pk)
+            )
+
+    def test_bulk_deactivation_preserves_other_administrator(self):
+        with patch.object(self.model_admin, "message_user"):
+            self.model_admin.deactivate_users(
+                self.request, User.objects.filter(pk=self.user.pk)
+            )
+        self.assertFalse(User.objects.get(pk=self.user.pk).is_active)
+        self.assertTrue(User.objects.get(pk=self.actor.pk).is_active)
+
+    def test_last_active_administrator_guard_excludes_inactive_staff(self):
+        User.objects.exclude(pk=self.actor.pk).update(is_active=False)
+        inactive = UserFactory.create_user(is_staff=True, is_active=False)
+        with lock_managed_users(
+            actor_id=self.actor.pk, user_ids=[inactive.pk]
+        ), self.assertRaises(ConflictError) as error:
+            ensure_admins_remain(removed_ids=[self.actor.pk])
+        self.assertEqual(error.exception.code, "last_active_admin")
+
+    def test_admin_selected_delete_failure_does_not_publish_false_audit_entries(self):
+        self.client.force_login(self.actor)
+        response = self.client.post(
+            reverse("admin:user_user_changelist"),
+            {
+                "action": "delete_selected",
+                "post": "yes",
+                "_selected_action": [str(self.actor.pk), str(self.user.pk)],
+            },
+        )
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(User.objects.count(), self.initial_user_count)
+        self.assertEqual(LogEntry.objects.count(), 0)
+
+    def test_admin_single_delete_error_is_presented_without_server_error(self):
+        self.client.force_login(self.actor)
+        response = self.client.post(
+            reverse("admin:user_user_delete", args=[self.actor.pk]), {"post": "yes"}
+        )
+        self.assertEqual(response.status_code, 302)
+        self.assertTrue(User.objects.filter(pk=self.actor.pk).exists())
+        self.assertEqual(LogEntry.objects.count(), 0)
+
+    def test_separate_admin_password_route_rechecks_actor(self):
+        old_hash = self.user.password
+        User.objects.filter(pk=self.actor.pk).update(is_staff=False)
+        request = RequestFactory().post(
+            "/admin/user/user/password/",
+            {
+                "password1": "changed-test-password",
+                "password2": "changed-test-password",
+            },
+        )
+        request.user = self.actor
+        with patch.object(self.model_admin, "message_user") as message:
+            response = self.model_admin.user_change_password(request, str(self.user.pk))
+        self.assertEqual(response.status_code, 302)
+        message.assert_called_once()
+        self.user.refresh_from_db()
+        self.assertEqual(self.user.password, old_hash)

tests/apps/user/test_management_concurrency.py

@@ -0,0 +1,159 @@
+"""Use real row locks to exercise concurrent administrative mutations."""
+
+from concurrent.futures import ThreadPoolExecutor
+from threading import Event
+
+from apps.core.exceptions import PermissionDeniedError
+from apps.user.admin import UserAdmin
+from apps.user.models import User
+from apps.user.services import UserService
+from django.contrib.admin import AdminSite
+from django.db import close_old_connections, connection, transaction
+from django.test import RequestFactory, TransactionTestCase, skipUnlessDBFeature
+from django.urls import reverse
+from rest_framework.test import APIClient
+
+from tests.apps.user.factories import UserFactory
+
+
+@skipUnlessDBFeature("has_select_for_update")
+class UserManagementConcurrencyTest(TransactionTestCase):
+    """A request authenticated before deletion must lose mutation privileges."""
+
+    def _assert_deleted_actor_cannot_mutate(self, operation: str) -> None:
+        actor = UserFactory.create_superuser()
+        removed_actor = UserFactory.create_superuser()
+        actor_id, removed_id = actor.pk, removed_actor.pk
+        first_deleted = Event()
+        second_lock_query = Event()
+        release_first = Event()
+
+        def delete_other_actor():
+            close_old_connections()
+            try:
+                with transaction.atomic():
+                    UserService.delete_user(removed_id, actor_id=actor_id)
+                    first_deleted.set()
+                    if not release_first.wait(10):
+                        raise AssertionError("Concurrent test did not release deletion")
+            finally:
+                connection.close()
+
+        def observe_lock(execute, sql, params, many, context):
+            if "FOR UPDATE" in sql:
+                second_lock_query.set()
+            return execute(sql, params, many, context)
+
+        def stale_request():
+            close_old_connections()
+            try:
+                with connection.execute_wrapper(observe_lock):
+                    if operation == "admin-deactivate":
+                        request = RequestFactory().post("/admin/user/user/")
+                        request.user = removed_actor
+                        model_admin = UserAdmin(User, AdminSite())
+                        try:
+                            model_admin.deactivate_users(
+                                request, User.objects.filter(pk=actor_id)
+                            )
+                        except PermissionDeniedError:
+                            return 403
+                        return 200
+                    client = APIClient()
+                    client.force_authenticate(removed_actor)
+                    if operation == "self-password":
+                        return client.post(
+                            reverse("api_v1:user:password_change"),
+                            {
+                                "old_password": "testpass123",
+                                "new_password": "changed-test-password",
+                                "new_password_confirm": "changed-test-password",
+                            },
+                            format="json",
+                        ).status_code
+                    if operation == "self-update":
+                        return client.patch(
+                            reverse("api_v1:user:user_update"),
+                            {
+                                "username": "replacement-admin",
+                            },
+                            format="json",
+                        ).status_code
+                    if operation == "create-admin":
+                        return client.post(
+                            reverse("api_v1:user:admin_users"),
+                            {
+                                "username": "replacement-admin",
+                                "email": "replacement@example.test",
+                                "password": "integration-test-password",
+                                "role": "admin",
+                                "first_name": "Test",
+                                "last_name": "Admin",
+                            },
+                            format="json",
+                        ).status_code
+                    url = reverse("api_v1:user:admin-user-detail", args=[actor_id])
+                    if operation == "delete":
+                        return client.delete(url).status_code
+                    if operation == "demote":
+                        return client.patch(
+                            url, {"role": "user"}, format="json"
+                        ).status_code
+                    if operation == "patch-inactive":
+                        return client.patch(
+                            url, {"is_active": False}, format="json"
+                        ).status_code
+                    url = reverse("api_v1:user:admin-user-deactivate", args=[actor_id])
+                    return client.post(url).status_code
+            finally:
+                connection.close()
+
+        with ThreadPoolExecutor(max_workers=2) as pool:
+            first = pool.submit(delete_other_actor)
+            try:
+                self.assertTrue(
+                    first_deleted.wait(10),
+                    "First deletion did not reach its transaction barrier",
+                )
+                second = pool.submit(stale_request)
+                self.assertTrue(
+                    second_lock_query.wait(10),
+                    "Second request did not try to acquire row locks",
+                )
+                self.assertFalse(second.done())
+            finally:
+                release_first.set()
+            first.result(timeout=10)
+            expected_status = (
+                404 if operation in {"self-password", "self-update"} else 403
+            )
+            self.assertEqual(second.result(timeout=10), expected_status)
+        self.assertFalse(User.objects.filter(pk=removed_id).exists())
+        self.assertTrue(
+            User.objects.filter(pk=actor_id, is_active=True, is_staff=True).exists()
+        )
+        self.assertFalse(User.objects.filter(username="replacement-admin").exists())
+
+    def test_two_admins_cannot_delete_each_other(self):
+        self._assert_deleted_actor_cannot_mutate("delete")
+
+    def test_deleted_admin_cannot_deactivate_survivor(self):
+        self._assert_deleted_actor_cannot_mutate("deactivate")
+
+    def test_deleted_admin_cannot_demote_survivor(self):
+        self._assert_deleted_actor_cannot_mutate("demote")
+
+    def test_deleted_admin_cannot_patch_survivor_inactive(self):
+        self._assert_deleted_actor_cannot_mutate("patch-inactive")
+
+    def test_deleted_admin_cannot_use_django_admin_bulk_deactivation(self):
+        self._assert_deleted_actor_cannot_mutate("admin-deactivate")
+
+    def test_deleted_admin_cannot_create_a_replacement_admin(self):
+        self._assert_deleted_actor_cannot_mutate("create-admin")
+
+    def test_in_flight_self_password_change_cannot_recreate_deleted_user(self):
+        self._assert_deleted_actor_cannot_mutate("self-password")
+
+    def test_in_flight_self_update_cannot_recreate_deleted_user(self):
+        self._assert_deleted_actor_cannot_mutate("self-update")

tests/apps/user/test_services.py

@@ -109,7 +109,8 @@ class UserServiceTest(TestCase):
     def test_delete_user_success(self):
         """Test successful user deletion"""
         user_id = self.user.id
-        UserService.delete_user(user_id)
+        admin = UserFactory.create_superuser()
+        UserService.delete_user(user_id, actor_id=admin.pk)
 
         # Verify user is deleted
         with self.assertRaises(NotFoundError):
@@ -118,8 +119,9 @@ class UserServiceTest(TestCase):
     def test_delete_user_not_found(self):
         """Test deleting non-existing user raises NotFoundError"""
         nonexistent_id = fake.pyint(min_value=900000, max_value=999999)
+        admin = UserFactory.create_superuser()
         with self.assertRaises(NotFoundError):
-            UserService.delete_user(nonexistent_id)
+            UserService.delete_user(nonexistent_id, actor_id=admin.pk)
 
     def test_get_tokens_for_user(self):
         """Test JWT token generation"""