chore(core): Synchronize SAML settings in multi-main (#19588)

2025-12-16 17:46:45 +00:00 · 2025-09-16 16:31:20 +02:00
parent a7f4e3e323
commit 0c0188fe40
5 changed files with 281 additions and 14 deletions
--- a/packages/cli/src/scaling/pubsub/pubsub.event-map.ts
+++ b/packages/cli/src/scaling/pubsub/pubsub.event-map.ts
@@ -15,6 +15,7 @@ export type PubSubCommandMap = {
 	// # region SSO

 	'reload-oidc-config': never;
+	'reload-saml-config': never;

 	// #endregion

--- a/packages/cli/src/scaling/pubsub/pubsub.types.ts
+++ b/packages/cli/src/scaling/pubsub/pubsub.types.ts
@@ -41,6 +41,7 @@ export namespace PubSub {
 	namespace Commands {
 		export type ReloadLicense = ToCommand<'reload-license'>;
 		export type ReloadOIDCConfiguration = ToCommand<'reload-oidc-config'>;
+		export type ReloadSamlConfiguration = ToCommand<'reload-saml-config'>;
 		export type RestartEventBus = ToCommand<'restart-event-bus'>;
 		export type ReloadExternalSecretsProviders = ToCommand<'reload-external-secrets-providers'>;
 		export type CommunityPackageInstall = ToCommand<'community-package-install'>;
@@ -74,7 +75,8 @@ export namespace PubSub {
 		| Commands.DisplayWorkflowActivationError
 		| Commands.RelayExecutionLifecycleEvent
 		| Commands.ClearTestWebhooks
-		| Commands.ReloadOIDCConfiguration;
+		| Commands.ReloadOIDCConfiguration
+		| Commands.ReloadSamlConfiguration;

 	// ----------------------------------
 	//         worker responses
--- a/packages/cli/src/sso.ee/saml/tests/saml.service.ee.test.ts
+++ b/packages/cli/src/sso.ee/saml/tests/saml.service.ee.test.ts
@@ -1,14 +1,22 @@
-import { mockInstance } from '@n8n/backend-test-utils';
+import type { SamlPreferences } from '@n8n/api-types';
+import { mockInstance, mockLogger } from '@n8n/backend-test-utils';
+import type { GlobalConfig } from '@n8n/config';
 import { SettingsRepository } from '@n8n/db';
+import type { UserRepository } from '@n8n/db';
 import type { Settings } from '@n8n/db';
+import { Container } from '@n8n/di';
 import axios from 'axios';
 import type express from 'express';
 import { mock } from 'jest-mock-extended';
+import type { InstanceSettings } from 'n8n-core';
 import type { IdentityProviderInstance, ServiceProviderInstance } from 'samlify';

 import { BadRequestError } from '@/errors/response-errors/bad-request.error';
+import { Publisher } from '@/scaling/pubsub/publisher.service';
+import type { UrlService } from '@/services/url.service';
 import * as samlHelpers from '@/sso.ee/saml/saml-helpers';
 import { SamlService } from '@/sso.ee/saml/saml.service.ee';
+import * as ssoHelpers from '@/sso.ee/sso-helpers';

 import { SAML_PREFERENCES_DB_KEY } from '../constants';
 import { InvalidSamlMetadataUrlError } from '../errors/invalid-saml-metadata-url.error';
@@ -137,16 +145,68 @@ const SamlSettingWithValidUrl: Settings = {
 };

 describe('SamlService', () => {
+	let samlService: SamlService;
+	let settingsRepository: SettingsRepository;
+	let instanceSettings: InstanceSettings;
+	let globalConfig: GlobalConfig;
 	const validator = new SamlValidator(mock());
-	const settingsRepository = mockInstance(SettingsRepository);
-	const samlService = new SamlService(mock(), mock(), validator, mock(), settingsRepository);
+	const logger = mockLogger();
+
+	const mockSamlConfig = {
+		mapping: {
+			email: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress',
+			firstName: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/firstname',
+			lastName: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/lastname',
+			userPrincipalName: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn',
+		},
+		metadata:
+			'<?xml version="1.0" encoding="UTF-8" standalone="no"?>\n<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://saml.example.com/entityid" validUntil="2035-05-07T13:33:47.181Z">\n  <md:IDPSSODescriptor WantAuthnRequestsSigned="true" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">\n    <md:KeyDescriptor use="signing">\n      <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">\n        <ds:X509Data>\n          <ds:X509Certificate>MIIC4jCCAcoCCQC33wnybT5QZDANBgkqhkiG9w0BAQsFADAyMQswCQYDVQQGEwJV\nSzEPMA0GA1UECgwGQm94eUhRMRIwEAYDVQQDDAlNb2NrIFNBTUwwIBcNMjIwMjI4\nMjE0NjM4WhgPMzAyMTA3MDEyMTQ2MzhaMDIxCzAJBgNVBAYTAlVLMQ8wDQYDVQQK\nDAZCb3h5SFExEjAQBgNVBAMMCU1vY2sgU0FNTDCCASIwDQYJKoZIhvcNAQEBBQAD\nggEPADCCAQoCggEBALGfYettMsct1T6tVUwTudNJH5Pnb9GGnkXi9Zw/e6x45DD0\nRuRONbFlJ2T4RjAE/uG+AjXxXQ8o2SZfb9+GgmCHuTJFNgHoZ1nFVXCmb/Hg8Hpd\n4vOAGXndixaReOiq3EH5XvpMjMkJ3+8+9VYMzMZOjkgQtAqO36eAFFfNKX7dTj3V\npwLkvz6/KFCq8OAwY+AUi4eZm5J57D31GzjHwfjH9WTeX0MyndmnNB1qV75qQR3b\n2/W5sGHRv+9AarggJkF+ptUkXoLtVA51wcfYm6hILptpde5FQC8RWY1YrswBWAEZ\nNfyrR4JeSweElNHg4NVOs4TwGjOPwWGqzTfgTlECAwEAATANBgkqhkiG9w0BAQsF\nAAOCAQEAAYRlYflSXAWoZpFfwNiCQVE5d9zZ0DPzNdWhAybXcTyMf0z5mDf6FWBW\n5Gyoi9u3EMEDnzLcJNkwJAAc39Apa4I2/tml+Jy29dk8bTyX6m93ngmCgdLh5Za4\nkhuU3AM3L63g7VexCuO7kwkjh/+LqdcIXsVGO6XDfu2QOs1Xpe9zIzLpwm/RNYeX\nUjbSj5ce/jekpAw7qyVVL4xOyh8AtUW1ek3wIw1MJvEgEPt0d16oshWJpoS1OT8L\nr/22SvYEo3EmSGdTVGgk3x3s+A0qWAqTcyjr7Q4s/GKYRFfomGwz0TZ4Iw1ZN99M\nm0eo2USlSRTVl7QHRTuiuSThHpLKQQ==</ds:X509Certificate>\n        </ds:X509Data>\n      </ds:KeyInfo>\n    </md:KeyDescriptor>\n    <md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat>\n    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://mocksaml.com/api/saml/sso"/>\n    <md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://mocksaml.com/api/saml/sso"/>\n  </md:IDPSSODescriptor>\n</md:EntityDescriptor>',
+		metadataUrl: '',
+		ignoreSSL: false,
+		loginBinding: 'redirect',
+		acsBinding: 'post',
+		authnRequestsSigned: false,
+		loginEnabled: false,
+		loginLabel: 'SAML',
+	};
+
+	const mockConfigFromDB = {
+		key: SAML_PREFERENCES_DB_KEY,
+		value: JSON.stringify(mockSamlConfig),
+		loadOnStartup: true,
+	};

 	beforeAll(async () => {
 		await validator.init();
 	});

-	beforeEach(() => {
-		jest.restoreAllMocks();
+	beforeEach(async () => {
+		jest.resetAllMocks();
+		Container.reset();
+
+		settingsRepository = mockInstance(SettingsRepository);
+		instanceSettings = mock<InstanceSettings>({
+			isMultiMain: true,
+		});
+		globalConfig = mock<GlobalConfig>({
+			sso: { saml: { loginEnabled: false } },
+		});
+
+		jest
+			.spyOn(ssoHelpers, 'reloadAuthenticationMethod')
+			.mockImplementation(async () => await Promise.resolve());
+		jest.spyOn(samlHelpers, 'isSamlLoginEnabled').mockReturnValue(true);
+
+		samlService = new SamlService(
+			logger,
+			mock<UrlService>(),
+			validator,
+			mock<UserRepository>(),
+			settingsRepository,
+			instanceSettings,
+		);
+		// Mock GlobalConfig container access
+		Container.set(require('@n8n/config').GlobalConfig, globalConfig);
 	});

 	describe('getAttributesFromLoginResponse', () => {
@@ -257,7 +317,7 @@ describe('SamlService', () => {
 			jest.spyOn(settingsRepository, 'findOne').mockResolvedValue(InvalidSamlSetting);

 			// ACT && ASSERT
-			await expect(samlService.loadFromDbAndApplySamlPreferences(true)).rejects.toThrowError(
+			await expect(samlService.loadFromDbAndApplySamlPreferences(true, false)).rejects.toThrowError(
 				InvalidSamlMetadataError,
 			);
 		});
@@ -269,7 +329,7 @@ describe('SamlService', () => {
 				.mockResolvedValue(SamlSettingWithInvalidUrlAndInvalidMetadataXML);

 			// ACT && ASSERT
-			await expect(samlService.loadFromDbAndApplySamlPreferences(true)).rejects.toThrowError(
+			await expect(samlService.loadFromDbAndApplySamlPreferences(true, false)).rejects.toThrowError(
 				InvalidSamlMetadataError,
 			);
 		});
@@ -279,7 +339,7 @@ describe('SamlService', () => {
 			jest.spyOn(settingsRepository, 'findOne').mockResolvedValue(SamlSettingWithInvalidUrl);

 			// ACT && ASSERT
-			await samlService.loadFromDbAndApplySamlPreferences(true);
+			await samlService.loadFromDbAndApplySamlPreferences(true, false);
 		});

 		test('does not throw an error when the metadata url is valid', async () => {
@@ -292,7 +352,7 @@ describe('SamlService', () => {
 				);

 			// ACT && ASSERT
-			await samlService.loadFromDbAndApplySamlPreferences(true);
+			await samlService.loadFromDbAndApplySamlPreferences(true, false);
 		});
 	});

@@ -412,12 +472,167 @@ describe('SamlService', () => {
 			await samlService.loadPreferencesWithoutValidation({
 				metadata: SamlMetadataWithoutRedirectBinding,
 			});
+			await samlService.loadSamlify();
 			expect(() => samlService.getIdentityProviderInstance(true)).toThrowError(
 				InvalidSamlMetadataError,
 			);
 		});
 	});

+	describe('broadcastReloadSAMLConfigurationCommand', () => {
+		const mockPublisher = { publishCommand: jest.fn() };
+		beforeEach(() => {
+			mockInstance(Publisher, mockPublisher);
+			// Mock all the validation and setup methods that setSamlPreferences calls
+			jest.spyOn(samlService, 'loadSamlify').mockResolvedValue(undefined);
+			jest.spyOn(validator, 'validateMetadata').mockResolvedValue(true);
+			jest.spyOn(samlService, 'getIdentityProviderInstance').mockReturnValue({} as any);
+			jest
+				.spyOn(samlService, 'saveSamlPreferencesToDb')
+				.mockResolvedValue(mockSamlConfig as SamlPreferences);
+			// Mock SAML login as disabled to avoid metadata validation
+			jest.spyOn(samlHelpers, 'isSamlLoginEnabled').mockReturnValue(false);
+		});
+
+		test('should publish reload command in multi-main setup', async () => {
+			(instanceSettings as any).isMultiMain = true;
+
+			await samlService.setSamlPreferences(
+				{ loginEnabled: false, metadata: mockSamlConfig.metadata },
+				false,
+				true,
+			);
+
+			expect(mockPublisher.publishCommand).toHaveBeenCalledWith({
+				command: 'reload-saml-config',
+			});
+		});
+
+		test('should not publish in single main setup', async () => {
+			(instanceSettings as any).isMultiMain = false;
+
+			await samlService.setSamlPreferences(
+				{ loginEnabled: false, metadata: mockSamlConfig.metadata },
+				false,
+				true,
+			);
+
+			expect(mockPublisher.publishCommand).not.toHaveBeenCalled();
+		});
+
+		test('should not publish when broadcastReload is false', async () => {
+			(instanceSettings as any).isMultiMain = true;
+
+			await samlService.setSamlPreferences(
+				{ loginEnabled: false, metadata: mockSamlConfig.metadata },
+				false,
+				false,
+			);
+
+			expect(mockPublisher.publishCommand).not.toHaveBeenCalled();
+		});
+	});
+
+	describe('reload', () => {
+		test('should reload SAML configuration from database', async () => {
+			settingsRepository.findOne = jest.fn().mockResolvedValue(mockConfigFromDB);
+			jest
+				.spyOn(samlService, 'loadFromDbAndApplySamlPreferences')
+				.mockResolvedValue(mockSamlConfig as SamlPreferences);
+
+			await samlService.reload();
+
+			expect(samlService.loadFromDbAndApplySamlPreferences).toHaveBeenCalledWith(true, false);
+			expect(ssoHelpers.reloadAuthenticationMethod).toHaveBeenCalled();
+			expect(globalConfig.sso.saml.loginEnabled).toBe(true);
+			expect(logger.debug).toHaveBeenCalledWith(
+				'SAML configuration changed, starting to load it from the database',
+			);
+		});
+
+		test('should prevent concurrent reloads with isReloading flag', async () => {
+			jest
+				.spyOn(samlService, 'loadFromDbAndApplySamlPreferences')
+				.mockResolvedValue(mockSamlConfig as SamlPreferences);
+
+			// Start first reload without awaiting
+			const firstReload = samlService.reload();
+			// Start second reload immediately
+			const secondReload = samlService.reload();
+
+			await Promise.all([firstReload, secondReload]);
+
+			// Should have called loadFromDbAndApplySamlPreferences only once due to isReloading flag
+			expect(samlService.loadFromDbAndApplySamlPreferences).toHaveBeenCalledTimes(1);
+			expect(logger.warn).toHaveBeenCalledWith('SAML configuration reload already in progress');
+		});
+
+		test('should handle errors during reload gracefully', async () => {
+			const error = new Error('Database connection failed');
+			jest.spyOn(samlService, 'loadFromDbAndApplySamlPreferences').mockRejectedValue(error);
+
+			await samlService.reload();
+
+			expect(logger.error).toHaveBeenCalledWith(
+				'SAML configuration changed, failed to reload SAML configuration',
+				{ error },
+			);
+			// Should reset isReloading flag even on error
+			// Test by calling reload again - should not be blocked
+			jest
+				.spyOn(samlService, 'loadFromDbAndApplySamlPreferences')
+				.mockResolvedValue(mockSamlConfig as SamlPreferences);
+
+			await samlService.reload();
+			expect(samlService.loadFromDbAndApplySamlPreferences).toHaveBeenCalledTimes(2);
+		});
+
+		test('should update GlobalConfig with login status', async () => {
+			jest
+				.spyOn(samlService, 'loadFromDbAndApplySamlPreferences')
+				.mockResolvedValue(mockSamlConfig as SamlPreferences);
+			// Mock SAML as disabled
+			jest.spyOn(samlHelpers, 'isSamlLoginEnabled').mockReturnValue(false);
+
+			await samlService.reload();
+
+			expect(globalConfig.sso.saml.loginEnabled).toBe(false);
+			expect(logger.debug).toHaveBeenCalledWith('SAML login is now disabled.');
+		});
+	});
+
+	describe('loadFromDbAndApplySamlPreferences with broadcastReload parameter', () => {
+		beforeEach(() => {
+			// Mock required methods to avoid complex initialization
+			jest.spyOn(samlService, 'loadSamlify').mockResolvedValue(undefined);
+			jest.spyOn(samlService, 'getIdentityProviderInstance').mockReturnValue({} as any);
+			jest
+				.spyOn(samlService, 'saveSamlPreferencesToDb')
+				.mockResolvedValue(mockSamlConfig as SamlPreferences);
+			jest
+				.spyOn(samlService as any, 'broadcastReloadSAMLConfigurationCommand')
+				.mockResolvedValue(undefined);
+		});
+
+		test('should call setSamlPreferences with broadcastReload=true by default', async () => {
+			settingsRepository.findOne = jest.fn().mockResolvedValue(mockConfigFromDB);
+			jest.spyOn(samlService, 'setSamlPreferences');
+
+			await samlService.loadFromDbAndApplySamlPreferences(true);
+
+			expect(samlService.setSamlPreferences).toHaveBeenCalledWith(mockSamlConfig, true, true);
+		});
+
+		test('should call setSamlPreferences with broadcastReload=false when specified', async () => {
+			settingsRepository.findOne = jest.fn().mockResolvedValue(mockConfigFromDB);
+			jest.spyOn(samlService, 'setSamlPreferences');
+
+			await samlService.loadFromDbAndApplySamlPreferences(true, false);
+
+			expect(samlService.setSamlPreferences).toHaveBeenCalledWith(mockSamlConfig, true, false);
+		});
+	});
+
 	describe('reset', () => {
 		test('disables saml login and deletes the saml `features.saml` key in the db', async () => {
 			// ARRANGE
--- a/packages/cli/src/sso.ee/saml/saml.service.ee.ts
+++ b/packages/cli/src/sso.ee/saml/saml.service.ee.ts
@@ -1,11 +1,14 @@
 import type { SamlPreferences } from '@n8n/api-types';
 import { Logger } from '@n8n/backend-common';
+import { GlobalConfig } from '@n8n/config';
 import type { Settings, User } from '@n8n/db';
 import { isValidEmail, SettingsRepository, UserRepository } from '@n8n/db';
-import { Service } from '@n8n/di';
+import { OnPubSubEvent } from '@n8n/decorators';
+import { Container, Service } from '@n8n/di';
 import axios from 'axios';
 import type express from 'express';
 import https from 'https';
+import { InstanceSettings } from 'n8n-core';
 import { jsonParse, UnexpectedError } from 'n8n-workflow';
 import { type IdentityProviderInstance, type ServiceProviderInstance } from 'samlify';
 import type { BindingContext, PostBindingContext } from 'samlify/types/src/entity';
@@ -30,7 +33,7 @@ import {
 import { SamlValidator } from './saml-validator';
 import { getServiceProviderInstance } from './service-provider.ee';
 import type { SamlLoginBinding, SamlUserAttributes } from './types';
-import { isSsoJustInTimeProvisioningEnabled } from '../sso-helpers';
+import { isSsoJustInTimeProvisioningEnabled, reloadAuthenticationMethod } from '../sso-helpers';

@Service()
 export class SamlService {
@@ -80,6 +83,7 @@ export class SamlService {
 		private readonly validator: SamlValidator,
 		private readonly userRepository: UserRepository,
 		private readonly settingsRepository: SettingsRepository,
+		private readonly instanceSettings: InstanceSettings,
 	) {}

 	async init(): Promise<void> {
@@ -247,9 +251,46 @@ export class SamlService {
 		};
 	}

+	private async broadcastReloadSAMLConfigurationCommand(): Promise<void> {
+		if (this.instanceSettings.isMultiMain) {
+			const { Publisher } = await import('@/scaling/pubsub/publisher.service');
+			await Container.get(Publisher).publishCommand({ command: 'reload-saml-config' });
+		}
+	}
+
+	private isReloading = false;
+
+	@OnPubSubEvent('reload-saml-config')
+	async reload(): Promise<void> {
+		if (this.isReloading) {
+			this.logger.warn('SAML configuration reload already in progress');
+			return;
+		}
+		this.isReloading = true;
+		try {
+			this.logger.debug('SAML configuration changed, starting to load it from the database');
+			await this.loadFromDbAndApplySamlPreferences(true, false);
+
+			await reloadAuthenticationMethod();
+
+			const samlLoginEnabled = isSamlLoginEnabled();
+
+			this.logger.debug(`SAML login is now ${samlLoginEnabled ? 'enabled' : 'disabled'}.`);
+
+			Container.get(GlobalConfig).sso.saml.loginEnabled = samlLoginEnabled;
+		} catch (error) {
+			this.logger.error('SAML configuration changed, failed to reload SAML configuration', {
+				error,
+			});
+		} finally {
+			this.isReloading = false;
+		}
+	}
+
 	async setSamlPreferences(
 		prefs: Partial<SamlPreferences>,
 		tryFallback: boolean = false,
+		broadcastReload: boolean = true,
 	): Promise<SamlPreferences | undefined> {
 		await this.loadSamlify();
 		const previousMetadataUrl = this._samlPreferences.metadataUrl;
@@ -304,6 +345,10 @@ export class SamlService {
 		}
 		this.getIdentityProviderInstance(true);
 		const result = await this.saveSamlPreferencesToDb();
+
+		if (broadcastReload) {
+			await this.broadcastReloadSAMLConfigurationCommand();
+		}
 		return result;
 	}

@@ -332,7 +377,10 @@ export class SamlService {
 		setSamlLoginLabel(prefs.loginLabel ?? getSamlLoginLabel());
 	}

-	async loadFromDbAndApplySamlPreferences(apply = true): Promise<SamlPreferences | undefined> {
+	async loadFromDbAndApplySamlPreferences(
+		apply = true,
+		broadcastReload: boolean = true,
+	): Promise<SamlPreferences | undefined> {
 		const samlPreferences = await this.settingsRepository.findOne({
 			where: { key: SAML_PREFERENCES_DB_KEY },
 		});
@@ -340,7 +388,7 @@ export class SamlService {
 			const prefs = jsonParse<SamlPreferences>(samlPreferences.value);
 			if (prefs) {
 				if (apply) {
-					await this.setSamlPreferences(prefs, true);
+					await this.setSamlPreferences(prefs, true, broadcastReload);
 				} else {
 					await this.loadPreferencesWithoutValidation(prefs);
 				}