admin/n8n-enterprise-unlocked
1
0
Fork 0
mirror of https://github.com/Abdulazizzn/n8n-enterprise-unlocked.git synced 2025-12-16 09:36:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): Fix role management controller no-changelog (#19107)

Browse Source
This commit is contained in:
Andreas Fitzek
2025-09-02 19:08:04 +02:00
committed by GitHub
parent 69e25606d8
commit 5be3181f2b
8 changed files with 219 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
packages/@n8n/api-types/src/dto/index.ts
View File

@@ -78,8 +78,8 @@ export {
USERS_LIST_SORT_OPTIONS,
} from './user/users-list-filter.dto';
export type { UpdateRoleDto } from './roles/update-role.dto';
export type { CreateRoleDto } from './roles/create-role.dto';
export { UpdateRoleDto } from './roles/update-role.dto';
export { CreateRoleDto } from './roles/create-role.dto';
export { OidcConfigDto } from './oidc/config.dto';

10
packages/@n8n/api-types/src/dto/roles/__tests__/create-role.dto.test.ts
View File

@@ -1,6 +1,6 @@
import { ALL_SCOPES } from '@n8n/permissions';
import { createRoleDtoSchema } from '../create-role.dto';
import { CreateRoleDto } from '../create-role.dto';
describe('createRoleDtoSchema', () => {
describe('Valid requests', () => {
@@ -120,7 +120,7 @@ describe('createRoleDtoSchema', () => {
},
},
])('should validate $name', ({ request }) => {
const result = createRoleDtoSchema.safeParse(request);
const result = CreateRoleDto.safeParse(request);
expect(result.success).toBe(true);
});
});
@@ -307,7 +307,7 @@ describe('createRoleDtoSchema', () => {
expectedErrorPath: ['scopes', 1],
},
])('should fail validation for $name', ({ request, expectedErrorPath }) => {
const result = createRoleDtoSchema.safeParse(request);
const result = CreateRoleDto.safeParse(request);
expect(result.success).toBe(false);
@@ -328,7 +328,7 @@ describe('createRoleDtoSchema', () => {
scopes: [scope],
};
const result = createRoleDtoSchema.safeParse(request);
const result = CreateRoleDto.safeParse(request);
expect(result.success).toBe(true);
}
});
@@ -354,7 +354,7 @@ describe('createRoleDtoSchema', () => {
scopes: [scope],
};
const result = createRoleDtoSchema.safeParse(request);
const result = CreateRoleDto.safeParse(request);
expect(result.success).toBe(false);
}
});

18
packages/@n8n/api-types/src/dto/roles/__tests__/update-role.dto.test.ts
View File

@@ -1,6 +1,6 @@
import { ALL_SCOPES } from '@n8n/permissions';
import { updateRoleDtoSchema } from '../update-role.dto';
import { UpdateRoleDto } from '../update-role.dto';
describe('updateRoleDtoSchema', () => {
describe('Valid requests', () => {
@@ -144,7 +144,7 @@ describe('updateRoleDtoSchema', () => {
},
},
])('should validate $name', ({ request }) => {
const result = updateRoleDtoSchema.safeParse(request);
const result = UpdateRoleDto.safeParse(request);
expect(result.success).toBe(true);
});
});
@@ -327,7 +327,7 @@ describe('updateRoleDtoSchema', () => {
expectedErrorPath: ['scopes', 1],
},
])('should fail validation for $name', ({ request, expectedErrorPath }) => {
const result = updateRoleDtoSchema.safeParse(request);
const result = UpdateRoleDto.safeParse(request);
expect(result.success).toBe(false);
@@ -346,7 +346,7 @@ describe('updateRoleDtoSchema', () => {
scopes: [scope],
};
const result = updateRoleDtoSchema.safeParse(request);
const result = UpdateRoleDto.safeParse(request);
expect(result.success).toBe(true);
}
});
@@ -370,7 +370,7 @@ describe('updateRoleDtoSchema', () => {
scopes: [scope],
};
const result = updateRoleDtoSchema.safeParse(request);
const result = UpdateRoleDto.safeParse(request);
expect(result.success).toBe(false);
}
});
@@ -380,7 +380,7 @@ describe('updateRoleDtoSchema', () => {
scopes: ['project:read', 'invalid-scope', 'workflow:execute'],
};
const result = updateRoleDtoSchema.safeParse(request);
const result = UpdateRoleDto.safeParse(request);
expect(result.success).toBe(false);
expect(result.error?.issues[0].path).toEqual(['scopes', 1]);
});
@@ -403,7 +403,7 @@ describe('updateRoleDtoSchema', () => {
];
for (const request of validCombinations) {
const result = updateRoleDtoSchema.safeParse(request);
const result = UpdateRoleDto.safeParse(request);
expect(result.success).toBe(true);
}
});
@@ -415,7 +415,7 @@ describe('updateRoleDtoSchema', () => {
scopes: ['*'], // global wildcard
};
const result = updateRoleDtoSchema.safeParse(request);
const result = UpdateRoleDto.safeParse(request);
expect(result.success).toBe(true);
});
});
@@ -450,7 +450,7 @@ describe('updateRoleDtoSchema', () => {
request: { scopes: undefined },
},
])('should handle $name correctly', ({ request, expectedErrorPath }) => {
const result = updateRoleDtoSchema.safeParse(request);
const result = UpdateRoleDto.safeParse(request);
if (expectedErrorPath) {
expect(result.success).toBe(false);

7
packages/@n8n/api-types/src/dto/roles/create-role.dto.ts
View File

@@ -1,11 +1,10 @@
import { scopeSchema } from '@n8n/permissions';
import { z } from 'zod';
import { Z } from 'zod-class';
export const createRoleDtoSchema = z.object({
export class CreateRoleDto extends Z.class({
displayName: z.string().min(2).max(100),
description: z.string().max(500).optional(),
roleType: z.enum(['project']),
scopes: z.array(scopeSchema),
});
export type CreateRoleDto = z.infer<typeof createRoleDtoSchema>;
}) {}

7
packages/@n8n/api-types/src/dto/roles/update-role.dto.ts
View File

@@ -1,10 +1,9 @@
import { scopeSchema } from '@n8n/permissions';
import { z } from 'zod';
import { Z } from 'zod-class';
export const updateRoleDtoSchema = z.object({
export class UpdateRoleDto extends Z.class({
displayName: z.string().min(2).max(100).optional(),
description: z.string().max(500).optional(),
scopes: z.array(scopeSchema).optional(),
});
export type UpdateRoleDto = z.infer<typeof updateRoleDtoSchema>;
}) {}

30
packages/cli/src/controllers/role.controller.ts
View File

@@ -1,5 +1,6 @@
import { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types';
import { LICENSE_FEATURES } from '@n8n/constants';
import { AuthenticatedRequest } from '@n8n/db';
import {
Body,
Delete,
@@ -31,28 +32,45 @@ export class RoleController {
}
@Get('/:slug')
async getRoleBySlug(@Param('slug') slug: string): Promise<RoleDTO> {
async getRoleBySlug(
_req: AuthenticatedRequest,
_res: Response,
@Param('slug') slug: string,
): Promise<RoleDTO> {
return await this.roleService.getRole(slug);
}
@Patch('/:slug')
@GlobalScope('role:manage')
@Licensed(LICENSE_FEATURES.CUSTOM_ROLES)
async updateRole(@Param('slug') slug: string, @Body body: UpdateRoleDto): Promise<RoleDTO> {
return await this.roleService.updateCustomRole(slug, body);
async updateRole(
_req: AuthenticatedRequest,
_res: Response,
@Param('slug') slug: string,
@Body updateRole: UpdateRoleDto,
): Promise<RoleDTO> {
return await this.roleService.updateCustomRole(slug, updateRole);
}
@Delete('/:slug')
@GlobalScope('role:manage')
@Licensed(LICENSE_FEATURES.CUSTOM_ROLES)
async deleteRole(@Param('slug') slug: string): Promise<RoleDTO> {
async deleteRole(
_req: AuthenticatedRequest,
_res: Response,
@Param('slug') slug: string,
): Promise<RoleDTO> {
return await this.roleService.removeCustomRole(slug);
}
@Post('/')
@GlobalScope('role:manage')
@Licensed(LICENSE_FEATURES.CUSTOM_ROLES)
async createRole(@Body body: CreateRoleDto): Promise<RoleDTO> {
return await this.roleService.createCustomRole(body);
async createRole(
_req: AuthenticatedRequest,
_res: Response,
@Body createRole: CreateRoleDto,
): Promise<RoleDTO> {
return await this.roleService.createCustomRole(createRole);
}
}

167
packages/cli/test/integration/controllers/role.controller-db.test.ts Normal file
View File

@@ -0,0 +1,167 @@
import type { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types';
import { testDb } from '@n8n/backend-test-utils';
import { cleanupRolesAndScopes } from '../shared/db/roles';
import { createMember, createOwner } from '../shared/db/users';
import type { SuperAgentTest } from '../shared/types';
import { setupTestServer } from '../shared/utils';
import {
PROJECT_ADMIN_ROLE,
PROJECT_EDITOR_ROLE,
PROJECT_OWNER_ROLE,
PROJECT_VIEWER_ROLE,
} from '@n8n/db';
describe('RoleController - Integration Tests', () => {
const testServer = setupTestServer({ endpointGroups: ['role'] });
let ownerAgent: SuperAgentTest;
let memberAgent: SuperAgentTest;
beforeAll(async () => {
await testDb.init();
const owner = await createOwner();
const member = await createMember();
ownerAgent = testServer.authAgentFor(owner);
memberAgent = testServer.authAgentFor(member);
});
beforeEach(() => {
// Enable CUSTOM_ROLES license for all tests by default
testServer.license.enable('feat:customRoles');
});
afterEach(async () => {
await cleanupRolesAndScopes();
});
afterAll(async () => {
await testDb.terminate();
});
it.each([PROJECT_ADMIN_ROLE, PROJECT_EDITOR_ROLE, PROJECT_VIEWER_ROLE, PROJECT_OWNER_ROLE])(
'should return 200 and the role data for role $slug',
async (role) => {
//
// ACT
//
const response = await memberAgent.get(`/roles/${role.slug}`).expect(200);
//
// ASSERT
//
response.body.data.scopes.sort();
expect(response.body).toEqual({
data: {
slug: role.slug,
displayName: role.displayName,
description: role.description,
systemRole: role.systemRole,
roleType: role.roleType,
scopes: role.scopes.map((scope) => scope.slug).sort(),
licensed: expect.any(Boolean),
},
});
},
);
it('should create a custom role', async () => {
//
// ARRANGE
//
const createRoleDto: CreateRoleDto = {
displayName: 'Custom Project Role',
description: 'A custom role for project management',
roleType: 'project',
scopes: ['workflow:read', 'workflow:create'].sort(),
};
//
// ACT
//
const response = await ownerAgent.post('/roles').send(createRoleDto).expect(200);
//
// ASSERT
//
response.body.data.scopes.sort();
expect(response.body).toEqual({
data: {
...createRoleDto,
slug: expect.any(String),
licensed: expect.any(Boolean),
systemRole: false,
},
});
const availableRole = await memberAgent.get(`/roles/${response.body.data.slug}`).expect(200);
availableRole.body.data.scopes.sort();
expect(availableRole.body).toEqual({
data: {
...createRoleDto,
slug: response.body.data.slug,
licensed: expect.any(Boolean),
systemRole: false,
},
});
});
it('should update a custom role', async () => {
//
// ARRANGE
//
const createRoleDto: CreateRoleDto = {
displayName: 'Custom Project Role',
description: 'A custom role for project management',
roleType: 'project',
scopes: ['workflow:read', 'workflow:create'].sort(),
};
const createResponse = await ownerAgent.post('/roles').send(createRoleDto).expect(200);
expect(createResponse.body?.data?.slug).toBeDefined();
const generatedRoleSlug = createResponse.body.data.slug;
const updateRoleDto: UpdateRoleDto = {
displayName: 'Custom Project Role Updated',
description: 'A custom role for project management - updated',
};
//
// ACT
//
const response = await ownerAgent
.patch(`/roles/${generatedRoleSlug}`)
.send(updateRoleDto)
.expect(200);
//
// ASSERT
//
response.body.data.scopes.sort();
expect(response.body).toEqual({
data: {
...updateRoleDto,
scopes: ['workflow:read', 'workflow:create'].sort(),
slug: generatedRoleSlug,
roleType: 'project',
licensed: expect.any(Boolean),
systemRole: false,
},
});
const availableRole = await memberAgent.get(`/roles/${response.body.data.slug}`).expect(200);
availableRole.body.data.scopes.sort();
expect(availableRole.body).toEqual({
data: {
...updateRoleDto,
scopes: ['workflow:read', 'workflow:create'].sort(),
slug: generatedRoleSlug,
roleType: 'project',
licensed: expect.any(Boolean),
systemRole: false,
},
});
});
});

10
packages/cli/test/integration/controllers/role.controller.test.ts
View File

@@ -1,6 +1,6 @@
import type { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types';
import { mockInstance } from '@n8n/backend-test-utils';
import type { Role } from '@n8n/permissions';
import { type Role } from '@n8n/permissions';
import { RoleService } from '@/services/role.service';
@@ -314,7 +314,7 @@ describe('RoleController', () => {
//
expect(response.body).toEqual({ data: mockCreatedRole });
// Parameter verification skipped - test framework issue
expect(roleService.createCustomRole).toHaveBeenCalledTimes(1);
expect(roleService.createCustomRole).toHaveBeenCalledWith(createRoleDto);
});
it('should create role without description', async () => {
@@ -348,6 +348,7 @@ describe('RoleController', () => {
// ASSERT
//
expect(response.body).toEqual({ data: mockCreatedRole });
expect(roleService.createCustomRole).toHaveBeenCalledWith(createRoleDto);
});
it('should handle service errors gracefully', async () => {
@@ -398,7 +399,7 @@ describe('RoleController', () => {
const updateRoleDto: UpdateRoleDto = {
displayName: 'Updated Role Name',
description: 'Updated description',
scopes: ['workflow:read', 'workflow:edit'],
scopes: ['workflow:read', 'workflow:update'],
};
const mockUpdatedRole: Role = {
@@ -423,7 +424,7 @@ describe('RoleController', () => {
//
expect(response.body).toEqual({ data: mockUpdatedRole });
// Parameter verification skipped - test framework issue
expect(roleService.updateCustomRole).toHaveBeenCalledTimes(1);
expect(roleService.updateCustomRole).toHaveBeenCalledWith(roleSlug, updateRoleDto);
});
it('should update only provided fields', async () => {
@@ -456,6 +457,7 @@ describe('RoleController', () => {
// ASSERT
//
expect(response.body).toEqual({ data: mockUpdatedRole });
expect(roleService.updateCustomRole).toHaveBeenCalledWith(roleSlug, updateRoleDto);
});
it('should handle service errors gracefully', async () => {