diff --git a/lib/__tests__/oauth-discovery.test.ts b/lib/__tests__/oauth-discovery.test.ts index 5b8228af..63f95c96 100644 --- a/lib/__tests__/oauth-discovery.test.ts +++ b/lib/__tests__/oauth-discovery.test.ts @@ -46,7 +46,8 @@ describe('oauth/discovery', () => { expect(result).toEqual(VALID_METADATA); expect(fetch).toHaveBeenCalledTimes(1); expect(fetch).toHaveBeenCalledWith( - 'https://mail.example.com/.well-known/oauth-authorization-server' + 'https://mail.example.com/.well-known/oauth-authorization-server', + expect.objectContaining({ signal: expect.any(AbortSignal) }), ); }); @@ -64,7 +65,8 @@ describe('oauth/discovery', () => { expect(fetch).toHaveBeenCalledTimes(2); expect(fetch).toHaveBeenNthCalledWith( 2, - 'https://fallback.example.com/.well-known/openid-configuration' + 'https://fallback.example.com/.well-known/openid-configuration', + expect.objectContaining({ signal: expect.any(AbortSignal) }), ); }); @@ -176,4 +178,83 @@ describe('oauth/discovery', () => { expect(second).toEqual(VALID_METADATA); expect(fetch).toHaveBeenCalledTimes(1); }); + + it('bounds each discovery fetch with an AbortSignal timeout (no hang on unresponsive IdP)', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const fetchMock = vi.fn().mockRejectedValue( + Object.assign(new Error('The operation timed out'), { name: 'TimeoutError' }), + ); + vi.stubGlobal('fetch', fetchMock); + + const result = await discoverOAuth('https://unresponsive.example.com', { validateEndpoint }); + + expect(result).toBeNull(); + // Every discovery fetch must carry an AbortSignal so an unresponsive IdP is + // aborted (DISCOVERY_TIMEOUT_MS) instead of hanging the request - and, with + // it, the login page's SSO button. + expect(fetchMock.mock.calls.length).toBeGreaterThan(0); + for (const call of fetchMock.mock.calls) { + expect(call[1]).toEqual(expect.objectContaining({ signal: expect.any(AbortSignal) })); + } + }); + + it('retries once when the first attempt fails, then succeeds', async () => { + // Attempt 1: both well-known URLs fail. Attempt 2: first URL succeeds. + vi.stubGlobal('fetch', vi.fn() + .mockResolvedValueOnce({ ok: false, status: 503 }) + .mockResolvedValueOnce({ ok: false, status: 503 }) + .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(VALID_METADATA) })); + + const result = await discoverOAuth('https://flaky.example.com', { validateEndpoint }); + + expect(result).toEqual(VALID_METADATA); + // 2 failures (attempt 1) + 1 success (attempt 2 retry). + expect(fetch).toHaveBeenCalledTimes(3); + }); + + it('serves stale cached metadata when a refresh fails (keeps the SSO button up)', async () => { + vi.useFakeTimers(); + try { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + // First call succeeds and caches the metadata. + vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve(VALID_METADATA), + })); + const first = await discoverOAuth('https://stale.example.com', { validateEndpoint }); + expect(first).toEqual(VALID_METADATA); + + // Expire the cache (positive TTL is 10 min). + vi.advanceTimersByTime(10 * 60 * 1000 + 1); + + // Refresh now fails on every URL/attempt: the stale-but-usable value must + // be returned instead of null so the SSO button keeps rendering. + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network down'))); + const pending = discoverOAuth('https://stale.example.com', { validateEndpoint }); + await vi.advanceTimersByTimeAsync(1000); // fire the retry backoff timer + const second = await pending; + + expect(second).toEqual(VALID_METADATA); + expect(warnSpy).toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it('negative-caches a total failure (no cached value) to avoid hammering the IdP', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const fetchMock = vi.fn().mockRejectedValue(new Error('network down')); + vi.stubGlobal('fetch', fetchMock); + + const first = await discoverOAuth('https://down.example.com', { validateEndpoint }); + const callsAfterFirst = fetchMock.mock.calls.length; + const second = await discoverOAuth('https://down.example.com', { validateEndpoint }); + + expect(first).toBeNull(); + expect(second).toBeNull(); + // The immediate second call is short-circuited by the negative cache, so no + // additional fetches are made. + expect(fetchMock).toHaveBeenCalledTimes(callsAfterFirst); + }); }); diff --git a/lib/oauth/discovery.ts b/lib/oauth/discovery.ts index 8a84ac68..75c0e226 100644 --- a/lib/oauth/discovery.ts +++ b/lib/oauth/discovery.ts @@ -21,6 +21,26 @@ const CACHE_TTL_MS = 10 * 60 * 1000; const CACHE_MAX_ENTRIES = 64; const metadataCache = new Map(); +// --- Discovery hardening --------------------------------------------------- +// The login page's "Sign in with SSO" button is gated on OIDC discovery +// succeeding. An un-timed, un-retried fetch that dropped its cached value on +// failure let a single transient blip to the IdP silently hide the button. +// A per-fetch timeout, one retry, and serving stale-but-usable metadata on +// failure keep the button up through a transient blip. +const DISCOVERY_TIMEOUT_MS = 4000; +const DISCOVERY_RETRIES = 1; +const DISCOVERY_RETRY_DELAY_MS = 300; +// When discovery fails, remember the outcome briefly so repeated login-page +// loads during an outage don't hammer the IdP. Also throttles re-discovery +// while serving stale metadata. Kept short so recovery is fast. +const DISCOVERY_FAILURE_TTL_MS = 15 * 1000; + +// Records recent failures for serverUrls that have no cached metadata to serve. +const negativeCache = new Map(); + +const sleep = (ms: number): Promise => + new Promise((resolve) => setTimeout(resolve, ms)); + function rememberMetadata(serverUrl: string, metadata: OAuthMetadata): void { // Bound the cache so callers that can supply arbitrary serverUrl values // (e.g. unauthenticated routes that fall back to user input) cannot @@ -33,6 +53,16 @@ function rememberMetadata(serverUrl: string, metadata: OAuthMetadata): void { metadataCache.set(serverUrl, { metadata, expiresAt: Date.now() + CACHE_TTL_MS }); } +function rememberFailure(serverUrl: string): void { + // Bound like metadataCache: a user-supplied serverUrl must not grow this map + // without limit. + if (negativeCache.size >= CACHE_MAX_ENTRIES) { + const oldest = negativeCache.keys().next().value; + if (oldest !== undefined) negativeCache.delete(oldest); + } + negativeCache.set(serverUrl, Date.now() + DISCOVERY_FAILURE_TTL_MS); +} + // Endpoints come from an attacker-controllable JSON document when callers pass // a user-supplied serverUrl (e.g. /api/auth/totp-token-exchange under // allowCustomJmapEndpoint). Without a validator, a malicious metadata document @@ -52,24 +82,18 @@ async function endpointsArePublic( return true; } -export async function discoverOAuth( - serverUrl: string, - options?: DiscoverOAuthOptions, +// One pass over the well-known documents. Returns usable metadata, or null +// (pushing diagnostics into `errors`) when neither URL yields a public, +// complete document. Each fetch is bounded by a timeout so an unresponsive IdP +// can never hang the request (and, with it, the login page's SSO button). +async function attemptDiscovery( + urls: string[], + validate: EndpointValidator | undefined, + errors: string[], ): Promise { - const cached = metadataCache.get(serverUrl); - if (cached && cached.expiresAt > Date.now()) return cached.metadata; - if (cached) metadataCache.delete(serverUrl); - - const urls = [ - `${serverUrl}/.well-known/oauth-authorization-server`, - `${serverUrl}/.well-known/openid-configuration`, - ]; - - const errors: string[] = []; - for (const url of urls) { try { - const response = await fetch(url); + const response = await fetch(url, { signal: AbortSignal.timeout(DISCOVERY_TIMEOUT_MS) }); if (!response.ok) { errors.push(`${url} returned HTTP ${response.status}`); continue; @@ -82,20 +106,18 @@ export async function discoverOAuth( data.token_endpoint, data.revocation_endpoint, data.end_session_endpoint, - ], options?.validateEndpoint); + ], validate); if (!allPublic) { errors.push(`${url} returned non-public or invalid endpoint URL`); continue; } - const metadata: OAuthMetadata = { + return { issuer: data.issuer, authorization_endpoint: data.authorization_endpoint, token_endpoint: data.token_endpoint, revocation_endpoint: data.revocation_endpoint, end_session_endpoint: data.end_session_endpoint, }; - rememberMetadata(serverUrl, metadata); - return metadata; } errors.push(`${url} response missing required endpoints`); } catch (err) { @@ -103,7 +125,51 @@ export async function discoverOAuth( continue; } } + return null; +} +export async function discoverOAuth( + serverUrl: string, + options?: DiscoverOAuthOptions, +): Promise { + const cached = metadataCache.get(serverUrl); + if (cached && cached.expiresAt > Date.now()) return cached.metadata; + // A stale entry is deliberately retained (not deleted) so it can be served + // as a fallback below if the refresh fails - this keeps the SSO button up + // through a transient IdP blip. + + // Nothing to serve and we failed recently: skip hammering the IdP. + if (!cached) { + const retryAfter = negativeCache.get(serverUrl); + if (retryAfter !== undefined && retryAfter > Date.now()) return null; + } + + const urls = [ + `${serverUrl}/.well-known/oauth-authorization-server`, + `${serverUrl}/.well-known/openid-configuration`, + ]; + + const errors: string[] = []; + for (let attempt = 0; attempt <= DISCOVERY_RETRIES; attempt++) { + if (attempt > 0) await sleep(DISCOVERY_RETRY_DELAY_MS); + const metadata = await attemptDiscovery(urls, options?.validateEndpoint, errors); + if (metadata) { + rememberMetadata(serverUrl, metadata); + negativeCache.delete(serverUrl); + return metadata; + } + } + + // Every attempt failed. Prefer stale-but-usable metadata over nothing so the + // login page keeps rendering the SSO button during the outage; throttle the + // next re-discovery so we don't retry on every request. + if (cached) { + cached.expiresAt = Date.now() + DISCOVERY_FAILURE_TTL_MS; + console.warn(`[OAuth] Discovery refresh failed for ${serverUrl}; serving stale metadata: ${errors.join('; ')}`); + return cached.metadata; + } + + rememberFailure(serverUrl); console.error(`[OAuth] Discovery failed for ${serverUrl}: ${errors.join('; ')}`); return null; }