fix(login): route every single-method account through the method chooser - #114
Open
yahyafakhroji wants to merge 1 commit into
Open
fix(login): route every single-method account through the method chooser#114yahyafakhroji wants to merge 1 commit into
yahyafakhroji wants to merge 1 commit into
Conversation
The identifier step picked a per-method screen from a static map, so an account
whose only method was a linked IdP was sent to /sso — a session-gated management
page that cannot start an intent. A Google-only user landed on a dead end instead
of at Google.
Route every account with at least one usable method to /login/method instead, which
renders exactly what that account has and auto-starts what needs no form. The
decision is pure over method KINDS, so it has no idpId and could only ever name a
static path; resolving the actual provider has to happen where the links are known.
- A sole linked IdP starts its intent in the LOADER, so the chooser never flashes.
Being in a loader is also what makes it dangerous: a loader re-runs when the user
presses Back at the provider, which would mint a new intent and bounce them
forward forever. A one-shot marker cookie (signed, httpOnly, 10 min) makes the
second arrival render the chooser instead, whose button restarts the ceremony
deliberately as a POST. It is retired on each fresh identifier submit so the next
sign-in auto-starts again.
- A sole passkey begins its ceremony on mount. The auto-begun attempt has no
transient user activation behind it, which WebKit requires, so its failure copy
is suppressed for that attempt only rather than accusing a user who has done
nothing but arrive.
- Sole-password accounts take one extra click. Deliberate: collapsing every branch
onto one destination is what keeps the unknown-identifier path indistinguishable.
The loader is state-changing and its outcomes are distinguishable from outside, so it
is gated on a live ceremony session for that loginName and throttled by two tiers — a
tight ip|loginName budget bounding intents minted against one address, and a loose ip
ceiling bounding enumeration breadth. The identifier POST that hands out that session
is throttled too; it was the one unthrottled member of the auth surface.
Anti-enumeration holds across the move. `ignoreUnknownUsernames` routes an unknown
identifier through the same decision a password-only account gets, and the chooser
serves it the same screen — same status, target, payload, cookies and audit line. Its
policy org is resolved from the identifier's domain, so an unknown address is judged
by the org that claims it rather than by the default org, whose policy can differ.
`?policyOrg=` is now HMAC-authenticated. It rides an IdP return URL, so it is fully
attacker-writable, and it is the one param there coupled to nothing else: it feeds the
callback's allowRegister gate and auto-create org alone, while the same-email lookup
stays instance-wide. A hand-written value would borrow a registration-open org's policy
across that gap. Length validation cannot help, since decoupling those two orgs is the
param's whole purpose — only provenance can.
Also fixes the 429 page's back-link, which hardcoded /id instead of deriving it from
APP_BASENAME and would 404 the user at the moment they are trying to recover.
Contributor
🧪 Test Summary
|
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
1 participant
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.
Problem
The identifier step picked a per-method screen from a static map, so an account whose only sign-in method was a linked IdP was sent to
/sso— a session-gated management page that cannot start an intent. A Google-only user landed on a dead end instead of at Google.Change
Every account with at least one usable method now routes to
/login/method, which renders exactly what that account has and auto-starts what needs no form: