feat: passkey management, login polish, and two-step enrollment (Phase A) - #102
Conversation
🧪 Test Summary
|
🧪 Test Summary
|
🧪 Test Summary
|
🧪 Test Summary
|
c2cbad1 to
be70693
Compare
c7008a6 to
ec79c00
Compare
🧪 Test Summary
|
🧪 Test Summary
|
🧪 Test Summary
|
🧪 Test Summary
|
🧪 Test Summary
|
🧪 Test Summary
|
🧪 Test Summary
|
f855224 to
362836e
Compare
🧪 Test Summary
|
362836e to
8313a8d
Compare
🧪 Test Summary
|
🧪 Test Summary
|
3e2b5b0 to
7876927
Compare
🧪 Test Summary
|
🧪 Test Summary
|
|
@yahyafakhroji couple things from LLM but looks good! High1.
|
🧪 Test Summary
|
🧪 Test Summary
|
|
Nice work on this — the sudo model is coherent and the server-side enforcement points are in the right place. Another pass over the latest state; everything below is still open, and a few are defects that came in with the most recent fixes. High1. A null WebAuthn challenge deadlocks the whole chooserWhere: const consumedChallenge = useRef<unknown>(null); // initial value: null
...
if (consumedChallenge.current === publicKeyCredentialRequestOptions) return;
With a null challenge, the ref's initial
Fix: sentinel the ref so const UNSET = Symbol('unset');
const consumedChallenge = useRef<unknown>(UNSET);
...
if (consumedChallenge.current === options) return;
consumedChallenge.current = options;
if (!options) { setPhase('idle'); setReason('unknown'); return; } // renders WebAuthnReasonCopy2.
|
🧪 Test Summary
|
…-device sign-out Adds /id/passkeys (list, sudo-gated remove with a server-side last-method guard, post-removal "sign out other sessions" offer), two-step passkey enrollment (ceremony, then name the credential), created-at metadata on enrolled passkeys, and true cross-device sign-out (deletes the user's other provider-side sessions, not just the local cookie).
…gn-in ceremony
Introduces the shared IdentityBadge ("Logged in/Signing up as X — Not you?")
used consistently across login, signup, reauth, passkeys, and accounts;
consolidates Back-link and sign-out affordances that had drifted per-route;
and adds the in-place passkey sign-in ceremony (challenge → assertion →
submit without navigating away) reused by /login and /login/method.
…-session recovery Adds the in-place passkey ceremony to /reauth (mirrors the login-side one) with busy/disabled states across the chooser while it runs, and an active-identity badge with a Back-link fix. Also recovers from stale cross-browser session tokens (getSession/findUser throwing a non-transient ProviderError instead of returning null) by bouncing to /login instead of crashing, and standardizes "Not you?" wording across surfaces.
Adds a fourth reauth factor: re-verifying the active session via one of the user's linked (non-LDAP) external IdPs, mirroring the login-side IdP flow but pointed at dedicated /reauth/:provider/callback|error routes so it can never create a session or sign in as a different identity — it only verifies the idpIntent onto the EXISTING session.
/login/method rendered a generic UserCircle icon for every linked IdP instead of the provider's real logo. Adds the missing google.dark.png IdP icon asset and wires the method chooser to the same IdpIcon/logoUrl resolution the rest of the app already uses.
…6 areas - reauth: never trust a client-echoed absolute returnTo; re-resolve the configured default (admin console / Zitadel default / env default) server-side instead of silently collapsing to a hardcoded /passkeys. - login/method: the IdP entry now POSTs through an action (CSRF, server-side linked-provider re-check) and starts sign-in in place, instead of navigating straight to /sso with no verification. - webauthn: recover from stale cross-browser session tokens during enrollment (loader pre-check) the same way other surfaces do. - reauth: the idp-reauth action verifies the target IdP is actually linked to the current user before starting the OAuth round-trip. - passkeys: serialize concurrent removals (per-user in-process mutex) to close a TOCTOU race in the last-method guard — two simultaneous removes could otherwise both read "still have a backup" and both succeed. - reauth/fake-provider: corrected error copy and removePasskey fidelity (static seed authMethods now cleared consistently with the dynamic set).
- reauth: the idp-reauth action also re-resolves the configured default returnTo (same fix as the main performReauth path) instead of hardcoding /passkeys when the caller's returnTo doesn't validate. - passkeys: the last-method guard now gates by ACTUAL usable policy (passkeysType / allowExternalIdp / allowPassword / emailDeliveryEnabled), not just raw enrollment — an enrolled-but-policy-disabled method is not a real backup. - auth: isStaleSessionError flipped from a fail-open denylist (assume stale unless the code is on a known-transient allowlist) to a fail-closed allowlist (assume the error propagates unless the code is a confirmed-stale one) — safer default when a new ProviderErrorCode is added later.
- Both passkey ceremony hooks (login + reauth) used `null` as the ref
sentinel for "already consumed this challenge", but a challenge-mint
failure also legitimately returns null — on first render this made the
ref's initial value match, so the hook returned before setPhase('ceremony')
ever ran and stayed stuck on 'loading-challenge' forever, disabling every
other sign-in method on /login/method and /reauth. Fixed with a Symbol
sentinel and an explicit null-challenge branch.
- /id/reauth, /id/passkeys, and /id/login/method had no rate limiting despite
sitting beside ten throttled siblings — added three ip-keyed limiters.
- performReauth's updateSession catch rethrew a stale cross-browser session
token uncaught (500), including mid-OAuth at /reauth/:provider/callback;
now classified via isStaleSessionError → SESSION_EXPIRED. Its returnTo
fallback also used the PRE-rotation session token for the isInstanceAdmin
check, silently misclassifying admins after a token rotation — now uses
the just-rotated token. Also threads a best-effort login_hint into the
reauth IdP round-trip, matching the login-side behavior.
- passkeys: the last-method guard counted raw passkey rows including inactive ones, so a passkey-only account with 1 active + 1 inactive passkey slipped past the <= 1 check and could be locked out. Now filters to state === 'active' first. - login/method: collapsed the unknown-user and idp-not-linked action responses into an identical one — the action is unauthenticated and the two previously-distinguishable outcomes made it a linked-IdP identity oracle. - setup/passkey: a failed step-2 (Save) submit silently reset to step 1, but the WebAuthn credential was already created in step 1 — retrying silently created a second, orphaned credential. Now shows a persistent notice instead of silently duplicating. - Misc: translated the Remove-passkey aria-label/title, dropped the unused userId field from the passkeys view payload, threaded the page's own external returnTo through the Add-passkey round-trip, SignOutButton now uses APP_BASENAME instead of a hardcoded literal, and aaguid.ts's CBOR length reader now forces an unsigned 32-bit result.
4d59be7 to
721e71c
Compare
|
@mattdjenkinson @gaghan430 — this is ready for another pass. Since your last reviews:
Let me know if anything still looks off or if you'd like any of the previously-deferred lower-priority items (e.g. merging the two passkey-ceremony hooks, the hydration/client-hints date formatting) picked up as follow-ups. |
🧪 Test Summary
|
3 participants
What
Passkey support for the identity UI: sign-in with a passkey plus a full self-service management surface.
/id/passkeysmanagement page: list, add, and remove passkeys, with a confirm dialog, designed empty state, and a one-time "Passkey added." notice/id/reauthinterstitialTesting
Cypress component suite (fake provider) plus request-driven e2e. Manual verification steps against a real Zitadel live in
acceptance/PASSKEY-TESTING.md.