diff --git a/server/src/routes/agent-oauth.ts b/server/src/routes/agent-oauth.ts index 9cbe4c0df0..6d14d3832a 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 '../utils/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 controller.abort(), EXCHANGE_TIMEOUT_MS); diff --git a/server/src/utils/oauth-safe-fetch.ts b/server/src/utils/oauth-safe-fetch.ts new file mode 100644 index 0000000000..a56267f064 --- /dev/null +++ b/server/src/utils/oauth-safe-fetch.ts @@ -0,0 +1,51 @@ +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. 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)) { + 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 }), + ...(method === 'POST' && { maxRedirects: 0 }), + ...(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..e779f28f9f 100644 --- a/server/tests/unit/agent-oauth-membership-guard.test.ts +++ b/server/tests/unit/agent-oauth-membership-guard.test.ts @@ -39,6 +39,7 @@ const agentContextDbMocks = vi.hoisted(() => ({ getOAuthClient: vi.fn(), clearOAuthClient: vi.fn(), removeOAuthTokens: vi.fn(), + hasValidOAuthTokens: vi.fn(), }, })); const adapterMocks = vi.hoisted(() => ({ @@ -106,14 +107,21 @@ import { createAgentOAuthRouter, createMembershipGuardedPendingFlowStore, } from '../../src/routes/agent-oauth.js'; +import { oauthSafeFetch } from '../../src/utils/oauth-safe-fetch.js'; const TEST_USER_ID = 'user_123'; 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(); + if (stateCookie) { + app.use((req, _res, next) => { + req.cookies = { adcp_oauth_state: stateCookie }; + next(); + }); + } app.use('/api/oauth/agent', createAgentOAuthRouter()); return app; } @@ -130,6 +138,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 +153,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 +252,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 +307,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 +336,45 @@ 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('state_123')) + .get('/api/oauth/agent/callback') + .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-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 new file mode 100644 index 0000000000..b10b4315fa --- /dev/null +++ b/server/tests/unit/oauth-safe-fetch.test.ts @@ -0,0 +1,86 @@ +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/utils/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, + maxRedirects: 0, + }, + ); + }); + + 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(); + }); +}); 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(() => {