Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ node_modules
/cypress/screenshots
.claude/settings.local.json
.claude/settings.json
# Local-only Claude Code state (generated skills, worktree checkouts)
.claude/skills/
.claude/worktrees/
.superpowers/
app/modules/i18n/locales/*.ts
app/modules/i18n/locales/*.js
Expand Down
6 changes: 3 additions & 3 deletions app/modules/i18n/locales/en.po
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ msgid "Sign in"
msgstr "Sign in"

#: app/routes/login/password.tsx:195
#: app/routes/logout/success.tsx:27
#: app/routes/logout/success.tsx:57
#: app/utils/errors/auth-error-recovery.tsx:47
msgid "Sign in again"
msgstr "Sign in again"
Expand Down Expand Up @@ -995,7 +995,7 @@ msgstr "You signed in as a different account than the one you were re-authentica
msgid "You'll be asked to sign in before authorizing."
msgstr "You'll be asked to sign in before authorizing."

#: app/routes/logout/success.tsx:18
#: app/routes/logout/success.tsx:48
msgid "You've been signed out"
msgstr "You've been signed out"

Expand Down Expand Up @@ -1027,7 +1027,7 @@ msgstr "Your password has expired. Please reset it to continue."
msgid "Your previous attempt may have already created a passkey on this device. Retrying will register a new one — you can remove the unused entry later from your device or password manager."
msgstr "Your previous attempt may have already created a passkey on this device. Retrying will register a new one — you can remove the unused entry later from your device or password manager."

#: app/routes/logout/success.tsx:20
#: app/routes/logout/success.tsx:50
msgid "Your session has ended and you've been securely signed out of Datum. You can safely close this tab, or sign back in any time to pick up where you left off."
msgstr "Your session has ended and you've been securely signed out of Datum. You can safely close this tab, or sign back in any time to pick up where you left off."

Expand Down
13 changes: 10 additions & 3 deletions app/resources/webauthn/webauthn-verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,20 @@ export function createWebAuthnVerifyHandlers(cfg: WebAuthnVerifyConfig) {
userVerificationRequirement: cfg.userVerificationRequirement,
challengeAuditEvent: cfg.challengeAuditEvent,
},
{ loginName, requestId, organization, domain: url.hostname }
{ loginName, requestId, organization, domain: url.hostname },
// Opt in to the stale-session self-heal: a `sessions` cookie can outlive the session it
// names (OIDC logout, admin revoke, another browser), and without this the screen renders
// a null challenge that only surfaces as "verification failed" on click.
{ request }
);
if (result.kind === 'redirect') return redirect(result.target);

const [csrfToken, setCookie] = await getCsrfToken(request);
const headers: Record<string, string> = {};
if (setCookie !== null) headers['set-cookie'] = setCookie;
// Headers, not a plain record: a self-heal emits its own sessions/fingerprint cookies
// alongside the CSRF one, and multiple Set-Cookie values cannot be joined into one string.
const headers = new Headers();
if (setCookie !== null) headers.append('set-cookie', setCookie);
for (const cookie of result.setCookies ?? []) headers.append('set-cookie', cookie);

return data<WebAuthnVerifyLoaderData>(
{
Expand Down
86 changes: 83 additions & 3 deletions app/resources/webauthn/webauthn.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,22 @@ export interface WebAuthnChallengeRedirect {
export interface WebAuthnChallengeData {
kind: 'challenge';
publicKeyCredentialRequestOptions: unknown;
/**
* Set-Cookie values the caller MUST append. Only populated by the stale-session
* self-heal below, which supersedes the dead entry with a freshly minted one — the
* challenge is armed on the new session, so dropping these cookies would leave the
* browser pointing at the dead entry and the assertion would fail to verify.
*/
setCookies?: string[];
}

/**
* Opt-in recovery for a session that is dead PROVIDER-SIDE but still present (and
* apparently unexpired) in the signed `sessions` cookie — see requestWebAuthnChallenge.
* Carries the Request because re-minting needs the fingerprint + user-agent.
*/
export interface StaleSessionRecovery {
request: Request;
}

export type WebAuthnChallengeResult = WebAuthnChallengeRedirect | WebAuthnChallengeData;
Expand All @@ -121,7 +137,8 @@ export async function requestWebAuthnChallenge(
provider: AuthProvider,
sessions: SessionEntry[],
cfg: WebAuthnChallengeConfig,
{ loginName, requestId, organization, domain }: WebAuthnChallengeInput
{ loginName, requestId, organization, domain }: WebAuthnChallengeInput,
recovery?: StaleSessionRecovery
): Promise<WebAuthnChallengeResult> {
const entry = byLoginName(sessions, loginName, organization);
if (!entry) return { kind: 'redirect', target: loginBounceTarget(requestId, organization) };
Expand All @@ -138,14 +155,77 @@ export async function requestWebAuthnChallenge(
});
publicKeyCredentialRequestOptions =
session.challenges?.webAuthN?.publicKeyCredentialRequestOptions ?? null;
} catch {
} catch (err) {
logAuthEvent(cfg.challengeAuditEvent, 'failure', { loginName });
// Challenge failure is not fatal — the browser will show an error when the button is clicked.
// A STALE session is not the same failure as an unreachable backend, and conflating them
// is what produced the staging bug: the cookie entry above passed byLoginName (its
// expirationTs is cookie-local, so a provider-side termination is invisible to it), the
// challenge request threw NOT_FOUND/PERMISSION_DENIED, and the null options below reached
// WebAuthnButton's `!publicKey` guard — which tells the user "The passkey verification
// failed. Please try again." No verification was attempted and no retry could ever succeed,
// because every retry re-reads the same dead entry. Re-mint instead (below).
if (recovery && isStaleSessionError(err)) {
return recoverStaleChallenge(provider, recovery.request, sessions, {
loginName,
requestId,
organization,
domain,
});
}
// Any OTHER failure stays non-fatal — a transient backend fault is genuinely retryable,
// so render the screen and let the button surface the error on click.
}

return { kind: 'challenge', publicKeyCredentialRequestOptions };
}

/**
* Recover from a session that the provider has already terminated: mint a fresh user-bound
* session and arm the challenge on THAT, so the click that follows opens a real passkey
* dialog instead of a dead end.
*
* Only ever reached from requestWebAuthnChallenge's catch, and only when the caller opted in
* by passing `recovery` — armUserBoundChallenge calls requestWebAuthnChallenge itself, so
* unconditional recovery would let a stale error recurse back into it. Omitting the parameter
* on that internal call makes the cycle structurally impossible rather than merely unlikely
* (types.ts also warns the stale classifier is "NOT appropriate for a session just created
* earlier in the SAME request").
*
* Not an authentication bypass: reaching here requires an entry in the HMAC-signed `sessions`
* cookie naming this loginName, so the loginName cannot be forged through the URL param, and
* the minted session carries no verified factors until the assertion below it succeeds.
*
* CONTRACT NOTE: armUserBoundChallenge supersedes EVERY same-loginName entry, while we have
* only proven the one byLoginName selected is dead. byLoginName returns the most recent, so
* older duplicates are all but certainly dead too — and the blast radius is bounded either
* way, since dropping a session reference can only force a re-authentication, never grant one.
*/
async function recoverStaleChallenge(
provider: AuthProvider,
request: Request,
sessions: SessionEntry[],
{ loginName, requestId, organization, domain }: WebAuthnChallengeInput
): Promise<WebAuthnChallengeResult> {
const bounce: WebAuthnChallengeRedirect = {
kind: 'redirect',
target: loginBounceTarget(requestId, organization),
};

const user = await provider.findUser(loginName, organization);
if (!user) return bounce;

const armed = await armUserBoundChallenge(provider, request, sessions, user, domain);
// Re-mint failed (no passkey, provider refused the challenge) — resolve away from a verify
// screen that cannot work rather than rendering it with nothing armed.
if (!armed) return bounce;

return {
kind: 'challenge',
publicKeyCredentialRequestOptions: armed.publicKeyCredentialRequestOptions,
setCookies: armed.setCookies,
};
}

// ── USER-BOUND CHALLENGE ARM (usernameless entry points) ──────────────────────

export interface ArmedUserBoundChallenge {
Expand Down
32 changes: 31 additions & 1 deletion app/routes/logout/success.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,43 @@
import { AuthCard } from '@/components/auth-card/auth-card';
import { TrackOnMount, clearIdentifiedUser } from '@/modules/analytics/rybbit';
import { completeOidcLogout } from '@/resources/session';
import { providerForRequest } from '@/server/auth-context.server';
import { LinkButton } from '@datum-cloud/datum-ui/button';
import { Trans } from '@lingui/react/macro';
import { useEffect } from 'react';
import { Link } from 'react-router';
import { Link, data, type LoaderFunctionArgs } from 'react-router';
import type { MetaFunction } from 'react-router';

export const meta: MetaFunction = () => [{ title: 'Signed out' }];

/**
* Terminal sign-out page — and the OIDC RP's REGISTERED post-logout landing page
* (`post-logout-redirect-uris` points straight here), which makes it a cookie-clearing
* site, not just a render.
*
* On that hop Zitadel ends its own SSO session and 302s the browser here DIRECTLY, so
* /logout never runs and its clearing never happens. The local `sessions` and
* `passkey-hint` cookies then outlive the provider session, and because listSessions()
* judges liveness from cookie-local expiry alone (a provider-side termination is invisible
* to it), the orphaned entry reads as live for its full 24h. That suppressed the /login
* passkey fast path via `hasLiveSession`, leaving returning users on a bare email field.
*
* Arriving here means logout already happened, so complete it locally: terminate any
* residual v2 sessions provider-side (best-effort, per entry) and clear both cookies.
* Idempotent — the redirect from /logout arrives with an already-empty cookie, which
* makes this a no-op, and direct navigation while signed in is a deliberate sign-out.
*/
export async function loader({ request }: LoaderFunctionArgs) {
const provider = providerForRequest(request);
const outcome = await completeOidcLogout(provider, request);

const headers = new Headers();
headers.append('set-cookie', outcome.setCookie);
if (outcome.clearHintCookie) headers.append('set-cookie', outcome.clearHintCookie);

return data(null, { headers });
}

export default function LogoutSuccess() {
useEffect(() => {
clearIdentifiedUser();
Expand Down
60 changes: 60 additions & 0 deletions cypress/component/resources/webauthn/webauthn.service.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,66 @@ describe('requestWebAuthnChallenge — guard-fail bounce target threading', () =
});
});

// Singleton seed: u5 passkey-user@acme.test carries a passkey (same constant as
// routes/login/conditional-passkey-loader.cy.ts).
const PK_USER = 'passkey-user@acme.test';

describe('requestWebAuthnChallenge — stale provider-side session self-heal', () => {
// Staging bug: after an OIDC logout the signed `sessions` cookie survived with a
// still-future expirationTs while Zitadel had already terminated the session. byLoginName
// therefore handed a DEAD entry to updateSession, which threw NOT_FOUND/PERMISSION_DENIED.
// The bare `catch {}` swallowed it and returned a null challenge, so the screen rendered
// normally and WebAuthnButton's `!publicKey` guard fired on click — surfacing
// "The passkey verification failed. Please try again." without ever calling
// navigator.credentials.get(). Nothing was verified; the advice to retry could never work.
it('re-mints a user-bound session and arms a fresh challenge when the cookie entry is dead provider-side', () => {
callService({
fn: 'requestWebAuthnChallenge',
provider: 'singleton',
request: {
url: 'http://localhost/id/login/passkey',
// Names a REAL seeded passkey user, but an id/token the provider has never issued —
// the fake throws ProviderError('NOT_FOUND') exactly as staging Zitadel does.
sessions: [{ id: 'dead-session', token: 'dead-token', loginName: PK_USER }],
},
attestationInput: { loginName: PK_USER, domain: 'localhost' },
}).then((v) => {
const o = v.outcome as {
kind: string;
publicKeyCredentialRequestOptions?: unknown;
setCookies?: string[];
};
expect(o.kind).to.equal('challenge');
expect(o.publicKeyCredentialRequestOptions, 'a fresh challenge is armed').to.exist;
// The dead entry must be superseded so the cookie stops advertising it as live.
expect(
(o.setCookies ?? []).some((c) => c.startsWith('sessions=')),
'sessions Set-Cookie'
).to.be.true;
});
});

it('bounces to /login when the session is dead AND the user no longer resolves', () => {
callService({
fn: 'requestWebAuthnChallenge',
provider: 'singleton',
request: {
url: 'http://localhost/id/login/passkey',
sessions: [{ id: 'dead-session', token: 'dead-token', loginName: 'ghost@acme.test' }],
},
attestationInput: {
loginName: 'ghost@acme.test',
domain: 'localhost',
requestId: 'oidc_V2_1',
},
}).then((v) => {
const o = v.outcome as { kind: string; target?: string };
expect(o.kind).to.equal('redirect');
expect(o.target).to.equal('/login?requestId=oidc_V2_1');
});
});
});

describe('verifyPasskeyEnrollment', () => {
it('threads checkAfter routing params, rejects malformed/invalid credentials, and expires with no session', () => {
callService({
Expand Down
65 changes: 65 additions & 0 deletions cypress/component/routes/login/passkey-stale-session.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// cypress/component/routes/login/passkey-stale-session.cy.ts
//
// Staging repro, at the HTTP boundary the browser actually hits:
// GET /id/login/passkey.data?loginName=… with a `sessions` cookie whose entry the
// provider has already terminated (OIDC logout ended the Zitadel session; the signed
// cookie survived carrying a still-future expirationTs).
//
// byLoginName cannot see a provider-side termination — expirationTs is cookie-local — so the
// dead entry passed its guard and the challenge request threw NOT_FOUND. That was swallowed
// into a null challenge, and WebAuthnButton's `!publicKey` guard then rendered
// "The passkey verification failed. Please try again." before any ceremony ran.
//
// The loader now self-heals. Asserting the Set-Cookie is the point of testing at THIS level
// rather than the service level: the challenge is armed on a NEWLY minted session, so if that
// session never reaches the browser the assertion posts against the dead entry and fails —
// the same bug, moved one step later and harder to spot.
import { callService } from '../../../support/node/call-service';

// Singleton seed: u5 carries a passkey (see conditional-passkey-loader.cy.ts).
const PK_USER = 'passkey-user@acme.test';
const DEAD_SESSION = { id: 'dead-session', token: 'dead-token', loginName: PK_USER };

type PasskeyLoaderBody = { publicKeyCredentialRequestOptions?: unknown; loginName?: string };

describe('/login/passkey loader — session dead provider-side', () => {
it('arms a fresh challenge and persists the re-minted session', () => {
callService({
fn: 'loginPasskeyLoader',
provider: 'singleton',
request: {
url: `http://localhost/id/login/passkey?loginName=${encodeURIComponent(PK_USER)}`,
sessions: [DEAD_SESSION],
},
}).then((v) => {
expect(v.error).to.be.undefined;
const body = v.response?.dataBody as PasskeyLoaderBody;

// Was null before the fix — the button had nothing to hand the authenticator.
expect(body.publicKeyCredentialRequestOptions, 'challenge armed').to.exist;
expect(body.loginName).to.equal(PK_USER);

// The re-minted session MUST ride back, or the assertion verifies against the dead entry.
const cookies = v.response?.dataSetCookies ?? [];
expect(
cookies.some((c: string) => c.startsWith('sessions=')),
'sessions Set-Cookie present'
).to.equal(true);
});
});

it('still bounces to /login when the cookie names nobody the provider knows', () => {
callService({
fn: 'loginPasskeyLoader',
provider: 'singleton',
request: {
url: 'http://localhost/id/login/passkey?loginName=ghost%40acme.test',
sessions: [{ id: 'dead-session', token: 'dead-token', loginName: 'ghost@acme.test' }],
},
}).then((v) => {
expect(v.error).to.be.undefined;
expect(v.response?.isResponse, 'redirect, not rendered data').to.equal(true);
expect(v.response?.status).to.be.oneOf([301, 302, 303, 307, 308]);
});
});
});
Loading
Loading