Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(API): Allow user to be added to and removed from projects #12329

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions packages/cli/src/controllers/project.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,7 @@ export class ProjectController {
await this.projectsService.updateProject(req.body.name, req.params.projectId);
}
if (req.body.relations) {
try {
await this.projectsService.syncProjectRelations(req.params.projectId, req.body.relations);
} catch (e) {
if (e instanceof UnlicensedProjectRoleError) {
throw new BadRequestError(e.message);
}
throw e;
}
await this.syncProjectRelations(req.params.projectId, req.body.relations);

this.eventService.emit('team-project-updated', {
userId: req.user.id,
Expand All @@ -214,6 +207,20 @@ export class ProjectController {
}
}

async syncProjectRelations(
projectId: string,
relations: ProjectRequest.ProjectRelationPayload[],
) {
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(req: ProjectRequest.Delete) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { encodeNextCursor } from '../../shared/services/pagination.service';
type Create = ProjectRequest.Create;
type Update = ProjectRequest.Update;
type Delete = ProjectRequest.Delete;
type DeleteUser = ProjectRequest.DeleteUser;
type GetAll = PaginatedRequest;

export = {
Expand Down Expand Up @@ -64,4 +65,26 @@ export = {
});
},
],
deleteUserFromProject: [
isLicensed('feat:projectRole:admin'),
globalScope('project:update'),
async (req: 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();
},
],
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
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'
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -23,6 +28,13 @@ put:
- Project
summary: Update a project
description: Update a project.
parameters:
- in: path
name: projectId
description: The ID of the project.
required: true
schema:
type: string
Comment on lines +31 to +37
Copy link
Contributor Author

@MarcL MarcL Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was missing the projectId parameter when run through an OpenAPI schema validator.

requestBody:
description: Updated project object.
content:
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/public-api/v1/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ paths:
$ref: './handlers/projects/spec/paths/projects.yml'
/projects/{projectId}:
$ref: './handlers/projects/spec/paths/projects.projectId.yml'
/projects/{projectId}/users/{id}:
$ref: './handlers/projects/spec/paths/projects.projectId.users.id.yml'
components:
schemas:
$ref: './shared/spec/schemas/_index.yml'
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ export declare namespace ProjectRequest {
{ name?: string; relations?: ProjectRelationPayload[] }
>;
type Delete = AuthenticatedRequest<{ projectId: string }, {}, {}, { transferId?: string }>;
type DeleteUser = AuthenticatedRequest<{ projectId: string; id: string }, {}, {}, {}>;
}

// ----------------------------------
Expand Down
126 changes: 124 additions & 2 deletions packages/cli/test/integration/public-api/projects.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
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,
} 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';
Expand Down Expand Up @@ -393,4 +402,117 @@ describe('Projects in Public API', () => {
expect(response.body).toHaveProperty('message', 'Forbidden');
});
});

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[0].userId).toEqual(owner.id);
expect(projectBefore[1].userId).toEqual(member.id);

expect(projectAfter.length).toEqual(1);
expect(projectBefore[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', 'Not found');
});

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(projectBefore[0].userId).toEqual(owner.id);
});
});
});
});
8 changes: 8 additions & 0 deletions packages/cli/test/integration/shared/db/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,11 @@ export const getProjectRelations = async ({
where: { projectId, userId, role },
});
};

export const getAllProjectRelations = async ({
projectId,
}: Partial<ProjectRelation>): Promise<ProjectRelation[]> => {
return await Container.get(ProjectRelationRepository).find({
where: { projectId },
});
};
Loading