use DTOs for these new endpoints

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2025-01-09 20:50:19 +01:00
parent 79d347fb23
commit d976cb6217
No known key found for this signature in database
16 changed files with 437 additions and 149 deletions

View file

@ -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';

View file

@ -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);
}
});
});
});

View file

@ -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);
}
});
});
});

View file

@ -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),
}) {}

View file

@ -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']),
}) {}

View file

@ -20,7 +20,7 @@ export const projectRoleSchema = z.enum([
export type ProjectRole = z.infer<typeof projectRoleSchema>;
export const projectRelationSchema = z.object({
userId: z.string(),
userId: z.string().min(1),
role: projectRoleSchema,
});
export type ProjectRelation = z.infer<typeof projectRelationSchema>;

View file

@ -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(

View file

@ -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();
},
],
};

View file

@ -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'

View file

@ -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'

View file

@ -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.

View file

@ -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'

View file

@ -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[] }>;
}
// ----------------------------------

View file

@ -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<UpdateProjectDto, 'name' | 'icon'>,
): Promise<Project> {
const result = await this.projectRepository.update({ id: projectId, type: 'team' }, data);
async updateProject(projectId: string, { name, icon }: UpdateProjectDto): Promise<void> {
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<Project | null> {
@ -211,22 +207,10 @@ export class ProjectService {
async syncProjectRelations(
projectId: string,
relations: Array<{ userId: string; role: ProjectRole }>,
relations: Required<UpdateProjectDto>['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<UpdateProjectDto>['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<UpdateProjectDto>['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: {

View file

@ -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);

View file

@ -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',