feat: add attemptWithReason() for login failure reasons - #37
Merged
Merged
Conversation
attempt() resolves the user and verifies the hash, then collapses both into a boolean. Callers who want to tell someone "no account found with that email" were paying a second lookup to recover what the attempt had already established one line earlier. That workaround is also wrong in a way that is hard to see: a Google or passkey signup has a row, so a findFirst by email reports the account as existing, and the user is told "invalid email or password" for an account where no password can ever succeed. Only the attempt itself can tell the difference. attemptWithReason() reports it as no_password. Two invariants hold the design: - attempt() keeps returning Promise<boolean>. Widening it would turn every `if (!await attempt())` downstream into a permanently false branch, since objects are always truthy, and log everyone in without a type error at the call site. - hash.verify() still runs unconditionally before any branch that can return early, against the cached dummy hash on a user miss. Asking for the reason must not buy back the timing oracle that exists to close. The reason is a fact, not a message. Deciding what reaches a screen stays with the application, so the disclosure policy ships as a guide rather than as configuration.
The reference declared the factory as `() => AuthInstance<TUser>`, which has been wrong since per-request autoTouch landed in 36970fe. A bug report against this package cited the stale signature as evidence that `auth({ autoTouch: true })` was unsupported, and it is supported.
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.
Why
attempt()resolves the user and verifies the hash, then collapses both into a boolean. Callers who want to tell someone "no account found with that email" were paying a second lookup to recover what the attempt had already established one line earlier.That workaround is also wrong in a way that's hard to see: a Google or passkey signup has a row, so
findFirst({ where: { email } })reports the account as existing, and the user is told "invalid email or password" for an account where no password can ever succeed. Only the attempt itself can tell the difference —attemptWithReason()reports it asno_password.API
Two invariants the design rests on
attempt()keeps returningPromise<boolean>. Widening it would turn everyif (!await attempt())downstream into a permanently false branch — objects are always truthy — and log everyone in with no type error at the call site.hash.verify()still runs unconditionally before any branch that can return early, against the cached dummy hash on a user miss. Asking for the reason must not buy back the timing oracle that exists to close. There's a test asserting the miss path still pays for a verify.Disclosure stays with the caller
The reason is a fact, not a message. What reaches a screen is an application decision, so the policy ships as a guide rather than as configuration — including the probe-budget pattern, the "never surface
bad_password" rule, and the point that withholding at login buys little if registration already answers the same question.Contents
types.ts—AttemptFailure,AttemptResult,AuthInstance.attemptWithReasonauth-instance.ts—attempt()body hoisted into a sharedrunAttempt()index.ts— exports both types__tests__/auth.test.ts— 7 tests: one per reason, success shape, dummy-hash-still-runs, agreement withattempt()Second commit is unrelated housekeeping kept separate: the reference declared the factory as
() => AuthInstance<TUser>, stale since per-request autoTouch landed in36970fe. A bug report cited it as evidenceauth({ autoTouch: true })was unsupported.Verification
bun test→ 233 pass, 0 fail. Docs build clean.