Let users and reviewers rename crackmes; users change username & email (#176) - #179
Let users and reviewers rename crackmes; users change username & email (#176)#179xusheng6 wants to merge 2 commits into
Conversation
…176) Usernames and crackme names were used as mutable cross-collection keys, so they could never be changed without orphaning content. This keeps the denormalized copies (they make listings query-cheap) but makes them mutable, cascading every change across all referencing collections. - Rename crackme: crackme_update now accepts `name` and cascades the copies on comment/solution/label_request via cascade_crackme_name. Owner edit form and the reviewer edit tool both expose the name field. - Rename username: user_rename cascades across the 8 username-referencing collections (USERNAME_REFERENCES). References are updated first and the user document last, so a mid-cascade failure can't lock the user out. - Change email: user_change_email updates the user doc, refreshes the copy on a pending deletion request, and drops stale reset tokens. - New /settings page for changing username/email, each gated by current-password and full registration-style uniqueness cross-checks (name/email can't collide with any other account's name or email). Reviewer identities (reviewed_by) are deliberately not cascaded — reviewers authenticate via review/users.json, a separate namespace. Pending crackme/writeup review queues read the author live from the DB, so a rename between submit and approval stays consistent. Known limitation: usernames/crackme names embedded in historical notification text keep their old literal value (functional references all stay correct). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…r devices Sessions are client-side signed cookies, so a username change on one device can't invalidate the stale name cached in another device's cookie. Left as-is, that stale session would post content under a username that no longer exists and lose ownership of its own (renamed) content, and if the freed name were later re-registered the cookie would silently act as the new account. Fix: store the immutable user id in the session at login and re-resolve the user from it in a before_request hook, refreshing the cookie's name in place (or clearing auth if the account is gone). Keying on the id also makes name reuse a non-issue. No extra queries on page views: the lookup replaces the per-request unread-notifications query the context processor already ran, and it resolves by the indexed _id (a point-read) rather than the unindexed name/hexid fields. It adds one indexed point-read only to authenticated requests that render no template (POST-redirects, downloads). Tests: stale-session name refresh, comment-from-stale-session uses the current name (the desync case), and session cleared when the user is gone. Test user fixtures now carry _id/hexid like real (user_create'd) users. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up: stale sessions on other devices (identity desync)Since usernames are now mutable and sessions are client-side signed cookies, a rename on one device can't reach the stale name cached in another device's cookie. Left unaddressed, that stale session would post content under the old (now-nonexistent) username and lose ownership of its own renamed content — and if the freed name were re-registered, the cookie would silently operate as the new account. Fix (latest commit): store the immutable user id in the session at login and re-resolve the user from it in a Query cost: none added on page views. The lookup replaces the per-request unread-notifications query the context processor already ran (context processor now reads the count off the already-loaded user), and it resolves by the indexed New tests cover: stale-session name refresh, a comment posted from a stale session using the current name (the desync case), and the session being cleared when the user no longer exists. 257 passing. |
Two changes on review feedback. The flag is now stored as-is instead of hashed. Reviewers need to read it: confirming a crackme is really solvable, and fixing a flag the author mistyped, both need the actual value, and a hash left a wrong flag undetectable until users started failing on it. The review page prints the flag directly, which retires the check-flag tool the hash needed. Cleartext means the flag must not leave the reviewer tool, so it is passed to that template and nowhere else; a test asserts the crackme page, listings, profiles, search and RSS never render it, and neither the operation log (mirrored to Discord) nor the author's notification quotes it. The reviewer crackme editor now covers everything, rather than difficulty being settable only while approving a submission - a typo there used to be unfixable once the crackme was live. It edits the flag (setting or clearing one turns auto-validation on or off), the official difficulty, the private source archive, and the metadata, labels and binary it already handled; it works on pending crackmes as well as approved ones, and the review page links to it. Clearing a flag leaves existing solves and their points alone - they were earned fairly. Its language/arch/platform option lists now come from app/services/crackme_fields.py like every other form, instead of a hardcoded copy that could drift from the upload form. Renaming a crackme is deliberately still not here: PR #179 delivers that across the whole site, including this page, and duplicating it would just collide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #176.
Problem
Usernames and crackme names were used as mutable cross-collection keys, so neither could be changed without orphaning content.
crackme_updateeven excludedname"to avoid database issues."Approach: cascade-on-rename (not full ID normalization)
The denormalized copies (
comment.crackmename,crackme.author, etc.) exist specifically so listings — latest crackmes, a user's writeups, profiles — need zero extra queries. Rather than normalize to immutable IDs (which would add a batched join to most listings and require a risky session-identity + live-data migration), this keeps the copies but makes them mutable and cascades every change across all referencing collections. No read-path query cost change.What's included
Rename a crackme —
crackme_updateacceptsnameand cascades the copies (comment.crackmename,solution.crackmename,label_request.crackme_name) via the sharedcascade_crackme_name. Both the owner edit form (/crackme/<id>/edit) and the reviewer edit tool (/review/editcrackme) expose the name field, with a per-author collision guard.Rename a username —
user_renamecascades across the 8 username-referencing collections (USERNAME_REFERENCES: crackme/comment/solution/both ratings/notifications/label_request/account_deletion_request), the same authoritative set the delete-user flow uses.Change an email —
user_change_emailupdates the user doc, refreshes the denormalized copy on any pending deletion request, and drops stale reset tokens for the old address.New
/settingspage — change username / email, each gated by current-password confirmation and the full registration-style uniqueness cross-checks (a new name/email can't collide with any other account's name or email). Linked from the profile page.Correctness notes
authorlive from the DB, so a rename between submit and approval stays consistent (covered bytest_pending_writeup_survives_username_change).reviewed_by) are deliberately not cascaded — reviewers authenticate viareview/users.json, a separate namespace from main-site usernames.user_renamecascades references first and flips the user document last, so a mid-cascade DB error leaves the user doc on the old name (session stays valid; a retry converges). Covered bytest_user_rename_failure_leaves_user_on_old_name.Addressed from an adversarial review pass
Fixed: non-atomic rename ordering; a rename notification that showed the old name; a create/edit strip mismatch that could misfire a rename cascade; and a DB-blip during the availability check returning 500 instead of a friendly error.
Known limitation
Usernames / crackme names embedded in historical notification text keep their old literal value (and old
/user/<name>links go stale). All functional references — ownership, delivery, listings, review queues — stay correct; only past log entries show the prior name. Rewriting stored notification HTML was judged too fragile to be worth it, but happy to revisit.Tests
New
tests/test_account_settings.py(rename cascade across every collection, email change, crackme rename via owner + reviewer forms, uniqueness rejection, wrong-password rejection, pending-writeup survival, no-lockout ordering). Updated existing crackme/reviewer edit tests to send the now-requirednamefield. 254 passing.🤖 Generated with Claude Code