feat(core): Rebuild project roles to load from the database (#17909)

This commit is contained in:
Guillaume Jacquart
2025-08-28 11:00:31 +02:00
committed by GitHub
parent ab7998b441
commit f757790394
63 changed files with 546 additions and 305 deletions

View File

@@ -328,7 +328,6 @@ describe('Member', () => {
.post('/api-keys')
.send({ label: 'My API Key', expiresAt: null, scopes: ['workflow:create'] });
console.log(newApiKeyResponse.body);
expect(newApiKeyResponse.statusCode).toBe(200);
expect(newApiKeyResponse.body.data.apiKey).toBeDefined();
expect(newApiKeyResponse.body.data.apiKey).not.toBeNull();

View File

@@ -13,13 +13,9 @@ import {
UserRepository,
} from '@n8n/db';
import { Container } from '@n8n/di';
import { PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions';
import { Not } from '@n8n/typeorm';
import { EventService } from '@/events/event.service';
import { ExternalHooks } from '@/external-hooks';
import { PasswordUtility } from '@/services/password.utility';
import { UserManagementMailer } from '@/user-management/email';
import {
assertReturnedUserProps,
assertStoredUserProps,
@@ -29,6 +25,11 @@ import { createMember, createOwner, createUserShell } from '../../shared/db/user
import * as utils from '../../shared/utils';
import type { UserInvitationResult } from '../../shared/utils/users';
import { EventService } from '@/events/event.service';
import { ExternalHooks } from '@/external-hooks';
import { PasswordUtility } from '@/services/password.utility';
import { UserManagementMailer } from '@/user-management/email';
describe('InvitationController', () => {
const mailer = mockInstance(UserManagementMailer);
const externalHooks = mockInstance(ExternalHooks);
@@ -296,7 +297,7 @@ describe('InvitationController', () => {
const projectRelation = await projectRelationRepository.findOneOrFail({
where: {
userId: storedUser.id,
role: 'project:personalOwner',
role: { slug: PROJECT_OWNER_ROLE_SLUG },
project: {
type: 'personal',
},

View File

@@ -20,12 +20,14 @@ import {
SharedWorkflowRepository,
} from '@n8n/db';
import { Container } from '@n8n/di';
import { getRoleScopes, type GlobalRole, type ProjectRole, type Scope } from '@n8n/permissions';
import {
getRoleScopes,
PROJECT_OWNER_ROLE_SLUG,
type GlobalRole,
type ProjectRole,
type Scope,
} from '@n8n/permissions';
import { EntityNotFoundError } from '@n8n/typeorm';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
import { getWorkflowById } from '@/public-api/v1/handlers/workflows/workflows.service';
import { CacheService } from '@/services/cache/cache.service';
import { createFolder } from '@test-integration/db/folders';
import {
@@ -36,6 +38,10 @@ import {
import { createMember, createOwner, createUser } from './shared/db/users';
import * as utils from './shared/utils/';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
import { getWorkflowById } from '@/public-api/v1/handlers/workflows/workflows.service';
import { CacheService } from '@/services/cache/cache.service';
const testServer = utils.setupTestServer({
endpointGroups: ['project'],
enabledFeatures: [
@@ -193,7 +199,7 @@ describe('GET /projects/my-projects', () => {
[
personalProject1,
{
role: 'project:personalOwner',
role: PROJECT_OWNER_ROLE_SLUG,
scopes: ['project:list', 'project:read', 'credential:create'],
},
],
@@ -270,7 +276,7 @@ describe('GET /projects/my-projects', () => {
[
ownerProject,
{
role: 'project:personalOwner',
role: PROJECT_OWNER_ROLE_SLUG,
scopes: [
'project:list',
'project:create',
@@ -596,15 +602,15 @@ describe('PATCH /projects/:projectId', () => {
expect(tp1Relations.find((p) => p.userId === testUser1.id)).not.toBeUndefined();
expect(tp1Relations.find((p) => p.userId === testUser2.id)).toBeUndefined();
expect(tp1Relations.find((p) => p.userId === testUser1.id)?.role).toBe('project:admin');
expect(tp1Relations.find((p) => p.userId === testUser3.id)?.role).toBe('project:editor');
expect(tp1Relations.find((p) => p.userId === ownerUser.id)?.role).toBe('project:viewer');
expect(tp1Relations.find((p) => p.userId === testUser1.id)?.role.slug).toBe('project:admin');
expect(tp1Relations.find((p) => p.userId === testUser3.id)?.role.slug).toBe('project:editor');
expect(tp1Relations.find((p) => p.userId === ownerUser.id)?.role.slug).toBe('project:viewer');
// Check we haven't modified the other team project
expect(tp2Relations.find((p) => p.userId === testUser2.id)).not.toBeUndefined();
expect(tp2Relations.find((p) => p.userId === testUser1.id)).toBeUndefined();
expect(tp2Relations.find((p) => p.userId === testUser2.id)?.role).toBe('project:editor');
expect(tp2Relations.find((p) => p.userId === ownerUser.id)?.role).toBe('project:editor');
expect(tp2Relations.find((p) => p.userId === testUser2.id)?.role.slug).toBe('project:editor');
expect(tp2Relations.find((p) => p.userId === ownerUser.id)?.role.slug).toBe('project:editor');
});
test.each([['project:viewer'], ['project:editor']] as const)(
@@ -656,8 +662,14 @@ describe('PATCH /projects/:projectId', () => {
expect(tp1Relations.length).toBe(2);
expect(tp1Relations).toMatchObject(
expect.arrayContaining([
expect.objectContaining({ userId: actor.id, role }),
expect.objectContaining({ userId: projectEditor.id, role: 'project:editor' }),
expect.objectContaining({
userId: actor.id,
role: expect.objectContaining({ slug: role }),
}),
expect.objectContaining({
userId: projectEditor.id,
role: expect.objectContaining({ slug: 'project:editor' }),
}),
]),
);
},
@@ -692,7 +704,10 @@ describe('PATCH /projects/:projectId', () => {
expect(tpRelations.length).toBe(1);
expect(tpRelations).toMatchObject(
expect.arrayContaining([
expect.objectContaining({ userId: projectAdmin.id, role: 'project:admin' }),
expect.objectContaining({
userId: projectAdmin.id,
role: expect.objectContaining({ slug: 'project:admin' }),
}),
]),
);
},
@@ -730,9 +745,9 @@ describe('PATCH /projects/:projectId', () => {
expect(tpRelations.find((p) => p.userId === testUser1.id)).not.toBeUndefined();
expect(tpRelations.find((p) => p.userId === testUser2.id)).not.toBeUndefined();
expect(tpRelations.find((p) => p.userId === testUser1.id)?.role).toBe('project:admin');
expect(tpRelations.find((p) => p.userId === testUser2.id)?.role).toBe('project:admin');
expect(tpRelations.find((p) => p.userId === testUser3.id)?.role).toBe('project:admin');
expect(tpRelations.find((p) => p.userId === testUser1.id)?.role?.slug).toBe('project:admin');
expect(tpRelations.find((p) => p.userId === testUser2.id)?.role?.slug).toBe('project:admin');
expect(tpRelations.find((p) => p.userId === testUser3.id)?.role?.slug).toBe('project:admin');
});
test("should edit a relation of a project when changing a user's role to an licensed role but unlicensed roles are present", async () => {
@@ -768,9 +783,9 @@ describe('PATCH /projects/:projectId', () => {
expect(tpRelations.find((p) => p.userId === testUser1.id)).not.toBeUndefined();
expect(tpRelations.find((p) => p.userId === testUser2.id)).not.toBeUndefined();
expect(tpRelations.find((p) => p.userId === testUser3.id)).not.toBeUndefined();
expect(tpRelations.find((p) => p.userId === testUser1.id)?.role).toBe('project:viewer');
expect(tpRelations.find((p) => p.userId === testUser2.id)?.role).toBe('project:admin');
expect(tpRelations.find((p) => p.userId === testUser3.id)?.role).toBe('project:admin');
expect(tpRelations.find((p) => p.userId === testUser1.id)?.role?.slug).toBe('project:viewer');
expect(tpRelations.find((p) => p.userId === testUser2.id)?.role?.slug).toBe('project:admin');
expect(tpRelations.find((p) => p.userId === testUser3.id)?.role?.slug).toBe('project:admin');
});
test('should not add or remove users from a personal project', async () => {
@@ -782,7 +797,7 @@ describe('PATCH /projects/:projectId', () => {
const resp = await memberAgent.patch(`/projects/${personalProject.id}`).send({
relations: [
{ userId: testUser1.id, role: 'project:personalOwner' },
{ userId: testUser1.id, role: PROJECT_OWNER_ROLE_SLUG },
{ userId: testUser2.id, role: 'project:admin' },
] as Array<{
userId: string;

View File

@@ -61,7 +61,7 @@ describe('ProjectService', () => {
expect(relations[0]).toMatchObject({
projectId: project.id,
userId: user.id,
role: 'project:admin',
role: { slug: 'project:admin' },
});
});
@@ -82,7 +82,7 @@ describe('ProjectService', () => {
expect(relations[0]).toMatchObject({
projectId: project.id,
userId: user.id,
role: 'project:editor',
role: { slug: 'project:editor' },
});
});
});
@@ -103,7 +103,7 @@ describe('ProjectService', () => {
expect(relations[0]).toMatchObject({
projectId: project.id,
userId: user.id,
role: 'project:admin',
role: { slug: 'project:admin' },
});
});
});

View File

@@ -564,23 +564,20 @@ describe('Projects in Public API', () => {
expect(projectAfter.length).toEqual(3);
const adminRelation = projectAfter.find(
(relation) => relation.userId === owner.id && relation.role === 'project:admin',
);
expect(adminRelation).toEqual(
expect.objectContaining({ userId: owner.id, role: 'project:admin' }),
(relation) => relation.userId === owner.id && relation.role.slug === 'project:admin',
);
expect(adminRelation!.userId).toBe(owner.id);
expect(adminRelation!.role.slug).toBe('project:admin');
const viewerRelation = projectAfter.find(
(relation) => relation.userId === member.id && relation.role === 'project:viewer',
);
expect(viewerRelation).toEqual(
expect.objectContaining({ userId: member.id, role: 'project:viewer' }),
(relation) => relation.userId === member.id && relation.role.slug === 'project:viewer',
);
expect(viewerRelation!.userId).toBe(member.id);
expect(viewerRelation!.role.slug).toBe('project:viewer');
const editorRelation = projectAfter.find(
(relation) => relation.userId === member2.id && relation.role === 'project:editor',
);
expect(editorRelation).toEqual(
expect.objectContaining({ userId: member2.id, role: 'project:editor' }),
(relation) => relation.userId === member2.id && relation.role.slug === 'project:editor',
);
expect(editorRelation!.userId).toBe(member2.id);
expect(editorRelation!.role.slug).toBe('project:editor');
});
it('should reject with 400 if license does not include user role', async () => {
@@ -797,8 +794,12 @@ describe('Projects in Public API', () => {
expect(response.status).toBe(204);
expect(projectBefore.length).toEqual(2);
expect(projectBefore.find((p) => p.role === 'project:admin')?.userId).toEqual(owner.id);
expect(projectBefore.find((p) => p.role === 'project:viewer')?.userId).toEqual(member.id);
expect(projectBefore.find((p) => p.role.slug === 'project:admin')?.userId).toEqual(
owner.id,
);
expect(projectBefore.find((p) => p.role.slug === 'project:viewer')?.userId).toEqual(
member.id,
);
expect(projectAfter.length).toEqual(1);
expect(projectAfter[0].userId).toEqual(owner.id);

View File

@@ -1,4 +1,4 @@
import { getRoleScopes } from '@n8n/permissions';
import { getRoleScopes, PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions';
import type {
GlobalRole,
ProjectRole,
@@ -76,8 +76,8 @@ beforeAll(async () => {
expectedProjectRoles = [
{
name: 'Project Owner',
role: 'project:personalOwner',
scopes: getRoleScopes('project:personalOwner'),
role: PROJECT_OWNER_ROLE_SLUG,
scopes: getRoleScopes(PROJECT_OWNER_ROLE_SLUG),
licensed: true,
description: 'Project Owner',
},

View File

@@ -1,12 +1,12 @@
import { testDb } from '@n8n/backend-test-utils';
import { ProjectRelationRepository, ProjectRepository } from '@n8n/db';
import { Container } from '@n8n/di';
import type { ProjectRole, Scope } from '@n8n/permissions';
import { ProjectService } from '@/services/project.service.ee';
import { PROJECT_OWNER_ROLE_SLUG, type ProjectRole, type Scope } from '@n8n/permissions';
import { createMember } from '../shared/db/users';
import { ProjectService } from '@/services/project.service.ee';
let projectRepository: ProjectRepository;
let projectService: ProjectService;
let projectRelationRepository: ProjectRelationRepository;
@@ -33,7 +33,7 @@ describe('ProjectService', () => {
'project:viewer',
'project:admin',
'project:editor',
'project:personalOwner',
PROJECT_OWNER_ROLE_SLUG,
] as ProjectRole[])(
'creates a relation between the user and the project using the role %s',
async (role) => {
@@ -57,7 +57,7 @@ describe('ProjectService', () => {
// ASSERT
//
await projectRelationRepository.findOneOrFail({
where: { userId: member.id, projectId: project.id, role },
where: { userId: member.id, projectId: project.id, role: { slug: role } },
});
},
);
@@ -76,7 +76,7 @@ describe('ProjectService', () => {
await projectService.addUser(project.id, { userId: member.id, role: 'project:viewer' });
await projectRelationRepository.findOneOrFail({
where: { userId: member.id, projectId: project.id, role: 'project:viewer' },
where: { userId: member.id, projectId: project.id, role: { slug: 'project:viewer' } },
});
//
@@ -89,10 +89,11 @@ describe('ProjectService', () => {
//
const relationships = await projectRelationRepository.find({
where: { userId: member.id, projectId: project.id },
relations: { role: true },
});
expect(relationships).toHaveLength(1);
expect(relationships[0]).toHaveProperty('role', 'project:admin');
expect(relationships[0]).toHaveProperty('role.slug', 'project:admin');
});
});
@@ -202,7 +203,7 @@ describe('ProjectService', () => {
describe('deleteUserFromProject', () => {
it('should not allow project owner to be removed from the project', async () => {
const role = 'project:personalOwner';
const role = PROJECT_OWNER_ROLE_SLUG;
const user = await createMember();
const project = await projectRepository.save(
@@ -233,7 +234,7 @@ describe('ProjectService', () => {
await projectService.deleteUserFromProject(project.id, user.id);
const relations = await projectRelationRepository.findOne({
where: { userId: user.id, projectId: project.id, role },
where: { userId: user.id, projectId: project.id, role: { slug: role } },
});
expect(relations).toBeNull();

View File

@@ -1092,7 +1092,7 @@ describe('DELETE /users/:id', () => {
id: teamProject.id,
projectRelations: {
userId: transferee.id,
role: 'project:editor',
role: { slug: 'project:editor' },
},
}),
).resolves.not.toBeNull(),