diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 9be09c02f3..7d355bc6a2 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -24,6 +24,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..5e92ef23cf --- /dev/null +++ b/packages/@n8n/api-types/src/dto/project/__tests__/add-users-to-project.dto.test.ts @@ -0,0 +1,138 @@ +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', + request: { + relations: [ + { userId: 'user-1', role: 'project:personalOwner' }, + { userId: 'user-2', role: 'project:admin' }, + { userId: 'user-3', role: 'project:editor' }, + { userId: 'user-4', 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], + }, + ])('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..f9a01dec26 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/project/change-user-role-in-project.dto.ts @@ -0,0 +1,7 @@ +import { Z } from 'zod-class'; + +import { projectRoleSchema } from '../../schemas/project.schema'; + +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 11c6cc2b37..9a52a51fc4 100644 --- a/packages/@n8n/api-types/src/schemas/project.schema.ts +++ b/packages/@n8n/api-types/src/schemas/project.schema.ts @@ -20,7 +20,7 @@ export const projectRoleSchema = z.enum([ export type ProjectRole = z.infer; export const projectRelationSchema = z.object({ - userId: z.string(), + userId: z.string().min(1), role: projectRoleSchema, }); export type ProjectRelation = z.infer; diff --git a/packages/cli/src/controllers/project.controller.ts b/packages/cli/src/controllers/project.controller.ts index 56e493bfcb..abc15d3aec 100644 --- a/packages/cli/src/controllers/project.controller.ts +++ b/packages/cli/src/controllers/project.controller.ts @@ -1,4 +1,3 @@ -import type { ProjectRelation } from '@n8n/api-types'; import { CreateProjectDto, DeleteProjectDto, UpdateProjectDto } from '@n8n/api-types'; import { combineScopes } from '@n8n/permissions'; import type { Scope } from '@n8n/permissions'; @@ -26,11 +25,7 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { EventService } from '@/events/event.service'; import type { ProjectRequest } from '@/requests'; import { AuthenticatedRequest } from '@/requests'; -import { - ProjectService, - TeamProjectOverQuotaError, - UnlicensedProjectRoleError, -} from '@/services/project.service.ee'; +import { ProjectService, TeamProjectOverQuotaError } from '@/services/project.service.ee'; import { RoleService } from '@/services/role.service'; @RestController('/projects') @@ -211,9 +206,9 @@ export class ProjectController { if (name || icon) { await this.projectsService.updateProject(projectId, { name, icon }); } - if (relations) { - await this.syncProjectRelations(projectId, relations); + if (relations) { + await this.projectsService.syncProjectRelations(projectId, relations); this.eventService.emit('team-project-updated', { userId: req.user.id, role: req.user.role, @@ -223,17 +218,6 @@ export class ProjectController { } } - async syncProjectRelations(projectId: string, relations: ProjectRelation[]) { - try { - await this.projectsService.syncProjectRelations(projectId, relations); - } catch (e) { - if (e instanceof UnlicensedProjectRoleError) { - throw new BadRequestError(e.message); - } - throw e; - } - } - @Delete('/:projectId') @ProjectScope('project:delete') async deleteProject( 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 85bf1674d7..890848c35d 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 { Container } from '@n8n/di'; import type { Response } from 'express'; import { ProjectController } from '@/controllers/project.controller'; import { ProjectRepository } from '@/databases/repositories/project.repository'; +import { ResponseError } from '@/errors/response-errors/abstract/response.error'; import type { PaginatedRequest } from '@/public-api/types'; -import type { AuthenticatedRequest, ProjectRequest } from '@/requests'; +import type { AuthenticatedRequest } from '@/requests'; +import { ProjectService } from '@/services/project.service.ee'; import { globalScope, isLicensed, validCursor } from '../../shared/middlewares/global.middleware'; import { encodeNextCursor } from '../../shared/services/pagination.service'; @@ -87,65 +95,67 @@ export = { }); }, ], - deleteUserFromProject: [ - isLicensed('feat:projectRole:admin'), - globalScope('project:update'), - async (req: ProjectRequest.DeleteUser, res: Response) => { - const { projectId, id: userId } = req.params; - - const project = await Container.get(ProjectRepository).findOne({ - where: { id: projectId }, - relations: { projectRelations: true }, - }); - - if (!project) { - return res.status(404).send({ message: 'Not found' }); - } - - const relations = project.projectRelations.filter((relation) => relation.userId !== userId); - - await Container.get(ProjectController).syncProjectRelations(projectId, relations); - - return res.status(204).send(); - }, - ], addUsersToProject: [ isLicensed('feat:projectRole:admin'), globalScope('project:update'), - async (req: ProjectRequest.AddUsers, res: Response) => { - const { projectId } = req.params; - const { users } = req.body; - - const project = await Container.get(ProjectRepository).findOne({ - where: { id: projectId }, - relations: { projectRelations: true }, - }); - - if (!project) { - return res.status(404).send({ message: 'Not found' }); + 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]); } - const existingUsers = project.projectRelations.map((relation) => ({ - userId: relation.userId, - role: relation.role, - })); - - // TODO: - // - What happens when the user is already in the project? - // - What happens when the user is not found on the instance? - try { - await Container.get(ProjectController).syncProjectRelations(projectId, [ - ...existingUsers, - ...users, - ]); + await Container.get(ProjectService).addUsersToProject( + req.params.projectId, + payload.data.relations, + ); } catch (error) { - return res - .status(400) - .send({ message: error instanceof Error ? error.message : 'Bad request' }); + 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'), + globalScope('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(201).send(); + }, + ], + deleteUserFromProject: [ + isLicensed('feat:projectRole:admin'), + globalScope('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.id.yml b/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.users.id.yml deleted file mode 100644 index d2bba47b95..0000000000 --- a/packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.users.id.yml +++ /dev/null @@ -1,24 +0,0 @@ -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 - - $ref: '../../../users/spec/schemas/parameters/userIdentifier.yml' - 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.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 index 8114c829dd..7ba861137d 100644 --- 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 @@ -19,9 +19,9 @@ post: schema: type: object properties: - users: + relations: type: array - description: A list of users and roles to add to the project. + description: A list of userIds and roles to add to the project. items: type: object properties: @@ -37,7 +37,7 @@ post: - userId - role required: - - users + - relations responses: '201': description: Operation successful. diff --git a/packages/cli/src/public-api/v1/openapi.yml b/packages/cli/src/public-api/v1/openapi.yml index 3f646dc2fb..7afae9651c 100644 --- a/packages/cli/src/public-api/v1/openapi.yml +++ b/packages/cli/src/public-api/v1/openapi.yml @@ -84,8 +84,8 @@ paths: $ref: './handlers/projects/spec/paths/projects.projectId.yml' /projects/{projectId}/users: $ref: './handlers/projects/spec/paths/projects.projectId.users.yml' - /projects/{projectId}/users/{id}: - $ref: './handlers/projects/spec/paths/projects.projectId.users.id.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/requests.ts b/packages/cli/src/requests.ts index 2a361ed51f..b9a5ae97a3 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -1,4 +1,4 @@ -import type { ProjectIcon, ProjectRelation, ProjectRole, ProjectType } from '@n8n/api-types'; +import type { ProjectIcon, ProjectRole, ProjectType } from '@n8n/api-types'; import type { Scope } from '@n8n/permissions'; import type express from 'express'; import type { @@ -396,9 +396,6 @@ export declare namespace ProjectRequest { relations: ProjectRelationResponse[]; scopes: Scope[]; }; - - type DeleteUser = AuthenticatedRequest<{ projectId: string; id: string }, {}, {}, {}>; - type AddUsers = AuthenticatedRequest<{ projectId: string }, {}, { users: ProjectRelation[] }>; } // ---------------------------------- diff --git a/packages/cli/src/services/project.service.ee.ts b/packages/cli/src/services/project.service.ee.ts index 43605939a3..a92029ee69 100644 --- a/packages/cli/src/services/project.service.ee.ts +++ b/packages/cli/src/services/project.service.ee.ts @@ -4,7 +4,7 @@ import { type Scope } from '@n8n/permissions'; // 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 { ApplicationError } from 'n8n-workflow'; import { UNLIMITED_LICENSE_QUOTA } from '@/constants'; @@ -16,7 +16,6 @@ import { ProjectRepository } from '@/databases/repositories/project.repository'; import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { License } from '@/license'; @@ -31,12 +30,18 @@ export class TeamProjectOverQuotaError extends ApplicationError { } } -export class UnlicensedProjectRoleError extends ApplicationError { +class UnlicensedProjectRoleError extends BadRequestError { constructor(role: ProjectRole) { super(`Your instance is not licensed to use role "${role}".`); } } +class ProjectNotFoundError extends NotFoundError { + constructor() { + super('Project not found.'); + } +} + @Service() export class ProjectService { constructor( @@ -76,8 +81,8 @@ export class ProjectService { } const project = await this.getProjectWithScope(user, projectId, ['project:delete']); - if (!project) { - throw new NotFoundError(`Could not find project with ID: ${projectId}`); + if (!project || project.type !== 'team') { + throw new ProjectNotFoundError(); } let targetProject: Project | null = null; @@ -94,13 +99,6 @@ export class ProjectService { } } - // 0. check if this is a team project - if (project.type !== 'team') { - throw new ForbiddenError( - `Can't delete project. Project with ID "${projectId}" is not a team project.`, - ); - } - // 1. delete or migrate workflows owned by this project const ownedSharedWorkflows = await this.sharedWorkflowRepository.find({ where: { projectId: project.id, role: 'workflow:owner' }, @@ -186,16 +184,14 @@ export class ProjectService { return project; } - 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(); } - return await this.projectRepository.findOneByOrFail({ id: projectId }); } async getPersonalProject(user: User): Promise { @@ -211,22 +207,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.roleService.isRoleLicensed(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); @@ -235,6 +219,63 @@ export class ProjectService { await this.clearCredentialCanUseExternalSecretsCache(projectId); } + async addUsersToProject(projectId: string, relations: Required['relations']) { + const project = await this.getTeamProjectWithRelations(projectId); + this.checkRolesLicensed(project, relations); + + // TODO: assert that the user exists, else invite the user first + // TODO: skip inserting if the user already exists + await this.projectRelationRepository.upsert( + relations.map((relation) => + this.projectRelationRepository.create({ projectId, ...relation }), + ), + ['projectId', 'userId'], + ); + } + + private async getTeamProjectWithRelations(projectId: string) { + const project = await this.projectRepository.findOne({ + where: { id: projectId, type: 'team' }, + relations: { projectRelations: true }, + }); + if (!project) { + throw new ProjectNotFoundError(); + } + return project; + } + + /** Check to see if the instance is licensed to use all roles provided */ + private checkRolesLicensed(project: Project, relations: Required['relations']) { + 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); + } + } + } + + async deleteUserFromProject(projectId: string, userId: string) { + const projectExists = await this.projectRepository.existsBy({ id: projectId }); + if (!projectExists) { + throw new ProjectNotFoundError(); + } + + // TODO: do we need to prevent project owner from being removed? + await this.projectRelationRepository.delete({ projectId, userId }); + } + + async changeUserRoleInProject(projectId: string, userId: string, role: ProjectRole) { + const projectUserExists = await this.projectRelationRepository.existsBy({ projectId, userId }); + if (!projectUserExists) { + throw new ProjectNotFoundError(); + } + + // TODO: do we need to block any specific roles here? + await this.projectRelationRepository.update({ projectId, userId }, { role }); + } + async clearCredentialCanUseExternalSecretsCache(projectId: string) { const shares = await this.sharedCredentialsRepository.find({ where: { diff --git a/packages/cli/test/integration/project.api.test.ts b/packages/cli/test/integration/project.api.test.ts index 5f7c7e1b27..6d7374a82e 100644 --- a/packages/cli/test/integration/project.api.test.ts +++ b/packages/cli/test/integration/project.api.test.ts @@ -473,7 +473,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'); @@ -821,7 +821,7 @@ describe('DELETE /project/:projectId', () => { const owner = await createOwner(); const project = await getPersonalProject(owner); - await testServer.authAgentFor(owner).delete(`/projects/${project.id}`).expect(403); + await testServer.authAgentFor(owner).delete(`/projects/${project.id}`).expect(404); const projectInDB = await findProject(project.id); diff --git a/packages/cli/test/integration/public-api/projects.test.ts b/packages/cli/test/integration/public-api/projects.test.ts index a1b2c9acb2..1d6b980d56 100644 --- a/packages/cli/test/integration/public-api/projects.test.ts +++ b/packages/cli/test/integration/public-api/projects.test.ts @@ -488,7 +488,7 @@ describe('Projects in Public API', () => { .delete(`/projects/123456/users/${member.id}`); expect(response.status).toBe(404); - expect(response.body).toHaveProperty('message', 'Not found'); + expect(response.body).toHaveProperty('message', 'Project not found.'); }); it('should remain unchanged if user if not in project', async () => { @@ -535,7 +535,7 @@ describe('Projects in Public API', () => { const member = await createMember(); const payload = { - users: [ + relations: [ { userId: member.id, role: 'project:viewer', @@ -562,7 +562,7 @@ describe('Projects in Public API', () => { const project = await createTeamProject(); const payload = { - users: [ + relations: [ { userId: member.id, role: 'project:viewer', @@ -590,7 +590,7 @@ describe('Projects in Public API', () => { const member = await createMember(); const payload = { - users: [ + relations: [ { userId: member.id, role: 'project:viewer', @@ -600,11 +600,11 @@ describe('Projects in Public API', () => { const response = await testServer .publicApiAgentFor(owner) - .post('/projects/123456/users/') + .post('/projects/123456/users') .send(payload); expect(response.status).toBe(404); - expect(response.body).toHaveProperty('message', 'Not found'); + expect(response.body).toHaveProperty('message', 'Project not found.'); }); it('should add expected users to project', async () => { @@ -619,7 +619,7 @@ describe('Projects in Public API', () => { }); const payload = { - users: [ + relations: [ { userId: member.id, role: 'project:viewer', @@ -662,7 +662,7 @@ describe('Projects in Public API', () => { const member = await createMember(); const payload = { - users: [ + relations: [ { userId: member.id, role: 'project:viewer',