From 2eb1de6c82cd3d642ff3bbb50489c9c1b552ca4a Mon Sep 17 00:00:00 2001 From: Konstantin Tieber <46342664+konstantintieber@users.noreply.github.com> Date: Mon, 3 Nov 2025 16:34:06 +0200 Subject: [PATCH] feat(core): Just in time role provisioning for SAML login (#21387) Co-authored-by: Stephen --- packages/@n8n/api-types/src/dto/index.ts | 1 + .../src/dto/provisioning/config.dto.ts | 5 - .../src/dto/saml/saml-preferences.dto.ts | 25 ++-- .../@n8n/config/src/configs/sso.config.ts | 6 - packages/@n8n/config/test/config.test.ts | 1 - .../provisioning.controller.ee.test.ts | 2 - .../__tests__/provisioning.service.ee.test.ts | 88 +++++------ .../provisioning.service.ee.ts | 29 +++- .../cli/src/sso.ee/oidc/oidc.service.ee.ts | 16 +- .../saml/__tests__/saml-helpers.test.ts | 115 ++++++++++++++ .../saml/__tests__/saml.service.ee.test.ts | 85 ++++++++++- .../__tests__/saml.controller.ee.test.ts | 11 +- .../sso.ee/saml/routes/saml.controller.ee.ts | 12 -- packages/cli/src/sso.ee/saml/saml-helpers.ts | 25 +++- .../cli/src/sso.ee/saml/saml.service.ee.ts | 36 +++-- packages/cli/src/sso.ee/saml/types.ts | 4 +- .../frontend/@n8n/i18n/src/locales/en.json | 9 +- .../rest-api-client/src/api/provisioning.ts | 1 - .../views/SettingsProvisioningView.vue | 141 +++++------------- 19 files changed, 364 insertions(+), 248 deletions(-) diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index de861340f9d..2a980ebcc0e 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -35,6 +35,7 @@ export { ChangeUserRoleInProject } from './project/change-user-role-in-project.d export { SamlAcsDto } from './saml/saml-acs.dto'; export { SamlPreferences } from './saml/saml-preferences.dto'; +export { SamlPreferencesAttributeMapping } from './saml/saml-preferences.dto'; export { SamlToggleDto } from './saml/saml-toggle.dto'; export { PasswordUpdateRequestDto } from './user/password-update-request.dto'; diff --git a/packages/@n8n/api-types/src/dto/provisioning/config.dto.ts b/packages/@n8n/api-types/src/dto/provisioning/config.dto.ts index ae71b2e9ee1..2e96335779c 100644 --- a/packages/@n8n/api-types/src/dto/provisioning/config.dto.ts +++ b/packages/@n8n/api-types/src/dto/provisioning/config.dto.ts @@ -4,7 +4,6 @@ import { Z } from 'zod-class'; export class ProvisioningConfigDto extends Z.class({ scopesProvisionInstanceRole: z.boolean(), scopesProvisionProjectRoles: z.boolean(), - scopesProvisioningFrequency: z.enum(['never', 'first_login', 'every_login']), scopesName: z.string(), scopesInstanceRoleClaimName: z.string(), scopesProjectsRolesClaimName: z.string(), @@ -13,10 +12,6 @@ export class ProvisioningConfigDto extends Z.class({ export class ProvisioningConfigPatchDto extends Z.class({ scopesProvisionInstanceRole: z.boolean().optional().nullable(), scopesProvisionProjectRoles: z.boolean().optional().nullable(), - scopesProvisioningFrequency: z - .enum(['never', 'first_login', 'every_login']) - .optional() - .nullable(), scopesName: z.string().optional().nullable(), scopesInstanceRoleClaimName: z.string().optional().nullable(), scopesProjectsRolesClaimName: z.string().optional().nullable(), 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 36178b75117..9aa3f3b6c89 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 @@ -12,17 +12,24 @@ const SignatureConfigSchema = z.object({ }), }); +export class SamlPreferencesAttributeMapping extends Z.class({ + /** SAML attribute mapped to the user's email. */ + email: z.string(), + /** SAML attribute mapped to the user's first name. */ + firstName: z.string(), + /** SAML attribute mapped to the user's last name. */ + lastName: z.string(), + /** SAML attribute mapped to the user's principal name. */ + userPrincipalName: z.string(), + /** SAML attribute mapped to the n8n instance role. */ + n8nInstanceRole: z.string().optional(), + /** Each element in the array is formatted like ":" */ + n8nProjectRoles: z.array(z.string()).optional(), +}) {} + export class SamlPreferences extends Z.class({ /** Mapping of SAML attributes to user fields. */ - mapping: z - .object({ - email: z.string(), - firstName: z.string(), - lastName: z.string(), - userPrincipalName: z.string(), - n8nInstanceRole: z.string(), - }) - .optional(), + mapping: SamlPreferencesAttributeMapping.optional(), /** SAML metadata in XML format. */ metadata: z.string().optional(), metadataUrl: z.string().optional(), diff --git a/packages/@n8n/config/src/configs/sso.config.ts b/packages/@n8n/config/src/configs/sso.config.ts index c5ef6f6ea96..2249f526fbc 100644 --- a/packages/@n8n/config/src/configs/sso.config.ts +++ b/packages/@n8n/config/src/configs/sso.config.ts @@ -1,7 +1,5 @@ import { Config, Env, Nested } from '../decorators'; -type ScopesProvisioningFrequency = 'never' | 'first_login' | 'every_login'; - @Config class SamlConfig { /** Whether to enable SAML SSO. */ @@ -39,10 +37,6 @@ class ProvisioningConfig { @Env('N8N_SSO_SCOPES_PROVISION_PROJECT_ROLES') scopesProvisionProjectRoles: boolean = false; - /** How often to trigger provisioning, never, fist login, or every login */ - @Env('N8N_SSO_SCOPES_PROVISIONING_FREQUENCY') - scopesProvisioningFrequency: ScopesProvisioningFrequency = 'never'; - /** The name of scope to request on oauth flows */ @Env('N8N_SSO_SCOPES_NAME') scopesName: string = 'n8n'; diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index 1416e663849..a28ba4312d8 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -381,7 +381,6 @@ describe('GlobalConfig', () => { provisioning: { scopesProvisionInstanceRole: false, scopesProvisionProjectRoles: false, - scopesProvisioningFrequency: 'never', scopesName: 'n8n', scopesInstanceRoleClaimName: 'n8n_instance_role', scopesProjectsRolesClaimName: 'n8n_projects', diff --git a/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.controller.ee.test.ts b/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.controller.ee.test.ts index 20a8e775d4f..1252942e69b 100644 --- a/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.controller.ee.test.ts +++ b/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.controller.ee.test.ts @@ -35,7 +35,6 @@ describe('ProvisioningController', () => { const configResponse: ProvisioningConfigDto = { scopesProvisionInstanceRole: true, scopesProvisionProjectRoles: true, - scopesProvisioningFrequency: 'every_login', scopesName: 'n8n_test_scope', scopesInstanceRoleClaimName: 'n8n_test_instance_role', scopesProjectsRolesClaimName: 'n8n_test_projects_roles', @@ -68,7 +67,6 @@ describe('ProvisioningController', () => { const configResponse: ProvisioningConfigDto = { scopesProvisionInstanceRole: false, scopesProvisionProjectRoles: false, - scopesProvisioningFrequency: 'never', scopesName: 'n8n_test_scope', scopesInstanceRoleClaimName: 'n8n_test_instance_role', scopesProjectsRolesClaimName: 'n8n_test_projects_roles', diff --git a/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.service.ee.test.ts b/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.service.ee.test.ts index 7a169695c39..9fa1bba6c31 100644 --- a/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.service.ee.test.ts +++ b/packages/cli/src/modules/provisioning.ee/__tests__/provisioning.service.ee.test.ts @@ -56,7 +56,6 @@ describe('ProvisioningService', () => { const provisioningConfigDto: ProvisioningConfigDto = { scopesProvisionInstanceRole: true, scopesProvisionProjectRoles: true, - scopesProvisioningFrequency: 'every_login', scopesName: 'n8n_test_scope', scopesInstanceRoleClaimName: 'n8n_test_instance_role', scopesProjectsRolesClaimName: 'n8n_test_projects_roles', @@ -143,7 +142,6 @@ describe('ProvisioningService', () => { const overriddenConfig = { scopesProvisionInstanceRole: false, scopesProvisionProjectRoles: false, - scopesProvisioningFrequency: 'never', scopesName: 'n8n_test_scope_overridden', scopesInstanceRoleClaimName: 'n8n_test_instance_role_overridden', scopesProjectsRolesClaimName: 'n8n_test_projects_roles_overridden', @@ -173,6 +171,8 @@ describe('ProvisioningService', () => { const user = mock({ role: { slug: 'global:member' } }); const roleSlug = 123; + provisioningService['isInstanceRoleProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionInstanceRoleForUser(user, roleSlug); expect(userRepository.update).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledTimes(1); @@ -189,6 +189,8 @@ describe('ProvisioningService', () => { roleRepository.findOneOrFail.mockRejectedValue(thrownError); + provisioningService['isInstanceRoleProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionInstanceRoleForUser(user, roleSlug); expect(userRepository.update).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledTimes(1); @@ -207,6 +209,8 @@ describe('ProvisioningService', () => { mock({ slug: 'global:member', roleType: 'global' }), ); + provisioningService['isInstanceRoleProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionInstanceRoleForUser(user, roleSlug); expect(userRepository.update).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledTimes(1); @@ -224,6 +228,8 @@ describe('ProvisioningService', () => { mock({ slug: 'global:member', roleType: 'global' }), ); + provisioningService['isInstanceRoleProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionInstanceRoleForUser(user, roleSlug); expect(userRepository.update).toHaveBeenCalledWith(user.id, { role: { slug: roleSlug } }); expect(logger.warn).not.toHaveBeenCalled(); @@ -237,6 +243,8 @@ describe('ProvisioningService', () => { mock({ slug: 'global:owner', roleType: 'global' }), ); + provisioningService['isInstanceRoleProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionInstanceRoleForUser(user, roleSlug); expect(userRepository.update).toHaveBeenCalledWith(user.id, { role: { slug: roleSlug } }); }); @@ -249,6 +257,8 @@ describe('ProvisioningService', () => { mock({ slug: 'global:owner', roleType: 'global' }), ); + provisioningService['isInstanceRoleProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionInstanceRoleForUser(user, roleSlug); expect(userRepository.update).not.toHaveBeenCalled(); expect(logger.warn).not.toHaveBeenCalled(); @@ -262,6 +272,8 @@ describe('ProvisioningService', () => { mock({ slug: 'global:owner', roleType: 'project' }), ); + provisioningService['isInstanceRoleProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionInstanceRoleForUser(user, roleSlug); expect(userRepository.update).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledTimes(1); @@ -277,6 +289,8 @@ describe('ProvisioningService', () => { const userId = 'user-id-123'; const projectIdToRole = { not: 'an array' }; + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); @@ -291,6 +305,8 @@ describe('ProvisioningService', () => { const userId = 'user-id-123'; const projectIdToRole = 'invalid-json-string'; + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); @@ -305,6 +321,8 @@ describe('ProvisioningService', () => { const userId = 'user-id-123'; const projectIdToRole = [{ projectId: 'project-1', role: 'viewer' }]; // invalid value type + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); @@ -321,6 +339,8 @@ describe('ProvisioningService', () => { projectRepository.find.mockResolvedValue([]); roleRepository.find.mockResolvedValue([mock({ slug: 'project:viewer' })]); + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); @@ -332,6 +352,8 @@ describe('ProvisioningService', () => { projectRepository.find.mockResolvedValue([mock({ id: 'project-1' })]); roleRepository.find.mockResolvedValue([]); + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); @@ -350,6 +372,8 @@ describe('ProvisioningService', () => { mock({ displayName: 'viewer', slug: 'project:viewer' }), ]); + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); @@ -375,6 +399,8 @@ describe('ProvisioningService', () => { mock({ displayName: 'editor', slug: 'project:editor' }), ]); + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).toHaveBeenCalledTimes(1); @@ -404,6 +430,8 @@ describe('ProvisioningService', () => { mock({ displayName: 'editor', slug: 'project:editor' }), ]); + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).toHaveBeenCalledTimes(2); @@ -424,6 +452,8 @@ describe('ProvisioningService', () => { projectRepository.find.mockResolvedValue([mock({ id: 'project1' })]); roleRepository.find.mockResolvedValue([]); + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); @@ -447,6 +477,8 @@ describe('ProvisioningService', () => { mock({ displayName: 'viewer', slug: 'project:viewer' }), ]); + provisioningService['isProjectRolesProvisioningEnabled'] = jest.fn().mockResolvedValue(true); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(entityManager.transaction).toHaveBeenCalledTimes(1); @@ -535,56 +567,4 @@ describe('ProvisioningService', () => { provisioningService.getConfig = originStateGetConfig; }); }); - - describe('isInstanceRoleProvisioningEnabled', () => { - it('should return true if the instance role provisioning config is enabled', async () => { - const originStateGetConfig = provisioningService.getConfig; - - const provisioningConfig = { ...provisioningConfigDto, scopesProvisionInstanceRole: true }; - provisioningService.getConfig = jest.fn().mockResolvedValue(provisioningConfig); - const isInstanceRoleProvisioningEnabled = - await provisioningService.isInstanceRoleProvisioningEnabled(); - expect(isInstanceRoleProvisioningEnabled).toBe(true); - - provisioningService.getConfig = originStateGetConfig; - }); - - it('should return false if the instance role provisioning config is not enabled', async () => { - const originStateGetConfig = provisioningService.getConfig; - - const provisioningConfig = { ...provisioningConfigDto, scopesProvisionInstanceRole: false }; - provisioningService.getConfig = jest.fn().mockResolvedValue(provisioningConfig); - const isInstanceRoleProvisioningEnabled = - await provisioningService.isInstanceRoleProvisioningEnabled(); - expect(isInstanceRoleProvisioningEnabled).toBe(false); - - provisioningService.getConfig = originStateGetConfig; - }); - }); - - describe('isProjectRolesProvisioningEnabled', () => { - it('should return true if the project roles provisioning config is enabled', async () => { - const originStateGetConfig = provisioningService.getConfig; - - const provisioningConfig = { ...provisioningConfigDto, scopesProvisionProjectRoles: true }; - provisioningService.getConfig = jest.fn().mockResolvedValue(provisioningConfig); - const isProjectRolesProvisioningEnabled = - await provisioningService.isProjectRolesProvisioningEnabled(); - expect(isProjectRolesProvisioningEnabled).toBe(true); - - provisioningService.getConfig = originStateGetConfig; - }); - - it('should return false if the project roles provisioning config is not enabled', async () => { - const originStateGetConfig = provisioningService.getConfig; - - const provisioningConfig = { ...provisioningConfigDto, scopesProvisionProjectRoles: false }; - provisioningService.getConfig = jest.fn().mockResolvedValue(provisioningConfig); - const isProjectRolesProvisioningEnabled = - await provisioningService.isProjectRolesProvisioningEnabled(); - expect(isProjectRolesProvisioningEnabled).toBe(false); - - provisioningService.getConfig = originStateGetConfig; - }); - }); }); diff --git a/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts b/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts index cca3bfc3e91..7890427a64e 100644 --- a/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts +++ b/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts @@ -50,6 +50,10 @@ export class ProvisioningService { } async provisionInstanceRoleForUser(user: User, roleSlug: unknown) { + if (!(await this.isInstanceRoleProvisioningEnabled())) { + return; + } + const globalOwnerRoleSlug = 'global:owner'; if (typeof roleSlug !== 'string') { @@ -116,6 +120,10 @@ export class ProvisioningService { * ] */ async provisionProjectRolesForUser(userId: string, projectIdToRoles: unknown): Promise { + if (!(await this.isProjectRolesProvisioningEnabled())) { + return; + } + if (!Array.isArray(projectIdToRoles)) { this.logger.warn( `Skipping project role provisioning. Invalid projectIdToRole type: expected array, received ${typeof projectIdToRoles}`, @@ -254,7 +262,6 @@ export class ProvisioningService { const supportedPatchFields = [ 'scopesProvisionInstanceRole', 'scopesProvisionProjectRoles', - 'scopesProvisioningFrequency', 'scopesName', 'scopesInstanceRoleClaimName', 'scopesProjectsRolesClaimName', @@ -330,6 +337,22 @@ export class ProvisioningService { return envProvidedConfig; } + async getInstanceRoleClaimName(): Promise { + if (!(await this.isInstanceRoleProvisioningEnabled())) { + return null; + } + const provisioningConfig = await this.getConfig(); + return provisioningConfig.scopesInstanceRoleClaimName; + } + + async getProjectsRolesClaimName(): Promise { + if (!(await this.isProjectRolesProvisioningEnabled())) { + return null; + } + const provisioningConfig = await this.getConfig(); + return provisioningConfig.scopesProjectsRolesClaimName; + } + async isProvisioningEnabled(): Promise { const provisioningConfig = await this.getConfig(); return ( @@ -338,12 +361,12 @@ export class ProvisioningService { ); } - async isInstanceRoleProvisioningEnabled(): Promise { + private async isInstanceRoleProvisioningEnabled(): Promise { const provisioningConfig = await this.getConfig(); return provisioningConfig.scopesProvisionInstanceRole; } - async isProjectRolesProvisioningEnabled(): Promise { + private async isProjectRolesProvisioningEnabled(): Promise { const provisioningConfig = await this.getConfig(); return provisioningConfig.scopesProvisionProjectRoles; } diff --git a/packages/cli/src/sso.ee/oidc/oidc.service.ee.ts b/packages/cli/src/sso.ee/oidc/oidc.service.ee.ts index f93a6dd6964..6f932257d3e 100644 --- a/packages/cli/src/sso.ee/oidc/oidc.service.ee.ts +++ b/packages/cli/src/sso.ee/oidc/oidc.service.ee.ts @@ -312,17 +312,13 @@ export class OidcService { private async applySsoProvisioning(user: User, claims: any) { const provisioningConfig = await this.provisioningService.getConfig(); - if (await this.provisioningService.isInstanceRoleProvisioningEnabled()) { - await this.provisioningService.provisionInstanceRoleForUser( - user, - claims[provisioningConfig.scopesInstanceRoleClaimName], - ); + const projectRoleMapping = claims[provisioningConfig.scopesProjectsRolesClaimName]; + const instanceRole = claims[provisioningConfig.scopesInstanceRoleClaimName]; + if (instanceRole) { + await this.provisioningService.provisionInstanceRoleForUser(user, instanceRole); } - if (await this.provisioningService.isProjectRolesProvisioningEnabled()) { - await this.provisioningService.provisionProjectRolesForUser( - user.id, - claims[provisioningConfig.scopesProjectsRolesClaimName], - ); + if (projectRoleMapping) { + await this.provisioningService.provisionProjectRolesForUser(user.id, projectRoleMapping); } } diff --git a/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts b/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts index c6d2bc8fa0b..ff21f14a964 100644 --- a/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts +++ b/packages/cli/src/sso.ee/saml/__tests__/saml-helpers.test.ts @@ -52,4 +52,119 @@ describe('sso/saml/samlHelpers', () => { expect(userRepository.update).not.toHaveBeenCalled(); }); }); + + describe('getMappedSamlAttributesFromFlowResult', () => { + test('returns the mapped attributes from the flow result', () => { + const flowResult = { + extract: { + attributes: { + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + }, + }, + } as any; + const attributeMapping = { + email: 'email', + firstName: 'firstName', + lastName: 'lastName', + userPrincipalName: 'userPrincipalName', + }; + const jitClaimNames = { + instanceRole: 'instanceRole', + projectRoles: 'projectRoles', + }; + + const result = helpers.getMappedSamlAttributesFromFlowResult( + flowResult, + attributeMapping, + jitClaimNames, + ); + + expect(result).toEqual({ + attributes: { + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + }, + missingAttributes: [], + }); + }); + + test('returns the missing attributes from the flow result', () => { + const flowResult = { + extract: { + attributes: { + email: 'test@test.com', + }, + }, + } as any; + const attributeMapping = { + email: 'email', + firstName: 'firstName', + lastName: 'lastName', + userPrincipalName: 'userPrincipalName', + }; + const jitClaimNames = { + instanceRole: 'instanceRole', + projectRoles: 'projectRoles', + }; + + const result = helpers.getMappedSamlAttributesFromFlowResult( + flowResult, + attributeMapping, + jitClaimNames, + ); + + expect(result).toEqual({ + attributes: { + email: 'test@test.com', + }, + missingAttributes: ['userPrincipalName', 'firstName', 'lastName'], + }); + }); + test('returns the attributes from the flow result with instance role', () => { + const flowResult = { + extract: { + attributes: { + email: 'test@test.com', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + projectRoles: ['projectRole1', 'projectRole2'], + instanceRole: 'instanceRole', + }, + }, + } as any; + const attributeMapping = { + email: 'email', + instanceRole: 'instanceRole', + firstName: 'firstName', + lastName: 'lastName', + userPrincipalName: 'userPrincipalName', + }; + const jitClaimNames = { + instanceRole: 'instanceRole', + projectRoles: 'projectRoles', + }; + const result = helpers.getMappedSamlAttributesFromFlowResult( + flowResult, + attributeMapping, + jitClaimNames, + ); + expect(result).toEqual({ + attributes: { + email: 'test@test.com', + n8nInstanceRole: 'instanceRole', + firstName: 'test', + lastName: 'test', + userPrincipalName: 'test', + n8nProjectRoles: ['projectRole1', 'projectRole2'], + }, + missingAttributes: [], + }); + }); + }); }); 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 0de1828d642..4e94331b713 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 @@ -24,6 +24,8 @@ 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 type { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee'; + jest.mock('axios'); const mockedAxios = axios as jest.Mocked; @@ -150,6 +152,8 @@ describe('SamlService', () => { let settingsRepository: SettingsRepository; let instanceSettings: InstanceSettings; let globalConfig: GlobalConfig; + let userRepository: UserRepository; + let provisioningService: ProvisioningService; const validator = new SamlValidator(mock()); const logger = mockLogger(); @@ -189,9 +193,12 @@ describe('SamlService', () => { instanceSettings = mock({ isMultiMain: true, }); + provisioningService = mock(); + userRepository = mock(); globalConfig = mock({ sso: { saml: { loginEnabled: false } }, }); + provisioningService = mock(); jest .spyOn(ssoHelpers, 'reloadAuthenticationMethod') @@ -202,9 +209,10 @@ describe('SamlService', () => { logger, mock(), validator, - mock(), + userRepository, settingsRepository, instanceSettings, + provisioningService, ); // Mock GlobalConfig container access Container.set(require('@n8n/config').GlobalConfig, globalConfig); @@ -312,6 +320,81 @@ describe('SamlService', () => { }); }); + // TODO: add tests for getAttributesFromLoginResponse + + describe('handleSamlLogin', () => { + // TODO: add test cases for remaining logic (so far only for onboarding user) + it('throws error for invalid email', async () => { + jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue({ + email: 'invalid', + firstName: '', + lastName: '', + userPrincipalName: '', + }); + + await expect( + samlService.handleSamlLogin(mock(), 'post'), + ).rejects.toThrowError(new BadRequestError('Invalid email format')); + }); + + it('logs in user that has already completed onboarding', async () => { + const samlAttributes = { + email: 'foo@bar.com', + firstName: '', + lastName: '', + userPrincipalName: 'foo@bar.com', + }; + const mockUser = { + id: '123', + email: samlAttributes.email, + authIdentities: [ + { providerType: 'saml', providerId: samlAttributes.userPrincipalName } as any, + ], + } as any; + jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue(samlAttributes); + jest.spyOn(userRepository, 'findOne').mockResolvedValue(mockUser); + + const loginResult = await samlService.handleSamlLogin(mock(), 'post'); + + expect(loginResult).toEqual({ + authenticatedUser: mockUser, + attributes: samlAttributes, + onboardingRequired: false, + }); + }); + + it('provisions instance and project role for onboarded user', async () => { + const samlAttributes = { + email: 'foo@bar.com', + firstName: '', + lastName: '', + userPrincipalName: 'foo@bar.com', + n8nInstanceRole: 'global:admin', + n8nProjectRoles: ['rgjhURvl0rnEQL3v:viewer', 'ussa2R6P7aDtuRaZ:viewer'], + }; + const mockUser = { + id: '123', + email: samlAttributes.email, + authIdentities: [ + { providerType: 'saml', providerId: samlAttributes.userPrincipalName } as any, + ], + } as any; + jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue(samlAttributes); + jest.spyOn(userRepository, 'findOne').mockResolvedValue(mockUser); + + await samlService.handleSamlLogin(mock(), 'post'); + + expect(provisioningService.provisionInstanceRoleForUser).toHaveBeenCalledWith( + mockUser, + samlAttributes.n8nInstanceRole, + ); + expect(provisioningService.provisionProjectRolesForUser).toHaveBeenCalledWith( + mockUser.id, + samlAttributes.n8nProjectRoles, + ); + }); + }); + describe('loadFromDbAndApplySamlPreferences', () => { test('does throw `InvalidSamlMetadataError` when no valid SAML metadata could have been loaded', async () => { // ARRANGE diff --git a/packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts b/packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts index faf68c91926..87ae313b71b 100644 --- a/packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts +++ b/packages/cli/src/sso.ee/saml/routes/__tests__/saml.controller.ee.test.ts @@ -19,20 +19,12 @@ jest.mock('../../saml-helpers', () => ({ })); import { isConnectionTestRequest, isSamlLicensedAndEnabled } from '../../saml-helpers'; -import { type ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee'; const authService = mock(); const samlService = mock(); const urlService = mock(); const eventService = mock(); -const provisioningService = mock(); -const controller = new SamlController( - authService, - samlService, - urlService, - eventService, - provisioningService, -); +const controller = new SamlController(authService, samlService, urlService, eventService); const user = mock({ id: '123', @@ -46,7 +38,6 @@ const attributes: SamlUserAttributes = { firstName: 'Test', lastName: 'User', userPrincipalName: 'upn:test@example.com', - n8nInstanceRole: 'n8n_instance_role', }; describe('Test views', () => { diff --git a/packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts index 450ebecbf4d..f679a2b5ff1 100644 --- a/packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso.ee/saml/routes/saml.controller.ee.ts @@ -26,7 +26,6 @@ import { } from '../service-provider.ee'; import type { SamlLoginBinding } from '../types'; import { getInitSSOFormView } from '../views/init-sso-post'; -import { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee'; @RestController('/sso/saml') export class SamlController { @@ -35,7 +34,6 @@ export class SamlController { private readonly samlService: SamlService, private readonly urlService: UrlService, private readonly eventService: EventService, - private readonly provisioningService: ProvisioningService, ) {} @Get('/metadata', { skipAuth: true }) @@ -131,16 +129,6 @@ export class SamlController { if (isSamlLicensedAndEnabled()) { this.authService.issueCookie(res, loginResult.authenticatedUser, true, req.browserId); - const isRoleProvisioningEnabled = - await this.provisioningService.isInstanceRoleProvisioningEnabled(); - - if (isRoleProvisioningEnabled && loginResult.attributes.n8nInstanceRole) { - await this.provisioningService.provisionInstanceRoleForUser( - loginResult.authenticatedUser, - loginResult.attributes.n8nInstanceRole, - ); - } - if (loginResult.onboardingRequired) { return res.redirect(this.urlService.getInstanceBaseUrl() + '/saml/onboarding'); } else { diff --git a/packages/cli/src/sso.ee/saml/saml-helpers.ts b/packages/cli/src/sso.ee/saml/saml-helpers.ts index 3c03d49fddc..20315b221db 100644 --- a/packages/cli/src/sso.ee/saml/saml-helpers.ts +++ b/packages/cli/src/sso.ee/saml/saml-helpers.ts @@ -136,6 +136,10 @@ type GetMappedSamlReturn = { export function getMappedSamlAttributesFromFlowResult( flowResult: FlowResult, attributeMapping: SamlAttributeMapping, + jitClaimNames: { + instanceRole: string | null; + projectRoles: string | null; + }, ): GetMappedSamlReturn { const result: GetMappedSamlReturn = { attributes: undefined, @@ -144,21 +148,28 @@ export function getMappedSamlAttributesFromFlowResult( // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access if (flowResult?.extract?.attributes) { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const attributes = flowResult.extract.attributes as { [key: string]: string }; + const attributes = flowResult.extract.attributes as { [key: string]: string | string[] }; // TODO:SAML: fetch mapped attributes from flowResult.extract.attributes and create or login user - const email = attributes[attributeMapping.email]; - const firstName = attributes[attributeMapping.firstName]; - const lastName = attributes[attributeMapping.lastName]; - const userPrincipalName = attributes[attributeMapping.userPrincipalName]; - const n8nInstanceRole = attributes[attributeMapping.n8nInstanceRole]; + const email = attributes[attributeMapping.email] as string; + const firstName = attributes[attributeMapping.firstName] as string; + const lastName = attributes[attributeMapping.lastName] as string; + const userPrincipalName = attributes[attributeMapping.userPrincipalName] as string; result.attributes = { email, firstName, lastName, userPrincipalName, - n8nInstanceRole, }; + if (jitClaimNames.instanceRole && typeof attributes[jitClaimNames.instanceRole] === 'string') { + result.attributes.n8nInstanceRole = attributes[jitClaimNames.instanceRole] as string; + } + if (jitClaimNames.projectRoles && attributes[jitClaimNames.projectRoles]) { + const projectRolesFromFlowResult = attributes[jitClaimNames.projectRoles]; + result.attributes.n8nProjectRoles = Array.isArray(projectRolesFromFlowResult) + ? projectRolesFromFlowResult + : [projectRolesFromFlowResult]; + } if (!email) result.missingAttributes.push(attributeMapping.email); if (!userPrincipalName) result.missingAttributes.push(attributeMapping.userPrincipalName); if (!firstName) result.missingAttributes.push(attributeMapping.firstName); 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 9ce94093454..c689c4c3525 100644 --- a/packages/cli/src/sso.ee/saml/saml.service.ee.ts +++ b/packages/cli/src/sso.ee/saml/saml.service.ee.ts @@ -1,4 +1,4 @@ -import type { ProvisioningConfigDto, SamlPreferences } from '@n8n/api-types'; +import type { SamlPreferences, SamlPreferencesAttributeMapping } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { GlobalConfig } from '@n8n/config'; import type { Settings, User } from '@n8n/db'; @@ -32,7 +32,7 @@ import { isSsoJustInTimeProvisioningEnabled, reloadAuthenticationMethod } from ' import { AuthError } from '@/errors/response-errors/auth.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { PROVISIONING_PREFERENCES_DB_KEY } from '@/modules/provisioning.ee/constants'; +import { ProvisioningService } from '@/modules/provisioning.ee/provisioning.service.ee'; import { UrlService } from '@/services/url.service'; @Service() @@ -48,8 +48,6 @@ export class SamlService { 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', - // this value is loaded on init from the provisioning config - n8nInstanceRole: '', }, metadata: '', metadataUrl: '', @@ -86,6 +84,7 @@ export class SamlService { private readonly userRepository: UserRepository, private readonly settingsRepository: SettingsRepository, private readonly instanceSettings: InstanceSettings, + private readonly provisioningService: ProvisioningService, ) {} async init(): Promise { @@ -220,6 +219,7 @@ export class SamlService { (e) => e.providerType === 'saml' && e.providerId === attributes.userPrincipalName, ) ) { + await this.applySsoProvisioning(user, attributes); return { authenticatedUser: user, attributes, @@ -229,6 +229,7 @@ export class SamlService { // Login path for existing users that are NOT fully set up for SAML const updatedUser = await updateUserFromSamlAttributes(user, attributes); const onboardingRequired = !updatedUser.firstName || !updatedUser.lastName; + await this.applySsoProvisioning(updatedUser, attributes); return { authenticatedUser: updatedUser, attributes, @@ -239,6 +240,7 @@ export class SamlService { // New users to be created JIT based on SAML attributes if (isSsoJustInTimeProvisioningEnabled()) { const newUser = await createUserFromSamlAttributes(attributes); + await this.applySsoProvisioning(newUser, attributes); return { authenticatedUser: newUser, attributes, @@ -255,6 +257,18 @@ export class SamlService { }; } + private async applySsoProvisioning(user: User, attributes: SamlPreferencesAttributeMapping) { + if (attributes?.n8nInstanceRole) { + await this.provisioningService.provisionInstanceRoleForUser(user, attributes.n8nInstanceRole); + } + if (attributes?.n8nProjectRoles) { + await this.provisioningService.provisionProjectRolesForUser( + user.id, + attributes.n8nProjectRoles, + ); + } + } + private async broadcastReloadSAMLConfigurationCommand(): Promise { if (this.instanceSettings.isMultiMain) { const { Publisher } = await import('@/scaling/pubsub/publisher.service'); @@ -388,18 +402,8 @@ export class SamlService { const samlPreferences = await this.settingsRepository.findOne({ where: { key: SAML_PREFERENCES_DB_KEY }, }); - const provisioningConfigObject = await this.settingsRepository.findOne({ - where: { key: PROVISIONING_PREFERENCES_DB_KEY }, - }); if (samlPreferences) { const prefs = jsonParse(samlPreferences.value); - const provisioningConfig = jsonParse( - provisioningConfigObject?.value ?? '{}', - ); - - if (prefs && prefs.mapping) { - prefs.mapping.n8nInstanceRole = provisioningConfig.scopesInstanceRoleClaimName; - } if (prefs) { if (apply) { @@ -500,6 +504,10 @@ export class SamlService { const { attributes, missingAttributes } = getMappedSamlAttributesFromFlowResult( parsedSamlResponse, this._samlPreferences.mapping, + { + instanceRole: await this.provisioningService.getInstanceRoleClaimName(), + projectRoles: await this.provisioningService.getProjectsRolesClaimName(), + }, ); if (!attributes) { throw new AuthError('SAML Authentication failed. Invalid SAML response.'); diff --git a/packages/cli/src/sso.ee/saml/types.ts b/packages/cli/src/sso.ee/saml/types.ts index 35687777b1a..6c9fa97f426 100644 --- a/packages/cli/src/sso.ee/saml/types.ts +++ b/packages/cli/src/sso.ee/saml/types.ts @@ -1,5 +1,5 @@ -import type { SamlPreferences } from '@n8n/api-types'; +import type { SamlPreferences, SamlPreferencesAttributeMapping } from '@n8n/api-types'; export type SamlLoginBinding = SamlPreferences['loginBinding']; -export type SamlAttributeMapping = NonNullable; +export type SamlAttributeMapping = NonNullable; export type SamlUserAttributes = SamlAttributeMapping; diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index adaa3355594..4f425b78c40 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -2428,13 +2428,8 @@ "settings.provisioning.scopesProjectsRolesClaimName": "Projects Roles Claim Name", "settings.provisioning.scopesProjectsRolesClaimName.placeholder": "Enter projects roles claim name", "settings.provisioning.scopesProjectsRolesClaimName.help": "The claim name used to provision projects and their roles from Oauth. For SAML / LDAP, this will be the attribute name checked.", - "settings.provisioning.scopesProvisionInstanceRole": "Provision Instance Role", - "settings.provisioning.scopesProvisionInstanceRole.help": "Enable automatic provisioning of instance roles", - "settings.provisioning.scopesProvisionProjectRoles": "Provision Project Roles", - "settings.provisioning.scopesProvisionProjectRoles.help": "Enable automatic provisioning of projects and project roles", - "settings.provisioning.scopesProvisioningFrequency": "Provisioning Frequency", - "settings.provisioning.scopesProvisioningFrequency.placeholder": "Select provisioning frequency", - "settings.provisioning.scopesProvisioningFrequency.help": "How often to provision projects and roles", + "settings.provisioning.toggle": "Provision instance and project roles", + "settings.provisioning.toggle.help": "Project access can only be defined on external provider. Any existing project access configured in n8n, but not on the provider, will be removed once a user logs in.", "settings.externalSecrets.title": "External Secrets", "settings.externalSecrets.info": "Connect external secrets tools for centralized credentials management across environments, and to enhance system security.", "settings.externalSecrets.info.link": "More info", diff --git a/packages/frontend/@n8n/rest-api-client/src/api/provisioning.ts b/packages/frontend/@n8n/rest-api-client/src/api/provisioning.ts index 9d7a59b5376..46542608e06 100644 --- a/packages/frontend/@n8n/rest-api-client/src/api/provisioning.ts +++ b/packages/frontend/@n8n/rest-api-client/src/api/provisioning.ts @@ -7,7 +7,6 @@ export interface ProvisioningConfig { scopesProjectsRolesClaimName: string; scopesProvisionInstanceRole: boolean; scopesProvisionProjectRoles: boolean; - scopesProvisioningFrequency: string; } export const getProvisioningConfig = async ( diff --git a/packages/frontend/editor-ui/src/features/settings/provisioning/views/SettingsProvisioningView.vue b/packages/frontend/editor-ui/src/features/settings/provisioning/views/SettingsProvisioningView.vue index a7188f2524b..bbb17837a50 100644 --- a/packages/frontend/editor-ui/src/features/settings/provisioning/views/SettingsProvisioningView.vue +++ b/packages/frontend/editor-ui/src/features/settings/provisioning/views/SettingsProvisioningView.vue @@ -7,15 +7,7 @@ import { useProvisioningStore } from '../provisioning.store'; import { useSettingsStore } from '@/app/stores/settings.store'; import { useRouter } from 'vue-router'; import { VIEWS } from '@/app/constants'; -import { - N8nHeading, - N8nText, - N8nSpinner, - N8nInput, - N8nButton, - N8nSelect, - N8nOption, -} from '@n8n/design-system'; +import { N8nHeading, N8nText, N8nSpinner, N8nInput, N8nButton } from '@n8n/design-system'; import { type ProvisioningConfig } from '@n8n/rest-api-client'; const i18n = useI18n(); @@ -53,30 +45,22 @@ const form = reactive({ scopesName: '', scopesInstanceRoleClaimName: '', scopesProjectsRolesClaimName: '', - scopesProvisionInstanceRole: false, - scopesProvisionProjectRoles: false, - scopesProvisioningFrequency: 'never', + provisioningEnabled: false, }); -// Frequency options -const frequencyOptions = [ - { label: 'Never', value: 'never' }, - { label: 'First Login', value: 'first_login' }, - { label: 'Every Login', value: 'every_login' }, -]; - const isFormDirty = computed(() => { - const cfg = provisioningStore.provisioningConfig; - if (!cfg) return false; - const keys: Array = [ + const config = provisioningStore.provisioningConfig; + if (!config) return false; + const formKeysThatMatchWithConfig: Array = [ 'scopesName', 'scopesInstanceRoleClaimName', 'scopesProjectsRolesClaimName', - 'scopesProvisionInstanceRole', - 'scopesProvisionProjectRoles', - 'scopesProvisioningFrequency', ]; - return keys.some((key) => form[key] !== cfg[key]); + const configChanged = formKeysThatMatchWithConfig.some((key) => form[key] !== config[key]); + const provisioningEnabledChanged = + form.provisioningEnabled !== + (config.scopesProvisionInstanceRole && config.scopesProvisionProjectRoles); + return configChanged || provisioningEnabledChanged; }); const loadFormData = () => { @@ -86,16 +70,19 @@ const loadFormData = () => { scopesName: cfg.scopesName || '', scopesInstanceRoleClaimName: cfg.scopesInstanceRoleClaimName || '', scopesProjectsRolesClaimName: cfg.scopesProjectsRolesClaimName || '', - scopesProvisionInstanceRole: cfg.scopesProvisionInstanceRole ?? false, - scopesProvisionProjectRoles: cfg.scopesProvisionProjectRoles ?? false, - scopesProvisioningFrequency: cfg.scopesProvisioningFrequency || 'never', }); + form.provisioningEnabled = cfg.scopesProvisionInstanceRole; }; const onSave = async () => { saving.value = true; try { - await provisioningStore.saveProvisioningConfig({ ...form }); + const { provisioningEnabled, ...dataToSave } = form; + await provisioningStore.saveProvisioningConfig({ + ...dataToSave, + scopesProvisionInstanceRole: provisioningEnabled, + scopesProvisionProjectRoles: provisioningEnabled, + }); await provisioningStore.getProvisioningConfig(); loadFormData(); @@ -115,7 +102,7 @@ const onSave = async () => {