Skip to content

Commit 8287dec

Browse files
committed
fix(auth-ui): require fresh auth to satisfy prompt=login via sessionId hand-back
resolveOidc's /authorize sessionId hand-back finalizes the callback for a query-supplied sessionId before decideAuthorize runs. For prompt=login that let a forged &sessionId=<stale-but-live session> skip the forced re-authentication the AS demanded. Gate it: a handed-back session satisfies prompt=login only if a primary factor (password/passkey/idpIntent) was verified within FRESH_LOGIN_WINDOW_MS (2 min); otherwise fall through to decideAuthorize -> /login. select_account is unaffected (selecting your own live session is safe). healIfSessionDead now returns the live session so the gate can read factor verifiedAt; nowMs is injected (default Date.now()) for deterministic tests.
1 parent 59d2897 commit 8287dec

2 files changed

Lines changed: 171 additions & 16 deletions

File tree

app/resources/authorize/__tests__/oidc-handback.test.ts

Lines changed: 130 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,20 @@
1414
// a prompt=select_account / prompt=login request bounced straight back to /accounts (or /login)
1515
// — forcing the user to pick the account they had just authenticated with. This test pins the
1616
// finalize-don't-bounce behavior at the resolveOidc layer.
17-
import { FakeAuthProvider } from '@/modules/auth/providers/fake/fake-provider';
17+
import { FakeAuthProvider, FIXED_NOW } from '@/modules/auth/providers/fake/fake-provider';
1818
import { sessionsCookie } from '@/modules/auth/session/cookie';
19-
import type { AuthRequest } from '@/modules/auth/types';
19+
import type { AuthRequest, Session } from '@/modules/auth/types';
2020
import { resolveAuthorize, outcomeToResponse } from '@/resources/authorize';
2121
import { describe, it, expect } from 'vitest';
2222

2323
const SESSION = { id: 'sess-live-1', token: 'tok-live-1' };
2424

25+
// seedLiveSession stamps factors at FIXED_NOW. Pin the loader's nowMs to FIXED_NOW so a seeded
26+
// session reads as freshly authenticated for the prompt=login freshness gate — keeping these
27+
// regression-lock cases deterministic instead of depending on wall-clock Date.now() (the fake's
28+
// own MERGE-RULE warns finite windows against FIXED_NOW rot into time bombs).
29+
const SEEDED_NOW_MS = Date.parse(FIXED_NOW);
30+
2531
/** Build a signed sessions cookie carrying a single live session entry. */
2632
async function mintSessionsCookie() {
2733
return sessionsCookie.serialize([
@@ -53,7 +59,8 @@ async function run(provider: FakeAuthProvider, cookie: string, search: string):
5359
const request = new Request(`http://localhost/id/authorize${search}`, {
5460
headers: { cookie: cookie.split(';')[0] },
5561
});
56-
const outcome = await resolveAuthorize(provider, request);
62+
// Pin nowMs to the fixture clock so seedLiveSession factors are fresh (see SEEDED_NOW_MS).
63+
const outcome = await resolveAuthorize(provider, request, SEEDED_NOW_MS);
5764
return outcomeToResponse(outcome, new URL(request.url));
5865
}
5966

@@ -97,3 +104,123 @@ describe('resolveOidc — explicit sessionId hand-back finalizes the callback (r
97104
expect(res.headers.get('location') ?? '').toContain('/accounts');
98105
});
99106
});
107+
108+
// ─────────────────────────────────────────────────────────────────────────────────
109+
// SECURITY REGRESSION LOCK: prompt=login freshness gate against forged sessionId hand-back.
110+
//
111+
// `?requestId=oidc_<id>&sessionId=<live>` lets ANY caller hand back a live cookie session id.
112+
// For prompt=login the Authorization Server explicitly demanded FRESH re-authentication this
113+
// ceremony — so a stale-but-live session id appended to the URL MUST NOT finalize the callback;
114+
// it must fall through to decideAuthorize, which routes prompt=login → /login (forced re-auth).
115+
// select_account is harmless (you may pick your own live session), so the gate is scoped to
116+
// prompt=login only.
117+
// ─────────────────────────────────────────────────────────────────────────────────
118+
119+
const NOW_MS = Date.parse('2026-06-24T12:00:00.000Z');
120+
const TEN_MIN_MS = 10 * 60 * 1000;
121+
const FIVE_SEC_MS = 5 * 1000;
122+
123+
/**
124+
* A FakeAuthProvider whose getSession returns a session with a CONTROLLABLE
125+
* `factors.password.verifiedAt`. The shared fake hardcodes verifiedAt to FIXED_NOW, so it
126+
* cannot express a "stale relative to nowMs" factor — this inline stub fills that gap while
127+
* delegating createCallback/getAuthRequest to the real fake behaviour.
128+
*/
129+
class FreshnessProvider extends FakeAuthProvider {
130+
constructor(
131+
prompt: AuthRequest['prompt'],
132+
private readonly verifiedAtMs: number
133+
) {
134+
super({ authRequests: { req1: { id: 'req1', clientId: 'client1', scopes: [], prompt } } });
135+
}
136+
137+
override async getSession(id: string, _token: string): Promise<Session | null> {
138+
if (id !== SESSION.id) return null;
139+
return {
140+
id: SESSION.id,
141+
token: SESSION.token,
142+
factors: { password: { verifiedAt: new Date(this.verifiedAtMs) } },
143+
expiresAt: '2099-01-01T00:00:00.000Z',
144+
changedAt: '2026-01-01T00:00:00.000Z',
145+
};
146+
}
147+
}
148+
149+
/** Drive resolveAuthorize with an injected nowMs (the route default is Date.now()). */
150+
async function runAt(
151+
provider: FakeAuthProvider,
152+
cookie: string,
153+
search: string,
154+
nowMs: number
155+
): Promise<Response> {
156+
const request = new Request(`http://localhost/id/authorize${search}`, {
157+
headers: { cookie: cookie.split(';')[0] },
158+
});
159+
const outcome = await resolveAuthorize(provider, request, nowMs);
160+
return outcomeToResponse(outcome, new URL(request.url));
161+
}
162+
163+
describe('resolveOidc — prompt=login freshness gate (anti-forgery hardening)', () => {
164+
it('prompt=login + STALE handed-back session → does NOT finalize; routes to /login (re-auth)', async () => {
165+
// The security regression lock: a stale (10-min-old) live session forged onto a prompt=login
166+
// URL must NOT skip the forced re-authentication the AS asked for.
167+
const provider = new FreshnessProvider(['login'], NOW_MS - TEN_MIN_MS);
168+
const cookie = await mintSessionsCookie();
169+
170+
const res = await runAt(
171+
provider,
172+
cookie,
173+
`?requestId=oidc_req1&sessionId=${SESSION.id}`,
174+
NOW_MS
175+
);
176+
177+
expect(res.status).toBe(302);
178+
const loc = res.headers.get('location') ?? '';
179+
// Did NOT finalize the callback — no client redirect / ?code=.
180+
expect(loc).not.toContain('client.acme.test/callback');
181+
expect(loc).not.toContain('code=');
182+
// Routed to forced re-auth, carrying the requestId so the ceremony resumes.
183+
expect(loc).toContain('/login');
184+
expect(loc).toContain('requestId=oidc_req1');
185+
});
186+
187+
it('prompt=login + FRESH handed-back session → finalizes the callback (legitimate login)', async () => {
188+
// A genuinely fresh (5-sec-old) session satisfies prompt=login — the normal post-auth
189+
// finalize redirect must still work.
190+
const provider = new FreshnessProvider(['login'], NOW_MS - FIVE_SEC_MS);
191+
const cookie = await mintSessionsCookie();
192+
193+
const res = await runAt(
194+
provider,
195+
cookie,
196+
`?requestId=oidc_req1&sessionId=${SESSION.id}`,
197+
NOW_MS
198+
);
199+
200+
expect(res.status).toBe(302);
201+
const loc = res.headers.get('location') ?? '';
202+
expect(loc).toContain('client.acme.test/callback');
203+
expect(loc).toContain(`code=fake_req1_${SESSION.id}`);
204+
expect(loc).not.toContain('/login');
205+
});
206+
207+
it('prompt=select_account + STALE handed-back session → still finalizes (gate is login-only)', async () => {
208+
// Selecting your own live session is harmless: the gate must NOT apply to select_account,
209+
// even with a stale factor. Confirms the hardening is scoped to prompt=login.
210+
const provider = new FreshnessProvider(['select_account'], NOW_MS - TEN_MIN_MS);
211+
const cookie = await mintSessionsCookie();
212+
213+
const res = await runAt(
214+
provider,
215+
cookie,
216+
`?requestId=oidc_req1&sessionId=${SESSION.id}`,
217+
NOW_MS
218+
);
219+
220+
expect(res.status).toBe(302);
221+
const loc = res.headers.get('location') ?? '';
222+
expect(loc).toContain('client.acme.test/callback');
223+
expect(loc).toContain(`code=fake_req1_${SESSION.id}`);
224+
expect(loc).not.toContain('/accounts');
225+
});
226+
});

app/resources/authorize/authorize.service.ts

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ import {
2020
serializeSessions,
2121
type SessionEntry,
2222
} from '@/modules/auth/session/cookie';
23-
import { ProviderError } from '@/modules/auth/types';
23+
import { ProviderError, type Session } from '@/modules/auth/types';
2424
import { decideAuthorize } from '@/resources/authorize/authorize-decision';
25+
import { primaryFresh } from '@/resources/shared/lifetimes';
2526
import { logAuthEvent } from '@/server/observability';
2627
import { type AuthErrorCode, providerErrorCode } from '@/utils/errors/auth-error';
2728
import { redirect } from 'react-router';
@@ -41,6 +42,13 @@ const DEAD_SESSION_CODES: ReadonlySet<ProviderError['code']> = new Set([
4142
'PERMISSION_DENIED',
4243
]);
4344

45+
// Freshness window for honoring a query-handed-back sessionId against a prompt=login request.
46+
// The post-auth finalize redirect is near-immediate (the ceremony hands the just-authenticated
47+
// session id straight back to /authorize), so 2 minutes generously covers redirect latency +
48+
// clock skew while keeping forced re-auth meaningful — a STALE session id forged onto the URL
49+
// can no longer satisfy prompt=login.
50+
const FRESH_LOGIN_WINDOW_MS = 2 * 60 * 1000; // 2 minutes
51+
4452
/**
4553
* A typed description of what the /authorize loader resolved to. The route (and the
4654
* `outcomeToResponse` translator below) turn this into a redirect/JSON Response. Keeping
@@ -118,8 +126,9 @@ function errorRedirect(url: URL, code: AuthErrorCode): Response {
118126
/**
119127
* Validate that a cookie session is still alive before reusing it in createCallback.
120128
*
121-
* Returns an AuthorizeOutcome (caller must return it immediately) for the two non-alive
122-
* outcomes, and `null` only when the session is ALIVE (caller proceeds to createCallback):
129+
* Returns an AuthorizeOutcome (which carries a `kind`; caller must return it immediately) for the
130+
* two non-alive outcomes, and `{ session }` ONLY when the session is ALIVE (caller proceeds to the
131+
* freshness gate / createCallback). Callers discriminate on `'kind' in result`:
123132
*
124133
* • CONFIRMED DEAD — getSession → null, or a ProviderError with a DEAD_SESSION_CODES code:
125134
* drop the stale entry from the cookie and re-prompt /login (self-heal; mirrors the SAML
@@ -131,6 +140,9 @@ function errorRedirect(url: URL, code: AuthErrorCode): Response {
131140
* `oidc_callback` failure WITH the code so the transient is diagnosable. A Zitadel hiccup
132141
* must never silently re-login a valid user, and must never be swallowed.
133142
*
143+
* • ALIVE — return the live `Session` so the caller can inspect its factors (the prompt=login
144+
* freshness gate needs `factors.*.verifiedAt`) before reusing it in createCallback.
145+
*
134146
* Distinguishing dead vs transient by the error code is the critical precision that prevents
135147
* introducing a new bug (logging out a valid user on a transient blip).
136148
*/
@@ -140,7 +152,7 @@ async function healIfSessionDead(
140152
entry: SessionEntry,
141153
requestId: string,
142154
rawId: string
143-
): Promise<AuthorizeOutcome | null> {
155+
): Promise<AuthorizeOutcome | { session: Session }> {
144156
let alive: Awaited<ReturnType<AuthProvider['getSession']>>;
145157
try {
146158
alive = await provider.getSession(entry.id, entry.token);
@@ -160,7 +172,7 @@ async function healIfSessionDead(
160172
return { kind: 'error-redirect', code: providerErrorCode(code) };
161173
}
162174
if (!alive) return healStaleEntry(list, entry, requestId, rawId); // confirmed dead
163-
return null; // alive → proceed to createCallback
175+
return { session: alive }; // alive → caller proceeds to the freshness gate / createCallback
164176
}
165177

166178
/** Drop the stale entry, re-prompt /login, and emit a traceable session_stale event. */
@@ -265,7 +277,8 @@ async function resolveOidc(
265277
provider: AuthProvider,
266278
request: Request,
267279
url: URL,
268-
requestId: string
280+
requestId: string,
281+
nowMs: number = Date.now()
269282
): Promise<AuthorizeOutcome> {
270283
const rawId = requestId.replace('oidc_', '');
271284

@@ -288,9 +301,21 @@ async function resolveOidc(
288301
if (entry) {
289302
// Validate liveness BEFORE reuse: a stale post-logout cookie self-heals to /login here
290303
// instead of reaching createCallback on a terminated session (→ ALREADY_DONE → /error).
291-
const healed = await healIfSessionDead(provider, list, entry, requestId, rawId);
292-
if (healed) return healed;
293-
return runCallback(provider, rawId, entry);
304+
const gate = await healIfSessionDead(provider, list, entry, requestId, rawId);
305+
if ('kind' in gate) return gate; // dead/transient → outcome already decided
306+
307+
// ANTI-FORGERY FRESHNESS GATE (prompt=login only). The sessionId is query-supplied, so a
308+
// caller can forge `&sessionId=<their_own_STALE_live_session>` onto a prompt=login request
309+
// to skip the forced re-authentication the AS explicitly demanded. A handed-back session may
310+
// satisfy prompt=login ONLY if it was genuinely freshly authenticated THIS ceremony (a
311+
// primary factor verified within FRESH_LOGIN_WINDOW_MS). If it is stale, fall through to
312+
// decideAuthorize, which routes prompt=login → /login (forced re-auth). select_account is
313+
// harmless (picking your own live session), so the gate is scoped to prompt=login.
314+
const mustReauth =
315+
authRequest.prompt.includes('login') &&
316+
!primaryFresh(gate.session.factors, nowMs, FRESH_LOGIN_WINDOW_MS);
317+
if (!mustReauth) return runCallback(provider, rawId, entry);
318+
// else: stale prompt=login → do NOT finalize; fall through to decideAuthorize below.
294319
}
295320
}
296321

@@ -304,9 +329,11 @@ async function resolveOidc(
304329
if (decision.target === 'callback') {
305330
const entry = byId(list, decision.params?.sessionId ?? '');
306331
if (!entry) return { kind: 'error-redirect', code: 'no_session' };
307-
// Validate liveness BEFORE reuse (same self-heal as the explicit-sessionId path above).
332+
// Validate liveness BEFORE reuse (same self-heal as the explicit-sessionId path above). No
333+
// freshness gate here: for prompt=login decideAuthorize returns target '/login', never
334+
// 'callback', so this branch is unreachable under prompt=login (only none/default reuse).
308335
const healed = await healIfSessionDead(provider, list, entry, requestId, rawId);
309-
if (healed) return healed;
336+
if ('kind' in healed) return healed;
310337
return runCallback(provider, rawId, entry);
311338
}
312339
if (decision.target === 'error') {
@@ -332,7 +359,8 @@ async function resolveOidc(
332359
*/
333360
export async function resolveAuthorize(
334361
provider: AuthProvider,
335-
request: Request
362+
request: Request,
363+
nowMs: number = Date.now()
336364
): Promise<AuthorizeOutcome> {
337365
const url = new URL(request.url);
338366
const requestId = normalizeRequestId(url);
@@ -344,5 +372,5 @@ export async function resolveAuthorize(
344372

345373
if (requestId.startsWith('saml_')) return resolveSaml(provider, request, requestId);
346374
if (requestId.startsWith('device_')) return resolveDevice(requestId);
347-
return resolveOidc(provider, request, url, requestId);
375+
return resolveOidc(provider, request, url, requestId, nowMs);
348376
}

0 commit comments

Comments
 (0)