feat(API): Add endpoint to delete a custom role (#36417)

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Sam Wooler 2026-08-18 09:10:10 +00:00 committed by GitHub
parent 76c6721129
commit 1cd8c91a20
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 647 additions and 130 deletions

View File

@ -41,21 +41,6 @@ describe('RoleController', () => {
});
describe('deleteRole', () => {
it('should emit custom-role-deleted', async () => {
const request = managerRequest();
roleService.getRole.mockResolvedValue({ roleType: 'project' } as Role);
roleService.removeCustomRole.mockResolvedValue({
slug: 'custom-editor',
} as Role);
await controller.deleteRole(request, mock(), 'custom-editor', mock());
expect(eventService.emit).toHaveBeenCalledWith('custom-role-deleted', {
userId: '123',
roleSlug: 'custom-editor',
});
});
it('should pass the reassignment role through to the service for an entitled caller', async () => {
const request = mock<AuthenticatedRequest>({
user: {
@ -76,10 +61,11 @@ describe('RoleController', () => {
reassignRoleSlug: 'global:member',
});
expect(roleService.removeCustomRole).toHaveBeenCalledWith(
'global:custom-editor',
'global:member',
);
expect(roleService.removeCustomRole).toHaveBeenCalledWith({
slug: 'global:custom-editor',
reassignRoleSlug: 'global:member',
userId: '123',
});
});
it('should ignore the reassignment role when the caller lacks user:changeRole', async () => {
@ -99,10 +85,11 @@ describe('RoleController', () => {
reassignRoleSlug: 'global:member',
});
expect(roleService.removeCustomRole).toHaveBeenCalledWith(
'global:custom-editor',
undefined,
);
expect(roleService.removeCustomRole).toHaveBeenCalledWith({
slug: 'global:custom-editor',
reassignRoleSlug: undefined,
userId: '123',
});
});
it('should ignore the reassignment role when the caller holds the role being deleted', async () => {
@ -125,10 +112,11 @@ describe('RoleController', () => {
reassignRoleSlug: 'global:member',
});
expect(roleService.removeCustomRole).toHaveBeenCalledWith(
'global:custom-editor',
undefined,
);
expect(roleService.removeCustomRole).toHaveBeenCalledWith({
slug: 'global:custom-editor',
reassignRoleSlug: undefined,
userId: '123',
});
});
it('should ignore the reassignment role for project roles', async () => {
@ -151,10 +139,11 @@ describe('RoleController', () => {
reassignRoleSlug: 'project:admin',
});
expect(roleService.removeCustomRole).toHaveBeenCalledWith(
'project:custom-editor',
undefined,
);
expect(roleService.removeCustomRole).toHaveBeenCalledWith({
slug: 'project:custom-editor',
reassignRoleSlug: undefined,
userId: '123',
});
});
});
});

View File

@ -14,7 +14,7 @@ import type {
RoleProjectMembersResponse,
} from '@n8n/api-types';
import { LICENSE_FEATURES } from '@n8n/constants';
import { AuthenticatedRequest, User } from '@n8n/db';
import { AuthenticatedRequest } from '@n8n/db';
import {
Body,
Delete,
@ -27,10 +27,10 @@ import {
Query,
RestController,
} from '@n8n/decorators';
import { hasGlobalScope, Role as RoleDTO } from '@n8n/permissions';
import { Role as RoleDTO } from '@n8n/permissions';
import { EventService } from '@/events/event.service';
import { assertCanManageRoleType } from '@/services/role-authorization';
import { assertCanManageRoleType, canReassignUsers } from '@/services/role-authorization';
import { RoleService } from '@/services/role.service';
@RestController('/roles')
@ -40,23 +40,6 @@ export class RoleController {
private readonly eventService: EventService,
) {}
/**
* Reassigning a deleted role's users is effectively a bulk instance-role change,
* so it is only honored for callers entitled to change users' instance roles
* (the same scopes the dedicated role-change endpoint requires), never for
* project roles, and never for the caller's own role. When it is not honored the
* reassignment target is ignored, so a role that still has users cannot be
* deleted.
*/
private canReassignUsers(user: User, role: RoleDTO): boolean {
return (
role.roleType === 'global' &&
user.role.slug !== role.slug &&
hasGlobalScope(user, 'role:manage') &&
hasGlobalScope(user, 'user:changeRole')
);
}
@Get('/')
async getAllRoles(
_req: AuthenticatedRequest,
@ -80,7 +63,10 @@ export class RoleController {
@Param('projectId') projectId: string,
): Promise<RoleProjectMembersResponse> {
const role = await this.roleService.getRole(slug);
assertCanManageRoleType(req.user, role.roleType);
assertCanManageRoleType({
roleType: role.roleType,
user: req.user,
});
const result = await this.roleService.getRoleProjectMembers(slug, projectId);
return RoleProjectMembersResponseDto.parse(result);
}
@ -92,7 +78,10 @@ export class RoleController {
@Param('slug') slug: string,
): Promise<RoleAssignmentsResponse> {
const role = await this.roleService.getRole(slug);
assertCanManageRoleType(req.user, role.roleType);
assertCanManageRoleType({
roleType: role.roleType,
user: req.user,
});
const result = await this.roleService.getRoleAssignments(slug);
return RoleAssignmentsResponseDto.parse(result);
}
@ -127,7 +116,10 @@ export class RoleController {
@Body updateRole: UpdateRoleDto,
): Promise<RoleDTO> {
const role = await this.roleService.getRole(slug);
assertCanManageRoleType(req.user, role.roleType);
assertCanManageRoleType({
roleType: role.roleType,
user: req.user,
});
return await this.roleService.updateCustomRole({
slug,
newRole: updateRole,
@ -144,16 +136,18 @@ export class RoleController {
@Query query: RoleDeleteQueryDto,
): Promise<RoleDTO> {
const role = await this.roleService.getRole(slug);
assertCanManageRoleType(req.user, role.roleType);
const reassignRoleSlug = this.canReassignUsers(req.user, role)
assertCanManageRoleType({
roleType: role.roleType,
user: req.user,
});
const reassignRoleSlug = canReassignUsers({ role, user: req.user })
? query.reassignRoleSlug
: undefined;
const result = await this.roleService.removeCustomRole(slug, reassignRoleSlug);
this.eventService.emit('custom-role-deleted', {
return await this.roleService.removeCustomRole({
slug,
reassignRoleSlug,
userId: req.user.id,
roleSlug: result.slug,
});
return result;
}
@Post('/')
@ -163,7 +157,10 @@ export class RoleController {
_res: Response,
@Body createRole: CreateRoleDto,
): Promise<RoleDTO> {
assertCanManageRoleType(req.user, createRole.roleType);
assertCanManageRoleType({
roleType: createRole.roleType,
user: req.user,
});
const result = await this.roleService.createCustomRole(createRole);
this.eventService.emit('custom-role-created', {
userId: req.user.id,

View File

@ -1,5 +1,6 @@
import {
CreateRoleDto,
RoleDeleteQueryDto,
RoleGetPublicDto,
RoleListPublicDto,
RoleListQueryPublicDto,
@ -16,6 +17,7 @@ import {
ApiSummary,
ApiTags,
Body,
Delete,
Get,
Licensed,
Param,
@ -29,7 +31,7 @@ import type { Response } from 'express';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { EventService } from '@/events/event.service';
import { assertCanManageRoleType } from '@/services/role-authorization';
import { assertCanManageRoleType, canReassignUsers } from '@/services/role-authorization';
import { RoleService } from '@/services/role.service';
type PublicRoleNamespace = Extract<RoleNamespace, 'global' | 'project'>;
@ -131,7 +133,11 @@ export class RolesPublicController {
_res: Response,
@Body createRole: CreateRoleDto,
): Promise<RolePublicDto> {
assertCanManageRoleType(req.user, createRole.roleType);
assertCanManageRoleType({
apiKeyScopes: req.tokenGrant?.apiKeyScopes ?? [],
roleType: createRole.roleType,
user: req.user,
});
const role = await this.roleService.createCustomRole(createRole);
@ -164,7 +170,12 @@ export class RolesPublicController {
if (!isPublicRole(role)) {
throw new NotFoundError('Role not found');
}
assertCanManageRoleType(req.user, role.roleType);
assertCanManageRoleType({
apiKeyScopes: req.tokenGrant?.apiKeyScopes ?? [],
roleType: role.roleType,
user: req.user,
});
const result = await this.roleService.updateCustomRole({
slug,
@ -174,4 +185,46 @@ export class RolesPublicController {
return toRolePublicDto({ ...result, roleType: role.roleType });
}
@Delete('/:slug')
@ApiKeyScope({ anyOf: ['role:manage', 'role:manageProject'] })
@Licensed(LICENSE_FEATURES.CUSTOM_ROLES)
@ApiSummary('Delete a custom role')
@ApiDescription(
'Deletes a custom role. System roles cannot be deleted. A role with users assigned cannot be deleted unless `reassignRoleSlug` is set to move those users to another role first.',
)
@ApiTags(['Role'])
@ApiResponse(200, RolePublicDto)
@ApiErrorResponse(404)
async deleteRole(
req: AuthenticatedRequest,
_res: Response,
@Param('slug') slug: string,
@Query query: RoleDeleteQueryDto,
): Promise<RolePublicDto> {
const role = await this.roleService.getRole(slug);
if (!isPublicRole(role)) {
throw new NotFoundError('Role not found');
}
const apiKeyScopes = req.tokenGrant?.apiKeyScopes ?? [];
assertCanManageRoleType({
apiKeyScopes,
roleType: role.roleType,
user: req.user,
});
const reassignRoleSlug = canReassignUsers({ apiKeyScopes, role, user: req.user })
? query.reassignRoleSlug
: undefined;
const result = await this.roleService.removeCustomRole({
slug,
reassignRoleSlug,
userId: req.user.id,
});
return toRolePublicDto({ ...result, roleType: role.roleType });
}
}

View File

@ -0,0 +1,42 @@
operationId: deleteRole
tags:
- Role
summary: Delete a custom role
description: Deletes a custom role. System roles cannot be deleted. A role with users assigned cannot be deleted unless `reassignRoleSlug` is set to move those users to another role first.
x-required-scope: role:manage,role:manageProject
x-eov-operation-id: unreachable
x-eov-operation-handler: v1/handlers/decorator-routed.handler
x-decorator-routed: true
parameters:
- schema:
type: string
required: true
name: slug
in: path
- schema:
anyOf:
- type: string
enum:
- global:admin
- global:member
- global:chatUser
- type: string
minLength: 1
required: false
name: reassignRoleSlug
in: query
responses:
'200':
description: Operation successful.
content:
application/json:
schema:
$ref: ../../../../shared/spec/schemas/rolePublicDto.generated.yml
'400':
$ref: ../../../../shared/spec/responses/badRequest.yml
'401':
$ref: ../../../../shared/spec/responses/unauthorized.yml
'403':
$ref: ../../../../shared/spec/responses/forbidden.yml
'404':
$ref: ../../../../shared/spec/responses/notFound.yml

View File

@ -16,6 +16,8 @@ paths:
$ref: ./handlers/roles/spec/paths/getRole.generated.yml
put:
$ref: ./handlers/roles/spec/paths/updateRole.generated.yml
delete:
$ref: ./handlers/roles/spec/paths/deleteRole.generated.yml
/tags:
get:
$ref: ./handlers/tags/spec/paths/getTags.generated.yml

View File

@ -9,24 +9,68 @@ describe('assertCanManageRoleType', () => {
const userWithScopes = (...scopes: Scope[]) =>
mock<User>({ role: { scopes: scopes.map((slug) => ({ slug })) } });
it('allows any role type for a user with role:manage', () => {
const user = userWithScopes('role:manage');
describe('without apiKeyScopes (Internal API - user scopes only)', () => {
it('allows any role type for a user with role:manage', () => {
const user = userWithScopes('role:manage');
expect(() => assertCanManageRoleType(user, 'global')).not.toThrow();
expect(() => assertCanManageRoleType(user, 'project')).not.toThrow();
expect(() => assertCanManageRoleType({ user, roleType: 'global' })).not.toThrow();
expect(() => assertCanManageRoleType({ user, roleType: 'project' })).not.toThrow();
});
it('allows only project roles for a user with role:manageProject', () => {
const user = userWithScopes('role:manageProject');
expect(() => assertCanManageRoleType({ user, roleType: 'project' })).not.toThrow();
expect(() => assertCanManageRoleType({ user, roleType: 'global' })).toThrow(ForbiddenError);
});
it('rejects a user with neither scope', () => {
const user = userWithScopes('role:read');
expect(() => assertCanManageRoleType({ user, roleType: 'project' })).toThrow(ForbiddenError);
expect(() => assertCanManageRoleType({ user, roleType: 'global' })).toThrow(ForbiddenError);
});
});
it('allows only project roles for a user with role:manageProject', () => {
const user = userWithScopes('role:manageProject');
expect(() => assertCanManageRoleType(user, 'project')).not.toThrow();
expect(() => assertCanManageRoleType(user, 'global')).toThrow(ForbiddenError);
});
it('rejects a user with neither scope', () => {
describe('with apiKeyScopes (Public API - decided entirely by the API key)', () => {
const user = userWithScopes('role:read');
expect(() => assertCanManageRoleType(user, 'project')).toThrow(ForbiddenError);
expect(() => assertCanManageRoleType(user, 'global')).toThrow(ForbiddenError);
it('allows a global role when the API key holds role:manage', () => {
expect(() =>
assertCanManageRoleType({ user, roleType: 'global', apiKeyScopes: ['role:manage'] }),
).not.toThrow();
});
it('allows a project role when the API key holds role:manageProject', () => {
expect(() =>
assertCanManageRoleType({
user,
roleType: 'project',
apiKeyScopes: ['role:manageProject'],
}),
).not.toThrow();
});
it('rejects a global role when the API key is scoped down to role:manageProject only', () => {
expect(() =>
assertCanManageRoleType({
user,
roleType: 'global',
apiKeyScopes: ['role:manageProject'],
}),
).toThrow(ForbiddenError);
});
it('rejects when the API key has no scopes at all', () => {
expect(() =>
assertCanManageRoleType({ user, roleType: 'project', apiKeyScopes: [] }),
).toThrow(ForbiddenError);
});
it('rejects when the API key holds neither role-management scope', () => {
expect(() =>
assertCanManageRoleType({ user, roleType: 'project', apiKeyScopes: ['role:read'] }),
).toThrow(ForbiddenError);
});
});
});

View File

@ -1,15 +1,50 @@
import type { User } from '@n8n/db';
import { hasGlobalScope, type RoleNamespace } from '@n8n/permissions';
import { getAuthPrincipalScopes, type Role as RoleDTO, type RoleNamespace } from '@n8n/permissions';
import { RESPONSE_ERROR_MESSAGES } from '@/constants';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
/**
* Managing a role requires `role:manage`; `role:manageProject` grants it for
* project roles only. Shared by the internal and public API controllers.
* Whether a caller can manage a role requires `role:manage`; `role:manageProject` grants it for
* project roles only.
*/
export function assertCanManageRoleType(user: User, roleType: RoleNamespace): void {
if (hasGlobalScope(user, 'role:manage')) return;
if (roleType === 'project' && hasGlobalScope(user, 'role:manageProject')) return;
throw new ForbiddenError(RESPONSE_ERROR_MESSAGES.MISSING_SCOPE);
export function assertCanManageRoleType({
apiKeyScopes,
roleType,
user,
}: {
apiKeyScopes?: readonly string[];
roleType: RoleNamespace;
user: User;
}): void {
const canManage = (scopes: readonly string[]) =>
scopes.includes('role:manage') ||
(roleType === 'project' && scopes.includes('role:manageProject'));
const scopes = apiKeyScopes ?? getAuthPrincipalScopes(user);
if (!canManage(scopes)) {
throw new ForbiddenError(RESPONSE_ERROR_MESSAGES.MISSING_SCOPE);
}
}
/**
* Whether the caller may reassign a global role's users to another role before deleting it.
*/
export function canReassignUsers({
apiKeyScopes,
role,
user,
}: {
apiKeyScopes?: readonly string[];
role: RoleDTO;
user: User;
}): boolean {
if (role.roleType !== 'global' || user.role.slug === role.slug) {
return false;
}
const scopes = apiKeyScopes ?? getAuthPrincipalScopes(user);
return scopes.includes('role:manage') && scopes.includes('user:changeRole');
}

View File

@ -141,7 +141,15 @@ export class RoleService {
return { members };
}
async removeCustomRole(slug: string, reassignRoleSlug?: string) {
async removeCustomRole({
slug,
reassignRoleSlug,
userId,
}: {
slug: string;
reassignRoleSlug?: string;
userId: string;
}) {
const role = await this.roleRepository.findBySlug(slug);
if (!role) {
throw new NotFoundError('Role not found');
@ -172,7 +180,14 @@ export class RoleService {
// Invalidate cache after role deletion
await this.roleCacheService.invalidateCache();
return this.dbRoleToRoleDTO(role);
const result = this.dbRoleToRoleDTO(role);
this.eventService.emit('custom-role-deleted', {
userId,
roleSlug: result.slug,
});
return result;
}
private async reassignUsersAndRemoveRole(role: Role, reassignRoleSlug: string) {

View File

@ -15,12 +15,14 @@ describe('RoleController', () => {
const testServer = setupTestServer({ endpointGroups: ['role'] });
let ownerAgent: SuperAgentTest;
let memberAgent: SuperAgentTest;
let ownerId: string;
beforeAll(async () => {
const owner = await createOwner();
const member = await createMember();
ownerAgent = testServer.authAgentFor(owner);
memberAgent = testServer.authAgentFor(member);
ownerId = owner.id;
});
beforeEach(() => {
@ -1160,7 +1162,11 @@ describe('RoleController', () => {
//
// ASSERT
//
expect(roleService.removeCustomRole).toHaveBeenCalledWith(roleSlug, 'global:member');
expect(roleService.removeCustomRole).toHaveBeenCalledWith({
slug: roleSlug,
reassignRoleSlug: 'global:member',
userId: ownerId,
});
});
it('should reject reassignment to the owner role', async () => {
@ -1197,7 +1203,11 @@ describe('RoleController', () => {
//
// ASSERT
//
expect(roleService.removeCustomRole).toHaveBeenCalledWith(roleSlug, undefined);
expect(roleService.removeCustomRole).toHaveBeenCalledWith({
slug: roleSlug,
reassignRoleSlug: undefined,
userId: ownerId,
});
});
it('should handle service errors gracefully', async () => {

View File

@ -3,7 +3,12 @@ import { RoleRepository, type User } from '@n8n/db';
import { Container } from '@n8n/di';
import { createCustomRoleWithScopeSlugs, createRole } from '@test-integration/db/roles';
import { addApiKey, createOwnerWithApiKey, createUser } from '@test-integration/db/users';
import {
addApiKey,
createOwnerWithApiKey,
createUser,
getUserById,
} from '@test-integration/db/users';
import { setupTestServer } from '@test-integration/utils';
describe('Roles in Public API', () => {
@ -24,6 +29,23 @@ describe('Roles in Public API', () => {
return testServer.publicApiAgentFor(user);
};
const createGlobalRole = async (
displayName: string,
scopes: string[] = ['user:read'],
): Promise<{
slug: string;
displayName: string;
description: string | null;
scopes: string[];
}> => {
const response = await testServer
.publicApiAgentFor(owner)
.post('/roles')
.send({ displayName, roleType: 'global', scopes });
expect(response.status).toBe(201);
return response.body;
};
beforeAll(async () => {
await testDb.init();
});
@ -320,6 +342,24 @@ describe('Roles in Public API', () => {
expect(response.status).toBe(403);
});
it('forbids creating a global role when the owner API key is scoped down to role:manageProject only (403)', async () => {
const scopedOwner = await createOwnerWithApiKey({ scopes: ['role:manageProject'] });
const response = await testServer
.publicApiAgentFor(scopedOwner)
.post('/roles')
.send({
displayName: 'PA global role via scoped key',
roleType: 'global',
scopes: ['user:read'],
});
expect(response.status).toBe(403);
expect(response.body.message).toEqual(
'User is missing a scope required to perform this action',
);
});
it('rejects an unknown scope slug with 400', async () => {
const response = await testServer
.publicApiAgentFor(owner)
@ -416,18 +456,6 @@ describe('Roles in Public API', () => {
scopes: string[];
};
const createGlobalRole = async (
displayName: string,
scopes: string[] = ['user:read'],
): Promise<CreatedRole> => {
const response = await testServer
.publicApiAgentFor(owner)
.post('/roles')
.send({ displayName, roleType: 'global', scopes });
expect(response.status).toBe(201);
return response.body;
};
const fullBody = (overrides: Partial<CreatedRole> = {}) => ({
displayName: 'PA updated role',
description: null,
@ -566,6 +594,21 @@ describe('Roles in Public API', () => {
expect(response.status).toBe(403);
});
it('forbids updating a global role when the owner API key is scoped down to role:manageProject only (403)', async () => {
const role = await createGlobalRole('PA global role for scoped-key put test');
const scopedOwner = await createOwnerWithApiKey({ scopes: ['role:manageProject'] });
const response = await testServer
.publicApiAgentFor(scopedOwner)
.put(`/roles/${role.slug}`)
.send(fullBody({ displayName: 'Should not work via scoped key' }));
expect(response.status).toBe(403);
expect(response.body.message).toEqual(
'User is missing a scope required to perform this action',
);
});
it('returns 404 for an unknown slug', async () => {
const response = await testServer
.publicApiAgentFor(owner)
@ -693,4 +736,247 @@ describe('Roles in Public API', () => {
testServer.license.enable('feat:customRoles');
});
});
describe('DELETE /roles/{slug}', () => {
it('deletes a custom global role, returns its full public shape, and the slug becomes unreachable', async () => {
const role = await createGlobalRole('PA delete all fields', ['user:read', 'user:list']);
const response = await testServer.publicApiAgentFor(owner).delete(`/roles/${role.slug}`);
expect(response.status).toBe(200);
// Full-shape assertion also proves no extra fields leak (e.g. licensed, usedByUsers).
expect(response.body).toEqual({
slug: role.slug,
displayName: 'PA delete all fields',
description: null,
systemRole: false,
roleType: 'global',
scopes: expect.arrayContaining(['user:read', 'user:list']),
createdAt: expect.any(String),
updatedAt: expect.any(String),
});
const getResponse = await testServer.publicApiAgentFor(owner).get(`/roles/${role.slug}`);
expect(getResponse.status).toBe(404);
});
it('deletes a custom project role', async () => {
const created = await testServer
.publicApiAgentFor(owner)
.post('/roles')
.send({
displayName: 'PA delete project role',
roleType: 'project',
scopes: ['workflow:read'],
});
expect(created.status).toBe(201);
const response = await testServer
.publicApiAgentFor(owner)
.delete(`/roles/${created.body.slug}`);
expect(response.status).toBe(200);
expect(response.body.roleType).toBe('project');
});
it('lets a role:manageProject key delete a project role (200)', async () => {
const agent = await makeManageProjectUserAgent();
const created = await agent.post('/roles').send({
displayName: 'MP delete project role',
roleType: 'project',
scopes: ['workflow:read'],
});
expect(created.status).toBe(201);
const response = await agent.delete(`/roles/${created.body.slug}`);
expect(response.status).toBe(200);
expect(response.body.slug).toBe(created.body.slug);
});
it('forbids a role:manageProject key from deleting a global role (403)', async () => {
const role = await createGlobalRole('PA global role for MP delete test');
const agent = await makeManageProjectUserAgent();
const response = await agent.delete(`/roles/${role.slug}`);
expect(response.status).toBe(403);
});
it('forbids deleting a global role when the owner API key is scoped down to role:manageProject only (403)', async () => {
const role = await createGlobalRole('PA global role for scoped-key delete test');
const scopedOwner = await createOwnerWithApiKey({ scopes: ['role:manageProject'] });
const response = await testServer
.publicApiAgentFor(scopedOwner)
.delete(`/roles/${role.slug}`);
expect(response.status).toBe(403);
expect(response.body.message).toEqual(
'User is missing a scope required to perform this action',
);
});
it('rejects deleting a role assigned to users without reassignRoleSlug (400)', async () => {
const roleInUse = await createCustomRoleWithScopeSlugs(['user:read'], { roleType: 'global' });
await createUser({ role: roleInUse });
const response = await testServer.publicApiAgentFor(owner).delete(`/roles/${roleInUse.slug}`);
expect(response.status).toBe(400);
expect(response.body.message).toContain('Cannot delete role assigned to users');
});
it('reassigns users to another role and deletes it, when the caller and the API key both hold user:changeRole', async () => {
const roleInUse = await createCustomRoleWithScopeSlugs(['user:read'], { roleType: 'global' });
const targetRole = await createCustomRoleWithScopeSlugs(['user:read'], {
roleType: 'global',
});
const user = await createUser({ role: roleInUse });
const response = await testServer
.publicApiAgentFor(owner)
.delete(`/roles/${roleInUse.slug}`)
.query({ reassignRoleSlug: targetRole.slug });
expect(response.status).toBe(200);
expect(response.body.slug).toBe(roleInUse.slug);
const reassignedUser = await getUserById(user.id);
expect(reassignedUser.role.slug).toBe(targetRole.slug);
const getResponse = await testServer.publicApiAgentFor(owner).get(`/roles/${roleInUse.slug}`);
expect(getResponse.status).toBe(404);
});
it('ignores the reassignment target when the API key lacks user:changeRole (400)', async () => {
const roleInUse = await createCustomRoleWithScopeSlugs(['user:read'], { roleType: 'global' });
const targetRole = await createCustomRoleWithScopeSlugs(['user:read'], {
roleType: 'global',
});
await createUser({ role: roleInUse });
// The owner user holds user:changeRole, but this key was granted only role scopes.
const scopedOwner = await createOwnerWithApiKey({
scopes: ['role:manage', 'role:manageProject', 'role:list', 'role:read'],
});
const response = await testServer
.publicApiAgentFor(scopedOwner)
.delete(`/roles/${roleInUse.slug}`)
.query({ reassignRoleSlug: targetRole.slug });
expect(response.status).toBe(400);
expect(response.body.message).toContain('Cannot delete role assigned to users');
});
it('ignores the reassignment target when the caller holds the role being deleted (400)', async () => {
const roleInUse = await createCustomRoleWithScopeSlugs(
['role:manage', 'user:changeRole', 'role:read'],
{ roleType: 'global' },
);
const targetRole = await createCustomRoleWithScopeSlugs(['user:read'], {
roleType: 'global',
});
const caller = await createUser({ role: roleInUse });
caller.apiKeys = [await addApiKey(caller, { scopes: ['role:manage', 'user:changeRole'] })];
const response = await testServer
.publicApiAgentFor(caller)
.delete(`/roles/${roleInUse.slug}`)
.query({ reassignRoleSlug: targetRole.slug });
expect(response.status).toBe(400);
expect(response.body.message).toContain('Cannot delete role assigned to users');
});
it('rejects reassigning users onto the role being deleted with 400', async () => {
const roleInUse = await createCustomRoleWithScopeSlugs(['user:read'], { roleType: 'global' });
await createUser({ role: roleInUse });
const response = await testServer
.publicApiAgentFor(owner)
.delete(`/roles/${roleInUse.slug}`)
.query({ reassignRoleSlug: roleInUse.slug });
expect(response.status).toBe(400);
expect(response.body.message).toContain('Cannot reassign users to the role being deleted');
});
it('rejects a reassignRoleSlug of global:owner with 400', async () => {
const roleInUse = await createCustomRoleWithScopeSlugs(['user:read'], { roleType: 'global' });
await createUser({ role: roleInUse });
const response = await testServer
.publicApiAgentFor(owner)
.delete(`/roles/${roleInUse.slug}`)
.query({ reassignRoleSlug: 'global:owner' });
expect(response.status).toBe(400);
});
it('returns 404 for an unknown slug', async () => {
const response = await testServer
.publicApiAgentFor(owner)
.delete('/roles/global:does-not-exist');
expect(response.status).toBe(404);
});
it('returns 404 for a role type not exposed by the public API (e.g. credential)', async () => {
const credentialRole = await createRole({ roleType: 'credential', scopes: [] });
const response = await testServer
.publicApiAgentFor(owner)
.delete(`/roles/${credentialRole.slug}`);
expect(response.status).toBe(404);
});
it('rejects deleting a system role with 400', async () => {
const response = await testServer.publicApiAgentFor(owner).delete('/roles/global:owner');
expect(response.status).toBe(400);
expect(response.body.message).toContain('Cannot delete system roles');
});
it('rejects with 401 without an API key', async () => {
const role = await createGlobalRole('PA delete no key');
const response = await testServer.publicApiAgentWithoutApiKey().delete(`/roles/${role.slug}`);
expect(response.status).toBe(401);
});
it('rejects with 401 with an invalid API key', async () => {
const role = await createGlobalRole('PA delete bad key');
const response = await testServer
.publicApiAgentWithApiKey('invalid-key')
.delete(`/roles/${role.slug}`);
expect(response.status).toBe(401);
});
it('rejects with 403 when the key lacks a role scope', async () => {
const role = await createGlobalRole('PA delete no scope');
const scopedOwner = await createOwnerWithApiKey({ scopes: ['user:read'] });
const response = await testServer
.publicApiAgentFor(scopedOwner)
.delete(`/roles/${role.slug}`);
expect(response.status).toBe(403);
});
it('rejects with 403 when the custom roles feature is not licensed', async () => {
const role = await createGlobalRole('PA delete unlicensed');
testServer.license.disable('feat:customRoles');
const response = await testServer.publicApiAgentFor(owner).delete(`/roles/${role.slug}`);
expect(response.status).toBe(403);
testServer.license.enable('feat:customRoles');
});
});
});

View File

@ -1272,7 +1272,10 @@ describe('RoleService', () => {
//
// ACT
//
const result = await roleService.removeCustomRole(customRole.slug);
const result = await roleService.removeCustomRole({
slug: customRole.slug,
userId: 'test-user-id',
});
//
// ASSERT
@ -1301,7 +1304,10 @@ describe('RoleService', () => {
//
// ACT
//
const result = await roleService.removeCustomRole(customRole.slug);
const result = await roleService.removeCustomRole({
slug: customRole.slug,
userId: 'test-user-id',
});
//
// ASSERT
@ -1326,8 +1332,12 @@ describe('RoleService', () => {
//
// ACT & ASSERT
//
await expect(roleService.removeCustomRole(nonExistentSlug)).rejects.toThrow(NotFoundError);
await expect(roleService.removeCustomRole(nonExistentSlug)).rejects.toThrow('Role not found');
await expect(
roleService.removeCustomRole({ slug: nonExistentSlug, userId: 'test-user-id' }),
).rejects.toThrow(NotFoundError);
await expect(
roleService.removeCustomRole({ slug: nonExistentSlug, userId: 'test-user-id' }),
).rejects.toThrow('Role not found');
});
it('should throw BadRequestError when trying to delete system role', async () => {
@ -1341,10 +1351,12 @@ describe('RoleService', () => {
//
// ACT & ASSERT
//
await expect(roleService.removeCustomRole(systemRole.slug)).rejects.toThrow(BadRequestError);
await expect(roleService.removeCustomRole(systemRole.slug)).rejects.toThrow(
'Cannot delete system roles',
);
await expect(
roleService.removeCustomRole({ slug: systemRole.slug, userId: 'test-user-id' }),
).rejects.toThrow(BadRequestError);
await expect(
roleService.removeCustomRole({ slug: systemRole.slug, userId: 'test-user-id' }),
).rejects.toThrow('Cannot delete system roles');
// Verify system role still exists
const stillExistsRole = await roleRepository.findBySlug(systemRole.slug);
@ -1370,10 +1382,12 @@ describe('RoleService', () => {
//
// ACT & ASSERT
//
await expect(roleService.removeCustomRole(roleInUse.slug)).rejects.toThrow(BadRequestError);
await expect(roleService.removeCustomRole(roleInUse.slug)).rejects.toThrow(
'Cannot delete role assigned to users',
);
await expect(
roleService.removeCustomRole({ slug: roleInUse.slug, userId: 'test-user-id' }),
).rejects.toThrow(BadRequestError);
await expect(
roleService.removeCustomRole({ slug: roleInUse.slug, userId: 'test-user-id' }),
).rejects.toThrow('Cannot delete role assigned to users');
});
it('should throw error when trying to delete role assigned to users on project', async () => {
@ -1401,10 +1415,12 @@ describe('RoleService', () => {
//
// ACT & ASSERT
//
await expect(roleService.removeCustomRole(roleInUse.slug)).rejects.toThrow(BadRequestError);
await expect(roleService.removeCustomRole(roleInUse.slug)).rejects.toThrow(
'Cannot delete role assigned to users',
);
await expect(
roleService.removeCustomRole({ slug: roleInUse.slug, userId: 'test-user-id' }),
).rejects.toThrow(BadRequestError);
await expect(
roleService.removeCustomRole({ slug: roleInUse.slug, userId: 'test-user-id' }),
).rejects.toThrow('Cannot delete role assigned to users');
});
it('should reassign globally assigned users to another role, then delete', async () => {
@ -1429,7 +1445,11 @@ describe('RoleService', () => {
//
// ACT
//
const result = await roleService.removeCustomRole(roleInUse.slug, targetRole.slug);
const result = await roleService.removeCustomRole({
slug: roleInUse.slug,
reassignRoleSlug: targetRole.slug,
userId: 'test-user-id',
});
//
// ASSERT
@ -1455,7 +1475,11 @@ describe('RoleService', () => {
await userRepository.save(user);
await expect(
roleService.removeCustomRole(roleInUse.slug, 'global:does-not-exist'),
roleService.removeCustomRole({
slug: roleInUse.slug,
reassignRoleSlug: 'global:does-not-exist',
userId: 'test-user-id',
}),
).rejects.toThrow('Reassignment role "global:does-not-exist" does not exist');
// Role is preserved when reassignment fails.
@ -1477,7 +1501,11 @@ describe('RoleService', () => {
await userRepository.save(user);
await expect(
roleService.removeCustomRole(roleInUse.slug, projectTarget.slug),
roleService.removeCustomRole({
slug: roleInUse.slug,
reassignRoleSlug: projectTarget.slug,
userId: 'test-user-id',
}),
).rejects.toThrow('Reassignment role must be of the same type as the deleted role');
});
@ -1491,9 +1519,13 @@ describe('RoleService', () => {
user.role = roleInUse;
await userRepository.save(user);
await expect(roleService.removeCustomRole(roleInUse.slug, roleInUse.slug)).rejects.toThrow(
'Cannot reassign users to the role being deleted',
);
await expect(
roleService.removeCustomRole({
slug: roleInUse.slug,
reassignRoleSlug: roleInUse.slug,
userId: 'test-user-id',
}),
).rejects.toThrow('Cannot reassign users to the role being deleted');
});
it('should ignore the reassignment role when no users are assigned', async () => {
@ -1508,7 +1540,11 @@ describe('RoleService', () => {
systemRole: false,
});
const result = await roleService.removeCustomRole(unusedRole.slug, targetRole.slug);
const result = await roleService.removeCustomRole({
slug: unusedRole.slug,
reassignRoleSlug: targetRole.slug,
userId: 'test-user-id',
});
expect(result.slug).toBe(unusedRole.slug);
expect(await roleRepository.findBySlug(unusedRole.slug)).toBeNull();
@ -1551,10 +1587,12 @@ describe('RoleService', () => {
//
// ACT & ASSERT
//
await expect(roleService.removeCustomRole(role.slug)).rejects.toThrow(BadRequestError);
await expect(roleService.removeCustomRole(role.slug)).rejects.toThrow(
'Cannot delete role: referenced by 1 role mapping rule',
);
await expect(
roleService.removeCustomRole({ slug: role.slug, userId: 'test-user-id' }),
).rejects.toThrow(BadRequestError);
await expect(
roleService.removeCustomRole({ slug: role.slug, userId: 'test-user-id' }),
).rejects.toThrow('Cannot delete role: referenced by 1 role mapping rule');
// Role is preserved, not silently orphaned.
const stillExists = await roleRepository.findBySlug(role.slug);
@ -1568,7 +1606,10 @@ describe('RoleService', () => {
systemRole: false,
});
const result = await roleService.removeCustomRole(role.slug);
const result = await roleService.removeCustomRole({
slug: role.slug,
userId: 'test-user-id',
});
expect(result.slug).toEqual(role.slug);
const deletedRole = await roleRepository.findBySlug(role.slug);

View File

@ -17,6 +17,9 @@
"PUT /roles/{slug}": {
"status": "gap"
},
"DELETE /roles/{slug}": {
"status": "gap"
},
"POST /role-mapping-rules": {
"status": "gap"
},