From 1e60bfffa1313cfd9b0b713a0093d419ee727963 Mon Sep 17 00:00:00 2001 From: Vincenzo Scamporlino Date: Wed, 9 Apr 2025 22:24:18 +0200 Subject: [PATCH] permission-common: batching of permissions Signed-off-by: Vincenzo Scamporlino --- plugins/permission-common/config.d.ts | 5 + .../src/PermissionClient.test.ts | 282 +++++++++++++++++- .../permission-common/src/PermissionClient.ts | 77 ++++- plugins/permission-common/src/types/api.ts | 1 + 4 files changed, 358 insertions(+), 7 deletions(-) diff --git a/plugins/permission-common/config.d.ts b/plugins/permission-common/config.d.ts index 2378fa4a71..d43fc96586 100644 --- a/plugins/permission-common/config.d.ts +++ b/plugins/permission-common/config.d.ts @@ -23,5 +23,10 @@ export interface Config { * @visibility frontend */ enabled?: boolean; + + /** + * @visibility frontend + */ + EXPERIMENTAL_enableBatchedRequests?: boolean; }; } diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index 72707bf98a..b32c5f148e 100644 --- a/plugins/permission-common/src/PermissionClient.test.ts +++ b/plugins/permission-common/src/PermissionClient.test.ts @@ -17,7 +17,10 @@ import { RestContext, rest } from 'msw'; import { setupServer } from 'msw/node'; import { ConfigReader } from '@backstage/config'; -import { PermissionClient } from './PermissionClient'; +import { + BatchedAuthorizePermissionRequest, + PermissionClient, +} from './PermissionClient'; import { EvaluatePermissionRequest, AuthorizeResult, @@ -36,10 +39,7 @@ const discovery: DiscoveryApi = { return mockBaseUrl; }, }; -const client: PermissionClient = new PermissionClient({ - discovery, - config: new ConfigReader({ permission: { enabled: true } }), -}); +let client: PermissionClient; const mockPermission = createPermission({ name: 'test.permission', @@ -53,6 +53,13 @@ describe('PermissionClient', () => { afterEach(() => server.resetHandlers()); describe('authorize', () => { + beforeAll(() => { + client = new PermissionClient({ + discovery, + config: new ConfigReader({ permission: { enabled: true } }), + }); + }); + const mockAuthorizeConditional = { permission: mockPermission, resourceRef: 'foo:bar', @@ -211,7 +218,270 @@ describe('PermissionClient', () => { }); }); + describe('authorize (batched)', () => { + beforeAll(() => { + client = new PermissionClient({ + discovery, + config: new ConfigReader({ + permission: { + enabled: true, + EXPERIMENTAL_enableBatchedRequests: true, + }, + }), + }); + }); + + const mockAuthorizeConditional = { + permission: mockPermission, + resourceRef: 'foo:bar', + }; + + const mockAuthorizeHandler = jest.fn(); + + beforeEach(() => { + mockAuthorizeHandler.mockReset(); + server.use(rest.post(`${mockBaseUrl}/authorize`, mockAuthorizeHandler)); + + mockAuthorizeHandler.mockImplementation( + (req, res, { json }: RestContext) => { + const responses = req.body.items.map( + (a: BatchedAuthorizePermissionRequest) => ({ + id: a.id, + result: [AuthorizeResult.ALLOW], + }), + ); + + return res(json({ items: responses })); + }, + ); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should fetch entities from correct endpoint', async () => { + await client.authorize([mockAuthorizeConditional]); + expect(mockAuthorizeHandler).toHaveBeenCalled(); + }); + + it('should include a request body', async () => { + await client.authorize([mockAuthorizeConditional]); + + const request = mockAuthorizeHandler.mock.calls[0][0]; + + expect(request.body).toEqual({ + items: [ + expect.objectContaining({ + permission: mockPermission, + resourceRefs: ['foo:bar'], + }), + ], + }); + }); + + it('should return the response from the fetch request', async () => { + const response = await client.authorize([mockAuthorizeConditional]); + expect(response[0]).toEqual( + expect.objectContaining({ result: AuthorizeResult.ALLOW }), + ); + }); + + it('should not include authorization headers if no token is supplied', async () => { + await client.authorize([mockAuthorizeConditional]); + + const request = mockAuthorizeHandler.mock.calls[0][0]; + expect(request.headers.has('authorization')).toEqual(false); + }); + + it('should include correctly-constructed authorization header if token is supplied', async () => { + await client.authorize([mockAuthorizeConditional], { token }); + + const request = mockAuthorizeHandler.mock.calls[0][0]; + expect(request.headers.get('authorization')).toEqual('Bearer fake-token'); + }); + + it('should forward response errors', async () => { + mockAuthorizeHandler.mockImplementationOnce( + (_req, res, { status }: RestContext) => { + return res(status(401)); + }, + ); + await expect( + client.authorize([mockAuthorizeConditional], { token }), + ).rejects.toThrow(/request failed with 401/i); + }); + + it('should reject responses with missing ids', async () => { + mockAuthorizeHandler.mockImplementationOnce( + (_req, res, { json }: RestContext) => { + return res( + json({ + items: [{ id: 'wrong-id', result: [AuthorizeResult.ALLOW] }], + }), + ); + }, + ); + await expect( + client.authorize([mockAuthorizeConditional], { token }), + ).rejects.toThrow(/items in response do not match request/i); + }); + + it('should reject invalid responses', async () => { + mockAuthorizeHandler.mockImplementationOnce( + (req, res, { json }: RestContext) => { + const responses = req.body.items.map( + (a: IdentifiedPermissionMessage) => ({ + id: a.id, + outcome: AuthorizeResult.ALLOW, + }), + ); + + return res(json({ items: responses })); + }, + ); + await expect( + client.authorize([mockAuthorizeConditional], { token }), + ).rejects.toThrow(/invalid_type/i); + }); + + it('should allow all when permission.enabled is false', async () => { + const disabled = new PermissionClient({ + discovery, + config: new ConfigReader({ permission: { enabled: false } }), + }); + const response = await disabled.authorize([mockAuthorizeConditional]); + expect(response[0]).toEqual( + expect.objectContaining({ result: AuthorizeResult.ALLOW }), + ); + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should allow all when permission.enabled is not configured', async () => { + const disabled = new PermissionClient({ + discovery, + config: new ConfigReader({}), + }); + const response = await disabled.authorize([mockAuthorizeConditional]); + expect(response[0]).toEqual( + expect.objectContaining({ result: AuthorizeResult.ALLOW }), + ); + expect(mockAuthorizeHandler).not.toHaveBeenCalled(); + }); + + it('should properly map the permissions', async () => { + const mockPermission2 = createPermission({ + name: 'test.permission2', + attributes: {}, + resourceType: 'foo', + }); + + const mockPermission3 = createPermission({ + name: 'test.permission3', + attributes: {}, + }); + + mockAuthorizeHandler.mockImplementationOnce( + (req, res, { json }: RestContext) => { + return res( + json({ + items: [ + { + id: req.body.items[0].id, + result: [AuthorizeResult.ALLOW, AuthorizeResult.DENY], + }, + { + id: req.body.items[1].id, + result: [AuthorizeResult.DENY], + }, + { + id: req.body.items[2].id, + result: [AuthorizeResult.DENY, AuthorizeResult.ALLOW], + }, + ], + }), + ); + }, + ); + + const response = await client.authorize([ + { + permission: mockPermission, + resourceRef: 'foo:bar', // allow + }, + { + permission: mockPermission3, // deny + }, + { + permission: mockPermission, + resourceRef: 'foo:car', // deny + }, + { + permission: mockPermission3, // deny + }, + { + permission: mockPermission2, + resourceRef: 'r2', // deny + }, + { + permission: mockPermission2, + resourceRef: 'r1', // allow + }, + ]); + + expect(mockAuthorizeHandler.mock.calls[0][0].body).toEqual({ + items: [ + { + permission: { + type: 'resource', + name: 'test.permission', + attributes: {}, + resourceType: 'foo', + }, + resourceRefs: ['foo:bar', 'foo:car'], + id: expect.any(String), + }, + { + permission: { + type: 'basic', + name: 'test.permission3', + attributes: {}, + }, + resourceRefs: [], + id: expect.any(String), + }, + { + permission: { + type: 'resource', + name: 'test.permission2', + attributes: {}, + resourceType: 'foo', + }, + resourceRefs: ['r2', 'r1'], + id: expect.any(String), + }, + ], + }); + + expect(response).toEqual([ + { result: 'ALLOW' }, + { result: 'DENY' }, + { result: 'DENY' }, + { result: 'DENY' }, + { result: 'DENY' }, + { result: 'ALLOW' }, + ]); + }); + }); + describe('authorizeConditional', () => { + beforeAll(() => { + client = new PermissionClient({ + discovery, + config: new ConfigReader({ permission: { enabled: true } }), + }); + }); + const mockResourceAuthorizeConditional = { permission: mockPermission, }; @@ -420,7 +690,7 @@ describe('PermissionClient', () => { }), ); - return res(json(responses)); + return res(json({ items: responses })); }, ); const disabled = new PermissionClient({ diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts index 2414774775..04810ec66f 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -29,9 +29,10 @@ import { AuthorizePermissionRequest, AuthorizePermissionResponse, QueryPermissionResponse, + IdentifiedPermissionMessage, } from './types/api'; import { DiscoveryApi } from './types/discovery'; -import { AuthorizeRequestOptions } from './types/permission'; +import { AuthorizeRequestOptions, Permission } from './types/permission'; const permissionCriteriaSchema: z.ZodSchema< PermissionCriteria @@ -108,11 +109,17 @@ export type PermissionClientRequestOptions = { export class PermissionClient implements PermissionEvaluator { private readonly enabled: boolean; private readonly discovery: DiscoveryApi; + private readonly enableBatchedRequests: boolean; constructor(options: { discovery: DiscoveryApi; config: Config }) { this.discovery = options.discovery; this.enabled = options.config.getOptionalBoolean('permission.enabled') ?? false; + + this.enableBatchedRequests = + options.config.getOptionalBoolean( + 'permission.EXPERIMENTAL_enableBatchedRequests', + ) ?? false; } /** @@ -122,6 +129,10 @@ export class PermissionClient implements PermissionEvaluator { requests: AuthorizePermissionRequest[], options?: PermissionClientRequestOptions, ): Promise { + if (this.enableBatchedRequests) { + return this.makeBatchedRequest(requests, options); + } + return this.makeRequest( requests, authorizePermissionResponseSchema, @@ -183,7 +194,71 @@ export class PermissionClient implements PermissionEvaluator { return request.items.map(query => responsesById[query.id]); } + private async makeBatchedRequest( + queries: AuthorizePermissionRequest[], + options?: AuthorizeRequestOptions, + ) { + if (!this.enabled) { + return queries.map(_ => ({ result: AuthorizeResult.ALLOW as const })); + } + + const request: Record = {}; + + for (const query of queries) { + const { permission, resourceRef } = query; + + request[permission.name] ||= { + permission, + resourceRefs: [], + id: uuid.v4(), + }; + + if (resourceRef) { + request[permission.name].resourceRefs.push(resourceRef); + } + } + + const rawRequest = { items: Object.values(request) }; + const permissionApi = await this.discovery.getBaseUrl('permission'); + const response = await fetch(`${permissionApi}/authorize`, { + method: 'POST', + body: JSON.stringify(rawRequest), + headers: { + ...this.getAuthorizationHeader(options?.token), + 'content-type': 'application/json', + }, + }); + if (!response.ok) { + throw await ResponseError.fromResponse(response); + } + + const responseBody = await response.json(); + + const parsedResponse = responseSchema( + z.object({ + result: z.array( + z.literal(AuthorizeResult.ALLOW).or(z.literal(AuthorizeResult.DENY)), + ), + }), + new Set(rawRequest.items.map(({ id }) => id)), + ).parse(responseBody); + + return queries.map(query => { + const { id } = request[query.permission.name]; + + const item = parsedResponse.items.find(i => i.id === id)!; + return { + result: query.resourceRef ? item.result.shift()! : item.result[0], + }; + }); + } + private getAuthorizationHeader(token?: string): Record { return token ? { Authorization: `Bearer ${token}` } : {}; } } + +export type BatchedAuthorizePermissionRequest = IdentifiedPermissionMessage<{ + permission: Permission; + resourceRefs: string[]; +}>; diff --git a/plugins/permission-common/src/types/api.ts b/plugins/permission-common/src/types/api.ts index 072615989b..f70bd2d890 100644 --- a/plugins/permission-common/src/types/api.ts +++ b/plugins/permission-common/src/types/api.ts @@ -176,6 +176,7 @@ export type EvaluatePermissionRequest = { /** * A batch of requests sent to the permission backend. * @public + * @deprecated This type is not used and it will be removed in the future */ export type EvaluatePermissionRequestBatch = PermissionMessageBatch;