mirror of
https://github.com/n8n-io/n8n.git
synced 2026-07-29 12:05:04 +02:00
refactor(core): Migrate OIDC discovery fetch to outbound HTTP factory (no-changelog) (#32395)
This commit is contained in:
Lorent Lempereur
GitHub
parent
1d6415cd59
commit
ae17db7eb2
|
|
@ -1,5 +1,8 @@
|
|||
import type { OidcConfigDto } from '@n8n/api-types';
|
||||
import type { Logger } from '@n8n/backend-common';
|
||||
import type { HttpTransport, SsrfProtectionService } from '@n8n/backend-network';
|
||||
import { OutboundHttp } from '@n8n/backend-network';
|
||||
import { type LocalServer, startServer } from '@n8n/backend-network/testing';
|
||||
import { mockInstance, mockLogger } from '@n8n/backend-test-utils';
|
||||
import type { GlobalConfig } from '@n8n/config';
|
||||
import type { AuthIdentityRepository, SettingsRepository, User, UserRepository } from '@n8n/db';
|
||||
|
|
@ -7,7 +10,6 @@ import { Container } from '@n8n/di';
|
|||
import { mock } from 'jest-mock-extended';
|
||||
import type { Cipher, InstanceSettings } from 'n8n-core';
|
||||
import * as client from 'openid-client';
|
||||
import { EnvHttpProxyAgent } from 'undici';
|
||||
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
|
||||
|
|
@ -20,11 +22,6 @@ import * as ssoHelpers from '@/sso.ee/sso-helpers';
|
|||
import { OIDC_PREFERENCES_DB_KEY } from '../constants';
|
||||
import { OidcService } from '../oidc.service.ee';
|
||||
|
||||
jest.mock('undici', () => ({
|
||||
// eslint-disable-next-line @typescript-eslint/naming-convention
|
||||
EnvHttpProxyAgent: jest.fn().mockImplementation(() => ({})),
|
||||
}));
|
||||
|
||||
describe('OidcService', () => {
|
||||
let oidcService: OidcService;
|
||||
let settingsRepository: SettingsRepository;
|
||||
|
|
@ -36,6 +33,8 @@ describe('OidcService', () => {
|
|||
let provisioningService: ProvisioningService;
|
||||
let userRepository: UserRepository;
|
||||
let authIdentityRepository: AuthIdentityRepository;
|
||||
let outboundHttp: jest.Mocked<OutboundHttp>;
|
||||
let customFetch: jest.Mock;
|
||||
|
||||
const mockOidcConfig = {
|
||||
clientId: 'test-client-id',
|
||||
|
|
@ -70,6 +69,11 @@ describe('OidcService', () => {
|
|||
provisioningService = mock<ProvisioningService>();
|
||||
userRepository = mock<UserRepository>();
|
||||
authIdentityRepository = mock<AuthIdentityRepository>();
|
||||
customFetch = jest.fn();
|
||||
outboundHttp = mock<OutboundHttp>();
|
||||
outboundHttp.transport.mockReturnValue(
|
||||
mock<HttpTransport>({ asCustomFetch: () => customFetch }),
|
||||
);
|
||||
jest
|
||||
.spyOn(ssoHelpers, 'setCurrentAuthenticationMethod')
|
||||
.mockImplementation(async () => await Promise.resolve());
|
||||
|
|
@ -85,6 +89,7 @@ describe('OidcService', () => {
|
|||
jwtService,
|
||||
instanceSettings,
|
||||
provisioningService,
|
||||
outboundHttp,
|
||||
);
|
||||
|
||||
await oidcService.init();
|
||||
|
|
@ -676,342 +681,119 @@ describe('OidcService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('proxy configuration', () => {
|
||||
const originalEnv = process.env;
|
||||
describe('createProxyAwareConfiguration', () => {
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
// Helper function to create a proper mock Response
|
||||
const createMockResponse = () => {
|
||||
const mockData = {
|
||||
issuer: 'https://example.com',
|
||||
authorization_endpoint: 'https://example.com/auth',
|
||||
token_endpoint: 'https://example.com/token',
|
||||
userinfo_endpoint: 'https://example.com/userinfo',
|
||||
jwks_uri: 'https://example.com/jwks',
|
||||
};
|
||||
return new Response(JSON.stringify(mockData), {
|
||||
status: 200,
|
||||
// eslint-disable-next-line @typescript-eslint/naming-convention
|
||||
headers: { 'content-type': 'application/json' },
|
||||
const createConfiguration = async () =>
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return
|
||||
(await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
)) as client.Configuration;
|
||||
|
||||
it("obtains the custom fetch from the factory transport with SSRF 'disabled'", async () => {
|
||||
jest.spyOn(client, 'discovery').mockResolvedValue({} as client.Configuration);
|
||||
|
||||
await createConfiguration();
|
||||
|
||||
// The discovery / token / userinfo endpoints are admin-configured and may
|
||||
// legitimately point at an internal IdP, so SSRF protection is disabled.
|
||||
expect(outboundHttp.transport).toHaveBeenCalledWith({ ssrf: 'disabled' });
|
||||
});
|
||||
|
||||
it('always calls discovery with the factory customFetch (no proxy/no-proxy branch)', async () => {
|
||||
const discoverySpy = jest
|
||||
.spyOn(client, 'discovery')
|
||||
.mockResolvedValue({} as client.Configuration);
|
||||
|
||||
await createConfiguration();
|
||||
|
||||
expect(discoverySpy).toHaveBeenCalledWith(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
[client.customFetch]: customFetch,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('sets the factory customFetch on the returned configuration', async () => {
|
||||
jest.spyOn(client, 'discovery').mockResolvedValue({} as client.Configuration);
|
||||
|
||||
const result = await createConfiguration();
|
||||
|
||||
expect(result[client.customFetch]).toBe(customFetch);
|
||||
});
|
||||
});
|
||||
|
||||
// Exercises the customFetch produced by a real OutboundHttp against a real
|
||||
// loopback server. openid-client drives discovery / token / userinfo through
|
||||
// this fetch, so proving it performs a genuine HTTP round-trip (the same job
|
||||
// the old hand-rolled proxyFetch did) validates the migrated behavior.
|
||||
describe('factory customFetch (real HTTP round-trip)', () => {
|
||||
let idpServer: LocalServer;
|
||||
let realOidcService: OidcService;
|
||||
|
||||
beforeAll(async () => {
|
||||
idpServer = await startServer((req, res) => {
|
||||
res.writeHead(200, { 'content-type': 'application/json' });
|
||||
res.end(JSON.stringify({ ok: true, path: req.url }));
|
||||
});
|
||||
};
|
||||
});
|
||||
|
||||
afterAll(async () => await idpServer.close());
|
||||
|
||||
beforeEach(() => {
|
||||
// Reset environment before each test
|
||||
process.env = { ...originalEnv };
|
||||
// Reset the mock between tests
|
||||
(EnvHttpProxyAgent as unknown as jest.Mock).mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore original environment after each test
|
||||
process.env = originalEnv;
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ envVar: 'HTTP_PROXY', value: 'http://proxy.example.com:8080' },
|
||||
{ envVar: 'HTTPS_PROXY', value: 'https://proxy.example.com:8443' },
|
||||
{ envVar: 'ALL_PROXY', value: 'http://all-proxy.example.com:8888' },
|
||||
])('should instantiate EnvHttpProxyAgent when $envVar is set', async ({ envVar, value }) => {
|
||||
// Set proxy environment variable
|
||||
process.env[envVar] = value;
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
const discoverySpy = jest.spyOn(client, 'discovery').mockResolvedValue({
|
||||
serverMetadata: () => ({ issuer: 'https://example.com' }),
|
||||
} as unknown as client.Configuration);
|
||||
|
||||
// Call the private method directly using type assertion
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
idpServer.clear();
|
||||
const realOutboundHttp = new OutboundHttp(mock<SsrfProtectionService>(), logger);
|
||||
realOidcService = new OidcService(
|
||||
settingsRepository,
|
||||
authIdentityRepository,
|
||||
mock<UrlService>(),
|
||||
globalConfig,
|
||||
userRepository,
|
||||
cipher,
|
||||
logger,
|
||||
jwtService,
|
||||
instanceSettings,
|
||||
provisioningService,
|
||||
realOutboundHttp,
|
||||
);
|
||||
|
||||
// Verify EnvHttpProxyAgent was instantiated
|
||||
expect(EnvHttpProxyAgent).toHaveBeenCalled();
|
||||
discoverySpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should not instantiate EnvHttpProxyAgent when no proxy env vars are set', async () => {
|
||||
// Ensure no proxy env vars are set
|
||||
delete process.env.HTTP_PROXY;
|
||||
delete process.env.HTTPS_PROXY;
|
||||
delete process.env.ALL_PROXY;
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
const discoverySpy = jest.spyOn(client, 'discovery').mockResolvedValue({
|
||||
serverMetadata: () => ({ issuer: 'https://example.com' }),
|
||||
} as unknown as client.Configuration);
|
||||
|
||||
// Call the private method directly
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
);
|
||||
|
||||
// Should not instantiate EnvHttpProxyAgent when no proxy is configured
|
||||
expect(EnvHttpProxyAgent).not.toHaveBeenCalled();
|
||||
discoverySpy.mockRestore();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ envVar: 'HTTP_PROXY', value: 'http://proxy.example.com:8080' },
|
||||
{ envVar: 'HTTPS_PROXY', value: 'https://proxy.example.com:8443' },
|
||||
{ envVar: 'ALL_PROXY', value: 'http://all-proxy.example.com:8888' },
|
||||
])(
|
||||
'should call discovery with customFetch option when $envVar is configured',
|
||||
async ({ envVar, value }) => {
|
||||
process.env[envVar] = value;
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
global.fetch = jest.fn().mockResolvedValue(createMockResponse());
|
||||
|
||||
const discoverySpy = jest.spyOn(client, 'discovery').mockResolvedValue({
|
||||
serverMetadata: () => ({ issuer: 'https://example.com' }),
|
||||
} as unknown as client.Configuration);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
);
|
||||
|
||||
expect(discoverySpy).toHaveBeenCalledWith(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
[client.customFetch]: expect.any(Function),
|
||||
}),
|
||||
);
|
||||
|
||||
discoverySpy.mockRestore();
|
||||
},
|
||||
);
|
||||
|
||||
it('should call discovery without customFetch option when no proxy is configured', async () => {
|
||||
delete process.env.HTTP_PROXY;
|
||||
delete process.env.HTTPS_PROXY;
|
||||
delete process.env.ALL_PROXY;
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
global.fetch = jest.fn().mockResolvedValue(createMockResponse());
|
||||
|
||||
const discoverySpy = jest.spyOn(client, 'discovery').mockResolvedValue({
|
||||
serverMetadata: () => ({ issuer: 'https://example.com' }),
|
||||
} as unknown as client.Configuration);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
);
|
||||
|
||||
// Should be called with only 3 arguments (no options object)
|
||||
expect(discoverySpy).toHaveBeenCalledWith(discoveryUrl, clientId, clientSecret);
|
||||
|
||||
discoverySpy.mockRestore();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ envVar: 'HTTP_PROXY', value: 'http://proxy.example.com:8080' },
|
||||
{ envVar: 'HTTPS_PROXY', value: 'https://proxy.example.com:8443' },
|
||||
{ envVar: 'ALL_PROXY', value: 'http://all-proxy.example.com:8888' },
|
||||
])(
|
||||
'should set customFetch on returned configuration when $envVar is configured',
|
||||
async ({ envVar, value }) => {
|
||||
process.env[envVar] = value;
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
global.fetch = jest.fn().mockResolvedValue(createMockResponse());
|
||||
|
||||
const mockConfiguration = {
|
||||
serverMetadata: () => ({ issuer: 'https://example.com' }),
|
||||
} as unknown as client.Configuration;
|
||||
|
||||
jest.spyOn(client, 'discovery').mockResolvedValue(mockConfiguration);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
const result = await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
);
|
||||
|
||||
// Verify customFetch was set on the configuration
|
||||
expect(result[client.customFetch]).toBeDefined();
|
||||
expect(typeof result[client.customFetch]).toBe('function');
|
||||
},
|
||||
);
|
||||
|
||||
it('should not set customFetch on returned configuration when no proxy is configured', async () => {
|
||||
delete process.env.HTTP_PROXY;
|
||||
delete process.env.HTTPS_PROXY;
|
||||
delete process.env.ALL_PROXY;
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
global.fetch = jest.fn().mockResolvedValue(createMockResponse());
|
||||
|
||||
const mockConfiguration = {
|
||||
serverMetadata: () => ({ issuer: 'https://example.com' }),
|
||||
} as unknown as client.Configuration;
|
||||
|
||||
jest.spyOn(client, 'discovery').mockResolvedValue(mockConfiguration);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
const result = await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
);
|
||||
|
||||
// customFetch should not have been set on the configuration
|
||||
expect(result[client.customFetch]).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ envVar: 'HTTP_PROXY', value: 'http://proxy.example.com:8080' },
|
||||
{ envVar: 'HTTPS_PROXY', value: 'https://proxy.example.com:8443' },
|
||||
{ envVar: 'ALL_PROXY', value: 'http://all-proxy.example.com:8888' },
|
||||
])(
|
||||
'should use proxy agent dispatcher in customFetch when $envVar is configured',
|
||||
async ({ envVar, value }) => {
|
||||
process.env[envVar] = value;
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
const mockProxyAgent = { type: 'proxy-agent' };
|
||||
(EnvHttpProxyAgent as unknown as jest.Mock).mockImplementation(() => mockProxyAgent);
|
||||
|
||||
const fetchSpy = jest.fn().mockResolvedValue(createMockResponse());
|
||||
global.fetch = fetchSpy;
|
||||
|
||||
const mockConfiguration = {
|
||||
serverMetadata: () => ({ issuer: 'https://example.com' }),
|
||||
} as unknown as client.Configuration;
|
||||
|
||||
jest.spyOn(client, 'discovery').mockResolvedValue(mockConfiguration);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
const result = await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
);
|
||||
|
||||
// Get the customFetch function and call it
|
||||
const customFetch = result[client.customFetch];
|
||||
const testUrl = 'https://example.com/test';
|
||||
const testOptions = { method: 'GET' };
|
||||
|
||||
await customFetch(testUrl, testOptions);
|
||||
|
||||
// Verify fetch was called with dispatcher option
|
||||
expect(fetchSpy).toHaveBeenCalledWith(testUrl, {
|
||||
...testOptions,
|
||||
dispatcher: mockProxyAgent,
|
||||
it('routes openid-client fetches through a real HTTP socket', async () => {
|
||||
let factoryFetch: client.CustomFetch | undefined;
|
||||
jest
|
||||
.spyOn(client, 'discovery')
|
||||
.mockImplementation(async (_server, _clientId, _metadata, _auth, options) => {
|
||||
factoryFetch = options?.[client.customFetch];
|
||||
return {} as client.Configuration;
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it('should handle multiple proxy env vars with priority (first match)', async () => {
|
||||
// Set multiple proxy variables - any of them should trigger proxy mode
|
||||
process.env.HTTP_PROXY = 'http://http-proxy.example.com:8080';
|
||||
process.env.HTTPS_PROXY = 'https://https-proxy.example.com:8443';
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
const discoverySpy = jest.spyOn(client, 'discovery').mockResolvedValue({
|
||||
serverMetadata: () => ({ issuer: 'https://example.com' }),
|
||||
} as unknown as client.Configuration);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
await (realOidcService as any).createProxyAwareConfiguration(
|
||||
new URL('https://issuer.example.com/.well-known/openid-configuration'),
|
||||
'real-client',
|
||||
'real-secret',
|
||||
);
|
||||
|
||||
// EnvHttpProxyAgent should be instantiated once regardless of how many proxy vars are set
|
||||
expect(EnvHttpProxyAgent).toHaveBeenCalledTimes(1);
|
||||
discoverySpy.mockRestore();
|
||||
expect(factoryFetch).toBeDefined();
|
||||
|
||||
const response = await factoryFetch!(`${idpServer.url}/userinfo`, {
|
||||
method: 'GET',
|
||||
headers: {},
|
||||
body: null,
|
||||
redirect: 'manual',
|
||||
});
|
||||
const body = (await response.json()) as { ok: boolean; path: string };
|
||||
|
||||
expect(idpServer.captured).toEqual(['/userinfo']);
|
||||
expect(body).toEqual({ ok: true, path: '/userinfo' });
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ envVar: 'HTTP_PROXY', value: 'http://proxy.example.com:8080' },
|
||||
{ envVar: 'HTTPS_PROXY', value: 'https://proxy.example.com:8443' },
|
||||
{ envVar: 'ALL_PROXY', value: 'http://all-proxy.example.com:8888' },
|
||||
])(
|
||||
'should pass through fetch options correctly when $envVar is configured',
|
||||
async ({ envVar, value }) => {
|
||||
process.env[envVar] = value;
|
||||
|
||||
const discoveryUrl = new URL('https://example.com/.well-known/openid-configuration');
|
||||
const clientId = 'test-client';
|
||||
const clientSecret = 'test-secret';
|
||||
|
||||
const mockProxyAgent = { type: 'proxy-agent' };
|
||||
(EnvHttpProxyAgent as unknown as jest.Mock).mockImplementation(() => mockProxyAgent);
|
||||
|
||||
const fetchSpy = jest.fn().mockResolvedValue(createMockResponse());
|
||||
global.fetch = fetchSpy;
|
||||
|
||||
const mockConfiguration = {} as client.Configuration;
|
||||
jest.spyOn(client, 'discovery').mockResolvedValue(mockConfiguration);
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
|
||||
const result = await (oidcService as any).createProxyAwareConfiguration(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
);
|
||||
|
||||
const customFetch = result[client.customFetch];
|
||||
const testUrl = 'https://example.com/token';
|
||||
const testOptions = {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
|
||||
body: 'grant_type=authorization_code&code=test',
|
||||
};
|
||||
|
||||
await customFetch(testUrl, testOptions);
|
||||
|
||||
// Verify all original options are preserved and dispatcher is added
|
||||
expect(fetchSpy).toHaveBeenCalledWith(testUrl, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
|
||||
body: 'grant_type=authorization_code&code=test',
|
||||
dispatcher: mockProxyAgent,
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { OidcConfigDto } from '@n8n/api-types';
|
||||
import { Logger } from '@n8n/backend-common';
|
||||
import { OutboundHttp } from '@n8n/backend-network';
|
||||
import { GlobalConfig } from '@n8n/config';
|
||||
import {
|
||||
AuthIdentity,
|
||||
|
|
@ -16,7 +17,6 @@ import { randomUUID } from 'crypto';
|
|||
import { Cipher, InstanceSettings } from 'n8n-core';
|
||||
import { jsonParse, UserError } from 'n8n-workflow';
|
||||
import type * as openidClientTypes from 'openid-client';
|
||||
import { EnvHttpProxyAgent } from 'undici';
|
||||
import { inspect } from 'util';
|
||||
|
||||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
|
|
@ -99,6 +99,7 @@ export class OidcService {
|
|||
private readonly jwtService: JwtService,
|
||||
private readonly instanceSettings: InstanceSettings,
|
||||
private readonly provisioningService: ProvisioningService,
|
||||
private readonly outboundHttp: OutboundHttp,
|
||||
) {}
|
||||
|
||||
async init() {
|
||||
|
|
@ -695,8 +696,7 @@ export class OidcService {
|
|||
| undefined;
|
||||
|
||||
/**
|
||||
* Creates a proxy-aware configuration for openid-client.
|
||||
* This method configures customFetch to respect HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables.
|
||||
* Creates a configuration for openid-client whose HTTP calls route through the outbound HTTP factory.
|
||||
*/
|
||||
private async createProxyAwareConfiguration(
|
||||
discoveryUrl: URL,
|
||||
|
|
@ -705,46 +705,31 @@ export class OidcService {
|
|||
): Promise<openidClientTypes.Configuration> {
|
||||
await this.loadOpenIdClient();
|
||||
|
||||
// Check if proxy environment variables are set
|
||||
const hasProxyConfig =
|
||||
process.env.HTTP_PROXY ?? process.env.HTTPS_PROXY ?? process.env.ALL_PROXY;
|
||||
const customFetch = this.outboundHttp
|
||||
.transport({
|
||||
// SSRF is explicitly disabled: the discovery endpoint (and the issuer's
|
||||
// token/userinfo endpoints reached with the same `customFetch`) is
|
||||
// admin-configured and may legitimately point at an internal IdP, so enabling
|
||||
// SSRF protection here would block valid internal setups
|
||||
ssrf: 'disabled',
|
||||
// `proxy` defaults = `'env'`
|
||||
})
|
||||
.asCustomFetch() as unknown as openidClientTypes.CustomFetch;
|
||||
|
||||
if (hasProxyConfig) {
|
||||
this.logger.debug('Configuring OIDC client with proxy support', {
|
||||
HTTP_PROXY: process.env.HTTP_PROXY,
|
||||
HTTPS_PROXY: process.env.HTTPS_PROXY,
|
||||
NO_PROXY: process.env.NO_PROXY,
|
||||
ALL_PROXY: process.env.ALL_PROXY,
|
||||
});
|
||||
const configuration = await this.openidClient.discovery(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
undefined,
|
||||
{
|
||||
[this.openidClient.customFetch]: customFetch,
|
||||
},
|
||||
);
|
||||
|
||||
// Create a proxy agent that automatically reads from environment variables
|
||||
const proxyAgent = new EnvHttpProxyAgent();
|
||||
const proxyFetch: openidClientTypes.CustomFetch = async (url, options) => {
|
||||
return await fetch(url, {
|
||||
...options,
|
||||
// @ts-expect-error - dispatcher is an undici-specific option not in standard fetch
|
||||
dispatcher: proxyAgent,
|
||||
});
|
||||
};
|
||||
// Reuse the same fetch for token-exchange / userinfo on the returned configuration.
|
||||
configuration[this.openidClient.customFetch] = customFetch;
|
||||
|
||||
// discovery call with custom fetch client using proxy agent
|
||||
const configuration = await this.openidClient.discovery(
|
||||
discoveryUrl,
|
||||
clientId,
|
||||
clientSecret,
|
||||
undefined,
|
||||
{
|
||||
[this.openidClient.customFetch]: proxyFetch,
|
||||
},
|
||||
);
|
||||
|
||||
// Configure customFetch to use the proxy agent
|
||||
configuration[this.openidClient.customFetch] = proxyFetch;
|
||||
|
||||
return configuration;
|
||||
}
|
||||
|
||||
return await this.openidClient.discovery(discoveryUrl, clientId, clientSecret);
|
||||
return configuration;
|
||||
}
|
||||
|
||||
private async getOidcConfiguration(): Promise<openidClientTypes.Configuration> {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,90 @@
|
|||
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 { mock } from 'jest-mock-extended';
|
||||
import * as client from 'openid-client';
|
||||
|
||||
describe('OIDC discovery (real HTTP round-trip through the factory)', () => {
|
||||
let idpServer: LocalServer;
|
||||
|
||||
const buildCustomFetch = () => {
|
||||
const outboundHttp = new OutboundHttp(mock<SsrfProtectionService>(), mock<Logger>());
|
||||
return outboundHttp
|
||||
.transport({ ssrf: 'disabled' })
|
||||
.asCustomFetch() as unknown as client.CustomFetch;
|
||||
};
|
||||
|
||||
const discoveryDocument = (origin: string) => ({
|
||||
issuer: origin,
|
||||
authorization_endpoint: `${origin}/auth`,
|
||||
token_endpoint: `${origin}/token`,
|
||||
userinfo_endpoint: `${origin}/userinfo`,
|
||||
jwks_uri: `${origin}/jwks`,
|
||||
response_types_supported: ['code'],
|
||||
subject_types_supported: ['public'],
|
||||
id_token_signing_alg_values_supported: ['RS256'],
|
||||
});
|
||||
|
||||
beforeAll(async () => {
|
||||
idpServer = await startServer((_req, res) => {
|
||||
// eslint-disable-next-line @typescript-eslint/naming-convention
|
||||
res.writeHead(200, { 'content-type': 'application/json' });
|
||||
res.end(JSON.stringify(discoveryDocument(idpServer.url)));
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(async () => await idpServer.close());
|
||||
|
||||
beforeEach(() => idpServer.clear());
|
||||
|
||||
it('fetches and parses the discovery document over a real socket', async () => {
|
||||
const customFetch = buildCustomFetch();
|
||||
|
||||
const configuration = await client.discovery(
|
||||
new URL(`${idpServer.url}/.well-known/openid-configuration`),
|
||||
'real-client',
|
||||
'real-secret',
|
||||
undefined,
|
||||
{
|
||||
execute: [client.allowInsecureRequests],
|
||||
[client.customFetch]: customFetch,
|
||||
},
|
||||
);
|
||||
|
||||
// The request actually went out over the wire ...
|
||||
expect(idpServer.captured).toEqual(['/.well-known/openid-configuration']);
|
||||
// ... and openid-client parsed the served metadata into a Configuration.
|
||||
expect(configuration.serverMetadata().issuer).toBe(idpServer.url);
|
||||
expect(configuration.serverMetadata().token_endpoint).toBe(`${idpServer.url}/token`);
|
||||
});
|
||||
|
||||
it('reuses the same customFetch on the configuration for subsequent real requests', async () => {
|
||||
const customFetch = buildCustomFetch();
|
||||
|
||||
const configuration = await client.discovery(
|
||||
new URL(`${idpServer.url}/.well-known/openid-configuration`),
|
||||
'real-client',
|
||||
'real-secret',
|
||||
undefined,
|
||||
{
|
||||
execute: [client.allowInsecureRequests],
|
||||
[client.customFetch]: customFetch,
|
||||
},
|
||||
);
|
||||
// The service sets the same fetch on the returned configuration so that
|
||||
// token-exchange / userinfo route through the factory too.
|
||||
configuration[client.customFetch] = customFetch;
|
||||
idpServer.clear();
|
||||
|
||||
// Drive that carried fetch directly against a real endpoint.
|
||||
const response = await configuration[client.customFetch](`${idpServer.url}/token`, {
|
||||
method: 'GET',
|
||||
headers: {},
|
||||
body: null,
|
||||
redirect: 'manual',
|
||||
});
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(idpServer.captured).toEqual(['/token']);
|
||||
});
|
||||
});
|
||||
|
|
@ -50,6 +50,10 @@ describe('OIDC service', () => {
|
|||
});
|
||||
|
||||
describe('loadConfig', () => {
|
||||
beforeEach(() => {
|
||||
discoveryMock.mockResolvedValue({});
|
||||
});
|
||||
|
||||
it('should initialize with default config', () => {
|
||||
expect(oidcService.getRedactedConfig()).toEqual({
|
||||
clientId: '',
|
||||
|
|
@ -178,6 +182,10 @@ describe('OIDC service', () => {
|
|||
expect.any(URL),
|
||||
'test-client-id',
|
||||
'test-client-secret',
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
[real_odic_client.customFetch]: expect.any(Function),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user