refactor(core): Migrate OAuth service requests through the shared HTTP client (#32501)

2026-06-19 07:36:52 +00:00 · 2026-06-19 08:32:05 +02:00
parent e8d548210e
commit 532669c0c4
3 changed files with 631 additions and 216 deletions
@@ -0,0 +1,201 @@
+import type { Logger } from '@n8n/backend-common';
+import { OutboundHttp, type SsrfProtectionService } from '@n8n/backend-network';
+import { type LocalServer, startServer } from '@n8n/backend-network/testing';
+import type { GlobalConfig, SsrfProtectionConfig } from '@n8n/config';
+import type { CredentialsRepository } from '@n8n/db';
+import { mock } from 'jest-mock-extended';
+import type { Cipher } from 'n8n-core';
+import type { IncomingHttpHeaders } from 'node:http';
+
+import type { AuthService } from '@/auth/auth.service';
+import type { CredentialsFinderService } from '@/credentials/credentials-finder.service';
+import type { DynamicCredentialsProxy } from '@/credentials/dynamic-credentials-proxy';
+import type { CredentialsHelper } from '@/credentials-helper';
+import type { EventService } from '@/events/event.service';
+import type { ExternalHooks } from '@/external-hooks';
+import type { OAuthBrowserBindingService } from '@/oauth/oauth-browser-binding.service';
+import type { OAuthJweServiceProxy } from '@/oauth/oauth-jwe-service.proxy';
+import { OauthService, type OAuth1CredentialData } from '@/oauth/oauth.service';
+import type { CacheService } from '@/services/cache/cache.service';
+import type { UrlService } from '@/services/url.service';
+
+interface Received {
+	method?: string;
+	url?: string;
+	headers: IncomingHttpHeaders;
+	body: string;
+}
+
+/**
+ * Builds an `OauthService` whose only real collaborator is `OutboundHttp`; every
+ * other dependency is mocked. SSRF protection is enabled (the gate flag is on),
+ * but the loopback target is permitted via the bridge, mirroring the allowlist
+ * escape hatch rather than disabling the guard.
+ */
+function buildService() {
+	const ssrf = mock<SsrfProtectionService>();
+	ssrf.validateUrl.mockResolvedValue({ ok: true, result: undefined });
+	ssrf.validateConnectionHost.mockReturnValue({ ok: true, result: undefined });
+
+	return new OauthService(
+		mock<Logger>(),
+		mock<CredentialsHelper>(),
+		mock<CredentialsRepository>(),
+		mock<CredentialsFinderService>(),
+		mock<UrlService>(),
+		mock<GlobalConfig>(),
+		mock<ExternalHooks>(),
+		mock<Cipher>(),
+		mock<DynamicCredentialsProxy>(),
+		mock<AuthService>(),
+		mock<OAuthJweServiceProxy>(),
+		mock<OAuthBrowserBindingService>(),
+		mock<EventService>(),
+		mock<CacheService>(),
+		new OutboundHttp(ssrf, mock<Logger>()),
+		ssrf,
+		mock<SsrfProtectionConfig>({ enabled: true }),
+	);
+}
+
+/**
+ * Confirms the OAuth1 access-token exchange is equivalent over a real socket once
+ * routed through `OutboundHttp` (CAT-3373): the signed Authorization header and
+ * the form-urlencoded body actually cross the wire, and the form-urlencoded
+ * string response is parsed back into token data. SSRF protection stays ON; the
+ * loopback target is permitted via the bridge, mirroring the allowlist escape
+ * hatch rather than disabling the guard.
+ */
+describe('OauthService (real HTTP round-trip)', () => {
+	let server: LocalServer;
+	let received: Received[];
+
+	beforeAll(async () => {
+		server = await startServer((req, res) => {
+			const chunks: Buffer[] = [];
+			req.on('data', (chunk: Buffer) => chunks.push(chunk));
+			req.on('end', () => {
+				received.push({
+					method: req.method,
+					url: req.url,
+					headers: req.headers,
+					body: Buffer.concat(chunks).toString(),
+				});
+				res.setHeader('content-type', 'application/x-www-form-urlencoded');
+				res.writeHead(200);
+				res.end('oauth_token=access-token&oauth_token_secret=access-secret');
+			});
+		});
+	});
+
+	afterAll(async () => await server.close());
+
+	beforeEach(() => {
+		received = [];
+		server.clear();
+	});
+
+	it('exchanges an OAuth1 request token for an access token over a real socket', async () => {
+		const service = buildService();
+		const oauthCredentials: OAuth1CredentialData = {
+			consumerKey: 'consumer_key',
+			consumerSecret: 'consumer_secret',
+			requestTokenUrl: `${server.url}/request_token`,
+			authUrl: `${server.url}/authorize`,
+			accessTokenUrl: `${server.url}/access_token`,
+			signatureMethod: 'HMAC-SHA1',
+		};
+
+		const result = await service.getOAuth1AccessToken(oauthCredentials, {
+			oauthToken: 'request-token',
+			oauthVerifier: 'verifier',
+			oauthTokenSecret: 'request-secret',
+		});
+
+		expect(result).toEqual({
+			oauth_token: 'access-token',
+			oauth_token_secret: 'access-secret',
+		});
+
+		expect(received).toHaveLength(1);
+		const [request] = received;
+		expect(request.method).toBe('POST');
+		expect(request.url).toBe('/access_token');
+		expect(request.headers['content-type']).toBe('application/x-www-form-urlencoded');
+		expect(request.headers.authorization).toMatch(/^OAuth /);
+		expect(request.body).toBe('oauth_verifier=verifier');
+	});
+});
+
+/**
+ * Confirms the OAuth2 `.well-known` discovery GET is equivalent over a real socket
+ * once routed through `OutboundHttp` (CAT-3373). This is the callsite with the
+ * subtlest behavior, so it gets a wire-level check rather than a mock:
+ *
+ * - only a 200 is accepted: a non-200 response is treated as a miss and the loop
+ *   falls through to the next candidate URL (the old `validateStatus === 200`);
+ * - a 200 body is parsed from JSON back into the metadata object (`json: true`).
+ *
+ * SSRF protection stays ON; the loopback target is permitted via the bridge.
+ */
+describe('OauthService discovery (real HTTP round-trip)', () => {
+	let server: LocalServer;
+	let received: Received[];
+
+	const PROTECTED_RESOURCE_METADATA = {
+		authorization_servers: ['https://auth.example.com'],
+		resource: 'https://api.example.com/mcp',
+		scopes_supported: ['read', 'write'],
+	};
+
+	beforeAll(async () => {
+		server = await startServer((req, res) => {
+			received.push({ method: req.method, url: req.url, headers: req.headers, body: '' });
+			// The path-specific candidate (tried first) 404s; the root candidate
+			// returns the metadata. This exercises the non-200 skip + JSON parse over
+			// the wire, in order.
+			if (req.url === '/.well-known/oauth-protected-resource') {
+				res.setHeader('content-type', 'application/json');
+				res.writeHead(200);
+				res.end(JSON.stringify(PROTECTED_RESOURCE_METADATA));
+				return;
+			}
+			res.writeHead(404);
+			res.end('not found');
+		});
+	});
+
+	afterAll(async () => await server.close());
+
+	beforeEach(() => {
+		received = [];
+		server.clear();
+	});
+
+	it('skips a non-200 candidate and parses the 200 JSON metadata over a real socket', async () => {
+		const service = buildService();
+
+		// `discoverProtectedResourceMetadata` is the real discovery callsite; reach
+		// it directly so the test stays focused on the GET round-trip rather than the
+		// full auth-URI flow.
+		const metadata = await service['discoverProtectedResourceMetadata'](`${server.url}/mcp`);
+
+		// The 200 body crossed the wire as a JSON string and was parsed back into the
+		// metadata object (not a raw string), exactly as the old axios `get` did.
+		expect(metadata).toEqual(PROTECTED_RESOURCE_METADATA);
+
+		// Both candidates were requested in order: the path-specific 404 was skipped,
+		// then the root 200 succeeded.
+		expect(received).toHaveLength(2);
+		expect(received[0]).toMatchObject({
+			method: 'GET',
+			url: '/.well-known/oauth-protected-resource/mcp',
+		});
+		expect(received[1]).toMatchObject({
+			method: 'GET',
+			url: '/.well-known/oauth-protected-resource',
+		});
+		// `json: true` sets the Accept header on every discovery GET.
+		expect(received[1].headers.accept).toContain('application/json');
+	});
+});
@@ -1,5 +1,6 @@
 import { Logger } from '@n8n/backend-common';
-import { GlobalConfig } from '@n8n/config';
+import { OutboundHttp, SsrfProtectionService, type HttpRequestClient } from '@n8n/backend-network';
+import { GlobalConfig, SsrfProtectionConfig } from '@n8n/config';
 import type { AuthenticatedRequest, CredentialsEntity, ICredentialsDb } from '@n8n/db';
 import { CredentialsRepository } from '@n8n/db';
 import { Service } from '@n8n/di';
@@ -7,7 +8,7 @@ import Csrf from 'csrf';
 import type { Request, Response } from 'express';
 import { Credentials, Cipher } from 'n8n-core';
 import type { ICredentialDataDecryptedObject, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
-import { jsonParse, UnexpectedError } from 'n8n-workflow';
+import { jsonParse, OperationalError, UnexpectedError } from 'n8n-workflow';

 import {
 	GENERIC_OAUTH2_CREDENTIALS_WITH_EDITABLE_SCOPE,
@@ -31,7 +32,6 @@ import {
 	type OAuth2CredentialData,
 	type OAuth2GrantType,
 } from '@n8n/client-oauth2';
-import axios from 'axios';
 import {
 	oAuthAuthorizationServerMetadataSchema,
 	dynamicClientRegistrationResponseSchema,
@@ -40,7 +40,6 @@ import pkceChallenge from 'pkce-challenge';
 import * as qs from 'querystring';
 import split from 'lodash/split';
 import { ExternalHooks } from '@/external-hooks';
-import type { AxiosRequestConfig } from 'axios';
 import { createHmac } from 'crypto';
 import type { RequestOptions } from 'oauth-1.0a';
 import clientOAuth1 from 'oauth-1.0a';
@@ -58,6 +57,7 @@ import { EventService } from '@/events/event.service';
 import { OAuthJweServiceProxy } from '@/oauth/oauth-jwe-service.proxy';
 import { OAuthBrowserBindingService } from '@/oauth/oauth-browser-binding.service';
 import { CacheService } from '@/services/cache/cache.service';
+import { Time } from '@n8n/constants';

 /**
 * Per-flow OAuth state stored in CacheService, keyed by the CSRF state token.
@@ -72,6 +72,7 @@ export type OauthFlowState = {
 };

 const OAUTH_FLOW_CACHE_PREFIX = 'oauth:flow:';
+const OAUTH_REQUEST_TIMEOUT_MS = 30 * Time.seconds.toMilliseconds; // This might be added to a OAuth Config (there is currently none)

 export function shouldSkipAuthOnOAuthCallback() {
 	const value = process.env.N8N_SKIP_AUTH_ON_OAUTH_CALLBACK?.toLowerCase() ?? 'false';
@@ -113,7 +114,21 @@ export class OauthService {
 		private readonly browserBindingService: OAuthBrowserBindingService,
 		private readonly eventService: EventService,
 		private readonly cacheService: CacheService,
-	) {}
+		outboundHttp: OutboundHttp,
+		ssrfProtectionService: SsrfProtectionService,
+		ssrfProtectionConfig: SsrfProtectionConfig,
+	) {
+		// Unlike most OutboundHttp callsites, here we opt into SSRF protection (when the environment enables it) because the attack risk is higher:
+		// these URLs can be user-, instance- or remote-server-supplied (discovery / dynamic client registration),
+		// so the service can't tell at runtime which are trustworthy.
+		// Self-hosted users with an internal OAuth/MCP server are accommodated via the SSRF allowlist config, not by disabling the guard.
+		// In the future, enabling SSRF "per feature" could be refined through configuration.
+		this.http = outboundHttp.requests({
+			ssrf: ssrfProtectionConfig.enabled ? ssrfProtectionService : 'disabled',
+		});
+	}
+
+	private readonly http: HttpRequestClient;

 	private oauthFlowCacheKey(token: string): string {
 		return `${OAUTH_FLOW_CACHE_PREFIX}${token}`;
@@ -905,10 +920,7 @@ export class OauthService {
 				// Validate each URL before making request (defense-in-depth)
 				this.validateOAuthUrlOrThrow(url);

-				const response = await axios.get<unknown>(url, {
-					validateStatus: (status) => status === 200,
-				});
-				data = response.data;
+				data = await this.fetchDiscoveryDocument(url);
 				break; // Success - exit loop
 			} catch (error) {
 				lastError = error as Error;
@@ -979,10 +991,13 @@ export class OauthService {

 		await this.externalHooks.run('oauth2.dynamicClientRegistration', [registerPayload]);

-		const { data: registerResult } = await axios.post<unknown>(
-			registration_endpoint,
-			registerPayload,
-		);
+		const registerResult = await this.http.request({
+			url: registration_endpoint,
+			method: 'POST',
+			body: registerPayload,
+			json: true,
+			timeout: OAUTH_REQUEST_TIMEOUT_MS,
+		});
 		const registrationValidation =
 			dynamicClientRegistrationResponseSchema.safeParse(registerResult);
 		if (!registrationValidation.success) {
@@ -1048,15 +1063,13 @@ export class OauthService {

 		const data = oauth.toHeader(oauth.authorize(options));

-		const axiosConfig: AxiosRequestConfig = {
-			method: options.method,
+		const response = await this.http.request({
 			url: options.url,
-			headers: {
-				...data,
-			},
-		};
-
-		const { data: response } = await axios.request(axiosConfig);
+			method: 'POST',
+			headers: { ...data },
+			encoding: 'text',
+			timeout: OAUTH_REQUEST_TIMEOUT_MS,
+		});

 		// Response comes as x-www-form-urlencoded string so convert it to JSON
 		if (typeof response !== 'string') {
@@ -1131,14 +1144,16 @@ export class OauthService {
 		// `oauth_verifier` is part of the signature base string but is not emitted
 		// into the Authorization header by `toHeader`, so it must travel in the
 		// form-encoded body for the server to receive and verify it.
-		const { data: response } = await axios.request<string>({
-			method: 'POST',
+		const response = await this.http.request({
 			url: oauthCredentials.accessTokenUrl,
-			data: new URLSearchParams({ oauth_verifier: params.oauthVerifier }).toString(),
+			method: 'POST',
+			body: new URLSearchParams({ oauth_verifier: params.oauthVerifier }).toString(),
 			headers: {
 				...headers,
 				'content-type': 'application/x-www-form-urlencoded',
 			},
+			encoding: 'text',
+			timeout: OAUTH_REQUEST_TIMEOUT_MS,
 		});

 		// Response comes as x-www-form-urlencoded string so convert it to JSON
@@ -1179,6 +1194,26 @@ export class OauthService {
 		return options;
 	}

+	/**
+	 * Fetches a `.well-known` discovery document and returns its parsed JSON body.
+	 * Only a 200 is accepted (RFC 8414 / RFC 9728 / OpenID Connect discovery endpoints respond with 200).
+	 * Any other status, or a transport/SSRF failure, throws,
+	 * so the discovery loops can uniformly catch and fall through to the next candidate URL.
+	 */
+	private async fetchDiscoveryDocument(url: string): Promise<unknown> {
+		const response = await this.http.request({
+			url,
+			method: 'GET',
+			json: true,
+			returnFullResponse: true,
+			timeout: OAUTH_REQUEST_TIMEOUT_MS,
+		});
+		if (response.statusCode !== 200) {
+			throw new OperationalError(`Request failed with status code ${response.statusCode}`);
+		}
+		return response.body;
+	}
+
 	/**
 	 * Discovers OAuth 2.0 Protected Resource Metadata per RFC 9728.
 	 * This is the first step in MCP-compliant OAuth discovery.
@@ -1211,34 +1246,32 @@ export class OauthService {
 				// Validate each URL before making request (defense-in-depth)
 				this.validateOAuthUrlOrThrow(discoveryUrl);

-				const { data } = await axios.get(discoveryUrl, {
-					validateStatus: (status) => status === 200,
-				});
+				const data = await this.fetchDiscoveryDocument(discoveryUrl);

 				// Validate has authorization_servers field per RFC 9728
-				if (
-					data &&
-					Array.isArray(data.authorization_servers) &&
-					data.authorization_servers.length > 0
-				) {
-					const rawResource = (data as Record<string, unknown>).resource;
-					const resource =
-						typeof rawResource === 'string'
-							? this.validateResourceUrlOrThrow(rawResource)
+				if (data && typeof data === 'object') {
+					const record = data as Record<string, unknown>;
+					const authorizationServers = record.authorization_servers;
+					if (Array.isArray(authorizationServers) && authorizationServers.length > 0) {
+						const rawResource = record.resource;
+						const resource =
+							typeof rawResource === 'string'
+								? this.validateResourceUrlOrThrow(rawResource)
+								: undefined;
+						// Per RFC 9728 the protected resource advertises the scopes required to
+						// access it. Some authorization servers (e.g. Atlassian) omit
+						// scopes_supported from their RFC 8414 metadata, so these are the only
+						// scopes available for the request.
+						const rawScopes = record.scopes_supported;
+						const scopes_supported = Array.isArray(rawScopes)
+							? rawScopes.filter((s): s is string => typeof s === 'string')
 							: undefined;
-					// Per RFC 9728 the protected resource advertises the scopes required to
-					// access it. Some authorization servers (e.g. Atlassian) omit
-					// scopes_supported from their RFC 8414 metadata, so these are the only
-					// scopes available for the request.
-					const rawScopes = (data as Record<string, unknown>).scopes_supported;
-					const scopes_supported = Array.isArray(rawScopes)
-						? rawScopes.filter((s): s is string => typeof s === 'string')
-						: undefined;
-					return {
-						authorization_servers: data.authorization_servers,
-						...(resource ? { resource } : {}),
-						...(scopes_supported?.length ? { scopes_supported } : {}),
-					};
+						return {
+							authorization_servers: authorizationServers,
+							...(resource ? { resource } : {}),
+							...(scopes_supported?.length ? { scopes_supported } : {}),
+						};
+					}
 				}
 			} catch (error) {
 				// Continue to next URL