From 7853dac010e4b20a2b9a239b98ec1ffcaabe64fa Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Tue, 29 Apr 2025 10:46:10 +0200 Subject: [PATCH] permission-backend: accept resourceRef as array Signed-off-by: Vincenzo Scamporlino --- .../src/service/router.test.ts | 89 ++----------------- .../permission-backend/src/service/router.ts | 24 +---- .../src/PermissionClient.test.ts | 6 +- .../permission-common/src/PermissionClient.ts | 15 ++-- plugins/permission-node/report.api.md | 19 ++-- .../createPermissionIntegrationRouter.test.ts | 4 +- .../createPermissionIntegrationRouter.ts | 47 +++------- 7 files changed, 47 insertions(+), 157 deletions(-) diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index db9c915fae..386ae694ed 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -154,7 +154,7 @@ describe('createRouter', () => { }); }); - it('calls the permission policy with batched resourceRefs', async () => { + it('calls the permission policy with batched resourceRef as an array', async () => { policy.handle.mockResolvedValueOnce({ result: AuthorizeResult.CONDITIONAL, pluginId: 'test-plugin', @@ -181,7 +181,7 @@ describe('createRouter', () => { attributes: {}, resourceType: 'test-resource-1', }, - resourceRefs: ['resource:1', 'resource:2'], + resourceRef: ['resource:1', 'resource:2'], }, { id: '234', @@ -202,7 +202,7 @@ describe('createRouter', () => { conditions: { params: ['abc'], rule: 'test-rule' }, id: '123', pluginId: 'test-plugin', - resourceRefs: ['resource:1', 'resource:2'], + resourceRef: ['resource:1', 'resource:2'], resourceType: 'test-resource-1', result: 'CONDITIONAL', }, @@ -611,7 +611,7 @@ describe('createRouter', () => { }); }); - it('leaves conditional results without resourceRefs unchanged', async () => { + it('leaves conditional results without resourceRef unchanged', async () => { policy.handle .mockResolvedValueOnce({ result: AuthorizeResult.CONDITIONAL, @@ -881,79 +881,7 @@ describe('createRouter', () => { items: [ { id: '123', - // resource ref should be a string resourceRef: ['resource:1'], - permission: { - type: 'resource', - name: 'test.permission', - attributes: {}, - resourceType: 'test-resource-1', - }, - }, - ], - }, - { - items: [ - { - id: '123', - resourceRef: ['resource:1'], - permission: { - type: 'resource', - name: 'test.permission', - attributes: {}, - resourceType: 'test-resource-1', - }, - }, - ], - }, - { - items: [ - { - id: '123', - resourceRefs: 'resource:1', - permission: { - type: 'resource', - name: 'test.permission', - attributes: {}, - resourceType: 'test-resource-1', - }, - }, - ], - }, - { - items: [ - { - id: '123', - resourceRefs: [], - permission: { - type: 'resource', - name: 'test.permission', - attributes: {}, - resourceType: 'test-resource-1', - }, - }, - ], - }, - { - items: [ - { - id: '123', - resourceRefs: ['resource:1'], - resourceRef: 'resource:1', - permission: { - type: 'resource', - name: 'test.permission', - attributes: {}, - resourceType: 'test-resource-1', - }, - }, - ], - }, - { - items: [ - { - id: '123', - resourceRefs: ['resource:1'], permission: { type: 'basic', name: 'test.permission', @@ -966,16 +894,17 @@ describe('createRouter', () => { items: [ { id: '123', - resourceRef: 'resource:1', + resourceRef: [], permission: { - type: 'basic', + type: 'resource', name: 'test.permission', attributes: {}, + resourceType: 'test-resource-1', }, }, ], }, - ])('returns a 400 error for invalid request %#', async requestBody => { + ])('returns a 400 error for invalid request %o', async requestBody => { const response = await request(app).post('/authorize').send(requestBody); expect(response.status).toEqual(400); @@ -1017,7 +946,7 @@ describe('createRouter', () => { ); }); - it(`returns a 400 error if the request doesn't contain resourceRef or resourceRefs for credentials not issued by a service`, async () => { + it(`returns a 400 error if the request doesn't contain resourceRef for credentials not issued by a service`, async () => { policy.handle.mockResolvedValueOnce({ result: AuthorizeResult.CONDITIONAL, pluginId: 'test-plugin', diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 08bebcf8e3..b6f570562b 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -76,19 +76,13 @@ const evaluatePermissionRequestSchema = z.union([ z.object({ id: z.string(), resourceRef: z.undefined().optional(), - resourceRefs: z.undefined().optional(), permission: basicPermissionSchema, }), z.object({ id: z.string(), - resourceRef: z.string().optional(), - resourceRefs: z.undefined().optional(), - permission: resourcePermissionSchema, - }), - z.object({ - id: z.string(), - resourceRef: z.undefined().optional(), - resourceRefs: z.array(z.string()).nonempty().optional(), + resourceRef: z + .union([z.string(), z.array(z.string()).nonempty()]) + .optional(), permission: resourcePermissionSchema, }), ]); @@ -178,14 +172,6 @@ const handleRequest = async ( ); } - if (request.resourceRefs) { - return applyConditionsLoaderFor(decision.pluginId).load({ - id: request.id, - resourceRefs: request.resourceRefs, - ...decision, - }); - } - if (!request.resourceRef) { return { id: request.id, @@ -265,9 +251,7 @@ export async function createRouter( if ( body.items.some( r => - isResourcePermission(r.permission) && - r.resourceRef === undefined && - r.resourceRefs === undefined, + isResourcePermission(r.permission) && r.resourceRef === undefined, ) ) { throw new InputError( diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index 34b295778a..b55e5da94d 100644 --- a/plugins/permission-common/src/PermissionClient.test.ts +++ b/plugins/permission-common/src/PermissionClient.test.ts @@ -285,7 +285,7 @@ describe('PermissionClient', () => { { id: expect.any(String), permission: mockPermission, - resourceRefs: ['foo:bar', 'foo:car', 'foo:baz'], + resourceRef: ['foo:bar', 'foo:car', 'foo:baz'], }, { id: expect.any(String), @@ -453,7 +453,7 @@ describe('PermissionClient', () => { attributes: {}, resourceType: 'foo', }, - resourceRefs: ['foo:bar', 'foo:car'], + resourceRef: ['foo:bar', 'foo:car'], id: expect.any(String), }, { @@ -471,7 +471,7 @@ describe('PermissionClient', () => { attributes: {}, resourceType: 'foo', }, - resourceRefs: ['r2', 'r1'], + resourceRef: ['r2', 'r1'], id: expect.any(String), }, ], diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index 7fdb1c239f..93c3d5152f 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -210,12 +210,12 @@ export class PermissionClient implements PermissionEvaluator { if (isResourcePermission(permission)) { request[permission.name] ||= { permission, - resourceRefs: [], + resourceRef: [], id: uuid.v4(), }; if (resourceRef) { - request[permission.name].resourceRefs?.push(resourceRef); + request[permission.name].resourceRef?.push(resourceRef); } } else { request[permission.name] ||= { @@ -231,10 +231,15 @@ export class PermissionClient implements PermissionEvaluator { options, ); + const responsesById = parsedResponse.items.reduce((acc, r) => { + acc[r.id] = r; + return acc; + }, {} as Record); + return queries.map(query => { const { id } = request[query.permission.name]; - const item = parsedResponse.items.find(i => i.id === id)!; + const item = responsesById[id]; return { result: query.resourceRef ? item.result.shift()! : item.result[0], }; @@ -278,7 +283,7 @@ export class PermissionClient implements PermissionEvaluator { export type BatchedAuthorizePermissionRequest = IdentifiedPermissionMessage< | { permission: BasicPermission; - resourceRefs?: undefined; + resourceRef?: undefined; } - | { permission: ResourcePermission; resourceRefs: string[] } + | { permission: ResourcePermission; resourceRef: string[] } >; diff --git a/plugins/permission-node/report.api.md b/plugins/permission-node/report.api.md index af8c4f6455..6345861c93 100644 --- a/plugins/permission-node/report.api.md +++ b/plugins/permission-node/report.api.md @@ -38,20 +38,11 @@ export type ApplyConditionsRequest = { }; // @public -export type ApplyConditionsRequestEntry = IdentifiedPermissionMessage< - | { - resourceRef: string; - resourceRefs?: undefined; - resourceType: string; - conditions: PermissionCriteria; - } - | { - resourceRef?: undefined; - resourceRefs: string[]; - resourceType: string; - conditions: PermissionCriteria; - } ->; +export type ApplyConditionsRequestEntry = IdentifiedPermissionMessage<{ + resourceRef: string | string[]; + resourceType: string; + conditions: PermissionCriteria; +}>; // @public export type ApplyConditionsResponse = { diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index 973fb39d45..0d28ef710f 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -567,7 +567,7 @@ describe('createPermissionIntegrationRouter', () => { }); }); - describe('batched requests with resourceRefs', () => { + describe('batched requests with resourceRef as an array', () => { let response: Response; beforeEach(async () => { @@ -587,7 +587,7 @@ describe('createPermissionIntegrationRouter', () => { items: [ { id: '123', - resourceRefs: [ + resourceRef: [ 'default:test/resource-1', 'default:test/resource-2', ], diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index 78cf619206..4ee937866b 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -58,22 +58,12 @@ const permissionCriteriaSchema: z.ZodSchema< const applyConditionsRequestSchema = z.object({ items: z.array( - z.union([ - z.object({ - id: z.string(), - resourceRef: z.string(), - resourceRefs: z.undefined().optional(), - resourceType: z.string(), - conditions: permissionCriteriaSchema, - }), - z.object({ - id: z.string(), - resourceRef: z.undefined().optional(), - resourceRefs: z.array(z.string()), - resourceType: z.string(), - conditions: permissionCriteriaSchema, - }), - ]), + z.object({ + id: z.string(), + resourceRef: z.union([z.string(), z.array(z.string()).nonempty()]), + resourceType: z.string(), + conditions: permissionCriteriaSchema, + }), ), }); @@ -83,20 +73,11 @@ const applyConditionsRequestSchema = z.object({ * * @public */ -export type ApplyConditionsRequestEntry = IdentifiedPermissionMessage< - | { - resourceRef: string; - resourceRefs?: undefined; - resourceType: string; - conditions: PermissionCriteria; - } - | { - resourceRef?: undefined; - resourceRefs: string[]; - resourceType: string; - conditions: PermissionCriteria; - } ->; +export type ApplyConditionsRequestEntry = IdentifiedPermissionMessage<{ + resourceRef: string | string[]; + resourceType: string; + conditions: PermissionCriteria; +}>; /** * A batch of {@link ApplyConditionsRequestEntry} objects. @@ -536,7 +517,7 @@ export function createPermissionIntegrationRouter< requestedType, requests .filter(r => r.resourceType === requestedType) - .map(i => i.resourceRefs ?? [i.resourceRef]) + .map(i => i.resourceRef) .flat(), ); } @@ -544,8 +525,8 @@ export function createPermissionIntegrationRouter< res.json({ items: requests.map(request => ({ id: request.id, - result: request.resourceRefs - ? request.resourceRefs.map(resourceRef => + result: Array.isArray(request.resourceRef) + ? request.resourceRef.map(resourceRef => authorizeResult( request.conditions, resourcesByType[request.resourceType][resourceRef],