diff --git a/packages/@n8n/decorators/src/pubsub/pubsub-metadata.ts b/packages/@n8n/decorators/src/pubsub/pubsub-metadata.ts index 5583187c30..cd01b3476b 100644 --- a/packages/@n8n/decorators/src/pubsub/pubsub-metadata.ts +++ b/packages/@n8n/decorators/src/pubsub/pubsub-metadata.ts @@ -17,6 +17,7 @@ export type PubSubEventName = | 'reload-external-secrets-providers' | 'reload-license' | 'reload-oidc-config' + | 'reload-saml-config' | 'response-to-get-worker-status' | 'restart-event-bus' | 'relay-execution-lifecycle-event'; diff --git a/packages/cli/src/scaling/pubsub/pubsub.event-map.ts b/packages/cli/src/scaling/pubsub/pubsub.event-map.ts index 8784eedb30..2e772e3d6f 100644 --- 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 diff --git a/packages/cli/src/scaling/pubsub/pubsub.types.ts b/packages/cli/src/scaling/pubsub/pubsub.types.ts index c3d3b53b68..5aec2c381d 100644 --- 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 diff --git 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 index 1e8d9cc390..d3ab865b8d 100644 --- 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: + '\n\n \n \n \n \n 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==\n \n \n \n urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress\n \n \n \n', + 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({ + isMultiMain: true, + }); + globalConfig = mock({ + sso: { saml: { loginEnabled: false } }, + }); + + jest + .spyOn(ssoHelpers, 'reloadAuthenticationMethod') + .mockImplementation(async () => await Promise.resolve()); + jest.spyOn(samlHelpers, 'isSamlLoginEnabled').mockReturnValue(true); + + samlService = new SamlService( + logger, + mock(), + validator, + mock(), + 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 diff --git a/packages/cli/src/sso.ee/saml/saml.service.ee.ts b/packages/cli/src/sso.ee/saml/saml.service.ee.ts index 327b8417ce..084b702ca3 100644 --- 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 { @@ -247,9 +251,46 @@ export class SamlService { }; } + private async broadcastReloadSAMLConfigurationCommand(): Promise { + 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 { + 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, tryFallback: boolean = false, + broadcastReload: boolean = true, ): Promise { 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 { + async loadFromDbAndApplySamlPreferences( + apply = true, + broadcastReload: boolean = true, + ): Promise { const samlPreferences = await this.settingsRepository.findOne({ where: { key: SAML_PREFERENCES_DB_KEY }, }); @@ -340,7 +388,7 @@ export class SamlService { const prefs = jsonParse(samlPreferences.value); if (prefs) { if (apply) { - await this.setSamlPreferences(prefs, true); + await this.setSamlPreferences(prefs, true, broadcastReload); } else { await this.loadPreferencesWithoutValidation(prefs); }