From ea2885bbaa31d765cd9ac71213d058be45dcdee1 Mon Sep 17 00:00:00 2001 From: Xusheng Date: Sun, 26 Jul 2026 08:06:46 -0400 Subject: [PATCH] Preview deletion impact before approving an account-deletion request Approving a self-service account deletion previously ran the full, irreversible deletion straight from the queue behind only a JS confirm() dialog, showing the admin nothing about what would actually be removed. The manual delete-user tool, by contrast, walks the admin through a detailed preview (accounts, crackmes, cascaded solutions/comments/ratings, notifications) before confirming. Make approval use that same preview: - The "Review & approve deletion" button now POSTs to a first step that resolves the target account and renders the shared deletion-impact breakdown; a second POST (action=confirm_delete) performs the deletion, marks the request approved, and emails the former user. - Extract the preview markup/styles shared by both flows into a reviewer/_deletion_preview.html partial so the manual tool and the approval flow render an identical breakdown. Approval remains admin-only (@admin_required, unchanged); only the reject and list actions stay open to any reviewer. Co-Authored-By: Claude Opus 4.8 (1M context) --- review/routes.py | 24 +++- .../templates/reviewer/_deletion_preview.html | 105 ++++++++++++++++++ .../reviewer/accountdeletionrequests.html | 5 +- .../reviewer/approveaccountdeletion.html | 83 ++++++++++++++ review/templates/reviewer/deleteuser.html | 99 +---------------- tests/test_account_deletion.py | 14 +++ 6 files changed, 228 insertions(+), 102 deletions(-) create mode 100644 review/templates/reviewer/_deletion_preview.html create mode 100644 review/templates/reviewer/approveaccountdeletion.html diff --git a/review/routes.py b/review/routes.py index 7c2d778..eab0686 100644 --- a/review/routes.py +++ b/review/routes.py @@ -1894,9 +1894,16 @@ def accountdeletionrequests(current_user): @reviewer_bp.route('/approveaccountdeletion', methods=['POST']) @admin_required def approveaccountdeletion(current_user): - """Approve an account deletion request and delete the account (admin only).""" + """Preview and approve an account deletion request (admin only). + + Approval is a two-step flow that mirrors the manual delete-user tool: the + first POST resolves the target account and shows a full preview of what + deletion would remove; a second POST (``action=confirm_delete``) performs + the irreversible deletion. + """ validate_csrf_token() req_hexid = request.form.get('uuid') + action = request.form.get('action', 'preview') try: del_request = account_deletion_request_by_hexid(req_hexid) @@ -1924,6 +1931,21 @@ def approveaccountdeletion(current_user): message="Request has no email on file; cannot delete" )) + if action != 'confirm_delete': + # Step 1: show the same deletion impact preview the manual delete-user + # tool shows, so the admin sees exactly what approval will remove. + preview_data, error = preview_user_deletion(delete_email) + if error: + return redirect(url_for('reviewer.accountdeletionrequests', message=error)) + return render_template( + 'reviewer/approveaccountdeletion.html', + user=current_user['username'], + is_admin=current_user['is_admin'], + req=del_request, + preview=preview_data, + ) + + # Step 2: admin confirmed the preview -- perform the irreversible deletion. result = delete_user_account(delete_email, admin_username=current_user['username']) success = "successful" in result.lower() diff --git a/review/templates/reviewer/_deletion_preview.html b/review/templates/reviewer/_deletion_preview.html new file mode 100644 index 0000000..8e4e67c --- /dev/null +++ b/review/templates/reviewer/_deletion_preview.html @@ -0,0 +1,105 @@ +{# Shared account-deletion impact preview. + + Renders the breakdown produced by ``preview_user_deletion`` so both the + manual delete-user tool and the deletion-request approval flow show the + exact same "what will be removed" summary before a reviewer confirms. + Expects a ``preview`` dict in context. #} + + +
+
DELETION PREVIEW for {{ preview.username }} ({{ preview.email }})
+ +

The following items will be permanently deleted:

+ +
+ 1 User Account: {{ preview.username }} ({{ preview.email }}) +
+ +
+ {{ preview.notifications }} Notifications +
+ +
+ {{ preview.solutions }} Solutions posted by user +
+ +
+ {{ preview.user_comments }} Comments by user on other crackmes +
+ +
+ {{ preview.difficulty_ratings }} Difficulty Ratings by user +
+ +
+ {{ preview.quality_ratings }} Quality Ratings by user +
+ +
+ {{ preview.crackmes }} Crackmes by user (with CASCADE deletion): +
+ + {% if preview.crackme_details %} + {% for crackme in preview.crackme_details %} +
+
{{ crackme.name }} ({{ crackme.hexid }})
+
+ -> {{ crackme.solutions }} solutions will be cascade deleted
+ -> {{ crackme.comments }} comments will be cascade deleted
+ -> {{ crackme.difficulty_ratings }} difficulty ratings will be cascade deleted
+ -> {{ crackme.quality_ratings }} quality ratings will be cascade deleted +
+
+ {% endfor %} + {% endif %} + +
+ +
+ TOTAL ITEMS TO BE DELETED: +
    +
  • 1 user account
    • +
  • {{ preview.notifications }} notifications
    • +
  • {{ preview.solutions + preview.total_solutions_on_user_crackmes }} solutions ({{ preview.solutions }} by user + {{ preview.total_solutions_on_user_crackmes }} on user's crackmes)
    • +
  • {{ preview.total_comments }} comments ({{ preview.user_comments }} by user + {{ preview.total_comments - preview.user_comments }} on user's crackmes)
    • +
  • {{ preview.crackmes }} crackmes
    • +
  • Difficulty/quality ratings will be recalculated for affected crackmes
    • +
+
+
diff --git a/review/templates/reviewer/accountdeletionrequests.html b/review/templates/reviewer/accountdeletionrequests.html index c6a3469..9a2915d 100644 --- a/review/templates/reviewer/accountdeletionrequests.html +++ b/review/templates/reviewer/accountdeletionrequests.html @@ -54,11 +54,10 @@

Pending account deletion requests

{% endif %} {% if is_admin %} -
+ - +
{% endif %} diff --git a/review/templates/reviewer/approveaccountdeletion.html b/review/templates/reviewer/approveaccountdeletion.html new file mode 100644 index 0000000..2c573a3 --- /dev/null +++ b/review/templates/reviewer/approveaccountdeletion.html @@ -0,0 +1,83 @@ + + + + + Approve account deletion + + + + + + + +
+
+

Dashboard

+
+
+

Welcome, {{ user }}!{% if is_admin %} (Admin){% endif %}

+
+
+ crackmes.one + Logout +
+
+ +
+
+
+

Approve account deletion request

+
+
+

+ {{ req.requester }} requested deletion of their account on + {{ req.created_at }}. + {% if req.note %}
Reason: {{ req.note }}{% endif %} +

+

WARNING: Approving runs the same irreversible account + deletion as the manual delete-user tool. Review the impact below before + confirming.

+
+
+ + + {% include 'reviewer/_deletion_preview.html' %} + +
+

FINAL CONFIRMATION REQUIRED

+

This action is IRREVERSIBLE! Once you click the button below, all the data listed above will be permanently deleted and the requester will be emailed a confirmation.

+ +
+ + + + +
+ Cancel + +
+
+
+ + +
+ + + diff --git a/review/templates/reviewer/deleteuser.html b/review/templates/reviewer/deleteuser.html index c24187c..5aa74cb 100644 --- a/review/templates/reviewer/deleteuser.html +++ b/review/templates/reviewer/deleteuser.html @@ -7,43 +7,6 @@ - @@ -120,67 +83,7 @@

DANGER: IRREVERSIBLE OPERATION

{% else %} -
-
DELETION PREVIEW for {{ preview.username }} ({{ preview.email }})
- -

The following items will be permanently deleted:

- -
- 1 User Account: {{ preview.username }} ({{ preview.email }}) -
- -
- {{ preview.notifications }} Notifications -
- -
- {{ preview.solutions }} Solutions posted by user -
- -
- {{ preview.user_comments }} Comments by user on other crackmes -
- -
- {{ preview.difficulty_ratings }} Difficulty Ratings by user -
- -
- {{ preview.quality_ratings }} Quality Ratings by user -
- -
- {{ preview.crackmes }} Crackmes by user (with CASCADE deletion): -
- - {% if preview.crackme_details %} - {% for crackme in preview.crackme_details %} -
-
{{ crackme.name }} ({{ crackme.hexid }})
-
- -> {{ crackme.solutions }} solutions will be cascade deleted
- -> {{ crackme.comments }} comments will be cascade deleted
- -> {{ crackme.difficulty_ratings }} difficulty ratings will be cascade deleted
- -> {{ crackme.quality_ratings }} quality ratings will be cascade deleted -
-
- {% endfor %} - {% endif %} - -
- -
- TOTAL ITEMS TO BE DELETED: -
    -
  • 1 user account
    • -
  • {{ preview.notifications }} notifications
    • -
  • {{ preview.solutions + preview.total_solutions_on_user_crackmes }} solutions ({{ preview.solutions }} by user + {{ preview.total_solutions_on_user_crackmes }} on user's crackmes)
    • -
  • {{ preview.total_comments }} comments ({{ preview.user_comments }} by user + {{ preview.total_comments - preview.user_comments }} on user's crackmes)
    • -
  • {{ preview.crackmes }} crackmes
    • -
  • Difficulty/quality ratings will be recalculated for affected crackmes
    • -
-
-
+ {% include 'reviewer/_deletion_preview.html' %}

FINAL CONFIRMATION REQUIRED

diff --git a/tests/test_account_deletion.py b/tests/test_account_deletion.py index 7a6cbe0..aae0c71 100644 --- a/tests/test_account_deletion.py +++ b/tests/test_account_deletion.py @@ -92,8 +92,22 @@ def test_admin_approves_deletion_and_emails_user(app, db, alice, monkeypatch): monkeypatch.setattr(routes, 'log_reviewer_operation', lambda *a, **k: None) client = _admin_client(app) + + # Step 1: the first POST shows a deletion impact preview without deleting. + preview = client.post('/review/approveaccountdeletion', data={ + 'uuid': req['hexid'], 'csrf_token': 'admin-csrf', + }) + + assert preview.status_code == 200 + assert b'DELETION PREVIEW' in preview.data + assert db.user.find_one({'name': 'alice'}) is not None + assert db.account_deletion_request.find_one( + {'hexid': req['hexid']})['status'] == 'pending' + + # Step 2: confirming the preview performs the irreversible deletion. response = client.post('/review/approveaccountdeletion', data={ 'uuid': req['hexid'], 'csrf_token': 'admin-csrf', + 'action': 'confirm_delete', }) assert response.status_code == 302