fix(auth): per-user PasswordHistory uniqueness (login lockout) - #3173
Merged
Conversation
…f login Django upgrades a stale password hash on login (re-hash + save). The PasswordHistory pre_save handler re-recorded the previous hash, but hashed_password is globally unique, so once that hash was already stored the unguarded create() raised IntegrityError and aborted the whole User.save() — the upgraded hash never persisted and every subsequent login repeated the failure (permanent lockout). Swallow the duplicate inside a savepoint so it never breaks the user save. Reproduces ONADATA Sentry 83483831.
Address the code-quality bot's empty-except finding: replace the bare pass with an explicit return + comment. Behavior unchanged.
The global unique constraint on hashed_password was the real defect behind the login lockout: it made one user's history collide with another's, and forced a defensive IntegrityError swallow on the re-hash-on-login path. Re-scope uniqueness to (user, hashed_password) and record via get_or_create so re-recording a hash the user already has is idempotent — no swallow needed, no history lost.
This was referenced Jul 15, 2026
FrankApiyo
requested review from
alepietrobon,
kelvin-muchiri and
ukanga
and removed request for
alepietrobon
July 15, 2026 09:13
FrankApiyo
force-pushed
the
feat/password-history-per-user
branch
from
July 15, 2026 14:22
c4d0db6 to
154fd8f
Compare
Was 0021 on the combined verified-email-change branch, chained after the PendingEmailChange migration; this PR is independent of that one, so depend directly on 0019. If the email-change PR merges first, this renumbers again (or vice versa).
FrankApiyo
force-pushed
the
feat/password-history-per-user
branch
2 times, most recently
from
July 16, 2026 07:13
e524b9b to
db42a01
Compare
FrankApiyo
force-pushed
the
feat/password-history-per-user
branch
from
July 16, 2026 07:35
db42a01 to
416e453
Compare
The previous commit ordered by 'created_at', which does not exist on PasswordHistory and would raise FieldError at validation time. Use the actual 'changed_at' field and add a regression test proving the history limit keeps the most recently changed passwords rather than an arbitrary unordered slice.
kelvin-muchiri
approved these changes
Jul 16, 2026
ukanga
approved these changes
Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes / Features implemented
PasswordHistory's globalunique=Trueonhashed_passwordcould lock users out at login when the pre-save hash upgrade hit a duplicate hash (reproduced from a Sentry trace). Login no longer fails on this bookkeeping write, and uniqueness is re-scoped to per-user.Steps taken to verify this change does what is intended
Side effects of implementing this change
Before submitting this PR for review, please make sure you have:
Closes #3175