feat: self-service account erasure / consent withdrawal (#585)

[Joe-Heffer-Shef]

Summary

Closes #585. UK GDPR Art. 7(3) requires withdrawing consent to be as easy as giving it, but the only way to erase an account was previously a staff-only console action — there was no self-service route at all.

• Adds a "Delete my account" action to the profile page (/profile/delete/). In the common case this anonymises the account immediately (reusing the existing UserService.anonymise erasure logic from the staff console) and logs the user out.
• Handles the one real edge case where immediate erasure would harm others: if the requester is the sole admin of an organisation with other members, erasing them immediately would strand that org. Instead, a pending ErasureRequest is created, staff are emailed, and the user is told their request will be completed within 30 days. Staff complete it via the existing "Delete user" console action — no new completion UI was needed.
• Fixes a pre-existing gap: staff-initiated erasure via the console never recorded a DataProtectionEvent, even though EventType.ERASURE exists for exactly this. Both erasure paths now log a consistent audit event (requested_by/actioned_by).
• Adds a "Pending erasure requests" count/link on the staff console dashboard and a dedicated list page, plus a badge on the user detail page.

Test plan

• python manage.py test home/tests --parallel=auto --failfast — 164 tests pass
• make check — Django checks + migration check pass
• make lint — flake8 clean
• New home/tests/test_account_deletion.py covers: immediate self-erasure, deferral when sole-admin-with-other-members, non-blocking when sole admin of a solo org, non-blocking when a co-admin exists, anonymous redirect
• Extended test_console_views.py to assert the audit event is now recorded and that completing a staff deletion closes out any pending ErasureRequest
• Manual: log in as a non-admin user → Profile → Delete my account → confirm → verify logged out; as a sole admin of an org with another member → verify pending message + staff email (console backend in dev) + that the console "Delete user" flow still clears the pending request

🤖 Generated with Claude Code

[Joe-Heffer-Shef]

Review: PR #699 — Self-service account erasure / consent withdrawal

Overview

Adds a "Delete my account" self-service flow (Art. 7(3)/17(1)(b) UK GDPR), backed by the existing UserService.anonymise erasure logic. Handles the sole-admin-with-other-members edge case via a new ErasureRequest model, staff email notification, and a console list page. Also backfills a DataProtectionEvent audit record for both the new self-service path and the pre-existing staff console deletion. Well-scoped, reuses existing plumbing (console "Delete user" flow completes deferred requests), and comes with solid test coverage for the branching logic.

🔴 Correctness bug: erasure audit event pseudonymises the anonymised email, not the original

In home/services/user.py, anonymise():

user.email = f"deleted-{uuid.uuid4().hex}@{DELETED_ACCOUNT_EMAIL_DOMAIN}"
...
user.save()
OrganisationMembership.objects.filter(user=user).delete()
data_protection_service.record_event(
    event_type=DataProtectionEvent.EventType.ERASURE,
    subject_user=user,
    ...
)

record_event pseudonymises subject_user.email (data_protection.py) to build subject_identifier, the only durable way to correlate this event with other DataProtectionEvent rows for the same person (per that file's own docstring). But by the time record_event runs, user.email has already been overwritten with a random per-call UUID placeholder. The resulting subject_identifier is therefore a hash of a throwaway string, unique to this single call — it can never match the pseudonym recorded on any prior event (export, consent withdrawal, restriction, etc.) for the same real person.

This defeats the stated purpose of this PR's own fix ("staff-initiated erasure ... never recorded a DataProtectionEvent... Both erasure paths now log a consistent audit event") — the record now exists, but it's uncorrelatable with the rest of that person's accountability trail.

Fix: capture subject_email = user.email before mutating it, and pass that through (or reorder so record_event is called before the email is overwritten, using the pre-mutation user object).

🟠 Self-service erasure has no staff/superuser guard

The console's ConsoleDeleteUserView._check_safe explicitly blocks deleting staff/superuser accounts:

if target_user == request.user or target_user.is_staff or target_user.is_superuser:
    raise PermissionDenied

But AccountDeletionView (self-service) only requires LoginRequiredMixin — a staff or superuser can erase their own account via /profile/delete/ with no equivalent check. If that's intentional (staff should be able to withdraw their own consent too), fine — but worth confirming, since it's an inconsistency with the console path's stated safety intent, and could let the last superuser erase themselves with no warning about admin lockout.

🟡 Minor issues

• Duplicate staff notification emails: request_self_erasure calls notify_staff_of_pending_erasure(user) unconditionally, even when ErasureRequest.objects.get_or_create(...) returns an already-existing pending request. A user who repeatedly submits the confirm form (double-click, retry) triggers a fresh staff email each time with no de-duplication/rate-limit.
• Unhandled mail failure: send_mail(..., fail_silently=False) in notify_staff_of_pending_erasure will raise on any SMTP/config error, surfacing as a 500 to the end user after the ErasureRequest row has already been created — the user sees an error but the request is actually recorded. Consider fail_silently=True (or catching/logging) so a transient mail failure doesn't turn into a broken UX for a GDPR-sensitive flow.
• requested_by=target_user on staff-initiated deletion (console.py ConsoleDeleteUserView.post): when staff delete a user who has no pending self-service request (e.g. an arbitrary admin action), the audit event still records requested_by=target_user, implying the user asked for it. That's accurate for completing a pending ErasureRequest, but potentially misleading for other admin-initiated deletions (spam accounts, etc.) where no request was ever made.
• N+1 in get_sole_admin_orgs_with_other_members (organisation.py): loops over admin_org_ids running two .exists() queries per org. Fine at current scale (few orgs per user), but could be a single query with annotate(admin_count=...)/exclude if this becomes a hot path.
• TOCTOU race: get_sole_admin_orgs_with_other_members(...).exists() check and the subsequent anonymise()/ErasureRequest creation aren't wrapped in a transaction. A concurrent membership change between the check and the erasure could theoretically strand an org (low real-world likelihood, but worth a transaction.atomic() given this is a GDPR-compliance-critical path).

Style / conventions

• Import ordering in home/views/__init__.py: ConsolePendingErasureRequestsView inserted out of alphabetical order relative to ConsoleSurveyDetailView/ConsoleUserListView. Worth a quick flake8/isort check if the project enforces import sorting.
• notify_staff_of_pending_erasure and AccountDeletionView docstrings are clear and appropriately reference the originating service method — good practice, consistent with the rest of the codebase's commenting style.
• Local imports inside test methods (from home.models import ErasureRequest inside test_delete_user_completes_pending_erasure_request) — minor inconsistency vs. the top-of-file import in test_account_deletion.py; harmless but could be hoisted.

Test coverage

Good coverage of the branching logic (immediate erasure, solo-org non-blocking, sole-admin-with-others deferral, co-admin non-blocking, anonymous redirect) and of the console-completion path clearing a pending request. Missing:

• No test asserting the DataProtectionEvent.subject_identifier correlates correctly across events for the same user (would have caught the pseudonymisation bug above).
• No test for staff/superuser self-service deletion (ties to the guard question above).
• No test for the duplicate-notification-email behavior on repeated POSTs.

Security/privacy

• Erasure logic itself (wiping name/email/password, removing memberships, deactivating) looks correct and consistent with the existing staff-console path.
• pseudonymise_identifier use is appropriate in principle — the bug above is in how it's invoked, not the underlying design.
• No CSRF/permission issues in the new views; LoginRequiredMixin and StaffRequiredMixin are applied appropriately (aside from the staff self-erasure gap noted above).

Bottom line: solid feature with good UX and test discipline, but the audit-trail pseudonymisation bug should be fixed before merge since it directly undermines the accountability logging this PR advertises as a fix.

🤖 Generated with Claude Code