Background
users.github_username was originally proposed as a NOT NULL change in #448 (Tier 2 item 1). On audit, that change is unsafe because of an underlying design issue: we authenticate users by their mutable display name (github_username) instead of their immutable numeric ID (users.id, which is already the GitHub ID).
The smell
get_or_create_user_from_github() does:
- Look up the existing owner of the username
- If a different user already has it (because they renamed away on GitHub), call
clear_github_username() to set the old owner's github_username to NULL
- Assign the username to the new owner
This means a real user account can silently lose its display name when another user renames into their old handle. Next time the original user logs in, they can no longer be looked up by username (their row is now github_username = NULL).
Perfect-world fix
Use GitHub's immutable numeric ID for auth lookups. Treat github_username as display-only, refreshed on every login.
Schema changes
- Drop
UniqueConstraint("github_username", name="uq_users_github_username")
- Add
NOT NULL on github_username
- Optionally add a non-unique index on
github_username for admin search
Code changes
get_or_create_user_from_github() looks up by user_repo.get_by_id(github_id) instead of get_by_github_username(...)
- Delete
clear_github_username() repository method and service-level conflict handling
- Update tests in
api/tests/services/test_users_service.py (drop the username-conflict test path)
Migration
- Preflight:
SELECT COUNT(*) FROM users WHERE github_username IS NULL. Backfill or delete those rows.
- Drop the unique constraint.
- Add
NOT NULL.
Acceptance criteria
- Username collision on GitHub rename no longer mutates other user rows.
github_username is NOT NULL in the schema.
- Auth lookup uses
github_id only.
clear_github_username() and its callers are deleted.
Source
Audit finding from #448 (item 1). Replaces that item with a proper-scope fix.
Background
users.github_usernamewas originally proposed as aNOT NULLchange in #448 (Tier 2 item 1). On audit, that change is unsafe because of an underlying design issue: we authenticate users by their mutable display name (github_username) instead of their immutable numeric ID (users.id, which is already the GitHub ID).The smell
get_or_create_user_from_github()does:clear_github_username()to set the old owner'sgithub_usernametoNULLThis means a real user account can silently lose its display name when another user renames into their old handle. Next time the original user logs in, they can no longer be looked up by username (their row is now
github_username = NULL).Perfect-world fix
Use GitHub's immutable numeric ID for auth lookups. Treat
github_usernameas display-only, refreshed on every login.Schema changes
UniqueConstraint("github_username", name="uq_users_github_username")NOT NULLongithub_usernamegithub_usernamefor admin searchCode changes
get_or_create_user_from_github()looks up byuser_repo.get_by_id(github_id)instead ofget_by_github_username(...)clear_github_username()repository method and service-level conflict handlingapi/tests/services/test_users_service.py(drop the username-conflict test path)Migration
SELECT COUNT(*) FROM users WHERE github_username IS NULL. Backfill or delete those rows.NOT NULL.Acceptance criteria
github_usernameisNOT NULLin the schema.github_idonly.clear_github_username()and its callers are deleted.Source
Audit finding from #448 (item 1). Replaces that item with a proper-scope fix.