From 33a2d5de17d3b2e31ca3a66abc71283fb9fa4bb9 Mon Sep 17 00:00:00 2001 From: Andreas Fitzek Date: Wed, 17 Sep 2025 11:15:31 +0200 Subject: [PATCH] chore(core): Use dynamic role resolution for access control (#19400) --- .../shared-credentials.repository.ts | 6 +- packages/@n8n/permissions/src/index.ts | 2 +- .../__tests__/roles-with-scope.test.ts | 6 +- .../src/utilities/roles-with-scope.ee.ts | 37 -- .../utilities/static-roles-with-scope.ee.ts | 24 + .../credentials/credentials-finder.service.ts | 31 +- .../src/credentials/credentials.service.ee.ts | 7 +- .../src/credentials/credentials.service.ts | 8 +- .../__tests__/check-access.test.ts | 454 +++++++++++---- .../cli/src/permissions.ee/check-access.ts | 15 +- .../credentials-finder.service.test.ts | 521 +++++++++++++++++- .../__tests__/role-cache.service.test.ts | 298 ++++++++++ .../role.service.rolesWithScope.test.ts | 162 ++++++ .../cli/src/services/project.service.ee.ts | 3 +- .../cli/src/services/role-cache.service.ts | 115 ++++ packages/cli/src/services/role.service.ts | 32 +- .../src/workflows/workflow-finder.service.ts | 17 +- .../src/workflows/workflow-sharing.service.ts | 9 +- .../cli/src/workflows/workflows.controller.ts | 4 +- .../credentials/credentials.api.ee.test.ts | 5 + .../cli/test/integration/shared/db/roles.ts | 26 + 21 files changed, 1581 insertions(+), 201 deletions(-) delete mode 100644 packages/@n8n/permissions/src/utilities/roles-with-scope.ee.ts create mode 100644 packages/@n8n/permissions/src/utilities/static-roles-with-scope.ee.ts create mode 100644 packages/cli/src/services/__tests__/role-cache.service.test.ts create mode 100644 packages/cli/src/services/__tests__/role.service.rolesWithScope.test.ts create mode 100644 packages/cli/src/services/role-cache.service.ts diff --git a/packages/@n8n/db/src/repositories/shared-credentials.repository.ts b/packages/@n8n/db/src/repositories/shared-credentials.repository.ts index 75a321ca8b..9dc1dd88f1 100644 --- a/packages/@n8n/db/src/repositories/shared-credentials.repository.ts +++ b/packages/@n8n/db/src/repositories/shared-credentials.repository.ts @@ -1,5 +1,5 @@ import { Service } from '@n8n/di'; -import type { CredentialSharingRole, ProjectRole } from '@n8n/permissions'; +import type { CredentialSharingRole } from '@n8n/permissions'; import type { EntityManager, FindOptionsWhere } from '@n8n/typeorm'; import { DataSource, In, Not, Repository } from '@n8n/typeorm'; @@ -108,8 +108,8 @@ export class SharedCredentialsRepository extends Repository { async findCredentialsByRoles( userIds: string[], - projectRoles: ProjectRole[], - credentialRoles: CredentialSharingRole[], + projectRoles: string[], + credentialRoles: string[], trx?: EntityManager, ) { trx = trx ?? this.manager; diff --git a/packages/@n8n/permissions/src/index.ts b/packages/@n8n/permissions/src/index.ts index 2bc9f45640..64743f7fa7 100644 --- a/packages/@n8n/permissions/src/index.ts +++ b/packages/@n8n/permissions/src/index.ts @@ -20,7 +20,7 @@ export { export { hasScope } from './utilities/has-scope.ee'; export { hasGlobalScope } from './utilities/has-global-scope.ee'; export { combineScopes } from './utilities/combine-scopes.ee'; -export { rolesWithScope } from './utilities/roles-with-scope.ee'; +export { staticRolesWithScope } from './utilities/static-roles-with-scope.ee'; export { getGlobalScopes } from './utilities/get-global-scopes.ee'; export { getRoleScopes, getAuthPrincipalScopes } from './utilities/get-role-scopes.ee'; export { getResourcePermissions } from './utilities/get-resource-permissions.ee'; diff --git a/packages/@n8n/permissions/src/utilities/__tests__/roles-with-scope.test.ts b/packages/@n8n/permissions/src/utilities/__tests__/roles-with-scope.test.ts index e4d20159f3..62df2558b3 100644 --- a/packages/@n8n/permissions/src/utilities/__tests__/roles-with-scope.test.ts +++ b/packages/@n8n/permissions/src/utilities/__tests__/roles-with-scope.test.ts @@ -1,5 +1,5 @@ import type { GlobalRole, Scope } from '../../types.ee'; -import { rolesWithScope } from '../roles-with-scope.ee'; +import { staticRolesWithScope } from '../static-roles-with-scope.ee'; describe('rolesWithScope', () => { describe('global roles', () => { @@ -8,14 +8,14 @@ describe('rolesWithScope', () => { ['user:list', ['global:owner', 'global:admin', 'global:member']], ['invalid:scope', []], ] as Array<[Scope, GlobalRole[]]>)('%s -> %s', (scope, expected) => { - expect(rolesWithScope('global', scope)).toEqual(expected); + expect(staticRolesWithScope('global', scope)).toEqual(expected); }); }); describe('multiple scopes', () => { test('returns roles with all scopes', () => { expect( - rolesWithScope('global', [ + staticRolesWithScope('global', [ // all global roles have this scope 'tag:create', // only owner and admin have this scope diff --git a/packages/@n8n/permissions/src/utilities/roles-with-scope.ee.ts b/packages/@n8n/permissions/src/utilities/roles-with-scope.ee.ts deleted file mode 100644 index 1022d10adb..0000000000 --- a/packages/@n8n/permissions/src/utilities/roles-with-scope.ee.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { ALL_ROLE_MAPS } from '../roles/role-maps.ee'; -import type { - CredentialSharingRole, - GlobalRole, - ProjectRole, - RoleNamespace, - Scope, - WorkflowSharingRole, -} from '../types.ee'; - -/** - * Retrieves roles within a specific namespace that have all the given scopes. - * @param namespace - The role namespace to search in - * @param scopes - Scope(s) to filter by - */ -export function rolesWithScope(namespace: 'global', scopes: Scope | Scope[]): GlobalRole[]; -export function rolesWithScope(namespace: 'project', scopes: Scope | Scope[]): ProjectRole[]; -export function rolesWithScope( - namespace: 'credential', - scopes: Scope | Scope[], -): CredentialSharingRole[]; -export function rolesWithScope( - namespace: 'workflow', - scopes: Scope | Scope[], -): WorkflowSharingRole[]; -export function rolesWithScope(namespace: RoleNamespace, scopes: Scope | Scope[]) { - if (!Array.isArray(scopes)) { - scopes = [scopes]; - } - - return Object.keys(ALL_ROLE_MAPS[namespace]).filter((k) => { - return scopes.every((s) => - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access - ((ALL_ROLE_MAPS[namespace] as any)[k] as Scope[]).includes(s), - ); - }); -} diff --git a/packages/@n8n/permissions/src/utilities/static-roles-with-scope.ee.ts b/packages/@n8n/permissions/src/utilities/static-roles-with-scope.ee.ts new file mode 100644 index 0000000000..b7ea32fe6a --- /dev/null +++ b/packages/@n8n/permissions/src/utilities/static-roles-with-scope.ee.ts @@ -0,0 +1,24 @@ +import { ALL_ROLE_MAPS } from '../roles/role-maps.ee'; +import type { RoleNamespace, Scope } from '../types.ee'; + +/** + * Retrieves roles within a specific namespace that have all the given scopes. + * + * This is only valid for static roles defined in ALL_ROLE_MAPS, with custom roles + * being handled in the RoleService. + * + * @param namespace - The role namespace to search in + * @param scopes - Scope(s) to filter by + */ +export function staticRolesWithScope(namespace: RoleNamespace, scopes: Scope | Scope[]) { + if (!Array.isArray(scopes)) { + scopes = [scopes]; + } + + return Object.keys(ALL_ROLE_MAPS[namespace]).filter((k) => { + return scopes.every((s) => + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + ((ALL_ROLE_MAPS[namespace] as any)[k] as Scope[]).includes(s), + ); + }); +} diff --git a/packages/cli/src/credentials/credentials-finder.service.ts b/packages/cli/src/credentials/credentials-finder.service.ts index 82db5c4d42..f0ad42ad93 100644 --- a/packages/cli/src/credentials/credentials-finder.service.ts +++ b/packages/cli/src/credentials/credentials-finder.service.ts @@ -1,18 +1,21 @@ import type { CredentialsEntity, SharedCredentials, User } from '@n8n/db'; import { CredentialsRepository, SharedCredentialsRepository } from '@n8n/db'; import { Service } from '@n8n/di'; -import { hasGlobalScope, rolesWithScope } from '@n8n/permissions'; +import { hasGlobalScope } from '@n8n/permissions'; import type { CredentialSharingRole, ProjectRole, Scope } from '@n8n/permissions'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import type { EntityManager, FindOptionsWhere } from '@n8n/typeorm'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; +import { RoleService } from '@/services/role.service'; + @Service() export class CredentialsFinderService { constructor( private readonly sharedCredentialsRepository: SharedCredentialsRepository, private readonly credentialsRepository: CredentialsRepository, + private readonly roleService: RoleService, ) {} /** @@ -26,8 +29,10 @@ export class CredentialsFinderService { let where: FindOptionsWhere = {}; if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { - const projectRoles = rolesWithScope('project', scopes); - const credentialRoles = rolesWithScope('credential', scopes); + const [projectRoles, credentialRoles] = await Promise.all([ + this.roleService.rolesWithScope('project', scopes), + this.roleService.rolesWithScope('credential', scopes), + ]); where = { ...where, shared: { @@ -50,8 +55,10 @@ export class CredentialsFinderService { let where: FindOptionsWhere = { credentialsId }; if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { - const projectRoles = rolesWithScope('project', scopes); - const credentialRoles = rolesWithScope('credential', scopes); + const [projectRoles, credentialRoles] = await Promise.all([ + this.roleService.rolesWithScope('project', scopes), + this.roleService.rolesWithScope('credential', scopes), + ]); where = { ...where, role: In(credentialRoles), @@ -82,8 +89,10 @@ export class CredentialsFinderService { let where: FindOptionsWhere = {}; if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { - const projectRoles = rolesWithScope('project', scopes); - const credentialRoles = rolesWithScope('credential', scopes); + const [projectRoles, credentialRoles] = await Promise.all([ + this.roleService.rolesWithScope('project', scopes), + this.roleService.rolesWithScope('credential', scopes), + ]); where = { role: In(credentialRoles), project: { @@ -111,9 +120,13 @@ export class CredentialsFinderService { trx?: EntityManager, ) { const projectRoles = - 'scopes' in options ? rolesWithScope('project', options.scopes) : options.projectRoles; + 'scopes' in options + ? await this.roleService.rolesWithScope('project', options.scopes) + : options.projectRoles; const credentialRoles = - 'scopes' in options ? rolesWithScope('credential', options.scopes) : options.credentialRoles; + 'scopes' in options + ? await this.roleService.rolesWithScope('credential', options.scopes) + : options.credentialRoles; const sharings = await this.sharedCredentialsRepository.findCredentialsByRoles( userIds, diff --git a/packages/cli/src/credentials/credentials.service.ee.ts b/packages/cli/src/credentials/credentials.service.ee.ts index e9ff6d49d4..e3452eccc1 100644 --- a/packages/cli/src/credentials/credentials.service.ee.ts +++ b/packages/cli/src/credentials/credentials.service.ee.ts @@ -1,7 +1,7 @@ import type { CredentialsEntity, User } from '@n8n/db'; import { Project, SharedCredentials, SharedCredentialsRepository } from '@n8n/db'; import { Service } from '@n8n/di'; -import { hasGlobalScope, rolesWithScope } from '@n8n/permissions'; +import { hasGlobalScope } from '@n8n/permissions'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In, type EntityManager } from '@n8n/typeorm'; import type { ICredentialDataDecryptedObject } from 'n8n-workflow'; @@ -10,6 +10,7 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { TransferCredentialError } from '@/errors/response-errors/transfer-credential.error'; import { OwnershipService } from '@/services/ownership.service'; import { ProjectService } from '@/services/project.service.ee'; +import { RoleService } from '@/services/role.service'; import { CredentialsFinderService } from './credentials-finder.service'; import { CredentialsService } from './credentials.service'; @@ -22,6 +23,7 @@ export class EnterpriseCredentialsService { private readonly credentialsService: CredentialsService, private readonly projectService: ProjectService, private readonly credentialsFinderService: CredentialsFinderService, + private readonly roleService: RoleService, ) {} async shareWithProjects( @@ -31,6 +33,7 @@ export class EnterpriseCredentialsService { entityManager?: EntityManager, ) { const em = entityManager ?? this.sharedCredentialsRepository.manager; + const roles = await this.roleService.rolesWithScope('project', ['project:list']); let projects = await em.find(Project, { where: [ @@ -44,7 +47,7 @@ export class EnterpriseCredentialsService { : { projectRelations: { userId: user.id, - role: In(rolesWithScope('project', 'project:list')), + role: In(roles), }, }), }, diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index 1f01c2f5ce..49ebda1088 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -409,10 +409,6 @@ export class CredentialsService { const { manager: dbManager } = this.credentialsRepository; const result = await dbManager.transaction(async (transactionManager) => { - const savedCredential = await transactionManager.save(newCredential); - - savedCredential.data = newCredential.data; - const project = projectId === undefined ? await this.projectRepository.getPersonalProjectForUserOrFail( @@ -437,6 +433,10 @@ export class CredentialsService { throw new UnexpectedError('No personal project found'); } + const savedCredential = await transactionManager.save(newCredential); + + savedCredential.data = newCredential.data; + const newSharedCredential = this.sharedCredentialsRepository.create({ role: 'credential:owner', credentials: savedCredential, diff --git a/packages/cli/src/permissions.ee/__tests__/check-access.test.ts b/packages/cli/src/permissions.ee/__tests__/check-access.test.ts index 5eb14e023f..c2533ccb05 100644 --- a/packages/cli/src/permissions.ee/__tests__/check-access.test.ts +++ b/packages/cli/src/permissions.ee/__tests__/check-access.test.ts @@ -6,18 +6,25 @@ import { type User, } from '@n8n/db'; import { Container } from '@n8n/di'; -import type { Scope } from '@n8n/permissions'; +import { type Scope } from '@n8n/permissions'; import { mock } from 'jest-mock-extended'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { RoleService } from '@/services/role.service'; import { userHasScopes } from '../check-access'; describe('userHasScopes', () => { - const findByWorkflowMock = jest.fn(); - const findByCredentialMock = jest.fn(); + let findByWorkflowMock: jest.Mock; + let findByCredentialMock: jest.Mock; + let roleServiceMock: jest.Mock; + let mockQueryBuilder: any; beforeAll(() => { + findByWorkflowMock = jest.fn(); + findByCredentialMock = jest.fn(); + roleServiceMock = jest.fn(); + Container.set( SharedWorkflowRepository, mock({ @@ -32,7 +39,7 @@ describe('userHasScopes', () => { }), ); - const mockQueryBuilder = { + mockQueryBuilder = { innerJoin: jest.fn().mockReturnThis(), where: jest.fn().mockReturnThis(), andWhere: jest.fn().mockReturnThis(), @@ -48,117 +55,358 @@ describe('userHasScopes', () => { createQueryBuilder: jest.fn().mockReturnValue(mockQueryBuilder), }), ); + + Container.set( + RoleService, + mock({ + rolesWithScope: roleServiceMock, + }), + ); }); beforeEach(() => { + jest.clearAllMocks(); findByWorkflowMock.mockReset(); findByCredentialMock.mockReset(); + roleServiceMock.mockReset(); + + // Default mock responses + mockQueryBuilder.getRawMany.mockResolvedValue([{ id: 'projectId' }]); }); - it.each<{ type: 'workflow' | 'credential'; id: string }>([ - { - type: 'workflow', - id: 'workflowId', - }, - { - type: 'credential', - id: 'credentialId', - }, - ])('should return 404 if the resource is not found', async ({ type, id }) => { - findByWorkflowMock.mockResolvedValueOnce([]); - findByCredentialMock.mockResolvedValueOnce([]); + describe('resource not found scenarios', () => { + it.each<{ type: 'workflow' | 'credential'; id: string }>([ + { type: 'workflow', id: 'workflowId' }, + { type: 'credential', id: 'credentialId' }, + ])('should throw NotFoundError if $type is not found', async ({ type, id }) => { + findByWorkflowMock.mockResolvedValue([]); + findByCredentialMock.mockResolvedValue([]); - const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; - const scopes = ['workflow:read', 'credential:read'] as Scope[]; + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['workflow:read', 'credential:read'] as Scope[]; - const params: { credentialId?: string; workflowId?: string; projectId?: string } = { - projectId: 'projectId', - }; - if (type === 'credential') { - params.credentialId = id; - } else { - params.workflowId = id; - } - await expect(userHasScopes(user, scopes, false, params)).rejects.toThrow(NotFoundError); - }); - - test.each<{ - type: 'workflow' | 'credential'; - id: string; - role: string; - scope: Scope; - userScopes: Scope[]; - expected: boolean; - }>([ - { - type: 'workflow', - id: 'workflowId', - role: 'workflow:member', - scope: 'workflow:delete', - userScopes: [], - expected: false, - }, - { - type: 'credential', - id: 'credentialId', - role: 'credential:member', - scope: 'credential:delete', - userScopes: [], - expected: false, - }, - { - type: 'workflow', - id: 'workflowId', - role: 'workflow:editor', - scope: 'workflow:read', - userScopes: ['workflow:read'], - expected: true, - }, - { - type: 'credential', - id: 'credentialId', - role: 'credential:user', - scope: 'credential:read', - userScopes: ['credential:read'], - expected: true, - }, - ])( - 'should return $expected if the user has the required scopes for a $type', - async ({ type, id, role, scope, userScopes, expected }) => { - if (type === 'workflow') { - findByWorkflowMock.mockResolvedValueOnce([ - { - workflowId: id, - projectId: 'projectId', - role, - }, - ]); - } else { - findByCredentialMock.mockResolvedValueOnce([ - { - credentialId: id, - projectId: 'projectId', - role, - }, - ]); - } - - const user = { - id: 'userId', - scopes: userScopes, - role: GLOBAL_MEMBER_ROLE, - } as unknown as User; - const scopes = [scope] as Scope[]; - const params: { credentialId?: string; workflowId?: string; projectId?: string } = { - projectId: 'projectId', - }; + const params: { credentialId?: string; workflowId?: string; projectId?: string } = {}; if (type === 'credential') { params.credentialId = id; } else { params.workflowId = id; } - const result = await userHasScopes(user, scopes, false, params); - expect(result).toBe(expected); - }, - ); + + await expect(userHasScopes(user, scopes, false, params)).rejects.toThrow(NotFoundError); + }); + }); + + describe('RoleService integration', () => { + it('should use RoleService for credential role resolution', async () => { + const credentialId = 'cred123'; + const mockRoles = ['credential:owner', 'custom:credential-admin']; + + roleServiceMock.mockResolvedValue(mockRoles); + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'credential:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(roleServiceMock).toHaveBeenCalledWith('credential', scopes); + expect(result).toBe(true); + }); + + it('should use RoleService for workflow role resolution', async () => { + const workflowId = 'wf123'; + const mockRoles = ['workflow:owner', 'custom:workflow-manager']; + + roleServiceMock.mockResolvedValue(mockRoles); + findByWorkflowMock.mockResolvedValue([ + { + workflowId, + projectId: 'projectId', + role: 'workflow:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['workflow:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { workflowId }); + + expect(roleServiceMock).toHaveBeenCalledWith('workflow', scopes); + expect(result).toBe(true); + }); + + it('should handle custom database roles in access control', async () => { + const credentialId = 'cred123'; + const customRoles = ['custom:admin-role-abc123', 'credential:owner']; + + roleServiceMock.mockResolvedValue(customRoles); + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'custom:admin-role-abc123', // Custom role from database + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:update'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(roleServiceMock).toHaveBeenCalledWith('credential', scopes); + expect(result).toBe(true); + }); + + it('should propagate RoleService errors appropriately', async () => { + const credentialId = 'cred123'; + const roleServiceError = new Error('Role cache unavailable'); + + roleServiceMock.mockRejectedValue(roleServiceError); + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'credential:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + await expect(userHasScopes(user, scopes, false, { credentialId })).rejects.toThrow( + roleServiceError, + ); + }); + }); + + describe('namespace isolation', () => { + it('should use correct namespace for credential checks', async () => { + const credentialId = 'cred123'; + roleServiceMock.mockResolvedValue(['credential:owner']); + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'credential:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + + await userHasScopes(user, ['credential:read'], false, { credentialId }); + + expect(roleServiceMock).toHaveBeenCalledWith('credential', ['credential:read']); + expect(roleServiceMock).not.toHaveBeenCalledWith('workflow', expect.anything()); + }); + + it('should use correct namespace for workflow checks', async () => { + const workflowId = 'wf123'; + roleServiceMock.mockResolvedValue(['workflow:owner']); + findByWorkflowMock.mockResolvedValue([ + { + workflowId, + projectId: 'projectId', + role: 'workflow:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + + await userHasScopes(user, ['workflow:execute'], false, { workflowId }); + + expect(roleServiceMock).toHaveBeenCalledWith('workflow', ['workflow:execute']); + expect(roleServiceMock).not.toHaveBeenCalledWith('credential', expect.anything()); + }); + + it('should not allow workflow roles to access credentials', async () => { + const credentialId = 'cred123'; + // RoleService returns no matching credential roles + roleServiceMock.mockResolvedValue([]); + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'workflow:owner', // Wrong namespace role + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(result).toBe(false); + }); + }); + + describe('access control scenarios', () => { + it('should grant access when user has matching project and resource role', async () => { + const credentialId = 'cred123'; + roleServiceMock.mockResolvedValue(['credential:owner', 'credential:user']); + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'credential:user', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(result).toBe(true); + }); + + it('should deny access when user has project access but insufficient resource role', async () => { + const credentialId = 'cred123'; + roleServiceMock.mockResolvedValue(['credential:owner']); // Only owner role has required scopes + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'credential:viewer', // User has viewer role, but needs owner + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:delete'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(result).toBe(false); + }); + + it('should deny access when user has no project access', async () => { + const credentialId = 'cred123'; + mockQueryBuilder.getRawMany.mockResolvedValue([]); // No project access + roleServiceMock.mockResolvedValue(['credential:owner']); + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'otherProjectId', + role: 'credential:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(result).toBe(false); + }); + + it('should handle multiple scope requirements', async () => { + const workflowId = 'wf123'; + roleServiceMock.mockResolvedValue(['workflow:owner']); // Owner role has multiple scopes + findByWorkflowMock.mockResolvedValue([ + { + workflowId, + projectId: 'projectId', + role: 'workflow:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['workflow:read', 'workflow:execute'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { workflowId }); + + expect(roleServiceMock).toHaveBeenCalledWith('workflow', scopes); + expect(result).toBe(true); + }); + }); + + describe('edge cases', () => { + it('should handle empty role results from RoleService', async () => { + const credentialId = 'cred123'; + roleServiceMock.mockResolvedValue([]); // No roles match the scopes + findByCredentialMock.mockResolvedValue([ + { + credentialsId: credentialId, + projectId: 'projectId', + role: 'credential:owner', + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['*' as const] as Scope[]; // Use wildcard scope for testing + + const result = await userHasScopes(user, scopes, false, { credentialId }); + + expect(result).toBe(false); + }); + + it('should handle multiple resource shares in same project', async () => { + const workflowId = 'wf123'; + roleServiceMock.mockResolvedValue(['workflow:editor']); + findByWorkflowMock.mockResolvedValue([ + { + workflowId, + projectId: 'projectId', + role: 'workflow:viewer', // First share - insufficient + }, + { + workflowId, + projectId: 'projectId', + role: 'workflow:editor', // Second share - sufficient + }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['workflow:update'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { workflowId }); + + expect(result).toBe(true); + }); + + it('should handle project-only checks without RoleService', async () => { + const projectId = 'proj123'; + // Project checks don't use RoleService + mockQueryBuilder.getRawMany.mockResolvedValue([{ id: projectId }]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['project:read'] as Scope[]; + + const result = await userHasScopes(user, scopes, false, { projectId }); + + expect(roleServiceMock).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it('should handle concurrent permission checks', async () => { + const credentialId1 = 'cred1'; + const credentialId2 = 'cred2'; + + roleServiceMock.mockResolvedValue(['credential:owner']); + findByCredentialMock + .mockResolvedValueOnce([ + { credentialsId: credentialId1, projectId: 'projectId', role: 'credential:owner' }, + ]) + .mockResolvedValueOnce([ + { credentialsId: credentialId2, projectId: 'projectId', role: 'credential:viewer' }, + ]); + + const user = { id: 'userId', scopes: [], role: GLOBAL_MEMBER_ROLE } as unknown as User; + const scopes = ['credential:read'] as Scope[]; + + const [result1, result2] = await Promise.all([ + userHasScopes(user, scopes, false, { credentialId: credentialId1 }), + userHasScopes(user, scopes, false, { credentialId: credentialId2 }), + ]); + + expect(result1).toBe(true); + expect(result2).toBe(false); + expect(roleServiceMock).toHaveBeenCalledTimes(2); + }); + }); }); diff --git a/packages/cli/src/permissions.ee/check-access.ts b/packages/cli/src/permissions.ee/check-access.ts index 465775404c..585487c386 100644 --- a/packages/cli/src/permissions.ee/check-access.ts +++ b/packages/cli/src/permissions.ee/check-access.ts @@ -1,10 +1,11 @@ import type { User } from '@n8n/db'; import { ProjectRepository, SharedCredentialsRepository, SharedWorkflowRepository } from '@n8n/db'; import { Container } from '@n8n/di'; -import { hasGlobalScope, rolesWithScope, type Scope } from '@n8n/permissions'; +import { hasGlobalScope, type Scope } from '@n8n/permissions'; import { UnexpectedError } from 'n8n-workflow'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { RoleService } from '@/services/role.service'; /** * Check if a user has the required scopes. The check can be: @@ -47,6 +48,7 @@ export async function userHasScopes( // Find which resource roles are defined to contain the required scopes. // Then find at least one of the above qualifying projects having one of // those resource roles over the resource being checked. + const roleService = Container.get(RoleService); if (credentialId) { const credentials = await Container.get(SharedCredentialsRepository).findBy({ @@ -56,10 +58,10 @@ export async function userHasScopes( throw new NotFoundError(`Credential with ID "${credentialId}" not found.`); } + const validRoles = await roleService.rolesWithScope('credential', scopes); + return credentials.some( - (c) => - userProjectIds.includes(c.projectId) && - rolesWithScope('credential', scopes).includes(c.role), + (c) => userProjectIds.includes(c.projectId) && validRoles.includes(c.role), ); } @@ -72,9 +74,10 @@ export async function userHasScopes( throw new NotFoundError(`Workflow with ID "${workflowId}" not found.`); } + const validRoles = await roleService.rolesWithScope('workflow', scopes); + return workflows.some( - (w) => - userProjectIds.includes(w.projectId) && rolesWithScope('workflow', scopes).includes(w.role), + (w) => userProjectIds.includes(w.projectId) && validRoles.includes(w.role), ); } diff --git a/packages/cli/src/services/__tests__/credentials-finder.service.test.ts b/packages/cli/src/services/__tests__/credentials-finder.service.test.ts index 4eace74bf7..a4ba74fad3 100644 --- a/packages/cli/src/services/__tests__/credentials-finder.service.test.ts +++ b/packages/cli/src/services/__tests__/credentials-finder.service.test.ts @@ -1,4 +1,10 @@ -import { GLOBAL_MEMBER_ROLE, GLOBAL_OWNER_ROLE, SharedCredentials } from '@n8n/db'; +import { + GLOBAL_MEMBER_ROLE, + GLOBAL_OWNER_ROLE, + type SharedCredentials, + CredentialsRepository, + SharedCredentialsRepository, +} from '@n8n/db'; import type { CredentialsEntity, User } from '@n8n/db'; import { Container } from '@n8n/di'; import { @@ -8,15 +14,43 @@ import { PROJECT_VIEWER_ROLE_SLUG, } from '@n8n/permissions'; import { In } from '@n8n/typeorm'; -import { mockEntityManager } from '@test/mocking'; import { mock } from 'jest-mock-extended'; +import { mockInstance } from '@n8n/backend-test-utils'; import { CredentialsFinderService } from '@/credentials/credentials-finder.service'; +import { RoleService } from '../role.service'; describe('CredentialsFinderService', () => { - const entityManager = mockEntityManager(SharedCredentials); + const roleService = mockInstance(RoleService); + const credentialsRepository = mockInstance(CredentialsRepository); + const sharedCredentialsRepository = mockInstance(SharedCredentialsRepository); const credentialsFinderService = Container.get(CredentialsFinderService); + beforeAll(() => { + Container.set(RoleService, roleService); + Container.set(CredentialsRepository, credentialsRepository); + Container.set(SharedCredentialsRepository, sharedCredentialsRepository); + }); + + beforeEach(() => { + jest.clearAllMocks(); + + // Default mock implementation for all tests + roleService.rolesWithScope.mockImplementation(async (namespace) => { + if (namespace === 'project') { + return [ + PROJECT_ADMIN_ROLE_SLUG, + PROJECT_OWNER_ROLE_SLUG, + PROJECT_EDITOR_ROLE_SLUG, + PROJECT_VIEWER_ROLE_SLUG, + ]; + } else if (namespace === 'credential') { + return ['credential:owner', 'credential:user']; + } + return []; + }); + }); + describe('findCredentialForUser', () => { const credentialsId = 'cred_123'; const sharedCredential = mock(); @@ -29,33 +63,38 @@ describe('CredentialsFinderService', () => { id: 'test', }); - beforeEach(() => { - jest.resetAllMocks(); - }); - test('should allow instance owner access to all credentials', async () => { - entityManager.findOne.mockResolvedValueOnce(sharedCredential); + sharedCredentialsRepository.findOne.mockResolvedValueOnce(sharedCredential); const credential = await credentialsFinderService.findCredentialForUser( credentialsId, owner, - ['credential:read'], + ['credential:read' as const], ); - expect(entityManager.findOne).toHaveBeenCalledWith(SharedCredentials, { - relations: { credentials: { shared: { project: { projectRelations: { user: true } } } } }, + expect(sharedCredentialsRepository.findOne).toHaveBeenCalledWith({ where: { credentialsId }, + relations: { + credentials: { + shared: { project: { projectRelations: { user: true } } }, + }, + }, }); + expect(roleService.rolesWithScope).not.toHaveBeenCalled(); expect(credential).toEqual(sharedCredential.credentials); }); - test('should allow members', async () => { - entityManager.findOne.mockResolvedValueOnce(sharedCredential); + test('should allow members and call RoleService correctly', async () => { + sharedCredentialsRepository.findOne.mockResolvedValueOnce(sharedCredential); const credential = await credentialsFinderService.findCredentialForUser( credentialsId, member, - ['credential:read'], + ['credential:read' as const], ); - expect(entityManager.findOne).toHaveBeenCalledWith(SharedCredentials, { - relations: { credentials: { shared: { project: { projectRelations: { user: true } } } } }, + + expect(roleService.rolesWithScope).toHaveBeenCalledTimes(2); + expect(roleService.rolesWithScope).toHaveBeenCalledWith('project', ['credential:read']); + expect(roleService.rolesWithScope).toHaveBeenCalledWith('credential', ['credential:read']); + + expect(sharedCredentialsRepository.findOne).toHaveBeenCalledWith({ where: { credentialsId, role: In(['credential:owner', 'credential:user']), @@ -71,19 +110,23 @@ describe('CredentialsFinderService', () => { }, }, }, + relations: { + credentials: { + shared: { project: { projectRelations: { user: true } } }, + }, + }, }); expect(credential).toEqual(sharedCredential.credentials); }); test('should return null when no shared credential is found', async () => { - entityManager.findOne.mockResolvedValueOnce(null); + sharedCredentialsRepository.findOne.mockResolvedValueOnce(null); const credential = await credentialsFinderService.findCredentialForUser( credentialsId, member, - ['credential:read'], + ['credential:read' as const], ); - expect(entityManager.findOne).toHaveBeenCalledWith(SharedCredentials, { - relations: { credentials: { shared: { project: { projectRelations: { user: true } } } } }, + expect(sharedCredentialsRepository.findOne).toHaveBeenCalledWith({ where: { credentialsId, role: In(['credential:owner', 'credential:user']), @@ -99,8 +142,446 @@ describe('CredentialsFinderService', () => { }, }, }, + relations: { + credentials: { + shared: { project: { projectRelations: { user: true } } }, + }, + }, }); expect(credential).toEqual(null); }); + + test('should handle custom roles from RoleService', async () => { + roleService.rolesWithScope.mockImplementation(async (namespace) => { + if (namespace === 'project') { + return ['custom:project-admin-abc123', PROJECT_VIEWER_ROLE_SLUG]; + } else if (namespace === 'credential') { + return ['custom:cred-manager-xyz789', 'credential:user']; + } + return []; + }); + + sharedCredentialsRepository.findOne.mockResolvedValueOnce(sharedCredential); + const credential = await credentialsFinderService.findCredentialForUser( + credentialsId, + member, + ['credential:update' as const], + ); + + expect(sharedCredentialsRepository.findOne).toHaveBeenCalledWith({ + where: { + credentialsId, + role: In(['custom:cred-manager-xyz789', 'credential:user']), + project: { + projectRelations: { + role: In(['custom:project-admin-abc123', PROJECT_VIEWER_ROLE_SLUG]), + userId: member.id, + }, + }, + }, + relations: { + credentials: { + shared: { project: { projectRelations: { user: true } } }, + }, + }, + }); + expect(credential).toEqual(sharedCredential.credentials); + }); + + test('should handle RoleService failure gracefully', async () => { + roleService.rolesWithScope.mockRejectedValue(new Error('Role cache unavailable')); + + await expect( + credentialsFinderService.findCredentialForUser(credentialsId, member, [ + 'credential:read' as const, + ]), + ).rejects.toThrow('Role cache unavailable'); + + expect(sharedCredentialsRepository.findOne).not.toHaveBeenCalled(); + }); + }); + + describe('findCredentialsForUser', () => { + const credentials = [ + mock({ id: 'cred1', shared: [] }), + mock({ id: 'cred2', shared: [] }), + ]; + const owner = mock({ role: GLOBAL_OWNER_ROLE }); + const member = mock({ role: GLOBAL_MEMBER_ROLE, id: 'user123' }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('should allow global owner access to all credentials without role filtering', async () => { + credentialsRepository.find.mockResolvedValueOnce(credentials); + + const result = await credentialsFinderService.findCredentialsForUser(owner, [ + 'credential:read' as const, + ]); + + expect(credentialsRepository.find).toHaveBeenCalledWith({ + where: {}, + relations: { shared: true }, + }); + expect(roleService.rolesWithScope).not.toHaveBeenCalled(); + expect(result).toEqual(credentials); + }); + + test('should filter credentials by roles for regular members', async () => { + credentialsRepository.find.mockResolvedValueOnce(credentials); + + const result = await credentialsFinderService.findCredentialsForUser(member, [ + 'credential:update' as const, + ]); + + expect(roleService.rolesWithScope).toHaveBeenCalledWith('project', ['credential:update']); + expect(roleService.rolesWithScope).toHaveBeenCalledWith('credential', ['credential:update']); + expect(credentialsRepository.find).toHaveBeenCalledWith({ + where: { + shared: { + role: In(['credential:owner', 'credential:user']), + project: { + projectRelations: { + role: In([ + PROJECT_ADMIN_ROLE_SLUG, + PROJECT_OWNER_ROLE_SLUG, + PROJECT_EDITOR_ROLE_SLUG, + PROJECT_VIEWER_ROLE_SLUG, + ]), + userId: member.id, + }, + }, + }, + }, + relations: { shared: true }, + }); + expect(result).toEqual(credentials); + }); + + test('should handle custom roles in filtering', async () => { + roleService.rolesWithScope.mockImplementation(async (namespace) => { + if (namespace === 'project') return ['custom:project-lead-456']; + if (namespace === 'credential') return ['custom:cred-admin-789']; + return []; + }); + + const singleCredResult = [credentials[0]]; + credentialsRepository.find.mockResolvedValueOnce(singleCredResult); + + const result = await credentialsFinderService.findCredentialsForUser(member, [ + 'credential:delete' as const, + ]); + + expect(credentialsRepository.find).toHaveBeenCalledWith({ + where: { + shared: { + role: In(['custom:cred-admin-789']), + project: { + projectRelations: { + role: In(['custom:project-lead-456']), + userId: member.id, + }, + }, + }, + }, + relations: { shared: true }, + }); + expect(result).toEqual(singleCredResult); + }); + }); + + describe('findAllCredentialsForUser', () => { + const sharedCredentials = [ + mock({ + credentials: mock({ id: 'cred1' }), + projectId: 'proj1', + credentialsId: 'cred1', + role: 'credential:owner', + }), + mock({ + credentials: mock({ id: 'cred2' }), + projectId: 'proj2', + credentialsId: 'cred2', + role: 'credential:user', + }), + ]; + const owner = mock({ role: GLOBAL_OWNER_ROLE }); + const member = mock({ role: GLOBAL_MEMBER_ROLE, id: 'user123' }); + + beforeEach(() => { + jest.clearAllMocks(); + + // Reset to default implementation for each test + roleService.rolesWithScope.mockImplementation(async (namespace) => { + if (namespace === 'project') { + return [ + PROJECT_ADMIN_ROLE_SLUG, + PROJECT_OWNER_ROLE_SLUG, + PROJECT_EDITOR_ROLE_SLUG, + PROJECT_VIEWER_ROLE_SLUG, + ]; + } else if (namespace === 'credential') { + return ['credential:owner', 'credential:user']; + } + return []; + }); + }); + + test('should allow global owner access without filtering', async () => { + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce( + sharedCredentials, + ); + + const result = await credentialsFinderService.findAllCredentialsForUser(owner, [ + 'credential:read' as const, + ]); + + expect(sharedCredentialsRepository.findCredentialsWithOptions).toHaveBeenCalledWith( + {}, + undefined, + ); + expect(roleService.rolesWithScope).not.toHaveBeenCalled(); + expect(result).toEqual([ + { ...sharedCredentials[0].credentials, projectId: 'proj1' }, + { ...sharedCredentials[1].credentials, projectId: 'proj2' }, + ]); + }); + + test('should filter by roles for regular members', async () => { + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce([ + sharedCredentials[0], + ]); + + const result = await credentialsFinderService.findAllCredentialsForUser(member, [ + 'credential:read' as const, + ]); + + expect(roleService.rolesWithScope).toHaveBeenCalledWith('project', ['credential:read']); + expect(roleService.rolesWithScope).toHaveBeenCalledWith('credential', ['credential:read']); + expect(sharedCredentialsRepository.findCredentialsWithOptions).toHaveBeenCalledWith( + { + role: In(['credential:owner', 'credential:user']), + project: { + projectRelations: { + role: In([ + PROJECT_ADMIN_ROLE_SLUG, + PROJECT_OWNER_ROLE_SLUG, + PROJECT_EDITOR_ROLE_SLUG, + PROJECT_VIEWER_ROLE_SLUG, + ]), + userId: member.id, + }, + }, + }, + undefined, + ); + expect(result).toEqual([{ ...sharedCredentials[0].credentials, projectId: 'proj1' }]); + }); + + test('should support transaction manager', async () => { + const mockTrx = mock(); + sharedCredentialsRepository.findCredentialsWithOptions.mockResolvedValueOnce([]); + + await credentialsFinderService.findAllCredentialsForUser( + member, + ['credential:read' as const], + mockTrx, + ); + + expect(sharedCredentialsRepository.findCredentialsWithOptions).toHaveBeenCalledWith( + expect.any(Object), + mockTrx, + ); + }); + }); + + describe('getCredentialIdsByUserAndRole', () => { + const userIds = ['user1', 'user2']; + const mockSharings = [ + mock({ credentialsId: 'cred1' }), + mock({ credentialsId: 'cred2' }), + ]; + + beforeEach(() => { + jest.clearAllMocks(); + + // Reset to default implementation + roleService.rolesWithScope.mockImplementation(async (namespace) => { + if (namespace === 'project') { + return [ + PROJECT_ADMIN_ROLE_SLUG, + PROJECT_OWNER_ROLE_SLUG, + PROJECT_EDITOR_ROLE_SLUG, + PROJECT_VIEWER_ROLE_SLUG, + ]; + } else if (namespace === 'credential') { + return ['credential:owner', 'credential:user']; + } + return []; + }); + }); + + test('should use RoleService when scopes are provided', async () => { + sharedCredentialsRepository.findCredentialsByRoles.mockResolvedValueOnce(mockSharings); + + const result = await credentialsFinderService.getCredentialIdsByUserAndRole(userIds, { + scopes: ['credential:read' as const, 'credential:update' as const], + }); + + expect(roleService.rolesWithScope).toHaveBeenCalledWith('project', [ + 'credential:read', + 'credential:update', + ]); + expect(roleService.rolesWithScope).toHaveBeenCalledWith('credential', [ + 'credential:read', + 'credential:update', + ]); + expect(sharedCredentialsRepository.findCredentialsByRoles).toHaveBeenCalledWith( + userIds, + [ + PROJECT_ADMIN_ROLE_SLUG, + PROJECT_OWNER_ROLE_SLUG, + PROJECT_EDITOR_ROLE_SLUG, + PROJECT_VIEWER_ROLE_SLUG, + ], + ['credential:owner', 'credential:user'], + undefined, + ); + expect(result).toEqual([mockSharings[0].credentialsId, mockSharings[1].credentialsId]); + }); + + test('should use direct roles when provided', async () => { + const projectRoles = ['custom:project-admin'] as any; + const credentialRoles = ['custom:cred-viewer'] as any; + sharedCredentialsRepository.findCredentialsByRoles.mockResolvedValueOnce([mockSharings[0]]); + + const result = await credentialsFinderService.getCredentialIdsByUserAndRole(userIds, { + projectRoles, + credentialRoles, + }); + + expect(roleService.rolesWithScope).not.toHaveBeenCalled(); + expect(sharedCredentialsRepository.findCredentialsByRoles).toHaveBeenCalledWith( + userIds, + projectRoles, + credentialRoles, + undefined, + ); + expect(result).toEqual([mockSharings[0].credentialsId]); + }); + + test('should support transaction manager', async () => { + const mockTrx = mock(); + sharedCredentialsRepository.findCredentialsByRoles.mockResolvedValueOnce([]); + + await credentialsFinderService.getCredentialIdsByUserAndRole( + userIds, + { scopes: ['credential:read' as const] }, + mockTrx, + ); + + expect(sharedCredentialsRepository.findCredentialsByRoles).toHaveBeenCalledWith( + expect.any(Array), + expect.any(Array), + expect.any(Array), + mockTrx, + ); + }); + + test('should handle empty results', async () => { + sharedCredentialsRepository.findCredentialsByRoles.mockResolvedValueOnce([]); + + const result = await credentialsFinderService.getCredentialIdsByUserAndRole(userIds, { + scopes: ['credential:read' as const], + }); + + expect(result).toEqual([]); + }); + }); + + describe('RoleService integration edge cases', () => { + const member = mock({ role: GLOBAL_MEMBER_ROLE, id: 'user123' }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('should handle empty role results from RoleService', async () => { + roleService.rolesWithScope.mockResolvedValue([]); + const emptyResult: CredentialsEntity[] = []; + credentialsRepository.find.mockResolvedValueOnce(emptyResult); + + const result = await credentialsFinderService.findCredentialsForUser(member, [ + 'credential:read' as const, + ]); + + expect(credentialsRepository.find).toHaveBeenCalledWith({ + where: { + shared: { + role: In([]), + project: { + projectRelations: { + role: In([]), + userId: member.id, + }, + }, + }, + }, + relations: { shared: true }, + }); + expect(result).toEqual(emptyResult); + }); + + test('should handle RoleService failures in findCredentialsForUser', async () => { + roleService.rolesWithScope.mockRejectedValueOnce(new Error('Database connection failed')); + + await expect( + credentialsFinderService.findCredentialsForUser(member, ['credential:read' as const]), + ).rejects.toThrow('Database connection failed'); + + expect(credentialsRepository.find).not.toHaveBeenCalled(); + }); + + test('should handle partial RoleService failures', async () => { + roleService.rolesWithScope + .mockResolvedValueOnce(['project:admin']) // First call succeeds + .mockRejectedValueOnce(new Error('Credential role lookup failed')); // Second call fails + + await expect( + credentialsFinderService.findCredentialsForUser(member, ['credential:read' as const]), + ).rejects.toThrow('Credential role lookup failed'); + }); + + test('should maintain namespace isolation', async () => { + roleService.rolesWithScope.mockImplementation(async (namespace) => { + if (namespace === 'project') return ['workflow:owner']; // Wrong namespace + if (namespace === 'credential') return ['project:admin']; // Wrong namespace + return []; + }); + + const isolationResult: CredentialsEntity[] = []; + credentialsRepository.find.mockResolvedValueOnce(isolationResult); + + const result = await credentialsFinderService.findCredentialsForUser(member, [ + 'credential:read' as const, + ]); + + expect(credentialsRepository.find).toHaveBeenCalledWith({ + where: { + shared: { + role: In(['project:admin']), // Uses what RoleService returned for credential namespace + project: { + projectRelations: { + role: In(['workflow:owner']), // Uses what RoleService returned for project namespace + userId: member.id, + }, + }, + }, + }, + relations: { shared: true }, + }); + expect(result).toEqual(isolationResult); + }); }); }); diff --git a/packages/cli/src/services/__tests__/role-cache.service.test.ts b/packages/cli/src/services/__tests__/role-cache.service.test.ts new file mode 100644 index 0000000000..9959a7ad28 --- /dev/null +++ b/packages/cli/src/services/__tests__/role-cache.service.test.ts @@ -0,0 +1,298 @@ +import { Logger } from '@n8n/backend-common'; +import { mockInstance } from '@n8n/backend-test-utils'; +import { RoleRepository } from '@n8n/db'; +import { Container } from '@n8n/di'; +import { staticRolesWithScope } from '@n8n/permissions'; +import { mock } from 'jest-mock-extended'; + +import type { CacheService } from '@/services/cache/cache.service'; +import { RoleCacheService } from '@/services/role-cache.service'; + +// Mock static function +jest.mock('@n8n/permissions', () => ({ + ...jest.requireActual('@n8n/permissions'), + staticRolesWithScope: jest.fn(), +})); + +describe('RoleCacheService', () => { + const cacheService = mock(); + const logger = mockInstance(Logger); + const roleRepository = mockInstance(RoleRepository); + const staticRolesMock = staticRolesWithScope as jest.MockedFunction; + + const roleCacheService = new RoleCacheService(cacheService, logger); + + const mockRoleScopeMap = { + project: { + 'project:admin': { + scopes: ['project:read', 'project:update', 'project:delete', 'credential:read'], + }, + 'project:editor': { + scopes: ['project:read', 'project:update'], + }, + 'project:viewer': { + scopes: ['project:read'], + }, + }, + credential: { + 'credential:owner': { + scopes: ['credential:read', 'credential:update'], + }, + }, + workflow: { + 'workflow:owner': { + scopes: ['workflow:read', 'workflow:update', 'credential:read'], + }, + }, + }; + + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(Container, 'get').mockReturnValue(roleRepository); + }); + + describe('getRolesWithAllScopes', () => { + it('should return empty array when no scopes provided', async () => { + const result = await roleCacheService.getRolesWithAllScopes('project', []); + + expect(result).toEqual([]); + expect(cacheService.get).not.toHaveBeenCalled(); + }); + + it('should return roles matching namespace and all required scopes', async () => { + cacheService.get.mockResolvedValue(mockRoleScopeMap); + + const result = await roleCacheService.getRolesWithAllScopes('project', [ + 'project:read', + 'project:update', + ]); + + expect(result).toContain('project:admin'); + expect(result).toContain('project:editor'); + expect(result).not.toContain('project:viewer'); // Missing 'project:update' + expect(result).not.toContain('credential:owner'); // Wrong namespace + }); + + it('should exclude roles from different namespaces', async () => { + cacheService.get.mockResolvedValue(mockRoleScopeMap); + + const result = await roleCacheService.getRolesWithAllScopes('credential', [ + 'credential:read', + ]); + + expect(result).toContain('credential:owner'); + expect(result).not.toContain('project:admin'); + expect(result).not.toContain('workflow:owner'); + }); + + it('should exclude roles missing any required scope', async () => { + cacheService.get.mockResolvedValue(mockRoleScopeMap); + + const result = await roleCacheService.getRolesWithAllScopes('project', [ + 'project:read', + 'project:update', + 'project:delete', + ]); + + expect(result).toContain('project:admin'); // Has all three scopes + expect(result).not.toContain('project:editor'); // Missing 'project:delete' + expect(result).not.toContain('project:viewer'); // Missing 'project:update' and 'project:delete' + }); + }); + + describe('cache behavior', () => { + it('should use cached data when available', async () => { + cacheService.get.mockResolvedValue(mockRoleScopeMap); + + const result = await roleCacheService.getRolesWithAllScopes('project', ['project:read']); + + expect(cacheService.get).toHaveBeenCalledWith('roles:scope-map', { + refreshFn: expect.any(Function), + fallbackValue: undefined, + }); + expect(result).toContain('project:admin'); + expect(result).toContain('project:editor'); + expect(result).toContain('project:viewer'); + }); + + it('should fallback to static roles when cache returns undefined', async () => { + cacheService.get.mockResolvedValue(undefined); + staticRolesMock.mockReturnValue(['static:role']); + + const result = await roleCacheService.getRolesWithAllScopes('project', ['project:read']); + + expect(staticRolesMock).toHaveBeenCalledWith('project', ['project:read']); + expect(result).toEqual(['static:role']); + expect(logger.error).toHaveBeenCalledWith( + 'Role scope map is undefined, falling back to static roles', + ); + }); + }); + + describe('buildRoleScopeMap', () => { + it('should refresh cache with new data from database', async () => { + const mockRoles = [ + { + slug: 'project:admin', + displayName: 'Project Admin', + description: 'Admin role for projects', + systemRole: true, + roleType: 'project' as const, + projectRelations: [], + createdAt: new Date(), + updatedAt: new Date(), + setUpdateDate: () => {}, + scopes: [ + { + slug: 'project:read' as const, + displayName: 'Read Projects', + description: 'Can read project data', + }, + { + slug: 'project:update' as const, + displayName: 'Update Projects', + description: 'Can update project data', + }, + ], + }, + { + slug: 'credential:owner', + displayName: 'Credential Owner', + description: 'Owner of credentials', + systemRole: true, + createdAt: new Date(), + setUpdateDate: () => {}, + updatedAt: new Date(), + roleType: 'credential' as const, + projectRelations: [], + scopes: [ + { + slug: 'credential:read' as const, + displayName: 'Read Credentials', + description: 'Can read credential data', + }, + { + slug: 'credential:update' as const, + displayName: 'Update Credentials', + description: 'Can update credential data', + }, + ], + }, + ]; + roleRepository.findAll.mockResolvedValue(mockRoles); + + await roleCacheService.refreshCache(); + + expect(Container.get).toHaveBeenCalledWith(RoleRepository); + expect(roleRepository.findAll).toHaveBeenCalledTimes(1); + expect(cacheService.set).toHaveBeenCalledWith( + 'roles:scope-map', + { + project: { + 'project:admin': { + scopes: ['project:read', 'project:update'], + }, + }, + credential: { + 'credential:owner': { + scopes: ['credential:read', 'credential:update'], + }, + }, + }, + 300000, // 5 minutes TTL + ); + }); + + it('should handle database errors and rethrow with logging', async () => { + const dbError = new Error('Database connection failed'); + roleRepository.findAll.mockRejectedValue(dbError); + + await expect(roleCacheService.refreshCache()).rejects.toThrow(dbError); + + expect(logger.error).toHaveBeenCalledWith('Failed to build role scope from database', { + error: dbError, + }); + }); + }); + + describe('cache management', () => { + it('should invalidate cache correctly', async () => { + await roleCacheService.invalidateCache(); + + expect(cacheService.delete).toHaveBeenCalledWith('roles:scope-map'); + }); + + it('should refresh cache with new data from database', async () => { + const mockRoles = [ + { + slug: 'workflow:custom', + displayName: 'Custom Workflow Role', + description: 'Custom workflow access', + systemRole: false, + roleType: 'workflow' as const, + projectRelations: [], + createdAt: new Date(), + updatedAt: new Date(), + setUpdateDate: () => {}, + scopes: [ + { + slug: 'workflow:read' as const, + displayName: 'Read Workflows', + description: 'Can read workflow data', + }, + ], + }, + ]; + roleRepository.findAll.mockResolvedValue(mockRoles); + + await roleCacheService.refreshCache(); + + expect(roleRepository.findAll).toHaveBeenCalledTimes(1); + expect(cacheService.set).toHaveBeenCalledWith( + 'roles:scope-map', + { + workflow: { + 'workflow:custom': { + scopes: ['workflow:read'], + }, + }, + }, + 300000, // 5 minutes TTL + ); + }); + }); + + describe('error scenarios', () => { + it('should fail if cache service fails to provide data', async () => { + const cacheError = new Error('Cache service unavailable'); + cacheService.get.mockRejectedValue(cacheError); + staticRolesMock.mockReturnValue(['fallback:role']); + + // This should not throw, but rather handle the error gracefully + await expect( + roleCacheService.getRolesWithAllScopes('project', ['project:read']), + ).rejects.toThrow(cacheError); + }); + + it('should log errors and rethrow when database fails during refresh', async () => { + const dbError = new Error('Database query timeout'); + roleRepository.findAll.mockRejectedValue(dbError); + + // Trigger database call through cache refresh + cacheService.get.mockImplementation(async (key, options) => { + if (options?.refreshFn) { + await options.refreshFn(key); + } + return undefined; + }); + + await expect( + roleCacheService.getRolesWithAllScopes('project', ['project:read']), + ).rejects.toThrow(dbError); + + expect(logger.error).toHaveBeenCalledWith('Failed to build role scope from database', { + error: dbError, + }); + }); + }); +}); diff --git a/packages/cli/src/services/__tests__/role.service.rolesWithScope.test.ts b/packages/cli/src/services/__tests__/role.service.rolesWithScope.test.ts new file mode 100644 index 0000000000..c6172f38af --- /dev/null +++ b/packages/cli/src/services/__tests__/role.service.rolesWithScope.test.ts @@ -0,0 +1,162 @@ +import type { LicenseState } from '@n8n/backend-common'; +import { mockInstance } from '@n8n/backend-test-utils'; +import { RoleRepository, ScopeRepository } from '@n8n/db'; +import { mock } from 'jest-mock-extended'; + +import { RoleCacheService } from '@/services/role-cache.service'; +import { RoleService } from '@/services/role.service'; + +describe('RoleService.rolesWithScope', () => { + const licenseState = mock(); + const roleRepository = mockInstance(RoleRepository); + const scopeRepository = mockInstance(ScopeRepository); + const roleCacheService = mockInstance(RoleCacheService); + + const roleService = new RoleService( + licenseState, + roleRepository, + scopeRepository, + roleCacheService, + ); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('core functionality', () => { + it('should convert single scope to array and call cache service', async () => { + const mockRoles = ['project:admin', 'project:editor']; + roleCacheService.getRolesWithAllScopes.mockResolvedValue(mockRoles); + + const result = await roleService.rolesWithScope('project', 'project:read'); + + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenCalledWith('project', [ + 'project:read', + ]); + expect(result).toEqual(mockRoles); + }); + + it('should pass array scopes through correctly', async () => { + const mockRoles = ['project:admin']; + const inputScopes = ['project:read' as const, 'project:update' as const]; + roleCacheService.getRolesWithAllScopes.mockResolvedValue(mockRoles); + + const result = await roleService.rolesWithScope('project', inputScopes); + + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenCalledWith('project', inputScopes); + expect(result).toEqual(mockRoles); + }); + + it('should handle empty array input', async () => { + const mockRoles: string[] = []; + roleCacheService.getRolesWithAllScopes.mockResolvedValue(mockRoles); + + const result = await roleService.rolesWithScope('project', []); + + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenCalledWith('project', []); + expect(result).toEqual([]); + }); + }); + + describe('cache service integration', () => { + it('should work with different namespaces', async () => { + const credentialRoles = ['credential:owner']; + const workflowRoles = ['workflow:owner']; + + roleCacheService.getRolesWithAllScopes.mockImplementation(async (namespace) => { + if (namespace === 'credential') return credentialRoles; + if (namespace === 'workflow') return workflowRoles; + return []; + }); + + const credentialResult = await roleService.rolesWithScope('credential', ['credential:read']); + const workflowResult = await roleService.rolesWithScope('workflow', ['workflow:read']); + + expect(credentialResult).toEqual(credentialRoles); + expect(workflowResult).toEqual(workflowRoles); + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenCalledTimes(2); + }); + + it('should propagate cache service errors', async () => { + const cacheError = new Error('Cache service failed'); + roleCacheService.getRolesWithAllScopes.mockRejectedValue(cacheError); + + await expect(roleService.rolesWithScope('project', ['project:read'])).rejects.toThrow( + cacheError, + ); + }); + + it('should handle successful cache responses', async () => { + const mockRoles = ['project:admin', 'project:editor', 'project:viewer']; + roleCacheService.getRolesWithAllScopes.mockResolvedValue(mockRoles); + + const result = await roleService.rolesWithScope('project', [ + 'project:read', + 'project:update', + ]); + + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenCalledWith('project', [ + 'project:read', + 'project:update', + ]); + expect(result).toEqual(mockRoles); + }); + }); + + describe('input validation', () => { + it('should handle various valid scope formats', async () => { + const mockRoles = ['credential:owner']; + roleCacheService.getRolesWithAllScopes.mockResolvedValue(mockRoles); + + // Test different scope formats + await roleService.rolesWithScope('credential', 'credential:read'); + await roleService.rolesWithScope('workflow', 'workflow:execute'); + await roleService.rolesWithScope('project', 'project:delete'); + + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenCalledTimes(3); + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenNthCalledWith(1, 'credential', [ + 'credential:read', + ]); + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenNthCalledWith(2, 'workflow', [ + 'workflow:execute', + ]); + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenNthCalledWith(3, 'project', [ + 'project:delete', + ]); + }); + + it('should handle mixed scope arrays', async () => { + const mockRoles = ['project:admin']; + const mixedScopes = [ + 'project:read' as const, + 'project:update' as const, + 'project:delete' as const, + ]; + roleCacheService.getRolesWithAllScopes.mockResolvedValue(mockRoles); + + const result = await roleService.rolesWithScope('project', mixedScopes); + + expect(roleCacheService.getRolesWithAllScopes).toHaveBeenCalledWith('project', mixedScopes); + expect(result).toEqual(mockRoles); + }); + }); + + describe('edge cases', () => { + it('should handle empty results from cache service', async () => { + roleCacheService.getRolesWithAllScopes.mockResolvedValue([]); + + const result = await roleService.rolesWithScope('global', ['*' as const]); + + expect(result).toEqual([]); + }); + + it('should handle single role result', async () => { + const mockRoles = ['project:viewer']; + roleCacheService.getRolesWithAllScopes.mockResolvedValue(mockRoles); + + const result = await roleService.rolesWithScope('project', 'project:read'); + + expect(result).toEqual(mockRoles); + }); + }); +}); diff --git a/packages/cli/src/services/project.service.ee.ts b/packages/cli/src/services/project.service.ee.ts index 3141143400..8a1a4567c8 100644 --- a/packages/cli/src/services/project.service.ee.ts +++ b/packages/cli/src/services/project.service.ee.ts @@ -14,7 +14,6 @@ import { import { Container, Service } from '@n8n/di'; import { hasGlobalScope, - rolesWithScope, type Scope, type ProjectRole, AssignableProjectRole, @@ -445,7 +444,7 @@ export class ProjectService { }; if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { - const projectRoles = rolesWithScope('project', scopes); + const projectRoles = await this.roleService.rolesWithScope('project', scopes); where = { ...where, diff --git a/packages/cli/src/services/role-cache.service.ts b/packages/cli/src/services/role-cache.service.ts new file mode 100644 index 0000000000..450dbf7e28 --- /dev/null +++ b/packages/cli/src/services/role-cache.service.ts @@ -0,0 +1,115 @@ +import { Logger } from '@n8n/backend-common'; +import { Time } from '@n8n/constants'; +import { RoleRepository } from '@n8n/db'; +import { Container, Service } from '@n8n/di'; +import { staticRolesWithScope, type Scope } from '@n8n/permissions'; + +import { CacheService } from './cache/cache.service'; + +type RoleInfo = { + scopes: string[]; // array of scope slugs +}; + +interface RoleScopeMap { + global?: { + [roleSlug: string]: RoleInfo; + }; + project?: { + [roleSlug: string]: RoleInfo; + }; + credential?: { + [roleSlug: string]: RoleInfo; + }; + workflow?: { + [roleSlug: string]: RoleInfo; + }; +} + +@Service() +export class RoleCacheService { + private static readonly CACHE_KEY = 'roles:scope-map'; + private static readonly CACHE_TTL = 5 * Time.minutes.toMilliseconds; // 5 minutes TTL + + constructor( + private readonly cacheService: CacheService, + private readonly logger: Logger, + ) {} + + /** + * Get all roles from database and build scope map + */ + private async buildRoleScopeMap(): Promise { + try { + const roleRepository = Container.get(RoleRepository); + const roles = await roleRepository.findAll(); + + const roleScopeMap: RoleScopeMap = {}; + for (const role of roles) { + roleScopeMap[role.roleType] ??= {}; + roleScopeMap[role.roleType]![role.slug] = { + scopes: role.scopes.map((s) => s.slug), + }; + } + + return roleScopeMap; + } catch (error) { + this.logger.error('Failed to build role scope from database', { error }); + throw error; + } + } + + /** + * Get roles with all specified scopes (with caching) + */ + async getRolesWithAllScopes( + namespace: 'global' | 'project' | 'credential' | 'workflow', + requiredScopes: Scope[], + ): Promise { + if (requiredScopes.length === 0) return []; + + // Get cached role map with refresh function + const roleScopeMap = await this.cacheService.get(RoleCacheService.CACHE_KEY, { + refreshFn: async () => await this.buildRoleScopeMap(), + fallbackValue: undefined, + }); + + if (roleScopeMap === undefined) { + // TODO: actively report this case to sentry or similar system + this.logger.error('Role scope map is undefined, falling back to static roles'); + // Fallback to static roles if dynamic data is not available + return staticRolesWithScope(namespace, requiredScopes); + } + + // Filter roles by namespace and scopes + const matchingRoles: string[] = []; + + for (const [roleSlug, roleInfo] of Object.entries(roleScopeMap[namespace] ?? {})) { + // Check if role has ALL required scopes + const hasAllScopes = requiredScopes.every((scope) => roleInfo.scopes.includes(scope)); + if (hasAllScopes) { + matchingRoles.push(roleSlug); + } + } + + return matchingRoles; + } + + /** + * Invalidate the role cache (call after role changes) + */ + async invalidateCache(): Promise { + await this.cacheService.delete(RoleCacheService.CACHE_KEY); + } + + /** + * Force refresh the cache + */ + async refreshCache(): Promise { + const roleScopeMap = await this.buildRoleScopeMap(); + await this.cacheService.set( + RoleCacheService.CACHE_KEY, + roleScopeMap, + RoleCacheService.CACHE_TTL, + ); + } +} diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index 6f37ca2851..45a9663940 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -15,7 +15,12 @@ import { GLOBAL_ADMIN_ROLE, } from '@n8n/db'; import { Service } from '@n8n/di'; -import type { Scope, Role as RoleDTO, AssignableProjectRole } from '@n8n/permissions'; +import type { + Scope, + Role as RoleDTO, + AssignableProjectRole, + RoleNamespace, +} from '@n8n/permissions'; import { combineScopes, getAuthPrincipalScopes, @@ -29,6 +34,7 @@ import { UnexpectedError, UserError } from 'n8n-workflow'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { RoleCacheService } from './role-cache.service'; @Service() export class RoleService { @@ -36,6 +42,7 @@ export class RoleService { private readonly license: LicenseState, private readonly roleRepository: RoleRepository, private readonly scopeRepository: ScopeRepository, + private readonly roleCacheService: RoleCacheService, ) {} private dbRoleToRoleDTO(role: Role, usedByUsers?: number): RoleDTO { @@ -82,6 +89,10 @@ export class RoleService { throw new BadRequestError('Cannot delete system roles'); } await this.roleRepository.removeBySlug(slug); + + // Invalidate cache after role deletion + await this.roleCacheService.invalidateCache(); + return this.dbRoleToRoleDTO(role); } @@ -113,6 +124,9 @@ export class RoleService { scopes: await this.resolveScopes(scopeSlugs), }); + // Invalidate cache after role update + await this.roleCacheService.invalidateCache(); + return this.dbRoleToRoleDTO(updatedRole); } catch (error) { if (error instanceof UserError && error.message === 'Role not found') { @@ -139,6 +153,10 @@ export class RoleService { role.roleType = newRole.roleType; role.slug = `${newRole.roleType}:${newRole.displayName.toLowerCase().replace(/[^a-z0-9]+/g, '-')}-${Math.random().toString(36).substring(2, 8)}`; const createdRole = await this.roleRepository.save(role); + + // Invalidate cache after role creation + await this.roleCacheService.invalidateCache(); + return this.dbRoleToRoleDTO(createdRole); } @@ -246,6 +264,18 @@ export class RoleService { return [...scopesSet].sort(); } + /** + * Enhanced rolesWithScope function that combines static roles with database roles + * This replaces the original rolesWithScope function from @n8n/permissions + */ + async rolesWithScope(namespace: RoleNamespace, scopes: Scope | Scope[]): Promise { + if (!Array.isArray(scopes)) { + scopes = [scopes]; + } + // Get database roles from cache + return await this.roleCacheService.getRolesWithAllScopes(namespace, scopes); + } + isRoleLicensed(role: AssignableProjectRole) { // TODO: move this info into FrontendSettings diff --git a/packages/cli/src/workflows/workflow-finder.service.ts b/packages/cli/src/workflows/workflow-finder.service.ts index 15ccc97c85..5679dbeec1 100644 --- a/packages/cli/src/workflows/workflow-finder.service.ts +++ b/packages/cli/src/workflows/workflow-finder.service.ts @@ -1,17 +1,20 @@ import type { SharedWorkflow, User } from '@n8n/db'; import { SharedWorkflowRepository, FolderRepository } from '@n8n/db'; import { Service } from '@n8n/di'; -import { hasGlobalScope, rolesWithScope, type Scope } from '@n8n/permissions'; +import { hasGlobalScope, type Scope } from '@n8n/permissions'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import type { EntityManager, FindOptionsWhere } from '@n8n/typeorm'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { In } from '@n8n/typeorm'; +import { RoleService } from '@/services/role.service'; + @Service() export class WorkflowFinderService { constructor( private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly folderRepository: FolderRepository, + private readonly roleService: RoleService, ) {} async findWorkflowForUser( @@ -27,8 +30,10 @@ export class WorkflowFinderService { let where: FindOptionsWhere = {}; if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { - const projectRoles = rolesWithScope('project', scopes); - const workflowRoles = rolesWithScope('workflow', scopes); + const [projectRoles, workflowRoles] = await Promise.all([ + this.roleService.rolesWithScope('project', scopes), + this.roleService.rolesWithScope('workflow', scopes), + ]); where = { role: In(workflowRoles), @@ -78,8 +83,10 @@ export class WorkflowFinderService { } if (!hasGlobalScope(user, scopes, { mode: 'allOf' })) { - const projectRoles = rolesWithScope('project', scopes); - const workflowRoles = rolesWithScope('workflow', scopes); + const [projectRoles, workflowRoles] = await Promise.all([ + this.roleService.rolesWithScope('project', scopes), + this.roleService.rolesWithScope('workflow', scopes), + ]); where = { ...where, diff --git a/packages/cli/src/workflows/workflow-sharing.service.ts b/packages/cli/src/workflows/workflow-sharing.service.ts index 1ce3767757..429c8c87b7 100644 --- a/packages/cli/src/workflows/workflow-sharing.service.ts +++ b/packages/cli/src/workflows/workflow-sharing.service.ts @@ -3,7 +3,6 @@ import { ProjectRelationRepository, SharedWorkflowRepository } from '@n8n/db'; import { Service } from '@n8n/di'; import { hasGlobalScope, - rolesWithScope, type ProjectRole, type WorkflowSharingRole, type Scope, @@ -47,9 +46,13 @@ export class WorkflowSharingService { } const projectRoles = - 'scopes' in options ? rolesWithScope('project', options.scopes) : options.projectRoles; + 'scopes' in options + ? await this.roleService.rolesWithScope('project', options.scopes) + : options.projectRoles; const workflowRoles = - 'scopes' in options ? rolesWithScope('workflow', options.scopes) : options.workflowRoles; + 'scopes' in options + ? await this.roleService.rolesWithScope('workflow', options.scopes) + : options.workflowRoles; const sharedWorkflows = await this.sharedWorkflowRepository.find({ where: { diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 85abdbcfe8..59e4b93aa2 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -140,8 +140,6 @@ export class WorkflowsController { let project: Project | null; const savedWorkflow = await dbManager.transaction(async (transactionManager) => { - const workflow = await transactionManager.save(newWorkflow); - const { projectId, parentFolderId } = req.body; project = projectId === undefined @@ -164,6 +162,8 @@ export class WorkflowsController { throw new UnexpectedError('No personal project found'); } + const workflow = await transactionManager.save(newWorkflow); + if (parentFolderId) { try { const parentFolder = await this.folderService.findFolderInProjectOrFail( diff --git a/packages/cli/test/integration/credentials/credentials.api.ee.test.ts b/packages/cli/test/integration/credentials/credentials.api.ee.test.ts index 69d487489d..edbe31af3a 100644 --- a/packages/cli/test/integration/credentials/credentials.api.ee.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.ee.test.ts @@ -34,6 +34,7 @@ import { } from '../shared/db/users'; import type { SaveCredentialFunction, SuperAgentTest } from '../shared/types'; import * as utils from '../shared/utils'; +import { RoleCacheService } from '@/services/role-cache.service'; const testServer = utils.setupTestServer({ endpointGroups: ['credentials'], @@ -59,6 +60,10 @@ const mailer = mockInstance(UserManagementMailer); let projectService: ProjectService; let projectRepository: ProjectRepository; +beforeAll(async () => { + await Container.get(RoleCacheService).refreshCache(); +}); + beforeEach(async () => { await testDb.truncate(['SharedCredentials', 'CredentialsEntity', 'Project', 'ProjectRelation']); projectRepository = Container.get(ProjectRepository); diff --git a/packages/cli/test/integration/shared/db/roles.ts b/packages/cli/test/integration/shared/db/roles.ts index b9a3b4cba7..8a221fa569 100644 --- a/packages/cli/test/integration/shared/db/roles.ts +++ b/packages/cli/test/integration/shared/db/roles.ts @@ -49,6 +49,32 @@ export async function createCustomRoleWithScopes( }); } +/** + * Creates a custom role with specific scope slugs (using existing permission system scopes) + */ +export async function createCustomRoleWithScopeSlugs( + scopeSlugs: string[], + overrides: Partial = {}, +): Promise { + const scopeRepository = Container.get(ScopeRepository); + + // Find existing scopes by their slugs + const scopes = await scopeRepository.findByList(scopeSlugs); + + if (scopes.length !== scopeSlugs.length) { + const missingScopes = scopeSlugs.filter((slug) => !scopes.some((scope) => scope.slug === slug)); + throw new Error( + `Could not find all scopes. Expected ${scopeSlugs.length}, found ${scopes.length}, missing: ${missingScopes.join(', ')}`, + ); + } + + return await createRole({ + scopes, + systemRole: false, + ...overrides, + }); +} + /** * Creates a test scope with given parameters */