From fa4c979945faf6c836d74198e82fbcb1780e0af9 Mon Sep 17 00:00:00 2001 From: Konstantin Tieber <46342664+konstantintieber@users.noreply.github.com> Date: Thu, 23 Oct 2025 17:24:07 +0200 Subject: [PATCH] feat: Provision project roles from OIDC SSO (#21107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Stephen Co-authored-by: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Co-authored-by: Juuso Tapaninen Co-authored-by: Claude Co-authored-by: Iván Ovejero --- .../__tests__/provisioning.service.ee.test.ts | 86 ++++++++----------- .../provisioning.service.ee.ts | 55 ++++++++---- .../cli/src/sso.ee/oidc/oidc.service.ee.ts | 32 ++++--- 3 files changed, 91 insertions(+), 82 deletions(-) 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 f6aca3842e6..5c37db168fe 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 @@ -270,21 +270,21 @@ describe('ProvisioningService', () => { }); describe('provisionProjectRolesForUser', () => { - it('should do nothing if the projectIdToRole is not a string', async () => { + it('should do nothing if the projectIdToRole is not an array', async () => { const userId = 'user-id-123'; - const projectIdToRole = { not: 'a string' }; + const projectIdToRole = { not: 'an array' }; await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledTimes(1); expect(logger.warn).toHaveBeenCalledWith( - 'Skipping project role provisioning. Invalid projectIdToRole type: expected string, received object', - { userId, projectIdToRole }, + 'Skipping project role provisioning. Invalid projectIdToRole type: expected array, received object', + { userId, projectIdToRoles: projectIdToRole }, ); }); - it('should do nothing if projectIdToRole is not a valid JSON string', async () => { + it('should do nothing if projectIdToRole is not an array', async () => { const userId = 'user-id-123'; const projectIdToRole = 'invalid-json-string'; @@ -293,30 +293,28 @@ describe('ProvisioningService', () => { expect(projectService.addUser).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledTimes(1); expect(logger.warn).toHaveBeenCalledWith( - 'Skipping project role provisioning. Failed to parse project to role mapping.', - { userId, projectIdToRole }, + 'Skipping project role provisioning. Invalid projectIdToRole type: expected array, received string', + { userId, projectIdToRoles: projectIdToRole }, ); }); it('should filter out entries where key:value is not a string', async () => { const userId = 'user-id-123'; - const projectIdToRole = JSON.stringify({ project_1: { nested: 'object' } }); // invalid value type + const projectIdToRole = [{ projectId: 'project-1', role: 'viewer' }]; // invalid value type await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledTimes(1); expect(logger.warn).toHaveBeenCalledWith( - 'Skipping project role mapping for project_1:[object Object]. Invalid types: expected both key and value to be strings.', - { userId }, + 'Skipping invalid project role mapping entry. Expected string, received object.', + { userId, entry: projectIdToRole[0] }, ); }); it('should do nothing if the project does not exist', async () => { const userId = 'user-id-123'; - const projectIdToRole = JSON.stringify({ - 'non-existent-project': 'project:viewer', - }); + const projectIdToRole = ['non-existent-project:viewer']; projectRepository.find.mockResolvedValue([]); roleRepository.find.mockResolvedValue([mock({ slug: 'project:viewer' })]); @@ -327,9 +325,7 @@ describe('ProvisioningService', () => { it('should do nothing if the provided role does not exist', async () => { const userId = 'user-id-123'; - const projectIdToRole = JSON.stringify({ - 'project-1': 'project:non-existent-role', - }); + const projectIdToRole = ['project-1:non-existent-role']; projectRepository.find.mockResolvedValue([mock({ id: 'project-1' })]); roleRepository.find.mockResolvedValue([]); @@ -340,43 +336,40 @@ describe('ProvisioningService', () => { it('should do nothing if no valid project to role mappings are found', async () => { const userId = 'user-id-123'; - const projectIdToRole = { - nonExistentProject: 'non-existent-role', - anotherNonExistentProject: 'project:viewer', - }; + const projectIdToRole = [ + 'nonExistentProject:non-existent-role', + 'anotherNonExistentProject:viewer', + ]; // Mock that no projects exist projectRepository.find.mockResolvedValueOnce([]); // Mock that roles exist but projects don't - roleRepository.find.mockResolvedValue([mock({ slug: 'project:viewer' })]); + roleRepository.find.mockResolvedValue([ + mock({ displayName: 'viewer', slug: 'project:viewer' }), + ]); - await provisioningService.provisionProjectRolesForUser( - userId, - JSON.stringify(projectIdToRole), - ); + await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); expect(projectService.addUser).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledWith( - 'Skipping project role provisioning altogether. No valid project to role mappings found.', + 'Skipped provisioning project role for project with ID nonExistentProject, because project does not exist or is a personal project.', { userId, - projectRoleMap: projectIdToRole, + projectId: 'nonExistentProject', + roleSlug: 'project:non-existent-role', }, ); }); it('should skip projectIds that reference a personal project', async () => { const userId = 'user-id-123'; - const projectIdToRole = JSON.stringify({ - personalProject1: 'project:viewer', - teamProject1: 'project:editor', - }); + const projectIdToRole = ['personalProject1:viewer', 'teamProject1:editor']; // Mocks query to find existing projects projectRepository.find.mockResolvedValueOnce([mock({ id: 'teamProject1' })]); // Mocks query to find currently accessible projects projectRepository.find.mockResolvedValueOnce([]); roleRepository.find.mockResolvedValue([ - mock({ slug: 'project:viewer' }), - mock({ slug: 'project:editor' }), + mock({ displayName: 'viewer', slug: 'project:viewer' }), + mock({ displayName: 'editor', slug: 'project:editor' }), ]); await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); @@ -388,17 +381,14 @@ describe('ProvisioningService', () => { entityManager, ); expect(logger.warn).toHaveBeenCalledWith( - 'Skipping project role provisioning. Project with ID personalProject1 not found.', + 'Skipped provisioning project role for project with ID personalProject1, because project does not exist or is a personal project.', { userId, projectId: 'personalProject1', roleSlug: 'project:viewer' }, ); }); it('should provision project roles for the user', async () => { const userId = 'user-id-123'; - const projectIdToRole = JSON.stringify({ - 'project-1': 'project:viewer', - 'project-2': 'project:editor', - }); + const projectIdToRole = ['project-1:viewer', 'project-2:editor']; // Mocks query to find existing projects projectRepository.find.mockResolvedValueOnce([ mock({ id: 'project-1' }), @@ -407,8 +397,8 @@ describe('ProvisioningService', () => { // Mocks query to find currently accessible projects projectRepository.find.mockResolvedValueOnce([]); roleRepository.find.mockResolvedValue([ - mock({ slug: 'project:viewer' }), - mock({ slug: 'project:editor' }), + mock({ displayName: 'viewer', slug: 'project:viewer' }), + mock({ displayName: 'editor', slug: 'project:editor' }), ]); await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); @@ -427,9 +417,7 @@ describe('ProvisioningService', () => { }); it('should filter out non-project roles', async () => { const userId = 'user-id-123'; - const projectIdToRole = JSON.stringify({ - project1: 'global:admin', - }); + const projectIdToRole = ['project1:admin']; projectRepository.find.mockResolvedValue([mock({ id: 'project1' })]); roleRepository.find.mockResolvedValue([]); @@ -437,16 +425,14 @@ describe('ProvisioningService', () => { expect(projectService.addUser).not.toHaveBeenCalled(); expect(logger.warn).toHaveBeenCalledWith( - 'Skipping project role provisioning. Role with slug global:admin not found or is not a project role.', - { userId, projectId: 'project1', roleSlug: 'global:admin' }, + 'Skipping project role provisioning for role with slug project:admin, because role does not exist or is not specific to projects.', + { userId, projectId: 'project1', roleSlug: 'project:admin' }, ); }); it('removes existing access to non-personal projects that are no longer present in the provided mapping', async () => { const userId = 'user-id-123'; - const projectIdToRole = JSON.stringify({ - 'project-1': 'project:viewer', - }); + const projectIdToRole = ['project-1:viewer']; // Mocks query to find existing projects projectRepository.find.mockResolvedValueOnce([mock({ id: 'project-1' })]); // Mocks query to find currently accessible projects @@ -454,7 +440,9 @@ describe('ProvisioningService', () => { mock({ id: 'project-1', type: 'team' }), mock({ id: 'project-2', type: 'team' }), ]); - roleRepository.find.mockResolvedValue([mock({ slug: 'project:viewer' })]); + roleRepository.find.mockResolvedValue([ + mock({ displayName: 'viewer', slug: 'project:viewer' }), + ]); await provisioningService.provisionProjectRolesForUser(userId, projectIdToRole); 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 cdf98574d71..cdb9d589b2d 100644 --- a/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts +++ b/packages/cli/src/modules/provisioning.ee/provisioning.service.ee.ts @@ -106,33 +106,51 @@ export class ProvisioningService { } /** - * @param projectIdToRole expected to be a JSON string for a map of project IDs to role slugs + * @param projectIdToRole expected to be an array of strings like this: + * [ + * ":", + * ":", + * ... + * ] */ - async provisionProjectRolesForUser(userId: string, projectIdToRole: unknown): Promise { - if (typeof projectIdToRole !== 'string') { + async provisionProjectRolesForUser(userId: string, projectIdToRoles: unknown): Promise { + if (!Array.isArray(projectIdToRoles)) { this.logger.warn( - `Skipping project role provisioning. Invalid projectIdToRole type: expected string, received ${typeof projectIdToRole}`, - { userId, projectIdToRole }, + `Skipping project role provisioning. Invalid projectIdToRole type: expected array, received ${typeof projectIdToRoles}`, + { userId, projectIdToRoles }, ); return; } let projectRoleMap: Record; try { - projectRoleMap = JSON.parse(projectIdToRole); - for (const [key, value] of Object.entries(projectRoleMap)) { - if (typeof key !== 'string' || typeof value !== 'string') { + projectRoleMap = {}; + for (const entry of projectIdToRoles) { + if (typeof entry !== 'string') { this.logger.warn( - `Skipping project role mapping for ${key}:${value}. Invalid types: expected both key and value to be strings.`, - { userId }, + `Skipping invalid project role mapping entry. Expected string, received ${typeof entry}.`, + { userId, entry }, ); - delete projectRoleMap[key]; + continue; } + + const [projectId, roleSlugSuffix] = entry.split(':'); + if (!projectId || !roleSlugSuffix) { + this.logger.warn( + `Skipping invalid project role mapping entry. Expected format "projectId:displayName", received "${entry}".`, + { userId, entry }, + ); + continue; + } + + // This means that if the external SSO setup has two roles configured + // for the same projectId, we only provision the one we see last in the sent list. + projectRoleMap[projectId] = `project:${roleSlugSuffix}`; } } catch (error) { this.logger.warn( 'Skipping project role provisioning. Failed to parse project to role mapping.', - { userId, projectIdToRole }, + { userId, projectIdToRoles }, ); return; } @@ -154,11 +172,10 @@ export class ProvisioningService { slug: In(roleSlugs), roleType: 'project', }, - select: ['slug'], + select: ['displayName', 'slug'], }), ]); const existingProjectIds = new Set(existingProjects.map((project) => project.id)); - const existingRoleSlugs = new Set(existingRoles.map((r) => r.slug)); const validProjectToRoleMappings: Array<{ projectId: string; roleSlug: string }> = []; @@ -166,21 +183,21 @@ export class ProvisioningService { for (const [projectId, roleSlug] of Object.entries(projectRoleMap)) { if (!existingProjectIds.has(projectId)) { this.logger.warn( - `Skipping project role provisioning. Project with ID ${projectId} not found.`, + `Skipped provisioning project role for project with ID ${projectId}, because project does not exist or is a personal project.`, { userId, projectId, roleSlug }, ); continue; } - - if (!existingRoleSlugs.has(roleSlug)) { + const role = existingRoles.find((role) => role.slug === roleSlug); + if (!role) { this.logger.warn( - `Skipping project role provisioning. Role with slug ${roleSlug} not found or is not a project role.`, + `Skipping project role provisioning for role with slug ${roleSlug}, because role does not exist or is not specific to projects.`, { userId, projectId, roleSlug }, ); continue; } - validProjectToRoleMappings.push({ projectId, roleSlug }); + validProjectToRoleMappings.push({ projectId, roleSlug: role.slug }); } if (validProjectToRoleMappings.length === 0) { 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 a314c8f4f59..4b2aa2e029b 100644 --- a/packages/cli/src/sso.ee/oidc/oidc.service.ee.ts +++ b/packages/cli/src/sso.ee/oidc/oidc.service.ee.ts @@ -245,11 +245,6 @@ export class OidcService { throw new BadRequestError('Invalid email format'); } - const provisioningConfig = await this.provisioningService.getConfig(); - const provisioningEnabled = - provisioningConfig.scopesProvisionInstanceRole || - provisioningConfig.scopesProvisionProjectRoles; - const openidUser = await this.authIdentityRepository.findOne({ where: { providerId: claims.sub, providerType: 'oidc' }, relations: { @@ -260,12 +255,7 @@ export class OidcService { }); if (openidUser) { - if (provisioningEnabled) { - await this.provisioningService.provisionInstanceRoleForUser( - openidUser.user, - claims.n8n_instance_role, - ); - } + await this.applySsoProvisioning(openidUser.user, claims); return openidUser.user; } @@ -312,14 +302,28 @@ export class OidcService { }), ); - if (provisioningEnabled) { - await this.provisioningService.provisionInstanceRoleForUser(user, claims.n8n_instance_role); - } + await this.applySsoProvisioning(user, claims); return user; }); } + 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], + ); + } + if (await this.provisioningService.isProjectRolesProvisioningEnabled()) { + await this.provisioningService.provisionProjectRolesForUser( + user.id, + claims[provisioningConfig.scopesProjectsRolesClaimName], + ); + } + } + private async broadcastReloadOIDCConfigurationCommand(): Promise { if (this.instanceSettings.isMultiMain) { const { Publisher } = await import('@/scaling/pubsub/publisher.service');