From 027edbe89da29d773d20f82d946875314701064b Mon Sep 17 00:00:00 2001 From: Guillaume Jacquart Date: Mon, 1 Sep 2025 10:22:33 +0200 Subject: [PATCH] feat(core): Allow custom project roles from being set to a user project relation (#18926) --- .../invite-users-request.dto.test.ts | 3 +- .../invitation/invite-users-request.dto.ts | 5 +- .../add-users-to-project.dto.test.ts | 2 +- .../change-user-role-in-project.dto.test.ts | 7 - .../__tests__/update-project.dto.test.ts | 2 +- .../change-user-role-in-project.dto.ts | 4 +- .../__tests__/role-change-request.dto.test.ts | 25 +-- .../src/dto/user/role-change-request.dto.ts | 11 +- .../schemas/__tests__/project.schema.test.ts | 2 +- .../api-types/src/schemas/project.schema.ts | 4 +- .../backend-test-utils/src/db/projects.ts | 11 +- packages/@n8n/db/src/entities/types-db.ts | 6 - .../db/src/repositories/role.repository.ts | 9 +- .../permissions/src/__tests__/schemas.test.ts | 21 +- packages/@n8n/permissions/src/index.ts | 3 + packages/@n8n/permissions/src/schemas.ee.ts | 33 ++- packages/@n8n/permissions/src/types.ee.ts | 7 +- .../cli/src/controllers/project.controller.ts | 20 +- .../users/spec/paths/users.id.role.yml | 2 +- .../v1/handlers/users/spec/paths/users.yml | 2 +- packages/cli/src/requests.ts | 6 +- .../services/__tests__/user.service.test.ts | 63 +++++- .../cli/src/services/project.service.ee.ts | 47 ++++- packages/cli/src/services/role.service.ts | 41 +++- packages/cli/src/services/user.service.ts | 11 + .../__tests__/user-management-mailer.test.ts | 7 +- .../email/user-management-mailer.ts | 11 +- .../folder/folder.controller.test.ts | 14 +- .../integration/public-api/projects.test.ts | 43 +++- .../test/integration/public-api/users.test.ts | 70 +++++- .../services/project.service.test.ts | 199 +++++++++++++++++- .../cli/test/integration/users.api.test.ts | 27 +++ 32 files changed, 597 insertions(+), 121 deletions(-) diff --git a/packages/@n8n/api-types/src/dto/invitation/__tests__/invite-users-request.dto.test.ts b/packages/@n8n/api-types/src/dto/invitation/__tests__/invite-users-request.dto.test.ts index f47a138ed5..103e9bb4ab 100644 --- a/packages/@n8n/api-types/src/dto/invitation/__tests__/invite-users-request.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/invitation/__tests__/invite-users-request.dto.test.ts @@ -16,6 +16,7 @@ describe('InviteUsersRequestDto', () => { request: [ { email: 'user1@example.com', role: 'global:member' }, { email: 'user2@example.com', role: 'global:admin' }, + { email: 'user3@example.com', role: 'custom:role' }, ], }, ])('should validate $name', ({ request }) => { @@ -42,7 +43,7 @@ describe('InviteUsersRequestDto', () => { request: [ { email: 'user@example.com', - role: 'invalid-role', + role: 'global:owner', }, ], expectedErrorPath: [0, 'role'], diff --git a/packages/@n8n/api-types/src/dto/invitation/invite-users-request.dto.ts b/packages/@n8n/api-types/src/dto/invitation/invite-users-request.dto.ts index 9693234c64..e25e95c5e2 100644 --- a/packages/@n8n/api-types/src/dto/invitation/invite-users-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/invitation/invite-users-request.dto.ts @@ -1,10 +1,9 @@ +import { assignableGlobalRoleSchema } from '@n8n/permissions'; import { z } from 'zod'; -const roleSchema = z.enum(['global:member', 'global:admin']); - const invitedUserSchema = z.object({ email: z.string().email(), - role: roleSchema.default('global:member'), + role: assignableGlobalRoleSchema.default('global:member'), }); const invitationsSchema = z.array(invitedUserSchema); diff --git a/packages/@n8n/api-types/src/dto/project/__tests__/add-users-to-project.dto.test.ts b/packages/@n8n/api-types/src/dto/project/__tests__/add-users-to-project.dto.test.ts index 5a303f5882..dc166fa7db 100644 --- a/packages/@n8n/api-types/src/dto/project/__tests__/add-users-to-project.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/project/__tests__/add-users-to-project.dto.test.ts @@ -93,7 +93,7 @@ describe('AddUsersToProjectDto', () => { relations: [ { userId: 'user-123', - role: 'invalid-role', + role: '', }, ], }, diff --git a/packages/@n8n/api-types/src/dto/project/__tests__/change-user-role-in-project.dto.test.ts b/packages/@n8n/api-types/src/dto/project/__tests__/change-user-role-in-project.dto.test.ts index 27c7a938f2..10d017d9a5 100644 --- a/packages/@n8n/api-types/src/dto/project/__tests__/change-user-role-in-project.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/project/__tests__/change-user-role-in-project.dto.test.ts @@ -31,13 +31,6 @@ describe('ChangeUserRoleInProject', () => { }, expectedErrorPath: ['role'], }, - { - name: 'invalid role value', - request: { - role: 'invalid-role', - }, - expectedErrorPath: ['role'], - }, { name: 'personal owner role', request: { role: PROJECT_OWNER_ROLE_SLUG }, diff --git a/packages/@n8n/api-types/src/dto/project/__tests__/update-project.dto.test.ts b/packages/@n8n/api-types/src/dto/project/__tests__/update-project.dto.test.ts index f2bb497451..72a839ff46 100644 --- a/packages/@n8n/api-types/src/dto/project/__tests__/update-project.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/project/__tests__/update-project.dto.test.ts @@ -120,7 +120,7 @@ describe('UpdateProjectDto', () => { relations: [ { userId: 'user-123', - role: 'invalid-role', + role: 'project:personalOwner', }, ], }, diff --git a/packages/@n8n/api-types/src/dto/project/change-user-role-in-project.dto.ts b/packages/@n8n/api-types/src/dto/project/change-user-role-in-project.dto.ts index ab541ecb8e..840773499e 100644 --- a/packages/@n8n/api-types/src/dto/project/change-user-role-in-project.dto.ts +++ b/packages/@n8n/api-types/src/dto/project/change-user-role-in-project.dto.ts @@ -1,6 +1,6 @@ -import { teamRoleSchema } from '@n8n/permissions'; +import { assignableProjectRoleSchema } from '@n8n/permissions'; import { Z } from 'zod-class'; export class ChangeUserRoleInProject extends Z.class({ - role: teamRoleSchema, + role: assignableProjectRoleSchema, }) {} diff --git a/packages/@n8n/api-types/src/dto/user/__tests__/role-change-request.dto.test.ts b/packages/@n8n/api-types/src/dto/user/__tests__/role-change-request.dto.test.ts index 87ed9799d1..78062393fc 100644 --- a/packages/@n8n/api-types/src/dto/user/__tests__/role-change-request.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/user/__tests__/role-change-request.dto.test.ts @@ -11,27 +11,28 @@ describe('RoleChangeRequestDto', () => { expect(result.error?.issues[0].message).toBe('New role is required'); }); - it('should fail validation with invalid newRoleName', () => { + it('should fail validation with invalid newRoleName global:owner', () => { const data = { - newRoleName: 'invalidRole', + newRoleName: 'global:owner', }; const result = RoleChangeRequestDto.safeParse(data); expect(result.success).toBe(false); expect(result.error?.issues[0].path[0]).toBe('newRoleName'); - expect(result.error?.issues[0].message).toBe( - "Invalid enum value. Expected 'global:admin' | 'global:member', received 'invalidRole'", - ); + expect(result.error?.issues[0].message).toBe('This global role value is not assignable'); }); - it('should pass validation with valid data', () => { - const data = { - newRoleName: 'global:admin', - }; + it.each(['global:admin', 'custom:role'])( + 'should pass validation with valid newRoleName %s', + (role) => { + const data = { + newRoleName: role, + }; - const result = RoleChangeRequestDto.safeParse(data); + const result = RoleChangeRequestDto.safeParse(data); - expect(result.success).toBe(true); - }); + expect(result.success).toBe(true); + }, + ); }); diff --git a/packages/@n8n/api-types/src/dto/user/role-change-request.dto.ts b/packages/@n8n/api-types/src/dto/user/role-change-request.dto.ts index 4ddae2cd58..e744cbf93c 100644 --- a/packages/@n8n/api-types/src/dto/user/role-change-request.dto.ts +++ b/packages/@n8n/api-types/src/dto/user/role-change-request.dto.ts @@ -1,8 +1,11 @@ -import { z } from 'zod'; +import { assignableGlobalRoleSchema } from '@n8n/permissions'; import { Z } from 'zod-class'; export class RoleChangeRequestDto extends Z.class({ - newRoleName: z.enum(['global:admin', 'global:member'], { - required_error: 'New role is required', - }), + newRoleName: assignableGlobalRoleSchema + // enforce required (non-nullable, non-optional) with custom error message on undefined + .nullish() + .refine((val): val is NonNullable => val !== null && typeof val !== 'undefined', { + message: 'New role is required', + }), }) {} diff --git a/packages/@n8n/api-types/src/schemas/__tests__/project.schema.test.ts b/packages/@n8n/api-types/src/schemas/__tests__/project.schema.test.ts index 87c9d8f6d5..8e5149cceb 100644 --- a/packages/@n8n/api-types/src/schemas/__tests__/project.schema.test.ts +++ b/packages/@n8n/api-types/src/schemas/__tests__/project.schema.test.ts @@ -82,7 +82,7 @@ describe('project.schema', () => { }, { name: 'invalid role', - value: { userId: 'user-123', role: 'invalid-role' }, + value: { userId: 'user-123', role: 'project:personalOwner' }, expected: false, }, { diff --git a/packages/@n8n/api-types/src/schemas/project.schema.ts b/packages/@n8n/api-types/src/schemas/project.schema.ts index 55545e7271..69352e2a01 100644 --- a/packages/@n8n/api-types/src/schemas/project.schema.ts +++ b/packages/@n8n/api-types/src/schemas/project.schema.ts @@ -1,4 +1,4 @@ -import { teamRoleSchema } from '@n8n/permissions'; +import { assignableProjectRoleSchema } from '@n8n/permissions'; import { z } from 'zod'; export const projectNameSchema = z.string().min(1).max(255); @@ -16,6 +16,6 @@ export const projectDescriptionSchema = z.string().max(512); export const projectRelationSchema = z.object({ userId: z.string().min(1), - role: teamRoleSchema, + role: assignableProjectRoleSchema, }); export type ProjectRelation = z.infer; diff --git a/packages/@n8n/backend-test-utils/src/db/projects.ts b/packages/@n8n/backend-test-utils/src/db/projects.ts index 7da5b2863f..59f681ccaa 100644 --- a/packages/@n8n/backend-test-utils/src/db/projects.ts +++ b/packages/@n8n/backend-test-utils/src/db/projects.ts @@ -1,11 +1,16 @@ import type { Project, User, ProjectRelation } from '@n8n/db'; import { ProjectRelationRepository, ProjectRepository } from '@n8n/db'; import { Container } from '@n8n/di'; -import { PROJECT_OWNER_ROLE_SLUG, type CustomRole } from '@n8n/permissions'; +import type { AssignableProjectRole } from '@n8n/permissions'; +import { PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions'; import { randomName } from '../random'; -export const linkUserToProject = async (user: User, project: Project, role: CustomRole) => { +export const linkUserToProject = async ( + user: User, + project: Project, + role: AssignableProjectRole, +) => { const projectRelationRepository = Container.get(ProjectRelationRepository); await projectRelationRepository.save( projectRelationRepository.create({ @@ -68,7 +73,7 @@ export const getProjectRelations = async ({ export const getProjectRoleForUser = async ( projectId: string, userId: string, -): Promise => { +): Promise => { return ( await Container.get(ProjectRelationRepository).findOne({ where: { projectId, userId }, diff --git a/packages/@n8n/db/src/entities/types-db.ts b/packages/@n8n/db/src/entities/types-db.ts index ed51b3d292..859499a88f 100644 --- a/packages/@n8n/db/src/entities/types-db.ts +++ b/packages/@n8n/db/src/entities/types-db.ts @@ -327,12 +327,6 @@ export namespace ListQuery { }; } -export type ProjectRole = - | 'project:personalOwner' - | 'project:admin' - | 'project:editor' - | 'project:viewer'; - export interface IGetExecutionsQueryFilter { id?: FindOperator | string; finished?: boolean; diff --git a/packages/@n8n/db/src/repositories/role.repository.ts b/packages/@n8n/db/src/repositories/role.repository.ts index cd6419b674..8ad461eae2 100644 --- a/packages/@n8n/db/src/repositories/role.repository.ts +++ b/packages/@n8n/db/src/repositories/role.repository.ts @@ -1,6 +1,6 @@ import { DatabaseConfig } from '@n8n/config'; import { Service } from '@n8n/di'; -import { DataSource, EntityManager, Repository } from '@n8n/typeorm'; +import { DataSource, EntityManager, In, Repository } from '@n8n/typeorm'; import { UserError } from 'n8n-workflow'; import { Role } from '../entities'; @@ -25,6 +25,13 @@ export class RoleRepository extends Repository { }); } + async findBySlugs(slugs: string[], roleType: 'global' | 'project' | 'workflow' | 'credential') { + return await this.find({ + where: { slug: In(slugs), roleType }, + relations: ['scopes'], + }); + } + async removeBySlug(slug: string) { const result = await this.delete({ slug }); if (result.affected !== 1) { diff --git a/packages/@n8n/permissions/src/__tests__/schemas.test.ts b/packages/@n8n/permissions/src/__tests__/schemas.test.ts index 1add35ced7..40d5b53eeb 100644 --- a/packages/@n8n/permissions/src/__tests__/schemas.test.ts +++ b/packages/@n8n/permissions/src/__tests__/schemas.test.ts @@ -9,9 +9,10 @@ import { roleNamespaceSchema, globalRoleSchema, assignableGlobalRoleSchema, - projectRoleSchema, + systemProjectRoleSchema, credentialSharingRoleSchema, workflowSharingRoleSchema, + customProjectRoleSchema, } from '../schemas.ee'; describe('roleNamespaceSchema', () => { @@ -49,8 +50,6 @@ describe('assignableGlobalRoleSchema', () => { { name: 'excluded role: global:owner', value: 'global:owner', expected: false }, { name: 'valid role: global:admin', value: 'global:admin', expected: true }, { name: 'valid role: global:member', value: 'global:member', expected: true }, - { name: 'invalid role', value: 'global:invalid', expected: false }, - { name: 'invalid prefix', value: 'invalid:admin', expected: false }, { name: 'object value', value: {}, expected: false }, ])('should validate $name', ({ value, expected }) => { const result = assignableGlobalRoleSchema.safeParse(value); @@ -58,7 +57,7 @@ describe('assignableGlobalRoleSchema', () => { }); }); -describe('projectRoleSchema', () => { +describe('systemProjectRoleSchema', () => { test.each([ { name: `valid role: ${PROJECT_OWNER_ROLE_SLUG}`, @@ -82,7 +81,7 @@ describe('projectRoleSchema', () => { }, { name: 'invalid role', value: 'invalid-role', expected: false }, ])('should validate $name', ({ value, expected }) => { - const result = projectRoleSchema.safeParse(value); + const result = systemProjectRoleSchema.safeParse(value); expect(result.success).toBe(expected); }); }); @@ -114,3 +113,15 @@ describe('workflowSharingRoleSchema', () => { expect(result.success).toBe(expected); }); }); + +describe('customProjectRoleSchema', () => { + test.each([ + { name: 'valid role: custom:role', value: 'custom:role', expected: true }, + { name: 'undefined value', value: undefined, expected: false }, + { name: 'empty string', value: '', expected: false }, + { name: 'system role', value: PROJECT_ADMIN_ROLE_SLUG, expected: false }, + ])('should validate $name', ({ value, expected }) => { + const result = customProjectRoleSchema.safeParse(value); + expect(result.success).toBe(expected); + }); +}); diff --git a/packages/@n8n/permissions/src/index.ts b/packages/@n8n/permissions/src/index.ts index da3dd0b12e..2bc9f45640 100644 --- a/packages/@n8n/permissions/src/index.ts +++ b/packages/@n8n/permissions/src/index.ts @@ -7,6 +7,9 @@ export * from './roles/role-maps.ee'; export * from './roles/all-roles'; export { + systemProjectRoleSchema, + assignableProjectRoleSchema, + assignableGlobalRoleSchema, projectRoleSchema, teamRoleSchema, roleSchema, diff --git a/packages/@n8n/permissions/src/schemas.ee.ts b/packages/@n8n/permissions/src/schemas.ee.ts index bcb7418bb2..ea97f23110 100644 --- a/packages/@n8n/permissions/src/schemas.ee.ts +++ b/packages/@n8n/permissions/src/schemas.ee.ts @@ -7,21 +7,42 @@ export const roleNamespaceSchema = z.enum(['global', 'project', 'credential', 'w export const globalRoleSchema = z.enum(['global:owner', 'global:admin', 'global:member']); -export const assignableGlobalRoleSchema = globalRoleSchema.exclude([ - 'global:owner', // Owner cannot be changed +const customGlobalRoleSchema = z + .string() + .nonempty() + .refine((val) => !globalRoleSchema.safeParse(val).success, { + message: 'This global role value is not assignable', + }); + +export const assignableGlobalRoleSchema = z.union([ + globalRoleSchema.exclude([ + 'global:owner', // Owner cannot be changed + ]), + customGlobalRoleSchema, ]); export const personalRoleSchema = z.enum([ 'project:personalOwner', // personalOwner is only used for personal projects ]); +// Those are the system roles for projects assignable to a user export const teamRoleSchema = z.enum(['project:admin', 'project:editor', 'project:viewer']); -export const customRoleSchema = z.string().refine((val) => val !== PROJECT_OWNER_ROLE_SLUG, { - message: `'${PROJECT_OWNER_ROLE_SLUG}' is not assignable`, -}); +// Custom project role can be anything but the system roles +export const customProjectRoleSchema = z + .string() + .nonempty() + .refine((val) => val !== PROJECT_OWNER_ROLE_SLUG && !teamRoleSchema.safeParse(val).success, { + message: 'This global role value is not assignable', + }); -export const projectRoleSchema = z.union([personalRoleSchema, teamRoleSchema]); +// Those are all the system roles for projects +export const systemProjectRoleSchema = z.union([personalRoleSchema, teamRoleSchema]); + +// Those are the roles that can be assigned to a user for a project (all roles except personalOwner) +export const assignableProjectRoleSchema = z.union([teamRoleSchema, customProjectRoleSchema]); + +export const projectRoleSchema = z.union([systemProjectRoleSchema, customProjectRoleSchema]); export const credentialSharingRoleSchema = z.enum(['credential:owner', 'credential:user']); diff --git a/packages/@n8n/permissions/src/types.ee.ts b/packages/@n8n/permissions/src/types.ee.ts index 2c4aff009f..c3e2d4edd2 100644 --- a/packages/@n8n/permissions/src/types.ee.ts +++ b/packages/@n8n/permissions/src/types.ee.ts @@ -5,11 +5,12 @@ import type { assignableGlobalRoleSchema, credentialSharingRoleSchema, globalRoleSchema, - projectRoleSchema, Role, + systemProjectRoleSchema, roleNamespaceSchema, teamRoleSchema, workflowSharingRoleSchema, + assignableProjectRoleSchema, } from './schemas.ee'; import { ALL_API_KEY_SCOPES } from './scope-information'; @@ -58,8 +59,8 @@ export type AssignableGlobalRole = z.infer; export type CredentialSharingRole = z.infer; export type WorkflowSharingRole = z.infer; export type TeamProjectRole = z.infer; -export type ProjectRole = z.infer; -export type CustomRole = string; +export type ProjectRole = z.infer; +export type AssignableProjectRole = z.infer; /** Union of all possible role types in the system */ export type AllRoleTypes = GlobalRole | ProjectRole | WorkflowSharingRole | CredentialSharingRole; diff --git a/packages/cli/src/controllers/project.controller.ts b/packages/cli/src/controllers/project.controller.ts index a91cef1566..316d5ccd81 100644 --- a/packages/cli/src/controllers/project.controller.ts +++ b/packages/cli/src/controllers/project.controller.ts @@ -64,7 +64,10 @@ export class ProjectController { uiContext: payload.uiContext, }); - const relations = await this.projectsService.getProjectRelations(project.id); + const relation = await this.projectsService.getProjectRelationForUserAndProject( + req.user.id, + project.id, + ); return { ...project, @@ -72,10 +75,7 @@ export class ProjectController { scopes: [ ...combineScopes({ global: getAuthPrincipalScopes(req.user), - project: - relations - .find((pr) => pr.userId === req.user.id) - ?.role.scopes.map((scope) => scope.slug) || [], + project: relation?.role.scopes.map((scope) => scope.slug) ?? [], }), ], }; @@ -156,14 +156,14 @@ export class ProjectController { throw new NotFoundError('Could not find a personal project for this user'); } - const relations = await this.projectsService.getProjectRelations(project.id); + const relation = await this.projectsService.getProjectRelationForUserAndProject( + req.user.id, + project.id, + ); const scopes: Scope[] = [ ...combineScopes({ global: getAuthPrincipalScopes(req.user), - project: - relations - .find((pr) => pr.userId === req.user.id) - ?.role.scopes.map((scope) => scope.slug) ?? [], + project: relation?.role.scopes.map((scope) => scope.slug) ?? [], }), ]; return { diff --git a/packages/cli/src/public-api/v1/handlers/users/spec/paths/users.id.role.yml b/packages/cli/src/public-api/v1/handlers/users/spec/paths/users.id.role.yml index 92993adf7f..b7cbf4a98b 100644 --- a/packages/cli/src/public-api/v1/handlers/users/spec/paths/users.id.role.yml +++ b/packages/cli/src/public-api/v1/handlers/users/spec/paths/users.id.role.yml @@ -17,7 +17,7 @@ patch: properties: newRoleName: type: string - enum: [global:admin, global:member] + example: global:member required: - newRoleName responses: diff --git a/packages/cli/src/public-api/v1/handlers/users/spec/paths/users.yml b/packages/cli/src/public-api/v1/handlers/users/spec/paths/users.yml index e767ab33cc..7a91968615 100644 --- a/packages/cli/src/public-api/v1/handlers/users/spec/paths/users.yml +++ b/packages/cli/src/public-api/v1/handlers/users/spec/paths/users.yml @@ -48,7 +48,7 @@ post: format: email role: type: string - enum: [global:admin, global:member] + example: global:member required: - email responses: diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 2621d28508..dc35e87898 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -10,7 +10,7 @@ import type { } from '@n8n/db'; import type { AssignableGlobalRole, - CustomRole, + AssignableProjectRole, GlobalRole, ProjectRole, Scope, @@ -275,7 +275,7 @@ export declare namespace ActiveWorkflowRequest { export declare namespace ProjectRequest { type GetMyProjectsResponse = Array< - Project & { role: ProjectRole | GlobalRole | CustomRole; scopes?: Scope[] } + Project & { role: ProjectRole | AssignableProjectRole | GlobalRole; scopes?: Scope[] } >; type ProjectRelationResponse = { @@ -283,7 +283,7 @@ export declare namespace ProjectRequest { email: string; firstName: string; lastName: string; - role: ProjectRole | CustomRole; + role: ProjectRole | AssignableProjectRole; }; type ProjectWithRelations = { id: string; diff --git a/packages/cli/src/services/__tests__/user.service.test.ts b/packages/cli/src/services/__tests__/user.service.test.ts index 5cf2025556..7a609e29f6 100644 --- a/packages/cli/src/services/__tests__/user.service.test.ts +++ b/packages/cli/src/services/__tests__/user.service.test.ts @@ -1,12 +1,17 @@ import { mockInstance } from '@n8n/backend-test-utils'; import { GlobalConfig } from '@n8n/config'; -import { GLOBAL_MEMBER_ROLE, User, UserRepository } from '@n8n/db'; +import type { Project } from '@n8n/db'; +import { GLOBAL_ADMIN_ROLE, GLOBAL_MEMBER_ROLE, User, UserRepository } from '@n8n/db'; +import type { EntityManager } from '@n8n/typeorm'; import { mock } from 'jest-mock-extended'; import { v4 as uuid } from 'uuid'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { UrlService } from '@/services/url.service'; import { UserService } from '@/services/user.service'; +import type { RoleService } from '../role.service'; + describe('UserService', () => { const globalConfig = mockInstance(GlobalConfig, { host: 'localhost', @@ -17,8 +22,22 @@ describe('UserService', () => { editorBaseUrl: '', }); const urlService = new UrlService(globalConfig); - const userRepository = mockInstance(UserRepository); - const userService = new UserService(mock(), userRepository, mock(), urlService, mock(), mock()); + const userRepository = mockInstance(UserRepository, { + manager: mock({ + transaction: async (cb) => + typeof cb === 'function' ? await cb(mock()) : await Promise.resolve(), + }), + }); + const roleService = mock(); + const userService = new UserService( + mock(), + userRepository, + mock(), + urlService, + mock(), + mock(), + roleService, + ); const commonMockUser = Object.assign(new User(), { id: uuid(), @@ -107,4 +126,42 @@ describe('UserService', () => { expect(userRepository.update).not.toHaveBeenCalled(); }); }); + + describe('inviteUsers', () => { + it('should invite users', async () => { + const owner = Object.assign(new User(), { id: uuid() }); + const invitations = [ + { email: 'test1@example.com', role: GLOBAL_ADMIN_ROLE.slug }, + { email: 'test2@example.com', role: GLOBAL_MEMBER_ROLE.slug }, + { email: 'test3@example.com', role: 'custom:role' }, + ]; + + roleService.checkRolesExist.mockResolvedValue(); + userRepository.findManyByEmail.mockResolvedValue([]); + userRepository.createUserWithProject.mockImplementation(async (userData) => { + return { user: { ...userData, id: uuid() } as User, project: mock() }; + }); + + await userService.inviteUsers(owner, invitations); + + expect(userRepository.createUserWithProject).toHaveBeenCalledTimes(3); + }); + + it('should fail if role do not exist', async () => { + const owner = Object.assign(new User(), { id: uuid() }); + const invitations = [{ email: 'test1@example.com', role: 'nonexistent:role' }]; + + roleService.checkRolesExist.mockRejectedValue( + new BadRequestError('Role nonexistent:role does not exist'), + ); + userRepository.findManyByEmail.mockResolvedValue([]); + userRepository.createUserWithProject.mockImplementation(async (userData) => { + return { user: { ...userData, id: uuid() } as User, project: mock() }; + }); + + await expect(userService.inviteUsers(owner, invitations)).rejects.toThrowError( + 'Role nonexistent:role does not exist', + ); + }); + }); }); diff --git a/packages/cli/src/services/project.service.ee.ts b/packages/cli/src/services/project.service.ee.ts index ea014e7123..3141143400 100644 --- a/packages/cli/src/services/project.service.ee.ts +++ b/packages/cli/src/services/project.service.ee.ts @@ -17,7 +17,7 @@ import { rolesWithScope, type Scope, type ProjectRole, - CustomRole, + AssignableProjectRole, PROJECT_OWNER_ROLE_SLUG, PROJECT_ADMIN_ROLE_SLUG, } from '@n8n/permissions'; @@ -43,7 +43,7 @@ export class TeamProjectOverQuotaError extends UserError { } export class UnlicensedProjectRoleError extends UserError { - constructor(role: ProjectRole | CustomRole) { + constructor(role: ProjectRole | AssignableProjectRole) { super(`Your instance is not licensed to use role "${role}".`); } } @@ -272,11 +272,20 @@ export class ProjectService { async syncProjectRelations( projectId: string, - relations: Required['relations'], - ): Promise<{ project: Project; newRelations: Required['relations'] }> { + relations: Array<{ role: AssignableProjectRole; userId: string }>, + ): Promise<{ + project: Project; + newRelations: Array<{ role: AssignableProjectRole; userId: string }>; + }> { const project = await this.getTeamProjectWithRelations(projectId); this.checkRolesLicensed(project, relations); + // Check that all roles exist + await this.roleService.checkRolesExist( + relations.map((r) => r.role), + 'project', + ); + await this.projectRelationRepository.manager.transaction(async (em) => { await this.pruneRelations(em, project); await this.addManyRelations(em, project, relations); @@ -298,11 +307,17 @@ export class ProjectService { */ async addUsersToProject( projectId: string, - relations: Array<{ userId: string; role: ProjectRole | CustomRole }>, + relations: Array<{ userId: string; role: AssignableProjectRole }>, ) { const project = await this.getTeamProjectWithRelations(projectId); this.checkRolesLicensed(project, relations); + // Check that project role exists + await this.roleService.checkRolesExist( + relations.map((r) => r.role), + 'project', + ); + if (project.type === 'personal') { throw new ForbiddenError("Can't add users to personal projects."); } @@ -332,7 +347,7 @@ export class ProjectService { /** Check to see if the instance is licensed to use all roles provided */ private checkRolesLicensed( project: Project, - relations: Array<{ role: ProjectRole | CustomRole; userId: string }>, + relations: Array<{ role: AssignableProjectRole; userId: string }>, ) { for (const { role, userId } of relations) { const existing = project.projectRelations.find((pr) => pr.userId === userId); @@ -361,12 +376,16 @@ export class ProjectService { await this.projectRelationRepository.delete({ projectId: project.id, userId }); } - async changeUserRoleInProject(projectId: string, userId: string, role: ProjectRole) { + async changeUserRoleInProject(projectId: string, userId: string, role: AssignableProjectRole) { if (role === PROJECT_OWNER_ROLE_SLUG) { throw new ForbiddenError('Personal owner cannot be added to a team project.'); } const project = await this.getTeamProjectWithRelations(projectId); + + // Check that project role exists + await this.roleService.checkRolesExist([role], 'project'); + ProjectNotFoundError.isDefinedAndNotNull(project, projectId); const projectUserExists = project.projectRelations.some((r) => r.userId === userId); @@ -399,7 +418,7 @@ export class ProjectService { async addManyRelations( em: EntityManager, project: Project, - relations: Array<{ userId: string; role: ProjectRole | CustomRole }>, + relations: Array<{ userId: string; role: AssignableProjectRole }>, ) { await em.insert( ProjectRelation, @@ -450,7 +469,7 @@ export class ProjectService { */ async addUser( projectId: string, - { userId, role }: { userId: string; role: ProjectRole | CustomRole }, + { userId, role }: { userId: string; role: AssignableProjectRole }, trx?: EntityManager, ) { trx = trx ?? this.projectRelationRepository.manager; @@ -476,6 +495,16 @@ export class ProjectService { }); } + async getProjectRelationForUserAndProject( + userId: string, + projectId: string, + ): Promise { + return await this.projectRelationRepository.findOne({ + where: { projectId, userId }, + relations: { user: true, role: true }, + }); + } + async getUserOwnedOrAdminProjects(userId: string): Promise { return await this.projectRepository.find({ where: { diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index 7726e368da..2fc448c7f5 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -1,3 +1,4 @@ +import { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types'; import { CredentialsEntity, SharedCredentials, @@ -10,21 +11,24 @@ import { Role, Scope as DBScope, ScopeRepository, + GLOBAL_ADMIN_ROLE, } from '@n8n/db'; import { Service } from '@n8n/di'; -import type { CustomRole, ProjectRole, Scope, Role as RoleDTO } from '@n8n/permissions'; +import type { Scope, Role as RoleDTO, AssignableProjectRole } from '@n8n/permissions'; import { combineScopes, getAuthPrincipalScopes, getRoleScopes, isBuiltInRole, + PROJECT_ADMIN_ROLE_SLUG, + PROJECT_EDITOR_ROLE_SLUG, + PROJECT_VIEWER_ROLE_SLUG, } from '@n8n/permissions'; import { UnexpectedError, UserError } from 'n8n-workflow'; -import { License } from '@/license'; -import { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types'; -import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { License } from '@/license'; @Service() export class RoleService { @@ -124,6 +128,25 @@ export class RoleService { return this.dbRoleToRoleDTO(createdRole); } + async checkRolesExist( + roleSlugs: string[], + roleType: 'global' | 'project' | 'workflow' | 'credential', + ) { + const uniqueRoleSlugs = new Set(roleSlugs); + const roles = await this.roleRepository.findBySlugs(Array.from(uniqueRoleSlugs), roleType); + + if (roles.length < uniqueRoleSlugs.size) { + const nonExistentRoles = Array.from(uniqueRoleSlugs).filter( + (slug) => !roles.find((role) => role.slug === slug), + ); + throw new BadRequestError( + nonExistentRoles.length === 1 + ? `Role ${nonExistentRoles[0]} does not exist` + : `Roles ${nonExistentRoles.join(', ')} do not exist`, + ); + } + } + addScopes( rawWorkflow: ListQueryDb.Workflow.WithSharing | ListQueryDb.Workflow.WithOwnedByAndSharedWith, user: User, @@ -209,7 +232,7 @@ export class RoleService { return [...scopesSet].sort(); } - isRoleLicensed(role: ProjectRole | CustomRole) { + isRoleLicensed(role: AssignableProjectRole) { // TODO: move this info into FrontendSettings if (!isBuiltInRole(role)) { @@ -220,13 +243,13 @@ export class RoleService { } switch (role) { - case 'project:admin': + case PROJECT_ADMIN_ROLE_SLUG: return this.license.isProjectRoleAdminLicensed(); - case 'project:editor': + case PROJECT_EDITOR_ROLE_SLUG: return this.license.isProjectRoleEditorLicensed(); - case 'project:viewer': + case PROJECT_VIEWER_ROLE_SLUG: return this.license.isProjectRoleViewerLicensed(); - case 'global:admin': + case GLOBAL_ADMIN_ROLE.slug: return this.license.isAdvancedPermissionsLicensed(); default: // TODO: handle custom roles licensing diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index df5e591824..2fb06a3309 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -16,6 +16,7 @@ import { UrlService } from '@/services/url.service'; import { UserManagementMailer } from '@/user-management/email'; import { PublicApiKeyService } from './public-api-key.service'; +import { RoleService } from './role.service'; @Service() export class UserService { @@ -26,6 +27,7 @@ export class UserService { private readonly urlService: UrlService, private readonly eventService: EventService, private readonly publicApiKeyService: PublicApiKeyService, + private readonly roleService: RoleService, ) {} async update(userId: string, data: Partial) { @@ -210,6 +212,12 @@ export class UserService { : 'Creating 1 user shell...', ); + // Check that all roles in the invitations exist in the database + await this.roleService.checkRolesExist( + invitations.map(({ role }) => role), + 'global', + ); + try { await this.getManager().transaction( async (transactionManager) => @@ -246,6 +254,9 @@ export class UserService { } async changeUserRole(user: User, targetUser: User, newRole: RoleChangeRequestDto) { + // Check that new role exists + await this.roleService.checkRolesExist([newRole.newRoleName], 'global'); + return await this.userRepository.manager.transaction(async (trx) => { await trx.update(User, { id: targetUser.id }, { role: { slug: newRole.newRoleName } }); diff --git a/packages/cli/src/user-management/email/__tests__/user-management-mailer.test.ts b/packages/cli/src/user-management/email/__tests__/user-management-mailer.test.ts index 69370035fc..d0c50ce5d8 100644 --- a/packages/cli/src/user-management/email/__tests__/user-management-mailer.test.ts +++ b/packages/cli/src/user-management/email/__tests__/user-management-mailer.test.ts @@ -1,6 +1,7 @@ import { mockInstance } from '@n8n/backend-test-utils'; import type { GlobalConfig } from '@n8n/config'; -import type { ProjectRole, User, UserRepository } from '@n8n/db'; +import type { User, UserRepository } from '@n8n/db'; +import { PROJECT_EDITOR_ROLE_SLUG, PROJECT_VIEWER_ROLE_SLUG } from '@n8n/permissions'; import { mock } from 'jest-mock-extended'; import type { IWorkflowBase } from 'n8n-workflow'; @@ -157,8 +158,8 @@ describe('UserManagementMailer', () => { it('should send project share notifications', async () => { const sharer = mock({ firstName: 'Sharer', email: 'sharer@user.com' }); const newSharees = [ - { userId: 'recipient1', role: 'project:editor' as ProjectRole }, - { userId: 'recipient2', role: 'project:viewer' as ProjectRole }, + { userId: 'recipient1', role: PROJECT_EDITOR_ROLE_SLUG }, + { userId: 'recipient2', role: PROJECT_VIEWER_ROLE_SLUG }, ]; const project = { id: 'project1', name: 'Test Project' }; userRepository.getEmailsByIds.mockResolvedValue([ diff --git a/packages/cli/src/user-management/email/user-management-mailer.ts b/packages/cli/src/user-management/email/user-management-mailer.ts index 45b0fd9de9..764b8195ea 100644 --- a/packages/cli/src/user-management/email/user-management-mailer.ts +++ b/packages/cli/src/user-management/email/user-management-mailer.ts @@ -1,23 +1,24 @@ import { inTest, Logger } from '@n8n/backend-common'; import { GlobalConfig } from '@n8n/config'; -import type { ProjectRole, User } from '@n8n/db'; +import type { User } from '@n8n/db'; import { UserRepository } from '@n8n/db'; import { Container, Service } from '@n8n/di'; +import { AssignableProjectRole } from '@n8n/permissions'; import { existsSync } from 'fs'; import { readFile } from 'fs/promises'; import Handlebars from 'handlebars'; import type { IWorkflowBase } from 'n8n-workflow'; import { join as pathJoin } from 'path'; +import type { InviteEmailData, PasswordResetData, SendEmailResult } from './interfaces'; +import { NodeMailer } from './node-mailer'; + import { InternalServerError } from '@/errors/response-errors/internal-server.error'; import { EventService } from '@/events/event.service'; import type { RelayEventMap } from '@/events/maps/relay.event-map'; import { UrlService } from '@/services/url.service'; import { toError } from '@/utils'; -import type { InviteEmailData, PasswordResetData, SendEmailResult } from './interfaces'; -import { NodeMailer } from './node-mailer'; - type Template = HandlebarsTemplateDelegate; type TemplateName = | 'user-invited' @@ -193,7 +194,7 @@ export class UserManagementMailer { project, }: { sharer: User; - newSharees: Array<{ userId: string; role: ProjectRole }>; + newSharees: Array<{ userId: string; role: AssignableProjectRole }>; project: { id: string; name: string }; }): Promise { const recipients = await this.userRepository.getEmailsByIds(newSharees.map((s) => s.userId)); diff --git a/packages/cli/test/integration/folder/folder.controller.test.ts b/packages/cli/test/integration/folder/folder.controller.test.ts index 653a0d3b8a..29dd4759ce 100644 --- a/packages/cli/test/integration/folder/folder.controller.test.ts +++ b/packages/cli/test/integration/folder/folder.controller.test.ts @@ -9,13 +9,11 @@ import { testDb, mockInstance, } from '@n8n/backend-test-utils'; -import type { Project, ProjectRole, User } from '@n8n/db'; +import type { Project, User } from '@n8n/db'; import { FolderRepository, ProjectRepository, WorkflowRepository } from '@n8n/db'; import { Container } from '@n8n/di'; -import { DateTime } from 'luxon'; -import { ApplicationError, PROJECT_ROOT } from 'n8n-workflow'; - -import { ActiveWorkflowManager } from '@/active-workflow-manager'; +import type { ProjectRole } from '@n8n/permissions'; +import { PROJECT_EDITOR_ROLE_SLUG, PROJECT_VIEWER_ROLE_SLUG } from '@n8n/permissions'; import { createCredentials, getCredentialSharings, @@ -25,11 +23,15 @@ import { } from '@test-integration/db/credentials'; import { createFolder } from '@test-integration/db/folders'; import { createTag } from '@test-integration/db/tags'; +import { DateTime } from 'luxon'; +import { ApplicationError, PROJECT_ROOT } from 'n8n-workflow'; import { createOwner, createMember, createUser, createAdmin } from '../shared/db/users'; import type { SuperAgentTest } from '../shared/types'; import * as utils from '../shared/utils/'; +import { ActiveWorkflowManager } from '@/active-workflow-manager'; + let owner: User; let member: User; let authOwnerAgent: SuperAgentTest; @@ -1863,7 +1865,7 @@ describe('PUT /projects/:projectId/folders/:folderId/transfer', () => { .expect(404); }); - test.each(['project:editor', 'project:viewer'])( + test.each([PROJECT_EDITOR_ROLE_SLUG, PROJECT_VIEWER_ROLE_SLUG])( '%ss cannot transfer workflows', async (projectRole) => { // diff --git a/packages/cli/test/integration/public-api/projects.test.ts b/packages/cli/test/integration/public-api/projects.test.ts index 199f7c1eae..1b2953b0c8 100644 --- a/packages/cli/test/integration/public-api/projects.test.ts +++ b/packages/cli/test/integration/public-api/projects.test.ts @@ -483,8 +483,8 @@ describe('Projects in Public API', () => { relations: [ { userId: member.id, - // role does not exist - role: 'project:boss', + // field does not exist + invalidField: 'invalidValue', }, ], }; @@ -499,10 +499,33 @@ describe('Projects in Public API', () => { // ASSERT expect(response.body).toHaveProperty( 'message', - "Invalid enum value. Expected 'project:admin' | 'project:editor' | 'project:viewer', received 'project:boss'", + "request/body/relations/0 must have required property 'role'", ); }); + it('should reject if the relations have a role that do not exist', async () => { + const owner = await createOwnerWithApiKey(); + const member = await createMember(); + const project = await createTeamProject('shared-project', owner); + + const payload = { + relations: [ + { + userId: member.id, + role: 'project:invalid-role', + }, + ], + }; + + await testServer + .publicApiAgentFor(owner) + .post(`/projects/${project.id}/users`) + .send(payload) + .expect(400); + + // TODO: add message check once we properly validate role from database + }); + it('should reject with 404 if no project found', async () => { const owner = await createOwnerWithApiKey(); const member = await createMember(); @@ -654,23 +677,23 @@ describe('Projects in Public API', () => { testServer.license.enable('feat:projectRole:admin'); }); - it("should reject with 400 if the payload can't be validated", async () => { + it('should reject with 400 if the role do not exist', async () => { // ARRANGE const owner = await createOwnerWithApiKey(); + const member = await createMember(); + const project = await createTeamProject('shared-project', owner); + await linkUserToProject(member, project, 'project:viewer'); // ACT - const response = await testServer + await testServer .publicApiAgentFor(owner) - .patch('/projects/1234/users/1235') + .patch(`/projects/${project.id}/users/${member.id}`) // role does not exist .send({ role: 'project:boss' }) .expect(400); // ASSERT - expect(response.body).toHaveProperty( - 'message', - "Invalid enum value. Expected 'project:admin' | 'project:editor' | 'project:viewer', received 'project:boss'", - ); + // TODO: add message check once we properly validate that the role exists }); it("should change a user's role in a project", async () => { diff --git a/packages/cli/test/integration/public-api/users.test.ts b/packages/cli/test/integration/public-api/users.test.ts index e42f192608..8bfa0c5ae6 100644 --- a/packages/cli/test/integration/public-api/users.test.ts +++ b/packages/cli/test/integration/public-api/users.test.ts @@ -9,6 +9,7 @@ import { getUserById, } from '@test-integration/db/users'; import { setupTestServer } from '@test-integration/utils'; +import { createRole } from '@test-integration/db/roles'; describe('Users in Public API', () => { const testServer = setupTestServer({ endpointGroups: ['publicApi'] }); @@ -61,13 +62,32 @@ describe('Users in Public API', () => { expect(response.body).toHaveProperty('message', 'Forbidden'); }); + it('should fail if role does not exist', async () => { + /** + * Arrange + */ + testServer.license.enable('feat:advancedPermissions'); + const owner = await createOwnerWithApiKey(); + const payload = [{ email: 'test@test.com', role: 'non-existing-role' }]; + + /** + * Act + */ + const response = await testServer.publicApiAgentFor(owner).post('/users').send(payload); + + /** + * Assert + */ + expect(response.status).toBe(400); + expect(response.body).toHaveProperty('message', 'Role non-existing-role does not exist'); + }); + it('should create a user', async () => { /** * Arrange */ testServer.license.enable('feat:advancedPermissions'); const owner = await createOwnerWithApiKey(); - await createOwnerWithApiKey(); const payload = [{ email: 'test@test.com', role: 'global:admin' }]; /** @@ -97,6 +117,27 @@ describe('Users in Public API', () => { expect(returnedUser.email).toBe(payloadUser.email); expect(storedUser.role.slug).toBe(payloadUser.role); }); + + it('should create a user with an existing custom role', async () => { + /** + * Arrange + */ + testServer.license.enable('feat:advancedPermissions'); + const owner = await createOwnerWithApiKey(); + const customRole = 'custom:role'; + await createRole({ slug: customRole, displayName: 'Custom role', roleType: 'global' }); + const payload = [{ email: 'test@test.com', role: customRole }]; + + /** + * Act + */ + const response = await testServer.publicApiAgentFor(owner).post('/users').send(payload); + + /** + * Assert + */ + expect(response.status).toBe(201); + }); }); describe('DELETE /users/:id', () => { @@ -277,5 +318,32 @@ describe('Users in Public API', () => { const storedUser = await getUserById(member.id); expect(storedUser.role.slug).toBe(payload.newRoleName); }); + + it('should change a user role to an existing custom role', async () => { + /** + * Arrange + */ + testServer.license.enable('feat:advancedPermissions'); + const owner = await createOwnerWithApiKey(); + const member = await createMember(); + const customRole = 'custom:role'; + await createRole({ slug: customRole, displayName: 'Custom role', roleType: 'global' }); + const payload = { newRoleName: customRole }; + + /** + * Act + */ + const response = await testServer + .publicApiAgentFor(owner) + .patch(`/users/${member.id}/role`) + .send(payload); + + /** + * Assert + */ + expect(response.status).toBe(204); + const storedUser = await getUserById(member.id); + expect(storedUser.role.slug).toBe(payload.newRoleName); + }); }); }); diff --git a/packages/cli/test/integration/services/project.service.test.ts b/packages/cli/test/integration/services/project.service.test.ts index ce46330a33..79d109fba8 100644 --- a/packages/cli/test/integration/services/project.service.test.ts +++ b/packages/cli/test/integration/services/project.service.test.ts @@ -3,13 +3,16 @@ import { ProjectRelationRepository, ProjectRepository } from '@n8n/db'; import { Container } from '@n8n/di'; import { PROJECT_OWNER_ROLE_SLUG, type ProjectRole, type Scope } from '@n8n/permissions'; -import { createMember } from '../shared/db/users'; - +import { License } from '@/license'; import { ProjectService } from '@/services/project.service.ee'; +import { createRole } from '@test-integration/db/roles'; + +import { createMember } from '../shared/db/users'; let projectRepository: ProjectRepository; let projectService: ProjectService; let projectRelationRepository: ProjectRelationRepository; +let license: License; beforeAll(async () => { await testDb.init(); @@ -17,6 +20,7 @@ beforeAll(async () => { projectRepository = Container.get(ProjectRepository); projectService = Container.get(ProjectService); projectRelationRepository = Container.get(ProjectRelationRepository); + license = Container.get(License); }); afterAll(async () => { @@ -95,6 +99,36 @@ describe('ProjectService', () => { expect(relationships).toHaveLength(1); expect(relationships[0]).toHaveProperty('role.slug', 'project:admin'); }); + + it('adds a user to a project with a custom role', async () => { + // + // ARRANGE + // + const member = await createMember(); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + const role = await createRole({ slug: 'project:custom', displayName: 'Custom Role' }); + + // + // ACT + // + await projectService.addUser(project.id, { userId: member.id, role: role.slug }); + + // + // ASSERT + // + const relationships = await projectRelationRepository.find({ + where: { userId: member.id, projectId: project.id }, + relations: { role: true }, + }); + + expect(relationships).toHaveLength(1); + expect(relationships[0].role.slug).toBe('project:custom'); + }); }); describe('getProjectWithScope', () => { @@ -240,4 +274,165 @@ describe('ProjectService', () => { expect(relations).toBeNull(); }); }); + + describe('addUsersToProject', () => { + it('should add multiple users to a project', async () => { + // + // ARRANGE + // + const members = await Promise.all([createMember(), createMember()]); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true); + + // + // ACT + // + await projectService.addUsersToProject( + project.id, + members.map((member) => ({ userId: member.id, role: 'project:editor' })), + ); + + // + // ASSERT + // + const relations = await projectRelationRepository.find({ + where: { projectId: project.id }, + }); + + expect(relations).toHaveLength(members.length); + }); + + it('fails to add a user to a project with a non-existing role', async () => { + // + // ARRANGE + // + const member = await createMember(); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + + // + // ACT + // + await expect( + projectService.addUsersToProject(project.id, [ + { userId: member.id, role: 'custom:non-existing' }, + ]), + ).rejects.toThrowError('Role custom:non-existing does not exist'); + }); + }); + + describe('syncProjectRelations', () => { + it('should synchronize project relations for a user', async () => { + // + // ARRANGE + // + const user = await createMember(); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true); + + // + // ACT + // + await projectService.syncProjectRelations(project.id, [ + { userId: user.id, role: 'project:editor' }, + ]); + + // + // ASSERT + // + const relations = await projectRelationRepository.find({ + where: { userId: user.id, projectId: project.id }, + }); + expect(relations).toHaveLength(1); + }); + + it('should fail to synchronize users with non-existing roles', async () => { + // + // ARRANGE + // + const user = await createMember(); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true); + + // + // ACT + // + await expect( + projectService.syncProjectRelations(project.id, [ + { userId: user.id, role: 'project:non-existing' }, + ]), + ).rejects.toThrowError('Role project:non-existing does not exist'); + }); + }); + + describe('changeUserRoleInProject', () => { + it('should change user role in project', async () => { + // + // ARRANGE + // + const user = await createMember(); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + + // + // ACT + // + await projectService.addUser(project.id, { userId: user.id, role: 'project:viewer' }); + await projectService.changeUserRoleInProject(project.id, user.id, 'project:editor'); + + // + // ASSERT + // + const relations = await projectRelationRepository.find({ + where: { userId: user.id, projectId: project.id }, + relations: { role: true }, + }); + expect(relations).toHaveLength(1); + expect(relations[0].role.slug).toBe('project:editor'); + }); + + it('should fail to change user role in project with non-existing role', async () => { + // + // ARRANGE + // + const user = await createMember(); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + + // + // ACT + // + await projectService.addUser(project.id, { userId: user.id, role: 'project:viewer' }); + await expect( + projectService.changeUserRoleInProject(project.id, user.id, 'project:non-existing'), + ).rejects.toThrowError('Role project:non-existing does not exist'); + }); + }); }); diff --git a/packages/cli/test/integration/users.api.test.ts b/packages/cli/test/integration/users.api.test.ts index b041b36a25..e6db3f68d9 100644 --- a/packages/cli/test/integration/users.api.test.ts +++ b/packages/cli/test/integration/users.api.test.ts @@ -41,6 +41,7 @@ import { createAdmin, createMember, createOwner, createUser, getUserById } from import type { SuperAgentTest } from './shared/types'; import * as utils from './shared/utils/'; import { validateUser } from './shared/utils/users'; +import { createRole } from '@test-integration/db/roles'; mockInstance(Telemetry); mockInstance(ExecutionService); @@ -1573,4 +1574,30 @@ describe('PATCH /users/:id/role', () => { expect(response.statusCode).toBe(200); expect(response.body.data).toStrictEqual({ success: true }); }); + + test('should fail to change to non-existing role', async () => { + const customRole = 'custom:project-role'; + await createRole({ slug: customRole, displayName: 'Custom Role', roleType: 'project' }); + const response = await ownerAgent.patch(`/users/${member.id}/role`).send({ + newRoleName: customRole, + }); + + expect(response.statusCode).toBe(400); + expect(response.body.message).toBe('Role custom:project-role does not exist'); + }); + + test('should change to existing custom role', async () => { + const customRole = 'custom:role'; + await createRole({ slug: customRole, displayName: 'Custom Role', roleType: 'global' }); + const response = await ownerAgent.patch(`/users/${member.id}/role`).send({ + newRoleName: customRole, + }); + + expect(response.statusCode).toBe(200); + expect(response.body.data).toStrictEqual({ success: true }); + + const user = await getUserById(member.id); + + expect(user.role.slug).toBe(customRole); + }); });