diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index f2d315d9d9..277d3b9c0a 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -28,6 +28,8 @@ export { ChangePasswordRequestDto } from './password-reset/change-password-reque export { CreateProjectDto } from './project/create-project.dto'; export { UpdateProjectDto } from './project/update-project.dto'; export { DeleteProjectDto } from './project/delete-project.dto'; +export { AddUsersToProjectDto } from './project/add-users-to-project.dto'; +export { ChangeUserRoleInProject } from './project/change-user-role-in-project.dto'; export { SamlAcsDto } from './saml/saml-acs.dto'; export { SamlPreferences } from './saml/saml-preferences.dto'; 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 new file mode 100644 index 0000000000..5a303f5882 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/project/__tests__/add-users-to-project.dto.test.ts @@ -0,0 +1,143 @@ +import { AddUsersToProjectDto } from '../add-users-to-project.dto'; + +describe('AddUsersToProjectDto', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'with single user', + request: { + relations: [ + { + userId: 'user-123', + role: 'project:admin', + }, + ], + }, + }, + { + name: 'with multiple relations', + request: { + relations: [ + { + userId: 'user-123', + role: 'project:admin', + }, + { + userId: 'user-456', + role: 'project:editor', + }, + { + userId: 'user-789', + role: 'project:viewer', + }, + ], + }, + }, + { + name: 'with all possible roles unless the `project:personalOwner`', + request: { + relations: [ + { userId: 'user-1', role: 'project:admin' }, + { userId: 'user-2', role: 'project:editor' }, + { userId: 'user-3', role: 'project:viewer' }, + ], + }, + }, + ])('should validate $name', ({ request }) => { + const result = AddUsersToProjectDto.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'missing relations array', + request: {}, + expectedErrorPath: ['relations'], + }, + { + name: 'empty relations array', + request: { + relations: [], + }, + expectedErrorPath: ['relations'], + }, + { + name: 'invalid userId type', + request: { + relations: [ + { + userId: 123, + role: 'project:admin', + }, + ], + }, + expectedErrorPath: ['relations', 0, 'userId'], + }, + { + name: 'empty userId', + request: { + relations: [ + { + userId: '', + role: 'project:admin', + }, + ], + }, + expectedErrorPath: ['relations', 0, 'userId'], + }, + { + name: 'invalid role', + request: { + relations: [ + { + userId: 'user-123', + role: 'invalid-role', + }, + ], + }, + expectedErrorPath: ['relations', 0, 'role'], + }, + { + name: 'missing role', + request: { + relations: [ + { + userId: 'user-123', + }, + ], + }, + expectedErrorPath: ['relations', 0, 'role'], + }, + { + name: 'invalid relations array type', + request: { + relations: 'not-an-array', + }, + expectedErrorPath: ['relations'], + }, + { + name: 'invalid user object in array', + request: { + relations: ['not-an-object'], + }, + expectedErrorPath: ['relations', 0], + }, + { + name: 'invalid with `project:personalOwner` role', + request: { + relations: [{ userId: 'user-1', role: 'project:personalOwner' }], + }, + }, + ])('should fail validation for $name', ({ request, expectedErrorPath }) => { + const result = AddUsersToProjectDto.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); 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 new file mode 100644 index 0000000000..e215aee546 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/project/__tests__/change-user-role-in-project.dto.test.ts @@ -0,0 +1,54 @@ +import { ChangeUserRoleInProject } from '../change-user-role-in-project.dto'; + +describe('ChangeUserRoleInProject', () => { + describe('Allow valid roles', () => { + test.each(['project:admin', 'project:editor', 'project:viewer'])('should allow %s', (role) => { + const result = ChangeUserRoleInProject.safeParse({ role }); + expect(result.success).toBe(true); + }); + }); + + describe('Reject invalid roles', () => { + test.each([ + { + name: 'missing role', + request: {}, + expectedErrorPath: ['role'], + }, + { + name: 'empty role', + request: { + role: '', + }, + expectedErrorPath: ['role'], + }, + { + name: 'invalid role type', + request: { + role: 123, + }, + expectedErrorPath: ['role'], + }, + { + name: 'invalid role value', + request: { + role: 'invalid-role', + }, + expectedErrorPath: ['role'], + }, + { + name: 'personal owner role', + request: { role: 'project:personalOwner' }, + expectedErrorPath: ['role'], + }, + ])('should reject $name', ({ request, expectedErrorPath }) => { + const result = ChangeUserRoleInProject.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/project/add-users-to-project.dto.ts b/packages/@n8n/api-types/src/dto/project/add-users-to-project.dto.ts new file mode 100644 index 0000000000..afa1c57f15 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/project/add-users-to-project.dto.ts @@ -0,0 +1,8 @@ +import { z } from 'zod'; +import { Z } from 'zod-class'; + +import { projectRelationSchema } from '../../schemas/project.schema'; + +export class AddUsersToProjectDto extends Z.class({ + relations: z.array(projectRelationSchema).min(1), +}) {} 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 new file mode 100644 index 0000000000..17550855a4 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/project/change-user-role-in-project.dto.ts @@ -0,0 +1,6 @@ +import { projectRoleSchema } from '@n8n/permissions'; +import { Z } from 'zod-class'; + +export class ChangeUserRoleInProject extends Z.class({ + role: projectRoleSchema.exclude(['project:personalOwner']), +}) {} diff --git a/packages/@n8n/api-types/src/schemas/project.schema.ts b/packages/@n8n/api-types/src/schemas/project.schema.ts index 6eb7cb5c8f..67de6fbd02 100644 --- a/packages/@n8n/api-types/src/schemas/project.schema.ts +++ b/packages/@n8n/api-types/src/schemas/project.schema.ts @@ -13,7 +13,7 @@ export const projectIconSchema = z.object({ export type ProjectIcon = z.infer; export const projectRelationSchema = z.object({ - userId: z.string(), - role: projectRoleSchema, + userId: z.string().min(1), + role: projectRoleSchema.exclude(['project:personalOwner']), }); export type ProjectRelation = z.infer; diff --git a/packages/@n8n/permissions/src/schemas.ee.ts b/packages/@n8n/permissions/src/schemas.ee.ts index 2a17457c27..8fd21af361 100644 --- a/packages/@n8n/permissions/src/schemas.ee.ts +++ b/packages/@n8n/permissions/src/schemas.ee.ts @@ -8,13 +8,14 @@ export const assignableGlobalRoleSchema = globalRoleSchema.exclude([ 'global:owner', // Owner cannot be changed ]); -export const projectRoleSchema = z.enum([ +export const personalRoleSchema = z.enum([ 'project:personalOwner', // personalOwner is only used for personal projects - 'project:admin', - 'project:editor', - 'project:viewer', ]); +export const teamRoleSchema = z.enum(['project:admin', 'project:editor', 'project:viewer']); + +export const projectRoleSchema = z.enum([...personalRoleSchema.options, ...teamRoleSchema.options]); + export const credentialSharingRoleSchema = z.enum(['credential:owner', 'credential:user']); export const workflowSharingRoleSchema = z.enum(['workflow:owner', 'workflow:editor']); diff --git a/packages/@n8n/permissions/src/types.ee.ts b/packages/@n8n/permissions/src/types.ee.ts index e08699f074..c9dcb8d8bd 100644 --- a/packages/@n8n/permissions/src/types.ee.ts +++ b/packages/@n8n/permissions/src/types.ee.ts @@ -7,6 +7,7 @@ import type { globalRoleSchema, projectRoleSchema, roleNamespaceSchema, + teamRoleSchema, workflowSharingRoleSchema, } from './schemas.ee'; @@ -49,6 +50,7 @@ export type GlobalRole = z.infer; 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; /** Union of all possible role types in the system */ diff --git a/packages/cli/src/public-api/v1/handlers/projects/projects.handler.ts b/packages/cli/src/public-api/v1/handlers/projects/projects.handler.ts index 1810225b74..24001cedff 100644 --- a/packages/cli/src/public-api/v1/handlers/projects/projects.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/projects/projects.handler.ts @@ -1,11 +1,19 @@ -import { CreateProjectDto, DeleteProjectDto, UpdateProjectDto } from '@n8n/api-types'; +import { + AddUsersToProjectDto, + ChangeUserRoleInProject, + CreateProjectDto, + DeleteProjectDto, + UpdateProjectDto, +} from '@n8n/api-types'; import { ProjectRepository } from '@n8n/db'; import { Container } from '@n8n/di'; import type { Response } from 'express'; import { ProjectController } from '@/controllers/project.controller'; +import { ResponseError } from '@/errors/response-errors/abstract/response.error'; import type { PaginatedRequest } from '@/public-api/types'; import type { AuthenticatedRequest } from '@/requests'; +import { ProjectService } from '@/services/project.service.ee'; import { apiKeyHasScopeWithGlobalScopeFallback, @@ -15,7 +23,6 @@ import { import { encodeNextCursor } from '../../shared/services/pagination.service'; type GetAll = PaginatedRequest; - export = { createProject: [ isLicensed('feat:projectRole:admin'), @@ -91,4 +98,67 @@ export = { }); }, ], + addUsersToProject: [ + isLicensed('feat:projectRole:admin'), + apiKeyHasScopeWithGlobalScopeFallback({ scope: 'project:update' }), + async (req: AuthenticatedRequest<{ projectId: string }>, res: Response) => { + const payload = AddUsersToProjectDto.safeParse(req.body); + if (payload.error) { + return res.status(400).json(payload.error.errors[0]); + } + + try { + await Container.get(ProjectService).addUsersToProject( + req.params.projectId, + payload.data.relations, + ); + } catch (error) { + if (error instanceof ResponseError) { + return res.status(error.httpStatusCode).send({ message: error.message }); + } + throw error; + } + + return res.status(201).send(); + }, + ], + changeUserRoleInProject: [ + isLicensed('feat:projectRole:admin'), + apiKeyHasScopeWithGlobalScopeFallback({ scope: 'project:update' }), + async (req: AuthenticatedRequest<{ projectId: string; userId: string }>, res: Response) => { + const payload = ChangeUserRoleInProject.safeParse(req.body); + if (payload.error) { + return res.status(400).json(payload.error.errors[0]); + } + + const { projectId, userId } = req.params; + const { role } = payload.data; + try { + await Container.get(ProjectService).changeUserRoleInProject(projectId, userId, role); + } catch (error) { + if (error instanceof ResponseError) { + return res.status(error.httpStatusCode).send({ message: error.message }); + } + throw error; + } + + return res.status(204).send(); + }, + ], + deleteUserFromProject: [ + isLicensed('feat:projectRole:admin'), + apiKeyHasScopeWithGlobalScopeFallback({ scope: 'project:update' }), + async (req: AuthenticatedRequest<{ projectId: string; userId: string }>, res: Response) => { + const { projectId, userId } = req.params; + try { + await Container.get(ProjectService).deleteUserFromProject(projectId, userId); + } catch (error) { + if (error instanceof ResponseError) { + return res.status(error.httpStatusCode).send({ message: error.message }); + } + throw error; + } + return res.status(204).send(); + }, + ], }; diff --git a/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.users.userId.yml b/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.users.userId.yml new file mode 100644 index 0000000000..0e417ee5d6 --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.users.userId.yml @@ -0,0 +1,71 @@ +delete: + x-eov-operation-id: deleteUserFromProject + x-eov-operation-handler: v1/handlers/projects/projects.handler + tags: + - Projects + summary: Delete a user from a project + description: Delete a user from a project from your instance. + parameters: + - name: projectId + in: path + description: The ID of the project. + required: true + schema: + type: string + - name: userId + in: path + description: The ID of the user. + required: true + schema: + type: string + responses: + '204': + description: Operation successful. + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' + '403': + $ref: '../../../../shared/spec/responses/forbidden.yml' + '404': + $ref: '../../../../shared/spec/responses/notFound.yml' +patch: + x-eov-operation-id: changeUserRoleInProject + x-eov-operation-handler: v1/handlers/projects/projects.handler + tags: + - Projects + summary: Change a user's role in a project + description: Change a user's role in a project. + parameters: + - name: projectId + in: path + description: The ID of the project. + required: true + schema: + type: string + - name: userId + in: path + description: The ID of the user. + required: true + schema: + type: string + requestBody: + description: Updated project object. + content: + application/json: + schema: + type: object + properties: + role: + type: string + description: The role assigned to the user in the project. + example: 'project:viewer' + required: + - role + responses: + '204': + description: Operation successful. + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' + '403': + $ref: '../../../../shared/spec/responses/forbidden.yml' + '404': + $ref: '../../../../shared/spec/responses/notFound.yml' diff --git a/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.users.yml b/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.users.yml new file mode 100644 index 0000000000..7ba861137d --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.users.yml @@ -0,0 +1,49 @@ +post: + x-eov-operation-id: addUsersToProject + x-eov-operation-handler: v1/handlers/projects/projects.handler + tags: + - Projects + summary: Add one or more users to a project + description: Add one or more users to a project from your instance. + parameters: + - name: projectId + in: path + description: The ID of the project. + required: true + schema: + type: string + requestBody: + description: Payload containing an array of one or more users to add to the project. + content: + application/json: + schema: + type: object + properties: + relations: + type: array + description: A list of userIds and roles to add to the project. + items: + type: object + properties: + userId: + type: string + description: The unique identifier of the user. + example: '91765f0d-3b29-45df-adb9-35b23937eb92' + role: + type: string + description: The role assigned to the user in the project. + example: 'project:viewer' + required: + - userId + - role + required: + - relations + responses: + '201': + description: Operation successful. + '401': + $ref: '../../../../shared/spec/responses/unauthorized.yml' + '403': + $ref: '../../../../shared/spec/responses/forbidden.yml' + '404': + $ref: '../../../../shared/spec/responses/notFound.yml' diff --git a/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.yml b/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.yml index a5aab19b3d..145c5e85ec 100644 --- a/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.yml +++ b/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.yml @@ -6,7 +6,12 @@ delete: summary: Delete a project description: Delete a project from your instance. parameters: - - $ref: '../schemas/parameters/projectId.yml' + - in: path + name: projectId + description: The ID of the project. + required: true + schema: + type: string responses: '204': description: Operation successful. @@ -20,9 +25,16 @@ put: x-eov-operation-id: updateProject x-eov-operation-handler: v1/handlers/projects/projects.handler tags: - - Project + - Projects summary: Update a project description: Update a project. + parameters: + - in: path + name: projectId + description: The ID of the project. + required: true + schema: + type: string requestBody: description: Updated project object. content: diff --git a/packages/cli/src/public-api/v1/openapi.yml b/packages/cli/src/public-api/v1/openapi.yml index 2608f34980..7afae9651c 100644 --- a/packages/cli/src/public-api/v1/openapi.yml +++ b/packages/cli/src/public-api/v1/openapi.yml @@ -82,6 +82,10 @@ paths: $ref: './handlers/projects/spec/paths/projects.yml' /projects/{projectId}: $ref: './handlers/projects/spec/paths/projects.projectId.yml' + /projects/{projectId}/users: + $ref: './handlers/projects/spec/paths/projects.projectId.users.yml' + /projects/{projectId}/users/{userId}: + $ref: './handlers/projects/spec/paths/projects.projectId.users.userId.yml' components: schemas: $ref: './shared/spec/schemas/_index.yml' diff --git a/packages/cli/src/services/__tests__/project.service.ee.test.ts b/packages/cli/src/services/__tests__/project.service.ee.test.ts new file mode 100644 index 0000000000..e3d08f86f0 --- /dev/null +++ b/packages/cli/src/services/__tests__/project.service.ee.test.ts @@ -0,0 +1,237 @@ +import type { ProjectRelation } from '@n8n/api-types'; +import type { DatabaseConfig } from '@n8n/config'; +import type { + Project, + ProjectRepository, + SharedCredentialsRepository, + ProjectRelationRepository, + SharedCredentials, +} from '@n8n/db'; +import type { EntityManager } from '@n8n/typeorm'; +import { mock } from 'jest-mock-extended'; + +import type { CacheService } from '../cache/cache.service'; +import { ProjectService } from '../project.service.ee'; +import type { RoleService } from '../role.service'; + +describe('ProjectService', () => { + const manager = mock(); + const projectRepository = mock(); + const projectRelationRepository = mock({ manager }); + const roleService = mock(); + const sharedCredentialsRepository = mock(); + const cacheService = mock(); + const projectService = new ProjectService( + mock(), + projectRepository, + projectRelationRepository, + roleService, + sharedCredentialsRepository, + cacheService, + mock(), + mock({ type: 'postgresdb' }), + ); + + describe('addUsersToProject', () => { + it('throws if called with a personal project', async () => { + // ARRANGE + const projectId = '12345'; + projectRepository.findOne.mockResolvedValueOnce( + mock({ type: 'personal', projectRelations: [] }), + ); + roleService.isRoleLicensed.mockReturnValueOnce(true); + + // ACT & ASSERT + await expect( + projectService.addUsersToProject(projectId, [{ userId: '1234', role: 'project:admin' }]), + ).rejects.toThrowError("Can't add users to personal projects."); + }); + + it('throws if trying to add a personalOwner to a team project', async () => { + // ARRANGE + const projectId = '12345'; + projectRepository.findOne.mockResolvedValueOnce( + mock({ type: 'team', projectRelations: [] }), + ); + roleService.isRoleLicensed.mockReturnValueOnce(true); + + // ACT & ASSERT + await expect( + projectService.addUsersToProject(projectId, [ + { userId: '1234', role: 'project:personalOwner' }, + ]), + ).rejects.toThrowError("Can't add a personalOwner to a team project."); + }); + }); + + describe('syncProjectRelations', () => { + const projectId = '12345'; + const mockRelations: ProjectRelation[] = [ + { userId: 'user1', role: 'project:admin' }, + { userId: 'user2', role: 'project:viewer' }, + ]; + + beforeEach(() => { + jest.clearAllMocks(); + manager.transaction.mockImplementation(async (arg1: unknown, arg2?: unknown) => { + const runInTransaction = (arg2 ?? arg1) as ( + entityManager: EntityManager, + ) => Promise; + return await runInTransaction(manager); + }); + }); + + it('should successfully sync project relations', async () => { + projectRepository.findOne.mockResolvedValueOnce( + mock({ + id: projectId, + type: 'team', + projectRelations: [], + }), + ); + roleService.isRoleLicensed.mockReturnValue(true); + + sharedCredentialsRepository.find.mockResolvedValueOnce([ + mock({ credentialsId: 'cred1' }), + mock({ credentialsId: 'cred2' }), + ]); + + await projectService.syncProjectRelations(projectId, mockRelations); + + expect(projectRepository.findOne).toHaveBeenCalledWith({ + where: { id: projectId, type: 'team' }, + relations: { projectRelations: true }, + }); + + expect(manager.delete).toHaveBeenCalled(); + expect(manager.insert).toHaveBeenCalled(); + expect(cacheService.deleteMany).toHaveBeenCalledWith([ + 'credential-can-use-secrets:cred1', + 'credential-can-use-secrets:cred2', + ]); + }); + + it('should throw error if project not found', async () => { + projectRepository.findOne.mockResolvedValueOnce(null); + + await expect(projectService.syncProjectRelations(projectId, mockRelations)).rejects.toThrow( + `Could not find project with ID: ${projectId}`, + ); + }); + + it('should throw error if unlicensed role is used', async () => { + projectRepository.findOne.mockResolvedValueOnce( + mock({ + id: projectId, + type: 'team', + projectRelations: [], + }), + ); + roleService.isRoleLicensed.mockReturnValue(false); + + await expect(projectService.syncProjectRelations(projectId, mockRelations)).rejects.toThrow( + 'Your instance is not licensed to use role "project:admin"', + ); + }); + + it('should not throw error for existing role even if unlicensed', async () => { + projectRepository.findOne.mockResolvedValueOnce( + mock({ + id: projectId, + type: 'team', + projectRelations: [{ userId: 'user1', role: 'project:admin' }], + }), + ); + roleService.isRoleLicensed.mockReturnValue(false); + + sharedCredentialsRepository.find.mockResolvedValueOnce([]); + + await expect( + projectService.syncProjectRelations(projectId, [ + { userId: 'user1', role: 'project:admin' }, + ]), + ).resolves.not.toThrow(); + }); + }); + + describe('changeUserRoleInProject', () => { + const projectId = '12345'; + const mockRelations: ProjectRelation[] = [ + { userId: 'user1', role: 'project:admin' }, + { userId: 'user2', role: 'project:viewer' }, + ]; + + beforeEach(() => { + jest.clearAllMocks(); + manager.transaction.mockImplementation(async (arg1: unknown, arg2?: unknown) => { + const runInTransaction = (arg2 ?? arg1) as ( + entityManager: EntityManager, + ) => Promise; + return await runInTransaction(manager); + }); + }); + + it('should successfully change the user role in the project', async () => { + projectRepository.findOne.mockResolvedValueOnce( + mock({ + id: projectId, + type: 'team', + projectRelations: mockRelations, + }), + ); + roleService.isRoleLicensed.mockReturnValue(true); + + await projectService.changeUserRoleInProject(projectId, 'user2', 'project:admin'); + + expect(projectRepository.findOne).toHaveBeenCalledWith({ + where: { id: projectId, type: 'team' }, + relations: { projectRelations: true }, + }); + + expect(projectRelationRepository.update).toHaveBeenCalledWith( + { projectId, userId: 'user2' }, + { role: 'project:admin' }, + ); + }); + + it('should throw if the user is not part of the project', async () => { + projectRepository.findOne.mockResolvedValueOnce( + mock({ + id: projectId, + type: 'team', + projectRelations: mockRelations, + }), + ); + roleService.isRoleLicensed.mockReturnValue(true); + + await expect( + projectService.changeUserRoleInProject(projectId, 'user3', 'project:admin'), + ).rejects.toThrow(`Could not find project with ID: ${projectId}`); + + expect(projectRepository.findOne).toHaveBeenCalledWith({ + where: { id: projectId, type: 'team' }, + relations: { projectRelations: true }, + }); + }); + + it('should throw if the role to be set is `project:personalOwner`', async () => { + await expect( + projectService.changeUserRoleInProject(projectId, 'user2', 'project:personalOwner'), + ).rejects.toThrow('Personal owner cannot be added to a team project.'); + }); + + it('should throw if the project is not a team project', async () => { + projectRepository.findOne.mockResolvedValueOnce(null); + roleService.isRoleLicensed.mockReturnValue(true); + + await expect( + projectService.changeUserRoleInProject(projectId, 'user2', 'project:admin'), + ).rejects.toThrow(`Could not find project with ID: ${projectId}`); + + expect(projectRepository.findOne).toHaveBeenCalledWith({ + where: { id: projectId, type: 'team' }, + relations: { projectRelations: true }, + }); + }); + }); +}); diff --git a/packages/cli/src/services/project.service.ee.ts b/packages/cli/src/services/project.service.ee.ts index b051241fc4..b53a51a828 100644 --- a/packages/cli/src/services/project.service.ee.ts +++ b/packages/cli/src/services/project.service.ee.ts @@ -16,7 +16,7 @@ import { hasGlobalScope, rolesWithScope, type Scope, type ProjectRole } from '@n // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import type { FindOptionsWhere, EntityManager } from '@n8n/typeorm'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import -import { In, Not } from '@n8n/typeorm'; +import { In } from '@n8n/typeorm'; import { UserError } from 'n8n-workflow'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; @@ -24,6 +24,9 @@ import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { CacheService } from './cache/cache.service'; +import { RoleService } from './role.service'; + +type Relation = Pick; export class TeamProjectOverQuotaError extends UserError { constructor(limit: number) { @@ -39,12 +42,28 @@ export class UnlicensedProjectRoleError extends UserError { } } +class ProjectNotFoundError extends NotFoundError { + constructor(projectId: string) { + super(`Could not find project with ID: ${projectId}`); + } + + static isDefinedAndNotNull( + value: T | undefined | null, + projectId: string, + ): asserts value is T { + if (value === undefined || value === null) { + throw new ProjectNotFoundError(projectId); + } + } +} + @Service() export class ProjectService { constructor( private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly projectRepository: ProjectRepository, private readonly projectRelationRepository: ProjectRelationRepository, + private readonly roleService: RoleService, private readonly sharedCredentialsRepository: SharedCredentialsRepository, private readonly cacheService: CacheService, private readonly licenseState: LicenseState, @@ -86,9 +105,7 @@ export class ProjectService { } const project = await this.getProjectWithScope(user, projectId, ['project:delete']); - if (!project) { - throw new NotFoundError(`Could not find project with ID: ${projectId}`); - } + ProjectNotFoundError.isDefinedAndNotNull(project, projectId); let targetProject: Project | null = null; if (migrateToProject) { @@ -202,7 +219,7 @@ export class ProjectService { ); // Link admin - await this.addUser(project.id, adminUser.id, 'project:admin', trx); + await this.addUser(project.id, { userId: adminUser.id, role: 'project:admin' }, trx); return project; } @@ -225,16 +242,14 @@ export class ProjectService { } } - async updateProject( - projectId: string, - data: Pick, - ): Promise { - const result = await this.projectRepository.update({ id: projectId, type: 'team' }, data); - + async updateProject(projectId: string, { name, icon }: UpdateProjectDto): Promise { + const result = await this.projectRepository.update( + { id: projectId, type: 'team' }, + { name, icon }, + ); if (!result.affected) { - throw new ForbiddenError('Project not found'); + throw new ProjectNotFoundError(projectId); } - return await this.projectRepository.findOneByOrFail({ id: projectId }); } async getPersonalProject(user: User): Promise { @@ -250,22 +265,10 @@ export class ProjectService { async syncProjectRelations( projectId: string, - relations: Array<{ userId: string; role: ProjectRole }>, + relations: Required['relations'], ) { - const project = await this.projectRepository.findOneOrFail({ - where: { id: projectId, type: Not('personal') }, - relations: { projectRelations: true }, - }); - - // Check to see if the instance is licensed to use all roles provided - for (const r of relations) { - const existing = project.projectRelations.find((pr) => pr.userId === r.userId); - // We don't throw an error if the user already exists with that role so - // existing projects continue working as is. - if (existing?.role !== r.role && !this.isProjectRoleLicensed(r.role)) { - throw new UnlicensedProjectRoleError(r.role); - } - } + const project = await this.getTeamProjectWithRelations(projectId); + this.checkRolesLicensed(project, relations); await this.projectRelationRepository.manager.transaction(async (em) => { await this.pruneRelations(em, project); @@ -274,17 +277,81 @@ export class ProjectService { await this.clearCredentialCanUseExternalSecretsCache(projectId); } - private isProjectRoleLicensed(role: ProjectRole) { - switch (role) { - case 'project:admin': - return this.licenseState.isProjectRoleAdminLicensed(); - case 'project:editor': - return this.licenseState.isProjectRoleEditorLicensed(); - case 'project:viewer': - return this.licenseState.isProjectRoleViewerLicensed(); - default: - return true; + /** + * Adds users to a team project with specified roles. + * + * Throws if you the project is a personal project. + * Throws if the relations contain `project:personalOwner`. + */ + async addUsersToProject(projectId: string, relations: Relation[]) { + const project = await this.getTeamProjectWithRelations(projectId); + this.checkRolesLicensed(project, relations); + + if (project.type === 'personal') { + throw new ForbiddenError("Can't add users to personal projects."); } + + if (relations.some((r) => r.role === 'project:personalOwner')) { + throw new ForbiddenError("Can't add a personalOwner to a team project."); + } + + await this.projectRelationRepository.save( + relations.map((relation) => ({ projectId, ...relation })), + ); + } + + private async getTeamProjectWithRelations(projectId: string) { + const project = await this.projectRepository.findOne({ + where: { id: projectId, type: 'team' }, + relations: { projectRelations: true }, + }); + ProjectNotFoundError.isDefinedAndNotNull(project, projectId); + return project; + } + + /** Check to see if the instance is licensed to use all roles provided */ + private checkRolesLicensed(project: Project, relations: Relation[]) { + for (const { role, userId } of relations) { + const existing = project.projectRelations.find((pr) => pr.userId === userId); + // We don't throw an error if the user already exists with that role so + // existing projects continue working as is. + if (existing?.role !== role && !this.roleService.isRoleLicensed(role)) { + throw new UnlicensedProjectRoleError(role); + } + } + } + + private isUserProjectOwner(project: Project, userId: string) { + return project.projectRelations.some( + (pr) => pr.userId === userId && pr.role === 'project:personalOwner', + ); + } + + async deleteUserFromProject(projectId: string, userId: string) { + const project = await this.getTeamProjectWithRelations(projectId); + + // Prevent project owner from being removed + if (this.isUserProjectOwner(project, userId)) { + throw new ForbiddenError('Project owner cannot be removed from the project'); + } + + await this.projectRelationRepository.delete({ projectId: project.id, userId }); + } + + async changeUserRoleInProject(projectId: string, userId: string, role: ProjectRole) { + if (role === 'project:personalOwner') { + throw new ForbiddenError('Personal owner cannot be added to a team project.'); + } + + const project = await this.getTeamProjectWithRelations(projectId); + ProjectNotFoundError.isDefinedAndNotNull(project, projectId); + + const projectUserExists = project.projectRelations.some((r) => r.userId === userId); + if (!projectUserExists) { + throw new ProjectNotFoundError(projectId); + } + + await this.projectRelationRepository.update({ projectId, userId }, { role }); } async clearCredentialCanUseExternalSecretsCache(projectId: string) { @@ -351,7 +418,13 @@ export class ProjectService { }); } - async addUser(projectId: string, userId: string, role: ProjectRole, trx?: EntityManager) { + /** + * Add a user to a team project with specified roles. + * + * Throws if you the project is a personal project. + * Throws if the relations contain `project:personalOwner`. + */ + async addUser(projectId: string, { userId, role }: Relation, trx?: EntityManager) { trx = trx ?? this.projectRelationRepository.manager; return await trx.save(ProjectRelation, { projectId, diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index b706c80c0d..da1dd406d9 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -112,7 +112,7 @@ export class RoleService { return [...scopesSet].sort(); } - private isRoleLicensed(role: AllRoleTypes) { + isRoleLicensed(role: AllRoleTypes) { // TODO: move this info into FrontendSettings switch (role) { case 'project:admin': 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 5f80b2ac87..0633e5f828 100644 --- a/packages/cli/test/integration/credentials/credentials.api.ee.test.ts +++ b/packages/cli/test/integration/credentials/credentials.api.ee.test.ts @@ -231,7 +231,7 @@ describe('GET /credentials', () => { // ARRANGE // const project1 = await projectService.createTeamProject(member, { name: 'Team Project' }); - await projectService.addUser(project1.id, anotherMember.id, 'project:editor'); + await projectService.addUser(project1.id, { userId: anotherMember.id, role: 'project:editor' }); // anotherMember should see this one const credential1 = await saveCredential(randomCredentialPayload(), { project: project1 }); diff --git a/packages/cli/test/integration/project.api.test.ts b/packages/cli/test/integration/project.api.test.ts index 0aac69e2a0..257600c852 100644 --- a/packages/cli/test/integration/project.api.test.ts +++ b/packages/cli/test/integration/project.api.test.ts @@ -510,7 +510,7 @@ describe('PATCH /projects/:projectId', () => { const resp = await ownerAgent .patch(`/projects/${personalProject.id}`) .send({ name: 'New Name' }); - expect(resp.status).toBe(403); + expect(resp.status).toBe(404); const updatedProject = await findProject(personalProject.id); expect(updatedProject.name).not.toEqual('New Name'); diff --git a/packages/cli/test/integration/project.service.integration.test.ts b/packages/cli/test/integration/project.service.integration.test.ts index 62c29a9a89..b676e1791d 100644 --- a/packages/cli/test/integration/project.service.integration.test.ts +++ b/packages/cli/test/integration/project.service.integration.test.ts @@ -1,24 +1,28 @@ import { SharedWorkflowRepository } from '@n8n/db'; import { Container } from '@n8n/di'; +import { License } from '@/license'; import { ProjectService } from '@/services/project.service.ee'; +import { LicenseMocker } from '@test-integration/license'; -import { linkUserToProject, createTeamProject } from './shared/db/projects'; +import { linkUserToProject, createTeamProject, getAllProjectRelations } from './shared/db/projects'; import { createUser } from './shared/db/users'; import { createWorkflow } from './shared/db/workflows'; import * as testDb from './shared/test-db'; describe('ProjectService', () => { let projectService: ProjectService; - let sharedWorkflowRepository: SharedWorkflowRepository; beforeAll(async () => { await testDb.init(); projectService = Container.get(ProjectService); - sharedWorkflowRepository = Container.get(SharedWorkflowRepository); + + const license: LicenseMocker = new LicenseMocker(); + license.mock(Container.get(License)); + license.enable('feat:projectRole:editor'); }); afterEach(async () => { @@ -35,6 +39,71 @@ describe('ProjectService', () => { await testDb.terminate(); }); + describe('addUsersToProject', () => { + it("don't throw a unique constraint violation error when adding a user that is already part of the project", async () => { + // ARRANGE + const user = await createUser(); + const project = await createTeamProject('project', user); + + // ACT + // add user again + await projectService.addUsersToProject(project.id, [ + { userId: user.id, role: 'project:admin' }, + ]); + + // ASSERT + const relations = await getAllProjectRelations({ projectId: project.id }); + expect(relations).toHaveLength(1); + expect(relations[0]).toMatchObject({ + projectId: project.id, + userId: user.id, + role: 'project:admin', + }); + }); + + it('allows changing a users role', async () => { + // ARRANGE + const user = await createUser(); + const project = await createTeamProject('project', user); + + // ACT + // add user again + await projectService.addUsersToProject(project.id, [ + { userId: user.id, role: 'project:editor' }, + ]); + + // ASSERT + const relations = await getAllProjectRelations({ projectId: project.id }); + expect(relations).toHaveLength(1); + expect(relations[0]).toMatchObject({ + projectId: project.id, + userId: user.id, + role: 'project:editor', + }); + }); + }); + + describe('addUser', () => { + it("don't throw a unique constraint violation error when adding a user that is already part of the project", async () => { + // ARRANGE + const user = await createUser(); + const project = await createTeamProject('project', user); + + // ACT + // add user again + await projectService.addUser(project.id, { userId: user.id, role: 'project:admin' }); + + // ASSERT + const relations = await getAllProjectRelations({ projectId: project.id }); + expect(relations).toHaveLength(1); + expect(relations[0]).toMatchObject({ + projectId: project.id, + userId: user.id, + role: 'project:admin', + }); + }); + }); + describe('findRolesInProjects', () => { describe('when user has roles in projects where workflow is accessible', () => { it('should return roles and project IDs', async () => { diff --git a/packages/cli/test/integration/public-api/projects.test.ts b/packages/cli/test/integration/public-api/projects.test.ts index 4283382558..f1a3eb475f 100644 --- a/packages/cli/test/integration/public-api/projects.test.ts +++ b/packages/cli/test/integration/public-api/projects.test.ts @@ -1,8 +1,18 @@ import { FeatureNotLicensedError } from '@/errors/feature-not-licensed.error'; import { Telemetry } from '@/telemetry'; import { mockInstance } from '@test/mocking'; -import { createTeamProject, getProjectByNameOrFail } from '@test-integration/db/projects'; -import { createMemberWithApiKey, createOwnerWithApiKey } from '@test-integration/db/users'; +import { + createTeamProject, + getProjectByNameOrFail, + linkUserToProject, + getAllProjectRelations, + getProjectRoleForUser, +} from '@test-integration/db/projects'; +import { + createMemberWithApiKey, + createOwnerWithApiKey, + createMember, +} from '@test-integration/db/users'; import { setupTestServer } from '@test-integration/utils'; import * as testDb from '../shared/test-db'; @@ -394,4 +404,440 @@ describe('Projects in Public API', () => { expect(response.body).toHaveProperty('message', 'Forbidden'); }); }); + + describe('POST /projects/:id/users', () => { + it('if not authenticated, should reject with 401', async () => { + const project = await createTeamProject(); + + const response = await testServer + .publicApiAgentWithoutApiKey() + .post(`/projects/${project.id}/users`); + + expect(response.status).toBe(401); + expect(response.body).toHaveProperty('message', "'X-N8N-API-KEY' header required"); + }); + + it('if not licensed, should reject with a 403', async () => { + const owner = await createOwnerWithApiKey(); + const project = await createTeamProject(); + const member = await createMember(); + + const payload = { + relations: [ + { + userId: member.id, + role: 'project:viewer', + }, + ], + }; + + const response = await testServer + .publicApiAgentFor(owner) + .post(`/projects/${project.id}/users`) + .send(payload); + + expect(response.status).toBe(403); + expect(response.body).toHaveProperty( + 'message', + new FeatureNotLicensedError('feat:projectRole:admin').message, + ); + }); + + it('if missing scope, should reject with 403', async () => { + testServer.license.setQuota('quota:maxTeamProjects', -1); + testServer.license.enable('feat:projectRole:admin'); + const member = await createMemberWithApiKey(); + const project = await createTeamProject(); + + const payload = { + relations: [ + { + userId: member.id, + role: 'project:viewer', + }, + ], + }; + + const response = await testServer + .publicApiAgentFor(member) + .post(`/projects/${project.id}/users`) + .send(payload); + + expect(response.status).toBe(403); + expect(response.body).toHaveProperty('message', 'Forbidden'); + }); + + describe('when user has correct license', () => { + beforeEach(() => { + testServer.license.setQuota('quota:maxTeamProjects', -1); + testServer.license.enable('feat:projectRole:admin'); + }); + + it("should reject with 400 if the payload can't be validated", async () => { + // ARRANGE + const owner = await createOwnerWithApiKey(); + const member = await createMember(); + + const payload = { + relations: [ + { + userId: member.id, + // role does not exist + role: 'project:boss', + }, + ], + }; + + // ACT + const response = await testServer + .publicApiAgentFor(owner) + .post('/projects/123456/users') + .send(payload) + .expect(400); + + // ASSERT + expect(response.body).toHaveProperty( + 'message', + "Invalid enum value. Expected 'project:admin' | 'project:editor' | 'project:viewer', received 'project:boss'", + ); + }); + + it('should reject with 404 if no project found', async () => { + const owner = await createOwnerWithApiKey(); + const member = await createMember(); + + const payload = { + relations: [ + { + userId: member.id, + role: 'project:viewer', + }, + ], + }; + + const response = await testServer + .publicApiAgentFor(owner) + .post('/projects/123456/users') + .send(payload); + + expect(response.status).toBe(404); + expect(response.body).toHaveProperty('message', 'Could not find project with ID: 123456'); + }); + + it('should add expected users to project', async () => { + testServer.license.enable('feat:projectRole:viewer'); + testServer.license.enable('feat:projectRole:editor'); + const owner = await createOwnerWithApiKey(); + const project = await createTeamProject('shared-project', owner); + const member = await createMember(); + const member2 = await createMember(); + const projectBefore = await getAllProjectRelations({ + projectId: project.id, + }); + + const payload = { + relations: [ + { + userId: member.id, + role: 'project:viewer', + }, + { + userId: member2.id, + role: 'project:editor', + }, + ], + }; + + const response = await testServer + .publicApiAgentFor(owner) + .post(`/projects/${project.id}/users`) + .send(payload); + + const projectAfter = await getAllProjectRelations({ + projectId: project.id, + }); + + expect(response.status).toBe(201); + expect(projectBefore.length).toEqual(1); + expect(projectBefore[0].userId).toEqual(owner.id); + + expect(projectAfter.length).toEqual(3); + const adminRelation = projectAfter.find( + (relation) => relation.userId === owner.id && relation.role === 'project:admin', + ); + expect(adminRelation).toEqual( + expect.objectContaining({ userId: owner.id, role: 'project:admin' }), + ); + const viewerRelation = projectAfter.find( + (relation) => relation.userId === member.id && relation.role === 'project:viewer', + ); + expect(viewerRelation).toEqual( + expect.objectContaining({ userId: member.id, role: 'project:viewer' }), + ); + const editorRelation = projectAfter.find( + (relation) => relation.userId === member2.id && relation.role === 'project:editor', + ); + expect(editorRelation).toEqual( + expect.objectContaining({ userId: member2.id, role: 'project:editor' }), + ); + }); + + it('should reject with 400 if license does not include user role', async () => { + const owner = await createOwnerWithApiKey(); + const project = await createTeamProject('shared-project', owner); + const member = await createMember(); + + const payload = { + relations: [ + { + userId: member.id, + role: 'project:viewer', + }, + ], + }; + + const response = await testServer + .publicApiAgentFor(owner) + .post(`/projects/${project.id}/users`) + .send(payload); + + expect(response.status).toBe(400); + expect(response.body).toHaveProperty( + 'message', + 'Your instance is not licensed to use role "project:viewer".', + ); + }); + }); + }); + + describe('PATCH /projects/:id/users/:userId', () => { + it('if not authenticated, should reject with 401', async () => { + const response = await testServer + .publicApiAgentWithoutApiKey() + .patch('/projects/123/users/456') + .send({ role: 'project:viewer' }); + + expect(response.status).toBe(401); + expect(response.body).toHaveProperty('message', "'X-N8N-API-KEY' header required"); + }); + + it('if not licensed, should reject with a 403', async () => { + const owner = await createOwnerWithApiKey(); + + const response = await testServer + .publicApiAgentFor(owner) + .patch('/projects/123/users/456') + .send({ role: 'project:viewer' }); + + expect(response.status).toBe(403); + expect(response.body).toHaveProperty( + 'message', + new FeatureNotLicensedError('feat:projectRole:admin').message, + ); + }); + + it('if missing scope, should reject with 403', async () => { + testServer.license.setQuota('quota:maxTeamProjects', -1); + testServer.license.enable('feat:projectRole:admin'); + const member = await createMemberWithApiKey(); + + const response = await testServer + .publicApiAgentFor(member) + .patch('/projects/123/users/456') + .send({ role: 'project:viewer' }); + + expect(response.status).toBe(403); + expect(response.body).toHaveProperty('message', 'Forbidden'); + }); + + describe('when user has correct license', () => { + beforeEach(() => { + testServer.license.setQuota('quota:maxTeamProjects', -1); + testServer.license.enable('feat:projectRole:admin'); + }); + + it("should reject with 400 if the payload can't be validated", async () => { + // ARRANGE + const owner = await createOwnerWithApiKey(); + + // ACT + const response = await testServer + .publicApiAgentFor(owner) + .patch('/projects/1234/users/1235') + // 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'", + ); + }); + + it("should change a user's role in a project", async () => { + const owner = await createOwnerWithApiKey(); + const project = await createTeamProject('shared-project', owner); + + const member = await createMember(); + expect(await getProjectRoleForUser(project.id, member.id)).toBeUndefined(); + + await linkUserToProject(member, project, 'project:viewer'); + expect(await getProjectRoleForUser(project.id, member.id)).toBe('project:viewer'); + + await testServer + .publicApiAgentFor(owner) + .patch(`/projects/${project.id}/users/${member.id}`) + .send({ role: 'project:editor' }) + .expect(204); + + expect(await getProjectRoleForUser(project.id, member.id)).toBe('project:editor'); + }); + + it('should reject with 404 if no project found', async () => { + const owner = await createOwnerWithApiKey(); + const member = await createMember(); + + const response = await testServer + .publicApiAgentFor(owner) + .patch(`/projects/123456/users/${member.id}`) + .send({ role: 'project:editor' }) + .expect(404); + + expect(response.body).toHaveProperty('message', 'Could not find project with ID: 123456'); + }); + + it('should reject with 404 if user is not in the project', async () => { + const owner = await createOwnerWithApiKey(); + const project = await createTeamProject('shared-project', owner); + const member = await createMember(); + + expect(await getProjectRoleForUser(project.id, member.id)).toBeUndefined(); + + const response = await testServer + .publicApiAgentFor(owner) + .patch(`/projects/${project.id}/users/${member.id}`) + .send({ role: 'project:editor' }) + .expect(404); + + expect(response.body).toHaveProperty( + 'message', + `Could not find project with ID: ${project.id}`, + ); + }); + }); + }); + + describe('DELETE /projects/:id/users/:userId', () => { + it('if not authenticated, should reject with 401', async () => { + const project = await createTeamProject(); + const member = await createMember(); + + const response = await testServer + .publicApiAgentWithoutApiKey() + .delete(`/projects/${project.id}/users/${member.id}`); + + expect(response.status).toBe(401); + expect(response.body).toHaveProperty('message', "'X-N8N-API-KEY' header required"); + }); + + it('if not licensed, should reject with a 403', async () => { + const owner = await createOwnerWithApiKey(); + const project = await createTeamProject(); + const member = await createMember(); + + const response = await testServer + .publicApiAgentFor(owner) + .delete(`/projects/${project.id}/users/${member.id}`); + + expect(response.status).toBe(403); + expect(response.body).toHaveProperty( + 'message', + new FeatureNotLicensedError('feat:projectRole:admin').message, + ); + }); + + it('if missing scope, should reject with 403', async () => { + testServer.license.setQuota('quota:maxTeamProjects', -1); + testServer.license.enable('feat:projectRole:admin'); + const member = await createMemberWithApiKey(); + const project = await createTeamProject(); + + const response = await testServer + .publicApiAgentFor(member) + .delete(`/projects/${project.id}/users/${member.id}`); + + expect(response.status).toBe(403); + expect(response.body).toHaveProperty('message', 'Forbidden'); + }); + + describe('when user has correct license', () => { + beforeEach(() => { + testServer.license.setQuota('quota:maxTeamProjects', -1); + testServer.license.enable('feat:projectRole:admin'); + }); + + it('should remove given user from project', async () => { + const owner = await createOwnerWithApiKey(); + const project = await createTeamProject('shared-project', owner); + const member = await createMember(); + await linkUserToProject(member, project, 'project:viewer'); + const projectBefore = await getAllProjectRelations({ + projectId: project.id, + }); + + const response = await testServer + .publicApiAgentFor(owner) + .delete(`/projects/${project.id}/users/${member.id}`); + + const projectAfter = await getAllProjectRelations({ + projectId: project.id, + }); + + expect(response.status).toBe(204); + expect(projectBefore.length).toEqual(2); + expect(projectBefore.find((p) => p.role === 'project:admin')?.userId).toEqual(owner.id); + expect(projectBefore.find((p) => p.role === 'project:viewer')?.userId).toEqual(member.id); + + expect(projectAfter.length).toEqual(1); + expect(projectAfter[0].userId).toEqual(owner.id); + }); + + it('should reject with 404 if no project found', async () => { + const owner = await createOwnerWithApiKey(); + const member = await createMember(); + + const response = await testServer + .publicApiAgentFor(owner) + .delete(`/projects/123456/users/${member.id}`); + + expect(response.status).toBe(404); + expect(response.body).toHaveProperty('message', 'Could not find project with ID: 123456'); + }); + + it('should remain unchanged if user if not in project', async () => { + const owner = await createOwnerWithApiKey(); + const project = await createTeamProject('shared-project', owner); + const member = await createMember(); + const projectBefore = await getAllProjectRelations({ + projectId: project.id, + }); + + const response = await testServer + .publicApiAgentFor(owner) + .delete(`/projects/${project.id}/users/${member.id}`); + + const projectAfter = await getAllProjectRelations({ + projectId: project.id, + }); + + expect(response.status).toBe(204); + expect(projectBefore.length).toEqual(1); + expect(projectBefore[0].userId).toEqual(owner.id); + + expect(projectAfter.length).toEqual(1); + expect(projectAfter[0].userId).toEqual(owner.id); + }); + }); + }); }); diff --git a/packages/cli/test/integration/services/project.service.test.ts b/packages/cli/test/integration/services/project.service.test.ts index 050666f588..2454f66611 100644 --- a/packages/cli/test/integration/services/project.service.test.ts +++ b/packages/cli/test/integration/services/project.service.test.ts @@ -52,7 +52,7 @@ describe('ProjectService', () => { // // ACT // - await projectService.addUser(project.id, member.id, role); + await projectService.addUser(project.id, { userId: member.id, role }); // // ASSERT @@ -74,7 +74,7 @@ describe('ProjectService', () => { type: 'team', }), ); - await projectService.addUser(project.id, member.id, 'project:viewer'); + await projectService.addUser(project.id, { userId: member.id, role: 'project:viewer' }); await projectRelationRepository.findOneOrFail({ where: { userId: member.id, projectId: project.id, role: 'project:viewer' }, @@ -83,7 +83,7 @@ describe('ProjectService', () => { // // ACT // - await projectService.addUser(project.id, member.id, 'project:admin'); + await projectService.addUser(project.id, { userId: member.id, role: 'project:admin' }); // // ASSERT @@ -117,7 +117,7 @@ describe('ProjectService', () => { type: 'team', }), ); - await projectService.addUser(project.id, projectOwner.id, role); + await projectService.addUser(project.id, { userId: projectOwner.id, role }); // // ACT @@ -157,7 +157,7 @@ describe('ProjectService', () => { type: 'team', }), ); - await projectService.addUser(project.id, projectViewer.id, role); + await projectService.addUser(project.id, { userId: projectViewer.id, role }); // // ACT @@ -200,4 +200,44 @@ describe('ProjectService', () => { expect(projectFromService).toBeNull(); }); }); + + describe('deleteUserFromProject', () => { + it('should not allow project owner to be removed from the project', async () => { + const role = 'project:personalOwner'; + + const user = await createMember(); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + await projectService.addUser(project.id, { userId: user.id, role }); + + await expect(projectService.deleteUserFromProject(project.id, user.id)).rejects.toThrowError( + /^Project owner cannot be removed from the project$/, + ); + }); + + it('should remove user from project if not owner', async () => { + const role = 'project:editor'; + + const user = await createMember(); + const project = await projectRepository.save( + projectRepository.create({ + name: 'Team Project', + type: 'team', + }), + ); + await projectService.addUser(project.id, { userId: user.id, role }); + + await projectService.deleteUserFromProject(project.id, user.id); + + const relations = await projectRelationRepository.findOne({ + where: { userId: user.id, projectId: project.id, role }, + }); + + expect(relations).toBeNull(); + }); + }); }); diff --git a/packages/cli/test/integration/shared/db/projects.ts b/packages/cli/test/integration/shared/db/projects.ts index 5e910952dc..ba5bf3d78b 100644 --- a/packages/cli/test/integration/shared/db/projects.ts +++ b/packages/cli/test/integration/shared/db/projects.ts @@ -66,3 +66,23 @@ export const getProjectRelations = async ({ where: { projectId, userId, role }, }); }; + +export const getProjectRoleForUser = async ( + projectId: string, + userId: string, +): Promise => { + return ( + await Container.get(ProjectRelationRepository).findOne({ + select: ['role'], + where: { projectId, userId }, + }) + )?.role; +}; + +export const getAllProjectRelations = async ({ + projectId, +}: Partial): Promise => { + return await Container.get(ProjectRelationRepository).find({ + where: { projectId }, + }); +}; diff --git a/packages/cli/test/integration/workflows/workflow-sharing.service.test.ts b/packages/cli/test/integration/workflows/workflow-sharing.service.test.ts index a88ba352a2..a1647349f9 100644 --- a/packages/cli/test/integration/workflows/workflow-sharing.service.test.ts +++ b/packages/cli/test/integration/workflows/workflow-sharing.service.test.ts @@ -72,7 +72,7 @@ describe('WorkflowSharingService', () => { // ARRANGE // const project = await projectService.createTeamProject(member, { name: 'Team Project' }); - await projectService.addUser(project.id, anotherMember.id, 'project:admin'); + await projectService.addUser(project.id, { userId: anotherMember.id, role: 'project:admin' }); const workflow = await createWorkflow(undefined, project); // @@ -96,8 +96,14 @@ describe('WorkflowSharingService', () => { const workflow1 = await createWorkflow(undefined, project1); const project2 = await projectService.createTeamProject(member, { name: 'Team Project 2' }); const workflow2 = await createWorkflow(undefined, project2); - await projectService.addUser(project1.id, anotherMember.id, 'project:admin'); - await projectService.addUser(project2.id, anotherMember.id, 'project:viewer'); + await projectService.addUser(project1.id, { + userId: anotherMember.id, + role: 'project:admin', + }); + await projectService.addUser(project2.id, { + userId: anotherMember.id, + role: 'project:viewer', + }); // // ACT diff --git a/packages/cli/test/integration/workflows/workflows.controller.test.ts b/packages/cli/test/integration/workflows/workflows.controller.test.ts index baadbc6785..491013a55a 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.test.ts @@ -54,7 +54,6 @@ const { objectContaining, arrayContaining, any } = expect; const activeWorkflowManagerLike = mockInstance(ActiveWorkflowManager); let projectRepository: ProjectRepository; -let projectService: ProjectService; beforeEach(async () => { await testDb.truncate([ @@ -68,7 +67,6 @@ beforeEach(async () => { 'User', ]); projectRepository = Container.get(ProjectRepository); - projectService = Container.get(ProjectService); owner = await createOwner(); authOwnerAgent = testServer.authAgentFor(owner); member = await createMember(); @@ -288,7 +286,10 @@ describe('POST /workflows', () => { type: 'team', }), ); - await projectService.addUser(project.id, owner.id, 'project:admin'); + await Container.get(ProjectService).addUser(project.id, { + userId: owner.id, + role: 'project:admin', + }); // // ACT @@ -362,7 +363,10 @@ describe('POST /workflows', () => { type: 'team', }), ); - await projectService.addUser(project.id, member.id, 'project:viewer'); + await Container.get(ProjectService).addUser(project.id, { + userId: member.id, + role: 'project:viewer', + }); // // ACT diff --git a/packages/frontend/editor-ui/src/views/ProjectSettings.vue b/packages/frontend/editor-ui/src/views/ProjectSettings.vue index 5ff3186c46..73577c7555 100644 --- a/packages/frontend/editor-ui/src/views/ProjectSettings.vue +++ b/packages/frontend/editor-ui/src/views/ProjectSettings.vue @@ -1,5 +1,5 @@