From e4a9027b30f702a8fe77fc52e24e674dbda19f07 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Mon, 19 Oct 2026 01:32:24 +0200 Subject: [PATCH] fix: stop OAuth discovery from reading a well-known document off another host --- lib/__tests__/oauth-discovery.test.ts | 24 ++++++++++++++++++++++++ lib/oauth/discovery.ts | 26 +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/lib/__tests__/oauth-discovery.test.ts b/lib/__tests__/oauth-discovery.test.ts index 63f95c961..8697c7779 100644 --- a/lib/__tests__/oauth-discovery.test.ts +++ b/lib/__tests__/oauth-discovery.test.ts @@ -35,6 +35,30 @@ describe('oauth/discovery', () => { discoverOAuth = mod.discoverOAuth; }); + it('does not read a well-known document from another host the configured one redirects to', async () => { + // The endpoints inside the document were validated, but the document + // itself followed redirects anywhere. + const redirect = { status: 302, ok: false, headers: new Headers({ location: 'https://evil.example/.well-known/openid-configuration' }), json: async () => ({}) }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(redirect)); + const result = await discoverOAuth('https://mail.example.com', { validateEndpoint }); + expect(result).toBeNull(); + for (const call of (fetch as unknown as { mock: { calls: unknown[][] } }).mock.calls) { + expect(String(call[0])).toContain('mail.example.com'); + } + }); + + it('follows one redirect that stays on the same host', async () => { + const redirect = { status: 301, ok: false, headers: new Headers({ location: '/auth/.well-known/openid-configuration' }), json: async () => ({}) }; + const document = { status: 200, ok: true, headers: new Headers(), json: async () => VALID_METADATA }; + vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce(redirect).mockResolvedValueOnce(document)); + const result = await discoverOAuth('https://mail.example.com', { validateEndpoint }); + expect(result).toEqual(VALID_METADATA); + expect(fetch).toHaveBeenLastCalledWith( + 'https://mail.example.com/auth/.well-known/openid-configuration', + expect.objectContaining({ redirect: 'error' }), + ); + }); + it('discovers metadata from oauth-authorization-server', async () => { vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce({ ok: true, diff --git a/lib/oauth/discovery.ts b/lib/oauth/discovery.ts index d4117c6b0..e4964d459 100644 --- a/lib/oauth/discovery.ts +++ b/lib/oauth/discovery.ts @@ -84,6 +84,30 @@ async function findRejectedEndpoint( return null; } +/** + * Fetch a well-known document without following redirects anywhere: the + * endpoints inside it are validated, but the document itself was fetched + * with `fetch`'s default `follow`, so a configured host that answered with a + * redirect had the metadata read from wherever it pointed. One hop is + * followed when it stays on the same host (an http -> https upgrade or a + * path change); anything else is an error. + */ +async function fetchDiscoveryDocument(url: string): Promise { + const first = await fetch(url, { signal: AbortSignal.timeout(DISCOVERY_TIMEOUT_MS), redirect: 'manual' }); + const status = typeof first.status === 'number' ? first.status : 200; + if (status < 300 || status >= 400) return first; + const location = first.headers.get('location'); + if (!location) throw new Error(`${url} redirected without a Location header`); + const next = new URL(location, url); + const from = new URL(url); + const sameHost = next.hostname === from.hostname; + const noDowngrade = next.protocol === 'https:' || next.protocol === from.protocol; + if (!sameHost || !noDowngrade) { + throw new Error(`${url} redirects to ${next.origin}, which is another host`); + } + return fetch(next.toString(), { signal: AbortSignal.timeout(DISCOVERY_TIMEOUT_MS), redirect: 'error' }); +} + // 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 @@ -95,7 +119,7 @@ async function attemptDiscovery( ): Promise { for (const url of urls) { try { - const response = await fetch(url, { signal: AbortSignal.timeout(DISCOVERY_TIMEOUT_MS) }); + const response = await fetchDiscoveryDocument(url); if (!response.ok) { errors.push(`${url} returned HTTP ${response.status}`); continue;