From 94f89954b4e177831c2ce68df02f896c517d40d9 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Jul 2026 21:21:03 +0100 Subject: [PATCH 1/4] fix(oauth): route agent flow requests through safe fetch --- server/src/routes/agent-oauth.ts | 18 +++- server/src/routes/helpers/oauth-safe-fetch.ts | 48 +++++++++++ .../unit/agent-oauth-membership-guard.test.ts | 63 +++++++++++++- server/tests/unit/oauth-safe-fetch.test.ts | 85 +++++++++++++++++++ 4 files changed, 208 insertions(+), 6 deletions(-) create mode 100644 server/src/routes/helpers/oauth-safe-fetch.ts create mode 100644 server/tests/unit/oauth-safe-fetch.test.ts diff --git a/server/src/routes/agent-oauth.ts b/server/src/routes/agent-oauth.ts index 9cbe4c0df0..694184109f 100644 --- a/server/src/routes/agent-oauth.ts +++ b/server/src/routes/agent-oauth.ts @@ -45,6 +45,7 @@ import { requireAuth } from '../middleware/auth.js'; import { AgentContextDatabase } from '../db/agent-context-db.js'; import { getWorkos } from '../auth/workos-client.js'; import { createWebOAuthAdapters, AgentOAuthPendingFlowStore } from './helpers/web-oauth-stores.js'; +import { oauthSafeFetch } from './helpers/oauth-safe-fetch.js'; import { validateExternalUrl } from '../utils/url-security.js'; const logger = createLogger('agent-oauth'); @@ -72,7 +73,11 @@ async function durableOAuthScopeHintForAgent(agentUrl: string): Promise { + if (typeof input !== 'string' && !(input instanceof URL)) { + throw new Error('OAuth safe fetch only supports string and URL inputs'); + } + + const method = (init?.method ?? 'GET').toUpperCase(); + if (method !== 'GET' && method !== 'POST') { + throw new Error(`OAuth safe fetch does not support ${method} requests`); + } + + const headers = Object.fromEntries(new Headers(init?.headers).entries()); + const rawBody = init?.body; + let body: string | Uint8Array | undefined; + + if (rawBody === undefined || rawBody === null) { + body = undefined; + } else if (typeof rawBody === 'string' || rawBody instanceof Uint8Array) { + body = rawBody; + } else if (rawBody instanceof URLSearchParams) { + body = rawBody.toString(); + } else { + throw new Error( + 'OAuth safe fetch only supports string, Uint8Array, and URLSearchParams request bodies', + ); + } + + if (method === 'GET' && body !== undefined) { + throw new Error('OAuth safe fetch GET requests cannot carry a body'); + } + if (method === 'POST' && body === undefined) { + throw new Error('OAuth safe fetch POST requests require a body'); + } + + return safeFetch(input.toString(), { + method, + headers, + ...(body !== undefined && { body }), + ...(init?.signal && { signal: init.signal }), + }); +}; diff --git a/server/tests/unit/agent-oauth-membership-guard.test.ts b/server/tests/unit/agent-oauth-membership-guard.test.ts index b66bc1deb2..798826185c 100644 --- a/server/tests/unit/agent-oauth-membership-guard.test.ts +++ b/server/tests/unit/agent-oauth-membership-guard.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import express from 'express'; +import cookieParser from 'cookie-parser'; import request from 'supertest'; import type { PendingWebFlow, PendingWebFlowStore } from '@adcp/sdk/auth'; @@ -39,6 +40,7 @@ const agentContextDbMocks = vi.hoisted(() => ({ getOAuthClient: vi.fn(), clearOAuthClient: vi.fn(), removeOAuthTokens: vi.fn(), + hasValidOAuthTokens: vi.fn(), }, })); const adapterMocks = vi.hoisted(() => ({ @@ -106,6 +108,7 @@ import { createAgentOAuthRouter, createMembershipGuardedPendingFlowStore, } from '../../src/routes/agent-oauth.js'; +import { oauthSafeFetch } from '../../src/routes/helpers/oauth-safe-fetch.js'; const TEST_USER_ID = 'user_123'; const TEST_ORG_ID = 'org_123'; @@ -114,6 +117,7 @@ const TEST_AGENT_URL = 'https://agent.example.com/mcp'; function makeApp() { const app = express(); + app.use(cookieParser()); app.use('/api/oauth/agent', createAgentOAuthRouter()); return app; } @@ -130,6 +134,7 @@ beforeEach(() => { agentContextDbMocks.instance.getOAuthClient.mockReset(); agentContextDbMocks.instance.clearOAuthClient.mockReset(); agentContextDbMocks.instance.removeOAuthTokens.mockReset(); + agentContextDbMocks.instance.hasValidOAuthTokens.mockReset(); adapterMocks.pendingFlowStore.put.mockReset(); adapterMocks.pendingFlowStore.consume.mockReset(); adapterMocks.agentStorage.loadAgent.mockReset(); @@ -144,6 +149,7 @@ beforeEach(() => { agent_url: TEST_AGENT_URL, }); agentContextDbMocks.instance.getOAuthClient.mockResolvedValue(null); + agentContextDbMocks.instance.hasValidOAuthTokens.mockReturnValue(false); adapterMocks.createWebOAuthAdapters.mockReturnValue({ pendingFlowStore: adapterMocks.pendingFlowStore, agentStorage: adapterMocks.agentStorage, @@ -242,10 +248,18 @@ describe('GET /api/oauth/agent/start durable scope hint', () => { expect(res.status).toBe(302); expect(res.headers.location).toBe('https://auth.example.com/authorize'); - expect(mcpAuthMocks.discoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith(TEST_AGENT_URL); - expect(mcpAuthMocks.discoverAuthorizationServerMetadata).toHaveBeenCalledWith('https://auth.example.com'); + expect(mcpAuthMocks.discoverOAuthProtectedResourceMetadata).toHaveBeenCalledWith( + TEST_AGENT_URL, + undefined, + oauthSafeFetch, + ); + expect(mcpAuthMocks.discoverAuthorizationServerMetadata).toHaveBeenCalledWith( + 'https://auth.example.com', + { fetchFn: oauthSafeFetch }, + ); expect(sdkMocks.startWebOAuthFlow).toHaveBeenCalledWith(expect.objectContaining({ agent: expect.objectContaining({ id: AGENT_CONTEXT_ID, agent_uri: TEST_AGENT_URL }), + fetch: oauthSafeFetch, scopeHint: 'openid profile offline_access', clientMetadata: { scope: 'openid profile offline_access' }, carry: expect.objectContaining({ @@ -289,7 +303,10 @@ describe('GET /api/oauth/agent/start durable scope hint', () => { .query({ agent_context_id: AGENT_CONTEXT_ID }) .expect(302); - expect(mcpAuthMocks.discoverAuthorizationServerMetadata).toHaveBeenCalledWith(TEST_AGENT_URL); + expect(mcpAuthMocks.discoverAuthorizationServerMetadata).toHaveBeenCalledWith( + TEST_AGENT_URL, + { fetchFn: oauthSafeFetch }, + ); expect(sdkMocks.startWebOAuthFlow).toHaveBeenCalledWith(expect.objectContaining({ scopeHint: 'offline_access', clientMetadata: { scope: 'offline_access' }, @@ -315,6 +332,46 @@ describe('GET /api/oauth/agent/start durable scope hint', () => { }); }); +describe('agent OAuth safe fetch injection', () => { + it('passes the scoped fetcher to callback token exchange', async () => { + sdkMocks.completeWebOAuthFlow.mockResolvedValueOnce({ + agentId: AGENT_CONTEXT_ID, + agentUrl: TEST_AGENT_URL, + tokens: { access_token: 'token', token_type: 'bearer' }, + carry: { organization_id: TEST_ORG_ID }, + persisted: true, + }); + + await request(makeApp()) + .get('/api/oauth/agent/callback') + .set('Cookie', 'adcp_oauth_state=state_123') + .query({ code: 'code_123', state: 'state_123' }) + .expect(302); + + expect(sdkMocks.completeWebOAuthFlow).toHaveBeenCalledWith(expect.objectContaining({ + state: 'state_123', + code: 'code_123', + expectedState: 'state_123', + fetch: oauthSafeFetch, + })); + }); + + it('passes the scoped fetcher to status discovery', async () => { + sdkMocks.discoverOAuthMetadata.mockResolvedValueOnce({ + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + }); + + await request(makeApp()) + .get(`/api/oauth/agent/${AGENT_CONTEXT_ID}/status`) + .expect(200); + + expect(sdkMocks.discoverOAuthMetadata).toHaveBeenCalledWith(TEST_AGENT_URL, { + fetch: oauthSafeFetch, + }); + }); +}); + describe('buildDurableOAuthScopeHint', () => { it('preserves advertised scopes and appends offline_access', () => { expect(buildDurableOAuthScopeHint(['openid', 'profile', 'email'])).toBe('openid profile email offline_access'); diff --git a/server/tests/unit/oauth-safe-fetch.test.ts b/server/tests/unit/oauth-safe-fetch.test.ts new file mode 100644 index 0000000000..7260257bb5 --- /dev/null +++ b/server/tests/unit/oauth-safe-fetch.test.ts @@ -0,0 +1,85 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const urlSecurityMocks = vi.hoisted(() => ({ + safeFetch: vi.fn(), +})); + +vi.mock('../../src/utils/url-security.js', () => ({ + safeFetch: urlSecurityMocks.safeFetch, +})); + +import { oauthSafeFetch } from '../../src/routes/helpers/oauth-safe-fetch.js'; + +describe('oauthSafeFetch', () => { + beforeEach(() => { + urlSecurityMocks.safeFetch.mockReset(); + urlSecurityMocks.safeFetch.mockResolvedValue(new Response('{}')); + }); + + it('normalizes a URL, Headers, and signal for GET discovery requests', async () => { + const signal = new AbortController().signal; + const headers = new Headers([ + ['Accept', 'application/json'], + ['X-Protocol-Version', '2026-06-18'], + ]); + + await oauthSafeFetch(new URL('https://agent.example.com/.well-known/oauth'), { + headers, + signal, + }); + + expect(urlSecurityMocks.safeFetch).toHaveBeenCalledWith( + 'https://agent.example.com/.well-known/oauth', + { + method: 'GET', + headers: { + accept: 'application/json', + 'x-protocol-version': '2026-06-18', + }, + signal, + }, + ); + }); + + it.each([ + ['string', '{"redirect_uris":[]}', '{"redirect_uris":[]}'], + ['Uint8Array', new Uint8Array([1, 2, 3]), new Uint8Array([1, 2, 3])], + [ + 'URLSearchParams', + new URLSearchParams({ grant_type: 'authorization_code', code: 'abc' }), + 'grant_type=authorization_code&code=abc', + ], + ])('normalizes a %s POST body', async (_name, inputBody, expectedBody) => { + await oauthSafeFetch('https://auth.example.com/token', { + method: 'post', + headers: [['Content-Type', 'application/x-www-form-urlencoded']], + body: inputBody, + }); + + expect(urlSecurityMocks.safeFetch).toHaveBeenCalledWith( + 'https://auth.example.com/token', + { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: expectedBody, + }, + ); + }); + + it('rejects request inputs, methods, and bodies outside the OAuth adapter surface', async () => { + await expect(oauthSafeFetch(new Request('https://agent.example.com'))) + .rejects.toThrow('only supports string and URL inputs'); + await expect(oauthSafeFetch('https://agent.example.com', { method: 'DELETE' })) + .rejects.toThrow('does not support DELETE requests'); + await expect(oauthSafeFetch('https://agent.example.com', { body: 'unexpected' })) + .rejects.toThrow('GET requests cannot carry a body'); + await expect(oauthSafeFetch('https://agent.example.com', { method: 'POST' })) + .rejects.toThrow('POST requests require a body'); + await expect(oauthSafeFetch('https://agent.example.com', { + method: 'POST', + body: new Blob(['unsupported']), + })).rejects.toThrow('only supports string, Uint8Array, and URLSearchParams'); + + expect(urlSecurityMocks.safeFetch).not.toHaveBeenCalled(); + }); +}); From 4e4a4cd50651717675b250984cccac9f497f3367 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Jul 2026 21:27:05 +0100 Subject: [PATCH 2/4] test(oauth): inject callback state without cookie middleware --- .../tests/unit/agent-oauth-membership-guard.test.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/server/tests/unit/agent-oauth-membership-guard.test.ts b/server/tests/unit/agent-oauth-membership-guard.test.ts index 798826185c..a16a11d03d 100644 --- a/server/tests/unit/agent-oauth-membership-guard.test.ts +++ b/server/tests/unit/agent-oauth-membership-guard.test.ts @@ -1,6 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import express from 'express'; -import cookieParser from 'cookie-parser'; import request from 'supertest'; import type { PendingWebFlow, PendingWebFlowStore } from '@adcp/sdk/auth'; @@ -115,9 +114,14 @@ const TEST_ORG_ID = 'org_123'; const AGENT_CONTEXT_ID = '11111111-1111-4111-8111-111111111111'; const TEST_AGENT_URL = 'https://agent.example.com/mcp'; -function makeApp() { +function makeApp(stateCookie?: string) { const app = express(); - app.use(cookieParser()); + if (stateCookie) { + app.use((req, _res, next) => { + req.cookies = { adcp_oauth_state: stateCookie }; + next(); + }); + } app.use('/api/oauth/agent', createAgentOAuthRouter()); return app; } @@ -342,9 +346,8 @@ describe('agent OAuth safe fetch injection', () => { persisted: true, }); - await request(makeApp()) + await request(makeApp('state_123')) .get('/api/oauth/agent/callback') - .set('Cookie', 'adcp_oauth_state=state_123') .query({ code: 'code_123', state: 'state_123' }) .expect(302); From 50627bbc04f013c53dfd6a2bfc09deeea435511e Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Tue, 21 Jul 2026 21:34:23 +0100 Subject: [PATCH 3/4] fix(oauth): secure client credential token exchange --- server/src/routes/agent-oauth.ts | 2 +- .../oauth-client-credentials-exchange.ts | 3 +- .../helpers => utils}/oauth-safe-fetch.ts | 10 +-- .../unit/agent-oauth-membership-guard.test.ts | 2 +- .../oauth-client-credentials-exchange.test.ts | 72 +++++++++++++------ server/tests/unit/oauth-safe-fetch.test.ts | 2 +- 6 files changed, 62 insertions(+), 29 deletions(-) rename server/src/{routes/helpers => utils}/oauth-safe-fetch.ts (79%) diff --git a/server/src/routes/agent-oauth.ts b/server/src/routes/agent-oauth.ts index 694184109f..6d14d3832a 100644 --- a/server/src/routes/agent-oauth.ts +++ b/server/src/routes/agent-oauth.ts @@ -45,7 +45,7 @@ import { requireAuth } from '../middleware/auth.js'; import { AgentContextDatabase } from '../db/agent-context-db.js'; import { getWorkos } from '../auth/workos-client.js'; import { createWebOAuthAdapters, AgentOAuthPendingFlowStore } from './helpers/web-oauth-stores.js'; -import { oauthSafeFetch } from './helpers/oauth-safe-fetch.js'; +import { oauthSafeFetch } from '../utils/oauth-safe-fetch.js'; import { validateExternalUrl } from '../utils/url-security.js'; const logger = createLogger('agent-oauth'); diff --git a/server/src/services/oauth-client-credentials-exchange.ts b/server/src/services/oauth-client-credentials-exchange.ts index 5999d2e27a..5e1cbb9757 100644 --- a/server/src/services/oauth-client-credentials-exchange.ts +++ b/server/src/services/oauth-client-credentials-exchange.ts @@ -25,6 +25,7 @@ import type { OAuthClientCredentials } from '../db/agent-context-db.js'; import { createLogger } from '../logger.js'; +import { oauthSafeFetch } from '../utils/oauth-safe-fetch.js'; const logger = createLogger('oauth-client-credentials-exchange'); @@ -100,7 +101,7 @@ export async function exchangeClientCredentials( headers['Authorization'] = `Basic ${encoded}`; } - const fetchImpl = options.fetchImpl ?? fetch; + const fetchImpl = options.fetchImpl ?? oauthSafeFetch; const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), EXCHANGE_TIMEOUT_MS); diff --git a/server/src/routes/helpers/oauth-safe-fetch.ts b/server/src/utils/oauth-safe-fetch.ts similarity index 79% rename from server/src/routes/helpers/oauth-safe-fetch.ts rename to server/src/utils/oauth-safe-fetch.ts index 6eaaec05ad..9fbffba5d7 100644 --- a/server/src/routes/helpers/oauth-safe-fetch.ts +++ b/server/src/utils/oauth-safe-fetch.ts @@ -1,10 +1,10 @@ -import { safeFetch } from '../../utils/url-security.js'; +import { safeFetch } from './url-security.js'; /** - * Fetch adapter for the OAuth libraries used by the agent authorization - * routes. OAuth discovery, dynamic registration, and token exchange only use - * GET and POST requests, so keep this surface deliberately narrow while - * routing every request and redirect hop through the SSRF-safe fetcher. + * Fetch adapter for server-side OAuth requests. OAuth discovery, dynamic + * registration, and token exchange only use GET and POST requests, so keep + * this surface deliberately narrow while routing every request and redirect + * hop through the SSRF-safe fetcher. */ export const oauthSafeFetch: typeof fetch = async (input, init) => { if (typeof input !== 'string' && !(input instanceof URL)) { diff --git a/server/tests/unit/agent-oauth-membership-guard.test.ts b/server/tests/unit/agent-oauth-membership-guard.test.ts index a16a11d03d..e779f28f9f 100644 --- a/server/tests/unit/agent-oauth-membership-guard.test.ts +++ b/server/tests/unit/agent-oauth-membership-guard.test.ts @@ -107,7 +107,7 @@ import { createAgentOAuthRouter, createMembershipGuardedPendingFlowStore, } from '../../src/routes/agent-oauth.js'; -import { oauthSafeFetch } from '../../src/routes/helpers/oauth-safe-fetch.js'; +import { oauthSafeFetch } from '../../src/utils/oauth-safe-fetch.js'; const TEST_USER_ID = 'user_123'; const TEST_ORG_ID = 'org_123'; diff --git a/server/tests/unit/oauth-client-credentials-exchange.test.ts b/server/tests/unit/oauth-client-credentials-exchange.test.ts index 1d72bbbe67..faa44dcc13 100644 --- a/server/tests/unit/oauth-client-credentials-exchange.test.ts +++ b/server/tests/unit/oauth-client-credentials-exchange.test.ts @@ -1,4 +1,13 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; + +const oauthFetchMocks = vi.hoisted(() => ({ + oauthSafeFetch: vi.fn(), +})); + +vi.mock('../../src/utils/oauth-safe-fetch.js', () => ({ + oauthSafeFetch: oauthFetchMocks.oauthSafeFetch, +})); + import { exchangeClientCredentials, resolveEnvReference, @@ -35,6 +44,10 @@ function mockFetch(response: { status: number; body: string | object }) { }); } +beforeEach(() => { + oauthFetchMocks.oauthSafeFetch.mockReset(); +}); + describe('resolveEnvReference', () => { beforeEach(() => { delete process.env.ADCP_OAUTH_TEST; @@ -78,6 +91,30 @@ describe('exchangeClientCredentials', () => { client_secret: 'secret-xyz', }; + it('uses the SSRF-safe OAuth fetcher by default and preserves the token request', async () => { + oauthFetchMocks.oauthSafeFetch.mockImplementation(mockFetch({ + status: 200, + body: { access_token: 'safe-token', expires_in: 900 }, + })); + + const result = await exchangeClientCredentials(baseCreds); + + expect(result).toEqual({ ok: true, access_token: 'safe-token', expires_in: 900 }); + expect(oauthFetchMocks.oauthSafeFetch).toHaveBeenCalledWith( + baseCreds.token_endpoint, + { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + Accept: 'application/json', + Authorization: expect.stringMatching(/^Basic /), + }, + body: 'grant_type=client_credentials', + signal: expect.any(AbortSignal), + }, + ); + }); + it('returns a bearer token from a successful exchange', async () => { const fetchImpl = mockFetch({ status: 200, @@ -203,12 +240,10 @@ describe('adaptAuthForSdk', () => { it('exchanges oauth_client_credentials and narrows to bearer', async () => { process.env.ADCP_OAUTH_BRIDGE_TEST = 'sec'; - // Swap global fetch for this test only - const original = globalThis.fetch; - globalThis.fetch = mockFetch({ + oauthFetchMocks.oauthSafeFetch.mockImplementation(mockFetch({ status: 200, body: { access_token: 'exchanged-tok', expires_in: 600 }, - }) as unknown as typeof fetch; + })); try { const result = await adaptAuthForSdk({ type: 'oauth_client_credentials', @@ -220,26 +255,23 @@ describe('adaptAuthForSdk', () => { }); expect(result).toEqual({ type: 'bearer', token: 'exchanged-tok' }); } finally { - globalThis.fetch = original; delete process.env.ADCP_OAUTH_BRIDGE_TEST; } }); it('returns undefined (falls back to unauthenticated) when the exchange fails', async () => { - const original = globalThis.fetch; - globalThis.fetch = mockFetch({ status: 500, body: 'internal' }) as unknown as typeof fetch; - try { - const result = await adaptAuthForSdk({ - type: 'oauth_client_credentials', - credentials: { - token_endpoint: 'https://idp.example/token', - client_id: 'c', - client_secret: 's', - }, - }); - expect(result).toBeUndefined(); - } finally { - globalThis.fetch = original; - } + oauthFetchMocks.oauthSafeFetch.mockImplementation( + mockFetch({ status: 500, body: 'internal' }), + ); + + const result = await adaptAuthForSdk({ + type: 'oauth_client_credentials', + credentials: { + token_endpoint: 'https://idp.example/token', + client_id: 'c', + client_secret: 's', + }, + }); + expect(result).toBeUndefined(); }); }); diff --git a/server/tests/unit/oauth-safe-fetch.test.ts b/server/tests/unit/oauth-safe-fetch.test.ts index 7260257bb5..6071527c45 100644 --- a/server/tests/unit/oauth-safe-fetch.test.ts +++ b/server/tests/unit/oauth-safe-fetch.test.ts @@ -8,7 +8,7 @@ vi.mock('../../src/utils/url-security.js', () => ({ safeFetch: urlSecurityMocks.safeFetch, })); -import { oauthSafeFetch } from '../../src/routes/helpers/oauth-safe-fetch.js'; +import { oauthSafeFetch } from '../../src/utils/oauth-safe-fetch.js'; describe('oauthSafeFetch', () => { beforeEach(() => { From 37ddedad3173daed502e374e833f642cae961da9 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 23 Jul 2026 21:17:28 +0100 Subject: [PATCH 4/4] fix(oauth): block redirects for credential posts (#5980) --- server/src/utils/oauth-safe-fetch.ts | 5 +- server/tests/unit/oauth-safe-fetch.test.ts | 1 + .../tests/unit/url-security-redirects.test.ts | 68 +++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/server/src/utils/oauth-safe-fetch.ts b/server/src/utils/oauth-safe-fetch.ts index 9fbffba5d7..a56267f064 100644 --- a/server/src/utils/oauth-safe-fetch.ts +++ b/server/src/utils/oauth-safe-fetch.ts @@ -4,7 +4,9 @@ import { safeFetch } from './url-security.js'; * Fetch adapter for server-side OAuth requests. OAuth discovery, dynamic * registration, and token exchange only use GET and POST requests, so keep * this surface deliberately narrow while routing every request and redirect - * hop through the SSRF-safe fetcher. + * hop through the SSRF-safe fetcher. Credential-bearing POST requests never + * follow redirects; OAuth token and dynamic-registration endpoints are not + * expected to redirect, and forwarding their headers or bodies is unsafe. */ export const oauthSafeFetch: typeof fetch = async (input, init) => { if (typeof input !== 'string' && !(input instanceof URL)) { @@ -43,6 +45,7 @@ export const oauthSafeFetch: typeof fetch = async (input, init) => { method, headers, ...(body !== undefined && { body }), + ...(method === 'POST' && { maxRedirects: 0 }), ...(init?.signal && { signal: init.signal }), }); }; diff --git a/server/tests/unit/oauth-safe-fetch.test.ts b/server/tests/unit/oauth-safe-fetch.test.ts index 6071527c45..b10b4315fa 100644 --- a/server/tests/unit/oauth-safe-fetch.test.ts +++ b/server/tests/unit/oauth-safe-fetch.test.ts @@ -62,6 +62,7 @@ describe('oauthSafeFetch', () => { method: 'POST', headers: { 'content-type': 'application/x-www-form-urlencoded' }, body: expectedBody, + maxRedirects: 0, }, ); }); diff --git a/server/tests/unit/url-security-redirects.test.ts b/server/tests/unit/url-security-redirects.test.ts index a8ff121a59..20f415f894 100644 --- a/server/tests/unit/url-security-redirects.test.ts +++ b/server/tests/unit/url-security-redirects.test.ts @@ -26,6 +26,74 @@ vi.mock('dns/promises', () => ({ })); import { safeFetch } from '../../src/utils/url-security.js'; +import { oauthSafeFetch } from '../../src/utils/oauth-safe-fetch.js'; + +describe('oauthSafeFetch redirect policy', () => { + beforeEach(() => { + fetchMock.mockReset(); + agentMock.mockClear(); + resolve4Mock.mockClear(); + resolve6Mock.mockClear(); + resolve4Mock.mockResolvedValue(['93.184.216.34']); + resolve6Mock.mockResolvedValue([]); + }); + + it.each([302, 307, 308])( + 'does not forward credential-bearing POST data across a %i redirect', + async (status) => { + fetchMock.mockResolvedValueOnce(new Response('', { + status, + headers: { Location: 'https://attacker.example/collect' }, + })); + + const response = await oauthSafeFetch('https://auth.example/token', { + method: 'POST', + headers: { + Authorization: 'Basic client-secret', + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body: new URLSearchParams({ + grant_type: 'authorization_code', + code: 'authorization-code', + code_verifier: 'pkce-verifier', + }), + }); + + expect(response.status).toBe(status); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock.mock.calls[0][0]).toBe('https://auth.example/token'); + expect(fetchMock.mock.calls[0][1]).toMatchObject({ + method: 'POST', + headers: { + authorization: 'Basic client-secret', + 'content-type': 'application/x-www-form-urlencoded', + }, + body: 'grant_type=authorization_code&code=authorization-code&code_verifier=pkce-verifier', + }); + }, + ); + + it('continues to follow same-origin discovery GET redirects', async () => { + fetchMock + .mockResolvedValueOnce(new Response('', { + status: 302, + headers: { Location: '/oauth-configuration' }, + })) + .mockResolvedValueOnce(new Response('{"token_endpoint":"https://auth.example/token"}', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + })); + + const response = await oauthSafeFetch( + 'https://auth.example/.well-known/oauth-authorization-server', + ); + + expect(response.status).toBe(200); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(fetchMock.mock.calls[1][0]).toBe('https://auth.example/oauth-configuration'); + expect(fetchMock.mock.calls[1][1]).toMatchObject({ method: 'GET' }); + }); +}); describe('safeFetch redirects', () => { beforeEach(() => {