Skip to content

Commit 514bdc5

Browse files
committed
auth(oidc): retry strategy registration with capped backoff (L-AUTH-5)
The account service registered the OIDC passport strategy only once at startup via a single fire-and-forget Issuer.discover(). A transient issuer outage in that one boot moment left the 'oidc' strategy permanently unregistered, producing 500 "Unknown authentication strategy" on every OpenID login until a manual container restart (observed: ~10 days). Make registration self-healing: - Extract registerOidcStrategyWithRetry(), which retries the ENTIRE registration chain (discover -> new Client -> new Strategy -> passport.use) with capped exponential backoff (5s, x2, cap 60s), unbounded. The loop only returns after passport.use succeeds, so a failure at client or strategy construction (e.g. temporarily incomplete issuer metadata during an IdP upgrade) is retried too instead of permanently disabling OIDC. discover and sleep are injectable for tests; passport is a structural { use } interface. It never rejects: the success log runs failure-safe AFTER passport.use and returns immediately, so a throwing logger can never re-enter the loop and register the strategy a second time; the failure warn is likewise guarded so a throwing sink cannot break the never-rejects contract. - Gate log flooding via shouldWarnOnAttempt(): warn on the first 5 attempts and every 60th thereafter (~24 warn lines/day at the 60s cap instead of ~1440), following the branch's L-AUTH-2 rationale; error stacks only under OIDC_DEBUG. Success logs once with the attempt count. - Wire it fire-and-forget in registerOpenid() and flip an oidcReady flag on success. The .then body is a plain boolean flip that cannot throw; a trailing .catch(() => {}) is belt-and-suspenders so this fire-and-forget chain can never surface an unhandled rejection. Routes stay registered synchronously. - During the pending window, /auth/openid responds 503 with Retry-After: 5 and a short text body instead of a 500 or a redirect (the login app ignores an error query param, and L-AUTH-4 only covers the callback path). Add the package's first tests: the retry helper in isolation (schedule, 60s cap, whole-attempt retry incl. client-construction failure, warn gating, OIDC_DEBUG both ways, a throwing success log that must not re-register, and a throwing failure warn that must not reject) and the registerOpenid wiring (synchronous route registration, 503 pending window, exactly-once strategy registration). Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
1 parent d05f4f9 commit 514bdc5

2 files changed

Lines changed: 481 additions & 21 deletions

File tree

Lines changed: 327 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,327 @@
1+
//
2+
// Copyright © 2026 Hardcore Engineering Inc.
3+
//
4+
// Licensed under the Eclipse Public License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License. You may
6+
// obtain a copy of the License at https://www.eclipse.org/legal/epl-2.0
7+
//
8+
// SPDX-License-Identifier: EPL-2.0
9+
//
10+
// Coverage for the OIDC registration retry (incident: a transient issuer
11+
// outage at boot left the account service without the 'oidc' passport strategy
12+
// for 10 days -> 500 "Unknown authentication strategy" until a manual
13+
// restart). Block A tests the retry helper in isolation (injected discover +
14+
// sleep, fake passport): schedule, cap, whole-attempt retry semantics, warn
15+
// gating, OIDC_DEBUG both ways. Block B tests the registerOpenid wiring with a
16+
// fully mocked openid-client: non-blocking startup, 503 + Retry-After during
17+
// the pending window, and exactly-once strategy registration.
18+
//
19+
/* eslint-disable @typescript-eslint/unbound-method */
20+
21+
import Router from 'koa-router'
22+
import { Issuer, Strategy } from 'openid-client'
23+
import { registerOidcStrategyWithRetry, registerOpenid, shouldWarnOnAttempt } from '../openid'
24+
25+
jest.mock('openid-client', () => ({
26+
Issuer: { discover: jest.fn() },
27+
Strategy: jest.fn().mockImplementation(() => ({}))
28+
}))
29+
30+
function makeMeasureCtx (): any {
31+
return { info: jest.fn(), warn: jest.fn(), error: jest.fn() }
32+
}
33+
34+
const params = {
35+
issuerUrl: 'https://idp.example.com',
36+
clientId: 'client-id',
37+
clientSecret: 'client-secret',
38+
redirectUri: 'http://accounts.example.com/auth/openid/callback'
39+
}
40+
41+
function makeSleepRecorder (): { sleep: (ms: number) => Promise<void>, delays: number[] } {
42+
const delays: number[] = []
43+
return {
44+
delays,
45+
sleep: async (ms: number): Promise<void> => {
46+
delays.push(ms)
47+
}
48+
}
49+
}
50+
51+
describe('A. registerOidcStrategyWithRetry — schedule, cap, whole-attempt retry, warn gating', () => {
52+
test('1. fails K=3 times then succeeds: registers once after K+1 attempts, default schedule', async () => {
53+
const ctx = makeMeasureCtx()
54+
const passport = { use: jest.fn() }
55+
const { sleep, delays } = makeSleepRecorder()
56+
const discover = jest
57+
.fn()
58+
.mockRejectedValueOnce(new Error('ECONNREFUSED'))
59+
.mockRejectedValueOnce(new Error('ECONNREFUSED'))
60+
.mockRejectedValueOnce(new Error('ECONNREFUSED'))
61+
.mockResolvedValue({ Client: jest.fn() })
62+
63+
const res = await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
64+
65+
expect(res.attempts).toBe(4)
66+
expect(discover).toHaveBeenCalledTimes(4)
67+
expect(discover).toHaveBeenCalledWith('https://idp.example.com')
68+
expect(delays).toEqual([5000, 10000, 20000])
69+
expect(passport.use).toHaveBeenCalledTimes(1)
70+
expect(passport.use).toHaveBeenCalledWith('oidc', expect.anything())
71+
expect(Strategy).toHaveBeenCalledTimes(1)
72+
expect(ctx.warn).toHaveBeenCalledTimes(3)
73+
expect(ctx.info).toHaveBeenCalledWith('Registered OIDC strategy', { attempts: 4 })
74+
expect(ctx.error).not.toHaveBeenCalled()
75+
})
76+
77+
test('2. backoff is capped at 60s and stays there', async () => {
78+
const ctx = makeMeasureCtx()
79+
const passport = { use: jest.fn() }
80+
const { sleep, delays } = makeSleepRecorder()
81+
const discover = jest.fn()
82+
for (let i = 0; i < 7; i++) discover.mockRejectedValueOnce(new Error('boom'))
83+
discover.mockResolvedValue({ Client: jest.fn() })
84+
85+
const res = await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
86+
87+
expect(res.attempts).toBe(8)
88+
expect(delays).toEqual([5000, 10000, 20000, 40000, 60000, 60000, 60000])
89+
})
90+
91+
test('3. immediate success: one attempt, no sleep, no warn, exactly one passport.use', async () => {
92+
const ctx = makeMeasureCtx()
93+
const passport = { use: jest.fn() }
94+
const sleep = jest.fn()
95+
const discover = jest.fn().mockResolvedValue({ Client: jest.fn() })
96+
97+
const res = await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
98+
99+
expect(res.attempts).toBe(1)
100+
expect(sleep).not.toHaveBeenCalled()
101+
expect(ctx.warn).not.toHaveBeenCalled()
102+
expect(passport.use).toHaveBeenCalledTimes(1)
103+
})
104+
105+
test('4. retry covers the whole attempt: client construction failure retries too (HIGH-1)', async () => {
106+
const ctx = makeMeasureCtx()
107+
const passport = { use: jest.fn() }
108+
const { sleep, delays } = makeSleepRecorder()
109+
const throwingClient = jest.fn().mockImplementation(() => {
110+
throw new Error('unsupported code_challenge_method')
111+
})
112+
const discover = jest
113+
.fn()
114+
.mockResolvedValueOnce({ Client: throwingClient })
115+
.mockResolvedValue({ Client: jest.fn() })
116+
117+
const res = await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
118+
119+
expect(res.attempts).toBe(2)
120+
expect(discover).toHaveBeenCalledTimes(2)
121+
expect(delays).toEqual([5000])
122+
expect(passport.use).toHaveBeenCalledTimes(1)
123+
expect(ctx.warn).toHaveBeenCalledTimes(1)
124+
expect(ctx.error).not.toHaveBeenCalled()
125+
})
126+
127+
test('5. warn gating: attempts 1-5 and every 60th log, others are silent (MED-2)', async () => {
128+
const ctx = makeMeasureCtx()
129+
const passport = { use: jest.fn() }
130+
const { sleep } = makeSleepRecorder()
131+
const discover = jest.fn()
132+
for (let i = 0; i < 65; i++) discover.mockRejectedValueOnce(new Error('boom'))
133+
discover.mockResolvedValue({ Client: jest.fn() })
134+
135+
const res = await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
136+
137+
expect(res.attempts).toBe(66)
138+
expect(ctx.warn).toHaveBeenCalledTimes(6)
139+
expect(ctx.warn.mock.calls.map((c: any[]) => c[1].attempt)).toEqual([1, 2, 3, 4, 5, 60])
140+
expect(ctx.info).toHaveBeenCalledWith('Registered OIDC strategy', { attempts: 66 })
141+
// pure gating function, spot checks
142+
expect(shouldWarnOnAttempt(6)).toBe(false)
143+
expect(shouldWarnOnAttempt(59)).toBe(false)
144+
expect(shouldWarnOnAttempt(120)).toBe(true)
145+
})
146+
147+
test('6. errStack absent without OIDC_DEBUG (L-AUTH-2 convention)', async () => {
148+
const ctx = makeMeasureCtx()
149+
const passport = { use: jest.fn() }
150+
const { sleep } = makeSleepRecorder()
151+
const discover = jest.fn().mockRejectedValueOnce(new Error('x')).mockResolvedValue({ Client: jest.fn() })
152+
const prev = process.env.OIDC_DEBUG
153+
try {
154+
delete process.env.OIDC_DEBUG
155+
await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
156+
expect(ctx.warn.mock.calls[0][1]).not.toHaveProperty('errStack')
157+
} finally {
158+
if (prev === undefined) delete process.env.OIDC_DEBUG
159+
else process.env.OIDC_DEBUG = prev
160+
}
161+
})
162+
163+
test('7. errStack present with OIDC_DEBUG=true (L-AUTH-2 convention)', async () => {
164+
const ctx = makeMeasureCtx()
165+
const passport = { use: jest.fn() }
166+
const { sleep } = makeSleepRecorder()
167+
const discover = jest.fn().mockRejectedValueOnce(new Error('x')).mockResolvedValue({ Client: jest.fn() })
168+
const prev = process.env.OIDC_DEBUG
169+
try {
170+
process.env.OIDC_DEBUG = 'true'
171+
await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
172+
expect(ctx.warn.mock.calls[0][1]).toHaveProperty('errStack')
173+
expect(typeof ctx.warn.mock.calls[0][1].errStack).toBe('string')
174+
} finally {
175+
if (prev === undefined) delete process.env.OIDC_DEBUG
176+
else process.env.OIDC_DEBUG = prev
177+
}
178+
})
179+
180+
test('8. success log throwing after passport.use does not re-register (LOW-1)', async () => {
181+
const ctx = makeMeasureCtx()
182+
ctx.info = jest.fn((msg: string) => {
183+
if (msg === 'Registered OIDC strategy') throw new Error('logger down')
184+
})
185+
const passport = { use: jest.fn() }
186+
const { sleep } = makeSleepRecorder()
187+
const discover = jest.fn().mockResolvedValue({ Client: jest.fn() })
188+
189+
const res = await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
190+
191+
// The throw from the success log must not re-enter the retry loop.
192+
expect(res.attempts).toBe(1)
193+
expect(passport.use).toHaveBeenCalledTimes(1)
194+
expect(discover).toHaveBeenCalledTimes(1)
195+
expect(ctx.warn).not.toHaveBeenCalled()
196+
})
197+
198+
test('9. helper never rejects even if the failure warn throws (LOW-2)', async () => {
199+
const ctx = makeMeasureCtx()
200+
ctx.warn = jest.fn(() => {
201+
throw new Error('warn sink down')
202+
})
203+
const passport = { use: jest.fn() }
204+
const { sleep } = makeSleepRecorder()
205+
const discover = jest.fn().mockRejectedValueOnce(new Error('ECONNREFUSED')).mockResolvedValue({ Client: jest.fn() })
206+
207+
// Must resolve (contract: never rejects), not throw the warn error.
208+
const res = await registerOidcStrategyWithRetry(ctx, passport, params, { discover, sleep })
209+
210+
expect(res.attempts).toBe(2)
211+
expect(passport.use).toHaveBeenCalledTimes(1)
212+
})
213+
})
214+
215+
describe('B. registerOpenid wiring — non-blocking startup, 503 pending window, exactly-once use', () => {
216+
const env = process.env
217+
218+
beforeEach(() => {
219+
jest.useFakeTimers()
220+
jest.clearAllMocks()
221+
process.env = {
222+
...env,
223+
OPENID_CLIENT_ID: 'client-id',
224+
OPENID_CLIENT_SECRET: 'client-secret',
225+
OPENID_ISSUER: 'https://idp.example.com',
226+
OPENID_DISPLAY_NAME: 'Example IdP'
227+
}
228+
})
229+
230+
afterEach(() => {
231+
jest.useRealTimers()
232+
process.env = env
233+
})
234+
235+
function callRegister (): { passport: any, router: Router<any, any>, ctx: any } {
236+
const passport: any = { use: jest.fn(), authenticate: jest.fn(() => async () => {}) }
237+
const router = new Router<any, any>()
238+
const ctx = makeMeasureCtx()
239+
const info = registerOpenid(
240+
ctx,
241+
passport,
242+
router,
243+
'http://accounts.example.com',
244+
Promise.resolve({} as any),
245+
'http://front.example.com',
246+
{}
247+
)
248+
expect(info).toEqual({ name: 'openid', displayName: 'Example IdP' })
249+
return { passport, router, ctx }
250+
}
251+
252+
async function invokeAuthRoute (router: Router<any, any>): Promise<any> {
253+
const layer = router.stack.find((l) => l.path === '/auth/openid')
254+
expect(layer).toBeDefined()
255+
// encodeState dereferences ctx.request.headers (utils.ts getHost) and
256+
// ctx.query — both must exist on the fixture. set/status/body carry the
257+
// 503 pending-window response.
258+
const koaCtx: any = {
259+
redirect: jest.fn(),
260+
set: jest.fn(),
261+
status: undefined,
262+
body: undefined,
263+
query: {},
264+
state: {},
265+
request: { headers: {} }
266+
}
267+
await (layer as any).stack[0](koaCtx, async () => {})
268+
return koaCtx
269+
}
270+
271+
test('1. route registered synchronously; pending window responds 503 + Retry-After', async () => {
272+
const discover = Issuer.discover as jest.Mock
273+
discover.mockReturnValue(new Promise(() => {})) // discovery never settles
274+
275+
const { passport, router } = callRegister()
276+
const koaCtx = await invokeAuthRoute(router)
277+
278+
expect(koaCtx.status).toBe(503)
279+
expect(koaCtx.set).toHaveBeenCalledWith('Retry-After', '5')
280+
expect(koaCtx.body).toEqual(expect.any(String))
281+
expect(koaCtx.redirect).not.toHaveBeenCalled()
282+
expect(passport.authenticate).not.toHaveBeenCalled()
283+
expect(passport.use).not.toHaveBeenCalled()
284+
})
285+
286+
test('2. discover fails twice then succeeds: strategy registered exactly once, retries stop', async () => {
287+
const discover = Issuer.discover as jest.Mock
288+
discover
289+
.mockRejectedValueOnce(new Error('ECONNREFUSED'))
290+
.mockRejectedValueOnce(new Error('ECONNREFUSED'))
291+
.mockResolvedValue({ Client: jest.fn() })
292+
293+
const { passport, ctx } = callRegister()
294+
295+
// attempt 1 fails immediately; backoff 5s -> attempt 2 fails; 10s -> attempt 3 succeeds
296+
await jest.advanceTimersByTimeAsync(5000)
297+
await jest.advanceTimersByTimeAsync(10000)
298+
299+
expect(discover).toHaveBeenCalledTimes(3)
300+
expect(passport.use).toHaveBeenCalledTimes(1)
301+
expect(passport.use).toHaveBeenCalledWith('oidc', expect.anything())
302+
expect(Strategy).toHaveBeenCalledTimes(1)
303+
expect(ctx.info).toHaveBeenCalledWith('Registered OIDC strategy', { attempts: 3 })
304+
305+
// no further attempts, no double registration
306+
await jest.advanceTimersByTimeAsync(600000)
307+
expect(discover).toHaveBeenCalledTimes(3)
308+
expect(passport.use).toHaveBeenCalledTimes(1)
309+
})
310+
311+
test('3. after successful registration the auth route takes the passport path, no 503', async () => {
312+
const discover = Issuer.discover as jest.Mock
313+
discover.mockResolvedValue({ Client: jest.fn() })
314+
315+
const { passport, router } = callRegister()
316+
await jest.advanceTimersByTimeAsync(0) // flush the resolved registration
317+
318+
const koaCtx = await invokeAuthRoute(router)
319+
expect(koaCtx.status).not.toBe(503)
320+
expect(koaCtx.set).not.toHaveBeenCalled()
321+
expect(koaCtx.redirect).not.toHaveBeenCalled()
322+
expect(passport.authenticate).toHaveBeenCalledWith(
323+
'oidc',
324+
expect.objectContaining({ scope: 'openid profile email' })
325+
)
326+
})
327+
})

0 commit comments

Comments
 (0)