chore(core): Synchronize OIDC settings updates in multi main (#19360)

This commit is contained in:
Andreas Fitzek
2025-09-15 12:42:11 +02:00
committed by GitHub
parent eab99bf87e
commit 82184c770e
13 changed files with 549 additions and 30 deletions

View File

@@ -0,0 +1,40 @@
import { isAuthProviderType } from '../types-db';
describe('types-db', () => {
describe('isAuthProviderType', () => {
it.each(['ldap', 'email', 'saml', 'oidc'])(
'should return true for valid "%s" auth provider types',
(provider) => {
expect(isAuthProviderType(provider)).toBe(true);
},
);
it.each([
'google',
'facebook',
'github',
'oauth2',
'jwt',
'basic',
'',
'LDAP', // case sensitive
'OIDC',
'Email',
])('should return false for invalid "%s" auth provider types', (provider) => {
expect(isAuthProviderType(provider)).toBe(false);
});
it.each([null, undefined, 123, true, false, {}, [], { type: 'oidc' }])(
'should return false for non-string value "%s"',
(value) => {
expect(isAuthProviderType(value as string)).toBe(false);
},
);
it('should handle edge cases', () => {
expect(isAuthProviderType(' oidc ')).toBe(false); // whitespace
expect(isAuthProviderType('oidc\n')).toBe(false); // newline
expect(isAuthProviderType('oidc\t')).toBe(false); // tab
});
});
});

View File

@@ -13,6 +13,7 @@ import type {
ExecutionSummary,
IUser,
} from 'n8n-workflow';
import { z } from 'zod';
import type { CredentialsEntity } from './credentials-entity';
import type { Folder } from './folder';
@@ -272,7 +273,13 @@ export const enum StatisticsNames {
dataLoaded = 'data_loaded',
}
export type AuthProviderType = 'ldap' | 'email' | 'saml' | 'oidc'; // | 'google';
const ALL_AUTH_PROVIDERS = z.enum(['ldap', 'email', 'saml', 'oidc']);
export type AuthProviderType = z.infer<typeof ALL_AUTH_PROVIDERS>;
export function isAuthProviderType(value: string): value is AuthProviderType {
return ALL_AUTH_PROVIDERS.safeParse(value).success;
}
export type FolderWithWorkflowAndSubFolderCount = Folder & {
workflowCount?: boolean;

View File

@@ -16,6 +16,7 @@ export type PubSubEventName =
| 'get-worker-status'
| 'reload-external-secrets-providers'
| 'reload-license'
| 'reload-oidc-config'
| 'response-to-get-worker-status'
| 'restart-event-bus'
| 'relay-execution-lifecycle-event';

View File

@@ -211,4 +211,54 @@ describe('PubSubRegistry', () => {
pubsubEventBus.emit('add-webhooks-triggers-and-pollers', { workflowId });
expect(onLeaderInstanceSpy).not.toHaveBeenCalled();
});
it('should clean up event handlers when reinitializing', () => {
const TestService = createTestServiceClass();
const testService = Container.get(TestService);
const onMainInstanceSpy = jest.spyOn(testService, 'onMainInstance');
const pubSubRegistry = new PubSubRegistry(
logger,
leaderInstanceSettings,
metadata,
pubsubEventBus,
);
// First initialization
pubSubRegistry.init();
// Emit event to verify handler is registered
pubsubEventBus.emit('reload-external-secrets-providers');
expect(onMainInstanceSpy).toHaveBeenCalledTimes(1);
// Reinitialize - should clean up previous handlers
onMainInstanceSpy.mockClear();
pubSubRegistry.init();
// Emit event again - should only be called once (not twice due to duplicate handlers)
pubsubEventBus.emit('reload-external-secrets-providers');
expect(onMainInstanceSpy).toHaveBeenCalledTimes(1);
});
it('should handle multiple reinitializations without memory leaks', () => {
const TestService = createTestServiceClass();
const testService = Container.get(TestService);
const onAllInstancesSpy = jest.spyOn(testService, 'onAllInstances');
const pubSubRegistry = new PubSubRegistry(
logger,
leaderInstanceSettings,
metadata,
pubsubEventBus,
);
// Multiple initializations
for (let i = 0; i < 5; i++) {
pubSubRegistry.init();
}
// Event should only trigger once per emission, not 5 times
pubsubEventBus.emit('clear-test-webhooks');
expect(onAllInstancesSpy).toHaveBeenCalledTimes(1);
});
});

View File

@@ -12,6 +12,12 @@ export type PubSubCommandMap = {
// #endregion
// # region SSO
'reload-oidc-config': never;
// #endregion
// #region Community packages
'community-package-install': {

View File

@@ -1,5 +1,5 @@
import { Logger } from '@n8n/backend-common';
import { PubSubMetadata } from '@n8n/decorators';
import { PubSubEventName, PubSubMetadata } from '@n8n/decorators';
import { Container, Service } from '@n8n/di';
import { InstanceSettings } from 'n8n-core';
@@ -16,8 +16,20 @@ export class PubSubRegistry {
this.logger = this.logger.scoped('pubsub');
}
private eventHandlers: Array<{
eventName: PubSubEventName;
handler: Parameters<PubSubEventBus['on']>[1];
}> = [];
init() {
const { instanceSettings, pubSubMetadata } = this;
// We clear the event handlers before registering new ones
for (const { eventName, handler } of this.eventHandlers) {
this.pubsubEventBus.off(eventName, handler);
}
this.eventHandlers = [];
// Register all event handlers that match the current instance type and role
const handlers = pubSubMetadata.getHandlers();
for (const { eventHandlerClass, methodName, eventName, filter } of handlers) {
const handlerClass = Container.get(eventHandlerClass);
@@ -25,14 +37,16 @@ export class PubSubRegistry {
this.logger.debug(
`Registered a "${eventName}" event handler on ${eventHandlerClass.name}#${methodName}`,
);
this.pubsubEventBus.on(eventName, async (...args: unknown[]) => {
const eventHandler = async (...args: unknown[]) => {
// Since the instance role can change, this check needs to be in the event listener
const shouldTrigger =
filter?.instanceType !== 'main' ||
!filter.instanceRole ||
filter.instanceRole === instanceSettings.instanceRole;
if (shouldTrigger) await handlerClass[methodName].call(handlerClass, ...args);
});
};
this.pubsubEventBus.on(eventName, eventHandler);
this.eventHandlers.push({ eventName, handler: eventHandler });
}
}
}

View File

@@ -40,6 +40,7 @@ export namespace PubSub {
namespace Commands {
export type ReloadLicense = ToCommand<'reload-license'>;
export type ReloadOIDCConfiguration = ToCommand<'reload-oidc-config'>;
export type RestartEventBus = ToCommand<'restart-event-bus'>;
export type ReloadExternalSecretsProviders = ToCommand<'reload-external-secrets-providers'>;
export type CommunityPackageInstall = ToCommand<'community-package-install'>;
@@ -72,7 +73,8 @@ export namespace PubSub {
| Commands.DisplayWorkflowDeactivation
| Commands.DisplayWorkflowActivationError
| Commands.RelayExecutionLifecycleEvent
| Commands.ClearTestWebhooks;
| Commands.ClearTestWebhooks
| Commands.ReloadOIDCConfiguration;
// ----------------------------------
// worker responses

View File

@@ -66,6 +66,7 @@ import '@/webhooks/webhooks.controller';
import { ChatServer } from './chat/chat-server';
import { MfaService } from './mfa/mfa.service';
import { PubSubRegistry } from './scaling/pubsub/pubsub.registry';
@Service()
export class Server extends AbstractServer {
@@ -252,6 +253,9 @@ export class Server extends AbstractServer {
await this.registerAdditionalControllers();
// Reinitialize the PubSubRegistry
Container.get(PubSubRegistry).init();
// register all known controllers
Container.get(ControllerRegistry).activate(app);

View File

@@ -0,0 +1,95 @@
import { SettingsRepository } from '@n8n/db';
import { Container } from '@n8n/di';
import { mock } from 'jest-mock-extended';
import config from '@/config';
import { reloadAuthenticationMethod } from '../sso-helpers';
jest.mock('@/config');
describe('sso-helpers', () => {
let settingsRepository: SettingsRepository;
let mockConfig: any;
beforeEach(() => {
jest.resetAllMocks();
Container.reset();
settingsRepository = mock<SettingsRepository>();
Container.set(SettingsRepository, settingsRepository);
mockConfig = {
set: jest.fn(),
};
(config as any).set = mockConfig.set;
});
describe('reloadAuthenticationMethod', () => {
it('should reload authentication method from database', async () => {
const mockSetting = {
key: 'userManagement.authenticationMethod',
value: 'oidc',
};
settingsRepository.findByKey = jest.fn().mockResolvedValue(mockSetting);
await reloadAuthenticationMethod();
expect(settingsRepository.findByKey).toHaveBeenCalledWith(
'userManagement.authenticationMethod',
);
expect(mockConfig.set).toHaveBeenCalledWith('userManagement.authenticationMethod', 'oidc');
});
it('should handle valid authentication methods', async () => {
const validMethods = ['ldap', 'email', 'saml', 'oidc'];
for (const method of validMethods) {
const mockSetting = {
key: 'userManagement.authenticationMethod',
value: method,
};
settingsRepository.findByKey = jest.fn().mockResolvedValue(mockSetting);
await reloadAuthenticationMethod();
expect(mockConfig.set).toHaveBeenCalledWith('userManagement.authenticationMethod', method);
}
});
it('should handle invalid authentication method', async () => {
const mockSetting = {
key: 'userManagement.authenticationMethod',
value: 'invalid-method',
};
settingsRepository.findByKey = jest.fn().mockResolvedValue(mockSetting);
await reloadAuthenticationMethod();
expect(mockConfig.set).not.toHaveBeenCalled();
});
it('should handle missing authentication method setting', async () => {
settingsRepository.findByKey = jest.fn().mockResolvedValue(null);
await reloadAuthenticationMethod();
expect(settingsRepository.findByKey).toHaveBeenCalledWith(
'userManagement.authenticationMethod',
);
expect(mockConfig.set).not.toHaveBeenCalled();
});
it('should handle database errors gracefully', async () => {
const error = new Error('Database connection failed');
settingsRepository.findByKey = jest.fn().mockRejectedValue(error);
await expect(reloadAuthenticationMethod()).rejects.toThrow('Database connection failed');
expect(mockConfig.set).not.toHaveBeenCalled();
});
});
});

View File

@@ -0,0 +1,232 @@
import type { Logger } from '@n8n/backend-common';
import { mockInstance, mockLogger } from '@n8n/backend-test-utils';
import type { GlobalConfig } from '@n8n/config';
import type { AuthIdentityRepository, SettingsRepository, UserRepository } from '@n8n/db';
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 type { JwtService } from '@/services/jwt.service';
import type { UrlService } from '@/services/url.service';
import * as ssoHelpers from '../../sso-helpers';
import { OIDC_PREFERENCES_DB_KEY } from '../constants';
import { OidcService } from '../oidc.service.ee';
import { Publisher } from '@/scaling/pubsub/publisher.service';
describe('OidcService', () => {
let oidcService: OidcService;
let settingsRepository: SettingsRepository;
let globalConfig: GlobalConfig;
let instanceSettings: InstanceSettings;
let cipher: Cipher;
let logger: Logger;
let jwtService: JwtService;
const mockOidcConfig = {
clientId: 'test-client-id',
clientSecret: 'test-client-secret',
discoveryEndpoint: 'https://example.com/.well-known/openid_configuration',
scope: 'openid profile email',
loginEnabled: true,
loginLabel: 'Login with OIDC',
loginButtonColor: '#1f2937',
};
const mockConfigFromDB = {
key: OIDC_PREFERENCES_DB_KEY,
value: JSON.stringify(mockOidcConfig),
loadOnStartup: true,
};
beforeEach(async () => {
jest.resetAllMocks();
Container.reset();
settingsRepository = mock<SettingsRepository>();
globalConfig = mock<GlobalConfig>({
sso: { oidc: { loginEnabled: false } },
});
instanceSettings = mock<InstanceSettings>({
isMultiMain: true,
});
cipher = mock<Cipher>();
logger = mockLogger();
jwtService = mock<JwtService>();
jest
.spyOn(ssoHelpers, 'setCurrentAuthenticationMethod')
.mockImplementation(async () => await Promise.resolve());
oidcService = new OidcService(
settingsRepository,
mock<AuthIdentityRepository>(),
mock<UrlService>(),
globalConfig,
mock<UserRepository>(),
cipher,
logger,
jwtService,
instanceSettings,
);
await oidcService.init();
});
describe('reload', () => {
it('should reload OIDC configuration from database', async () => {
settingsRepository.findByKey = jest.fn().mockResolvedValue(mockConfigFromDB);
// Mock the discovery endpoint response
global.fetch = jest.fn().mockResolvedValue({
ok: true,
json: async () => {
return await Promise.resolve({
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',
});
},
});
await oidcService.reload();
expect(settingsRepository.findByKey).toHaveBeenCalledWith(OIDC_PREFERENCES_DB_KEY);
expect(logger.debug).toHaveBeenCalledWith(
'OIDC configuration changed, starting to load it from the database',
);
});
it('should handle reload when no config exists in database', async () => {
settingsRepository.findByKey = jest.fn().mockResolvedValue(null);
await oidcService.reload();
expect(logger.warn).toHaveBeenCalledWith(
'OIDC configuration not found in database, ignoring reload message',
);
});
it('should handle errors during reload', async () => {
const error = new Error('Database error');
settingsRepository.findByKey = jest.fn().mockRejectedValue(error);
await oidcService.reload();
expect(logger.error).toHaveBeenCalledWith(
'OIDC configuration changed, failed to reload OIDC configuration',
{ error },
);
});
});
describe('loadConfigurationFromDatabase', () => {
it('should return undefined for empty discovery endpoint', async () => {
const configWithEmptyEndpoint = {
...mockOidcConfig,
discoveryEndpoint: '',
};
settingsRepository.findByKey = jest.fn().mockResolvedValue({
key: OIDC_PREFERENCES_DB_KEY,
value: JSON.stringify(configWithEmptyEndpoint),
loadOnStartup: true,
});
const result = await oidcService.loadConfigurationFromDatabase();
expect(result).toBeUndefined();
});
it('should handle invalid JSON in database', async () => {
settingsRepository.findByKey = jest.fn().mockResolvedValue({
key: OIDC_PREFERENCES_DB_KEY,
value: 'invalid json',
loadOnStartup: true,
});
const result = await oidcService.loadConfigurationFromDatabase();
expect(result).toBeUndefined();
expect(logger.warn).toHaveBeenCalledWith(
'Failed to load OIDC configuration from database, falling back to default configuration.',
expect.any(Object),
);
});
it('should decrypt client secret when requested', async () => {
const encryptedSecret = 'encrypted-secret';
const decryptedSecret = 'decrypted-secret';
cipher.decrypt = jest.fn().mockReturnValue(decryptedSecret);
const configWithEncryptedSecret = {
...mockOidcConfig,
clientSecret: encryptedSecret,
};
settingsRepository.findByKey = jest.fn().mockResolvedValue({
key: OIDC_PREFERENCES_DB_KEY,
value: JSON.stringify(configWithEncryptedSecret),
loadOnStartup: true,
});
global.fetch = jest.fn().mockResolvedValue({
ok: true,
json: async () => {
return await Promise.resolve({
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',
});
},
});
const result = await oidcService.loadConfigurationFromDatabase(true);
expect(cipher.decrypt).toHaveBeenCalledWith(encryptedSecret);
expect(result?.clientSecret).toBe(decryptedSecret);
});
});
describe('broadcastReloadOIDCConfigurationCommand', () => {
const mockPublisher = { publishCommand: jest.fn() };
beforeEach(() => {
mockInstance(Publisher, mockPublisher);
});
it('should publish reload command in multi-main setup', async () => {
(instanceSettings as any).isMultiMain = true;
// Trigger broadcast by updating config
settingsRepository.save = jest.fn().mockResolvedValue(mockConfigFromDB);
settingsRepository.findByKey = jest.fn().mockResolvedValue(mockConfigFromDB);
jest.spyOn(client, 'discovery').mockResolvedValue({} as client.Configuration);
await oidcService.updateConfig(mockOidcConfig);
// In multi-main setup, should attempt to publish
expect(mockPublisher.publishCommand).toHaveBeenCalledWith({
command: 'reload-oidc-config',
});
});
it('should not publish in single main setup', async () => {
(instanceSettings as any).isMultiMain = false;
settingsRepository.update = jest.fn().mockResolvedValue(mockConfigFromDB);
settingsRepository.findByKey = jest.fn().mockResolvedValue(mockConfigFromDB);
jest.spyOn(client, 'discovery').mockResolvedValue({} as client.Configuration);
await oidcService.updateConfig(mockOidcConfig);
// Should not attempt to import Publisher in single main setup
expect(mockPublisher.publishCommand).not.toHaveBeenCalled();
});
});
});

View File

@@ -12,7 +12,7 @@ import {
} from '@n8n/db';
import { Container, Service } from '@n8n/di';
import { randomUUID } from 'crypto';
import { Cipher } from 'n8n-core';
import { Cipher, InstanceSettings } from 'n8n-core';
import { jsonParse, UserError } from 'n8n-workflow';
import * as client from 'openid-client';
@@ -26,9 +26,11 @@ import {
getCurrentAuthenticationMethod,
isEmailCurrentAuthenticationMethod,
isOidcCurrentAuthenticationMethod,
reloadAuthenticationMethod,
setCurrentAuthenticationMethod,
} from '../sso-helpers';
import { OIDC_CLIENT_SECRET_REDACTED_VALUE, OIDC_PREFERENCES_DB_KEY } from './constants';
import { OnPubSubEvent } from '@n8n/decorators';
const DEFAULT_OIDC_CONFIG: OidcConfigDto = {
clientId: '',
@@ -59,6 +61,7 @@ export class OidcService {
private readonly cipher: Cipher,
private readonly logger: Logger,
private readonly jwtService: JwtService,
private readonly instanceSettings: InstanceSettings,
) {}
async init() {
@@ -283,16 +286,57 @@ export class OidcService {
});
}
async loadConfig(decryptSecret = false): Promise<OidcRuntimeConfig> {
const currentConfig = await this.settingsRepository.findOneBy({
key: OIDC_PREFERENCES_DB_KEY,
});
private async broadcastReloadOIDCConfigurationCommand(): Promise<void> {
if (this.instanceSettings.isMultiMain) {
const { Publisher } = await import('@/scaling/pubsub/publisher.service');
await Container.get(Publisher).publishCommand({ command: 'reload-oidc-config' });
}
}
if (currentConfig) {
private isReloading = false;
@OnPubSubEvent('reload-oidc-config')
async reload(): Promise<void> {
if (this.isReloading) {
this.logger.warn('OIDC configuration reload already in progress');
return;
}
this.isReloading = true;
try {
this.logger.debug('OIDC configuration changed, starting to load it from the database');
const configFromDB = await this.loadConfigurationFromDatabase(true);
if (configFromDB) {
this.oidcConfig = configFromDB;
this.cachedOidcConfiguration = undefined;
} else {
this.logger.warn('OIDC configuration not found in database, ignoring reload message');
}
await reloadAuthenticationMethod();
const isOidcLoginEnabled = isOidcCurrentAuthenticationMethod();
this.logger.debug(`OIDC login is now ${isOidcLoginEnabled ? 'enabled' : 'disabled'}.`);
Container.get(GlobalConfig).sso.oidc.loginEnabled = isOidcLoginEnabled;
} catch (error) {
this.logger.error('OIDC configuration changed, failed to reload OIDC configuration', {
error,
});
} finally {
this.isReloading = false;
}
}
async loadConfigurationFromDatabase(
decryptSecret = false,
): Promise<OidcRuntimeConfig | undefined> {
const configFromDB = await this.settingsRepository.findByKey(OIDC_PREFERENCES_DB_KEY);
if (configFromDB) {
try {
const oidcConfig = jsonParse<OidcConfigDto>(currentConfig.value);
const oidcConfig = jsonParse<OidcConfigDto>(configFromDB.value);
if (oidcConfig.discoveryEndpoint === '') return DEFAULT_OIDC_RUNTIME_CONFIG;
if (oidcConfig.discoveryEndpoint === '') return undefined;
const discoveryUrl = new URL(oidcConfig.discoveryEndpoint);
@@ -311,12 +355,16 @@ export class OidcService {
);
}
}
return undefined;
}
async loadConfig(decryptSecret = false): Promise<OidcRuntimeConfig> {
const currentConfig = await this.loadConfigurationFromDatabase(decryptSecret);
if (currentConfig) {
return currentConfig;
}
await this.settingsRepository.save({
key: OIDC_PREFERENCES_DB_KEY,
value: JSON.stringify(DEFAULT_OIDC_CONFIG),
loadOnStartup: true,
});
return DEFAULT_OIDC_RUNTIME_CONFIG;
}
@@ -344,17 +392,14 @@ export class OidcService {
this.logger.error('Failed to discover OIDC metadata', { error });
throw new UserError('Failed to discover OIDC metadata, based on the provided configuration');
}
await this.settingsRepository.update(
{
key: OIDC_PREFERENCES_DB_KEY,
},
{
value: JSON.stringify({
...newConfig,
clientSecret: this.cipher.encrypt(newConfig.clientSecret),
}),
},
);
await this.settingsRepository.save({
key: OIDC_PREFERENCES_DB_KEY,
value: JSON.stringify({
...newConfig,
clientSecret: this.cipher.encrypt(newConfig.clientSecret),
}),
loadOnStartup: true,
});
// TODO: Discuss this in product
// if (this.oidcConfig.loginEnabled && !newConfig.loginEnabled) {
@@ -371,6 +416,8 @@ export class OidcService {
);
await this.setOidcLoginEnabled(this.oidcConfig.loginEnabled);
await this.broadcastReloadOIDCConfigurationCommand();
}
private async setOidcLoginEnabled(enabled: boolean): Promise<void> {

View File

@@ -1,8 +1,9 @@
import { GlobalConfig } from '@n8n/config';
import { SettingsRepository, type AuthProviderType } from '@n8n/db';
import { isAuthProviderType, SettingsRepository, type AuthProviderType } from '@n8n/db';
import { Container } from '@n8n/di';
import config from '@/config';
import { Logger } from '@n8n/backend-common';
/**
* Only one authentication method can be active at a time. This function sets
@@ -24,6 +25,25 @@ export async function setCurrentAuthenticationMethod(
);
}
export async function reloadAuthenticationMethod(): Promise<void> {
const settings = await Container.get(SettingsRepository).findByKey(
'userManagement.authenticationMethod',
);
if (settings) {
if (isAuthProviderType(settings.value)) {
const authenticationMethod = settings.value;
config.set('userManagement.authenticationMethod', authenticationMethod);
Container.get(Logger).debug('Reloaded authentication method from the database', {
authenticationMethod,
});
} else {
Container.get(Logger).warn('Invalid authentication method read from the database', {
value: settings.value,
});
}
}
}
export function getCurrentAuthenticationMethod(): AuthProviderType {
return config.getEnv('userManagement.authenticationMethod');
}

View File

@@ -76,6 +76,7 @@ describe('OIDC service', () => {
};
await oidcService.updateConfig(newConfig);
const loadedConfig = await oidcService.loadConfig();
expect(loadedConfig.clientId).toEqual('test-client-id');