fix: a password reset request answers the same either way - #868
Merged
blaipr merged 2 commits intoAug 23, 2026
Conversation
The forgot-my-password endpoint needs no session, so whatever it distinguishes it distinguishes for anybody. It answered four different things: User not found the login does not exist Wrong data it does, but that is not the address on file Unable to reset the password it does, address matches, account disabled or LDAP Request sent it worked That is an oracle over the whole user table, offered unauthenticated: whether a login exists, which address belongs to it — by trying addresses against a login that answers "Wrong data" rather than "User not found" — and whether the account is usable. Tracking rate-limits the guessing; it does not make the answers the same. Half of this had already been closed. The disabled and LDAP refusals were deliberately collapsed into one message, and the test pinning them says why: "the reply does not say which of the two applied, so an unauthenticated caller learns nothing about the account from asking". Collapsing two of the four still left the first question answerable. This finishes it — every outcome now answers "Request sent". The sibling settles the shape rather than it being invented here: Login answers "Wrong login" for an unknown user and a wrong password alike. The rate limit stays distinguishable on purpose. "Attempts exceeded" is about the caller's own behaviour, reveals nothing about any account, and hiding it would leave somebody who had locked themselves out with no way to find out why. What is lost is the message telling an honest user they mistyped their address, and the one telling a disabled user to contact an administrator. Both are recoverable; an enumerable user list is not. The real outcome is still recorded — the exception event still fires and the tracking entry is still added — so an administrator can see what happened and a stranger cannot. Two tests were added beyond updating the existing three: a login that does not exist, which is the half that was still open, and one that the request which succeeds still sends the mail. That second one matters because a response saying "Request sent" whatever happened is also what an endpoint that had quietly stopped sending anything would produce, and the failure path here is deliberately swallowed — so it asserts the mail, at the address on the account, rather than the body. Checked by putting the distinguishable answer back: all four refusal tests fail, and the two success ones do not.
The refusal test from #857 caught this: moving the tracking check into its own try block left the "attempts exceeded" path returning without calling addTracking(), so an attempt made while already blocked was no longer recorded. checkTracking() throws once the limit has been reached, and recording the attempt anyway is what makes further hammering extend the block rather than sit out a window that has stopped growing. It is counted again. The unit test that pinned the old behaviour of the failure path is updated rather than kept: it asserted the exception's message came back, which is the thing this change removes.
blaipr
deleted the
fix/a-password-reset-request-answers-the-same-either-way
branch
August 23, 2026 23:19
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
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.
The forgot-my-password endpoint needs no session, so whatever it distinguishes it
distinguishes for anybody. It answered four different things:
User not found the login does not exist
Wrong data it does, but that is not the address on file
Unable to reset the password it does, address matches, account disabled or LDAP
Request sent it worked
That is an oracle over the whole user table, offered unauthenticated: whether a login
exists, which address belongs to it — by trying addresses against a login that answers
"Wrong data" rather than "User not found" — and whether the account is usable. Tracking
rate-limits the guessing; it does not make the answers the same.
Half of this had already been closed. The disabled and LDAP refusals were deliberately
collapsed into one message, and the test pinning them says why: "the reply does not say
which of the two applied, so an unauthenticated caller learns nothing about the account
from asking". Collapsing two of the four still left the first question answerable. This
finishes it — every outcome now answers "Request sent".
The sibling settles the shape rather than it being invented here: Login answers "Wrong
login" for an unknown user and a wrong password alike.
The rate limit stays distinguishable on purpose. "Attempts exceeded" is about the
caller's own behaviour, reveals nothing about any account, and hiding it would leave
somebody who had locked themselves out with no way to find out why.
What is lost is the message telling an honest user they mistyped their address, and the
one telling a disabled user to contact an administrator. Both are recoverable; an
enumerable user list is not. The real outcome is still recorded — the exception event
still fires and the tracking entry is still added — so an administrator can see what
happened and a stranger cannot.
Two tests were added beyond updating the existing three: a login that does not exist,
which is the half that was still open, and one that the request which succeeds still
sends the mail. That second one matters because a response saying "Request sent"
whatever happened is also what an endpoint that had quietly stopped sending anything
would produce, and the failure path here is deliberately swallowed — so it asserts the
mail, at the address on the account, rather than the body.
Checked by putting the distinguishable answer back: all four refusal tests fail, and the
two success ones do not.
One correction, caught by the refusal test added in #857 rather than by me: moving the
tracking check into its own try block first left the "attempts exceeded" path returning
without calling addTracking(), so an attempt made while already blocked stopped being
recorded — which would have let hammering sit out a window that had stopped growing
instead of extending it. It is counted again, and the test that noticed is why.