From cf8337aeb12bab92a5ca8d23a79092968eca3d2e Mon Sep 17 00:00:00 2001 From: Ali Elkhateeb Date: Mon, 20 Jul 2026 12:15:32 +0200 Subject: [PATCH] feat(API): Add endpoint to set the SAML SSO configuration (#34224) --- .../helpers/zod-object-keys-match.ts | 78 ++++ packages/@n8n/api-types/src/dto/index.ts | 1 + .../__tests__/saml-preferences.dto.test.ts | 342 +++++++++++------- .../saml/saml-configuration-response.dto.ts | 6 +- .../src/dto/saml/saml-preferences.dto.ts | 38 ++ .../security-settings/security-policy.dto.ts | 13 +- .../__tests__/saml.service.ee.test.ts | 40 ++ .../src/modules/sso-saml/saml.service.ee.ts | 38 +- packages/cli/src/public-api/types.ts | 2 + .../public-api-error-response.test.ts | 8 + .../spec/paths/security-policy.yml | 16 +- .../spec/schemas/security-policy.update.yml | 21 +- .../sso-saml/spec/paths/settings.sso.saml.yml | 38 ++ .../schemas/saml-configuration.update.yml | 151 ++++++++ .../spec/schemas/saml-configuration.yml | 32 +- .../v1/handlers/sso-saml/sso-saml.handler.ts | 30 +- .../v1/handlers/sso-saml/sso-saml.mapper.ts | 64 +++- .../public-api/security-policy.test.ts | 61 +++- .../integration/public-api/sso-saml.test.ts | 198 ++++++++++ .../test/integration/saml/sample-metadata.ts | 3 + .../nodes/N8n/n8n-api-coverage.json | 3 + 21 files changed, 1008 insertions(+), 175 deletions(-) create mode 100644 packages/@n8n/api-types/src/__tests__/helpers/zod-object-keys-match.ts create mode 100644 packages/cli/src/public-api/v1/handlers/sso-saml/spec/schemas/saml-configuration.update.yml diff --git a/packages/@n8n/api-types/src/__tests__/helpers/zod-object-keys-match.ts b/packages/@n8n/api-types/src/__tests__/helpers/zod-object-keys-match.ts new file mode 100644 index 00000000000..338d7b6f8a8 --- /dev/null +++ b/packages/@n8n/api-types/src/__tests__/helpers/zod-object-keys-match.ts @@ -0,0 +1,78 @@ +import { isDeepStrictEqual } from 'node:util'; +import type { ZodArray, ZodDefault, ZodObject, ZodOptional, ZodRawShape, ZodTypeAny } from 'zod'; +import { z } from 'zod'; + +type ZodObjectKeyTree = { + [key: string]: ZodObjectKeyTree | true; +}; + +const isZodOptional = (schema: ZodTypeAny): schema is ZodOptional => { + return schema instanceof z.ZodOptional; +}; + +const isZodDefault = (schema: ZodTypeAny): schema is ZodDefault => { + return schema instanceof z.ZodDefault; +}; + +const isZodObject = (schema: ZodTypeAny): schema is ZodObject => { + return schema instanceof z.ZodObject; +}; + +const isZodArray = (schema: ZodTypeAny): schema is ZodArray => { + return schema instanceof z.ZodArray; +}; + +const unwrapOptionalOrDefault = (schema: ZodTypeAny): ZodTypeAny => { + if (isZodOptional(schema) || isZodDefault(schema)) { + return unwrapOptionalOrDefault(schema._def.innerType); + } + return schema; +}; + +/** Nested ZodObject key tree; non-objects become `true` leaf markers. */ +const zodObjectKeyTree = (schema: ZodTypeAny): ZodObjectKeyTree | true => { + const current = unwrapOptionalOrDefault(schema); + if (!isZodObject(current)) { + return true; + } + + const tree: ZodObjectKeyTree = {}; + const shape: ZodRawShape = current.shape; + for (const key of Object.keys(shape)) { + const fieldSchema = shape[key]; + if (fieldSchema === undefined) { + continue; + } + tree[key] = zodObjectKeyTree(fieldSchema); + } + return tree; +}; + +/** + * Whether two Zod schemas have the same nested object keys after stripping + * `.optional()` / `.default()` wrappers. Useful to keep a full Public API PUT + * body aligned with the internal preferences DTO. + */ +export const zodObjectKeysMatch = (a: ZodTypeAny, b: ZodTypeAny): boolean => { + return isDeepStrictEqual(zodObjectKeyTree(a), zodObjectKeyTree(b)); +}; + +/** Whether a Zod object (and nested objects/array items) has no `.optional()` / `.default()` fields. */ +export const zodObjectFieldsAreAllRequired = (schema: ZodTypeAny): boolean => { + if (isZodOptional(schema) || isZodDefault(schema)) { + return false; + } + if (isZodArray(schema)) { + return zodObjectFieldsAreAllRequired(schema.element); + } + if (!isZodObject(schema)) { + return true; + } + + for (const fieldSchema of Object.values(schema.shape)) { + if (fieldSchema !== undefined && !zodObjectFieldsAreAllRequired(fieldSchema)) { + return false; + } + } + return true; +}; diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index a3a603333ea..161bd0be672 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -75,6 +75,7 @@ export { ListProjectsQueryDto } from './project/list-projects-query.dto'; export { SamlAcsDto } from './saml/saml-acs.dto'; export { SamlPreferences } from './saml/saml-preferences.dto'; export { SamlPreferencesAttributeMapping } from './saml/saml-preferences.dto'; +export { UpdateSamlConfigurationDto } from './saml/saml-preferences.dto'; export { SamlToggleDto } from './saml/saml-toggle.dto'; export { type SamlConfigurationResponse } from './saml/saml-configuration-response.dto'; diff --git a/packages/@n8n/api-types/src/dto/saml/__tests__/saml-preferences.dto.test.ts b/packages/@n8n/api-types/src/dto/saml/__tests__/saml-preferences.dto.test.ts index f6171efc54b..76765ef0e1d 100644 --- a/packages/@n8n/api-types/src/dto/saml/__tests__/saml-preferences.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/saml/__tests__/saml-preferences.dto.test.ts @@ -1,157 +1,237 @@ -import { SamlPreferences } from '../saml-preferences.dto'; +import { + zodObjectFieldsAreAllRequired, + zodObjectKeysMatch, +} from '../../../__tests__/helpers/zod-object-keys-match'; +import { SamlPreferences, UpdateSamlConfigurationDto } from '../saml-preferences.dto'; -describe('SamlPreferences', () => { - describe('Valid requests', () => { - test.each([ - { - name: 'valid minimal configuration', - request: { - mapping: { - email: 'user@example.com', - firstName: 'John', - lastName: 'Doe', - userPrincipalName: 'johndoe', - n8nInstanceRole: 'n8n_instance_role', +describe('SAML preference DTOs', () => { + it('keeps UpdateSamlConfigurationDto and SamlPreferences object keys in sync', () => { + expect(zodObjectKeysMatch(UpdateSamlConfigurationDto.schema, SamlPreferences.schema)).toBe( + true, + ); + }); + + describe('SamlPreferences', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'valid minimal configuration', + request: { + mapping: { + email: 'user@example.com', + firstName: 'John', + lastName: 'Doe', + userPrincipalName: 'johndoe', + n8nInstanceRole: 'n8n_instance_role', + }, + metadata: 'metadata', + metadataUrl: 'https://example.com/metadata', + loginEnabled: true, + loginLabel: 'Login with SAML', }, - metadata: 'metadata', - metadataUrl: 'https://example.com/metadata', - loginEnabled: true, - loginLabel: 'Login with SAML', }, - }, - { - name: 'valid full configuration', - request: { - mapping: { - email: 'user@example.com', - firstName: 'John', - lastName: 'Doe', - userPrincipalName: 'johndoe', - n8nInstanceRole: 'n8n_instance_role', + { + name: 'valid full configuration', + request: { + mapping: { + email: 'user@example.com', + firstName: 'John', + lastName: 'Doe', + userPrincipalName: 'johndoe', + n8nInstanceRole: 'n8n_instance_role', + }, + metadata: 'metadata', + metadataUrl: 'https://example.com/metadata', + ignoreSSL: true, + loginBinding: 'post', + loginEnabled: true, + loginLabel: 'Login with SAML', + authnRequestsSigned: true, + wantAssertionsSigned: true, + wantMessageSigned: true, + acsBinding: 'redirect', + signatureConfig: { + prefix: 'ds', + location: { + reference: '/samlp:Response/saml:Issuer', + action: 'after', + }, + }, + relayState: 'https://example.com/relay', }, - metadata: 'metadata', - metadataUrl: 'https://example.com/metadata', - ignoreSSL: true, - loginBinding: 'post', - loginEnabled: true, - loginLabel: 'Login with SAML', - authnRequestsSigned: true, - wantAssertionsSigned: true, - wantMessageSigned: true, - acsBinding: 'redirect', - signatureConfig: { + }, + ])('should validate $name', ({ request }) => { + const result = SamlPreferences.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'invalid loginBinding', + request: { + loginBinding: 'invalid', + }, + expectedErrorPath: ['loginBinding'], + }, + { + name: 'invalid acsBinding', + request: { + acsBinding: 'invalid', + }, + expectedErrorPath: ['acsBinding'], + }, + { + name: 'invalid signatureConfig location action', + request: { + signatureConfig: { + prefix: 'ds', + location: { + reference: '/samlp:Response/saml:Issuer', + action: 'invalid', + }, + }, + }, + expectedErrorPath: ['signatureConfig', 'location', 'action'], + }, + { + name: 'missing signatureConfig location reference', + request: { + signatureConfig: { + prefix: 'ds', + location: { + action: 'after', + }, + }, + }, + expectedErrorPath: ['signatureConfig', 'location', 'reference'], + }, + { + name: 'invalid mapping email', + request: { + mapping: { + email: 123, + firstName: 'John', + lastName: 'Doe', + userPrincipalName: 'johndoe', + }, + }, + expectedErrorPath: ['mapping', 'email'], + }, + ])('should fail validation for $name', ({ request, expectedErrorPath }) => { + const result = SamlPreferences.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + + describe('Edge cases', () => { + test('should handle optional fields correctly', () => { + const validRequest = { + mapping: undefined, + metadata: undefined, + metadataUrl: undefined, + loginEnabled: undefined, + loginLabel: undefined, + }; + + const result = SamlPreferences.safeParse(validRequest); + expect(result.success).toBe(true); + }); + + test('should handle default values correctly', () => { + const validRequest = {}; + + const result = SamlPreferences.safeParse(validRequest); + expect(result.success).toBe(true); + expect(result.data?.ignoreSSL).toBe(false); + expect(result.data?.loginBinding).toBe('redirect'); + expect(result.data?.authnRequestsSigned).toBe(false); + expect(result.data?.wantAssertionsSigned).toBe(true); + expect(result.data?.wantMessageSigned).toBe(true); + expect(result.data?.acsBinding).toBe('post'); + expect(result.data?.signatureConfig).toEqual({ prefix: 'ds', location: { reference: '/samlp:Response/saml:Issuer', action: 'after', }, - }, - relayState: 'https://example.com/relay', - }, - }, - ])('should validate $name', ({ request }) => { - const result = SamlPreferences.safeParse(request); - expect(result.success).toBe(true); + }); + expect(result.data?.relayState).toBe(''); + }); + }); }); }); - describe('Invalid requests', () => { - test.each([ - { - name: 'invalid loginBinding', - request: { - loginBinding: 'invalid', - }, - expectedErrorPath: ['loginBinding'], + describe('UpdateSamlConfigurationDto', () => { + const fullBody = { + mapping: { + email: 'user@example.com', + firstName: 'John', + lastName: 'Doe', + userPrincipalName: 'johndoe', + n8nInstanceRole: '', + n8nProjectRoles: [] as string[], }, - { - name: 'invalid acsBinding', - request: { - acsBinding: 'invalid', + metadata: '', + metadataUrl: '', + ignoreSSL: false, + loginBinding: 'redirect' as const, + loginEnabled: false, + loginLabel: 'SAML', + authnRequestsSigned: false, + wantAssertionsSigned: true, + wantMessageSigned: true, + signingPrivateKey: '', + signingCertificate: '', + acsBinding: 'post' as const, + signatureConfig: { + prefix: 'custom', + location: { + reference: '/samlp:Response/saml:Issuer', + action: 'after' as const, }, - expectedErrorPath: ['acsBinding'], }, - { - name: 'invalid signatureConfig location action', - request: { - signatureConfig: { - prefix: 'ds', - location: { - reference: '/samlp:Response/saml:Issuer', - action: 'invalid', - }, - }, - }, - expectedErrorPath: ['signatureConfig', 'location', 'action'], - }, - { - name: 'missing signatureConfig location reference', - request: { - signatureConfig: { - prefix: 'ds', - location: { - action: 'after', - }, - }, - }, - expectedErrorPath: ['signatureConfig', 'location', 'reference'], - }, - { - name: 'invalid mapping email', - request: { - mapping: { - email: 123, - firstName: 'John', - lastName: 'Doe', - userPrincipalName: 'johndoe', - }, - }, - expectedErrorPath: ['mapping', 'email'], - }, - ])('should fail validation for $name', ({ request, expectedErrorPath }) => { - const result = SamlPreferences.safeParse(request); + relayState: '', + }; + it('rejects an empty body with an error for every top-level field - Guards against .optional() / .default() on PUT fields', () => { + const result = UpdateSamlConfigurationDto.safeParse({}); expect(result.success).toBe(false); - if (expectedErrorPath) { - expect(result.error?.issues[0].path).toEqual(expectedErrorPath); - } + const erroredFields = new Set( + result.error?.issues.map((issue) => String(issue.path[0])) ?? [], + ); + const requiredFields = Object.keys(UpdateSamlConfigurationDto.schema.shape).sort(); + + expect([...erroredFields].sort()).toEqual(requiredFields); }); - describe('Edge cases', () => { - test('should handle optional fields correctly', () => { - const validRequest = { - mapping: undefined, - metadata: undefined, - metadataUrl: undefined, - loginEnabled: undefined, - loginLabel: undefined, - }; + it('requires every nested field with no optional or default', () => { + expect(zodObjectFieldsAreAllRequired(UpdateSamlConfigurationDto.schema)).toBe(true); + }); - const result = SamlPreferences.safeParse(validRequest); - expect(result.success).toBe(true); + it('accepts a complete signatureConfig', () => { + const result = UpdateSamlConfigurationDto.safeParse(fullBody); + expect(result.success).toBe(true); + expect(result.data?.signatureConfig.prefix).toBe('custom'); + }); + + it('rejects signatureConfig when prefix is omitted instead of defaulting to ds', () => { + const { prefix: _prefix, ...locationOnly } = fullBody.signatureConfig; + const result = UpdateSamlConfigurationDto.safeParse({ + ...fullBody, + signatureConfig: { + location: locationOnly.location, + }, }); - test('should handle default values correctly', () => { - const validRequest = {}; - - const result = SamlPreferences.safeParse(validRequest); - expect(result.success).toBe(true); - expect(result.data?.ignoreSSL).toBe(false); - expect(result.data?.loginBinding).toBe('redirect'); - expect(result.data?.authnRequestsSigned).toBe(false); - expect(result.data?.wantAssertionsSigned).toBe(true); - expect(result.data?.wantMessageSigned).toBe(true); - expect(result.data?.acsBinding).toBe('post'); - expect(result.data?.signatureConfig).toEqual({ - prefix: 'ds', - location: { - reference: '/samlp:Response/saml:Issuer', - action: 'after', - }, - }); - expect(result.data?.relayState).toBe(''); - }); + expect(result.success).toBe(false); + expect(result.error?.issues[0].path).toEqual(['signatureConfig', 'prefix']); }); }); }); diff --git a/packages/@n8n/api-types/src/dto/saml/saml-configuration-response.dto.ts b/packages/@n8n/api-types/src/dto/saml/saml-configuration-response.dto.ts index d3f5cb84e19..9568299ad4f 100644 --- a/packages/@n8n/api-types/src/dto/saml/saml-configuration-response.dto.ts +++ b/packages/@n8n/api-types/src/dto/saml/saml-configuration-response.dto.ts @@ -1,9 +1,11 @@ -import type { SamlPreferences } from './saml-preferences.dto'; +import type { UpdateSamlConfigurationDto } from './saml-preferences.dto'; /** * Public API response shape for the SAML SSO configuration group. + * Derived from the PUT body plus read-only service-provider fields so a GET + * response can be sent back as a PUT body without drifting out of sync. */ -export type SamlConfigurationResponse = SamlPreferences & { +export type SamlConfigurationResponse = UpdateSamlConfigurationDto & { entityID: string; returnUrl: string; }; diff --git a/packages/@n8n/api-types/src/dto/saml/saml-preferences.dto.ts b/packages/@n8n/api-types/src/dto/saml/saml-preferences.dto.ts index a4b8d16fb0e..2535aa0b27d 100644 --- a/packages/@n8n/api-types/src/dto/saml/saml-preferences.dto.ts +++ b/packages/@n8n/api-types/src/dto/saml/saml-preferences.dto.ts @@ -13,6 +13,15 @@ const SignatureConfigSchema = z.object({ }), }); +/** Same shape without defaults — required for full-object Public API PUTs. */ +const SignatureConfigRequiredSchema = z.object({ + prefix: z.string(), + location: z.object({ + reference: z.string(), + action: z.enum(['before', 'after', 'prepend', 'append']), + }), +}); + export class SamlPreferencesAttributeMapping extends Z.class({ /** SAML attribute mapped to the user's email. */ email: z.string(), @@ -62,3 +71,32 @@ export class SamlPreferences extends Z.class({ relayState: z.string().default(''), }) {} + +/** + * Public API PUT body for SAML configuration. Clients must send every writable + * field; use empty strings / empty arrays when a value is unset. + */ +export class UpdateSamlConfigurationDto extends Z.class({ + mapping: z.object({ + email: z.string(), + firstName: z.string(), + lastName: z.string(), + userPrincipalName: z.string(), + n8nInstanceRole: z.string(), + n8nProjectRoles: z.array(z.string()), + }), + metadata: z.string(), + metadataUrl: z.string(), + ignoreSSL: z.boolean(), + loginBinding: SamlLoginBindingSchema, + loginEnabled: z.boolean(), + loginLabel: z.string(), + authnRequestsSigned: z.boolean(), + wantAssertionsSigned: z.boolean(), + wantMessageSigned: z.boolean(), + signingPrivateKey: z.string(), + signingCertificate: z.string(), + acsBinding: SamlLoginBindingSchema, + signatureConfig: SignatureConfigRequiredSchema, + relayState: z.string(), +}) {} diff --git a/packages/@n8n/api-types/src/dto/security-settings/security-policy.dto.ts b/packages/@n8n/api-types/src/dto/security-settings/security-policy.dto.ts index b7dfbf41577..d2f7333910f 100644 --- a/packages/@n8n/api-types/src/dto/security-settings/security-policy.dto.ts +++ b/packages/@n8n/api-types/src/dto/security-settings/security-policy.dto.ts @@ -8,14 +8,15 @@ const redactionEnforcementFieldSchema = z.object({ }); /** - * Public API request body for updating the security policy group. Mirrors the - * writable subset of the internal security settings, validated with the same - * field schemas so behaviour stays in sync. + * Public API PUT body for the security policy group. Clients must send the full + * writable configuration; partial updates are rejected. Mirrors the writable + * subset of the internal security settings, validated with the same field + * schemas so behaviour stays in sync. */ export class UpdateSecurityPolicyDto extends Z.class({ - personalSpacePublishing: z.boolean().optional(), - personalSpaceSharing: z.boolean().optional(), - redactionEnforcement: redactionEnforcementFieldSchema.optional(), + personalSpacePublishing: z.boolean(), + personalSpaceSharing: z.boolean(), + redactionEnforcement: redactionEnforcementFieldSchema, }) {} /** 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 ffed3803b3a..21e7e104230 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 @@ -809,6 +809,46 @@ describe('SamlService', () => { expect(samlService.samlPreferences.metadataUrl).toBe(metadataUrlTestData); }); + test('clears metadata and metadataUrl when set to empty strings', async () => { + await samlService.loadPreferencesWithoutValidation({ + metadata: 'existing', + metadataUrl: 'https://idp.example.com/metadata', + }); + + await samlService.loadPreferencesWithoutValidation({ + metadata: '', + metadataUrl: '', + }); + + expect(samlService.samlPreferences.metadata).toBe(''); + expect(samlService.samlPreferences.metadataUrl).toBeUndefined(); + }); + + test('clears metadataUrl when empty string is provided alone', async () => { + await samlService.loadPreferencesWithoutValidation({ + metadataUrl: 'https://idp.example.com/metadata', + }); + + await samlService.loadPreferencesWithoutValidation({ + metadataUrl: '', + }); + + expect(samlService.samlPreferences.metadataUrl).toBeUndefined(); + }); + + test('clears metadataUrl when XML metadata is provided without a URL', async () => { + await samlService.loadPreferencesWithoutValidation({ + metadataUrl: 'https://idp.example.com/metadata', + }); + + await samlService.loadPreferencesWithoutValidation({ + metadata: 'new', + }); + + expect(samlService.samlPreferences.metadata).toBe('new'); + expect(samlService.samlPreferences.metadataUrl).toBeUndefined(); + }); + test('does throw `InvalidSamlMetadataError` in case saml login is turned on and no valid metadata is available', async () => { await samlService.loadPreferencesWithoutValidation({ metadata: 'not valid data', 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 3e75433d225..54a1661aa36 100644 --- a/packages/cli/src/modules/sso-saml/saml.service.ee.ts +++ b/packages/cli/src/modules/sso-saml/saml.service.ee.ts @@ -564,12 +564,23 @@ export class SamlService { throw new InvalidSamlMetadataError(); } } - this.getIdentityProviderInstance(true); + if (this._samlPreferences.metadata) { + this.getIdentityProviderInstance(true); + } else { + // Metadata was cleared — drop the cached IdP so a later configure can recreate it. + this.identityProviderInstance = undefined; + } } async loadPreferencesWithoutValidation(prefs: Partial) { + await this.applySamlPreferenceFields(prefs); + this.applyIdpMetadataPreferences(prefs); + await setSamlLoginEnabled(prefs.loginEnabled ?? isSamlLoginEnabled()); + setSamlLoginLabel(prefs.loginLabel ?? getSamlLoginLabel()); + } + + private async applySamlPreferenceFields(prefs: Partial) { this._samlPreferences.loginBinding = prefs.loginBinding ?? this._samlPreferences.loginBinding; - this._samlPreferences.metadata = prefs.metadata ?? this._samlPreferences.metadata; this._samlPreferences.mapping = prefs.mapping ?? this._samlPreferences.mapping; this._samlPreferences.ignoreSSL = prefs.ignoreSSL ?? this._samlPreferences.ignoreSSL; this._samlPreferences.acsBinding = prefs.acsBinding ?? this._samlPreferences.acsBinding; @@ -581,12 +592,14 @@ export class SamlService { prefs.wantAssertionsSigned ?? this._samlPreferences.wantAssertionsSigned; this._samlPreferences.wantMessageSigned = prefs.wantMessageSigned ?? this._samlPreferences.wantMessageSigned; + if (prefs.signingCertificate === '') { this._samlPreferences.signingCertificate = undefined; } else { this._samlPreferences.signingCertificate = prefs.signingCertificate ?? this._samlPreferences.signingCertificate; } + if ( prefs.signingPrivateKey !== undefined && prefs.signingPrivateKey !== CREDENTIAL_BLANKING_VALUE @@ -604,15 +617,22 @@ export class SamlService { this._samlPreferences.signingPrivateKey = prefs.signingPrivateKey; } } - if (prefs.metadataUrl) { - this._samlPreferences.metadataUrl = prefs.metadataUrl; - } else if (prefs.metadata) { - // remove metadataUrl if metadata is set directly - this._samlPreferences.metadataUrl = undefined; + } + + /** + * Apply IdP metadata sources. `undefined` leaves the field unchanged; `''` clears it + * (PUT replacement). Providing XML without a URL also clears the URL alternate. + */ + private applyIdpMetadataPreferences(prefs: Partial) { + if (prefs.metadata !== undefined) { this._samlPreferences.metadata = prefs.metadata; } - await setSamlLoginEnabled(prefs.loginEnabled ?? isSamlLoginEnabled()); - setSamlLoginLabel(prefs.loginLabel ?? getSamlLoginLabel()); + if (prefs.metadataUrl !== undefined) { + this._samlPreferences.metadataUrl = prefs.metadataUrl || undefined; + } + if (prefs.metadata && !prefs.metadataUrl) { + this._samlPreferences.metadataUrl = undefined; + } } async loadFromDbAndApplySamlPreferences( diff --git a/packages/cli/src/public-api/types.ts b/packages/cli/src/public-api/types.ts index c9b9cd26663..05e11a377d2 100644 --- a/packages/cli/src/public-api/types.ts +++ b/packages/cli/src/public-api/types.ts @@ -9,6 +9,7 @@ import type { UpsertDataTableRowDto, UpdateSecurityPolicyDto, PublicCreateDestination, + UpdateSamlConfigurationDto, } from '@n8n/api-types'; import type { AuthenticatedRequest, TagEntity, WorkflowEntity } from '@n8n/db'; import type { ExecutionStatus, ICredentialDataDecryptedObject } from 'n8n-workflow'; @@ -418,4 +419,5 @@ export declare namespace LogStreamingRequest { export declare namespace SsoSamlRequest { type Get = AuthenticatedRequest; + type Update = AuthenticatedRequest<{}, {}, UpdateSamlConfigurationDto>; } diff --git a/packages/cli/src/public-api/v1/__tests__/public-api-error-response.test.ts b/packages/cli/src/public-api/v1/__tests__/public-api-error-response.test.ts index 441c19e0b92..d011ebd4e7d 100644 --- a/packages/cli/src/public-api/v1/__tests__/public-api-error-response.test.ts +++ b/packages/cli/src/public-api/v1/__tests__/public-api-error-response.test.ts @@ -3,6 +3,7 @@ import { BadRequest } from 'express-openapi-validator/dist/framework/types'; import { UnexpectedError, UserError, OperationalError } from 'n8n-workflow'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { ConflictError } from '@/errors/response-errors/conflict.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { sendPublicApiErrorResponse } from '../public-api-error-response'; @@ -40,6 +41,13 @@ describe('sendPublicApiErrorResponse', () => { expect(res._payload.body).toEqual({ message: 'invalid' }); }); + it('maps ConflictError to 409', () => { + const res = createMockRes(); + sendPublicApiErrorResponse(res, new ConflictError('managed declaratively')); + expect(res._payload.statusCode).toBe(409); + expect(res._payload.body).toEqual({ message: 'managed declaratively' }); + }); + it('maps UserError to 400', () => { const res = createMockRes(); sendPublicApiErrorResponse(res, new UserError('bad input')); diff --git a/packages/cli/src/public-api/v1/handlers/security-policy/spec/paths/security-policy.yml b/packages/cli/src/public-api/v1/handlers/security-policy/spec/paths/security-policy.yml index 9d13beea138..881c204692c 100644 --- a/packages/cli/src/public-api/v1/handlers/security-policy/spec/paths/security-policy.yml +++ b/packages/cli/src/public-api/v1/handlers/security-policy/spec/paths/security-policy.yml @@ -26,15 +26,17 @@ put: x-eov-operation-handler: v1/handlers/security-policy/security-policy.handler tags: - SecurityPolicy - summary: Update the security policy + summary: Set the security policy description: > - Update the instance security policy. Only the provided fields are changed and the update - takes effect exactly as it would from the UI, using the same validation. Requires the - `securitySettings:manage` scope and the Personal Space Policy feature to be licensed. - When the group is managed via environment variables, the write is rejected with 409 and no - changes are made; a read still returns the current values. + Replace the instance security policy with the provided full object. Every writable + field must be sent. Read-only usage counts from GET are ignored if included, so a GET + response can be sent back as a PUT body. The update takes effect exactly as it would + from the UI, using the same validation. Requires the `securitySettings:manage` scope + and the Personal Space Policy feature to be licensed. When the group is managed via + environment variables, the write is rejected with 409 and no changes are made; a read + still returns the current values. requestBody: - description: The security policy fields to update. + description: The full security policy to set. required: true content: application/json: diff --git a/packages/cli/src/public-api/v1/handlers/security-policy/spec/schemas/security-policy.update.yml b/packages/cli/src/public-api/v1/handlers/security-policy/spec/schemas/security-policy.update.yml index 9577d900761..0560ad2cca8 100644 --- a/packages/cli/src/public-api/v1/handlers/security-policy/spec/schemas/security-policy.update.yml +++ b/packages/cli/src/public-api/v1/handlers/security-policy/spec/schemas/security-policy.update.yml @@ -1,6 +1,10 @@ type: object additionalProperties: false -description: Security policy fields to update. Omitted fields are left unchanged. +description: Full security policy. All writable fields must be provided; partial updates are not supported. +required: + - personalSpacePublishing + - personalSpaceSharing + - redactionEnforcement properties: personalSpacePublishing: type: boolean @@ -21,3 +25,18 @@ properties: enum: [off, production, all] description: Minimum execution-data redaction level enforced across the instance. example: production + publishedPersonalWorkflowsCount: + type: integer + description: > + Read-only usage count returned by GET. Ignored on write so a GET response can be + sent back as a PUT body. + sharedPersonalWorkflowsCount: + type: integer + description: > + Read-only usage count returned by GET. Ignored on write so a GET response can be + sent back as a PUT body. + sharedPersonalCredentialsCount: + type: integer + description: > + Read-only usage count returned by GET. Ignored on write so a GET response can be + sent back as a PUT body. diff --git a/packages/cli/src/public-api/v1/handlers/sso-saml/spec/paths/settings.sso.saml.yml b/packages/cli/src/public-api/v1/handlers/sso-saml/spec/paths/settings.sso.saml.yml index 84a07e613c0..45abd811431 100644 --- a/packages/cli/src/public-api/v1/handlers/sso-saml/spec/paths/settings.sso.saml.yml +++ b/packages/cli/src/public-api/v1/handlers/sso-saml/spec/paths/settings.sso.saml.yml @@ -21,3 +21,41 @@ get: $ref: '../../../../shared/spec/responses/unauthorized.yml' '403': $ref: '../../../../shared/spec/responses/forbidden.yml' +put: + x-eov-operation-id: updateSamlConfiguration + x-required-scope: saml:manage + x-eov-operation-handler: v1/handlers/sso-saml/sso-saml.handler + tags: + - SettingsSsoSaml + summary: Set the SAML SSO configuration + description: > + Replace the SAML SSO configuration with the provided full object. Every writable field + must be sent; use empty strings or empty arrays when a value is unset. Read-only + `entityID` / `returnUrl` from GET are ignored if included, so a GET response can be + sent back as a PUT body. Redacted secret placeholders keep the stored values unchanged. + The update takes effect exactly as it would from the UI, using the same validation. + Requires the `saml:manage` scope and the SAML feature to be licensed. When the + configuration is managed via environment variables, the write is rejected with 409 and no + changes are made. + requestBody: + description: The full SAML SSO configuration to set. + required: true + content: + application/json: + schema: + $ref: '../schemas/saml-configuration.update.yml' + responses: + '200': + description: Operation successful. + content: + application/json: + schema: + $ref: '../schemas/saml-configuration.yml' + '400': + $ref: '../../../../shared/spec/responses/badRequest.yml' + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' + '403': + $ref: '../../../../shared/spec/responses/forbidden.yml' + '409': + $ref: '../../../../shared/spec/responses/conflict.yml' diff --git a/packages/cli/src/public-api/v1/handlers/sso-saml/spec/schemas/saml-configuration.update.yml b/packages/cli/src/public-api/v1/handlers/sso-saml/spec/schemas/saml-configuration.update.yml new file mode 100644 index 00000000000..7f17d37f307 --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/sso-saml/spec/schemas/saml-configuration.update.yml @@ -0,0 +1,151 @@ +type: object +additionalProperties: false +description: > + Full SAML SSO configuration. Every field must be provided; use empty strings or + empty arrays when a value is unset. Partial updates are not supported. +required: + - mapping + - metadata + - metadataUrl + - ignoreSSL + - loginBinding + - loginEnabled + - loginLabel + - authnRequestsSigned + - wantAssertionsSigned + - wantMessageSigned + - signingPrivateKey + - signingCertificate + - acsBinding + - signatureConfig + - relayState +properties: + mapping: + type: object + description: Mapping of SAML attributes to n8n user fields. Use empty strings / empty arrays for unused attributes. + additionalProperties: false + required: + - email + - firstName + - lastName + - userPrincipalName + - n8nInstanceRole + - n8nProjectRoles + properties: + email: + type: string + description: SAML attribute mapped to the user's email. + firstName: + type: string + description: SAML attribute mapped to the user's first name. + lastName: + type: string + description: SAML attribute mapped to the user's last name. + userPrincipalName: + type: string + description: SAML attribute mapped to the user's principal name. + n8nInstanceRole: + type: string + description: SAML attribute mapped to the n8n instance role. Use an empty string when unused. + n8nProjectRoles: + type: array + items: + type: string + description: > + SAML attributes mapped to n8n project roles, formatted as `:`. + Use an empty array when unused. + metadata: + type: string + description: > + Identity provider metadata in XML format. Use an empty string to clear stored metadata + (also clears metadataUrl when no URL is provided). Use the redaction placeholder from a + prior GET to leave an existing value unchanged. + metadataUrl: + type: string + description: > + URL to fetch identity provider metadata from. Use an empty string to clear a stored URL. + ignoreSSL: + type: boolean + description: Whether to ignore SSL certificate errors when fetching metadata from a URL. + example: false + loginBinding: + type: string + enum: [redirect, post] + description: SAML login request binding. + example: redirect + loginEnabled: + type: boolean + description: Whether SAML login is enabled. + example: false + loginLabel: + type: string + description: Label shown on the SAML login button. + example: SAML + authnRequestsSigned: + type: boolean + description: Whether authentication requests are signed. + example: false + wantAssertionsSigned: + type: boolean + description: Whether signed assertions are required. + example: true + wantMessageSigned: + type: boolean + description: Whether signed SAML messages are required. + example: true + signingPrivateKey: + type: string + description: > + PEM-encoded private key for signing SAML AuthnRequests. Use an empty string to clear + an existing key, or the redaction placeholder from a prior GET to leave it unchanged. + signingCertificate: + type: string + description: > + PEM-encoded certificate containing the public key matching the signing private key. + Use an empty string when unused or to clear an existing certificate. + acsBinding: + type: string + enum: [redirect, post] + description: Assertion Consumer Service binding. + example: post + signatureConfig: + type: object + description: Configuration for the signature in SAML requests and responses. + additionalProperties: false + required: + - prefix + - location + properties: + prefix: + type: string + example: ds + location: + type: object + additionalProperties: false + required: + - reference + - action + properties: + reference: + type: string + example: /samlp:Response/saml:Issuer + action: + type: string + enum: [before, after, prepend, append] + example: after + relayState: + type: string + description: Default relay state value for SAML requests. Use an empty string when unused. + example: https://n8n.example.com + entityID: + type: string + description: > + Service provider entity ID. Returned by GET for convenience; ignored on write so a GET + response can be sent back as a PUT body. + example: https://n8n.example.com/rest/sso/saml/metadata + returnUrl: + type: string + description: > + Assertion Consumer Service return URL. Returned by GET for convenience; ignored on write + so a GET response can be sent back as a PUT body. + example: https://n8n.example.com/rest/sso/saml/acs diff --git a/packages/cli/src/public-api/v1/handlers/sso-saml/spec/schemas/saml-configuration.yml b/packages/cli/src/public-api/v1/handlers/sso-saml/spec/schemas/saml-configuration.yml index 4253bc8ec2e..9e2514596e9 100644 --- a/packages/cli/src/public-api/v1/handlers/sso-saml/spec/schemas/saml-configuration.yml +++ b/packages/cli/src/public-api/v1/handlers/sso-saml/spec/schemas/saml-configuration.yml @@ -3,12 +3,19 @@ additionalProperties: false required: - entityID - returnUrl + - mapping + - metadata + - metadataUrl - ignoreSSL - loginBinding - - acsBinding + - loginEnabled + - loginLabel - authnRequestsSigned - wantAssertionsSigned - wantMessageSigned + - signingPrivateKey + - signingCertificate + - acsBinding - signatureConfig - relayState properties: @@ -26,6 +33,13 @@ properties: type: object description: Mapping of SAML attributes to n8n user fields. additionalProperties: false + required: + - email + - firstName + - lastName + - userPrincipalName + - n8nInstanceRole + - n8nProjectRoles properties: email: type: string @@ -51,11 +65,11 @@ properties: type: string description: > Identity provider metadata in XML format. Redacted on read when set because it - contains IdP certificates; never echoed back in plaintext. + contains IdP certificates; never echoed back in plaintext. Use an empty string when unset. example: '**hidden**' metadataUrl: type: string - description: URL to fetch identity provider metadata from. + description: URL to fetch identity provider metadata from. Use an empty string when unset. ignoreSSL: type: boolean description: Whether to ignore SSL certificate errors when fetching metadata from a URL. @@ -89,13 +103,13 @@ properties: type: string description: > PEM-encoded private key for signing SAML AuthnRequests. Redacted on read when set; - never echoed back in plaintext. + never echoed back in plaintext. Use an empty string when unset. example: '**hidden**' signingCertificate: type: string description: > PEM-encoded certificate containing the public key matching the signing private key. - Redacted on read when set; never echoed back in plaintext. + Redacted on read when set; never echoed back in plaintext. Use an empty string when unset. example: '**hidden**' acsBinding: type: string @@ -106,6 +120,9 @@ properties: type: object description: Configuration for the signature in SAML requests and responses. additionalProperties: false + required: + - prefix + - location properties: prefix: type: string @@ -113,6 +130,9 @@ properties: location: type: object additionalProperties: false + required: + - reference + - action properties: reference: type: string @@ -123,5 +143,5 @@ properties: example: after relayState: type: string - description: Default relay state value for SAML requests. + description: Default relay state value for SAML requests. Use an empty string when unset. example: https://n8n.example.com diff --git a/packages/cli/src/public-api/v1/handlers/sso-saml/sso-saml.handler.ts b/packages/cli/src/public-api/v1/handlers/sso-saml/sso-saml.handler.ts index e117cfb08eb..1fbf4e575e7 100644 --- a/packages/cli/src/public-api/v1/handlers/sso-saml/sso-saml.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/sso-saml/sso-saml.handler.ts @@ -1,17 +1,22 @@ +import { UpdateSamlConfigurationDto } from '@n8n/api-types'; +import { InstanceSettingsLoaderConfig } from '@n8n/config'; import { Container } from '@n8n/di'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { ConflictError } from '@/errors/response-errors/conflict.error'; import { SamlService } from '@/modules/sso-saml/saml.service.ee'; +import { toSamlConfigurationResponse, toSamlPreferencesUpdate } from './sso-saml.mapper'; import type { SsoSamlRequest } from '../../../types'; import type { PublicAPIEndpoint } from '../../shared/handler.types'; import { apiKeyHasScopeWithGlobalScopeFallback, isLicensed, } from '../../shared/middlewares/global.middleware'; -import { toSamlConfigurationResponse } from './sso-saml.mapper'; type SsoSamlHandlers = { getSamlConfiguration: PublicAPIEndpoint; + updateSamlConfiguration: PublicAPIEndpoint; }; const ssoSamlHandlers: SsoSamlHandlers = { @@ -24,6 +29,29 @@ const ssoSamlHandlers: SsoSamlHandlers = { return res.json(toSamlConfigurationResponse(samlService.samlPreferences)); }, ], + + updateSamlConfiguration: [ + isLicensed('feat:saml'), + apiKeyHasScopeWithGlobalScopeFallback({ scope: 'saml:manage' }), + async (req, res) => { + const payload = UpdateSamlConfigurationDto.safeParse(req.body); + if (!payload.success) { + throw new BadRequestError(payload.error.errors[0]?.message ?? 'Invalid request body'); + } + + const { ssoManagedByEnv } = Container.get(InstanceSettingsLoaderConfig); + if (ssoManagedByEnv) { + throw new ConflictError( + 'SSO configuration is managed declaratively and cannot be modified through the API', + ); + } + + const samlService = Container.get(SamlService); + await samlService.setSamlPreferences(toSamlPreferencesUpdate(payload.data)); + + return res.json(toSamlConfigurationResponse(samlService.samlPreferences)); + }, + ], }; export = ssoSamlHandlers; diff --git a/packages/cli/src/public-api/v1/handlers/sso-saml/sso-saml.mapper.ts b/packages/cli/src/public-api/v1/handlers/sso-saml/sso-saml.mapper.ts index 6f2a70bec1d..c4205363b41 100644 --- a/packages/cli/src/public-api/v1/handlers/sso-saml/sso-saml.mapper.ts +++ b/packages/cli/src/public-api/v1/handlers/sso-saml/sso-saml.mapper.ts @@ -1,4 +1,8 @@ -import type { SamlConfigurationResponse, SamlPreferences } from '@n8n/api-types'; +import type { + SamlConfigurationResponse, + SamlPreferences, + UpdateSamlConfigurationDto, +} from '@n8n/api-types'; import { CREDENTIAL_BLANKING_VALUE } from 'n8n-workflow'; import { @@ -6,13 +10,63 @@ import { getServiceProviderReturnUrl, } from '@/modules/sso-saml/service-provider.ee'; +/** + * Normalize preferences into the public API response shape. + * + * Every writable PUT field is always present so a GET response can be sent back + * as a PUT body. Secrets are redacted with the blanking placeholder when set, or + * `""` when unset. Read-only `entityID` / `returnUrl` are included and ignored on write. + */ export function toSamlConfigurationResponse(prefs: SamlPreferences): SamlConfigurationResponse { return { - ...prefs, - metadata: prefs.metadata ? CREDENTIAL_BLANKING_VALUE : undefined, - signingPrivateKey: prefs.signingPrivateKey ? CREDENTIAL_BLANKING_VALUE : undefined, - signingCertificate: prefs.signingCertificate ? CREDENTIAL_BLANKING_VALUE : undefined, + mapping: { + email: prefs.mapping?.email ?? '', + firstName: prefs.mapping?.firstName ?? '', + lastName: prefs.mapping?.lastName ?? '', + userPrincipalName: prefs.mapping?.userPrincipalName ?? '', + n8nInstanceRole: prefs.mapping?.n8nInstanceRole ?? '', + n8nProjectRoles: prefs.mapping?.n8nProjectRoles ?? [], + }, + metadata: prefs.metadata ? CREDENTIAL_BLANKING_VALUE : '', + metadataUrl: prefs.metadataUrl ?? '', + ignoreSSL: prefs.ignoreSSL ?? false, + loginBinding: prefs.loginBinding ?? 'redirect', + loginEnabled: prefs.loginEnabled ?? false, + loginLabel: prefs.loginLabel ?? '', + authnRequestsSigned: prefs.authnRequestsSigned ?? false, + wantAssertionsSigned: prefs.wantAssertionsSigned ?? true, + wantMessageSigned: prefs.wantMessageSigned ?? true, + signingPrivateKey: prefs.signingPrivateKey ? CREDENTIAL_BLANKING_VALUE : '', + signingCertificate: prefs.signingCertificate ? CREDENTIAL_BLANKING_VALUE : '', + acsBinding: prefs.acsBinding ?? 'post', + signatureConfig: prefs.signatureConfig ?? { + prefix: 'ds', + location: { + reference: '/samlp:Response/saml:Issuer', + action: 'after', + }, + }, + relayState: prefs.relayState ?? '', entityID: getServiceProviderEntityId(), returnUrl: getServiceProviderReturnUrl(), }; } + +/** + * Convert a validated PUT body into preferences for `setSamlPreferences`. + * Treats redaction placeholders as "keep existing" (omit the field). The service + * already does this for `signingPrivateKey`; we mirror it here for metadata and + * signingCertificate, which the service does not handle the same way. + */ +export function toSamlPreferencesUpdate( + data: UpdateSamlConfigurationDto, +): Partial { + const { metadata, signingCertificate, signingPrivateKey, ...rest } = data; + + return { + ...rest, + ...(metadata === CREDENTIAL_BLANKING_VALUE ? {} : { metadata }), + ...(signingCertificate === CREDENTIAL_BLANKING_VALUE ? {} : { signingCertificate }), + ...(signingPrivateKey === CREDENTIAL_BLANKING_VALUE ? {} : { signingPrivateKey }), + }; +} diff --git a/packages/cli/test/integration/public-api/security-policy.test.ts b/packages/cli/test/integration/public-api/security-policy.test.ts index e57ba9abafb..3adaa9c2b13 100644 --- a/packages/cli/test/integration/public-api/security-policy.test.ts +++ b/packages/cli/test/integration/public-api/security-policy.test.ts @@ -106,13 +106,19 @@ describe('Security policy in Public API', () => { }); describe('PUT /settings/security-policy', () => { + const fullPolicy = { + personalSpacePublishing: false, + personalSpaceSharing: true, + redactionEnforcement: { floor: 'production' as const }, + }; + it('updates the policy and returns the new values', async () => { testServer.license.enable('feat:personalSpacePolicy'); const response = await testServer .publicApiAgentFor(owner) .put('/settings/security-policy') - .send({ personalSpacePublishing: false, redactionEnforcement: { floor: 'production' } }); + .send(fullPolicy); expect(response.status).toBe(200); expect(response.body).toMatchObject({ @@ -127,17 +133,54 @@ describe('Security policy in Public API', () => { .get('/settings/security-policy'); expect(readResponse.body).toMatchObject({ personalSpacePublishing: false, + personalSpaceSharing: true, redactionEnforcement: { floor: 'production' }, }); }); + it('accepts a GET response body as a PUT body', async () => { + testServer.license.enable('feat:personalSpacePolicy'); + + const getResponse = await testServer + .publicApiAgentFor(owner) + .get('/settings/security-policy'); + expect(getResponse.status).toBe(200); + + const putResponse = await testServer + .publicApiAgentFor(owner) + .put('/settings/security-policy') + .send({ + ...getResponse.body, + personalSpacePublishing: false, + }); + + expect(putResponse.status).toBe(200); + expect(putResponse.body.personalSpacePublishing).toBe(false); + expect(putResponse.body.publishedPersonalWorkflowsCount).toBe(0); + }); + + it('rejects a partial request body with 400', async () => { + testServer.license.enable('feat:personalSpacePolicy'); + + const response = await testServer + .publicApiAgentFor(owner) + .put('/settings/security-policy') + .send({ personalSpacePublishing: false }); + + expect(response.status).toBe(400); + }); + it('rejects a malformed request body with 400', async () => { testServer.license.enable('feat:personalSpacePolicy'); const response = await testServer .publicApiAgentFor(owner) .put('/settings/security-policy') - .send({ personalSpacePublishing: 'not-a-boolean', redactionEnforcement: 'nope' }); + .send({ + personalSpacePublishing: 'not-a-boolean', + personalSpaceSharing: true, + redactionEnforcement: 'nope', + }); expect(response.status).toBe(400); }); @@ -148,7 +191,11 @@ describe('Security policy in Public API', () => { const response = await testServer .publicApiAgentFor(owner) .put('/settings/security-policy') - .send({ redactionEnforcement: { floor: 'bogus' } }); + .send({ + personalSpacePublishing: false, + personalSpaceSharing: true, + redactionEnforcement: { floor: 'bogus' }, + }); expect(response.status).toBe(400); expect(response.body).toHaveProperty('message'); @@ -160,7 +207,7 @@ describe('Security policy in Public API', () => { const response = await testServer .publicApiAgentWithoutApiKey() .put('/settings/security-policy') - .send({ personalSpacePublishing: false }); + .send(fullPolicy); expect(response.status).toBe(401); }); @@ -169,7 +216,7 @@ describe('Security policy in Public API', () => { const response = await testServer .publicApiAgentFor(owner) .put('/settings/security-policy') - .send({ personalSpacePublishing: false }); + .send(fullPolicy); expect(response.status).toBe(403); expect(response.body).toHaveProperty('message', licenseErrorMessage); @@ -182,7 +229,7 @@ describe('Security policy in Public API', () => { const response = await testServer .publicApiAgentFor(scopedOwner) .put('/settings/security-policy') - .send({ personalSpacePublishing: false }); + .send(fullPolicy); expect(response.status).toBe(403); }); @@ -194,7 +241,7 @@ describe('Security policy in Public API', () => { const writeResponse = await testServer .publicApiAgentFor(owner) .put('/settings/security-policy') - .send({ personalSpacePublishing: false }); + .send(fullPolicy); expect(writeResponse.status).toBe(409); diff --git a/packages/cli/test/integration/public-api/sso-saml.test.ts b/packages/cli/test/integration/public-api/sso-saml.test.ts index d600e354721..e4447dc4485 100644 --- a/packages/cli/test/integration/public-api/sso-saml.test.ts +++ b/packages/cli/test/integration/public-api/sso-saml.test.ts @@ -1,4 +1,5 @@ import { testDb } from '@n8n/backend-test-utils'; +import { InstanceSettingsLoaderConfig } from '@n8n/config'; import type { User } from '@n8n/db'; import { Container } from '@n8n/di'; import { CREDENTIAL_BLANKING_VALUE } from 'n8n-workflow'; @@ -20,17 +21,23 @@ describe('SAML SSO configuration in Public API', () => { }); const licenseErrorMessage = new FeatureNotLicensedError('feat:saml').message; + const setManagedByEnv = (value: boolean) => { + Container.get(InstanceSettingsLoaderConfig).ssoManagedByEnv = value; + }; + beforeAll(async () => { await testDb.init(); }); beforeEach(async () => { await testDb.truncate(['User']); + setManagedByEnv(false); owner = await createOwnerWithApiKey(); }); afterEach(() => { delete process.env.N8N_ENV_FEAT_SIGNED_SAML_REQUESTS; + setManagedByEnv(false); }); describe('GET /settings/sso/saml', () => { @@ -76,6 +83,10 @@ describe('SAML SSO configuration in Public API', () => { expect(response.body.metadata).toBe(CREDENTIAL_BLANKING_VALUE); expect(response.body.metadata).not.toContain('BEGIN CERTIFICATE'); expect(response.body.signingCertificate).not.toContain('BEGIN CERTIFICATE'); + expect(response.body.mapping).toMatchObject({ + n8nInstanceRole: expect.any(String), + n8nProjectRoles: expect.any(Array), + }); }); it('rejects with 403 when not licensed', async () => { @@ -113,4 +124,191 @@ describe('SAML SSO configuration in Public API', () => { expect(response.body.loginLabel).toBe(samlService.samlPreferences.loginLabel); }); }); + + describe('PUT /settings/sso/saml', () => { + it('sets the SAML configuration and returns the updated values', async () => { + testServer.license.enable('feat:saml'); + + const response = await testServer + .publicApiAgentFor(owner) + .put('/settings/sso/saml') + .send({ + ...sampleConfig, + loginLabel: 'Updated SAML Label', + }); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + loginLabel: 'Updated SAML Label', + loginEnabled: sampleConfig.loginEnabled, + ignoreSSL: sampleConfig.ignoreSSL, + }); + expect(response.body.entityID).toContain('/rest/sso/saml/metadata'); + expect(response.body.returnUrl).toContain('/rest/sso/saml/acs'); + + const readResponse = await testServer.publicApiAgentFor(owner).get('/settings/sso/saml'); + expect(readResponse.body.loginLabel).toBe('Updated SAML Label'); + }); + + it('accepts a GET response body as a PUT body and preserves redacted secrets', async () => { + testServer.license.enable('feat:saml'); + process.env.N8N_ENV_FEAT_SIGNED_SAML_REQUESTS = 'true'; + + await testServer + .authAgentFor(owner) + .post('/sso/saml/config') + .send({ + ...sampleConfig, + signingPrivateKey: RSA_TEST_PRIVATE_KEY, + signingCertificate: RSA_TEST_CERTIFICATE, + }); + + const getResponse = await testServer.publicApiAgentFor(owner).get('/settings/sso/saml'); + expect(getResponse.status).toBe(200); + + const putResponse = await testServer + .publicApiAgentFor(owner) + .put('/settings/sso/saml') + .send({ + ...getResponse.body, + loginLabel: 'Round-tripped Label', + }); + + expect(putResponse.status).toBe(200); + expect(putResponse.body.loginLabel).toBe('Round-tripped Label'); + expect(putResponse.body.signingPrivateKey).toBe(CREDENTIAL_BLANKING_VALUE); + expect(putResponse.body.signingCertificate).toBe(CREDENTIAL_BLANKING_VALUE); + expect(putResponse.body.metadata).toBe(CREDENTIAL_BLANKING_VALUE); + + const samlService = Container.get(SamlService); + expect(samlService.samlPreferences.signingPrivateKey).toBeTruthy(); + expect(samlService.samlPreferences.signingCertificate).toBe(RSA_TEST_CERTIFICATE); + expect(samlService.samlPreferences.metadata).toContain('EntityDescriptor'); + }); + + it('clears IdP metadata when metadata and metadataUrl are sent as empty strings', async () => { + testServer.license.enable('feat:saml'); + + await testServer.publicApiAgentFor(owner).put('/settings/sso/saml').send(sampleConfig); + + const clearResponse = await testServer + .publicApiAgentFor(owner) + .put('/settings/sso/saml') + .send({ + ...sampleConfig, + metadata: '', + metadataUrl: '', + }); + + expect(clearResponse.status).toBe(200); + expect(clearResponse.body.metadata).toBe(''); + expect(clearResponse.body.metadataUrl).toBe(''); + + const samlService = Container.get(SamlService); + expect(samlService.samlPreferences.metadata).toBe(''); + expect(samlService.samlPreferences.metadataUrl).toBeUndefined(); + }); + + it('rejects a partial request body with 400', async () => { + testServer.license.enable('feat:saml'); + + const response = await testServer + .publicApiAgentFor(owner) + .put('/settings/sso/saml') + .send({ loginLabel: 'SAML' }); + + expect(response.status).toBe(400); + }); + + it('rejects a malformed request body with 400', async () => { + testServer.license.enable('feat:saml'); + + const response = await testServer + .publicApiAgentFor(owner) + .put('/settings/sso/saml') + .send({ + ...sampleConfig, + ignoreSSL: 'not-a-boolean', + }); + + expect(response.status).toBe(400); + }); + + it('rejects a well-formed body with invalid values with 400', async () => { + testServer.license.enable('feat:saml'); + process.env.N8N_ENV_FEAT_SIGNED_SAML_REQUESTS = 'true'; + + const samlService = Container.get(SamlService); + type PrivatePrefs = { _samlPreferences: typeof samlService.samlPreferences }; + (samlService as unknown as PrivatePrefs)._samlPreferences.signingPrivateKey = undefined; + (samlService as unknown as PrivatePrefs)._samlPreferences.signingCertificate = undefined; + + const response = await testServer + .publicApiAgentFor(owner) + .put('/settings/sso/saml') + .send({ + ...sampleConfig, + authnRequestsSigned: true, + }); + + expect(response.status).toBe(400); + expect(response.body).toHaveProperty('message'); + }); + + it('rejects with 401 without a valid API key', async () => { + testServer.license.enable('feat:saml'); + + const response = await testServer + .publicApiAgentWithoutApiKey() + .put('/settings/sso/saml') + .send(sampleConfig); + + expect(response.status).toBe(401); + }); + + it('rejects with 403 when not licensed', async () => { + const response = await testServer + .publicApiAgentFor(owner) + .put('/settings/sso/saml') + .send(sampleConfig); + + expect(response.status).toBe(403); + expect(response.body).toHaveProperty('message', licenseErrorMessage); + }); + + it('rejects with 403 when the API key lacks the saml:manage scope', async () => { + testServer.license.enable('feat:saml'); + const scopedOwner = await createOwnerWithApiKey({ scopes: ['workflow:read'] }); + + const response = await testServer + .publicApiAgentFor(scopedOwner) + .put('/settings/sso/saml') + .send(sampleConfig); + + expect(response.status).toBe(403); + }); + + it('rejects a write with 409 when managed declaratively, but still allows reads', async () => { + testServer.license.enable('feat:saml'); + setManagedByEnv(true); + + const writeResponse = await testServer + .publicApiAgentFor(owner) + .put('/settings/sso/saml') + .send({ + ...sampleConfig, + loginLabel: 'Blocked Label', + }); + + expect(writeResponse.status).toBe(409); + expect(writeResponse.body).toMatchObject({ + message: + 'SSO configuration is managed declaratively and cannot be modified through the API', + }); + + const readResponse = await testServer.publicApiAgentFor(owner).get('/settings/sso/saml'); + expect(readResponse.status).toBe(200); + expect(readResponse.body.loginLabel).not.toBe('Blocked Label'); + }); + }); }); diff --git a/packages/cli/test/integration/saml/sample-metadata.ts b/packages/cli/test/integration/saml/sample-metadata.ts index 60f18426aa5..469fb611e28 100644 --- a/packages/cli/test/integration/saml/sample-metadata.ts +++ b/packages/cli/test/integration/saml/sample-metadata.ts @@ -9,6 +9,7 @@ export const sampleConfig: SamlPreferences = { lastName: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/lastname', userPrincipalName: 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn', n8nInstanceRole: 'n8n_instance_role', + n8nProjectRoles: [], }, metadata: sampleMetadata, metadataUrl: '', @@ -20,6 +21,8 @@ export const sampleConfig: SamlPreferences = { loginLabel: 'SAML Login', wantAssertionsSigned: true, wantMessageSigned: true, + signingPrivateKey: '', + signingCertificate: '', signatureConfig: { prefix: 'ds', location: { diff --git a/packages/nodes-base/nodes/N8n/n8n-api-coverage.json b/packages/nodes-base/nodes/N8n/n8n-api-coverage.json index 72ac6743208..b18ef62c28b 100644 --- a/packages/nodes-base/nodes/N8n/n8n-api-coverage.json +++ b/packages/nodes-base/nodes/N8n/n8n-api-coverage.json @@ -184,6 +184,9 @@ "GET /settings/sso/saml": { "status": "gap" }, + "PUT /settings/sso/saml": { + "status": "gap" + }, "GET /settings/log-streaming/event-types": { "status": "gap" },