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): Add user management endpoints to the Projects Public API #12329

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5a5ce6e
feat(API): Allow deletion of users from projects
MarcL Dec 19, 2024
6dcbb7a
fix: tidy up schema references
MarcL Dec 20, 2024
d471ed1
fix: refactor schema to make test server work
MarcL Dec 20, 2024
0cd58cb
test: add tests for deleting users from project
MarcL Dec 20, 2024
73a4b9b
fix: move all project endpoints into same tag for docs
MarcL Dec 20, 2024
cdfc40d
feat: add ability to add users to projects via API
MarcL Dec 20, 2024
e9d96f5
Merge remote-tracking branch 'origin/master' into pay-1852-public-api…
netroy Jan 9, 2025
79d347f
Merge remote-tracking branch 'origin/master' into pay-1852-public-api…
netroy Jan 13, 2025
d976cb6
use DTOs for these new endpoints
netroy Jan 9, 2025
e5ece7f
more tests
netroy Jan 13, 2025
9f4727f
Merge remote-tracking branch 'origin/master' into pay-1852-public-api…
netroy Jan 13, 2025
da0fbfe
add projectId to error message
despairblue Jan 14, 2025
7dafb2a
send back ForbiddenError when the user tries to delete a personal pro…
despairblue Jan 14, 2025
70c6792
use type guard
despairblue Jan 14, 2025
741791e
add test for validation error
despairblue Jan 14, 2025
a7af9dd
prevent unique constraint errors
despairblue Jan 14, 2025
307ede1
use `addUsersToProject` to implement `addUser`
despairblue Jan 14, 2025
f3c99b9
don't allow adding personal owners to team project, nor allow editing…
despairblue Jan 15, 2025
4835ad2
test public api validation
despairblue Jan 15, 2025
0a030ed
Merge remote-tracking branch 'origin/master' into pay-1852-public-api…
netroy Jan 20, 2025
fc38d4b
fix the build
netroy Jan 20, 2025
8404464
Merge remote-tracking branch 'origin/master' into pay-1852-public-api…
netroy Jan 24, 2025
c9052b3
revert the addUser change for now
netroy Jan 24, 2025
a042310
syncProjectRelations tests
netroy Jan 24, 2025
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
2 changes: 2 additions & 0 deletions packages/@n8n/api-types/src/dto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
});
});
});
Original file line number Diff line number Diff line change
@@ -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);
}
});
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if the public API DTOs should be in it's own folder?

Copy link
Contributor

Choose a reason for hiding this comment

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

And if the public API should define their own DTOs. We don't want to give the public API more power by accident when we add more power to the internal API.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should definitely keep all public API DTOs in the public-api folder, since they don't need to be shared with editor-ui. but we can do that later in another PR.

Original file line number Diff line number Diff line change
@@ -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),
}) {}
Original file line number Diff line number Diff line change
@@ -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']),
}) {}
4 changes: 2 additions & 2 deletions packages/@n8n/api-types/src/schemas/project.schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const projectRoleSchema = z.enum([
export type ProjectRole = z.infer<typeof projectRoleSchema>;

export const projectRelationSchema = z.object({
userId: z.string(),
role: projectRoleSchema,
userId: z.string().min(1),
netroy marked this conversation as resolved.
Show resolved Hide resolved
role: projectRoleSchema.exclude(['project:personalOwner']),
Copy link
Contributor

Choose a reason for hiding this comment

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

I've excluded the project:personalOwner here too. Personal projects are special, they can only have one owner with that role. There should not be any API endpoint that allows changing the relation. They are created when the user is created and deleted when the user is deleted.

Copy link
Member

Choose a reason for hiding this comment

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

I've temporarily reverted this change, to make the build pass again.
Since there is a a lot more projects related types that are still duplicated between packages, I'd like to clean this up later in a separate PR as well.

});
export type ProjectRelation = z.infer<typeof projectRelationSchema>;
17 changes: 3 additions & 14 deletions packages/cli/src/controllers/project.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,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')
Expand Down Expand Up @@ -210,16 +206,9 @@ export class ProjectController {
if (name || icon) {
await this.projectsService.updateProject(projectId, { name, icon });
}
if (relations) {
try {
await this.projectsService.syncProjectRelations(projectId, relations);
} catch (e) {
if (e instanceof UnlicensedProjectRoleError) {
throw new BadRequestError(e.message);
}
throw e;
}

if (relations) {
await this.projectsService.syncProjectRelations(projectId, relations);
this.eventService.emit('team-project-updated', {
userId: req.user.id,
role: req.user.role,
Expand Down
Original file line number Diff line number Diff line change
@@ -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 } 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';
Expand Down Expand Up @@ -87,4 +95,67 @@ export = {
});
},
],
addUsersToProject: [
isLicensed('feat:projectRole:admin'),
globalScope('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'),
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(204).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();
},
],
};
Loading
Loading