diff --git a/packages/@n8n/permissions/src/constants.ee.ts b/packages/@n8n/permissions/src/constants.ee.ts index 44425200f20..fc49460b09e 100644 --- a/packages/@n8n/permissions/src/constants.ee.ts +++ b/packages/@n8n/permissions/src/constants.ee.ts @@ -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'; diff --git a/packages/@n8n/permissions/src/public-api-permissions.ee.ts b/packages/@n8n/permissions/src/public-api-permissions.ee.ts index 749b96c2406..56c6a22d30d 100644 --- a/packages/@n8n/permissions/src/public-api-permissions.ee.ts +++ b/packages/@n8n/permissions/src/public-api-permissions.ee.ts @@ -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; diff --git a/packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.controller.ee.test.ts b/packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.controller.ee.test.ts index d04edd9987e..91f58df783c 100644 --- a/packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.controller.ee.test.ts +++ b/packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.controller.ee.test.ts @@ -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, + }); }); }); diff --git a/packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.service.ee.test.ts b/packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.service.ee.test.ts index dc3b48482dd..7fff6505394 100644 --- a/packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.service.ee.test.ts +++ b/packages/cli/src/modules/provisioning.ee/__tests__/role-mapping-rule.service.ee.test.ts @@ -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'], + }); }); }); diff --git a/packages/cli/src/modules/provisioning.ee/role-mapping-rule.controller.ee.ts b/packages/cli/src/modules/provisioning.ee/role-mapping-rule.controller.ee.ts index 3fd42eeaa91..8fa709f362a 100644 --- a/packages/cli/src/modules/provisioning.ee/role-mapping-rule.controller.ee.ts +++ b/packages/cli/src/modules/provisioning.ee/role-mapping-rule.controller.ee.ts @@ -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') diff --git a/packages/cli/src/modules/provisioning.ee/role-mapping-rule.service.ee.ts b/packages/cli/src/modules/provisioning.ee/role-mapping-rule.service.ee.ts index e120b2a91af..291ddc92242 100644 --- a/packages/cli/src/modules/provisioning.ee/role-mapping-rule.service.ee.ts +++ b/packages/cli/src/modules/provisioning.ee/role-mapping-rule.service.ee.ts @@ -275,7 +275,17 @@ export class RoleMappingRuleService { return result.affected ?? 0; } - async move(id: string, targetIndex: number): Promise { + async move({ + id, + targetIndex, + userId, + userEmail, + }: { + id: string; + targetIndex: number; + userId: string; + userEmail?: string; + }): Promise { 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 { diff --git a/packages/cli/src/public-api/v1/controllers/role-mapping-rules.public.controller.ts b/packages/cli/src/public-api/v1/controllers/role-mapping-rules.public.controller.ts index 3ef52defc96..b4777262712 100644 --- a/packages/cli/src/public-api/v1/controllers/role-mapping-rules.public.controller.ts +++ b/packages/cli/src/public-api/v1/controllers/role-mapping-rules.public.controller.ts @@ -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 { + 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'); diff --git a/packages/cli/src/public-api/v1/handlers/role-mapping-rules/spec/paths/createRoleMappingRule.generated.yml b/packages/cli/src/public-api/v1/handlers/role-mapping-rules/spec/paths/createRoleMappingRule.generated.yml index f49a06f09bc..f35a161ab2b 100644 --- a/packages/cli/src/public-api/v1/handlers/role-mapping-rules/spec/paths/createRoleMappingRule.generated.yml +++ b/packages/cli/src/public-api/v1/handlers/role-mapping-rules/spec/paths/createRoleMappingRule.generated.yml @@ -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': diff --git a/packages/cli/src/public-api/v1/handlers/role-mapping-rules/spec/paths/moveRoleMappingRule.generated.yml b/packages/cli/src/public-api/v1/handlers/role-mapping-rules/spec/paths/moveRoleMappingRule.generated.yml new file mode 100644 index 00000000000..b01e164f392 --- /dev/null +++ b/packages/cli/src/public-api/v1/handlers/role-mapping-rules/spec/paths/moveRoleMappingRule.generated.yml @@ -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 diff --git a/packages/cli/src/public-api/v1/openapi.decorator-routes.generated.yml b/packages/cli/src/public-api/v1/openapi.decorator-routes.generated.yml index 1cf210b065c..eb667a21230 100644 --- a/packages/cli/src/public-api/v1/openapi.decorator-routes.generated.yml +++ b/packages/cli/src/public-api/v1/openapi.decorator-routes.generated.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 diff --git a/packages/cli/src/public-api/v1/shared/spec/schemas/roleMappingRulePublicDto.generated.yml b/packages/cli/src/public-api/v1/shared/spec/schemas/roleMappingRulePublicDto.generated.yml new file mode 100644 index 00000000000..1967c5384cf --- /dev/null +++ b/packages/cli/src/public-api/v1/shared/spec/schemas/roleMappingRulePublicDto.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 diff --git a/packages/cli/test/integration/public-api/role-mapping-rules.test.ts b/packages/cli/test/integration/public-api/role-mapping-rules.test.ts index ce19c9bd62f..602c83b2604 100644 --- a/packages/cli/test/integration/public-api/role-mapping-rules.test.ts +++ b/packages/cli/test/integration/public-api/role-mapping-rules.test.ts @@ -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'); + }); + }); }); diff --git a/packages/nodes-base/nodes/N8n/n8n-api-coverage.json b/packages/nodes-base/nodes/N8n/n8n-api-coverage.json index 7749aa06d46..939e032067d 100644 --- a/packages/nodes-base/nodes/N8n/n8n-api-coverage.json +++ b/packages/nodes-base/nodes/N8n/n8n-api-coverage.json @@ -47,6 +47,9 @@ "POST /role-mapping-rules": { "status": "gap" }, + "POST /role-mapping-rules/{roleMappingRuleId}/move": { + "status": "gap" + }, "GET /settings/security-policy": { "status": "gap" },