diff --git a/packages/@n8n/client-oauth2/src/client-oauth2.ts b/packages/@n8n/client-oauth2/src/client-oauth2.ts index 7deb2673a58..ee1f4374d15 100644 --- a/packages/@n8n/client-oauth2/src/client-oauth2.ts +++ b/packages/@n8n/client-oauth2/src/client-oauth2.ts @@ -114,9 +114,13 @@ export class ClientOAuth2 { // Axios rejects the promise by default for all status codes 4xx. // We override this to reject promises only on 5xxs validateStatus: (status) => status < 500, - // Disable axios's built-in proxy handling so requests are routed - // through n8n's global proxy agents (HttpProxyManager / HttpsProxyManager) - // instead of being double-proxied in corporate proxy-chain environments. + // In the shipped artifact this package resolves its own axios copy, which + // n8n's shared axios defaults (including the 300s timeout) do not reach — + // so bound the request explicitly. Matches the shared default. + timeout: 300_000, + // Disable axios's built-in proxy handling; the agents built below own + // env-proxy routing, avoiding double-proxying in corporate proxy-chain + // environments. proxy: false, }; @@ -134,25 +138,31 @@ export class ClientOAuth2 { }; } + const proxyUrl = resolveProxyUrl(url); + // Resolution is re-checked on the agent, so a hostname that resolves to a // different address between validation and connect is still caught. Only for // direct connections though: through a proxy the agent resolves the proxy host, // not the final target, so applying the lookup there would check the wrong host. - const lookup = resolveProxyUrl(url) ? undefined : ssrfBridge?.createSecureLookup(); + const lookup = proxyUrl ? undefined : ssrfBridge?.createSecureLookup(); - // Agents are built here rather than left to the global ones so the per-request - // `lookup` and relaxed TLS apply. Both factories are proxy-aware, so ignoring SSL - // issues still routes through the env proxy (HTTP(S)_PROXY / NO_PROXY) instead of - // connecting directly and bypassing it. - if (options.ignoreSSLIssues || lookup) { + // Agents are built per request whenever a proxy applies (not only for the + // `lookup` and relaxed-TLS cases). Whether this package's axios shares the + // instance that n8n's agent-injecting interceptor patches depends on package + // layout: the shipped artifact materialises its own copy, so without these + // agents a process lacking the global env-proxy agents connects directly and + // bypasses the proxy. + if (options.ignoreSSLIssues || lookup || proxyUrl) { requestConfig.httpsAgent = createHttpsProxyAgent(url, undefined, { ...(options.ignoreSSLIssues ? { rejectUnauthorized: false } : {}), ...(lookup ? { lookup } : {}), }); } - if (lookup) { - requestConfig.httpAgent = createHttpProxyAgent(url, undefined, { lookup }); + if (lookup || proxyUrl) { + requestConfig.httpAgent = createHttpProxyAgent(url, undefined, { + ...(lookup ? { lookup } : {}), + }); } const response = await axios.request(requestConfig); diff --git a/packages/@n8n/client-oauth2/test/client-oauth2.test.ts b/packages/@n8n/client-oauth2/test/client-oauth2.test.ts index bc0ae8f74c3..40720538922 100644 --- a/packages/@n8n/client-oauth2/test/client-oauth2.test.ts +++ b/packages/@n8n/client-oauth2/test/client-oauth2.test.ts @@ -344,10 +344,60 @@ describe('ClientOAuth2', () => { expect(httpsAgent).not.toBeInstanceOf(HttpsProxyAgent); expect(httpsAgent.options.rejectUnauthorized).toBe(false); }); + }); - it('should not set an httpsAgent when ignoreSSLIssues is false, leaving the global proxy agent in place', async () => { + describe('env proxy for standard requests', () => { + const PROXY_ENV_VARS = ['HTTPS_PROXY', 'https_proxy', 'NO_PROXY', 'no_proxy'] as const; + let savedProxyEnv: Record; + + beforeEach(() => { + savedProxyEnv = {}; + for (const key of PROXY_ENV_VARS) { + savedProxyEnv[key] = process.env[key]; + delete process.env[key]; + } + }); + + afterEach(() => { + for (const key of PROXY_ENV_VARS) { + if (savedProxyEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedProxyEnv[key]; + } + } + vi.restoreAllMocks(); + }); + + it('should route through an https proxy agent when HTTPS_PROXY is set', async () => { process.env.HTTPS_PROXY = 'http://fake-proxy.example'; + const axiosSpy = vi.spyOn(axios, 'request').mockResolvedValue({ + status: 200, + headers: { contentType: 'application/json' }, + data: JSON.stringify({ + access_token: config.accessToken, + refresh_token: config.refreshToken, + }), + }); + + await makeTokenCall(); + + const requestConfig = axiosSpy.mock.calls[0][0]; + const httpsAgent = requestConfig.httpsAgent as HttpsProxyAgent; + expect(httpsAgent).toBeInstanceOf(HttpsProxyAgent); + // TLS verification of the target stays on for standard requests. + expect(httpsAgent.connectOpts.rejectUnauthorized).toBeUndefined(); + const httpAgent = requestConfig.httpAgent as { proxy?: URL }; + expect(httpAgent.proxy?.href).toBe('http://fake-proxy.example/'); + expect(requestConfig.proxy).toBe(false); + expect(requestConfig.timeout).toBe(300_000); + }); + + it('should honor NO_PROXY and leave agents unset for excluded targets', async () => { + process.env.HTTPS_PROXY = 'http://fake-proxy.example'; + process.env.NO_PROXY = new URL(config.baseUrl).hostname; + mockTokenResponse({ status: 200, headers: { contentType: 'application/json' }, @@ -359,20 +409,30 @@ describe('ClientOAuth2', () => { const axiosSpy = vi.spyOn(axios, 'request'); - await client.accessTokenRequest({ - url: config.accessTokenUri, - method: 'POST', - headers: { - authorization: authHeader, - accept: 'application/json', - contentType: 'application/x-www-form-urlencoded', - }, - body: { refresh_token: 'test', grant_type: 'refresh_token' }, - ignoreSSLIssues: false, - }); + await makeTokenCall(); const requestConfig = axiosSpy.mock.calls[0][0]; expect(requestConfig.httpsAgent).toBeUndefined(); + expect(requestConfig.httpAgent).toBeUndefined(); + }); + + it('should leave agents unset when no proxy is configured', async () => { + mockTokenResponse({ + status: 200, + headers: { contentType: 'application/json' }, + body: JSON.stringify({ + access_token: config.accessToken, + refresh_token: config.refreshToken, + }), + }); + + const axiosSpy = vi.spyOn(axios, 'request'); + + await makeTokenCall(); + + const requestConfig = axiosSpy.mock.calls[0][0]; + expect(requestConfig.httpsAgent).toBeUndefined(); + expect(requestConfig.httpAgent).toBeUndefined(); expect(requestConfig.proxy).toBe(false); }); }); @@ -513,9 +573,9 @@ describe('ClientOAuth2', () => { // The lookup would resolve the proxy, not the target, so the target policy // must not be applied to it — the proxy reaches the target on our behalf. expect(ssrfBridge.createSecureLookup).not.toHaveBeenCalled(); - const requestConfig = axiosSpy.mock.calls[0][0]; - expect(requestConfig.httpAgent).toBeUndefined(); - expect(requestConfig.httpsAgent).toBeUndefined(); + const httpsAgent = axiosSpy.mock.calls[0][0].httpsAgent as HttpsProxyAgent; + expect(httpsAgent).toBeInstanceOf(HttpsProxyAgent); + expect(httpsAgent.connectOpts.lookup).toBeUndefined(); // The pre-flight check on the target still runs. expect(ssrfBridge.validateUrl).toHaveBeenCalledWith(new URL(config.accessTokenUri)); }); diff --git a/packages/@n8n/client-oauth2/test/proxy-routing.integration.test.ts b/packages/@n8n/client-oauth2/test/proxy-routing.integration.test.ts new file mode 100644 index 00000000000..f1f493b1a05 --- /dev/null +++ b/packages/@n8n/client-oauth2/test/proxy-routing.integration.test.ts @@ -0,0 +1,113 @@ +import { createServer, type Server } from 'node:http'; +import type { AddressInfo } from 'node:net'; + +import { ClientOAuth2 } from '@/client-oauth2'; + +const PROXY_ENV_VARS = [ + 'HTTP_PROXY', + 'http_proxy', + 'HTTPS_PROXY', + 'https_proxy', + 'NO_PROXY', + 'no_proxy', + 'ALL_PROXY', + 'all_proxy', +] as const; + +const listen = async (server: Server) => + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', () => resolve((server.address() as AddressInfo).port)); + }); + +const close = async (server: Server) => + await new Promise((resolve) => server.close(() => resolve())); + +const refreshVia = async (accessTokenUri: string) => + await new ClientOAuth2({ + clientId: 'client-id', + clientSecret: 'client-secret', + accessTokenUri, + authentication: 'header', + }) + .createToken({ access_token: 'expired', refresh_token: 'refresh-1' }) + .refresh(); + +describe('token refresh proxy routing', () => { + let savedProxyEnv: Record; + const servers: Server[] = []; + + beforeEach(() => { + savedProxyEnv = {}; + for (const key of PROXY_ENV_VARS) { + savedProxyEnv[key] = process.env[key]; + delete process.env[key]; + } + }); + + afterEach(async () => { + for (const key of PROXY_ENV_VARS) { + if (savedProxyEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedProxyEnv[key]; + } + } + await Promise.all(servers.splice(0).map(close)); + }); + + it('should route an http token refresh through HTTP_PROXY', async () => { + const seen: Array<{ url?: string; host?: string }> = []; + const proxy = createServer((req, res) => { + seen.push({ url: req.url, host: req.headers.host }); + res.setHeader('Content-Type', 'application/json'); + res.end(JSON.stringify({ access_token: 'proxied-access', refresh_token: 'refresh-2' })); + }); + servers.push(proxy); + process.env.HTTP_PROXY = `http://127.0.0.1:${await listen(proxy)}`; + + const refreshed = await refreshVia('http://token.host.invalid/oauth/token'); + + expect(refreshed.accessToken).toBe('proxied-access'); + expect(seen).toEqual([ + { url: 'http://token.host.invalid/oauth/token', host: 'token.host.invalid' }, + ]); + }); + + it('should send an https token refresh to HTTPS_PROXY as a CONNECT to the token host', async () => { + const connects: string[] = []; + const proxy = createServer(); + proxy.on('connect', (req, socket) => { + connects.push(req.url ?? ''); + socket.end('HTTP/1.1 502 Bad Gateway\r\n\r\n'); + }); + servers.push(proxy); + process.env.HTTPS_PROXY = `http://127.0.0.1:${await listen(proxy)}`; + + await expect(refreshVia('https://token.host.invalid/oauth/token')).rejects.toThrow(); + + expect(connects).toEqual(['token.host.invalid:443']); + }); + + it('should connect directly when NO_PROXY excludes the token host', async () => { + const proxiedRequests: string[] = []; + const proxy = createServer((req, res) => { + proxiedRequests.push(req.url ?? ''); + res.end(); + }); + servers.push(proxy); + + const tokenServer = createServer((_req, res) => { + res.setHeader('Content-Type', 'application/json'); + res.end(JSON.stringify({ access_token: 'direct-access' })); + }); + servers.push(tokenServer); + + process.env.HTTP_PROXY = `http://127.0.0.1:${await listen(proxy)}`; + process.env.NO_PROXY = '127.0.0.1'; + + const refreshed = await refreshVia(`http://127.0.0.1:${await listen(tokenServer)}/oauth/token`); + + expect(refreshed.accessToken).toBe('direct-access'); + expect(proxiedRequests).toEqual([]); + }); +});