diff --git a/.changeset/lazy-tires-show.md b/.changeset/lazy-tires-show.md new file mode 100644 index 0000000000..3ccf5f155e --- /dev/null +++ b/.changeset/lazy-tires-show.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-permission-backend': minor +'@backstage/plugin-permission-common': minor +'@backstage/plugin-permission-node': minor +--- + +Fixed an issue causing the `PermissionClient` to exhaust the request body size limit too quickly when making many requests. diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts index 21d2f66887..610eb8b34b 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts @@ -34,9 +34,16 @@ const responseSchema = z.object({ items: z.array( z.object({ id: z.string(), - result: z - .literal(AuthorizeResult.ALLOW) - .or(z.literal(AuthorizeResult.DENY)), + result: z.union([ + z.literal(AuthorizeResult.ALLOW), + z.literal(AuthorizeResult.DENY), + z.array( + z.union([ + z.literal(AuthorizeResult.ALLOW), + z.literal(AuthorizeResult.DENY), + ]), + ), + ]), }), ), }); @@ -45,6 +52,9 @@ export type ResourcePolicyDecision = ConditionalPolicyDecision & { resourceRef: string; }; +/** + * @internal + */ export class PermissionIntegrationClient { private readonly discovery: DiscoveryService; private readonly auth: AuthService; @@ -74,14 +84,7 @@ export class PermissionIntegrationClient { const response = await fetch(endpoint, { method: 'POST', body: JSON.stringify({ - items: decisions.map( - ({ id, resourceRef, resourceType, conditions }) => ({ - id, - resourceRef, - resourceType, - conditions, - }), - ), + items: decisions, }), headers: { ...(token ? { authorization: `Bearer ${token}` } : {}), diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index 9c8fa4ad5f..386ae694ed 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -154,6 +154,95 @@ describe('createRouter', () => { }); }); + it('calls the permission policy with batched resourceRef as an array', async () => { + policy.handle.mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'test-plugin', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['abc'] }, + }); + + mockApplyConditions.mockResolvedValueOnce([ + { + id: '123', + result: [AuthorizeResult.ALLOW, AuthorizeResult.DENY], + }, + ]); + + const response = await request(app) + .post('/authorize') + .send({ + items: [ + { + id: '123', + permission: { + type: 'resource', + name: 'test.permission1', + attributes: {}, + resourceType: 'test-resource-1', + }, + resourceRef: ['resource:1', 'resource:2'], + }, + { + id: '234', + permission: { + type: 'basic', + name: 'test.permission2', + attributes: {}, + }, + }, + ], + }); + + expect(mockApplyConditions).toHaveBeenCalledWith( + 'test-plugin', + expect.any(Object), + [ + { + conditions: { params: ['abc'], rule: 'test-rule' }, + id: '123', + pluginId: 'test-plugin', + resourceRef: ['resource:1', 'resource:2'], + resourceType: 'test-resource-1', + result: 'CONDITIONAL', + }, + ], + ); + + expect(response.status).toEqual(200); + + expect(policy.handle).toHaveBeenCalledWith( + { + permission: { + type: 'resource', + name: 'test.permission1', + attributes: {}, + resourceType: 'test-resource-1', + }, + }, + undefined, + ); + expect(policy.handle).toHaveBeenCalledWith( + { + permission: { + type: 'basic', + name: 'test.permission2', + attributes: {}, + }, + }, + undefined, + ); + + expect(policy.handle).toHaveBeenCalledTimes(2); + + expect(response.body).toEqual({ + items: [ + { id: '123', result: [AuthorizeResult.ALLOW, AuthorizeResult.DENY] }, + { id: '234', result: AuthorizeResult.DENY }, + ], + }); + }); + it('resolves identity from the Authorization header', async () => { const response = await request(app) .post('/authorize') @@ -522,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, @@ -792,8 +881,20 @@ describe('createRouter', () => { items: [ { id: '123', - // resource ref should be a string resourceRef: ['resource:1'], + permission: { + type: 'basic', + name: 'test.permission', + attributes: {}, + }, + }, + ], + }, + { + items: [ + { + id: '123', + resourceRef: [], permission: { type: 'resource', name: 'test.permission', @@ -803,17 +904,11 @@ describe('createRouter', () => { }, ], }, - ])('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); - expect(response.body).toEqual( - expect.objectContaining({ - error: expect.objectContaining({ - message: expect.stringMatching(/invalid/i), - }), - }), - ); + expect(response.body.error.name).toEqual('InputError'); }); it('returns a 500 error if the policy returns a different resourceType', async () => { diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 0759f6df3a..b6f570562b 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -21,13 +21,11 @@ import { InputError } from '@backstage/errors'; import { IdentityApi } from '@backstage/plugin-auth-node'; import { AuthorizeResult, - EvaluatePermissionRequest, - EvaluatePermissionRequestBatch, EvaluatePermissionResponse, - EvaluatePermissionResponseBatch, IdentifiedPermissionMessage, isResourcePermission, PermissionAttributes, + PermissionMessageBatch, } from '@backstage/plugin-permission-common'; import { ApplyConditionsRequestEntry, @@ -74,9 +72,7 @@ const resourcePermissionSchema = z.object({ resourceType: z.string(), }); -const evaluatePermissionRequestSchema: z.ZodSchema< - IdentifiedPermissionMessage -> = z.union([ +const evaluatePermissionRequestSchema = z.union([ z.object({ id: z.string(), resourceRef: z.undefined().optional(), @@ -84,15 +80,16 @@ const evaluatePermissionRequestSchema: z.ZodSchema< }), z.object({ id: z.string(), - resourceRef: z.string().optional(), + resourceRef: z + .union([z.string(), z.array(z.string()).nonempty()]) + .optional(), permission: resourcePermissionSchema, }), ]); -const evaluatePermissionRequestBatchSchema: z.ZodSchema = - z.object({ - items: z.array(evaluatePermissionRequestSchema), - }); +const evaluatePermissionRequestBatchSchema = z.object({ + items: z.array(evaluatePermissionRequestSchema), +}); /** * Options required when constructing a new {@link express#Router} using @@ -112,7 +109,7 @@ export interface RouterOptions { } const handleRequest = async ( - requests: IdentifiedPermissionMessage[], + requests: z.infer['items'], policy: PermissionPolicy, permissionIntegrationClient: PermissionIntegrationClient, credentials: BackstageCredentials< @@ -120,7 +117,9 @@ const handleRequest = async ( >, auth: AuthService, userInfo: UserInfoService, -): Promise[]> => { +): Promise< + IdentifiedPermissionMessage[] +> => { const applyConditionsLoaderFor = memoize((pluginId: string) => { return new DataLoader< ApplyConditionsRequestEntry, @@ -150,40 +149,42 @@ const handleRequest = async ( } return Promise.all( - requests.map(({ id, resourceRef, ...request }) => - policy.handle(request, user).then(decision => { - if (decision.result !== AuthorizeResult.CONDITIONAL) { - return { - id, + requests.map(request => + policy + .handle({ permission: request.permission }, user) + .then(async decision => { + if (decision.result !== AuthorizeResult.CONDITIONAL) { + return { + id: request.id, + ...decision, + }; + } + + if (!isResourcePermission(request.permission)) { + throw new Error( + `Conditional decision returned from permission policy for non-resource permission ${request.permission.name}`, + ); + } + + if (decision.resourceType !== request.permission.resourceType) { + throw new Error( + `Invalid resource conditions returned from permission policy for permission ${request.permission.name}`, + ); + } + + if (!request.resourceRef) { + return { + id: request.id, + ...decision, + }; + } + + return applyConditionsLoaderFor(decision.pluginId).load({ + id: request.id, + resourceRef: request.resourceRef, ...decision, - }; - } - - if (!isResourcePermission(request.permission)) { - throw new Error( - `Conditional decision returned from permission policy for non-resource permission ${request.permission.name}`, - ); - } - - if (decision.resourceType !== request.permission.resourceType) { - throw new Error( - `Invalid resource conditions returned from permission policy for permission ${request.permission.name}`, - ); - } - - if (!resourceRef) { - return { - id, - ...decision, - }; - } - - return applyConditionsLoaderFor(decision.pluginId).load({ - id, - resourceRef, - ...decision, - }); - }), + }); + }), ), ); }; @@ -226,8 +227,8 @@ export async function createRouter( router.post( '/authorize', async ( - req: Request, - res: Response, + req: Request, + res: Response>, ) => { const credentials = await httpAuth.credentials(req, { allow: ['user', 'none'], @@ -274,3 +275,12 @@ export async function createRouter( return router; } + +/** + * @internal + */ +type InternalEvaluatePermissionResponse = + | EvaluatePermissionResponse + | { + result: Array; + }; 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/report.api.md b/plugins/permission-common/report.api.md index 465c43846f..ff3c03efb5 100644 --- a/plugins/permission-common/report.api.md +++ b/plugins/permission-common/report.api.md @@ -83,7 +83,7 @@ export type EvaluatePermissionRequest = { resourceRef?: string; }; -// @public +// @public @deprecated export type EvaluatePermissionRequestBatch = PermissionMessageBatch; diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts index 72707bf98a..b55e5da94d 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,284 @@ 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 () => { + const basicPermission = createPermission({ + name: 'test.permission-basic', + attributes: {}, + }); + + await client.authorize([ + { permission: mockPermission, resourceRef: 'foo:bar' }, + { permission: mockPermission, resourceRef: 'foo:car' }, + { permission: mockPermission, resourceRef: 'foo:baz' }, + { permission: basicPermission }, + ]); + + const request = mockAuthorizeHandler.mock.calls[0][0]; + + expect(request.body).toEqual({ + items: [ + { + id: expect.any(String), + permission: mockPermission, + resourceRef: ['foo:bar', 'foo:car', 'foo:baz'], + }, + { + id: expect.any(String), + permission: basicPermission, + }, + ], + }); + }); + + 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', + }, + resourceRef: ['foo:bar', 'foo:car'], + id: expect.any(String), + }, + { + permission: { + type: 'basic', + name: 'test.permission3', + attributes: {}, + }, + id: expect.any(String), + }, + { + permission: { + type: 'resource', + name: 'test.permission2', + attributes: {}, + resourceType: 'foo', + }, + resourceRef: ['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 +704,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..93c3d5152f 100644 --- a/plugins/permission-common/src/PermissionClient.ts +++ b/plugins/permission-common/src/PermissionClient.ts @@ -29,9 +29,15 @@ import { AuthorizePermissionRequest, AuthorizePermissionResponse, QueryPermissionResponse, + IdentifiedPermissionMessage, } from './types/api'; import { DiscoveryApi } from './types/discovery'; -import { AuthorizeRequestOptions } from './types/permission'; +import { + AuthorizeRequestOptions, + BasicPermission, + ResourcePermission, +} from './types/permission'; +import { isResourcePermission } from './permissions'; const permissionCriteriaSchema: z.ZodSchema< PermissionCriteria @@ -54,6 +60,15 @@ const authorizePermissionResponseSchema: z.ZodSchema = z.union([ z.object({ @@ -108,11 +123,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 +143,14 @@ export class PermissionClient implements PermissionEvaluator { requests: AuthorizePermissionRequest[], options?: PermissionClientRequestOptions, ): Promise { + if (!this.enabled) { + return requests.map(_ => ({ result: AuthorizeResult.ALLOW as const })); + } + + if (this.enableBatchedRequests) { + return this.makeBatchedRequest(requests, options); + } + return this.makeRequest( requests, authorizePermissionResponseSchema, @@ -136,6 +165,10 @@ export class PermissionClient implements PermissionEvaluator { queries: QueryPermissionRequest[], options?: PermissionClientRequestOptions, ): Promise { + if (!this.enabled) { + return queries.map(_ => ({ result: AuthorizeResult.ALLOW as const })); + } + return this.makeRequest(queries, queryPermissionResponseSchema, options); } @@ -144,10 +177,6 @@ export class PermissionClient implements PermissionEvaluator { itemSchema: z.ZodSchema, options?: AuthorizeRequestOptions, ) { - if (!this.enabled) { - return queries.map(_ => ({ result: AuthorizeResult.ALLOW as const })); - } - const request: PermissionMessageBatch = { items: queries.map(query => ({ id: uuid.v4(), @@ -155,6 +184,73 @@ export class PermissionClient implements PermissionEvaluator { })), }; + const parsedResponse = await this.makeRawRequest( + request, + itemSchema, + options, + ); + + const responsesById = parsedResponse.items.reduce((acc, r) => { + acc[r.id] = r; + return acc; + }, {} as Record>); + + return request.items.map(query => responsesById[query.id]); + } + + private async makeBatchedRequest( + queries: AuthorizePermissionRequest[], + options?: AuthorizeRequestOptions, + ) { + const request: Record = {}; + + for (const query of queries) { + const { permission, resourceRef } = query; + + if (isResourcePermission(permission)) { + request[permission.name] ||= { + permission, + resourceRef: [], + id: uuid.v4(), + }; + + if (resourceRef) { + request[permission.name].resourceRef?.push(resourceRef); + } + } else { + request[permission.name] ||= { + permission, + id: uuid.v4(), + }; + } + } + + const parsedResponse = await this.makeRawRequest( + { items: Object.values(request) }, + authorizePermissionResponseBatchSchema, + 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 = responsesById[id]; + return { + result: query.resourceRef ? item.result.shift()! : item.result[0], + }; + }); + } + + private async makeRawRequest( + request: PermissionMessageBatch, + itemSchema: z.ZodSchema, + options?: AuthorizeRequestOptions, + ) { const permissionApi = await this.discovery.getBaseUrl('permission'); const response = await fetch(`${permissionApi}/authorize`, { method: 'POST', @@ -170,20 +266,24 @@ export class PermissionClient implements PermissionEvaluator { const responseBody = await response.json(); - const parsedResponse = responseSchema( + return responseSchema( itemSchema, new Set(request.items.map(({ id }) => id)), ).parse(responseBody); - - const responsesById = parsedResponse.items.reduce((acc, r) => { - acc[r.id] = r; - return acc; - }, {} as Record>); - - return request.items.map(query => responsesById[query.id]); } private getAuthorizationHeader(token?: string): Record { return token ? { Authorization: `Bearer ${token}` } : {}; } } + +/** + * @internal + */ +export type BatchedAuthorizePermissionRequest = IdentifiedPermissionMessage< + | { + permission: BasicPermission; + resourceRef?: undefined; + } + | { permission: ResourcePermission; resourceRef: string[] } +>; diff --git a/plugins/permission-common/src/index.ts b/plugins/permission-common/src/index.ts index 7c3f6dd613..3c28921a39 100644 --- a/plugins/permission-common/src/index.ts +++ b/plugins/permission-common/src/index.ts @@ -21,4 +21,7 @@ */ export * from './types'; export * from './permissions'; -export * from './PermissionClient'; +export { + PermissionClient, + type PermissionClientRequestOptions, +} from './PermissionClient'; 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; diff --git a/plugins/permission-node/report.api.md b/plugins/permission-node/report.api.md index 8c743a0f6c..6345861c93 100644 --- a/plugins/permission-node/report.api.md +++ b/plugins/permission-node/report.api.md @@ -7,6 +7,7 @@ import { AllOfCriteria } from '@backstage/plugin-permission-common'; import { AnyOfCriteria } from '@backstage/plugin-permission-common'; import { AuthorizePermissionRequest } from '@backstage/plugin-permission-common'; import { AuthorizePermissionResponse } from '@backstage/plugin-permission-common'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { AuthService } from '@backstage/backend-plugin-api'; import { BackstageCredentials } from '@backstage/backend-plugin-api'; import { BackstageUserIdentity } from '@backstage/plugin-auth-node'; @@ -38,7 +39,7 @@ export type ApplyConditionsRequest = { // @public export type ApplyConditionsRequestEntry = IdentifiedPermissionMessage<{ - resourceRef: string; + resourceRef: string | string[]; resourceType: string; conditions: PermissionCriteria; }>; @@ -49,8 +50,12 @@ export type ApplyConditionsResponse = { }; // @public -export type ApplyConditionsResponseEntry = - IdentifiedPermissionMessage; +export type ApplyConditionsResponseEntry = IdentifiedPermissionMessage< + | DefinitivePolicyDecision + | { + result: Array; + } +>; // @public export type Condition = TRule extends PermissionRule< diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index dbb5647157..0d28ef710f 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts @@ -567,6 +567,101 @@ describe('createPermissionIntegrationRouter', () => { }); }); + describe('batched requests with resourceRef as an array', () => { + let response: Response; + + beforeEach(async () => { + const app = express().use( + createPermissionIntegrationRouter(mockedOptionResources).use( + middleware.error(), + ), + ); + + mockTestRule1Apply.mockReturnValueOnce(true); + mockTestRule1Apply.mockReturnValueOnce(false); + mockTestRule1Apply.mockReturnValueOnce(false); + + response = await request(app) + .post('/.well-known/backstage/permissions/apply-conditions') + .send({ + items: [ + { + id: '123', + resourceRef: [ + 'default:test/resource-1', + 'default:test/resource-2', + ], + resourceType: 'test-resource', + conditions: { + rule: 'test-rule-1', + resourceType: 'test-resource', + params: { + foo: 'a', + bar: 1, + }, + }, + }, + { + id: '234', + resourceRef: 'default:test/resource-3', + resourceType: 'test-resource', + conditions: { + rule: 'test-rule-1', + resourceType: 'test-resource', + params: { + foo: 'a', + bar: 1, + }, + }, + }, + { + id: '345', + resourceRef: 'default:test/resource-2', + resourceType: 'test-resource-2', + conditions: { + not: { + rule: 'test-rule-1', + resourceType: 'test-resource-2', + params: { + foo: 'a', + bar: 1, + }, + }, + }, + }, + ], + }); + }); + + it('processes batched requests', () => { + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + items: [ + { + id: '123', + result: [AuthorizeResult.ALLOW, AuthorizeResult.DENY], + }, + { + id: '234', + result: AuthorizeResult.DENY, + }, + { id: '345', result: AuthorizeResult.ALLOW }, + ], + }); + }); + + it('calls getResources for all required resources at once', () => { + expect(defaultMockedGetResources1).toHaveBeenCalledWith([ + 'default:test/resource-1', + 'default:test/resource-2', + 'default:test/resource-3', + ]); + expect(defaultMockedGetResources2).toHaveBeenCalledWith([ + 'default:test/resource-2', + ]); + }); + }); + it('returns 400 when called with incorrect resource type', async () => { const response = await request(createApp()) .post('/.well-known/backstage/permissions/apply-conditions') diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts index cf68af3a3e..4ee937866b 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -60,7 +60,7 @@ const applyConditionsRequestSchema = z.object({ items: z.array( z.object({ id: z.string(), - resourceRef: z.string(), + resourceRef: z.union([z.string(), z.array(z.string()).nonempty()]), resourceType: z.string(), conditions: permissionCriteriaSchema, }), @@ -74,7 +74,7 @@ const applyConditionsRequestSchema = z.object({ * @public */ export type ApplyConditionsRequestEntry = IdentifiedPermissionMessage<{ - resourceRef: string; + resourceRef: string | string[]; resourceType: string; conditions: PermissionCriteria; }>; @@ -94,8 +94,12 @@ export type ApplyConditionsRequest = { * * @public */ -export type ApplyConditionsResponseEntry = - IdentifiedPermissionMessage; +export type ApplyConditionsResponseEntry = IdentifiedPermissionMessage< + | DefinitivePolicyDecision + | { + result: Array; + } +>; /** * A batch of {@link ApplyConditionsResponseEntry} objects. @@ -158,6 +162,16 @@ const applyConditions = ( return rule.apply(resource, criteria.params ?? {}); }; +function authorizeResult( + criteria: PermissionCriteria>, + resource: TResource | undefined, + getRule: (name: string) => PermissionRule, +) { + return applyConditions(criteria, resource, getRule) + ? AuthorizeResult.ALLOW + : AuthorizeResult.DENY; +} + /** * Takes some permission conditions and returns a definitive authorization result * on the resource to which they apply. @@ -478,7 +492,7 @@ export function createPermissionIntegrationRouter< router.post( '/.well-known/backstage/permissions/apply-conditions', - async (req, res: Response) => { + async (req, res: Response) => { const parseResult = applyConditionsRequestSchema.safeParse(req.body); if (!parseResult.success) { throw new InputError(parseResult.error.toString()); @@ -503,20 +517,27 @@ export function createPermissionIntegrationRouter< requestedType, requests .filter(r => r.resourceType === requestedType) - .map(i => i.resourceRef), + .map(i => i.resourceRef) + .flat(), ); } res.json({ items: requests.map(request => ({ id: request.id, - result: applyConditions( - request.conditions, - resourcesByType[request.resourceType][request.resourceRef], - store.getRuleMapper(request.resourceType), - ) - ? AuthorizeResult.ALLOW - : AuthorizeResult.DENY, + result: Array.isArray(request.resourceRef) + ? request.resourceRef.map(resourceRef => + authorizeResult( + request.conditions, + resourcesByType[request.resourceType][resourceRef], + store.getRuleMapper(request.resourceType), + ), + ) + : authorizeResult( + request.conditions, + resourcesByType[request.resourceType][request.resourceRef], + store.getRuleMapper(request.resourceType), + ), })), }); },