diff --git a/app/modules/auth/session/idp-autostart.ts b/app/modules/auth/session/idp-autostart.ts new file mode 100644 index 0000000000..354ad0801b --- /dev/null +++ b/app/modules/auth/session/idp-autostart.ts @@ -0,0 +1,64 @@ +import { env } from '@/server/infra/env.server'; +import { createCookie } from 'react-router'; + +/** + * ONE-SHOT marker for /login/method's sole-linked-IdP auto-start: the loginName whose IdP + * intent this browser has ALREADY been sent to the provider for. + * + * The auto-start lives in a LOADER (a server redirect, so the chooser never flashes), and a + * loader re-runs on every arrival at the URL — including the one the browser makes when the + * user presses Back at the provider. Without this marker that Back mints a brand-new Zitadel + * intent and bounces them straight forward again: the user can never return to the app. With + * it, the second arrival for the SAME loginName falls through and renders the chooser, whose + * IdP button re-starts the ceremony deliberately (a POST, where Back does not re-fire). + * + * NEVER an auth signal and never rendered — it only suppresses one automatic redirect. + * Same protection class as the `sessions` / `reauth-intent` cookies (which already store a + * loginName): httpOnly, sameSite lax, scoped to `/id`, signed with SESSION_SECRET. + * SHORT maxAge (10 min, matching reauth-intent): a sign-in completes promptly, and a stale + * marker must not keep suppressing the no-flash fast path for a later, unrelated visit. + */ +export const idpAutostartCookie = createCookie('idp-autostart', { + httpOnly: true, + sameSite: 'lax', + path: '/id', + secure: env.NODE_ENV === 'production', + secrets: [env.SESSION_SECRET], + maxAge: 60 * 10, // 10 minutes +}); + +/** Serialize the marker (the loginName just auto-started) to a Set-Cookie string. */ +export async function serializeIdpAutostart(loginName: string): Promise { + return idpAutostartCookie.serialize(loginName); +} + +/** + * Expire the marker — emitted by the IDENTIFIER submit, which is the moment a NEW sign-in + * ceremony begins. + * + * The marker only ever needs to survive one ceremony: it exists to stop the Back-from-the- + * provider arrival re-minting an intent, and Back does not re-POST the identifier. Left to its + * own 10-minute maxAge it outlived the ceremony that wrote it, so a user who signed in, signed + * out, and signed straight back in got the one-button chooser instead of the auto-start the + * whole feature exists to give them. Clearing it here scopes the one-shot to exactly one + * ceremony without weakening the Back guard at all. + */ +export async function clearIdpAutostart(): Promise { + return idpAutostartCookie.serialize('', { maxAge: 0 }); +} + +/** Read the marked loginName. Returns null when the cookie is absent, invalid, or empty. */ +export async function readIdpAutostart(request: Request): Promise { + const value = await idpAutostartCookie.parse(request.headers.get('cookie')); + return typeof value === 'string' && value.length > 0 ? value : null; +} + +/** + * True when this browser has already been auto-started into an IdP for `loginName`. + * Case-insensitive: the marker is written from the URL's loginName while the arrival that + * reads it may carry a different casing of the same account (IdPs / SAML round-trips and + * hand-typed identifiers both do this) — an exact compare would silently re-arm the trap. + */ +export function idpAutostartMatches(marker: string | null, loginName: string): boolean { + return marker !== null && marker.trim().toLowerCase() === loginName.trim().toLowerCase(); +} diff --git a/app/modules/i18n/locales/en.po b/app/modules/i18n/locales/en.po index 4bc049f5f2..34c67eb47b 100644 --- a/app/modules/i18n/locales/en.po +++ b/app/modules/i18n/locales/en.po @@ -78,7 +78,7 @@ msgstr "Additional verification is required to continue." msgid "Already have an account?" msgstr "Already have an account?" -#: app/routes/login/index.tsx:424 +#: app/routes/login/index.tsx:434 msgid "An account with this email already exists — sign in to continue." msgstr "An account with this email already exists — sign in to continue." @@ -172,7 +172,7 @@ msgstr "Choose a new password" msgid "Choose an account" msgstr "Choose an account" -#: app/routes/login/method.tsx:184 +#: app/routes/login/method.tsx:341 msgid "Choose how to sign in" msgstr "Choose how to sign in" @@ -180,7 +180,7 @@ msgstr "Choose how to sign in" msgid "Choose how you want to verify your identity." msgstr "Choose how you want to verify your identity." -#: app/routes/login/index.tsx:420 +#: app/routes/login/index.tsx:430 msgid "Choose your login method" msgstr "Choose your login method" @@ -211,7 +211,7 @@ msgid "Connected accounts" msgstr "Connected accounts" #: app/routes/device/index.tsx:73 -#: app/routes/login/index.tsx:546 +#: app/routes/login/index.tsx:556 #: app/routes/signup/index.tsx:291 msgid "Continue" msgstr "Continue" @@ -241,7 +241,7 @@ msgstr "Couldn't verify" msgid "Create a new account" msgstr "Create a new account" -#: app/routes/login/index.tsx:591 +#: app/routes/login/index.tsx:601 #: app/routes/signup/password.tsx:237 msgid "Create account" msgstr "Create account" @@ -268,8 +268,8 @@ msgstr "Device code" msgid "Device denied" msgstr "Device denied" -#: app/routes/login/index.tsx:377 -#: app/routes/login/index.tsx:392 +#: app/routes/login/index.tsx:387 +#: app/routes/login/index.tsx:402 #: app/routes/signup/index.tsx:250 #: app/routes/signup/index.tsx:275 msgid "Email" @@ -283,9 +283,9 @@ msgstr "Email code" msgid "Email me a code" msgstr "Email me a code" -#: app/routes/login/index.tsx:557 #: app/routes/login/index.tsx:567 -#: app/routes/login/method.tsx:238 +#: app/routes/login/index.tsx:577 +#: app/routes/login/method.tsx:411 #: app/routes/signup/method.tsx:339 msgid "Email me a sign-in link" msgstr "Email me a sign-in link" @@ -306,7 +306,7 @@ msgstr "Email OTP" msgid "Email sign-in isn't available — use your username." msgstr "Email sign-in isn't available — use your username." -#: app/routes/login/index.tsx:376 +#: app/routes/login/index.tsx:386 msgid "Email, phone, or username" msgstr "Email, phone, or username" @@ -446,13 +446,13 @@ msgstr "No signed-in accounts." msgid "Not now" msgstr "Not now" -#: app/routes/login/index.tsx:589 +#: app/routes/login/index.tsx:599 msgid "Not registered?" msgstr "Not registered?" #: app/components/identity-badge/identity-badge.tsx:30 #: app/routes/device/authorize.tsx:122 -#: app/routes/login/method.tsx:193 +#: app/routes/login/method.tsx:350 #: app/routes/passkeys.tsx:249 #: app/routes/reauth.tsx:280 #: app/routes/signed-in.tsx:49 @@ -472,8 +472,8 @@ msgstr "or" msgid "Or import this URI in your authenticator app" msgstr "Or import this URI in your authenticator app" -#: app/routes/login/index.tsx:487 -#: app/routes/login/method.tsx:221 +#: app/routes/login/index.tsx:497 +#: app/routes/login/method.tsx:394 #: app/routes/reauth.tsx:310 #: app/routes/setup/mfa.tsx:46 msgid "Passkey" @@ -511,7 +511,7 @@ msgstr "Passkeys" msgid "Passkeys let you sign in with your fingerprint, face, or device PIN." msgstr "Passkeys let you sign in with your fingerprint, face, or device PIN." -#: app/routes/login/method.tsx:255 +#: app/routes/login/method.tsx:428 #: app/routes/reauth.tsx:339 #: app/routes/reauth.tsx:377 #: app/routes/signup/password.tsx:229 @@ -543,8 +543,8 @@ msgstr "Password must contain an uppercase letter." msgid "Password sign-in isn't available for this account." msgstr "Password sign-in isn't available for this account." -#: app/routes/login/index.tsx:379 -#: app/routes/login/index.tsx:394 +#: app/routes/login/index.tsx:389 +#: app/routes/login/index.tsx:404 msgid "Phone" msgstr "Phone" @@ -702,7 +702,7 @@ msgstr "Sign out of" msgid "Sign out other sessions" msgstr "Sign out other sessions" -#: app/routes/login/index.tsx:579 +#: app/routes/login/index.tsx:589 msgid "Sign-in is currently unavailable for this account. Please contact your administrator." msgstr "Sign-in is currently unavailable for this account. Please contact your administrator." @@ -715,7 +715,7 @@ msgstr "Signed-in sessions on other devices can still be active. Sign out your o msgid "Signing in as" msgstr "Signing in as" -#: app/routes/login/method.tsx:187 +#: app/routes/login/method.tsx:344 msgid "Signing in as <0>{loginName}." msgstr "Signing in as <0>{loginName}." @@ -871,8 +871,8 @@ msgstr "Use your passkey to verify your identity." msgid "Use your security key to verify your identity." msgstr "Use your security key to verify your identity." -#: app/routes/login/index.tsx:380 -#: app/routes/login/index.tsx:395 +#: app/routes/login/index.tsx:390 +#: app/routes/login/index.tsx:405 #: app/routes/sso/ldap.tsx:76 msgid "Username" msgstr "Username" @@ -943,7 +943,7 @@ msgstr "We've sent a password reset link to <0>{0}" msgid "We've sent a verification link to <0>{0}" msgstr "We've sent a verification link to <0>{0}" -#: app/routes/login/index.tsx:417 +#: app/routes/login/index.tsx:427 msgid "Welcome" msgstr "Welcome" diff --git a/app/resources/login/login-decision.ts b/app/resources/login/login-decision.ts index 66b0fcea4f..18cfaf5b96 100644 --- a/app/resources/login/login-decision.ts +++ b/app/resources/login/login-decision.ts @@ -8,13 +8,6 @@ export type Decision = | { kind: 'redirect'; path: string; params?: Record } | { kind: 'error'; error: string }; -const SINGLE_TARGET: Record = { - passkey: '/login/passkey', - idp: '/sso', - password: '/login/password', - otp_email: '/login/verify/email', -}; - export function decideAfterIdentifier({ methods, settings, @@ -41,9 +34,11 @@ export function decideAfterIdentifier({ if (methods.includes('password') && settings.allowPassword) available.push('password'); if (methods.includes('otp_email') && emailDeliveryEnabled) available.push('otp_email'); - if (available.length >= 2) return { kind: 'redirect', path: '/login/method' }; - - if (available.length === 1) return { kind: 'redirect', path: SINGLE_TARGET[available[0]] }; + // Every account with at least one usable method goes to the chooser, which renders + // exactly the methods it has (and auto-starts the ones that need no form). Routing + // per-method from here is what sent sole-IdP users to a bare /sso page: this function + // is pure over method KINDS, so it has no idpId and could only name a static path. + if (available.length >= 1) return { kind: 'redirect', path: '/login/method' }; // available.length === 0: surface the most actionable error. if (methods.includes('password') && !settings.allowPassword) { diff --git a/app/resources/login/login.service.ts b/app/resources/login/login.service.ts index 4f45d02a97..42f70d0e7e 100644 --- a/app/resources/login/login.service.ts +++ b/app/resources/login/login.service.ts @@ -25,8 +25,13 @@ import type { LoginSettings, ProviderErrorCode } from '@/modules/auth/types'; import { ProviderError } from '@/modules/auth/types'; import { decideAfterIdentifier } from '@/resources/login/login-decision'; import { isEmailLike } from '@/resources/login/login.schema'; +import { + GHOST_METHODS, + emailDomain, + resolveGhostPolicyOrg, +} from '@/resources/login/method-options'; import { nextStepFromSession, threadParams } from '@/resources/shared/next-step-params'; -import { resolveOrg } from '@/resources/shared/resolve-org'; +import { resolveOrg, resolvePolicyOrg } from '@/resources/shared/resolve-org'; import { idpReturnUrls } from '@/resources/sso/idp-return-urls'; import { paths } from '@/routes/paths'; import { logAuthEvent, hashActor } from '@/server/observability'; @@ -69,6 +74,12 @@ export interface StartIdpInput { * is the real guard, this only improves the picker UX. */ reauthHint?: string; + /** + * The POLICY org this intent was decided under, when it differs from `organization`. + * Rides on the success URL so the callback gates `allowRegister` (and auto-creates) in the + * SAME org whose IdP list the caller picked from. See IdpReturnOpts.policyOrg. + */ + policyOrg?: string; /** MaxMind device-fingerprint token captured client-side; attached as session metadata. */ deviceTrackingToken?: string; } @@ -85,12 +96,21 @@ export type StartIdpResult = { ok: true; authUrl: string } | { ok: false; error: */ export async function startIdpIntent( provider: AuthProvider, - { idpId, origin, requestId, organization, reauthHint, deviceTrackingToken }: StartIdpInput + { + idpId, + origin, + requestId, + organization, + reauthHint, + policyOrg, + deviceTrackingToken, + }: StartIdpInput ): Promise { const slug = idpTypeToSlug(idpId) ?? idpId; const { success, failure } = idpReturnUrls(origin, slug, { requestId, organization, + policyOrg, deviceTrackingToken, }); // Guard the provider call: a ProviderError (Zitadel unavailable / IdP intent rejected) is the @@ -183,13 +203,6 @@ export type ResolveIdentifierResult = | ResolveIdentifierStartIdp | { ok: false; error: ResolveIdentifierError }; -/** Lowercased domain of an email-style identifier, or null when it is not an email. */ -function emailDomain(loginName: string): string | null { - const at = loginName.lastIndexOf('@'); - if (at <= 0 || at === loginName.length - 1) return null; - return loginName.slice(at + 1).toLowerCase(); -} - /** * Identifier step: resolve the user, create the ceremony session, decide the next * factor screen, and shape the ceremony-session entry + threaded query params. @@ -247,16 +260,30 @@ export async function resolveIdentifier( } } + // The caller's already-fetched settings describe `organization`, so they are reusable only + // while the resolved org still equals it (domain discovery above may have moved `org`). + // Shared by BOTH branches below: the known path saves an RPC with it, and the unknown path + // must save exactly the same one — an extra provider round-trip on one side only is a timing + // difference between "account exists" and "account does not", which is what this whole flag + // exists to erase. + const reusableSettings = + threadedSettings && org !== undefined && org === organization ? threadedSettings : undefined; + const user = await provider.findUser(loginName, org); if (!user) { // ignoreUnknownUsernames (settings-gated, DEFAULT-OFF): with the flag off this is - // byte-identical to before (USER_NOT_FOUND). With it on, proceed to the password - // step bound to the typed loginName but with NO user attached — the credential check - // then fails generically, identical to a wrong password for a real account. - // Org-first: resolve the org (explicit wins; else default org) before reading the policy that - // gates ignoreUnknownUsernames / disableLoginWithEmail. Reading raw `org` (often undefined - // here) returned INSTANCE settings, so these gates could diverge from the default org's policy. - const settings = await provider.getLoginSettings(await resolveOrg(provider, org)); + // byte-identical to before (USER_NOT_FOUND). With it on, proceed bound to the typed loginName + // but with NO user attached — the credential check then fails generically, identical to a + // wrong password for a real account. + // Resolve the policy org from the identifier's DOMAIN, not the default org — see + // resolveGhostPolicyOrg. Reading the default org here judged `nobody@acme.test` under a + // different policy than `mia@acme.test`, and when the two disagreed on `allowPassword` the + // redirect targets split (known → /login/method, ghost → /error): the exact account-existence + // oracle this flag exists to close. It also gates ignoreUnknownUsernames / + // disableLoginWithEmail, which now answer for the org that actually claims the address. + const settings = + reusableSettings ?? + (await provider.getLoginSettings(await resolveGhostPolicyOrg(provider, org, loginName))); if (settings.ignoreUnknownUsernames !== true) { // disableLoginWithEmail (settings-gated, DEFAULT-OFF): an email is never a valid loginname // under this policy, so the lookup already failed. Refine the generic not-found into a @@ -280,7 +307,28 @@ export async function resolveIdentifier( sessionEntryFromSession(ghostSession, { loginName, organization: org, requestId }) ); const ghostParams = new URLSearchParams(threadParams(loginName, requestId, org)); - return { ok: true, target: '/login/password', params: ghostParams, sessions: ghostSessions }; + // ROUTE THE GHOST THROUGH THE REAL DECISION, never a target of its own. + // + // This branch's ONLY job is to be indistinguishable from a real account, and a hardcoded + // '/login/password' stopped being that the moment every known account with >= 1 usable method + // started resolving to /login/method: known → /login/method, unknown → /login/password turns + // the redirect target into a perfect account-existence oracle and silently disables the very + // setting that put us here. The collision was the protection, so re-establish it structurally — + // ask decideAfterIdentifier the same question a PASSWORD-ONLY account asks (GHOST_METHODS), + // under the same settings, and take the same answer. An org that forbids passwords sends both + // to /error; every other org sends both to /login/method, whose loader serves the ghost the + // identical chooser (see routes/login/method.tsx's ghost-subject note). + const ghostDecision = decideAfterIdentifier({ + methods: [...GHOST_METHODS], + settings, + emailDeliveryEnabled, + context: { role: 'primary' }, + }); + if (ghostDecision.kind === 'redirect') { + Object.entries(ghostDecision.params ?? {}).forEach(([k, v]) => ghostParams.set(k, v)); + return { ok: true, target: ghostDecision.path, params: ghostParams, sessions: ghostSessions }; + } + return { ok: true, target: paths.error(), params: ghostParams, sessions: ghostSessions }; } logAuthEvent('identifier', 'success', { actor: hashActor(loginName) }); @@ -294,13 +342,18 @@ export async function resolveIdentifier( // pinned and domain-discovery didn't run (org === undefined), the user may live in a // different org than the default. Use user.orgId as the authoritative org so the // settings (allowPassword, passkeysType, …) match the user's actual policies. - // The caller's threadedSettings are only reused when both the caller pinned an explicit - // org AND the user's org matches it (org !== undefined && org === organization). - const userOrg = org ?? user.orgId; - const settings = - threadedSettings && org !== undefined && org === organization - ? threadedSettings - : await provider.getLoginSettings(userOrg); + // + // resolvePolicyOrg, NOT a bare `org ?? user.orgId`: `User.orgId` is OPTIONAL, and when it was + // absent this read fell through to INSTANCE settings while /login/method's loader — which + // re-computes the very availability decided here — fell through to the DEFAULT ORG's. Those + // two policies can disagree, and when they do this function approves a method the chooser then + // computes away, bouncing the user to /error. Both sides now share the one helper. + // `reusableSettings` (computed above, shared with the unknown-identifier branch) already + // encodes the only condition under which the caller's threaded settings describe this org: + // an explicit `organization` that survived discovery — in which case resolvePolicyOrg is the + // identity on `org` anyway. + const userOrg = await resolvePolicyOrg(provider, org, user.orgId); + const settings = reusableSettings ?? (await provider.getLoginSettings(userOrg)); const decision = decideAfterIdentifier({ methods, settings, diff --git a/app/resources/login/method-options.ts b/app/resources/login/method-options.ts new file mode 100644 index 0000000000..8773907b27 --- /dev/null +++ b/app/resources/login/method-options.ts @@ -0,0 +1,159 @@ +// app/resources/login/method-options.ts +// +// "What can this identified account actually sign in with RIGHT NOW?" — the resolution behind +// the /login/method chooser, extracted from the route loader so that loader stays a thin +// request/response shell (session gate → reads → decide → render). +// +// Applies the SAME policy gates decideAfterIdentifier (login-decision.ts) applies, plus the one +// thing that function structurally cannot know: whether the account's enrolled 'idp' method +// resolves to any actually-usable linked provider. decideAfterIdentifier is pure over method +// KINDS, so it counts 'idp' from `methods.includes('idp') && settings.allowExternalIdp` alone; +// only this resolution sees that the links may all be missing, deactivated, or LDAP-only. +import type { AuthProvider } from '@/modules/auth/auth-provider'; +import type { AuthMethod, IdProvider, LoginSettings } from '@/modules/auth/types'; +import { resolveOrg } from '@/resources/shared/resolve-org'; +import { getActiveIdPs } from '@/resources/sso/idp-providers'; +import { joinLinkedIdps, type LinkedIdpView } from '@/resources/sso/sso-management'; + +/** The four primary sign-in methods the chooser can offer. Closed set. */ +export type ChooserMethod = 'passkey' | 'password' | 'otp_email' | 'idp'; + +/** + * The enrolment the chooser SYNTHESIZES for a GHOST subject — an identifier that cleared + * /login/method's session gate but resolves to no account. + * + * `ignoreUnknownUsernames` is a tenant-facing anti-enumeration setting: with it on, /login's + * identifier step plants a real ceremony session for an UNKNOWN identifier and routes it to the + * SAME target a known account gets, so the response reveals nothing. That protection is ENTIRELY + * the indistinguishability, so the chooser must serve the ghost the very screen a real + * PASSWORD-ONLY account gets. + * + * Password, and only password, because it is the one method an unknown identifier can plausibly + * own end-to-end: the credential check at /login/password already fails generically for a ghost + * session (that is the whole design of the flag), whereas a passkey or IdP button would be an + * offer the ceremony cannot honour — and failing differently is exactly the leak we are closing. + * + * Fed through {@link resolveMethodOptions} like any other enrolment rather than short-circuiting + * the render, so an org that forbids passwords dead-ends the ghost at the same place it dead-ends + * a real password-only account instead of offering a button policy would refuse. + */ +export const GHOST_METHODS: readonly AuthMethod[] = Object.freeze(['password']); + +/** Lowercased domain of an email-style identifier, or null when it is not an email. */ +export function emailDomain(loginName: string): string | null { + const at = loginName.lastIndexOf('@'); + if (at <= 0 || at === loginName.length - 1) return null; + return loginName.slice(at + 1).toLowerCase(); +} + +/** + * The POLICY org a GHOST subject is judged under — the counterpart to {@link resolvePolicyOrg} + * for a subject that resolves to no account. + * + * WHY THIS EXISTS. A ghost has no `user.orgId`, so `resolvePolicyOrg`'s middle rung has nothing + * to bite on and the read fell through to the DEFAULT org — while a real account at the same + * address was judged by its OWN org. When those two policies disagreed the two branches took + * different routes (known → /login/method, ghost → /error for a default org with + * `allowPassword: false`), which is precisely the account-existence oracle + * `ignoreUnknownUsernames` exists to prevent. Both hops read this — the identifier decision + * (login.service.ts) and the chooser loader (routes/login/method.tsx) — so they cannot drift. + * + * Resolve from the identifier's DOMAIN instead: `nobody@acme.test` is then judged by the very + * org `mia@acme.test` belongs to. + * + * DELIBERATELY NOT GATED on `allowDomainDiscovery`. That flag governs whether a domain may ROUTE + * a ceremony (map an org and skip screens); this is a POLICY READ only — it never changes where + * anyone lands, only which org's settings decide it. Gating it would leave the oracle open for + * every tenant with discovery off, which is the default. + * + * ACCEPTED TRADE: an org whose domain is registered becomes distinguishable from one whose is + * not. That is ORG-existence, not ACCOUNT-existence — it says nothing about whether any + * particular person has an account, which is the property the flag protects. + * + * SIDE BENEFIT: the extra read balances the known branch's `listAuthMethods`, so the two + * branches now spend the same number of provider calls for an email-style identifier — closing + * the coarse timing channel they otherwise had. A NON-EMAIL ghost has no domain to look up and + * still spends one call fewer; that residual is unavoidable without a wasted round-trip. + * + * NOT error-guarded, on purpose: the known branch's `listAuthMethods` is not either, so a + * provider outage must surface identically on both paths rather than degrading one of them. + */ +export async function resolveGhostPolicyOrg( + provider: AuthProvider, + urlOrg: string | undefined, + loginName: string +): Promise { + // An explicit (or already domain-discovered) org wins outright — same precedence + // resolvePolicyOrg gives it, and the caller's threaded settings already describe it. + if (urlOrg !== undefined) return resolveOrg(provider, urlOrg); + const domain = emailDomain(loginName); + const hit = domain ? await provider.findOrgByDomain(domain) : null; + return resolveOrg(provider, hit?.orgId); +} + +export interface MethodOptions { + /** Policy-permitted, actually-usable primary methods, in chooser display order. */ + available: ChooserMethod[]; + /** This user's linked, active, non-LDAP providers — the buttons behind `available: ['idp']`. */ + idps: IdProvider[]; +} + +export interface MethodOptionsInput { + userId: string; + /** + * Enrolled methods (provider.listAuthMethods) — the caller already fetched them, or + * {@link GHOST_METHODS} for a subject that resolves to no account. + */ + methods: readonly AuthMethod[]; + /** Login settings read for the POLICY org (see resolvePolicyOrg). */ + settings: LoginSettings; + /** The POLICY org — the same one `settings` was read for. */ + policyOrg?: string; + /** env.AUTH_EMAIL_DELIVERY_ENABLED, threaded so this stays a pure-input decision. */ + emailDeliveryEnabled: boolean; +} + +/** + * Resolve the chooser's offer for one identified user. The only provider I/O is the linked-IdP + * join, and only when 'idp' is both enrolled and policy-allowed — a password-only account costs + * nothing extra. + */ +export async function resolveMethodOptions( + provider: AuthProvider, + { userId, methods, settings, policyOrg, emailDeliveryEnabled }: MethodOptionsInput +): Promise { + const available: ChooserMethod[] = []; + if (methods.includes('passkey') && settings.passkeysType !== 'not_allowed') { + available.push('passkey'); + } + + // Resolve the user's linked (redirect-based) IdPs directly into IdpButtonList's IdProvider + // shape — mirrors reauth.service.ts's loadReauth idp-resolution, including the LDAP exclusion + // (LDAP needs its own credential form, not an OAuth round-trip). A link whose provider is no + // longer active has no name/type to join — filtered out, since there is no sign-in button to + // offer for a dead provider. + let idps: IdProvider[] = []; + if (methods.includes('idp') && settings.allowExternalIdp) { + const [links, active] = await Promise.all([ + provider.listIdpLinks(userId), + getActiveIdPs(provider, policyOrg), + ]); + idps = joinLinkedIdps(links, active) + .filter( + (l): l is LinkedIdpView & { name: string; type: string } => + l.name !== undefined && l.type !== undefined && l.type !== 'LDAP' + ) + .map((l) => ({ id: l.idpId, name: l.name, type: l.type, logoUrl: l.logoUrl })); + if (idps.length > 0) available.push('idp'); + } + + if (methods.includes('password') && settings.allowPassword) available.push('password'); + if (methods.includes('otp_email') && emailDeliveryEnabled) available.push('otp_email'); + + return { available, idps }; +} + +/** True when the account's ONLY usable method is exactly one linked IdP (the auto-start case). */ +export function isSoleLinkedIdp({ available, idps }: MethodOptions): boolean { + return available.length === 1 && available[0] === 'idp' && idps.length === 1; +} diff --git a/app/resources/shared/resolve-org.ts b/app/resources/shared/resolve-org.ts index a0d2d00801..03f00bafce 100644 --- a/app/resources/shared/resolve-org.ts +++ b/app/resources/shared/resolve-org.ts @@ -63,3 +63,42 @@ export async function resolveOrg( ): Promise { return urlOrg ?? env.ZITADEL_DEFAULT_ORG_ID ?? (await getCachedDefaultOrg(provider)) ?? undefined; } + +/** + * The POLICY org for a POST-IDENTIFIER decision — i.e. every read (`getLoginSettings`, + * `getActiveIdPs`) that decides which sign-in methods an IDENTIFIED user may use. + * + * Precedence: the ceremony's explicit `?organization=` wins, else the org the FOUND USER + * actually belongs to, else {@link resolveOrg}'s default-org fallback. + * + * SHARED ON PURPOSE. `resolveIdentifier` (login.service.ts) decides a user has a usable method + * and routes them to /login/method; that route's loader then RE-computes the same availability. + * If the two resolve a different org they read different policies, and the loader can compute + * `available: []` for a user the decision just approved — a bounce to /error on the product's + * most travelled path. They previously agreed only for users WITH an `orgId`: `User.orgId` is + * optional, and when it was absent the decision fell through to INSTANCE settings while the + * loader fell through to the DEFAULT ORG's. Routing both through this one helper makes the + * agreement structural instead of coincidental. + * + * KNOWN, PRE-EXISTING: `urlOrg` is the raw `?organization=` query param and it WINS here, so + * whoever composes a URL — not only the ceremony that minted it — picks which org's policy + * decides an identified user's available methods. A permissive org named there can re-enable a + * method the user's own org disabled (`allowPassword`, `passkeysType`); a restrictive one can + * collapse their options to a single IdP. That is the trust level `?organization=` has always + * carried (every per-method screen was directly reachable with it long before this chooser + * existed) and the credential checks downstream are enforced by Zitadel, not by these settings — + * but note the chooser LOADER is a STATE-CHANGING consumer of it, since a collapsed option set + * can trigger the sole-IdP auto-start, which the per-method screens never did. Closing it means + * authenticating the param the way sso's `policyOrg` now is (see server/signed-param.ts); + * deliberately out of scope here because it predates and outlives this flow. + * + * A subject that resolves to NO account has no `userOrgId` for the middle rung and must not + * silently fall through to the default org — see `resolveGhostPolicyOrg` (login/method-options). + */ +export async function resolvePolicyOrg( + provider: AuthProvider, + urlOrg: string | undefined, + userOrgId: string | undefined +): Promise { + return resolveOrg(provider, urlOrg ?? userOrgId); +} diff --git a/app/resources/sso/idp-return-urls.ts b/app/resources/sso/idp-return-urls.ts index bd1160749a..6e9f8eb2f4 100644 --- a/app/resources/sso/idp-return-urls.ts +++ b/app/resources/sso/idp-return-urls.ts @@ -14,14 +14,34 @@ // APP_BASENAME is defined once in resources/shared; re-exported here for back-compat // (the sso barrel and this module's URL builder both consume it). import { APP_BASENAME } from '@/resources/shared/app-basename'; +import { signParam } from '@/server/signed-param'; export { APP_BASENAME }; +/** + * HMAC purpose tag for `policyOrg`. Shared with the callback that verifies it — a mismatch here + * is a fail-closed (the callback falls back to `organization`), never an accepted forgery. + */ +export const POLICY_ORG_PURPOSE = 'sso.policyOrg'; + export interface IdpReturnOpts { /** Original OIDC/SAML request to resume after the IdP round-trip (e.g. `oidc_…`). */ requestId?: string; /** Org scope to carry through the ceremony. */ organization?: string; + /** + * The POLICY org this intent was decided under, when it differs from the ceremony + * `organization` (which is the RAW `?organization=` the user arrived with, often absent). + * + * The callback resolves the org it reads `allowRegister` from — and auto-creates into — from + * `organization`, which on a bare flow falls back to the DEFAULT org. A caller that picked + * this IdP out of a list resolved for a DIFFERENT org (e.g. /login/method resolving the found + * user's own org) would otherwise have its start decided under one policy and its landing + * decided under another. Carrying it separately fixes that divergence WITHOUT widening + * `organization`, whose absence is what keeps the callback's `findUser` lookups instance-wide + * (see the note at sso-callback.ts's `callbackOrg`). + */ + policyOrg?: string; /** Account-linking flow (sets `?link=true` for the callback). */ link?: boolean; /** @@ -45,6 +65,22 @@ export function idpReturnUrls( if (opts?.link) query.set('link', 'true'); if (opts?.requestId) query.set('requestId', opts.requestId); if (opts?.organization) query.set('organization', opts.organization); + // Only when it actually differs — an identical value would be pure URL noise, and omitting it + // keeps every existing caller's success URL byte-identical. + // + // SIGNED, because this param is the one thing on the URL that is NOT already coupled to + // something else. `organization` drags the callback's same-email findUser scope, the created + // session's orgId and signInWithIdpIntent along with it, so naming a foreign org there costs an + // attacker the whole flow (an accidental fail-closed). `policyOrg` drags nothing: it feeds + // `callbackOrg` alone, which is what gates `allowRegister` and picks the auto-create org. A + // hand-written `?policyOrg=` would therefore borrow that org's + // registration policy while leaving the same-email lookup instance-wide — with + // ALLOW_IDP_AUTO_LINK on, a path into a victim's account in ANY org. Length validation cannot + // help: decoupling those two orgs is the param's whole purpose, so only provenance can. + if (opts?.policyOrg && opts.policyOrg !== opts.organization) { + query.set('policyOrg', opts.policyOrg); + query.set('policyOrgSig', signParam(POLICY_ORG_PURPOSE, opts.policyOrg)); + } if (opts?.deviceTrackingToken) query.set('deviceTrackingToken', opts.deviceTrackingToken); const qs = query.toString(); // The failure URL MUST carry requestId/organization too — the IdP broker redirects here diff --git a/app/resources/sso/sso-callback.ts b/app/resources/sso/sso-callback.ts index 8e3720b313..faa62f7be6 100644 --- a/app/resources/sso/sso-callback.ts +++ b/app/resources/sso/sso-callback.ts @@ -23,10 +23,12 @@ import { registerAndLinkIdp } from '@/resources/signup'; import { MAXMIND_TRACKING_TOKEN_METADATA_KEY } from '@/resources/signup/signup.service'; import { deriveIdpProfileName } from '@/resources/sso/derive-idp-name'; import { decideIdpCallback } from '@/resources/sso/idp-callback'; +import { POLICY_ORG_PURPOSE } from '@/resources/sso/idp-return-urls'; import { signInWithIdpIntent, requestScopedProviderReads } from '@/resources/sso/idp-session'; import type { SsoOutcome } from '@/resources/sso/sso-outcome'; import { env } from '@/server/infra/env.server'; import { logAuthEvent } from '@/server/observability'; +import { verifyParam } from '@/server/signed-param'; import { getOrCreateFingerprintId, userAgentFromRequest } from '@/server/user-agent'; import { providerErrorCode } from '@/utils/errors/auth-error'; import { z } from 'zod'; @@ -48,6 +50,19 @@ export const CallbackQuery = z.object({ // Attacker-controllable from the callback URL; POSTURE B2 is fail-closed against a wrong org, but // cap the length as defense-in-depth (Zitadel org ids are short numeric strings). organization: z.string().max(64).optional(), + // The POLICY org the START side decided this intent under (idp-return-urls.ts). Feeds + // `callbackOrg` ONLY — never the findUser lookups below, which stay on raw `organization`. + // + // That asymmetry is exactly why it must be AUTHENTICATED and not merely capped. `organization` + // is attacker-controllable too, but naming a foreign org there also narrows the same-email + // lookup and poisons createSession's orgId — it fails closed. `policyOrg` is coupled to none of + // that: it decides `allowRegister` and the auto-create org and nothing else, so an unsigned one + // is a way to borrow a registration-open org's policy while keeping the lookup instance-wide. + // Verified against `policyOrgSig` below; anything that fails falls back to `organization`. + policyOrg: z.string().max(64).optional(), + // Detached HMAC over `policyOrg`, minted by idpReturnUrls with SESSION_SECRET. base64url + // SHA-256 is 43 chars; the cap is slack, not a check — verifyParam is the check. + policyOrgSig: z.string().max(128).optional(), // MaxMind device-fingerprint token captured client-side and threaded through the OAuth // round-trip (see idp-return-urls.ts) so it can be attached to the resulting session's metadata. deviceTrackingToken: z.string().optional(), @@ -94,7 +109,28 @@ export async function processIdpCallback( }; } - const { id, token, link, requestId, organization, deviceTrackingToken } = parsed.data; + const { id, token, link, requestId, organization, policyOrg, policyOrgSig, deviceTrackingToken } = + parsed.data; + + // AUTHENTICATE the policy org before it is allowed to influence anything. An unsigned or + // mis-signed value is treated as absent — the callback falls back to `organization`, i.e. to + // exactly the behavior it had before the param existed. Logged (org ids are not PII) because a + // signature failure here is either tampering or a SESSION_SECRET rotation mid-flight, and both + // are worth seeing. + let verifiedPolicyOrg: string | undefined; + if (policyOrg) { + if (verifyParam(POLICY_ORG_PURPOSE, policyOrg, policyOrgSig)) { + verifiedPolicyOrg = policyOrg; + } else { + // snake_case per the P5+ naming convention, and the same `invalid_signature` reason the + // `sessions` cookie's tamper signal uses — one vocabulary for "we did not mint this". + logAuthEvent('idp_policy_org', 'failure', { + reason: 'invalid_signature', + requestId, + policyOrg, + }); + } + } // Ensure the fingerprintId cookie exists for this browser. The SAME minted id feeds // every createSession userAgent below (no first-session gap); fingerprintCookie is @@ -115,14 +151,22 @@ export async function processIdpCallback( let intent: IdpIntentResult; let entries: Awaited>; let decision: ReturnType; - // Resolve the effective org for this callback ceremony — org-first (URL ?organization=), - // then ZITADEL_DEFAULT_ORG_ID env pin, then the provider's instance Default Organization. + // Resolve the effective org for this callback ceremony — policy-org-first (the org the START + // side decided the intent under, see idp-return-urls.ts), then the URL `?organization=`, then + // the ZITADEL_DEFAULT_ORG_ID env pin, then the provider's instance Default Organization. // Hoisted before the try block so both the decision try and the auto-create switch case // can reference it. This resolved org feeds the allowRegister gate (getLoginSettings) and // the auto-create register call so a bare (no ?organization=) flow always has a concrete // org, preventing Zitadel's FAILED_PRECONDITION on addHumanUser with no org. - // NOTE: findUser calls below deliberately stay on raw `organization` (instance-wide lookup). - const callbackOrg = await resolveOrg(provider, organization); + // + // `policyOrg` participates HERE and nowhere else. Without it, a start that resolved its IdP + // list under the found user's own org (e.g. /login/method's sole-linked-IdP auto-start) landed + // here with `organization` still undefined and had allowRegister decided by the DEFAULT org — + // a different policy from the one that offered the provider in the first place. + // NOTE: findUser calls below deliberately stay on raw `organization` (instance-wide lookup), + // which is exactly why the policy org rides in its OWN param instead of widening this one — + // and why only the SIGNATURE-VERIFIED value may be used here. + const callbackOrg = await resolveOrg(provider, verifiedPolicyOrg ?? organization); try { intent = await doRetrieveIdpIntent(id, token); diff --git a/app/routes/login/index.tsx b/app/routes/login/index.tsx index 6760436b2a..14590ab742 100644 --- a/app/routes/login/index.tsx +++ b/app/routes/login/index.tsx @@ -15,6 +15,7 @@ import { idpTypeToSlug } from '@/modules/auth/idp-slug'; // The locked plan block incorrectly listed them as coming from @/modules/auth/session/session // (that module only has pure helpers, no cookie I/O). import { readSessions, serializeSessions } from '@/modules/auth/session/cookie'; +import { clearIdpAutostart } from '@/modules/auth/session/idp-autostart'; import { readLastUsedLogin } from '@/modules/auth/session/last-used-login'; import { readPasskeyHint, clearPasskeyHint } from '@/modules/auth/session/passkey-hint'; import { readReauthIntent } from '@/modules/auth/session/reauth-intent'; @@ -253,6 +254,13 @@ export async function action({ request }: ActionFunctionArgs) { const headers = new Headers(); headers.append('set-cookie', await serializeSessions(result.sessions)); if (fpCookie) headers.append('set-cookie', fpCookie); + // A fresh ceremony starts here, so retire any one-shot IdP auto-start marker the previous one + // left behind — otherwise a sign-out/sign-in inside its 10-minute window suppressed the very + // auto-start the marker exists to protect. Unconditional, and identical on both the known and + // ghost branches: expiring an already-absent cookie is a no-op for the browser, and branching + // on its presence would both cost a read and make the two branches' headers differ. See + // clearIdpAutostart. + headers.append('set-cookie', await clearIdpAutostart()); // Domain-discovery single-IdP org: start the IdP intent directly. A plain redirect to /sso // (the old behavior) dead-ends session-less users in a /login ↔ /sso bounce loop. if ('startIdp' in result) { @@ -266,10 +274,12 @@ export async function action({ request }: ActionFunctionArgs) { if (!idpResult.ok) return data({ error: idpResult.error }, { status: 502 }); return redirect(idpResult.authUrl, { headers }); } - // Sole-passkey: redirect to /login/passkey like every other post-identifier path - // (password, OTP) — - // /login renders the chooser and nothing else; no route inlines an identity-bound, - // single-method screen except /login/method and /reauth, which are out of scope here. + // Every account with >= 1 usable method now resolves to the same target — /login/method — + // which renders exactly what that account has and auto-starts what needs no form (a sole + // linked IdP server-side in its loader, a sole passkey client-side on mount). This route + // deliberately picks no per-method screen: it is pure over method KINDS, so it has no idpId + // and could only ever name a static path (that is what sent sole-IdP users to a bare /sso). + // The zero-method legs (/verify, /error) are unchanged. return redirect(`${result.target}?${result.params}`, { headers }); } diff --git a/app/routes/login/method.tsx b/app/routes/login/method.tsx index ed93291411..710bb9aacd 100644 --- a/app/routes/login/method.tsx +++ b/app/routes/login/method.tsx @@ -5,26 +5,36 @@ import { useAuthActionError } from '@/hooks/use-auth-action-error'; import { useLoginContext } from '@/hooks/use-login-context'; import { usePasskeyLoginCeremony } from '@/hooks/use-passkey-login-ceremony'; import SplitLayout from '@/layouts/split.layout'; -import type { IdProvider } from '@/modules/auth/types'; +import { listSessions, readSessions } from '@/modules/auth/session/cookie'; +import { + idpAutostartMatches, + readIdpAutostart, + serializeIdpAutostart, +} from '@/modules/auth/session/idp-autostart'; import { startIdpIntent } from '@/resources/login'; -import { decideAfterIdentifier } from '@/resources/login/login-decision'; import { loginIdpSchema } from '@/resources/login/login.schema'; +import { + GHOST_METHODS, + isSoleLinkedIdp, + resolveGhostPolicyOrg, + resolveMethodOptions, +} from '@/resources/login/method-options'; import { readCeremonyParams } from '@/resources/shared/ceremony-params'; -import { resolveOrg } from '@/resources/shared/resolve-org'; +import { resolveOrg, resolvePolicyOrg } from '@/resources/shared/resolve-org'; import { joinLinkedIdps } from '@/resources/sso'; import { getActiveIdPs } from '@/resources/sso/idp-providers'; -import type { LinkedIdpView } from '@/resources/sso/sso-management'; import { redirectToLogin } from '@/routes/login-bounce'; import { paths } from '@/routes/paths'; import { providerForRequest } from '@/server/auth-context.server'; import { loaderCsrf, assertCsrf } from '@/server/csrf'; import { trustedAppOrigin } from '@/server/infra/app-origin.server'; import { env } from '@/server/infra/env.server'; -import { Button, LinkButton } from '@datum-cloud/datum-ui/button'; +import { LinkButton } from '@datum-cloud/datum-ui/button'; import { Icon } from '@datum-cloud/datum-ui/icons'; import { cn } from '@datum-cloud/datum-ui/utils'; import { Trans } from '@lingui/react/macro'; import { Key, Lock, Mail } from 'lucide-react'; +import { useEffect, useRef, useState } from 'react'; import { data, redirect, @@ -41,69 +51,164 @@ export const meta: MetaFunction = () => [{ title: 'Choose how to sign in' }]; export async function loader({ request }: LoaderFunctionArgs) { const provider = providerForRequest(request); const { loginName, requestId, organization } = readCeremonyParams(new URL(request.url)); + const bounce = () => redirect(redirectToLogin(requestId, organization)); - if (!loginName) return redirect(redirectToLogin(requestId, organization)); + if (!loginName) return bounce(); + + // ── SESSION GATE ──────────────────────────────────────────────────────────────────────── + // This loader is state-changing (it mints a real Zitadel IdP intent for a sole-linked-IdP + // account and 302s to the provider) and its outcomes are distinguishable from outside: + // 302-to-a-named-provider (sole IdP) vs 200 (several methods) vs 302-to-/verify|/error (none). + // Before the gate an unknown identifier added a fourth, 302-to-/login. Reachable by URL alone + // that made + // `GET /id/login/method?loginName=X` an account-existence AND identity-provider oracle, a + // login-CSRF vector (no token on a GET), and a bypass of ignoreUnknownUsernames — which is + // honoured only in resolveIdentifier (login.service.ts). + // + // Requiring the ceremony session the identifier step ALREADY planted for this loginName costs + // legitimate arrivals nothing: /login's identifier action serializes `result.sessions` into the + // cookie on the same response that redirects here, and every other path to this URL (the e2e + // helper's chooser hop, a Back from a later step, a reload) carries that same cookie. + // + // WHAT THIS GATE IS AND IS NOT. It is NOT an authorization boundary: the cookie it demands is + // SELF-ISSUABLE — any party can POST /id/login (CSRF-protected, so not forceable cross-site, + // but self-driven probing only needs to fetch the token first) and be handed one. So the gate + // raises the cost of reaching this loader by exactly one request; what actually bounds + // enumeration BREADTH is the two-tier GET limiter in server/middleware/rate-limit.ts, now + // backed by an ip ceiling on the identifier POST itself. What the gate genuinely buys is that + // `GET /id/login/method?loginName=X` alone — with no cookie at all — mints nothing and tells + // nobody anything, which is what makes the URL safe to hand out, bookmark and reload. + // + // NOT-KNOWN-EXPIRED, not "live": listSessions is the codebase's expiry-aware filter (the same + // idiom arm-login-passkey.ts uses) but it deliberately KEEPS entries whose expirationTs is + // empty or unparseable (session.ts) — and Zitadel Session-API sessions minted without an + // explicit lifetime, which is exactly what the ceremony createSession produces, carry no + // expirationDate. So a known-expired entry cannot pass, but an unknown-expiry one can. + // Case-insensitive: the URL carries the provider's canonical loginName while a hand-typed or + // IdP-returned identifier may differ in case. + const sessions = listSessions(await readSessions(request), Date.now()); + if (!sessions.some((s) => s.loginName.toLowerCase() === loginName.toLowerCase())) { + return bounce(); + } + // ── GHOST SUBJECT (anti-enumeration) ──────────────────────────────────────────────────── + // A findUser MISS here does NOT mean "no such account, bounce". `ignoreUnknownUsernames` makes + // /login's identifier step plant a real ceremony session for an UNKNOWN identifier and route it + // to THIS url — the same one a real password-only account is routed to — precisely so the two + // are indistinguishable. Bouncing on the miss would hand that difference straight back: a + // 302-to-/login for the unknown vs a 200 chooser for the known is a perfect existence oracle, + // and it would silently disable a security setting the tenant switched on. + // + // So a miss that CLEARED THE SESSION GATE above is served the password-only chooser (see + // GHOST_METHODS): same status, same single control, same branding resolution, same rotated CSRF + // token, same headers. Its Password link leads to /login/password, where the ghost session's + // credential check fails generically — exactly as it did before this route existed. + // + // The gate is what makes this safe to serve at all: the ONLY way to reach it is a live ceremony + // session this browser was handed for this loginName, which /login mints only under the flag. const user = await provider.findUser(loginName, organization); - if (!user) return redirect(redirectToLogin(requestId, organization)); // The method chooser is a branded screen — thread getBranding through so SplitLayout // renders the org logo, mirroring /login and /signup. Fetched in parallel with the rest. // Org-first: an explicit org wins, else the default org (matches the old app's // `organization ?? getDefaultOrg()`). findUser above stays instance-wide by design. - const settingsOrg = await resolveOrg(provider, organization); + const brandingOrg = await resolveOrg(provider, organization); + // POLICY reads (login settings + the org's active IdPs) must resolve to the SAME org + // resolveIdentifier used when it routed the user here. Both sides now call resolvePolicyOrg, + // so that agreement is structural rather than two look-alike expressions kept in sync by hand + // (they previously diverged for a user with NO orgId: instance settings there, default-org + // settings here). Branding is deliberately left org-first — the logo follows the ceremony's + // org, not the user's. Sequenced after brandingOrg so the memoized default-org lookup is + // already warm and this costs no second provider round-trip. + // A GHOST has no orgId for resolvePolicyOrg's middle rung to bite on, so this used to fall + // through to the DEFAULT org while a real account at the same address was judged by its own. + // That split `available` — and with it the render-vs-/error outcome — between the two branches, + // re-opening the existence oracle one hop after resolveIdentifier closed it. Both hops now + // resolve a ghost's policy org from the identifier's domain (see resolveGhostPolicyOrg), so the + // decision that routed the subject here and the one recomputed here read the same org either way. + const settingsOrg = user + ? await resolvePolicyOrg(provider, organization, user.orgId) + : await resolveGhostPolicyOrg(provider, organization, loginName); const [methods, settings, branding, { csrfToken, headers }] = await Promise.all([ - provider.listAuthMethods(user.id), + // A ghost has no id to enumerate methods for; it carries the synthesized password-only + // enrolment instead, and every read below is the one a real account makes. + user ? provider.listAuthMethods(user.id) : Promise.resolve([...GHOST_METHODS]), provider.getLoginSettings(settingsOrg), - provider.getBranding(settingsOrg), + provider.getBranding(brandingOrg), loaderCsrf(request), ]); - // Compute available primary sign-in methods using the same policy gates as - // decideAfterIdentifier (login-decision.ts). This screen only appears when - // available.length >= 2; we surface exactly those available methods. - const available: Array<'passkey' | 'password' | 'otp_email' | 'idp'> = []; - if (methods.includes('passkey') && settings.passkeysType !== 'not_allowed') - available.push('passkey'); + // Which methods this account can actually use right now (same policy gates as + // decideAfterIdentifier, plus the linked-IdP resolution that function cannot see). + // This screen is the destination for EVERY account with >= 1 available method. + const { available, idps } = await resolveMethodOptions(provider, { + // Never read for a ghost: GHOST_METHODS carries no 'idp', so the linked-provider join + // (the only place userId is used) does not run. + userId: user?.id ?? '', + methods, + settings, + policyOrg: settingsOrg, + emailDeliveryEnabled: env.AUTH_EMAIL_DELIVERY_ENABLED, + }); - // Resolve the user's linked (redirect-based) IdPs directly into IdpButtonList's - // IdProvider shape — mirrors reauth.service.ts's loadReauth idp-resolution, including - // the LDAP exclusion (LDAP needs its own credential form, not an OAuth round-trip). A - // link whose provider is no longer active has no name/type to join — filtered out, - // since there's no sign-in button to offer for a dead provider. - let idps: IdProvider[] = []; - if (methods.includes('idp') && settings.allowExternalIdp) { - const [links, active] = await Promise.all([ - provider.listIdpLinks(user.id), - getActiveIdPs(provider, settingsOrg), - ]); - idps = joinLinkedIdps(links, active) - .filter( - (l): l is LinkedIdpView & { name: string; type: string } => - l.name !== undefined && l.type !== undefined && l.type !== 'LDAP' - ) - .map((l) => ({ id: l.idpId, name: l.name, type: l.type, logoUrl: l.logoUrl })); - if (idps.length > 0) available.push('idp'); + // Sole linked IdP: start the intent here rather than rendering a one-button screen. + // Server-side, so the chooser never renders and the user sees no intermediate page. + // + // ONE-SHOT. Being in a LOADER is what buys the no-flash redirect, and it is also what makes + // this dangerous: a loader re-runs on every arrival at the URL, including the one the browser + // makes when the user presses Back at the provider. Unguarded, that Back mints a NEW Zitadel + // intent and bounces them forward again — they can never get back into the app. The marker + // cookie makes the second arrival for the same loginName fall through and RENDER the chooser, + // whose IdP button restarts the ceremony deliberately (a POST to this route's action, where + // Back does not re-fire — which is also why /login's domain-discovery branch, an action, never + // needed a guard like this one). + if (isSoleLinkedIdp({ available, idps })) { + const alreadyAutoStarted = idpAutostartMatches(await readIdpAutostart(request), loginName); + if (!alreadyAutoStarted) { + const idpResult = await startIdpIntent(provider, { + idpId: idps[0].id, + origin: trustedAppOrigin(request), + requestId, + organization, + // The IdP was picked out of a list resolved for settingsOrg; the callback must gate + // allowRegister in that same org rather than in whatever `organization` resolves to. + policyOrg: settingsOrg, + reauthHint: loginName, + }); + // On failure fall through and render the button, so the user keeps a way forward. + // The marker is written only on the redirect that actually mints — a failed start has + // nothing to come Back from. + if (idpResult.ok) { + return redirect(idpResult.authUrl, { + headers: { 'set-cookie': await serializeIdpAutostart(loginName) }, + }); + } + } } - if (methods.includes('password') && settings.allowPassword) available.push('password'); - if (methods.includes('otp_email') && env.AUTH_EMAIL_DELIVERY_ENABLED) available.push('otp_email'); - - // Defensive: if for some reason < 2 methods are available, run decideAfterIdentifier - // and redirect to the single target (or /error). Consume the Decision union by - // `kind` — 'redirect' → its path, 'error' → /error. - if (available.length < 2) { - const decision = decideAfterIdentifier({ - methods, - settings, - emailDeliveryEnabled: env.AUTH_EMAIL_DELIVERY_ENABLED, - context: { role: 'primary' }, // post-identifier decision is the primary flow - }); - const params = new URLSearchParams({ loginName }); - if (requestId) params.set('requestId', requestId); - if (organization) params.set('organization', organization); - const target = decision.kind === 'redirect' ? decision.path : paths.error(); - return redirect(`${target}?${params.toString()}`); + // Nothing usable. Exactly two outcomes are reachable from here, so name them directly + // rather than asking decideAfterIdentifier and filtering its answer: no enrolled methods + // at all is the invite path (/verify); anything else is a policy dead end (/error — the + // PASSWORD_NOT_ALLOWED / NO_SUPPORTED_METHOD legs, which carry no params of their own). + // + // A GHOST reaches this only where a real password-only account would: its synthesized + // enrolment is non-empty, so an org that forbids passwords sends both to /error, and neither + // can reach the /verify leg. (In practice resolveIdentifier already sent both there — this is + // the direct-URL arrival with a still-live session.) + // + // Deciding it here is what makes a self-redirect loop UNREPRESENTABLE. decideAfterIdentifier + // counts 'idp' as available from methods.includes('idp') && settings.allowExternalIdp alone + // — it has no visibility into the resolved `idps` list above, which is the ONLY source of + // truth this route has for actually-usable linked IdPs. A user whose sole enrolled method is + // 'idp' but whose links are all missing, inactive, or LDAP-only lands here (our own + // `available` is []) while that function still names THIS route. Following it would 302 into + // ourselves forever (same user, same inputs, same empty `available` every time); neither + // destination below can name /login/method, so there is nothing left to guard against. + if (available.length === 0) { + // Same query the manual URLSearchParams built (loginName, then requestId, then + // organization; undefined values skipped) — paths.* threads the ceremony for us. + const query = { loginName, requestId, organization }; + return redirect(methods.length === 0 ? paths.verify.index(query) : paths.error(query)); } return data( @@ -136,7 +241,11 @@ export async function action({ request }: ActionFunctionArgs) { // resolves to one of THIS identified user's own linked, active, non-LDAP providers // before starting the round-trip. Mirrors the same check reauth.tsx's idp-reauth // action uses for its own linked-IdP chooser. - const settingsOrg = await resolveOrg(provider, organization); + // + // Resolved through the SAME helper the loader uses, for the same reason: this check must + // accept precisely the set of buttons the loader rendered. Reading a different org's active + // IdPs here would 400 a legitimate click on a provider the loader itself just offered. + const settingsOrg = await resolvePolicyOrg(provider, organization, user.orgId); const [links, active] = await Promise.all([ provider.listIdpLinks(user.id), getActiveIdPs(provider, settingsOrg), @@ -151,12 +260,32 @@ export async function action({ request }: ActionFunctionArgs) { origin: trustedAppOrigin(request), requestId, organization, + // Same divergence the loader's auto-start closes: the button was offered from settingsOrg's + // IdP list, so the callback must gate allowRegister in settingsOrg too. + policyOrg: settingsOrg, reauthHint: loginName, }); if (!result.ok) return data({ error: result.error }, { status: 502 }); return redirect(result.authUrl); } +// This loader MINTS CEREMONY STATE — a Zitadel IdP intent for a sole-linked-IdP account, a +// rotated loaderCsrf token, the one-shot auto-start marker — and costs ~5 provider reads plus a +// rate-limit unit per run. The in-place passkey ceremony below submits its assertion to +// /login/passkey while THIS route stays mounted, and RR's default post-submit revalidation would +// re-run all of that on every ceremony step. Same guard, same marker, and the same WEBAU-3M9si +// class of hazard as login/index.tsx:284 and /login/passkey:40. Anything else revalidates normally. +export function shouldRevalidate({ + formData, + defaultShouldRevalidate, +}: { + formData?: FormData; + defaultShouldRevalidate: boolean; +}) { + if (formData?.get('passkeyCeremony') === '1') return false; + return defaultShouldRevalidate; +} + export default function LoginMethod() { const { methods, branding, idps, csrfToken } = useLoaderData(); const { loginName, requestId, organization } = useLoginContext(); @@ -169,6 +298,34 @@ export default function LoginMethod() { const serverError = useAuthActionError(ceremony.actionData); const passkeyBusy = ceremony.phase !== 'idle'; + // Sole passkey: the identifier step already captured intent, so run the ceremony on + // arrival instead of rendering a one-button screen. One-shot — a cancelled ceremony + // must leave the button usable rather than re-prompting in a loop. + const solePasskey = methods.length === 1 && methods[0] === 'passkey'; + const autoBegunRef = useRef(false); + // The auto-begun attempt has no transient user activation behind it (no click preceded the + // mount), and WebKit REQUIRES activation for navigator.credentials.get() — so on Safari it + // reliably rejects with NotAllowedError, which the ceremony classifies as 'not-allowed'. + // That reason's copy tells the user their passkey sign-in "was cancelled", accusing someone + // who has done nothing but arrive. Suppress the copy for THIS attempt only; the Passkey + // button below stays as the way forward, and a ceremony the user starts by clicking reports + // its failure exactly as it does on every other surface. + const [suppressAutoBeginReason, setSuppressAutoBeginReason] = useState(false); + useEffect(() => { + if (!solePasskey || autoBegunRef.current) return; + autoBegunRef.current = true; + setSuppressAutoBeginReason(true); + ceremony.begin(); + }, [solePasskey, ceremony]); + + // Any user-initiated ceremony re-arms the copy — begin() clears the previous reason, so the + // next one to land belongs to an attempt the user asked for. + const beginFromClick = () => { + setSuppressAutoBeginReason(false); + ceremony.begin(); + }; + const ceremonyReason = suppressAutoBeginReason ? null : ceremony.reason; + // Mirrors login/index.tsx's own submittingIdpId computation — drives IdpButtonList's // per-row loading state while its POST to this route's own action is in flight. const navigation = useNavigation(); @@ -193,9 +350,9 @@ export default function LoginMethod() { Not you?

- {ceremony.reason ? ( + {ceremonyReason ? ( - + ) : serverError ? ( {serverError} @@ -204,22 +361,38 @@ export default function LoginMethod() {
{methods.includes('passkey') ? ( - // Button (not LinkButton) — fires the ceremony IN PLACE (lazy challenge; - // a password pick never spends a session) instead of navigating to - // /login/passkey. The list stays visible as the fallback on failure. - + ) : null} {methods.includes('otp_email') ? ( diff --git a/app/server.ts b/app/server.ts index a66940305c..ca37afb13f 100644 --- a/app/server.ts +++ b/app/server.ts @@ -13,6 +13,9 @@ import { reauthRateLimit, passkeysRateLimit, loginMethodRateLimit, + loginMethodIntentRateLimit, + loginMethodIntentIpRateLimit, + loginIdentifierRateLimit, } from '@/server/middleware/rate-limit'; import { requestContext, type RequestContextEnv } from '@/server/middleware/request-context'; import { appSecureHeaders, resolveFrameAncestors } from '@/server/middleware/secure-headers'; @@ -108,6 +111,9 @@ export default await createHonoServer({ // normalized to lowercase by `normalizedPathname` in net.ts (toLowerCase + trailing-slash // strip + .data strip). Requests that do not match call `next()` immediately with no // meaningful overhead beyond a pathname parse. + // The identifier submit is what hands out the ceremony session /id/login/method's gate + // demands, so it needs its own ceiling — see the rationale in rate-limit.ts. + app.use('*', loginIdentifierRateLimit); app.use('*', loginPasswordRateLimit); app.use('*', signupRateLimit); app.use('*', passwordResetRateLimit); @@ -120,6 +126,10 @@ export default await createHonoServer({ app.use('*', reauthRateLimit); app.use('*', passkeysRateLimit); app.use('*', loginMethodRateLimit); + // The chooser GET is counted by BOTH tiers (tight ip|loginName, loose ip ceiling) — + // see the two-tier rationale in rate-limit.ts. + app.use('*', loginMethodIntentRateLimit); + app.use('*', loginMethodIntentIpRateLimit); app.get('/healthz', (c) => c.json({ status: 'ok' })); app.get('/readyz', (c) => c.json({ status: 'ready' })); app.get('/security', (c) => diff --git a/app/server/middleware/rate-limit.ts b/app/server/middleware/rate-limit.ts index 84e9517ea8..dd4de62fa8 100644 --- a/app/server/middleware/rate-limit.ts +++ b/app/server/middleware/rate-limit.ts @@ -83,6 +83,37 @@ export const verifyEmailSendRateLimit: MiddlewareHandler = createRateLimit({ key: (_c, ip) => ip, }); +// One shared limiter for the IDENTIFIER submit, POST /id/login: 120 attempts / 5 min per ip. +// +// This endpoint was the one unthrottled member of the auth surface, and the chooser work made +// that load-bearing rather than incidental: /id/login/method's session gate demands a ceremony +// session for the probed loginName, and THIS is the endpoint that hands one out. Unthrottled, a +// prober mints cookies for as many addresses as they like and the gate costs them a single extra +// request each. It is also the endpoint that reveals existence directly whenever +// ignoreUnknownUsernames is off (302 vs an inline USER_NOT_FOUND). +// +// IP-ONLY, deliberately. The loginName is in the POST body (body-stream hazard — the RR7 action +// downstream calls `await request.formData()`, and a Hono request body reads once), so a tight +// ip|loginName tier would need a `c.req.raw.clone()` and an async key, which createRateLimit's +// synchronous `key` contract does not take. An ip-only ceiling is what bounds enumeration +// BREADTH anyway; per-account brute force is not the threat at this step (no credential is +// submitted here) and Zitadel's failedAttempts policy backstops the password step that follows. +// +// 120/5min MIRRORS loginMethodIntentIpRateLimit on purpose — same endpoint class (one hit per +// sign-in, on the most travelled path in the product), so the two halves of the same ceremony +// cannot disagree about what a busy NAT'd office looks like. ~24 identifier submits/minute from +// a single egress IP. +const loginIdentifierLimiter = new RateLimiter({ limit: 120, windowMs: 5 * 60_000 }); + +// Mounted on '*' in server.ts. Self-guards on POST + the normalized path (which strips the RR7 +// single-fetch `.data` suffix, so the hydrated submit is counted too). +export const loginIdentifierRateLimit: MiddlewareHandler = createRateLimit({ + limiter: loginIdentifierLimiter, + match: (c, pathname) => c.req.method === 'POST' && pathname === '/id/login', + key: (_c, ip) => ip, + logFields: (c, ip, pathname) => ({ ip, path: pathname }), +}); + // One shared limiter for /login/password: 5 attempts / 5 min per (ip + loginName). const passwordLimiter = new RateLimiter({ limit: 5, windowMs: 5 * 60_000 }); @@ -292,3 +323,49 @@ export const loginMethodRateLimit: MiddlewareHandler = createRateLimit({ match: (c, pathname) => c.req.method === 'POST' && pathname === '/id/login/method', key: (_c, ip) => ip, }); + +// The /id/login/method LOADER is state-changing too, and the limiter above never saw it. +// When the identified user's only usable method is a single linked IdP, the loader mints a real +// Zitadel IdP intent and 302s to the provider (routes/login/method.tsx) — so +// `GET /id/login/method?loginName=X` changes provider-side state AND its redirect names the +// exact IdP that address uses. That is the same linked-IdP identity oracle the POST comment +// above describes, reachable by URL alone. (The loader now also requires a live ceremony session +// for that loginName, which closes the oracle to an unauthenticated prober; these limiters stay +// as the defence-in-depth layer underneath it, exactly like the POST's.) +// +// Matching requires a NON-EMPTY `loginName`: a bare GET (and a `?loginName=` with nothing in it) +// only bounces to /login and touches nothing, so it must not spend anyone's budget. +// +// SEPARATE BUDGET from the POST, deliberately. The POST is a chooser click; this GET is an +// ordinary page load on what is now the destination of EVERY post-identifier sign-in. +// +// TWO TIERS, because one key cannot serve both goals: +// • ip|loginName (10/5min) — the TIGHT one. Bounds how many intents can be minted against ONE +// address, which is what repeated probing of a known account (and the Back-button intent +// storm) looks like. A human signing in spends 1; a reload or two still fits. +// • ip (120/5min) — the LOOSE CEILING. Keying only by (ip, loginName) would hand an +// enumerator a fresh bucket per probed address, so breadth still needs an ip-only cap. It is +// deliberately generous: this path is now on every single sign-in, and a whole office behind +// one NAT egress IP must not 429 during a busy morning. 120/5min leaves ~24 sign-ins/minute +// from a single IP while still capping scripted enumeration breadth. +// Both are mounted; a request is counted by each, and whichever trips first answers 429. +const loginMethodIntentLimiter = new RateLimiter({ limit: 10, windowMs: 5 * 60_000 }); +const loginMethodIntentIpLimiter = new RateLimiter({ limit: 120, windowMs: 5 * 60_000 }); + +/** Shared self-guard: only an identified GET on the chooser is state-changing. */ +const isIdentifiedChooserGet = (c: Context, pathname: string): boolean => + c.req.method === 'GET' && pathname === '/id/login/method' && normalizedLoginName(c) !== ''; + +export const loginMethodIntentRateLimit: MiddlewareHandler = createRateLimit({ + limiter: loginMethodIntentLimiter, + match: isIdentifiedChooserGet, + key: (c, ip) => `${ip}|${normalizedLoginName(c)}`, + logFields: (c, ip) => ({ ip, actor: hashActor(normalizedLoginName(c)), tier: 'ip+loginName' }), +}); + +export const loginMethodIntentIpRateLimit: MiddlewareHandler = createRateLimit({ + limiter: loginMethodIntentIpLimiter, + match: isIdentifiedChooserGet, + key: (_c, ip) => ip, + logFields: (c, ip, pathname) => ({ ip, path: pathname, tier: 'ip' }), +}); diff --git a/app/server/net.ts b/app/server/net.ts index c4cc99c86f..e9069d53d7 100644 --- a/app/server/net.ts +++ b/app/server/net.ts @@ -3,6 +3,7 @@ // Shared network primitives for server middleware: client-IP extraction from the // X-Forwarded-For header, and a factory that owns the common rate-limit middleware // shape (pathname-normalize → IP-extract → check → audit-on-429 → 429 response). +import { APP_BASENAME } from '@/resources/shared/app-basename'; import type { RateLimiter } from '@/server/middleware/rate-limit'; import { logAuthEvent } from '@/server/observability'; import type { Context, MiddlewareHandler } from 'hono'; @@ -44,6 +45,61 @@ const RATE_LIMITED_BODY = { message: 'Too many attempts. Please try again later.', } as const; +/** + * True when this request is a TOP-LEVEL BROWSER NAVIGATION rather than a data fetch. + * + * Rate-limited POSTs and React-Router single-fetch `.data` requests are consumed by code (an + * action result, a fetcher) and must keep the JSON body every other limiter in this file + * returns. A rate-limited GET document request, by contrast, is rendered by the browser AS THE + * PAGE — a raw `{"error":"RATE_LIMITED"}` blob is what the user would see. Only that case gets + * HTML. + * + * `pathname` is already normalized (lowercased, `.data` stripped), so the raw URL is re-read here + * to tell `/id/login/method` from `/id/login/method.data`. + */ +function isTopLevelNavigation(c: Context): boolean { + if (c.req.method !== 'GET') return false; + if (new URL(c.req.url).pathname.toLowerCase().endsWith('.data')) return false; + return (c.req.header('accept') ?? '').includes('text/html'); +} + +/** + * Minimal, self-contained 429 page for a rate-limited top-level navigation. + * + * Hono middleware runs OUTSIDE the React/Lingui tree (same constraint as + * server/routes/saml-post.ts), so the copy is plain English here rather than a ``. + * No script (the prod CSP is nonce-based + strict-dynamic and this response carries no nonce); + * the inline + +
+

Too many attempts

+

You have made too many requests. Please try again ${wait}.

+

Back to sign in

+
+ `; +} + export interface CreateRateLimitOpts { /** The shared limiter instance (window + limit + store live here). */ limiter: RateLimiter; @@ -84,9 +140,11 @@ export function createRateLimit(opts: CreateRateLimitOpts): MiddlewareHandler { if (!allowed) { const fields = logFields ? logFields(c, ip, pathname) : { ip, path: pathname }; logAuthEvent('rate_limit', 'failure', fields); - return c.json(RATE_LIMITED_BODY, 429, { - 'Retry-After': String(Math.ceil(retryAfterMs / 1000)), - }); + const retryAfterSeconds = Math.ceil(retryAfterMs / 1000); + const headers = { 'Retry-After': String(retryAfterSeconds) }; + return isTopLevelNavigation(c) + ? c.html(renderRateLimitedPage(retryAfterSeconds), 429, headers) + : c.json(RATE_LIMITED_BODY, 429, headers); } await next(); }; diff --git a/app/server/signed-param.ts b/app/server/signed-param.ts new file mode 100644 index 0000000000..03febe8b97 --- /dev/null +++ b/app/server/signed-param.ts @@ -0,0 +1,52 @@ +// app/server/signed-param.ts +// +// Detached HMAC-SHA256 signatures for values that must survive a round-trip through an +// ATTACKER-CONTROLLED carrier and still be trusted on the way back. +// +// Every other value we hand to a browser and read again rides a signed cookie +// (`sessions`, `reauth-intent`, `idp-autostart`, …) — react-router's `createCookie({ secrets })` +// does the signing there. A QUERY PARAM on an IdP return URL cannot use that machinery: the +// URL is handed to the IdP broker, which redirects the browser to it directly, so the value +// travels in the open and anyone who can craft a callback URL can put whatever they like in it. +// A `.max(64)` on the way back proves nothing about WHO wrote it. +// +// Same secret and the same HMAC-SHA256 construction the cookies use (SESSION_SECRET, validated +// >= 32 chars at boot), so there is one key to rotate and one primitive to reason about. +import { env } from '@/server/infra/env.server'; +import { createHmac, timingSafeEqual } from 'node:crypto'; + +/** + * Domain separation. A signature minted for one param must never validate as another — without + * a purpose in the MAC input, a signed `policyOrg=` and any future signed param carrying the + * same string would be interchangeable, and the narrower capability could be swapped in for the + * wider one. + */ +function digest(purpose: string, value: string): Buffer { + return createHmac('sha256', env.SESSION_SECRET).update(`${purpose}:${value}`).digest(); +} + +/** base64url HMAC over `purpose:value`. URL-safe, so it needs no extra escaping in a query. */ +export function signParam(purpose: string, value: string): string { + return digest(purpose, value).toString('base64url'); +} + +/** + * True only for a signature THIS deployment minted for exactly this purpose + value. + * Absent, malformed, truncated, and forged signatures are all plain `false` — callers are + * expected to fall back to whatever they would have used with no value at all, never to + * trust the unverified value. + */ +export function verifyParam( + purpose: string, + value: string, + signature: string | null | undefined +): boolean { + if (!signature) return false; + const expected = digest(purpose, value); + // Buffer.from does not throw on stray base64url characters — it decodes what it can — so the + // length check is what rejects a malformed signature before timingSafeEqual (which throws on + // mismatched lengths). Comparison itself stays constant-time. + const provided = Buffer.from(signature, 'base64url'); + if (provided.length !== expected.length) return false; + return timingSafeEqual(provided, expected); +} diff --git a/cypress/component/resources/login/login-decision.cy.ts b/cypress/component/resources/login/login-decision.cy.ts index 1e5c40d888..33a36c2ec3 100644 --- a/cypress/component/resources/login/login-decision.cy.ts +++ b/cypress/component/resources/login/login-decision.cy.ts @@ -8,7 +8,7 @@ import { decideAfterIdentifier } from '@/resources/login/login-decision'; const PRIMARY = { role: 'primary' } as const; describe('decideAfterIdentifier → discriminated Decision union', () => { - it('does NOT route to /sso when allowExternalIdp is false (policy gate)', () => { + it('does NOT count idp when allowExternalIdp is false (policy gate)', () => { const d = decideAfterIdentifier({ methods: ['idp', 'password'], settings: { @@ -19,10 +19,12 @@ describe('decideAfterIdentifier → discriminated Decision union', () => { emailDeliveryEnabled: true, context: PRIMARY, }); - expect(d).to.deep.equal({ kind: 'redirect', path: '/login/password' }); + // Only password survives the gate — but a single method now still routes to + // the chooser, which renders exactly that one method. + expect(d).to.deep.equal({ kind: 'redirect', path: '/login/method' }); }); - it('routes to /sso when allowExternalIdp is true and idp is enrolled', () => { + it('a sole idp routes to /login/method, never to /sso', () => { const d = decideAfterIdentifier({ methods: ['idp'], settings: { @@ -33,6 +35,48 @@ describe('decideAfterIdentifier → discriminated Decision union', () => { emailDeliveryEnabled: true, context: PRIMARY, }); - expect(d).to.deep.equal({ kind: 'redirect', path: '/sso' }); + expect(d).to.deep.equal({ kind: 'redirect', path: '/login/method' }); + }); + + it('two methods still route to /login/method', () => { + const d = decideAfterIdentifier({ + methods: ['idp', 'password'], + settings: { + allowPassword: true, + allowExternalIdp: true, + passkeysType: 'not_allowed', + } as LoginSettings, + emailDeliveryEnabled: true, + context: PRIMARY, + }); + expect(d).to.deep.equal({ kind: 'redirect', path: '/login/method' }); + }); + + it('zero enrolled methods still routes to /verify (invite path)', () => { + const d = decideAfterIdentifier({ + methods: [], + settings: { + allowPassword: true, + allowExternalIdp: true, + passkeysType: 'not_allowed', + } as LoginSettings, + emailDeliveryEnabled: true, + context: PRIMARY, + }); + expect(d).to.deep.equal({ kind: 'redirect', path: '/verify' }); + }); + + it('enrolled password but policy forbids it → PASSWORD_NOT_ALLOWED', () => { + const d = decideAfterIdentifier({ + methods: ['password'], + settings: { + allowPassword: false, + allowExternalIdp: false, + passkeysType: 'not_allowed', + } as LoginSettings, + emailDeliveryEnabled: true, + context: PRIMARY, + }); + expect(d).to.deep.equal({ kind: 'error', error: 'PASSWORD_NOT_ALLOWED' }); }); }); diff --git a/cypress/component/resources/login/resolve-identifier-ignore-unknown.cy.ts b/cypress/component/resources/login/resolve-identifier-ignore-unknown.cy.ts index 9bbbed972f..bbe9f82195 100644 --- a/cypress/component/resources/login/resolve-identifier-ignore-unknown.cy.ts +++ b/cypress/component/resources/login/resolve-identifier-ignore-unknown.cy.ts @@ -13,4 +13,32 @@ describe('resolveIdentifier — ignoreUnknownUsernames', () => { }); expect(r).to.deep.equal({ ok: false, error: 'USER_NOT_FOUND' }); }); + + it('ON: the ghost target is whatever a real PASSWORD-ONLY account resolves to', () => { + // Pinned as a comparison, never as a literal. The ghost has no destination of its own — its + // only job is to collide with the password-only account's, and a literal '/login/method' here + // would pass just as happily on the day one of the two moves and the other does not. + const seed = { + users: [{ id: 'u1', loginName: 'alice@acme.test' }], + authMethods: { u1: ['password'] as const }, + settingsByOrg: { 'org-default-fake': { ignoreUnknownUsernames: true } }, + }; + cy.wrap(null).then(async () => { + const p = new FakeAuthProvider(seed as ConstructorParameters[0]); + const known = await resolveIdentifier(p, [], { + loginName: 'alice@acme.test', + emailDeliveryEnabled: true, + }); + const ghost = await resolveIdentifier(p, [], { + loginName: 'ghost@acme.test', + emailDeliveryEnabled: true, + }); + expect(known.ok && 'target' in known && known.target).to.be.a('string'); + expect(ghost.ok && 'target' in ghost && ghost.target).to.equal( + known.ok && 'target' in known && known.target + ); + // And it really did plant a ceremony session, or /login/method's gate would bounce it. + expect(ghost.ok && 'sessions' in ghost && ghost.sessions).to.have.length(1); + }); + }); }); diff --git a/cypress/component/resources/shared/org-first-settings-reads.cy.ts b/cypress/component/resources/shared/org-first-settings-reads.cy.ts index 2261983ed6..06ba7c429c 100644 --- a/cypress/component/resources/shared/org-first-settings-reads.cy.ts +++ b/cypress/component/resources/shared/org-first-settings-reads.cy.ts @@ -19,7 +19,13 @@ describe('login/method loader — org-first getLoginSettings + getBranding', () callService({ fn: 'loginMethodLoader', provider: 'singleton', - request: { url: `http://localhost/id/login/method?loginName=${ALICE}` }, + request: { + url: `http://localhost/id/login/method?loginName=${ALICE}`, + // The chooser loader is session-gated: it only identifies a user the ceremony has + // already planted a LIVE session for (the identifier step writes it on the same + // response that redirects here). + sessions: [{ id: 's1', token: 'tok-s1', loginName: ALICE }], + }, recordCalls: ['getDefaultOrg', 'getLoginSettings', 'getBranding'], }).then((v) => { expect(v.calls?.getDefaultOrg).to.have.length(1); @@ -34,6 +40,7 @@ describe('login/method loader — org-first getLoginSettings + getBranding', () provider: 'singleton', request: { url: `http://localhost/id/login/method?loginName=${ALICE}&organization=org-explicit`, + sessions: [{ id: 's1', token: 'tok-s1', loginName: ALICE }], }, recordCalls: ['getDefaultOrg', 'getLoginSettings', 'getBranding'], }).then((v) => { @@ -44,6 +51,58 @@ describe('login/method loader — org-first getLoginSettings + getBranding', () }); }); +describe('resolveIdentifier — post-identifier policy org agrees with the chooser', () => { + // The identifier step decides a user HAS a usable method and routes them to /login/method, + // whose loader then RE-computes that availability. If the two read a different org's policy the + // loader can compute `available: []` for a user the decision just approved and bounce them to + // /error. They previously agreed only for users WITH an orgId: `User.orgId` is OPTIONAL, and + // when it was absent this side fell through to INSTANCE settings (getLoginSettings(undefined)) + // while the chooser fell through to the DEFAULT ORG's. alice has no orgId, so she is exactly + // that case. + it('with NO ?organization AND no user.orgId, reads the DEFAULT org (not instance settings)', () => { + callService({ + fn: 'loginAction', + provider: 'singleton', + request: { + url: 'http://localhost/id/login', + form: { loginName: ALICE }, + csrf: true, + }, + recordCalls: ['getLoginSettings'], + }).then((v) => { + // LAST call, not a fixed index: resolveIdentifier's domain-discovery probe reads the + // BASE/instance settings first by design (the org isn't known yet), so the post-identifier + // read is the final one. + const orgArgs = (v.calls?.getLoginSettings ?? []).map((args) => args[0]); + expect(orgArgs.length, 'the post-identifier settings read ran').to.be.greaterThan(1); + expect(orgArgs.at(-1), 'must not fall through to INSTANCE settings').to.equal( + 'org-default-fake' + ); + // And the decision still routes to the chooser rather than a policy dead end. + expect(v.response?.location ?? '').to.contain('/login/method'); + }); + }); + + it('a user WITH an orgId still gets their OWN org, never the default', () => { + callService({ + fn: 'loginAction', + seed: { + users: [{ id: 'u1', loginName: 'mia@acme.test', orgId: 'org-mia' }], + authMethods: { u1: ['password'] }, + }, + request: { + url: 'http://localhost/id/login', + form: { loginName: 'mia@acme.test' }, + csrf: true, + }, + recordCalls: ['getLoginSettings'], + }).then((v) => { + const orgArgs = (v.calls?.getLoginSettings ?? []).map((args) => args[0]); + expect(orgArgs.at(-1)).to.equal('org-mia'); + }); + }); +}); + describe('session.service listAccounts — session-derived org-first getLoginSettings', () => { it('a cookie session WITH an organization → getLoginSettings gets it; provider default never consulted', () => { callService({ diff --git a/cypress/component/resources/sso/idp-return-urls.cy.ts b/cypress/component/resources/sso/idp-return-urls.cy.ts index 38b8bae87b..7c15947f8f 100644 --- a/cypress/component/resources/sso/idp-return-urls.cy.ts +++ b/cypress/component/resources/sso/idp-return-urls.cy.ts @@ -2,7 +2,8 @@ // // Component (no-mount) port of app/resources/sso/__tests__/idp-return-urls.test.ts. // idpReturnUrls is pure URL building (basename + requestId/link threading) → browser-side Chai. -import { idpReturnUrls, APP_BASENAME } from '@/resources/sso/idp-return-urls'; +import { idpReturnUrls, APP_BASENAME, POLICY_ORG_PURPOSE } from '@/resources/sso/idp-return-urls'; +import { signParam } from '@/server/signed-param'; describe('idpReturnUrls', () => { it('includes the /id basename so the IdP broker redirect hits the real route (regression: 404)', () => { @@ -46,9 +47,54 @@ describe('idpReturnUrls', () => { expect(failure).to.equal('http://localhost:3000/id/sso/google/error'); }); - it('failure url never carries link/deviceTrackingToken (success-path-only concerns)', () => { + it('carries a policyOrg that DIFFERS from organization, so the callback gates allowRegister in the deciding org', () => { + // /login/method picks a sole linked IdP out of a list resolved for the FOUND USER's org while + // the ceremony `organization` is still absent. Without this param the callback resolved + // allowRegister (and the auto-create org) from the DEFAULT org instead — a different policy + // from the one that offered the provider. + const { success } = idpReturnUrls('http://localhost:3000', 'google', { + policyOrg: 'org-mia', + }); + // SIGNED, and signed here at the START. This param is the only one on the URL that is not + // already coupled to something the attacker would also have to get right (see the note in + // idp-return-urls.ts), so the callback trusts provenance rather than shape — which means the + // producer must always emit the pair. Asserted through signParam rather than a literal so the + // spec pins the PAIRING, not one particular digest. + const sig = signParam(POLICY_ORG_PURPOSE, 'org-mia'); + expect(sig, 'a signature is actually produced').to.be.a('string').and.not.be.empty; + expect(success).to.equal( + `http://localhost:3000/id/sso/google/callback?policyOrg=org-mia&policyOrgSig=${encodeURIComponent(sig)}` + ); + }); + + it('never emits a bare policyOrg — an unsigned one is exactly what the callback refuses', () => { + // Regression guard on the producer half: dropping the signature here would not break any + // sign-in (the callback silently falls back to `organization`), it would just quietly restore + // the default-org policy divergence policyOrg exists to fix. Fail loudly instead. + const { success } = idpReturnUrls('http://localhost:3000', 'google', { + organization: 'org-1', + policyOrg: 'org-mia', + requestId: 'oidc_1', + }); + const params = new URL(success).searchParams; + expect(params.get('policyOrg')).to.equal('org-mia'); + expect(params.get('policyOrgSig'), 'signature rides with the value').to.equal( + signParam(POLICY_ORG_PURPOSE, 'org-mia') + ); + }); + + it('omits policyOrg when it is identical to organization (no redundant param)', () => { + const { success } = idpReturnUrls('http://localhost:3000', 'google', { + organization: 'org-1', + policyOrg: 'org-1', + }); + expect(success).to.equal('http://localhost:3000/id/sso/google/callback?organization=org-1'); + }); + + it('failure url never carries policyOrg/link/deviceTrackingToken (success-path-only concerns)', () => { const { failure } = idpReturnUrls('http://localhost:3000', 'google', { link: true, + policyOrg: 'org-mia', deviceTrackingToken: 'mm-token-abc', }); expect(failure).to.equal('http://localhost:3000/id/sso/google/error'); diff --git a/cypress/component/resources/sso/sso-callback.cy.ts b/cypress/component/resources/sso/sso-callback.cy.ts index 56fcd8bb9e..846456f215 100644 --- a/cypress/component/resources/sso/sso-callback.cy.ts +++ b/cypress/component/resources/sso/sso-callback.cy.ts @@ -310,6 +310,150 @@ describe('processIdpCallback — default-org resolution for IdP auto-create (bar }); }); +describe('processIdpCallback — ?policyOrg gates the allowRegister/auto-create org', () => { + // The START side (login/method.tsx's sole-linked-IdP auto-start, and its chooser action) picks + // an IdP out of a list resolved for the FOUND USER's org while the ceremony `organization` is + // still absent. `policyOrg` carries that deciding org across the round-trip so the callback + // gates allowRegister — and auto-creates — under the SAME policy that offered the provider, + // instead of falling back to the default org. + it('reads allowRegister from policyOrg and registers into it, ignoring the default-org fallback', () => { + callService({ + fn: 'processIdpCallback', + slug: 'google', + seed: {}, + idpIntent: REGISTER_INTENT_VERIFIED, + // signPolicyOrg mints the value + signature through the REAL idpReturnUrls, i.e. exactly + // what a legitimate start emits. Only that pair is honoured (see the forgery specs below). + request: { url: CB('google', 'id=intent-1&token=tok-1'), signPolicyOrg: 'org-mia' }, + recordCalls: ['getLoginSettings', 'register'], + }).then((v) => { + const settingsCalls = (v.calls?.['getLoginSettings'] ?? []) as Array<[string | undefined]>; + expect(settingsCalls[0]?.[0], 'allowRegister is read from the deciding org').to.equal( + 'org-mia' + ); + const registerCalls = (v.calls?.['register'] ?? []) as Array<[Record]>; + expect(registerCalls[0]?.[0]?.orgId, 'auto-create lands in the deciding org').to.equal( + 'org-mia' + ); + }); + }); + + it('does NOT org-scope the same-email findUser — that lookup stays instance-wide', () => { + // The whole reason policyOrg is its own param: widening `organization` instead would have + // silently narrowed these findUser lookups (see the note at sso-callback.ts's callbackOrg), + // so an existing account outside the named org would stop being found and the flow would + // register a duplicate. The seeded account has NO orgId, so an org-scoped lookup misses it. + callService({ + fn: 'processIdpCallback', + slug: 'google', + env: { ALLOW_IDP_AUTO_LINK: 'true' }, + seed: { users: [{ id: 'u1', loginName: 'you@gmail.com', displayName: 'You User' }] }, + idpIntent: REGISTER_INTENT_VERIFIED, + request: { url: CB('google', 'id=intent-1&token=tok-1'), signPolicyOrg: 'org-mia' }, + recordCalls: ['findUser', 'register'], + }).then((v) => { + const findUserCalls = (v.calls?.['findUser'] ?? []) as Array<[string, string | undefined]>; + expect(findUserCalls.length, 'the same-email pre-check ran').to.be.greaterThan(0); + // `undefined` crosses the cy.task JSON boundary as null — either way it is NOT 'org-mia'. + expect( + findUserCalls[0][1] ?? null, + 'findUser stays instance-wide (org arg untouched)' + ).to.equal(null); + // …and the existing account was actually found: it auto-linked instead of registering. + expect(v.calls?.['register'] ?? []).to.have.length(0); + }); + }); +}); + +describe('processIdpCallback — an unsigned/forged ?policyOrg is ignored', () => { + // The callback URL is handed to the IdP broker and then to the browser, so `?policyOrg=` is + // fully attacker-writable. It is also the ONE param on that URL coupled to nothing else: + // `organization` drags the same-email findUser scope, createSession's orgId and + // signInWithIdpIntent along with it, so naming a foreign org there fails closed. policyOrg feeds + // `callbackOrg` alone — which gates allowRegister and picks the auto-create org — so a crafted + // one borrows a registration-open org's policy WHILE the same-email lookup stays instance-wide. + // With ALLOW_IDP_AUTO_LINK on that is a path into a victim's account in any org. A `.max(64)` + // cannot help: decoupling those two orgs is the param's entire purpose, so only provenance can. + const FORGED = 'org-attacker-registration-open'; + + it('a hand-written policyOrg with NO signature falls back to `organization`', () => { + callService({ + fn: 'processIdpCallback', + slug: 'google', + seed: {}, + idpIntent: REGISTER_INTENT_VERIFIED, + request: { url: CB('google', `id=intent-1&token=tok-1&policyOrg=${FORGED}`) }, + recordCalls: ['getLoginSettings', 'register'], + }).then((v) => { + const settingsCalls = (v.calls?.['getLoginSettings'] ?? []) as Array<[string | undefined]>; + expect(settingsCalls[0]?.[0], 'allowRegister must NOT come from the forged org').to.equal( + 'org-default-fake' + ); + const registerCalls = (v.calls?.['register'] ?? []) as Array<[Record]>; + expect(registerCalls[0]?.[0]?.orgId, 'auto-create must NOT land in the forged org').to.equal( + 'org-default-fake' + ); + }); + }); + + it('a MIS-signed policyOrg (valid signature, swapped value) is ignored too', () => { + // The realistic forgery is not a random digest — it is a signature lifted from a legitimate + // start and pasted next to a different org. Domain separation + signing the VALUE is what + // makes that fail; a signature is a claim about one exact string, not a bearer token. + callService({ + fn: 'processIdpCallback', + slug: 'google', + seed: {}, + idpIntent: REGISTER_INTENT_VERIFIED, + // Mint a real pair for 'org-mia', then overwrite the VALUE while keeping the signature. + request: { + url: CB('google', 'id=intent-1&token=tok-1'), + signPolicyOrg: 'org-mia', + tamperPolicyOrg: FORGED, + }, + recordCalls: ['getLoginSettings'], + }).then((v) => { + const settingsCalls = (v.calls?.['getLoginSettings'] ?? []) as Array<[string | undefined]>; + expect(settingsCalls[0]?.[0], 'a mismatched value voids the signature').to.equal( + 'org-default-fake' + ); + }); + }); + + it('logs the rejection so tampering (or a mid-flight secret rotation) is visible', () => { + callService({ + fn: 'processIdpCallback', + slug: 'google', + seed: {}, + idpIntent: REGISTER_INTENT_VERIFIED, + request: { url: CB('google', `id=intent-1&token=tok-1&policyOrg=${FORGED}`) }, + }).then((v) => { + const line = v.audit.find((e) => e.event === 'idp_policy_org'); + expect(line, 'an audit line names the rejected param').to.not.equal(undefined); + expect(line?.outcome).to.equal('failure'); + expect((line as { reason?: string } | undefined)?.reason).to.equal('invalid_signature'); + }); + }); + + it('no policyOrg at all still resolves the default org (unchanged baseline)', () => { + callService({ + fn: 'processIdpCallback', + slug: 'google', + seed: {}, + idpIntent: REGISTER_INTENT_VERIFIED, + request: { url: CB('google', 'id=intent-1&token=tok-1') }, + recordCalls: ['getLoginSettings'], + }).then((v) => { + const settingsCalls = (v.calls?.['getLoginSettings'] ?? []) as Array<[string | undefined]>; + expect(settingsCalls[0]?.[0]).to.equal('org-default-fake'); + expect( + v.audit.some((e) => e.event === 'idp_policy_org'), + 'nothing to reject, nothing logged' + ).to.equal(false); + }); + }); +}); + describe('processIdpCallback — fresh-identity link ceremony (Req 2)', () => { // A FRESH external identity (intent.userId == null) attached to the ACTIVE session user via // ?link=true — the Req-2 wiring (sso-callback.ts:144-153) that the existing already-MAPPED link diff --git a/cypress/component/routes/login/enumeration-parity.cy.ts b/cypress/component/routes/login/enumeration-parity.cy.ts new file mode 100644 index 0000000000..276bb78295 --- /dev/null +++ b/cypress/component/routes/login/enumeration-parity.cy.ts @@ -0,0 +1,214 @@ +// cypress/component/routes/login/enumeration-parity.cy.ts +// +// `ignoreUnknownUsernames` is a tenant-facing ANTI-ENUMERATION setting, and its entire protection +// is that an UNKNOWN identifier and a real PASSWORD-ONLY account are indistinguishable to an +// unauthenticated observer. Routing every account with >= 1 usable method to /login/method broke +// exactly that: the known side moved and the ghost side did not, turning the identifier step's +// redirect target into a perfect account-existence oracle. +// +// These specs pin the parity itself rather than either side's destination, so a future re-route of +// one branch fails here instead of silently reopening the oracle. Four observable channels: +// status, redirect target, rendered chooser, Set-Cookie shape — plus the audit line. +import { callService } from '../../../support/node/call-service'; + +/** The default org the fake resolves to; where the tenant flips the flag. */ +const DEFAULT_ORG = 'org-default-fake'; + +/** One real password-only account, in an org that has anti-enumeration ON. */ +const ANTI_ENUM_SEED = { + users: [{ id: 'u1', loginName: 'alice@acme.test' }], + authMethods: { u1: ['password'] }, + settingsByOrg: { [DEFAULT_ORG]: { ignoreUnknownUsernames: true } }, +}; + +const KNOWN = 'alice@acme.test'; +const GHOST = 'nobody-here@acme.test'; + +/** The identifier submit, as an unauthenticated visitor makes it. */ +function submitIdentifier(loginName: string, seed: Record = ANTI_ENUM_SEED) { + return callService({ + fn: 'loginAction', + seed, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login', + form: { loginName }, + csrf: true, + }, + }); +} + +/** The chooser GET the identifier submit redirects to, carrying the session it just planted. */ +function loadChooser(loginName: string, seed: Record = ANTI_ENUM_SEED) { + return callService({ + fn: 'loginMethodLoader', + seed, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: `http://localhost/id/login/method?loginName=${encodeURIComponent(loginName)}`, + sessions: [{ id: 's1', token: 'tok-s1', loginName }], + }, + }); +} + +/** Cookie NAMES set by a response, sorted — the shape an observer can compare. */ +function cookieNames(setCookies: string[] | undefined): string[] { + return (setCookies ?? []).map((c) => c.split('=')[0]).sort(); +} + +describe('ignoreUnknownUsernames — identifier step is not an existence oracle', () => { + it('unknown and password-only land on the SAME target with the same status', () => { + submitIdentifier(KNOWN).then((known) => { + submitIdentifier(GHOST).then((ghost) => { + expect(known.response?.status, 'known status').to.equal(302); + expect(ghost.response?.status, 'ghost status').to.equal(known.response?.status); + + // Compare the whole Location minus the one field that legitimately differs: the + // identifier the visitor typed. Anything else diverging IS the oracle. + const strip = (loc: string) => loc.replace(/loginName=[^&]*/, 'loginName='); + const knownLoc = strip(known.response?.location ?? ''); + const ghostLoc = strip(ghost.response?.location ?? ''); + expect(knownLoc, 'known goes to the chooser').to.contain('/login/method'); + expect(ghostLoc, 'ghost redirect target must be byte-identical').to.equal(knownLoc); + }); + }); + }); + + it('both set the same Set-Cookie shape (a real ceremony session either way)', () => { + submitIdentifier(KNOWN).then((known) => { + submitIdentifier(GHOST).then((ghost) => { + const knownNames = cookieNames(known.response?.setCookies); + expect(knownNames, 'the known path persists a ceremony session').to.include('sessions'); + expect(cookieNames(ghost.response?.setCookies), 'same cookies, same order').to.deep.equal( + knownNames + ); + // Not merely present — the ghost cookie must carry a real entry, or the chooser's + // session gate would bounce it and re-open the oracle one hop later. + expect( + ghost.response?.cookieEntries ?? [], + 'ghost session entry is planted' + ).to.have.length(known.response?.cookieEntries?.length ?? 0); + }); + }); + }); + + it('emits the SAME audit event for both (no "not_found" tell in the logs)', () => { + submitIdentifier(KNOWN).then((known) => { + submitIdentifier(GHOST).then((ghost) => { + const shape = (v: typeof known) => + v.audit + .filter((e) => e.event === 'identifier') + .map((e) => `${e.event}:${e.outcome}:${(e as { reason?: string }).reason ?? ''}`); + expect(shape(known)).to.deep.equal(['identifier:success:']); + expect(shape(ghost), 'a failure/reason line here names the branch').to.deep.equal( + shape(known) + ); + }); + }); + }); + + it('OFF (default): the unknown identifier is still rejected, no ghost session', () => { + // The parity above must not have turned the flag into a no-op — with it off, an unknown + // identifier is still USER_NOT_FOUND and plants nothing. + submitIdentifier(GHOST, { + users: [{ id: 'u1', loginName: KNOWN }], + authMethods: { u1: ['password'] }, + }).then((v) => { + expect(v.response?.dataBody).to.deep.equal({ error: 'USER_NOT_FOUND' }); + expect(cookieNames(v.response?.dataSetCookies), 'no ceremony session').to.not.include( + 'sessions' + ); + }); + }); +}); + +describe('ignoreUnknownUsernames — the chooser renders the same screen for both', () => { + it('a ghost that clears the session gate RENDERS, it does not bounce', () => { + // The gate is the authorization (a live ceremony session for this loginName, which /login + // mints only under the flag). Bouncing on the findUser miss would move the oracle here. + loadChooser(GHOST).then((v) => { + expect(v.response?.status, 'must not redirect').to.not.equal(302); + expect((v.response?.dataBody as { methods: string[] }).methods).to.deep.equal(['password']); + }); + }); + + it('identical chooser payload: same controls, same branding, same CSRF handling', () => { + loadChooser(KNOWN).then((known) => { + loadChooser(GHOST).then((ghost) => { + type Body = { + loginName: string; + methods: string[]; + idps: unknown[]; + branding: unknown; + csrfToken: string; + }; + const k = known.response?.dataBody as Body; + const g = ghost.response?.dataBody as Body; + + expect(known.response?.status).to.equal(ghost.response?.status); + expect(g.methods, 'same single control').to.deep.equal(k.methods); + expect(g.idps, 'same (empty) IdP list').to.deep.equal(k.idps); + expect(g.branding, 'branding resolves identically').to.deep.equal(k.branding); + // A rotated per-request token either way — its VALUE must differ, its presence must not. + expect(g.csrfToken, 'ghost gets a real CSRF token').to.be.a('string').and.not.be.empty; + expect(typeof g.csrfToken).to.equal(typeof k.csrfToken); + expect(cookieNames(ghost.response?.dataSetCookies)).to.deep.equal( + cookieNames(known.response?.dataSetCookies) + ); + }); + }); + }); + + it('parity SURVIVES a user whose org is NOT the default org', () => { + // The regression this pins. A ghost has no user.orgId, so its policy read fell through to the + // DEFAULT org while a real account was judged by its OWN. Seed the two to DISAGREE — the + // default org forbids passwords, mia's org does not — and the old code split the pair + // (known -> /login/method, ghost -> /error): a perfect existence oracle, in the one + // configuration the rest of this file cannot reach because its users carry no orgId. + // Both branches now resolve a ghost's policy org from the identifier's DOMAIN + // (resolveGhostPolicyOrg), so nobody-here@acme.test is judged by the very org + // mia@acme.test belongs to. + const seed = { + users: [{ id: 'u1', loginName: 'mia@acme.test', orgId: 'org-mia' }], + authMethods: { u1: ['password'] }, + orgDomains: { 'acme.test': 'org-mia' }, + settingsByOrg: { + [DEFAULT_ORG]: { ignoreUnknownUsernames: true, allowPassword: false }, + 'org-mia': { ignoreUnknownUsernames: true }, + }, + }; + submitIdentifier('mia@acme.test', seed).then((known) => { + submitIdentifier('nobody-here@acme.test', seed).then((ghost) => { + const strip = (loc: string) => loc.replace(/loginName=[^&]*/, 'loginName='); + expect(known.response?.location ?? '', 'known reaches the chooser').to.contain( + '/login/method' + ); + expect(ghost.response?.status, 'same status').to.equal(known.response?.status); + expect( + strip(ghost.response?.location ?? ''), + 'the ghost must not be dead-ended by a policy the real account never sees' + ).to.equal(strip(known.response?.location ?? '')); + }); + }); + }); + + it('an org that forbids passwords dead-ends BOTH at /error, never one of them', () => { + // The ghost is decided by the same decideAfterIdentifier call a password-only account gets, + // so a policy that removes the only method removes it for both. Hardcoding a ghost target + // would send it to a chooser the real account is not allowed to reach. + const seed = { + users: [{ id: 'u1', loginName: KNOWN }], + authMethods: { u1: ['password'] }, + settingsByOrg: { + [DEFAULT_ORG]: { ignoreUnknownUsernames: true, allowPassword: false }, + }, + }; + submitIdentifier(KNOWN, seed).then((known) => { + submitIdentifier(GHOST, seed).then((ghost) => { + expect(known.response?.location ?? '', 'known: policy dead end').to.contain('/error'); + expect(ghost.response?.status).to.equal(known.response?.status); + expect(ghost.response?.location ?? '').to.contain('/error'); + }); + }); + }); +}); diff --git a/cypress/component/routes/login/index.cy.tsx b/cypress/component/routes/login/index.cy.tsx index 2105697f7b..3cb0952e3e 100644 --- a/cypress/component/routes/login/index.cy.tsx +++ b/cypress/component/routes/login/index.cy.tsx @@ -2,11 +2,15 @@ // // UI contract for /login: the chooser // renders unconditionally. There is no identity-bound inline passkey ceremony state on -// this route anymore — a sole-passkey identifier now REDIRECTS to /login/passkey (see -// the node-bound action test below) instead of the action returning inline challenge -// data. The gated Passkey SHORTCUT still drives the shared ceremony in place -// on this page, and its own failure surface is still covered here. Mirrors -// method.cy.tsx's stub /login/passkey route + capturedPosts convention (Task 2). +// this route anymore — every identifier with >= 1 usable method (sole-passkey included) +// now REDIRECTS to /login/method, the method chooser, instead of the action returning +// inline challenge data or redirecting straight to a per-method target (see the +// node-bound action test below). /login/method itself now owns auto-starting a sole +// passkey client-side (Task 3) and a sole linked IdP server-side (Task 2) — this route's +// own action no longer picks a per-method destination. The gated Passkey SHORTCUT still +// drives the shared ceremony in place on this page, and its own failure surface is still +// covered here. Mirrors method.cy.tsx's stub /login/passkey route + capturedPosts +// convention (Task 2). import { callService } from '../../../support/node/call-service'; import Login from '@/routes/login/index'; import { ConformAdapter } from '@datum-cloud/datum-ui/form/adapters/conform'; @@ -161,7 +165,12 @@ describe('/login — chooser (no inline ceremony)', () => { }); describe('/login action — sole-passkey identifier', () => { - it('a sole-passkey identifier now REDIRECTS to /login/passkey instead of returning inline data', () => { + it('a sole-passkey identifier REDIRECTS to /login/method (the chooser), not a per-method target', () => { + // Task 1 collapsed decideAfterIdentifier's per-method single-target branch: every + // account with >= 1 usable method — sole-passkey included — now redirects to the + // chooser. /login/method itself auto-begins a sole passkey ceremony client-side + // (Task 3), so the user still sees no intermediate chooser screen; this route's own + // action just hands off to /login/method rather than picking /login/passkey directly. callService({ fn: 'loginAction', provider: 'singleton', @@ -173,15 +182,18 @@ describe('/login action — sole-passkey identifier', () => { }).then((v) => { expect(v.response?.isResponse).to.equal(true); expect(v.response?.status).to.equal(302); - expect(v.response?.location ?? '').to.contain('/login/passkey'); + expect(v.response?.location ?? '').to.contain('/login/method'); + // Regression guard: must NOT fall back to the old per-method redirect target. + expect(v.response?.location ?? '').to.not.contain('/login/passkey'); expect(v.response?.location ?? '').to.contain('loginName='); }); }); }); // ── Identifier-form view logic ─────────────────────────────────────────────── -// Sole-passkey identifiers REDIRECT to /login/passkey (asserted above); these two -// cover the identifier-form visibility logic under restricted org policies. +// Sole-passkey (and every other >= 1-usable-method) identifier REDIRECTS to /login/method +// (asserted above); these two cover the identifier-form visibility logic under restricted +// org policies. describe('/login — identifier-form view logic', () => { // REGRESSION: with allowPassword=false the identifier form used to be hidden entirely, // while signInUnavailable stayed false (suppressed by showPasskeyPrompt) — so a fresh diff --git a/cypress/component/routes/login/method-chooser.cy.ts b/cypress/component/routes/login/method-chooser.cy.ts index 385fb97820..95c8d64758 100644 --- a/cypress/component/routes/login/method-chooser.cy.ts +++ b/cypress/component/routes/login/method-chooser.cy.ts @@ -5,6 +5,21 @@ // (B) /login/method loader → methods array, branding, redirect guards. import { callService } from '../../../support/node/call-service'; +// The chooser loader is SESSION-GATED: it acts only for a loginName this browser already holds a +// LIVE ceremony session for. /login's identifier action plants exactly this entry on the same +// response that redirects here, so supplying it is what makes these scenarios legitimate arrivals +// rather than the drive-by GETs the gate exists to refuse. +const ALICE_SESSION = { id: 's1', token: 'tok-s1', loginName: 'alice@acme.test' }; +const MIA_SESSION = { id: 's1', token: 'tok-s1', loginName: 'mia@acme.test' }; + +/** The sole-linked-IdP fixture: one enrolled method ('idp'), one active+linked provider. */ +const SOLE_IDP_SEED = { + users: [{ id: 'u1', loginName: 'mia@acme.test' }], + authMethods: { u1: ['idp'] }, + idps: [{ id: 'idp-google', name: 'Google', type: 'GOOGLE' }], + idpLinks: { u1: [{ idpId: 'idp-google', idpUserId: 'g-1' }] }, +}; + // ── (A) intent=email-link ────────────────────────────────────────────────────── describe('login action — intent=email-link', () => { @@ -31,16 +46,318 @@ describe('login action — intent=email-link', () => { // ── (B) /login/method loader ─────────────────────────────────────────────────── describe('/login/method loader', () => { - it('alice@acme.test (password-only) → redirects away (< 2 methods)', () => { + it('alice@acme.test (password-only) → RENDERS the chooser, never redirects', () => { + // Regression guard: this used to 302 to /login/password. Now that the decision + // returns /login/method for a single method, redirecting here is a self-redirect loop. callService({ fn: 'loginMethodLoader', provider: 'singleton', env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, - request: { url: 'http://localhost/id/login/method?loginName=alice%40acme.test' }, + request: { + url: 'http://localhost/id/login/method?loginName=alice%40acme.test', + sessions: [ALICE_SESSION], + }, + }).then((v) => { + expect(v.response?.status, 'must not redirect').to.not.equal(302); + const body = v.response?.dataBody as { methods: string[] }; + expect(body.methods).to.deep.equal(['password']); + }); + }); + + it('a sole linked IdP auto-starts the intent instead of rendering one button', () => { + callService({ + fn: 'loginMethodLoader', + seed: { + users: [{ id: 'u1', loginName: 'mia@acme.test' }], + authMethods: { u1: ['idp'] }, + idps: [{ id: 'idp-google', name: 'Google', type: 'GOOGLE' }], + idpLinks: { u1: [{ idpId: 'idp-google', idpUserId: 'g-1' }] }, + }, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + }, }).then((v) => { expect(v.response?.status).to.equal(302); const loc = v.response?.location ?? ''; - expect(loc).to.contain('/login/password'); + expect(loc, 'goes to the provider, not /sso').to.not.contain('/sso'); + expect(loc).to.contain('idp-google'); + }); + }); + + it('two linked IdPs render a picker rather than guessing one', () => { + callService({ + fn: 'loginMethodLoader', + seed: { + users: [{ id: 'u1', loginName: 'mia@acme.test' }], + authMethods: { u1: ['idp'] }, + idps: [ + { id: 'idp-google', name: 'Google', type: 'GOOGLE' }, + { id: 'idp-github', name: 'GitHub', type: 'GITHUB' }, + ], + idpLinks: { + u1: [ + { idpId: 'idp-google', idpUserId: 'g-1' }, + { idpId: 'idp-github', idpUserId: 'h-1' }, + ], + }, + }, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + }, + }).then((v) => { + expect(v.response?.status).to.not.equal(302); + const body = v.response?.dataBody as { idps: Array<{ id: string }> }; + expect(body.idps).to.have.length(2); + }); + }); + + it('idp enrolled with zero usable links never self-redirects to /login/method', () => { + // Regression guard for the loop the previous fix round missed: the loader's OWN + // `available` computation only counts 'idp' when a real, active, non-LDAP link + // resolves (method.tsx:75-87) — here there are none. decideAfterIdentifier, however, + // recomputes availability blind to that resolution (methods.includes('idp') && + // settings.allowExternalIdp alone), so it still names THIS route. Falling into the + // `available.length === 0` branch and following that decision verbatim would 302 back + // to /login/method with the exact same inputs — an infinite loop. + callService({ + fn: 'loginMethodLoader', + seed: { + users: [{ id: 'u1', loginName: 'mia@acme.test' }], + authMethods: { u1: ['idp'] }, + // No idpLinks entry for u1 at all: methods says 'idp' is enrolled, but nothing + // resolves to a usable, linked, active, non-LDAP provider. + }, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + }, + }).then((v) => { + // Name the destination, don't merely rule one out: `not.contain('/login/method')` also + // passes on a 200 that renders an EMPTY chooser, which is the other way this can go wrong. + // 'idp' enrolled means methods.length !== 0, so the policy-dead-end leg (/error) is the + // one and only correct answer here. + expect(v.response?.status, 'must redirect, not render an empty chooser').to.equal(302); + const loc = v.response?.location ?? ''; + expect(loc, 'a policy dead end goes to /error').to.contain('/error'); + expect(loc, 'must never redirect back to /login/method').to.not.contain('/login/method'); + }); + }); + + it('falls through and RENDERS the button when the sole-IdP intent fails to start', () => { + // A provider that accepts the call but returns no authUrl → IDP_UNAVAILABLE. Redirecting or + // erroring here would strand the user on a screen with nothing to press; the chooser must + // render its one Google button so they keep a way forward (and can retry via the action). + callService({ + fn: 'loginMethodLoader', + seed: SOLE_IDP_SEED, + failStartIdpIntent: true, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + }, + }).then((v) => { + expect(v.response?.status, 'no redirect — render the fallback').to.not.equal(302); + const body = v.response?.dataBody as { methods: string[]; idps: Array<{ id: string }> }; + expect(body.methods).to.deep.equal(['idp']); + expect(body.idps).to.have.length(1); + }); + }); +}); + +describe('/login/method loader — session gate', () => { + // GET /id/login/method?loginName=X is CSRF-token-free and state-changing (a sole-linked-IdP + // account makes it mint a real Zitadel intent and 302 to the provider, naming that provider). + // Ungated it was an account-existence AND identity-provider oracle reachable by URL alone, a + // login-CSRF vector, and a bypass of ignoreUnknownUsernames — which is honoured only in + // resolveIdentifier. The ceremony session the identifier step already planted is the gate. + it('bounces to /login with NO session cookie at all', () => { + callService({ + fn: 'loginMethodLoader', + seed: SOLE_IDP_SEED, + request: { url: 'http://localhost/id/login/method?loginName=mia%40acme.test' }, + }).then((v) => { + expect(v.response?.status).to.equal(302); + const loc = v.response?.location ?? ''; + expect(loc).to.match(/\/login(\?|$)/); + expect(loc, 'must not name the provider').to.not.contain('idp-google'); + }); + }); + + it('bounces when the session belongs to a DIFFERENT account (no borrowing someone else)', () => { + callService({ + fn: 'loginMethodLoader', + seed: SOLE_IDP_SEED, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [ALICE_SESSION], + }, + }).then((v) => { + expect(v.response?.location ?? '').to.match(/\/login(\?|$)/); + }); + }); + + it('bounces when the matching session has EXPIRED (present is not live)', () => { + callService({ + fn: 'loginMethodLoader', + seed: SOLE_IDP_SEED, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [{ ...MIA_SESSION, expirationTs: '2000-01-01T00:00:00.000Z' }], + }, + }).then((v) => { + expect(v.response?.location ?? '').to.match(/\/login(\?|$)/); + }); + }); + + it('accepts a session whose loginName differs only in CASE', () => { + // The URL carries the provider's canonical loginName; a hand-typed or IdP-returned + // identifier may differ in case. An exact compare would lock out a legitimate arrival. + callService({ + fn: 'loginMethodLoader', + seed: { + users: [{ id: 'u1', loginName: 'mia@acme.test' }], + authMethods: { u1: ['password'] }, + }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [{ ...MIA_SESSION, loginName: 'MIA@Acme.Test' }], + }, + }).then((v) => { + expect(v.response?.status).to.not.equal(302); + expect((v.response?.dataBody as { methods: string[] }).methods).to.deep.equal(['password']); + }); + }); + + it('threads requestId + organization onto the bounce so the ceremony survives', () => { + callService({ + fn: 'loginMethodLoader', + seed: SOLE_IDP_SEED, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test&requestId=oidc_1&organization=org-1', + }, + }).then((v) => { + const loc = v.response?.location ?? ''; + expect(loc).to.contain('requestId=oidc_1'); + expect(loc).to.contain('organization=org-1'); + }); + }); +}); + +describe('login action — retires the one-shot auto-start marker', () => { + it('expires idp-autostart so the NEXT ceremony can auto-start again', () => { + // The marker suppresses a second auto-start for the same loginName, which is what stops the + // Back-from-the-provider arrival re-minting an intent. Its 10-minute maxAge outlived the + // ceremony that wrote it, so a sign-out/sign-in inside that window got the one-button chooser + // instead of the auto-start. The identifier submit IS a new ceremony, so it clears the marker + // — Back never re-POSTs here, so the guard the marker provides is untouched. + callService({ + fn: 'loginAction', + seed: { + users: [{ id: 'u1', loginName: 'alice@acme.test' }], + authMethods: { u1: ['password'] }, + }, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login', + form: { loginName: 'alice@acme.test' }, + csrf: true, + idpAutostart: 'alice@acme.test', + }, + }).then((v) => { + const cleared = (v.response?.setCookies ?? []).find((c) => c.startsWith('idp-autostart=')); + expect(cleared, 'the action emits an idp-autostart cookie').to.be.a('string'); + // Expiry, not a rewrite: Max-Age=0 is what actually retires it in the browser. + expect(cleared ?? '', 'expired, not re-armed').to.contain('Max-Age=0'); + }); + }); +}); + +describe('/login/method loader — ONE-SHOT sole-IdP auto-start', () => { + // The auto-start lives in a LOADER, which re-runs on every arrival at the URL — including the + // one the browser makes when the user presses Back at the provider. Unguarded that Back mints + // a NEW intent and bounces them straight forward again: they can never return to the app. + it('FIRST arrival still auto-starts, and marks the browser', () => { + callService({ + fn: 'loginMethodLoader', + seed: SOLE_IDP_SEED, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + }, + }).then((v) => { + expect(v.response?.status).to.equal(302); + expect(v.response?.location ?? '').to.contain('idp-google'); + expect(v.response?.setCookie ?? '', 'writes the one-shot marker').to.contain('idp-autostart'); + }); + }); + + it('SECOND arrival for the same account renders the chooser instead of re-minting', () => { + callService({ + fn: 'loginMethodLoader', + seed: SOLE_IDP_SEED, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + idpAutostart: 'mia@acme.test', + }, + recordCalls: ['startIdpIntent'], + }).then((v) => { + expect(v.response?.status, 'must not redirect to the provider again').to.not.equal(302); + expect(v.calls?.startIdpIntent, 'no new Zitadel intent minted').to.have.length(0); + const body = v.response?.dataBody as { methods: string[]; idps: Array<{ id: string }> }; + expect(body.methods).to.deep.equal(['idp']); + expect(body.idps, 'the chooser offers the provider as a button').to.have.length(1); + }); + }); + + it('a marker for a DIFFERENT account does not suppress this one', () => { + callService({ + fn: 'loginMethodLoader', + seed: SOLE_IDP_SEED, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + idpAutostart: 'someone-else@acme.test', + }, + }).then((v) => { + expect(v.response?.status).to.equal(302); + expect(v.response?.location ?? '').to.contain('idp-google'); + }); + }); + + it("gates on the USER's own org policy, not the default org's, with no ?organization", () => { + // resolveIdentifier decides with `org ?? user.orgId` (login.service.ts) — the org the found + // user actually belongs to — and it is what routed the user here. This loader must gate on + // the same policy. When it resolved settings default-org-first instead, any user outside the + // default org signing in without an explicit ?organization got their method approved by one + // policy and then computed away by another: available=[] and a bounce to /error, on the most + // travelled path in the product. Here the default org forbids passwords and mia's org does + // not, so reading the wrong one is a redirect instead of a render. + callService({ + fn: 'loginMethodLoader', + seed: { + users: [{ id: 'u1', loginName: 'mia@acme.test', orgId: 'org-mia' }], + authMethods: { u1: ['password'] }, + settingsByOrg: { 'org-default-fake': { allowPassword: false } }, + }, + env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + }, + }).then((v) => { + expect(v.response?.status, 'must not bounce to /error').to.not.equal(302); + const body = v.response?.dataBody as { methods: string[] }; + expect(body.methods).to.deep.equal(['password']); }); }); @@ -58,7 +375,10 @@ describe('/login/method loader', () => { idpLinks: { u1: [{ idpId: 'idp-google', idpUserId: 'g-1' }] }, }, env: { AUTH_EMAIL_DELIVERY_ENABLED: 'true' }, - request: { url: 'http://localhost/id/login/method?loginName=mia%40acme.test' }, + request: { + url: 'http://localhost/id/login/method?loginName=mia%40acme.test', + sessions: [MIA_SESSION], + }, }).then((v) => { const body = v.response?.dataBody as { methods: string[]; idps: Array<{ id: string }> }; expect(body.methods).to.include('idp'); diff --git a/cypress/component/routes/login/method.cy.tsx b/cypress/component/routes/login/method.cy.tsx index 1786d9c541..8eb7a4bc73 100644 --- a/cypress/component/routes/login/method.cy.tsx +++ b/cypress/component/routes/login/method.cy.tsx @@ -1,10 +1,11 @@ // cypress/component/routes/login/method.cy.tsx // // UI contract for /login/method (A-P10): identity header ("Signing in as " / -// "Not you?") and the Passkey entry firing usePasskeyLoginCeremony IN PLACE (a Button -// that lazily loads the /login/passkey challenge and submits the pre-baked Cypress -// credential) instead of navigating there. The other method entries stay plain links. -import LoginMethod from '@/routes/login/method'; +// "Not you?") and the Passkey entry firing usePasskeyLoginCeremony IN PLACE — a real to +// /login/passkey whose click JS intercepts to lazily load the challenge and submit the pre-baked +// Cypress credential, so an unhydrated visitor simply follows the href instead. Every method +// entry is a link; only the IdP rows are submit buttons (starting an intent needs an action). +import LoginMethod, { shouldRevalidate } from '@/routes/login/method'; import { ConformAdapter } from '@datum-cloud/datum-ui/form/adapters/conform'; import { setupI18n } from '@lingui/core'; import { I18nProvider } from '@lingui/react'; @@ -24,6 +25,10 @@ const METHOD_LOADER_DATA = { }; const capturedPosts: Array> = []; +// Every challenge fetch the ceremony makes. Used as the sync point for the "did the +// auto-begun attempt actually run?" assertions below — a ceremony that fails at the +// challenge produces no POST, so capturedPosts cannot serve as the observable there. +const challengeLoads: string[] = []; function withI18n(node: React.ReactNode) { const i18n = setupI18n({ locale: 'en', messages: { en: {} } }); @@ -34,19 +39,35 @@ function withI18n(node: React.ReactNode) { ); } -function mountMethod() { +// Optional overrides let sole-passkey tests exercise a different `methods` array and +// `loginName` without duplicating the router wiring — same override-and-spread idiom +// passkey-button-visibility.cy.tsx's mountLogin() already uses for this same route family. +function mountMethod(opts?: { + methods?: Array<'passkey' | 'password' | 'otp_email' | 'idp'>; + loginName?: string; + // A null challenge is the deterministic failure seam: the hook treats a missing + // publicKeyCredentialRequestOptions as a non-fatal mint failure and sets reason='unknown' + // WITHOUT touching navigator.credentials, so a spec can exercise the failure copy without + // depending on the browser's real WebAuthn behavior. + challenge?: unknown; +}) { + const loginContext = { ...LOGIN_CONTEXT, loginName: opts?.loginName ?? LOGIN_CONTEXT.loginName }; + const methodLoaderData = { + ...METHOD_LOADER_DATA, + methods: opts?.methods ?? METHOD_LOADER_DATA.methods, + }; const router = createMemoryRouter( [ { id: 'login', path: '/login', - loader: () => LOGIN_CONTEXT, + loader: () => loginContext, children: [ { id: 'method', path: 'method', element: , - loader: async () => METHOD_LOADER_DATA, + loader: async () => methodLoaderData, }, // Stub /login/passkey — same route the shared ceremony hook lazily loads // (challenge) then posts to (credential). Mirrors passkeys-ui.cy.tsx's @@ -54,13 +75,17 @@ function mountMethod() { { id: 'passkey', path: 'passkey', - loader: async () => ({ - csrfToken: 'tok-1', - loginName: 'mia@acme.test', - requestId: undefined, - organization: undefined, - publicKeyCredentialRequestOptions: { publicKey: { challenge: 'x' } }, - }), + loader: async () => { + challengeLoads.push(loginContext.loginName); + return { + csrfToken: 'tok-1', + loginName: loginContext.loginName, + requestId: undefined, + organization: undefined, + publicKeyCredentialRequestOptions: + opts && 'challenge' in opts ? opts.challenge : { publicKey: { challenge: 'x' } }, + }; + }, action: async ({ request }: { request: Request }) => { capturedPosts.push(Object.fromEntries(await request.formData())); // Truthy (not null) — mirrors a real completed action so the ceremony's @@ -73,15 +98,25 @@ function mountMethod() { }, ], { - initialEntries: ['/login/method?loginName=mia%40acme.test'], + initialEntries: [`/login/method?loginName=${encodeURIComponent(loginContext.loginName)}`], hydrationData: { - loaderData: { login: LOGIN_CONTEXT, method: METHOD_LOADER_DATA }, + loaderData: { login: loginContext, method: methodLoaderData }, }, } ); return mount(withI18n()); } +// Same observable the neighbouring "Passkey fires the ceremony in place" test above uses to +// prove a ceremony ran: the captured POST to the /login/passkey stub action. Asserting on +// this (not on ceremony.phase / button-disabled) avoids racing the Cypress fake-credential +// seam, which resolves the ceremony without ever showing a real dialog. +function expectCeremonyStarted(times: number) { + cy.wrap(null).should(() => { + expect(capturedPosts).to.have.length(times); + }); +} + describe('/login/method — identity header + in-place passkey ceremony', () => { beforeEach(() => { capturedPosts.length = 0; @@ -97,9 +132,21 @@ describe('/login/method — identity header + in-place passkey ceremony', () => .and('not.contain', 'loginName'); }); + it('Passkey is a REAL link to /login/passkey, so it works without hydration', () => { + // It was a