diff --git a/app/hooks/use-conditional-passkey.ts b/app/hooks/use-conditional-passkey.ts index ac0e4f0dc..343777c92 100644 --- a/app/hooks/use-conditional-passkey.ts +++ b/app/hooks/use-conditional-passkey.ts @@ -177,6 +177,38 @@ export function useConditionalPasskey(input: ConditionalPasskeyInput) { body: new URLSearchParams({ csrf: csrfToken, credential: JSON.stringify(credential) }), }); if (abortedRef.current) return; + // 409 = the tapped credential belongs to an account this browser is ALREADY + // signed into. Not a failure: send the user there rather than showing ceremony + // error copy for a session that already exists. Full navigation, not RR + // routing — the destination resolves the session server-side. + if (res.status === 409) { + const body = (await res.json().catch(() => null)) as { loginName?: string } | null; + if (abortedRef.current) return; + setPhase('done'); + if (typeof body?.loginName === 'string' && body.loginName.length > 0) { + // Land on the ACCOUNTS PICKER, not /signed-in. /signed-in resolves + // mostRecent(sessions) — the most-recently-active account, not necessarily the + // one whose passkey was just tapped — so with two live sessions this could sign + // the user into the WRONG one. The picker's switchAccount (session.service.ts) + // validates the tapped session provider-side, resolves the continuation via + // resolveNextPath(..., { requestId }), and promotes it to most-recent — the + // existing, correct idiom for "continue as a specific already-live account". + // Threading requestId/organization (rather than dropping them, as /signed-in + // would) keeps a mid-ceremony ?add=1 arrival able to hand back to the + // OIDC/SAML/device callback the picker resolves; with none, it resolves the + // same default destination /signed-in would have. + window.location.assign( + `${APP_BASENAME}${paths.accounts({ + requestId: input.requestId, + organization: input.organization, + })}` + ); + return; + } + // Contract violation — never leave an explicit click with no outcome. + if (explicitRef.current) setReason('unknown'); + return; + } if (!res.ok) { // Opaque 400 (unknown user, no passkey, mint failure). Ambient flow: designed // non-event. Explicit (button) flow: the user acted, so say something — the diff --git a/app/modules/i18n/locales/en.po b/app/modules/i18n/locales/en.po index 8fd8a8b5b..4bc049f5f 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:474 +#: app/routes/login/index.tsx:424 msgid "An account with this email already exists — sign in to continue." msgstr "An account with this email already exists — sign in to continue." @@ -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:470 +#: app/routes/login/index.tsx:420 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:594 +#: app/routes/login/index.tsx:546 #: 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:639 +#: app/routes/login/index.tsx:591 #: 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:427 -#: app/routes/login/index.tsx:442 +#: app/routes/login/index.tsx:377 +#: app/routes/login/index.tsx:392 #: app/routes/signup/index.tsx:250 #: app/routes/signup/index.tsx:275 msgid "Email" @@ -283,8 +283,8 @@ msgstr "Email code" msgid "Email me a code" msgstr "Email me a code" -#: app/routes/login/index.tsx:605 -#: app/routes/login/index.tsx:615 +#: app/routes/login/index.tsx:557 +#: app/routes/login/index.tsx:567 #: app/routes/login/method.tsx:238 #: app/routes/signup/method.tsx:339 msgid "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:426 +#: app/routes/login/index.tsx:376 msgid "Email, phone, or username" msgstr "Email, phone, or username" @@ -446,7 +446,7 @@ msgstr "No signed-in accounts." msgid "Not now" msgstr "Not now" -#: app/routes/login/index.tsx:637 +#: app/routes/login/index.tsx:589 msgid "Not registered?" msgstr "Not registered?" @@ -472,7 +472,7 @@ 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:535 +#: app/routes/login/index.tsx:487 #: app/routes/login/method.tsx:221 #: app/routes/reauth.tsx:310 #: app/routes/setup/mfa.tsx:46 @@ -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:429 -#: app/routes/login/index.tsx:444 +#: app/routes/login/index.tsx:379 +#: app/routes/login/index.tsx:394 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:627 +#: app/routes/login/index.tsx:579 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." @@ -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:430 -#: app/routes/login/index.tsx:445 +#: app/routes/login/index.tsx:380 +#: app/routes/login/index.tsx:395 #: 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:467 +#: app/routes/login/index.tsx:417 msgid "Welcome" msgstr "Welcome" diff --git a/app/resources/webauthn/arm-login-passkey.ts b/app/resources/webauthn/arm-login-passkey.ts new file mode 100644 index 000000000..01e929846 --- /dev/null +++ b/app/resources/webauthn/arm-login-passkey.ts @@ -0,0 +1,96 @@ +// app/resources/webauthn/arm-login-passkey.ts +// +// The /login loader's passkey arming decision, extracted whole. +// +// Two arms, tried in order rather than as alternatives. The user-bound arm needs a +// resolvable identity and a session the provider will accept; the discovery arm needs +// neither (mintIdentityChallenge is SELF-issued — no provider round-trip, nothing +// persisted), so it can catch every decline the first arm produces. +// +// They were previously mutually exclusive (`if (hint) … else if (!hint) …`), which meant a +// declined hint arm dead-ended on the email field even though discovery would have worked. +// Combined with a zero-live-sessions guard on discovery, that made the Passkey button +// unusable for the entire add-another-account population. +import { mintIdentityChallenge } from './identity-challenge'; +import { armUserBoundChallenge } from './webauthn.service'; +import type { AuthProvider } from '@/modules/auth/auth-provider'; +import { listSessions, type SessionEntry } from '@/modules/auth/session/cookie'; + +export interface LoginPasskeyArming { + conditionalPasskey: { loginName: string; publicKeyCredentialRequestOptions: unknown } | null; + identityDiscovery: { publicKeyCredentialRequestOptions: unknown } | null; + /** Set-Cookie values the caller must append (ceremony session + fingerprint). */ + setCookies: string[]; + /** The hint named a user that can never fire — caller should clear the cookie. */ + clearHint: boolean; +} + +export interface ArmLoginPasskeyInput { + /** Signed passkey-hint cookie value, or null. */ + hint: string | null; + /** `?add=1` — the user explicitly wants a DIFFERENT identity. */ + isAddAccount: boolean; + sessions: SessionEntry[]; + /** Request hostname — the FIDO2 relying-party id. */ + hostname: string; + /** AUTH_PASSKEY_DISCOVERY_ENABLED — operational kill switch, default ON. */ + discoveryEnabled: boolean; +} + +export async function armLoginPasskey( + provider: AuthProvider, + request: Request, + { hint, isAddAccount, sessions, hostname, discoveryEnabled }: ArmLoginPasskeyInput +): Promise { + let conditionalPasskey: LoginPasskeyArming['conditionalPasskey'] = null; + const setCookies: string[] = []; + let clearHint = false; + + // ── Arm 1: user-bound, from the hint ──────────────────────────────────────── + // Skipped under ?add=1: the hint names the account the user ALREADY holds, so arming + // it would sign them back into it — the opposite of "add another account". + if (hint && !isAddAccount) { + // LIVE session, not merely a cookie entry: armUserBoundChallenge's caller contract + // requires this, because its same-loginName supersede is safe only against dead + // entries. listSessions is the codebase's expiry-aware filter. + const hasLiveSession = listSessions(sessions, Date.now()).some( + (s) => s.loginName.toLowerCase() === hint.toLowerCase() + ); + if (!hasLiveSession) { + const user = await provider.findUser(hint); + if (!user) { + // Deleted/renamed user — the hint can never fire; drop it. + clearHint = true; + } else if ((await provider.listAuthMethods(user.id)).includes('passkey')) { + try { + const armed = await armUserBoundChallenge(provider, request, sessions, user, hostname); + if (armed) { + setCookies.push(...armed.setCookies); + conditionalPasskey = { + loginName: armed.loginName, + publicKeyCredentialRequestOptions: armed.publicKeyCredentialRequestOptions, + }; + } + } catch { + // Session creation failed (deactivated user, provider hiccup) — clear the hint + // and let the discovery arm below catch it. + clearHint = true; + } + } + } + } + + // ── Arm 2: discovery, catching every decline above ────────────────────────── + // Free by design: self-minted options, no provider call, nothing persisted. Zitadel + // enters only at /login/passkey-discover, after a credential has actually been tapped. + // Deliberately NOT gated on live sessions: a signed-in visitor adding a second account + // is exactly the population this serves. /login/passkey-discover keeps its own + // per-user guard, so the loader decides what to OFFER and the action enforces what is + // ALLOWED. + const identityDiscovery = + !conditionalPasskey && discoveryEnabled + ? { publicKeyCredentialRequestOptions: mintIdentityChallenge(hostname) } + : null; + + return { conditionalPasskey, identityDiscovery, setCookies, clearHint }; +} diff --git a/app/routes/login/index.tsx b/app/routes/login/index.tsx index adfd6dac6..6760436b2 100644 --- a/app/routes/login/index.tsx +++ b/app/routes/login/index.tsx @@ -14,7 +14,7 @@ import { idpTypeToSlug } from '@/modules/auth/idp-slug'; // ADAPTATION (plan-drift fix): readSessions + serializeSessions live in @/modules/auth/session/cookie. // 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, listSessions } from '@/modules/auth/session/cookie'; +import { readSessions, serializeSessions } from '@/modules/auth/session/cookie'; 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'; @@ -28,8 +28,7 @@ import { } from '@/resources/login/login.schema'; import { resolveOrg } from '@/resources/shared/resolve-org'; import { getActiveIdPs } from '@/resources/sso/idp-providers'; -import { mintIdentityChallenge } from '@/resources/webauthn/identity-challenge'; -import { armUserBoundChallenge } from '@/resources/webauthn/webauthn.service'; +import { armLoginPasskey } from '@/resources/webauthn/arm-login-passkey'; import { paths } from '@/routes/paths'; import { providerForRequest } from '@/server/auth-context.server'; import { loaderCsrf, assertCsrf } from '@/server/csrf'; @@ -86,76 +85,27 @@ export async function loader({ request }: LoaderFunctionArgs) { const notice = url.searchParams.get('notice') ?? undefined; // ── Usernameless fast path: arm a conditional-mediation passkey ceremony ──── - // A hint is an inference; arm ONLY when nothing more specific is known. Explicit - // suppression list: ?add=1 (user asked for a different - // account), hinted user already live (nothing to log in), unresolvable user (clear the - // stale hint), no passkey method. Every suppression — and every mint failure — renders - // the ordinary page; arming is invisible either way. + // Two arms tried as a CASCADE (armLoginPasskey), not mutually-exclusive alternatives. + // The first arm binds a pre-minted, user-bound challenge to the passkey hint — but only + // when nothing more specific is known: it declines under ?add=1 (user asked for a + // different account), a hinted user already live (nothing to log in), an unresolvable + // user (the hint is cleared), or no passkey method. Every decline of the first arm falls + // through to the second: usernameless identity discovery, which serves ?add=1 and a live + // session just as well as a cold browser (gated only by the AUTH_PASSKEY_DISCOVERY_ENABLED + // kill switch). Every failure of both arms — and every mint failure — renders the + // ordinary page; arming is invisible either way. const responseHeaders = new Headers(headers); - let conditionalPasskey: { - loginName: string; - publicKeyCredentialRequestOptions: unknown; - } | null = null; - let identityDiscovery: { publicKeyCredentialRequestOptions: unknown } | null = null; const hint = await readPasskeyHint(request); - const isAddAccount = url.searchParams.get('add') === '1'; - if (hint && !isAddAccount) { - const sessions = await readSessions(request); - // LIVE session, not just any cookie entry: raw readSessions() output can carry stale - // (expired) entries, and a stale entry must not suppress the fast path — the - // suppression criterion is a LIVE session. listSessions is the codebase's expiry-aware - // filter (same usage as session.service.ts); unknown expiry counts as live. - const hasLiveSession = listSessions(sessions, Date.now()).some( - (s) => s.loginName.toLowerCase() === hint.toLowerCase() - ); - if (!hasLiveSession) { - const user = await provider.findUser(hint); - if (!user) { - // Deleted/renamed user — the hint can never fire; drop it so we stop re-checking. - responseHeaders.append('set-cookie', await clearPasskeyHint()); - } else if ((await provider.listAuthMethods(user.id)).includes('passkey')) { - try { - // Mirror resolveIdentifier's known-user session mint, then persist the entry so - // the /login/passkey verify action can resolve it by loginName. The loader-side - // Set-Cookie is the accepted side effect. - // `hasLiveSession` above satisfies armUserBoundChallenge's caller contract - // (its same-loginName supersede is only safe against dead entries). - const armed = await armUserBoundChallenge( - provider, - request, - sessions, - user, - url.hostname - ); - if (armed) { - for (const cookie of armed.setCookies) responseHeaders.append('set-cookie', cookie); - conditionalPasskey = { - loginName: armed.loginName, - publicKeyCredentialRequestOptions: armed.publicKeyCredentialRequestOptions, - }; - } - } catch { - // Session creation failed (deactivated user, provider hiccup) — - // clear the hint, render normally. - responseHeaders.append('set-cookie', await clearPasskeyHint()); - } - } - } - } else if (!hint && !isAddAccount) { - // ── Discovery arm ───────────────────────────────────────────────────────── - // Hintless visitors get a SELF-MINTED challenge: no Zitadel call, nothing - // persisted — the identity tap posts to /login/passkey-discover, which mints - // the real user-bound challenge only after a passkey was actually tapped. - // Suppressed when ANY live session exists (arming is inference; a logged-in - // visitor is better served by the ordinary page) and - // by the operational kill switch (env, default ON — incident mitigation). - const sessions = await readSessions(request); - if (env.AUTH_PASSKEY_DISCOVERY_ENABLED && listSessions(sessions, Date.now()).length === 0) { - identityDiscovery = { - publicKeyCredentialRequestOptions: mintIdentityChallenge(url.hostname), - }; - } - } + const arming = await armLoginPasskey(provider, request, { + hint, + isAddAccount: url.searchParams.get('add') === '1', + sessions: await readSessions(request), + hostname: url.hostname, + discoveryEnabled: env.AUTH_PASSKEY_DISCOVERY_ENABLED, + }); + for (const cookie of arming.setCookies) responseHeaders.append('set-cookie', cookie); + if (arming.clearHint) responseHeaders.append('set-cookie', await clearPasskeyHint()); + const { conditionalPasskey, identityDiscovery } = arming; return data( { @@ -521,8 +471,10 @@ export default function Login() { onClick={() => { // No resolvable identity — run the discovery ceremony MODALLY: the browser's native picker over the loader's // self-minted challenge, then the discover → verify pipeline. Fall back to - // the identifier step only when discovery can't start (not armed — e.g. - // ?add=1 or a live session — or WebAuthn unsupported). + // the identifier step only when discovery can't start — the + // AUTH_PASSKEY_DISCOVERY_ENABLED kill switch is off, or WebAuthn is + // unsupported (?add=1 and a live session no longer suppress discovery; the + // loader's cascade arms it for both). if (!passkeyIdentity) { if (!conditional.beginDiscovery()) setShowEmailField(true); return; diff --git a/app/routes/login/passkey-discover.tsx b/app/routes/login/passkey-discover.tsx index 8fc5a9e02..fbe881f01 100644 --- a/app/routes/login/passkey-discover.tsx +++ b/app/routes/login/passkey-discover.tsx @@ -83,15 +83,30 @@ export async function action({ request }: ActionFunctionArgs) { const user = await provider.getUser(userHandle); if (!user) return opaque('unresolved_user'); - // Cheap LOCAL guard before the second provider round-trip: armUserBoundChallenge's - // caller contract + crafted-POST protection. The loader suppresses discovery - // whenever a live session exists, so a live entry here means the POST bypassed - // the page. Refuse rather than let the arm supersede a LIVE cookie entry. + // Per-user guard. This endpoint decides what is ALLOWED independently of what the + // /login loader chose to OFFER, so it stays correct against a crafted POST — and the + // loader now arms discovery while a session is live, so a live entry here is ordinary + // rather than evidence the page was bypassed. + // + // armUserBoundChallenge's caller contract forbids superseding a LIVE cookie entry, so + // this case cannot proceed. It is NOT a failure though: the user tapped the passkey of + // an account they already hold, so say so and let the client route there. Not an + // enumeration leak — reaching this branch requires passing assertCsrf AND holding a + // live signed sessions cookie for this exact user, which reveals nothing /accounts + // does not already show. (The assertion signature is never verified here, so a forged + // userHandle is assumed; the guard keys on THIS browser's sessions, which a forger + // cannot influence.) const sessions = await readSessions(request); const hasLiveSession = listSessions(sessions, Date.now()).some( (s) => s.loginName.toLowerCase() === user.loginName.toLowerCase() ); - if (hasLiveSession) return opaque('live_session'); + if (hasLiveSession) { + logAuthEvent('passkey_discover', 'failure', { reason: 'already_signed_in' }); + return Response.json( + { error: 'ALREADY_SIGNED_IN', loginName: user.loginName }, + { status: 409 } + ); + } if (!(await provider.listAuthMethods(user.id)).includes('passkey')) { return opaque('no_passkey_method'); diff --git a/cypress/component/routes/login/conditional-passkey-loader.cy.ts b/cypress/component/routes/login/conditional-passkey-loader.cy.ts index 6e3d647ab..905ea1086 100644 --- a/cypress/component/routes/login/conditional-passkey-loader.cy.ts +++ b/cypress/component/routes/login/conditional-passkey-loader.cy.ts @@ -10,10 +10,11 @@ const PK_USER = 'passkey-user@acme.test'; const URL_BASE = 'http://localhost/id/login?organization=org1'; type LoaderBody = { conditionalPasskey?: { loginName?: string; publicKeyCredentialRequestOptions?: unknown } | null; + identityDiscovery?: { publicKeyCredentialRequestOptions?: unknown } | null; }; describe('/login loader — conditional passkey arming', () => { - it('no hint → arms nothing', () => { + it('no hint → the HINT arm stays inert (discovery arming is covered in discovery-loader.cy.ts)', () => { callService({ fn: 'loginLoader', provider: 'singleton', request: { url: URL_BASE } }).then( (v) => { expect((v.response?.dataBody as LoaderBody).conditionalPasskey).to.equal(null); @@ -36,30 +37,41 @@ describe('/login loader — conditional passkey arming', () => { }); }); - it('?add=1 (add-another-account arrival) suppresses arming', () => { + it('?add=1 suppresses the HINT arm but falls through to discovery', () => { callService({ fn: 'loginLoader', provider: 'singleton', request: { url: `${URL_BASE}&add=1`, passkeyHint: PK_USER }, }).then((v) => { - expect((v.response?.dataBody as LoaderBody).conditionalPasskey).to.equal(null); + const body = v.response?.dataBody as LoaderBody; + expect(body.conditionalPasskey).to.equal(null); + expect(body.identityDiscovery?.publicKeyCredentialRequestOptions, 'discovery armed').to.exist; + // Discovery is free — self-minted options, no Zitadel session, so still no cookie. expect( (v.response?.dataSetCookies ?? []).some((c: string) => c.startsWith('sessions=')) ).to.equal(false); }); }); - it('hinted user already has a live session → suppresses arming', () => { + it('hinted user WITH a live session falls through to discovery instead of dead-ending', () => { callService({ fn: 'loginLoader', provider: 'singleton', + liveSessions: [{ id: 's5', token: 't5', user: { id: 'u5', loginName: PK_USER } }], request: { url: URL_BASE, passkeyHint: PK_USER, sessions: [{ id: 's5', token: 't5', loginName: PK_USER }], }, }).then((v) => { - expect((v.response?.dataBody as LoaderBody).conditionalPasskey).to.equal(null); + const body = v.response?.dataBody as LoaderBody; + // Hint arm still declines — armUserBoundChallenge must not supersede a LIVE entry. + expect(body.conditionalPasskey).to.equal(null); + // ...but discovery now catches the decline. This is the bug being fixed. + expect( + body.identityDiscovery?.publicKeyCredentialRequestOptions, + 'discovery caught the declined hint arm' + ).to.exist; }); }); @@ -110,7 +122,10 @@ describe('/login loader — conditional passkey arming', () => { provider: 'singleton', request: { url: URL_BASE, passkeyHint: 'ghost@acme.test' }, }).then((v) => { - expect((v.response?.dataBody as LoaderBody).conditionalPasskey).to.equal(null); + const body = v.response?.dataBody as LoaderBody; + expect(body.conditionalPasskey).to.equal(null); + // The hint arm's decline falls through to the cascade's second arm. + expect(body.identityDiscovery?.publicKeyCredentialRequestOptions).to.exist; const cleared = (v.response?.dataSetCookies ?? []).find((c: string) => c.startsWith('passkey-hint=') ); @@ -124,7 +139,10 @@ describe('/login loader — conditional passkey arming', () => { provider: 'singleton', request: { url: URL_BASE, passkeyHint: 'alice@acme.test' }, }).then((v) => { - expect((v.response?.dataBody as LoaderBody).conditionalPasskey).to.equal(null); + const body = v.response?.dataBody as LoaderBody; + expect(body.conditionalPasskey).to.equal(null); + // The hint arm's decline falls through to the cascade's second arm. + expect(body.identityDiscovery?.publicKeyCredentialRequestOptions).to.exist; expect( (v.response?.dataSetCookies ?? []).some((c: string) => c.startsWith('passkey-hint=')) ).to.equal(false); diff --git a/cypress/component/routes/login/discovery-loader.cy.ts b/cypress/component/routes/login/discovery-loader.cy.ts index 1d553c0a6..c50864b2c 100644 --- a/cypress/component/routes/login/discovery-loader.cy.ts +++ b/cypress/component/routes/login/discovery-loader.cy.ts @@ -1,10 +1,11 @@ // cypress/component/routes/login/discovery-loader.cy.ts // // The /login loader's identity-discovery arming + suppression list, at the HTTP -// boundary. Discovery -// arms ONLY for the hintless population — and a discovery arm must be free: -// self-minted options, NO Zitadel session, NO Set-Cookie. Sibling of -// conditional-passkey-loader.cy.ts (the hinted path). +// boundary. Discovery is the SECOND arm of the loader's cascade (armLoginPasskey): it +// catches every decline of the hint-bound first arm — hintless, ?add=1, a live session, +// all of them — as long as the AUTH_PASSKEY_DISCOVERY_ENABLED kill switch is on. A +// discovery arm must be free: self-minted options, NO Zitadel session, NO Set-Cookie. +// Sibling of conditional-passkey-loader.cy.ts (the hinted path). import { callService } from '../../../support/node/call-service'; const PK_USER = 'passkey-user@acme.test'; // u5, authMethods ['password','passkey'] @@ -37,17 +38,21 @@ describe('/login loader — identity-discovery arming', () => { ); }); - it('?add=1 suppresses discovery (explicit intent)', () => { + it('?add=1 arms discovery — an explicit "different identity" is what discovery serves', () => { callService({ fn: 'loginLoader', provider: 'singleton', request: { url: `${URL_BASE}&add=1` }, }).then((v) => { - expect((v.response?.dataBody as LoaderBody).identityDiscovery).to.equal(null); + const body = v.response?.dataBody as LoaderBody; + expect(body.identityDiscovery?.publicKeyCredentialRequestOptions, 'discovery armed').to.exist; + // The HINT arm stays suppressed under ?add=1 — arming it would sign the user + // back into the account they already hold. + expect(body.conditionalPasskey).to.equal(null); }); }); - it('ANY live session suppresses discovery', () => { + it('a live session no longer suppresses discovery (the add-account population)', () => { callService({ fn: 'loginLoader', provider: 'singleton', @@ -57,7 +62,8 @@ describe('/login loader — identity-discovery arming', () => { sessions: [{ id: 's5', token: 't5', loginName: PK_USER }], }, }).then((v) => { - expect((v.response?.dataBody as LoaderBody).identityDiscovery).to.equal(null); + const body = v.response?.dataBody as LoaderBody; + expect(body.identityDiscovery?.publicKeyCredentialRequestOptions, 'discovery armed').to.exist; }); }); diff --git a/cypress/component/routes/login/passkey-button-visibility.cy.tsx b/cypress/component/routes/login/passkey-button-visibility.cy.tsx index 4899e9677..de3be6a3e 100644 --- a/cypress/component/routes/login/passkey-button-visibility.cy.tsx +++ b/cypress/component/routes/login/passkey-button-visibility.cy.tsx @@ -55,8 +55,9 @@ function mountLogin(opts?: { // Loader-resolved usernameless hint (Task 7's conditionalPasskey field). Null/undefined // mirrors a cold visit with no hint. conditionalPasskey?: { loginName: string; publicKeyCredentialRequestOptions: unknown } | null; - // Loader-armed identity discovery (fresh browser, no hint). Null/undefined mirrors a - // loader-SUPPRESSED visit (?add=1 / live session) where the button must fall back. + // Loader-armed identity discovery. Null/undefined mirrors the ONLY remaining unarmed + // state: the discovery kill switch (AUTH_PASSKEY_DISCOVERY_ENABLED=false). ?add=1 and + // live-session visits now arm discovery via the loader cascade. identityDiscovery?: { publicKeyCredentialRequestOptions: unknown } | null; }) { const loginContext = { ...LOGIN_CONTEXT, loginName: opts?.loginName ?? LOGIN_CONTEXT.loginName }; @@ -122,8 +123,8 @@ describe('/login Passkey button — visibility and identity binding', () => { .and('not.be.disabled'); }); - it('cold click with discovery UNARMED (loader-suppressed) falls back to the identifier field', () => { - // identityDiscovery null = the loader suppressed arming (?add=1 / live session). + it('cold click with discovery UNARMED (kill switch off) falls back to the identifier field', () => { + // identityDiscovery null = discovery kill switch is off (the only unarmed state left). // beginDiscovery has no options to run over → the identifier step is the fallback. mountLogin(); cy.contains('button', /passkey/i).click(); @@ -189,4 +190,59 @@ describe('/login Passkey button — visibility and identity binding', () => { }); }); }); + + it('a 409 with a malformed body (no loginName) surfaces error copy instead of a silent dead end', () => { + // NOTE: ordered BEFORE the successful-routing 409 test below. That test's + // window.location.assign() is a REAL navigation in this Cypress/Electron + // component runner (not a jsdom no-op), and it leaves the AUT on Cypress's + // "URL navigation disabled in component testing" page for any test that runs + // after it in the same spec — there's no per-`it()` page reset for component + // tests. Keeping the navigating test last avoids contaminating this one. + cy.intercept('POST', '**/id/login/passkey-discover', { + statusCode: 409, + body: { error: 'ALREADY_SIGNED_IN' }, // no loginName — contract violation + }).as('discover'); + mountLogin({ + identityDiscovery: { + publicKeyCredentialRequestOptions: { publicKey: { challenge: 'identity-x' } }, + }, + }); + cy.contains('button', /passkey/i).click(); + cy.wait('@discover'); + // No verify POST — the branch must not fall through to submit either. + cy.then(() => expect(capturedPosts).to.have.length(0)); + // The explicit click must not be left with zero visible outcome — falls back to + // the same 'unknown' copy the 200-path shape-validation branch uses. + cy.get('[role="alert"]').should('exist'); + }); + + it('a 409 ALREADY_SIGNED_IN routes to the accounts picker instead of reporting a failure', () => { + // NOTE: this environment (Cypress 15 component runner on Electron) cannot stub + // window.location.assign — both `cy.stub(win.location, 'assign')` and replacing + // `win.location` wholesale via Object.defineProperty throw "Cannot redefine + // property" (window.location is non-configurable here). So this test cannot + // assert the full-page navigation (to /accounts, carrying requestId/organization — + // NOT /signed-in, which would resolve mostRecent(sessions) rather than the tapped + // account) directly; it instead proves the negative that matters at this layer — + // the 409 branch must NOT fall through to the verify POST — and leaves the + // navigation assertion itself to the e2e coverage in cypress/e2e/passkey-conditional.cy.ts. + // ALSO: keep this test LAST in the file — see the note on the malformed-body + // test above (this one's assign() call really navigates the AUT). + cy.intercept('POST', '**/id/login/passkey-discover', { + statusCode: 409, + body: { error: 'ALREADY_SIGNED_IN', loginName: 'mia@acme.test' }, + }).as('discover'); + mountLogin({ + identityDiscovery: { + publicKeyCredentialRequestOptions: { publicKey: { challenge: 'identity-x' } }, + }, + }); + cy.contains('button', /passkey/i).click(); + cy.wait('@discover'); + // No verify POST — there is nothing to verify, the session already exists. + cy.then(() => expect(capturedPosts).to.have.length(0)); + // Nor should the ceremony-failure copy render — a 409 must not be swallowed by + // the opaque-400 `!res.ok` branch into a misleading "not-allowed" FormError. + cy.get('[role="alert"]').should('not.exist'); + }); }); diff --git a/cypress/component/routes/login/passkey-discover.cy.ts b/cypress/component/routes/login/passkey-discover.cy.ts index 38df64d95..0a16fa6f3 100644 --- a/cypress/component/routes/login/passkey-discover.cy.ts +++ b/cypress/component/routes/login/passkey-discover.cy.ts @@ -136,44 +136,49 @@ describe('/login/passkey-discover action', () => { }); }); - it('live session for the resolved user → opaque 400 (crafted-POST supersede guard)', () => { - // The loader suppresses discovery when a live session exists; a crafted POST must - // not bypass that and supersede a LIVE cookie entry via armUserBoundChallenge. + it('malformed credential JSON → opaque DISCOVERY_FAILED 400 (shape violations are non-events)', () => { callService({ fn: 'passkeyDiscoverAction', provider: 'singleton', - liveSessions: [{ id: 's5', token: 't5', user: { id: 'u5', loginName: PK_USER } }], - request: { - url: URL, - sessions: [{ id: 's5', token: 't5', loginName: PK_USER }], - form: { credential: assertionWith(B64_U5) }, - csrf: true, - }, + request: { url: URL, form: { credential: 'not-json{' }, csrf: true }, }).then((v) => { expect(v.response?.status).to.equal(400); expect((v.response?.dataBody as { error?: string }).error).to.equal('DISCOVERY_FAILED'); }); }); - it('malformed credential JSON → opaque DISCOVERY_FAILED 400 (shape violations are non-events)', () => { + it('missing credential field → INVALID_INPUT 400 (schema boundary, not user-dependent)', () => { callService({ fn: 'passkeyDiscoverAction', provider: 'singleton', - request: { url: URL, form: { credential: 'not-json{' }, csrf: true }, + request: { url: URL, form: {}, csrf: true }, }).then((v) => { expect(v.response?.status).to.equal(400); - expect((v.response?.dataBody as { error?: string }).error).to.equal('DISCOVERY_FAILED'); + expect((v.response?.dataBody as { error?: string }).error).to.equal('INVALID_INPUT'); }); }); +}); - it('missing credential field → INVALID_INPUT 400 (schema boundary, not user-dependent)', () => { +describe('passkey-discover — already signed in as the tapped account', () => { + // Reachable only since the loader cascade began arming discovery while a session is + // live. Previously an opaque 400, which read as "something went wrong" when the user + // had simply tapped the account they were already in. + it('returns 409 ALREADY_SIGNED_IN with the loginName, not the opaque 400', () => { callService({ fn: 'passkeyDiscoverAction', provider: 'singleton', - request: { url: URL, form: {}, csrf: true }, + liveSessions: [{ id: 's5', token: 't5', user: { id: 'u5', loginName: PK_USER } }], + request: { + url: URL, + sessions: [{ id: 's5', token: 't5', loginName: PK_USER }], + form: { credential: assertionWith(B64_U5) }, + csrf: true, + }, }).then((v) => { - expect(v.response?.status).to.equal(400); - expect((v.response?.dataBody as { error?: string }).error).to.equal('INVALID_INPUT'); + expect(v.response?.status).to.equal(409); + const body = v.response?.dataBody as { error?: string; loginName?: string }; + expect(body.error).to.equal('ALREADY_SIGNED_IN'); + expect(body.loginName).to.equal(PK_USER); }); }); }); diff --git a/cypress/e2e/passkey-conditional.cy.ts b/cypress/e2e/passkey-conditional.cy.ts index f14182cdf..540c58efd 100644 --- a/cypress/e2e/passkey-conditional.cy.ts +++ b/cypress/e2e/passkey-conditional.cy.ts @@ -86,12 +86,41 @@ describe('usernameless passkey fast path', () => { cy.getCookie('passkey-hint').should('exist'); }); - it('add-another-account arrival suppresses the fast path', () => { + it('add-another-account arrival keeps the hint dark but discovery still arms', () => { signInWithPassword(USER); cy.clearCookie('sessions'); visitLoginArmed('/id/login?add=1'); cy.settleHydration(); + // ?add=1 suppresses only the HINT arm — no zero-typing auto sign-in. cy.location('pathname').should('eq', '/id/login'); + // Discovery arms regardless of ?add=1 (the cascade) — the Passkey button still + // resolves the tapped credential's identity and signs in. + cy.contains('button', /passkey/i).click(); + cy.location('pathname').should('eq', '/id/signed-in'); + cy.contains(USER); + }); + + it("add-another-account with a LIVE session: tapping the signed-in account's own passkey routes to the accounts picker", () => { + // Distinct from the test above: THAT test clears the `sessions` cookie before + // visiting ?add=1, so it never reaches a live session and never reaches the 409 branch + // this covers. "Add another account" implies a session already exists — THIS test keeps + // it live. CYPRESS_CREDENTIAL's userHandle (base64url('u5')) resolves to USER, so tapping + // it while USER's own session is live is the reported bug's exact repro: the discover + // action's ALREADY_SIGNED_IN 409 branch must fire, and the client must land on the + // accounts picker (not /signed-in, which would resolve mostRecent() rather than the + // tapped account, and not the opaque-failure copy — the session already exists). + signInWithPassword(USER); + visitLoginArmed('/id/login?add=1'); + cy.settleHydration(); + cy.location('pathname').should('eq', '/id/login'); + cy.contains('button', /passkey/i).click(); + cy.location('pathname').should('eq', '/id/accounts'); + // The picker renders the seeded fake-provider displayName ("Passkey User"), not the + // raw loginName — same convention as accounts.tsx (`account.displayName ?? + // account.loginName`). "Session active" confirms it landed on USER's own live row, + // not an empty picker or a different account. + cy.contains('Passkey User'); + cy.contains('Session active'); }); it('an armed (un-resolved) ceremony never blocks the ordinary identifier flow', () => {