mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
feat(API): Add endpoint to move role-mapping rules (#36526)
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
e86ebf3d68
commit
d79cd10d0e
|
|
@ -118,7 +118,7 @@ export const API_KEY_RESOURCES = {
|
|||
folder: ['create', 'delete', 'read', 'update', 'list'] as const,
|
||||
insights: ['read'] as const,
|
||||
role: ['manage', 'manageProject', 'list', 'read'] as const,
|
||||
roleMappingRule: ['create', 'list'] as const,
|
||||
roleMappingRule: ['create', 'list', 'update'] as const,
|
||||
} as const;
|
||||
|
||||
export const GLOBAL_OWNER_ROLE_SLUG = 'global:owner';
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ export const OWNER_API_KEY_SCOPES: ApiKeyScope[] = [
|
|||
'role:read',
|
||||
'roleMappingRule:create',
|
||||
'roleMappingRule:list',
|
||||
'roleMappingRule:update',
|
||||
];
|
||||
|
||||
export const ADMIN_API_KEY_SCOPES: ApiKeyScope[] = OWNER_API_KEY_SCOPES;
|
||||
|
|
|
|||
|
|
@ -188,7 +188,12 @@ describe('RoleMappingRuleController', () => {
|
|||
const result = await controller.move(req, res, moveBody, ruleId);
|
||||
|
||||
expect(result).toEqual(moved);
|
||||
expect(roleMappingRuleService.move).toHaveBeenCalledWith(ruleId, moveBody.targetIndex);
|
||||
expect(roleMappingRuleService.move).toHaveBeenCalledWith({
|
||||
id: ruleId,
|
||||
targetIndex: moveBody.targetIndex,
|
||||
userId: req.user.id,
|
||||
userEmail: req.user.email,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -823,7 +823,16 @@ describe('RoleMappingRuleService', () => {
|
|||
it('should throw NotFoundError when rule does not exist', async () => {
|
||||
roleMappingRuleRepository.findOne.mockResolvedValue(null);
|
||||
|
||||
await expect(service.move('nonexistent', 0)).rejects.toThrow(NotFoundError);
|
||||
await expect(
|
||||
service.move({
|
||||
id: 'nonexistent',
|
||||
targetIndex: 0,
|
||||
userId: testUser.id,
|
||||
userEmail: testUser.email,
|
||||
}),
|
||||
).rejects.toThrow(NotFoundError);
|
||||
|
||||
expect(eventService.emit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should move first rule to last position', async () => {
|
||||
|
|
@ -837,10 +846,21 @@ describe('RoleMappingRuleService', () => {
|
|||
updatedAt: new Date(),
|
||||
} as unknown as RoleMappingRule);
|
||||
|
||||
await service.move('a', 2);
|
||||
await service.move({
|
||||
id: 'a',
|
||||
targetIndex: 2,
|
||||
userId: testUser.id,
|
||||
userEmail: testUser.email,
|
||||
});
|
||||
|
||||
// Verify applyOrder called with correct sequence: b, c, a
|
||||
expect(transactionSpy).toHaveBeenCalledTimes(1);
|
||||
expect(eventService.emit).toHaveBeenCalledWith('role-mapping-rule-updated', {
|
||||
user: { id: testUser.id, email: testUser.email },
|
||||
ruleId: 'a',
|
||||
ruleType: 'instance',
|
||||
patchedFields: ['order'],
|
||||
});
|
||||
});
|
||||
|
||||
it('should move last rule to first position', async () => {
|
||||
|
|
@ -854,9 +874,20 @@ describe('RoleMappingRuleService', () => {
|
|||
updatedAt: new Date(),
|
||||
} as unknown as RoleMappingRule);
|
||||
|
||||
await service.move('c', 0);
|
||||
await service.move({
|
||||
id: 'c',
|
||||
targetIndex: 0,
|
||||
userId: testUser.id,
|
||||
userEmail: testUser.email,
|
||||
});
|
||||
|
||||
expect(transactionSpy).toHaveBeenCalledTimes(1);
|
||||
expect(eventService.emit).toHaveBeenCalledWith('role-mapping-rule-updated', {
|
||||
user: { id: testUser.id, email: testUser.email },
|
||||
ruleId: 'c',
|
||||
ruleType: 'instance',
|
||||
patchedFields: ['order'],
|
||||
});
|
||||
});
|
||||
|
||||
it('should clamp targetIndex to last position when out of bounds', async () => {
|
||||
|
|
@ -871,9 +902,20 @@ describe('RoleMappingRuleService', () => {
|
|||
} as unknown as RoleMappingRule);
|
||||
|
||||
// targetIndex 999 should clamp to 1 (last valid index)
|
||||
await service.move('a', 999);
|
||||
await service.move({
|
||||
id: 'a',
|
||||
targetIndex: 999,
|
||||
userId: testUser.id,
|
||||
userEmail: testUser.email,
|
||||
});
|
||||
|
||||
expect(transactionSpy).toHaveBeenCalledTimes(1);
|
||||
expect(eventService.emit).toHaveBeenCalledWith('role-mapping-rule-updated', {
|
||||
user: { id: testUser.id, email: testUser.email },
|
||||
ruleId: 'a',
|
||||
ruleType: 'instance',
|
||||
patchedFields: ['order'],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -75,16 +75,12 @@ export class RoleMappingRuleController {
|
|||
return res.status(403).json({ message: 'Provisioning is not licensed' });
|
||||
}
|
||||
|
||||
const result = await this.roleMappingRuleService.move(id, body.targetIndex);
|
||||
|
||||
this.eventService.emit('role-mapping-rule-updated', {
|
||||
user: { id: req.user.id, email: req.user.email },
|
||||
ruleId: result.id,
|
||||
ruleType: result.type,
|
||||
patchedFields: ['order'],
|
||||
return await this.roleMappingRuleService.move({
|
||||
id,
|
||||
targetIndex: body.targetIndex,
|
||||
userId: req.user.id,
|
||||
userEmail: req.user.email,
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
@Patch('/:id')
|
||||
|
|
|
|||
|
|
@ -275,7 +275,17 @@ export class RoleMappingRuleService {
|
|||
return result.affected ?? 0;
|
||||
}
|
||||
|
||||
async move(id: string, targetIndex: number): Promise<RoleMappingRuleResponse> {
|
||||
async move({
|
||||
id,
|
||||
targetIndex,
|
||||
userId,
|
||||
userEmail,
|
||||
}: {
|
||||
id: string;
|
||||
targetIndex: number;
|
||||
userId: string;
|
||||
userEmail?: string;
|
||||
}): Promise<RoleMappingRuleResponse> {
|
||||
if (typeof id !== 'string' || id.length === 0) {
|
||||
throw new BadRequestError('Rule id is required');
|
||||
}
|
||||
|
|
@ -311,7 +321,16 @@ export class RoleMappingRuleService {
|
|||
relations: ['projects', 'role'],
|
||||
});
|
||||
|
||||
return this.toResponse(loaded);
|
||||
const result = this.toResponse(loaded);
|
||||
|
||||
this.eventService.emit('role-mapping-rule-updated', {
|
||||
user: { id: userId, email: userEmail },
|
||||
ruleId: result.id,
|
||||
ruleType: result.type,
|
||||
patchedFields: ['order'],
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
private async applyOrder(orderedIds: string[]): Promise<void> {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import {
|
||||
CreateRoleMappingRuleDto,
|
||||
MoveRoleMappingRuleDto,
|
||||
RoleMappingRuleListPublicDto,
|
||||
RoleMappingRuleListQueryPublicDto,
|
||||
RoleMappingRulePublicDto,
|
||||
|
|
@ -15,6 +16,7 @@ import {
|
|||
ApiTags,
|
||||
Body,
|
||||
Get,
|
||||
Param,
|
||||
Post,
|
||||
PublicApiController,
|
||||
Query,
|
||||
|
|
@ -103,6 +105,33 @@ export class RoleMappingRulesPublicController {
|
|||
return toRoleMappingRulePublicDto(rule);
|
||||
}
|
||||
|
||||
@Post('/:roleMappingRuleId/move')
|
||||
@ApiKeyScope('roleMappingRule:update')
|
||||
@ApiSummary('Move a role-mapping rule')
|
||||
@ApiDescription(
|
||||
"Changes a rule's position in the evaluation order for its type. `targetIndex` is the desired 0-based position within the rule's own `type` sequence; a value beyond the last position moves the rule to the end.",
|
||||
)
|
||||
@ApiTags(['RoleMappingRule'])
|
||||
@ApiResponse(200, RoleMappingRulePublicDto)
|
||||
@ApiErrorResponse(404)
|
||||
async moveRoleMappingRule(
|
||||
req: AuthenticatedRequest,
|
||||
_res: Response,
|
||||
@Param('roleMappingRuleId') roleMappingRuleId: string,
|
||||
@Body body: MoveRoleMappingRuleDto,
|
||||
): Promise<RoleMappingRulePublicDto> {
|
||||
this.assertProvisioningLicensed();
|
||||
|
||||
const rule = await this.roleMappingRuleService.move({
|
||||
id: roleMappingRuleId,
|
||||
targetIndex: body.targetIndex,
|
||||
userId: req.user.id,
|
||||
userEmail: req.user.email,
|
||||
});
|
||||
|
||||
return toRoleMappingRulePublicDto(rule);
|
||||
}
|
||||
|
||||
private assertProvisioningLicensed(): void {
|
||||
if (!this.licenseState.isProvisioningLicensed()) {
|
||||
throw new ForbiddenError('Provisioning is not licensed');
|
||||
|
|
|
|||
|
|
@ -42,40 +42,7 @@ responses:
|
|||
content:
|
||||
application/json:
|
||||
schema:
|
||||
type: object
|
||||
properties:
|
||||
id:
|
||||
type: string
|
||||
expression:
|
||||
type: string
|
||||
role:
|
||||
type: string
|
||||
type:
|
||||
type: string
|
||||
enum:
|
||||
- instance
|
||||
- project
|
||||
order:
|
||||
type: integer
|
||||
projectIds:
|
||||
type: array
|
||||
items:
|
||||
type: string
|
||||
createdAt:
|
||||
type: string
|
||||
format: date-time
|
||||
updatedAt:
|
||||
type: string
|
||||
format: date-time
|
||||
required:
|
||||
- id
|
||||
- expression
|
||||
- role
|
||||
- type
|
||||
- order
|
||||
- projectIds
|
||||
- createdAt
|
||||
- updatedAt
|
||||
$ref: ../../../../shared/spec/schemas/roleMappingRulePublicDto.generated.yml
|
||||
'400':
|
||||
$ref: ../../../../shared/spec/responses/badRequest.yml
|
||||
'401':
|
||||
|
|
|
|||
|
|
@ -0,0 +1,41 @@
|
|||
operationId: moveRoleMappingRule
|
||||
tags:
|
||||
- RoleMappingRule
|
||||
summary: Move a role-mapping rule
|
||||
description: Changes a rule's position in the evaluation order for its type. `targetIndex` is the desired 0-based position within the rule's own `type` sequence; a value beyond the last position moves the rule to the end.
|
||||
x-required-scope: roleMappingRule:update
|
||||
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: roleMappingRuleId
|
||||
in: path
|
||||
requestBody:
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
type: object
|
||||
properties:
|
||||
targetIndex:
|
||||
type: integer
|
||||
minimum: 0
|
||||
required:
|
||||
- targetIndex
|
||||
responses:
|
||||
'200':
|
||||
description: Operation successful.
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: ../../../../shared/spec/schemas/roleMappingRulePublicDto.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
|
||||
|
|
@ -26,6 +26,9 @@ paths:
|
|||
$ref: ./handlers/role-mapping-rules/spec/paths/getRoleMappingRules.generated.yml
|
||||
post:
|
||||
$ref: ./handlers/role-mapping-rules/spec/paths/createRoleMappingRule.generated.yml
|
||||
/role-mapping-rules/{roleMappingRuleId}/move:
|
||||
post:
|
||||
$ref: ./handlers/role-mapping-rules/spec/paths/moveRoleMappingRule.generated.yml
|
||||
/roles:
|
||||
get:
|
||||
$ref: ./handlers/roles/spec/paths/getAllRoles.generated.yml
|
||||
|
|
|
|||
|
|
@ -0,0 +1,34 @@
|
|||
type: object
|
||||
properties:
|
||||
id:
|
||||
type: string
|
||||
expression:
|
||||
type: string
|
||||
role:
|
||||
type: string
|
||||
type:
|
||||
type: string
|
||||
enum:
|
||||
- instance
|
||||
- project
|
||||
order:
|
||||
type: integer
|
||||
projectIds:
|
||||
type: array
|
||||
items:
|
||||
type: string
|
||||
createdAt:
|
||||
type: string
|
||||
format: date-time
|
||||
updatedAt:
|
||||
type: string
|
||||
format: date-time
|
||||
required:
|
||||
- id
|
||||
- expression
|
||||
- role
|
||||
- type
|
||||
- order
|
||||
- projectIds
|
||||
- createdAt
|
||||
- updatedAt
|
||||
|
|
@ -352,4 +352,205 @@ describe('Role mapping rules in Public API', () => {
|
|||
testServer.license.enable('feat:saml');
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /role-mapping-rules/{roleMappingRuleId}/move', () => {
|
||||
it('moves a rule to an earlier position and returns 200', async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
const { expression, role, type } = validInstancePayload;
|
||||
|
||||
const first = await agent.post('/role-mapping-rules').send(validInstancePayload);
|
||||
const second = await agent
|
||||
.post('/role-mapping-rules')
|
||||
.send({ expression: `${expression} || false`, role, type });
|
||||
|
||||
const response = await agent
|
||||
.post(`/role-mapping-rules/${second.body.id}/move`)
|
||||
.send({ targetIndex: 0 });
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body).toEqual({
|
||||
id: second.body.id,
|
||||
expression: second.body.expression,
|
||||
role: 'global:member',
|
||||
type: 'instance',
|
||||
order: 0,
|
||||
projectIds: [],
|
||||
createdAt: expect.any(String),
|
||||
updatedAt: expect.any(String),
|
||||
});
|
||||
|
||||
const list = await agent.get('/role-mapping-rules');
|
||||
expect(list.body.data.map((rule: { id: string }) => rule.id)).toEqual([
|
||||
second.body.id,
|
||||
first.body.id,
|
||||
]);
|
||||
});
|
||||
|
||||
it('renumbers sibling rules of the same type', async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
const { expression, role, type } = validInstancePayload;
|
||||
|
||||
const first = await agent.post('/role-mapping-rules').send(validInstancePayload);
|
||||
const second = await agent
|
||||
.post('/role-mapping-rules')
|
||||
.send({ expression: `${expression} || false`, role, type });
|
||||
const third = await agent
|
||||
.post('/role-mapping-rules')
|
||||
.send({ expression: `${expression} || true`, role, type });
|
||||
|
||||
await agent.post(`/role-mapping-rules/${third.body.id}/move`).send({ targetIndex: 0 });
|
||||
|
||||
const list = await agent.get('/role-mapping-rules');
|
||||
expect(
|
||||
list.body.data.map((rule: { id: string; order: number }) => ({
|
||||
id: rule.id,
|
||||
order: rule.order,
|
||||
})),
|
||||
).toEqual([
|
||||
{ id: third.body.id, order: 0 },
|
||||
{ id: first.body.id, order: 1 },
|
||||
{ id: second.body.id, order: 2 },
|
||||
]);
|
||||
});
|
||||
|
||||
it("only renumbers the moved rule's own type", async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
const { expression, role, type } = validInstancePayload;
|
||||
|
||||
await agent.post('/role-mapping-rules').send(validInstancePayload);
|
||||
const secondInstance = await agent
|
||||
.post('/role-mapping-rules')
|
||||
.send({ expression: `${expression} || false`, role, type });
|
||||
|
||||
const firstProject = await agent.post('/role-mapping-rules').send({
|
||||
expression: 'claims.project === "alpha"',
|
||||
role: 'project:editor',
|
||||
type: 'project',
|
||||
projectIds: [teamProject.id],
|
||||
});
|
||||
const secondProject = await agent.post('/role-mapping-rules').send({
|
||||
expression: 'claims.project === "beta"',
|
||||
role: 'project:editor',
|
||||
type: 'project',
|
||||
projectIds: [teamProject.id],
|
||||
});
|
||||
|
||||
await agent
|
||||
.post(`/role-mapping-rules/${secondInstance.body.id}/move`)
|
||||
.send({ targetIndex: 0 });
|
||||
|
||||
const projectRules = await agent.get('/role-mapping-rules').query({ type: 'project' });
|
||||
expect(
|
||||
projectRules.body.data.map((rule: { id: string; order: number }) => ({
|
||||
id: rule.id,
|
||||
order: rule.order,
|
||||
})),
|
||||
).toEqual([
|
||||
{ id: firstProject.body.id, order: 0 },
|
||||
{ id: secondProject.body.id, order: 1 },
|
||||
]);
|
||||
});
|
||||
|
||||
it('clamps a targetIndex beyond the last position to the end', async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
const { expression, role, type } = validInstancePayload;
|
||||
|
||||
const first = await agent.post('/role-mapping-rules').send(validInstancePayload);
|
||||
const second = await agent
|
||||
.post('/role-mapping-rules')
|
||||
.send({ expression: `${expression} || false`, role, type });
|
||||
|
||||
const response = await agent
|
||||
.post(`/role-mapping-rules/${first.body.id}/move`)
|
||||
.send({ targetIndex: 99 });
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.body.order).toBe(1);
|
||||
|
||||
const list = await agent.get('/role-mapping-rules');
|
||||
expect(list.body.data.map((rule: { id: string }) => rule.id)).toEqual([
|
||||
second.body.id,
|
||||
first.body.id,
|
||||
]);
|
||||
});
|
||||
|
||||
it('rejects a negative targetIndex with 400', async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
const created = await agent.post('/role-mapping-rules').send(validInstancePayload);
|
||||
|
||||
const response = await agent
|
||||
.post(`/role-mapping-rules/${created.body.id}/move`)
|
||||
.send({ targetIndex: -1 });
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
});
|
||||
|
||||
it('rejects a missing targetIndex with 400', async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
const created = await agent.post('/role-mapping-rules').send(validInstancePayload);
|
||||
|
||||
const response = await agent.post(`/role-mapping-rules/${created.body.id}/move`).send({});
|
||||
|
||||
expect(response.status).toBe(400);
|
||||
});
|
||||
|
||||
it('rejects an unknown rule id with 404', async () => {
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(owner)
|
||||
.post('/role-mapping-rules/does-not-exist/move')
|
||||
.send({ targetIndex: 0 });
|
||||
|
||||
expect(response.status).toBe(404);
|
||||
});
|
||||
|
||||
it('rejects with 401 without an API key', async () => {
|
||||
const response = await testServer
|
||||
.publicApiAgentWithoutApiKey()
|
||||
.post('/role-mapping-rules/does-not-exist/move')
|
||||
.send({ targetIndex: 0 });
|
||||
|
||||
expect(response.status).toBe(401);
|
||||
});
|
||||
|
||||
it('rejects with 401 with an invalid API key', async () => {
|
||||
const response = await testServer
|
||||
.publicApiAgentWithApiKey('invalid-key')
|
||||
.post('/role-mapping-rules/does-not-exist/move')
|
||||
.send({ targetIndex: 0 });
|
||||
|
||||
expect(response.status).toBe(401);
|
||||
});
|
||||
|
||||
it('rejects with 403 when the key lacks roleMappingRule:update', async () => {
|
||||
const scopedOwner = await createOwnerWithApiKey({ scopes: ['user:read'] });
|
||||
const created = await testServer
|
||||
.publicApiAgentFor(owner)
|
||||
.post('/role-mapping-rules')
|
||||
.send(validInstancePayload);
|
||||
|
||||
const response = await testServer
|
||||
.publicApiAgentFor(scopedOwner)
|
||||
.post(`/role-mapping-rules/${created.body.id}/move`)
|
||||
.send({ targetIndex: 0 });
|
||||
|
||||
expect(response.status).toBe(403);
|
||||
});
|
||||
|
||||
it('rejects with 403 when provisioning is not licensed', async () => {
|
||||
const agent = testServer.publicApiAgentFor(owner);
|
||||
const created = await agent.post('/role-mapping-rules').send(validInstancePayload);
|
||||
|
||||
testServer.license.disable('feat:saml');
|
||||
testServer.license.disable('feat:oidc');
|
||||
|
||||
const response = await agent
|
||||
.post(`/role-mapping-rules/${created.body.id}/move`)
|
||||
.send({ targetIndex: 0 });
|
||||
|
||||
expect(response.status).toBe(403);
|
||||
expect(response.body.message).toBe('Provisioning is not licensed');
|
||||
|
||||
testServer.license.enable('feat:saml');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -47,6 +47,9 @@
|
|||
"POST /role-mapping-rules": {
|
||||
"status": "gap"
|
||||
},
|
||||
"POST /role-mapping-rules/{roleMappingRuleId}/move": {
|
||||
"status": "gap"
|
||||
},
|
||||
"GET /settings/security-policy": {
|
||||
"status": "gap"
|
||||
},
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user