Skip to content

Commit 41675f1

Browse files
committed
fix(auth): widen fresh-session retry budget for Zitadel read-after-write lag
The post-register /authorize handback can transiently 404 on getSession when the read lands on a replica that has not caught up with the just-written session. A single 350ms retry lost that race intermittently (staging logs showed signup.created immediately followed by session_stale, bouncing fresh signups to /login). Replace the single retry with a bounded increasing-backoff poll (RETRY_BACKOFFS_MS = [300, 500, 800, 1200], ~2.8s max), scoped exactly as before: fresh entries only (signed creationTs within RETRY_FRESHNESS_WINDOW_MS), dead-code probes only, stopping the instant a probe comes back alive. An old/stale/forged entry still self-heals immediately on the first dead response — the anti-forgery guarantee is unchanged; the retry only adds patience for a session we know we just created. Adds a 'throw-times' fake-provider seam + a multi-retry regression test proving the loop keeps polling past the first attempt.
1 parent 151e727 commit 41675f1

4 files changed

Lines changed: 80 additions & 16 deletions

File tree

app/modules/auth/providers/fake/fake-provider.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,15 @@ import { ProviderError } from '@/modules/auth/types';
3737
// entry is deleted) so every subsequent call falls through to the real in-memory session lookup
3838
// — i.e. "fail once, then behave normally," which is exactly what a transient replica-lag 404
3939
// followed by a successful retry looks like.
40+
//
41+
// 'throw-times' generalises that to N consecutive throws before falling through — it drives the
42+
// BOUNDED-backoff-loop test (a replica that lags more than one read cycle), proving the retry
43+
// keeps polling past the first attempt rather than giving up after one.
4044
type FakeOutcomeScript =
4145
| { mode: 'null' }
4246
| { mode: 'throw'; code: ProviderErrorCode }
43-
| { mode: 'throw-once'; code: ProviderErrorCode };
47+
| { mode: 'throw-once'; code: ProviderErrorCode }
48+
| { mode: 'throw-times'; code: ProviderErrorCode; times: number };
4449

4550
export const FIXED_NOW = '2026-01-01T00:00:00.000Z'; // deterministic for tests (no Date.now())
4651
// Factor verifiedAt is `Date | null`. A frozen Date keeps the fake deterministic.
@@ -383,6 +388,17 @@ export class FakeAuthProvider implements AuthProvider {
383388
this.sessionResults.delete(id);
384389
throw new ProviderError(scripted.code, `scripted getSession ${scripted.code} (once)`);
385390
}
391+
if (scripted.mode === 'throw-times') {
392+
// Throw for the next `times` calls, decrementing each time; on the last one consume the
393+
// script so the following call falls through to the real lookup. Immutable update: re-set
394+
// a decremented copy rather than mutating the stored script object.
395+
if (scripted.times <= 1) this.sessionResults.delete(id);
396+
else this.sessionResults.set(id, { ...scripted, times: scripted.times - 1 });
397+
throw new ProviderError(
398+
scripted.code,
399+
`scripted getSession ${scripted.code} (${scripted.times} left)`
400+
);
401+
}
386402
throw new ProviderError(scripted.code, `scripted getSession ${scripted.code}`);
387403
}
388404
const s = this.sessions.get(id);

app/resources/authorize/authorize.service.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,15 @@ const DEAD_SESSION_CODES: ReadonlySet<ProviderError['code']> = new Set([
6565
// itself a retry. This preserves the existing anti-forgery guarantee: a stale post-logout
6666
// cookie (or a genuinely forged sessionId) still self-heals to /login on the FIRST dead
6767
// response, with zero added latency or leniency.
68-
// • It retries AT MOST ONCE. If the retry still comes back dead (or errors again), the
69-
// function falls through to the exact same self-heal path as if there had been no retry.
68+
// • It retries a BOUNDED number of times (RETRY_BACKOFFS_MS, ~2.8s total max), stopping early
69+
// the moment a probe comes back alive or non-dead. If every attempt still comes back dead (or
70+
// errors), the function falls through to the exact same self-heal path as if there had been no
71+
// retry — the retry only ever *adds* patience for a fresh session, never leniency.
7072
const RETRY_FRESHNESS_WINDOW_MS = 5000; // 5s — covers redirect latency + replica lag, nothing more
71-
const RETRY_BACKOFF_MS = 350; // short pause before the single retry attempt
73+
// Increasing backoffs between fresh-session liveness re-probes (~2.8s total max). A single short
74+
// retry loses the race when a replica lags >1 read cycle; a bounded poll rides out the write→read
75+
// replication for a session we KNOW we just created. Only ever applied to a fresh entry.
76+
const RETRY_BACKOFFS_MS = [300, 500, 800, 1200] as const;
7277

7378
/** True when `entry` was created within RETRY_FRESHNESS_WINDOW_MS of `nowMs`. A malformed or
7479
* missing creationTs (tsMs → NaN) is treated as NOT fresh — the safe default is the original,
@@ -232,7 +237,8 @@ function errorRedirect(url: URL, code: AuthErrorCode): Response {
232237
* and never mistaken for a genuine error. (The post-logout stale-cookie case.)
233238
*
234239
* EXCEPT: a DEAD_SESSION_CODES code on a FRESH entry (isFreshEntry — see the retry block
235-
* above) gets ONE retry, after a short backoff, before this self-heal fires — see the
240+
* above) is re-probed with a BOUNDED increasing backoff (RETRY_BACKOFFS_MS, ~2.8s total max),
241+
* stopping the instant it comes back alive, before this self-heal fires — see the
236242
* RETRY_FRESHNESS_WINDOW_MS comment for the full scoping rationale (Zitadel read-after-write
237243
* lag on a session THIS request just created). An old/stale/forged entry skips the retry
238244
* entirely and self-heals immediately, exactly as before.
@@ -260,13 +266,18 @@ async function healIfSessionDead(
260266
): Promise<AuthorizeOutcome | { session: Session }> {
261267
let probe = await probeSession(provider, entry);
262268

263-
// RETRY-ON-FRESH: see the RETRY_FRESHNESS_WINDOW_MS block above for the full scoping
264-
// rationale. Bounded to exactly one retry, and ONLY for a dead-code result on a genuinely
265-
// fresh cookie entry — every other case (already alive, confirmed-null, transient error, or a
266-
// dead code on an old/stale/forged entry) falls straight through to the original behavior.
267-
if (probe.kind === 'dead-code' && isFreshEntry(entry, nowMs)) {
268-
await sleep(RETRY_BACKOFF_MS);
269-
probe = await probeSession(provider, entry);
269+
// RETRY-ON-FRESH: see the RETRY_FRESHNESS_WINDOW_MS block above for the full scoping rationale.
270+
// Zitadel's read-after-write lag on a session THIS request just created can exceed a single
271+
// retry, so poll with increasing backoff (RETRY_BACKOFFS_MS, ~2.8s total max) — but ONLY while
272+
// the entry is genuinely fresh (isFreshEntry, signed creationTs) AND the probe is still a dead
273+
// code. Every other case (already alive, confirmed-null, transient error, or a dead code on an
274+
// old/stale/forged entry) falls straight through to the original behavior with no wait.
275+
if (isFreshEntry(entry, nowMs)) {
276+
for (const backoff of RETRY_BACKOFFS_MS) {
277+
if (probe.kind !== 'dead-code') break;
278+
await sleep(backoff);
279+
probe = await probeSession(provider, entry);
280+
}
270281
}
271282

272283
switch (probe.kind) {

cypress/component/resources/authorize/logout.cy.ts

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ describe('/authorize — stale-cookie self-heal (validate before reuse)', () =>
6969
// Read-after-write retry (Zitadel eventual consistency): a session THIS request just created
7070
// (the post-register /authorize handback is the concrete case) can transiently 404 on getSession
7171
// if the read lands on a replica that hasn't caught up with the write yet. healIfSessionDead
72-
// retries exactly once — but ONLY when the cookie entry's creationTs is freshbefore treating
73-
// a dead code as ground truth. See authorize.service.ts's RETRY_FRESHNESS_WINDOW_MS block for
74-
// the full scoping rationale.
72+
// re-probes with a bounded increasing backoff (stopping the instant it comes back alive)but
73+
// ONLY when the cookie entry's creationTs is fresh — before treating a dead code as ground truth.
74+
// See authorize.service.ts's RETRY_FRESHNESS_WINDOW_MS block for the full scoping rationale.
7575
describe('/authorize — read-after-write retry on a freshly-created session (NOT_FOUND)', () => {
7676
const NOW_MS = Date.parse('2026-06-24T12:00:00.000Z');
7777

@@ -109,6 +109,40 @@ describe('/authorize — read-after-write retry on a freshly-created session (NO
109109
});
110110
});
111111

112+
it('FRESH session lagging MORE than one read cycle: NOT_FOUND twice then alive → the bounded backoff loop keeps polling and finalizes (no self-heal)', () => {
113+
callService({
114+
fn: 'resolveAuthorize',
115+
provider: 'singleton',
116+
// Throws NOT_FOUND for the first TWO getSession calls (the initial probe + the first retry),
117+
// then falls through to this live session on the third — a replica that lags past a single
118+
// retry. Proves healIfSessionDead's loop keeps going instead of giving up after one attempt.
119+
liveSessions: [{ id: 'fresh-race-2', token: 'tok-fresh-race-2' }],
120+
sessionResults: {
121+
'fresh-race-2': { mode: 'throw-times', code: 'NOT_FOUND', times: 2 },
122+
},
123+
nowMs: NOW_MS,
124+
request: {
125+
url: `http://localhost/id/authorize?requestId=oidc_${RAW_ID}&sessionId=fresh-race-2`,
126+
sessions: [
127+
{
128+
id: 'fresh-race-2',
129+
token: 'tok-fresh-race-2',
130+
loginName: 'alice@acme.test',
131+
// 2s old — inside the retry window.
132+
creationTs: new Date(NOW_MS - 2000).toISOString(),
133+
},
134+
],
135+
},
136+
}).then((v) => {
137+
expect(v.response?.status).to.equal(302);
138+
const loc = v.response?.location ?? '';
139+
expect(loc).to.include('client.acme.test/callback');
140+
expect(loc).to.include(`code=fake_${RAW_ID}_fresh-race-2`);
141+
expect(loc).to.not.include('/login');
142+
expect(v.audit.some((e) => e.event === 'session_stale')).to.equal(false);
143+
});
144+
});
145+
112146
it('OLD session (creationTs outside the retry window): NOT_FOUND self-heals immediately — no retry, anti-forgery preserved', () => {
113147
callService({
114148
fn: 'resolveAuthorize',

cypress/support/node/scenario.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,10 @@ export type SessionResultScript =
260260
| { mode: 'throw'; code: ProviderErrorCode }
261261
// Fails the FIRST getSession call for that id, then falls through to the real fake behavior —
262262
// drives the healIfSessionDead read-after-write retry tests ("fail once, then succeed").
263-
| { mode: 'throw-once'; code: ProviderErrorCode };
263+
| { mode: 'throw-once'; code: ProviderErrorCode }
264+
// Fails the next `times` getSession calls, then falls through — drives the bounded-backoff-loop
265+
// test where a replica lags more than one read cycle ("fail N times, then succeed").
266+
| { mode: 'throw-times'; code: ProviderErrorCode; times: number };
264267
export type CallbackResultScript = { mode: 'throw'; code: ProviderErrorCode };
265268

266269
export interface Scenario {

0 commit comments

Comments
 (0)