diff --git a/packages/cli/src/modules/sso-saml/__tests__/saml.controller.ee.test.ts b/packages/cli/src/modules/sso-saml/__tests__/saml.controller.ee.test.ts index 02d3ae46813..fe9dc832573 100644 --- a/packages/cli/src/modules/sso-saml/__tests__/saml.controller.ee.test.ts +++ b/packages/cli/src/modules/sso-saml/__tests__/saml.controller.ee.test.ts @@ -197,7 +197,7 @@ describe('SAML Login Flow', () => { await controller.initSsoGet(req, res); - expect(samlService.getLoginRequestUrl).toHaveBeenCalledWith('/'); + expect(samlService.getLoginRequestUrl).toHaveBeenCalledWith('/', undefined, undefined); }); test('validates redirect URL that is passed in via referrer header', async () => { @@ -216,7 +216,7 @@ describe('SAML Login Flow', () => { await controller.initSsoGet(req, res); - expect(samlService.getLoginRequestUrl).toHaveBeenCalledWith('/'); + expect(samlService.getLoginRequestUrl).toHaveBeenCalledWith('/', undefined, undefined); }); const hostWithoutRedirect = 'http://localhost:5678/'; diff --git a/packages/cli/src/modules/sso-saml/__tests__/saml.service.ee.test.ts b/packages/cli/src/modules/sso-saml/__tests__/saml.service.ee.test.ts index 3df4719a42e..d7452f1bd4d 100644 --- a/packages/cli/src/modules/sso-saml/__tests__/saml.service.ee.test.ts +++ b/packages/cli/src/modules/sso-saml/__tests__/saml.service.ee.test.ts @@ -657,6 +657,16 @@ describe('SamlService', () => { }); describe('getIdentityProviderInstance', () => { + test('does throw `InvalidSamlMetadataError` when metadata is empty', async () => { + await samlService.loadPreferencesWithoutValidation({ + metadata: '', + }); + await samlService.loadSamlify(); + expect(() => samlService.getIdentityProviderInstance(true)).toThrowError( + InvalidSamlMetadataError, + ); + }); + test('does throw `InvalidSamlMetadataError` when a metadata does not contain redirect binding', async () => { await samlService.loadPreferencesWithoutValidation({ metadata: SamlMetadataWithoutRedirectBinding, diff --git a/packages/cli/src/modules/sso-saml/saml-validator.ts b/packages/cli/src/modules/sso-saml/saml-validator.ts index e5ff8d4bf92..da130a87a16 100644 --- a/packages/cli/src/modules/sso-saml/saml-validator.ts +++ b/packages/cli/src/modules/sso-saml/saml-validator.ts @@ -29,7 +29,7 @@ export class SamlValidator { this.xmllint = await import('xmllint-wasm'); } - validateIdentiyProvider(idp: IdentityProviderInstance) { + validateIdentityProvider(idp: IdentityProviderInstance) { const binding = idp.entityMeta.getSingleSignOnService( this.samlify.Constants.wording.binding.redirect, ); @@ -45,7 +45,7 @@ export class SamlValidator { const idp = this.samlify.IdentityProvider({ metadata, }); - this.validateIdentiyProvider(idp); + this.validateIdentityProvider(idp); } return validXML; diff --git a/packages/cli/src/modules/sso-saml/saml.controller.ee.ts b/packages/cli/src/modules/sso-saml/saml.controller.ee.ts index 69260e0a327..856b6904dfe 100644 --- a/packages/cli/src/modules/sso-saml/saml.controller.ee.ts +++ b/packages/cli/src/modules/sso-saml/saml.controller.ee.ts @@ -191,16 +191,30 @@ export class SamlController { /** * Test SAML config - * This endpoint is available if SAML is licensed and the requestor is an instance owner + * Accepts metadata from the request body so testing works without saving first. + * This endpoint is available if SAML is licensed and the requestor is an instance owner. */ - @Get('/config/test', { middlewares: [samlLicensedMiddleware] }) + @Post('/config/test', { middlewares: [samlLicensedMiddleware] }) @GlobalScope('saml:manage') - async configTestGet(_: AuthenticatedRequest, res: Response) { - return await this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl()); + async configTestPost(_req: AuthenticatedRequest, res: Response, @Body payload: SamlPreferences) { + return await this.handleInitSSO(res, getServiceProviderConfigTestReturnUrl(), payload); } - private async handleInitSSO(res: Response, relayState?: string) { - const result = await this.samlService.getLoginRequestUrl(relayState); + private async handleInitSSO(res: Response, relayState?: string, config?: SamlPreferences) { + let metadata: string | undefined; + if (config) { + metadata = config.metadata; + if (!metadata && config.metadataUrl) { + metadata = + (await this.samlService.fetchMetadataFromUrl(config.metadataUrl, config.ignoreSSL)) ?? ''; + } + } + + const result = await this.samlService.getLoginRequestUrl( + relayState, + config?.loginBinding, + metadata, + ); if (result?.binding === 'redirect') { return result.context.context; } else if (result?.binding === 'post') { diff --git a/packages/cli/src/modules/sso-saml/saml.service.ee.ts b/packages/cli/src/modules/sso-saml/saml.service.ee.ts index d2d5bbb2a10..37569e2e057 100644 --- a/packages/cli/src/modules/sso-saml/saml.service.ee.ts +++ b/packages/cli/src/modules/sso-saml/saml.service.ee.ts @@ -138,13 +138,18 @@ export class SamlService { if (this.samlify === undefined) { throw new UnexpectedError('Samlify is not initialized'); } + if (!this._samlPreferences.metadata) { + throw new InvalidSamlMetadataError( + 'No IdP metadata configured. Please provide valid identity provider metadata.', + ); + } if (this.identityProviderInstance === undefined || forceRecreate) { this.identityProviderInstance = this.samlify.IdentityProvider({ metadata: this._samlPreferences.metadata, }); } - this.validator.validateIdentiyProvider(this.identityProviderInstance); + this.validator.validateIdentityProvider(this.identityProviderInstance); return this.identityProviderInstance; } @@ -156,43 +161,44 @@ export class SamlService { return getServiceProviderInstance(this._samlPreferences, this.samlify); } + /** + * Generate a login request URL. + * When `metadata` is provided, creates a temporary IdP from it (for testing without saving). + * Otherwise uses the cached IdP from persisted preferences. + */ async getLoginRequestUrl( relayState?: string, binding?: SamlLoginBinding, + metadata?: string, ): Promise<{ binding: SamlLoginBinding; context: BindingContext | PostBindingContext; }> { await this.loadSamlify(); - if (binding === undefined) binding = this._samlPreferences.loginBinding ?? 'redirect'; - if (binding === 'post') { - return { - binding, - context: this.getPostLoginRequestUrl(relayState), - }; - } else { - return { - binding, - context: this.getRedirectLoginRequestUrl(relayState), - }; + if (this.samlify === undefined) { + throw new UnexpectedError('Samlify is not initialized'); } - } - private getRedirectLoginRequestUrl(relayState?: string): BindingContext { - const sp = this.getServiceProviderInstance(); - sp.entitySetting.relayState = relayState ?? this.urlService.getInstanceBaseUrl(); - const loginRequest = sp.createLoginRequest(this.getIdentityProviderInstance(), 'redirect'); - return loginRequest; - } + let idp: IdentityProviderInstance; + if (metadata) { + const validationResult = await this.validator.validateMetadata(metadata); + if (!validationResult) { + throw new InvalidSamlMetadataError(); + } + idp = this.samlify.IdentityProvider({ metadata }); + this.validator.validateIdentityProvider(idp); + } else { + idp = this.getIdentityProviderInstance(); + } - private getPostLoginRequestUrl(relayState?: string): PostBindingContext { + binding ??= this._samlPreferences.loginBinding ?? 'redirect'; const sp = this.getServiceProviderInstance(); sp.entitySetting.relayState = relayState ?? this.urlService.getInstanceBaseUrl(); - const loginRequest = sp.createLoginRequest( - this.getIdentityProviderInstance(), - 'post', - ) as PostBindingContext; - return loginRequest; + const loginRequest = sp.createLoginRequest(idp, binding); + return { + binding, + context: binding === 'post' ? (loginRequest as PostBindingContext) : loginRequest, + }; } async handleSamlLogin( @@ -446,23 +452,27 @@ export class SamlService { return; } - async fetchMetadataFromUrl(): Promise { + async fetchMetadataFromUrl( + metadataUrl?: string, + ignoreSSL?: boolean, + ): Promise { await this.loadSamlify(); - if (!this._samlPreferences.metadataUrl) - throw new BadRequestError('Error fetching SAML Metadata, no Metadata URL set'); + const url = metadataUrl ?? this._samlPreferences.metadataUrl; + const shouldIgnoreSSL = ignoreSSL ?? this._samlPreferences.ignoreSSL; + if (!url) throw new BadRequestError('Error fetching SAML Metadata, no Metadata URL set'); try { // Create a proxy-aware HTTPS agent that respects HTTP_PROXY, HTTPS_PROXY, and NO_PROXY // environment variables while also supporting SSL certificate validation options const httpsAgent = createHttpsProxyAgent( null, // Uses proxy from environment variables - this._samlPreferences.metadataUrl, + url, { - rejectUnauthorized: !this._samlPreferences.ignoreSSL, + rejectUnauthorized: !shouldIgnoreSSL, }, ); - const httpAgent = createHttpProxyAgent(null, this._samlPreferences.metadataUrl); + const httpAgent = createHttpProxyAgent(null, url); - const response = await axios.get(this._samlPreferences.metadataUrl, { + const response = await axios.get(url, { httpsAgent, httpAgent, }); @@ -470,16 +480,13 @@ export class SamlService { const xml = (await response.data) as string; const validationResult = await this.validator.validateMetadata(xml); if (!validationResult) { - throw new BadRequestError( - `Data received from ${this._samlPreferences.metadataUrl} is not valid SAML metadata.`, - ); + throw new BadRequestError(`Data received from ${url} is not valid SAML metadata.`); } return xml; } } catch (error) { - throw new BadRequestError( - `Error fetching SAML Metadata from ${this._samlPreferences.metadataUrl}: ${error}`, - ); + if (error instanceof BadRequestError) throw error; + throw new BadRequestError(`Error fetching SAML Metadata from ${url}: ${error}`); } return; } diff --git a/packages/cli/test/integration/saml/saml.api.test.ts b/packages/cli/test/integration/saml/saml.api.test.ts index f8b1639c090..75e5cd88ca3 100644 --- a/packages/cli/test/integration/saml/saml.api.test.ts +++ b/packages/cli/test/integration/saml/saml.api.test.ts @@ -201,8 +201,11 @@ describe('Check endpoint permissions', () => { await authOwnerAgent.get('/sso/saml/initsso').expect(200); }); - test('should be able to access GET /sso/saml/config/test', async () => { - await authOwnerAgent.get('/sso/saml/config/test').expect(200); + test('should be able to access POST /sso/saml/config/test', async () => { + await authOwnerAgent + .post('/sso/saml/config/test') + .send({ metadata: sampleConfig.metadata }) + .expect(200); }); }); @@ -241,8 +244,8 @@ describe('Check endpoint permissions', () => { await authMemberAgent.get('/sso/saml/initsso').expect(200); }); - test('should NOT be able to access GET /sso/saml/config/test', async () => { - await authMemberAgent.get('/sso/saml/config/test').expect(403); + test('should NOT be able to access POST /sso/saml/config/test', async () => { + await authMemberAgent.post('/sso/saml/config/test').expect(403); }); }); describe('Non-Authenticated User', () => { @@ -280,8 +283,8 @@ describe('Check endpoint permissions', () => { await testServer.authlessAgent.get('/sso/saml/initsso').expect(200); }); - test('should NOT be able to access GET /sso/saml/config/test', async () => { - await testServer.authlessAgent.get('/sso/saml/config/test').expect(401); + test('should NOT be able to access POST /sso/saml/config/test', async () => { + await testServer.authlessAgent.post('/sso/saml/config/test').expect(401); }); }); }); diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index b2217b3f32a..b4614281880 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -4082,6 +4082,10 @@ "settings.sso.settings.userRoleProvisioning.option.instanceAndProjectRoles.label": "Instance and project roles", "settings.sso.settings.test": "Test settings", "settings.sso.settings.save": "Save settings", + "settings.sso.settings.unsavedChanges.title": "Unsaved changes", + "settings.sso.settings.unsavedChanges.message": "You have unsaved changes. They'll be lost if you leave.", + "settings.sso.settings.unsavedChanges.saveAndLeave": "Save and leave", + "settings.sso.settings.unsavedChanges.leaveWithoutSaving": "Leave without saving", "settings.sso.settings.save.testConnection.title": "Test and activate SAML SSO", "settings.sso.settings.save.testConnection.message": "You are about to activate SSO via SAML. Test your SAML SSO settings first before proceeding.", "settings.sso.settings.save.testConnection.test": "Test settings", diff --git a/packages/frontend/@n8n/rest-api-client/src/api/sso.ts b/packages/frontend/@n8n/rest-api-client/src/api/sso.ts index 97291fc5ac1..5187a3d4dac 100644 --- a/packages/frontend/@n8n/rest-api-client/src/api/sso.ts +++ b/packages/frontend/@n8n/rest-api-client/src/api/sso.ts @@ -36,8 +36,11 @@ export const toggleSamlConfig = async ( return await makeRestApiRequest(context, 'POST', '/sso/saml/config/toggle', data); }; -export const testSamlConfig = async (context: IRestApiContext): Promise => { - return await makeRestApiRequest(context, 'GET', '/sso/saml/config/test'); +export const testSamlConfig = async ( + context: IRestApiContext, + data: Partial, +): Promise => { + return await makeRestApiRequest(context, 'POST', '/sso/saml/config/test', data); }; export const getOidcConfig = async (context: IRestApiContext): Promise => { diff --git a/packages/frontend/editor-ui/src/features/settings/sso/components/OidcSettingsForm.vue b/packages/frontend/editor-ui/src/features/settings/sso/components/OidcSettingsForm.vue index 29e028fc532..13e3602103f 100644 --- a/packages/frontend/editor-ui/src/features/settings/sso/components/OidcSettingsForm.vue +++ b/packages/frontend/editor-ui/src/features/settings/sso/components/OidcSettingsForm.vue @@ -183,6 +183,10 @@ function sendTrackingEvent(config: OidcConfigDto) { telemetry.track('User updated single sign on settings', trackingMetadata); } +const hasUnsavedChanges = computed(() => !cannotSaveOidcSettings.value && !savingForm.value); + +defineExpose({ hasUnsavedChanges, onSave: onOidcSettingsSave }); + onMounted(async () => { await loadOidcConfig(); }); diff --git a/packages/frontend/editor-ui/src/features/settings/sso/components/SamlSettingsForm.vue b/packages/frontend/editor-ui/src/features/settings/sso/components/SamlSettingsForm.vue index bdd3f4936db..496e7aa814d 100644 --- a/packages/frontend/editor-ui/src/features/settings/sso/components/SamlSettingsForm.vue +++ b/packages/frontend/editor-ui/src/features/settings/sso/components/SamlSettingsForm.vue @@ -241,7 +241,11 @@ const onSave = async (provisioningChangesConfirmed: boolean = false) => { const onTest = async () => { try { - const url = await ssoStore.testSamlConfig(); + const metaDataConfig: Partial = + ipsType.value === IdentityProviderSettingsType.URL + ? { metadataUrl: metadataUrl.value } + : { metadata: metadata.value }; + const url = await ssoStore.testSamlConfig(metaDataConfig); if (typeof window !== 'undefined') { window.open(url, '_blank'); @@ -270,6 +274,10 @@ const validateSamlInput = () => { } }; +const hasUnsavedChanges = computed(() => isSaveEnabled.value); + +defineExpose({ hasUnsavedChanges, onSave }); + onMounted(async () => { await loadSamlConfig(); }); diff --git a/packages/frontend/editor-ui/src/features/settings/sso/sso.store.ts b/packages/frontend/editor-ui/src/features/settings/sso/sso.store.ts index 0635ec72bb7..f849adc985c 100644 --- a/packages/frontend/editor-ui/src/features/settings/sso/sso.store.ts +++ b/packages/frontend/editor-ui/src/features/settings/sso/sso.store.ts @@ -110,7 +110,8 @@ export const useSSOStore = defineStore('sso', () => { const saveSamlConfig = async (config: Partial) => await ssoApi.saveSamlConfig(rootStore.restApiContext, config); - const testSamlConfig = async () => await ssoApi.testSamlConfig(rootStore.restApiContext); + const testSamlConfig = async (config: Partial) => + await ssoApi.testSamlConfig(rootStore.restApiContext, config); /** * OIDC diff --git a/packages/frontend/editor-ui/src/features/settings/sso/views/SettingsSso.vue b/packages/frontend/editor-ui/src/features/settings/sso/views/SettingsSso.vue index eea79ea4a6b..557734e825b 100644 --- a/packages/frontend/editor-ui/src/features/settings/sso/views/SettingsSso.vue +++ b/packages/frontend/editor-ui/src/features/settings/sso/views/SettingsSso.vue @@ -1,9 +1,12 @@