Skip to content

Let users and reviewers rename crackmes; users change username & email (#176) - #179

Open
xusheng6 wants to merge 2 commits into
mainfrom
allow-rename-users-and-crackmes
Open

Let users and reviewers rename crackmes; users change username & email (#176)#179
xusheng6 wants to merge 2 commits into
mainfrom
allow-rename-users-and-crackmes

Conversation

@xusheng6

Copy link
Copy Markdown
Contributor

Closes #176.

Problem

Usernames and crackme names were used as mutable cross-collection keys, so neither could be changed without orphaning content. crackme_update even excluded name "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 crackmecrackme_update accepts name and cascades the copies (comment.crackmename, solution.crackmename, label_request.crackme_name) via the shared cascade_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 usernameuser_rename cascades 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 emailuser_change_email updates the user doc, refreshes the denormalized copy on any pending deletion request, and drops stale reset tokens for the old address.

New /settings page — 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

  • Pending writeup + rename is safe. The reviewer queues read author live from the DB, so a rename between submit and approval stays consistent (covered by test_pending_writeup_survives_username_change).
  • Reviewer identities (reviewed_by) are deliberately not cascaded — reviewers authenticate via review/users.json, a separate namespace from main-site usernames.
  • No account lockout on partial failure. user_rename cascades 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 by test_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-required name field. 254 passing.

🤖 Generated with Claude Code

xusheng6 and others added 2 commits July 26, 2026 09:04
…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>
@xusheng6

Copy link
Copy Markdown
Contributor Author

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 before_request hook, refreshing the cookie's name in place (or clearing auth if the account was deleted). Keying on the id makes name-reuse a non-issue too.

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 _id (a point-read) instead of the unindexed name field — so it's actually cheaper per page. It adds a single indexed point-read only to authenticated requests that render no template (POST-redirects, downloads).

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not use user name and crackme names as keys in the databas

1 participant