feat(core): Allow custom project roles from being set to a user project relation (#18926)

This commit is contained in:
Guillaume Jacquart
2025-09-01 10:22:33 +02:00
committed by GitHub
parent 5b5f60212a
commit 027edbe89d
32 changed files with 597 additions and 121 deletions

View File

@@ -9,13 +9,11 @@ import {
testDb,
mockInstance,
} from '@n8n/backend-test-utils';
import type { Project, ProjectRole, User } from '@n8n/db';
import type { Project, User } from '@n8n/db';
import { FolderRepository, ProjectRepository, WorkflowRepository } from '@n8n/db';
import { Container } from '@n8n/di';
import { DateTime } from 'luxon';
import { ApplicationError, PROJECT_ROOT } from 'n8n-workflow';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
import type { ProjectRole } from '@n8n/permissions';
import { PROJECT_EDITOR_ROLE_SLUG, PROJECT_VIEWER_ROLE_SLUG } from '@n8n/permissions';
import {
createCredentials,
getCredentialSharings,
@@ -25,11 +23,15 @@ import {
} from '@test-integration/db/credentials';
import { createFolder } from '@test-integration/db/folders';
import { createTag } from '@test-integration/db/tags';
import { DateTime } from 'luxon';
import { ApplicationError, PROJECT_ROOT } from 'n8n-workflow';
import { createOwner, createMember, createUser, createAdmin } from '../shared/db/users';
import type { SuperAgentTest } from '../shared/types';
import * as utils from '../shared/utils/';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
let owner: User;
let member: User;
let authOwnerAgent: SuperAgentTest;
@@ -1863,7 +1865,7 @@ describe('PUT /projects/:projectId/folders/:folderId/transfer', () => {
.expect(404);
});
test.each<ProjectRole>(['project:editor', 'project:viewer'])(
test.each<ProjectRole>([PROJECT_EDITOR_ROLE_SLUG, PROJECT_VIEWER_ROLE_SLUG])(
'%ss cannot transfer workflows',
async (projectRole) => {
//

View File

@@ -483,8 +483,8 @@ describe('Projects in Public API', () => {
relations: [
{
userId: member.id,
// role does not exist
role: 'project:boss',
// field does not exist
invalidField: 'invalidValue',
},
],
};
@@ -499,10 +499,33 @@ describe('Projects in Public API', () => {
// ASSERT
expect(response.body).toHaveProperty(
'message',
"Invalid enum value. Expected 'project:admin' | 'project:editor' | 'project:viewer', received 'project:boss'",
"request/body/relations/0 must have required property 'role'",
);
});
it('should reject if the relations have a role that do not exist', async () => {
const owner = await createOwnerWithApiKey();
const member = await createMember();
const project = await createTeamProject('shared-project', owner);
const payload = {
relations: [
{
userId: member.id,
role: 'project:invalid-role',
},
],
};
await testServer
.publicApiAgentFor(owner)
.post(`/projects/${project.id}/users`)
.send(payload)
.expect(400);
// TODO: add message check once we properly validate role from database
});
it('should reject with 404 if no project found', async () => {
const owner = await createOwnerWithApiKey();
const member = await createMember();
@@ -654,23 +677,23 @@ describe('Projects in Public API', () => {
testServer.license.enable('feat:projectRole:admin');
});
it("should reject with 400 if the payload can't be validated", async () => {
it('should reject with 400 if the role do not exist', async () => {
// ARRANGE
const owner = await createOwnerWithApiKey();
const member = await createMember();
const project = await createTeamProject('shared-project', owner);
await linkUserToProject(member, project, 'project:viewer');
// ACT
const response = await testServer
await testServer
.publicApiAgentFor(owner)
.patch('/projects/1234/users/1235')
.patch(`/projects/${project.id}/users/${member.id}`)
// 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'",
);
// TODO: add message check once we properly validate that the role exists
});
it("should change a user's role in a project", async () => {

View File

@@ -9,6 +9,7 @@ import {
getUserById,
} from '@test-integration/db/users';
import { setupTestServer } from '@test-integration/utils';
import { createRole } from '@test-integration/db/roles';
describe('Users in Public API', () => {
const testServer = setupTestServer({ endpointGroups: ['publicApi'] });
@@ -61,13 +62,32 @@ describe('Users in Public API', () => {
expect(response.body).toHaveProperty('message', 'Forbidden');
});
it('should fail if role does not exist', async () => {
/**
* Arrange
*/
testServer.license.enable('feat:advancedPermissions');
const owner = await createOwnerWithApiKey();
const payload = [{ email: 'test@test.com', role: 'non-existing-role' }];
/**
* Act
*/
const response = await testServer.publicApiAgentFor(owner).post('/users').send(payload);
/**
* Assert
*/
expect(response.status).toBe(400);
expect(response.body).toHaveProperty('message', 'Role non-existing-role does not exist');
});
it('should create a user', async () => {
/**
* Arrange
*/
testServer.license.enable('feat:advancedPermissions');
const owner = await createOwnerWithApiKey();
await createOwnerWithApiKey();
const payload = [{ email: 'test@test.com', role: 'global:admin' }];
/**
@@ -97,6 +117,27 @@ describe('Users in Public API', () => {
expect(returnedUser.email).toBe(payloadUser.email);
expect(storedUser.role.slug).toBe(payloadUser.role);
});
it('should create a user with an existing custom role', async () => {
/**
* Arrange
*/
testServer.license.enable('feat:advancedPermissions');
const owner = await createOwnerWithApiKey();
const customRole = 'custom:role';
await createRole({ slug: customRole, displayName: 'Custom role', roleType: 'global' });
const payload = [{ email: 'test@test.com', role: customRole }];
/**
* Act
*/
const response = await testServer.publicApiAgentFor(owner).post('/users').send(payload);
/**
* Assert
*/
expect(response.status).toBe(201);
});
});
describe('DELETE /users/:id', () => {
@@ -277,5 +318,32 @@ describe('Users in Public API', () => {
const storedUser = await getUserById(member.id);
expect(storedUser.role.slug).toBe(payload.newRoleName);
});
it('should change a user role to an existing custom role', async () => {
/**
* Arrange
*/
testServer.license.enable('feat:advancedPermissions');
const owner = await createOwnerWithApiKey();
const member = await createMember();
const customRole = 'custom:role';
await createRole({ slug: customRole, displayName: 'Custom role', roleType: 'global' });
const payload = { newRoleName: customRole };
/**
* Act
*/
const response = await testServer
.publicApiAgentFor(owner)
.patch(`/users/${member.id}/role`)
.send(payload);
/**
* Assert
*/
expect(response.status).toBe(204);
const storedUser = await getUserById(member.id);
expect(storedUser.role.slug).toBe(payload.newRoleName);
});
});
});

View File

@@ -3,13 +3,16 @@ import { ProjectRelationRepository, ProjectRepository } from '@n8n/db';
import { Container } from '@n8n/di';
import { PROJECT_OWNER_ROLE_SLUG, type ProjectRole, type Scope } from '@n8n/permissions';
import { createMember } from '../shared/db/users';
import { License } from '@/license';
import { ProjectService } from '@/services/project.service.ee';
import { createRole } from '@test-integration/db/roles';
import { createMember } from '../shared/db/users';
let projectRepository: ProjectRepository;
let projectService: ProjectService;
let projectRelationRepository: ProjectRelationRepository;
let license: License;
beforeAll(async () => {
await testDb.init();
@@ -17,6 +20,7 @@ beforeAll(async () => {
projectRepository = Container.get(ProjectRepository);
projectService = Container.get(ProjectService);
projectRelationRepository = Container.get(ProjectRelationRepository);
license = Container.get(License);
});
afterAll(async () => {
@@ -95,6 +99,36 @@ describe('ProjectService', () => {
expect(relationships).toHaveLength(1);
expect(relationships[0]).toHaveProperty('role.slug', 'project:admin');
});
it('adds a user to a project with a custom role', async () => {
//
// ARRANGE
//
const member = await createMember();
const project = await projectRepository.save(
projectRepository.create({
name: 'Team Project',
type: 'team',
}),
);
const role = await createRole({ slug: 'project:custom', displayName: 'Custom Role' });
//
// ACT
//
await projectService.addUser(project.id, { userId: member.id, role: role.slug });
//
// ASSERT
//
const relationships = await projectRelationRepository.find({
where: { userId: member.id, projectId: project.id },
relations: { role: true },
});
expect(relationships).toHaveLength(1);
expect(relationships[0].role.slug).toBe('project:custom');
});
});
describe('getProjectWithScope', () => {
@@ -240,4 +274,165 @@ describe('ProjectService', () => {
expect(relations).toBeNull();
});
});
describe('addUsersToProject', () => {
it('should add multiple users to a project', async () => {
//
// ARRANGE
//
const members = await Promise.all([createMember(), createMember()]);
const project = await projectRepository.save(
projectRepository.create({
name: 'Team Project',
type: 'team',
}),
);
jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true);
//
// ACT
//
await projectService.addUsersToProject(
project.id,
members.map((member) => ({ userId: member.id, role: 'project:editor' })),
);
//
// ASSERT
//
const relations = await projectRelationRepository.find({
where: { projectId: project.id },
});
expect(relations).toHaveLength(members.length);
});
it('fails to add a user to a project with a non-existing role', async () => {
//
// ARRANGE
//
const member = await createMember();
const project = await projectRepository.save(
projectRepository.create({
name: 'Team Project',
type: 'team',
}),
);
//
// ACT
//
await expect(
projectService.addUsersToProject(project.id, [
{ userId: member.id, role: 'custom:non-existing' },
]),
).rejects.toThrowError('Role custom:non-existing does not exist');
});
});
describe('syncProjectRelations', () => {
it('should synchronize project relations for a user', async () => {
//
// ARRANGE
//
const user = await createMember();
const project = await projectRepository.save(
projectRepository.create({
name: 'Team Project',
type: 'team',
}),
);
jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true);
//
// ACT
//
await projectService.syncProjectRelations(project.id, [
{ userId: user.id, role: 'project:editor' },
]);
//
// ASSERT
//
const relations = await projectRelationRepository.find({
where: { userId: user.id, projectId: project.id },
});
expect(relations).toHaveLength(1);
});
it('should fail to synchronize users with non-existing roles', async () => {
//
// ARRANGE
//
const user = await createMember();
const project = await projectRepository.save(
projectRepository.create({
name: 'Team Project',
type: 'team',
}),
);
jest.spyOn(license, 'isProjectRoleEditorLicensed').mockReturnValue(true);
//
// ACT
//
await expect(
projectService.syncProjectRelations(project.id, [
{ userId: user.id, role: 'project:non-existing' },
]),
).rejects.toThrowError('Role project:non-existing does not exist');
});
});
describe('changeUserRoleInProject', () => {
it('should change user role in project', async () => {
//
// ARRANGE
//
const user = await createMember();
const project = await projectRepository.save(
projectRepository.create({
name: 'Team Project',
type: 'team',
}),
);
//
// ACT
//
await projectService.addUser(project.id, { userId: user.id, role: 'project:viewer' });
await projectService.changeUserRoleInProject(project.id, user.id, 'project:editor');
//
// ASSERT
//
const relations = await projectRelationRepository.find({
where: { userId: user.id, projectId: project.id },
relations: { role: true },
});
expect(relations).toHaveLength(1);
expect(relations[0].role.slug).toBe('project:editor');
});
it('should fail to change user role in project with non-existing role', async () => {
//
// ARRANGE
//
const user = await createMember();
const project = await projectRepository.save(
projectRepository.create({
name: 'Team Project',
type: 'team',
}),
);
//
// ACT
//
await projectService.addUser(project.id, { userId: user.id, role: 'project:viewer' });
await expect(
projectService.changeUserRoleInProject(project.id, user.id, 'project:non-existing'),
).rejects.toThrowError('Role project:non-existing does not exist');
});
});
});

View File

@@ -41,6 +41,7 @@ import { createAdmin, createMember, createOwner, createUser, getUserById } from
import type { SuperAgentTest } from './shared/types';
import * as utils from './shared/utils/';
import { validateUser } from './shared/utils/users';
import { createRole } from '@test-integration/db/roles';
mockInstance(Telemetry);
mockInstance(ExecutionService);
@@ -1573,4 +1574,30 @@ describe('PATCH /users/:id/role', () => {
expect(response.statusCode).toBe(200);
expect(response.body.data).toStrictEqual({ success: true });
});
test('should fail to change to non-existing role', async () => {
const customRole = 'custom:project-role';
await createRole({ slug: customRole, displayName: 'Custom Role', roleType: 'project' });
const response = await ownerAgent.patch(`/users/${member.id}/role`).send({
newRoleName: customRole,
});
expect(response.statusCode).toBe(400);
expect(response.body.message).toBe('Role custom:project-role does not exist');
});
test('should change to existing custom role', async () => {
const customRole = 'custom:role';
await createRole({ slug: customRole, displayName: 'Custom Role', roleType: 'global' });
const response = await ownerAgent.patch(`/users/${member.id}/role`).send({
newRoleName: customRole,
});
expect(response.statusCode).toBe(200);
expect(response.body.data).toStrictEqual({ success: true });
const user = await getUserById(member.id);
expect(user.role.slug).toBe(customRole);
});
});