diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts index cc1de05127..814ccd3f38 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts @@ -77,11 +77,9 @@ describe('PermissionIntegrationClient', () => { }); it('should make a POST request to the correct endpoint', async () => { - await client.applyConditions([ + await client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, @@ -92,11 +90,9 @@ describe('PermissionIntegrationClient', () => { }); it('should include a request body', async () => { - await client.applyConditions([ + await client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, @@ -120,11 +116,9 @@ describe('PermissionIntegrationClient', () => { }); it('should return the response from the fetch request', async () => { - const response = await client.applyConditions([ + const response = await client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, @@ -137,11 +131,9 @@ describe('PermissionIntegrationClient', () => { }); it('should not include authorization headers if no token is supplied', async () => { - await client.applyConditions([ + await client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, @@ -154,11 +146,10 @@ describe('PermissionIntegrationClient', () => { it('should include correctly-constructed authorization header if token is supplied', async () => { await client.applyConditions( + 'plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, @@ -179,11 +170,9 @@ describe('PermissionIntegrationClient', () => { ); await expect( - client.applyConditions([ + client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, @@ -200,11 +189,9 @@ describe('PermissionIntegrationClient', () => { ); await expect( - client.applyConditions([ + client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, @@ -227,27 +214,21 @@ describe('PermissionIntegrationClient', () => { ); await expect( - client.applyConditions([ + client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }, { id: '456', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, }, { id: '789', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: mockConditions, @@ -266,8 +247,7 @@ describe('PermissionIntegrationClient', () => { describe('integration with @backstage/plugin-permission-node', () => { let server: Server; let client: PermissionIntegrationClient; - let plugin1Router: RequestHandler; - let plugin2Router: RequestHandler; + let routerSpy: RequestHandler; beforeAll(async () => { const router = Router(); @@ -302,11 +282,9 @@ describe('PermissionIntegrationClient', () => { const app = express(); - plugin1Router = jest.fn(router); - plugin2Router = jest.fn(router); + routerSpy = jest.fn(router); - app.use('/plugin-1', plugin1Router); - app.use('/plugin-2', plugin2Router); + app.use('/plugin-1', routerSpy); await new Promise(resolve => { server = app.listen(resolve); @@ -341,11 +319,9 @@ describe('PermissionIntegrationClient', () => { it('works for simple conditions', async () => { await expect( - client.applyConditions([ + client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: { rule: 'RULE_1', params: ['no'] }, @@ -356,11 +332,9 @@ describe('PermissionIntegrationClient', () => { it('works for complex criteria', async () => { await expect( - client.applyConditions([ + client.applyConditions('plugin-1', [ { id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', resourceRef: 'testResource1', resourceType: 'test-resource', conditions: { @@ -385,180 +359,5 @@ describe('PermissionIntegrationClient', () => { ]), ).resolves.toEqual([{ id: '123', result: AuthorizeResult.ALLOW }]); }); - - it('makes separate batched requests to multiple plugin backends', async () => { - await expect( - client.applyConditions([ - { - id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['yes'] }, - }, - { - id: '234', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-2', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['no'] }, - }, - { - id: '345', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['no'] }, - }, - { - id: '456', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-2', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['yes'] }, - }, - ]), - ).resolves.toEqual([ - { id: '123', result: AuthorizeResult.ALLOW }, - { id: '234', result: AuthorizeResult.DENY }, - { id: '345', result: AuthorizeResult.DENY }, - { id: '456', result: AuthorizeResult.ALLOW }, - ]); - - expect(plugin1Router).toHaveBeenCalledTimes(1); - expect(plugin2Router).toHaveBeenCalledTimes(1); - }); - - it('leaves definitive results unchanged', async () => { - await expect( - client.applyConditions([ - { - id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['yes'] }, - }, - { - id: '234', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-2', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['no'] }, - }, - { - id: '345', - result: AuthorizeResult.ALLOW, - }, - { - id: '456', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['no'] }, - }, - { - id: '567', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-2', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['yes'] }, - }, - ]), - ).resolves.toEqual([ - { id: '123', result: AuthorizeResult.ALLOW }, - { id: '234', result: AuthorizeResult.DENY }, - { id: '345', result: AuthorizeResult.ALLOW }, - { id: '456', result: AuthorizeResult.DENY }, - { id: '567', result: AuthorizeResult.ALLOW }, - ]); - - expect(plugin1Router).toHaveBeenCalledTimes(1); - expect(plugin2Router).toHaveBeenCalledTimes(1); - }); - - it('leaves conditional results without resourceRefs unchanged', async () => { - await expect( - client.applyConditions([ - { - id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['yes'] }, - }, - { - id: '234', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-2', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['no'] }, - }, - { - id: '345', - result: AuthorizeResult.ALLOW, - }, - { - id: '456', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-1', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['no'] }, - }, - { - id: '567', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-2', - resourceRef: 'testResource1', - resourceType: 'test-resource', - conditions: { rule: 'RULE_1', params: ['yes'] }, - }, - { - id: '789', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-2', - resourceType: 'test-resource', - conditions: { - anyOf: [ - { rule: 'RULE_1', params: ['yes'] }, - { rule: 'RULE_2', params: ['yes'] }, - ], - }, - }, - ]), - ).resolves.toEqual([ - { id: '123', result: AuthorizeResult.ALLOW }, - { id: '234', result: AuthorizeResult.DENY }, - { id: '345', result: AuthorizeResult.ALLOW }, - { id: '456', result: AuthorizeResult.DENY }, - { id: '567', result: AuthorizeResult.ALLOW }, - { - id: '789', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'plugin-2', - resourceType: 'test-resource', - conditions: { - anyOf: [ - { rule: 'RULE_1', params: ['yes'] }, - { rule: 'RULE_2', params: ['yes'] }, - ], - }, - }, - ]); - - expect(plugin1Router).toHaveBeenCalledTimes(1); - expect(plugin2Router).toHaveBeenCalledTimes(1); - }); }); }); diff --git a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts index 520e8b19ae..f4fdb6cbfc 100644 --- a/plugins/permission-backend/src/service/PermissionIntegrationClient.ts +++ b/plugins/permission-backend/src/service/PermissionIntegrationClient.ts @@ -14,21 +14,14 @@ * limitations under the License. */ -import { memoize } from 'lodash'; import fetch from 'node-fetch'; import { z } from 'zod'; -import DataLoader from 'dataloader'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; +import { AuthorizeResult } from '@backstage/plugin-permission-common'; import { - AuthorizeResponse, - AuthorizeResult, - Identified, -} from '@backstage/plugin-permission-common'; -import { + ApplyConditionsRequestEntry, ApplyConditionsResponse, - ApplyConditionsResponseEntry, ConditionalPolicyDecision, - PolicyDecision, } from '@backstage/plugin-permission-node'; const responseSchema = z.array( @@ -40,11 +33,9 @@ const responseSchema = z.array( }), ); -type ResourceDecision = Identified< - T & { - resourceRef?: string; - } ->; +export type ResourcePolicyDecision = ConditionalPolicyDecision & { + resourceRef: string; +}; export class PermissionIntegrationClient { private readonly discovery: PluginEndpointDiscovery; @@ -54,34 +45,8 @@ export class PermissionIntegrationClient { } async applyConditions( - decisions: ResourceDecision[], - authHeader?: string, - ): Promise[]> { - const loaderFor = memoize( - (pluginId: string) => - new DataLoader< - Identified, - ApplyConditionsResponseEntry - >(requests => this.makeRequest(pluginId, requests, authHeader)), - ); - - return Promise.all( - decisions.map(decision => { - if ( - decision.result !== AuthorizeResult.CONDITIONAL || - !decision.resourceRef - ) { - return decision; - } - - return loaderFor(decision.pluginId).load(decision); - }), - ); - } - - private async makeRequest( pluginId: string, - decisions: readonly ResourceDecision[], + decisions: readonly ApplyConditionsRequestEntry[], authHeader?: string, ): Promise { const endpoint = `${await this.discovery.getBaseUrl( diff --git a/plugins/permission-backend/src/service/router.test.ts b/plugins/permission-backend/src/service/router.test.ts index ef165b2ac5..971cb6a223 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -19,32 +19,28 @@ import request from 'supertest'; import { getVoidLogger } from '@backstage/backend-common'; import { IdentityClient } from '@backstage/plugin-auth-backend'; import { AuthorizeResult } from '@backstage/plugin-permission-common'; -import { ApplyConditionsResponseEntry } from '@backstage/plugin-permission-node'; +import { + ApplyConditionsRequestEntry, + ApplyConditionsResponseEntry, +} from '@backstage/plugin-permission-node'; import { PermissionIntegrationClient } from './PermissionIntegrationClient'; import { createRouter } from './router'; const mockApplyConditions: jest.MockedFunction< InstanceType['applyConditions'] -> = jest.fn(async decisions => - decisions.map(decision => { - if ( - decision.result === AuthorizeResult.CONDITIONAL && - decision.resourceRef - ) { - return { id: decision.id, result: AuthorizeResult.DENY as const }; - } - - if (decision.result === AuthorizeResult.CONDITIONAL) { - return { - id: decision.id, - result: decision.result, - conditions: decision.conditions, - }; - } - - return decision; - }), +> = jest.fn( + async ( + _pluginId: string, + decisions: readonly ApplyConditionsRequestEntry[], + ) => + decisions.map(decision => ({ + id: decision.id, + result: + (decision.conditions as any).params[0] === 'yes' + ? (AuthorizeResult.ALLOW as const) + : (AuthorizeResult.DENY as const), + })), ); jest.mock('./PermissionIntegrationClient', () => ({ @@ -90,6 +86,10 @@ describe('createRouter', () => { app = express().use(router); }); + afterEach(() => { + jest.clearAllMocks(); + }); + describe('GET /health', () => { it('returns ok', async () => { const response = await request(app).get('/health'); @@ -178,18 +178,14 @@ describe('createRouter', () => { }); describe('conditional policy result', () => { - beforeEach(() => { - policy.handle.mockResolvedValue({ + it('returns conditions if no resourceRef is supplied', async () => { + policy.handle.mockResolvedValueOnce({ result: AuthorizeResult.CONDITIONAL, pluginId: 'test-plugin', resourceType: 'test-resource-1', - conditions: { - anyOf: [{ rule: 'test-rule', params: ['abc'] }], - }, + conditions: { rule: 'test-rule', params: ['abc'] }, }); - }); - it('returns conditions if no resourceRef is supplied', async () => { const response = await request(app) .post('/authorize') .send([ @@ -208,17 +204,388 @@ describe('createRouter', () => { { id: '123', result: AuthorizeResult.CONDITIONAL, - conditions: { anyOf: [{ rule: 'test-rule', params: ['abc'] }] }, + pluginId: 'test-plugin', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['abc'] }, }, ]); }); - it.each([ - AuthorizeResult.ALLOW, - AuthorizeResult.DENY, + it('makes separate batched requests to multiple plugin backends', async () => { + policy.handle + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-1', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['yes'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-2', + resourceType: 'test-resource-2', + conditions: { rule: 'test-rule', params: ['yes'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-1', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['no'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-2', + resourceType: 'test-resource-2', + conditions: { rule: 'test-rule', params: ['no'] }, + }); + + const response = await request(app) + .post('/authorize') + .auth('test-token', { type: 'bearer' }) + .send([ + { + id: '123', + permission: { + name: 'test.permission.1', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:1', + }, + { + id: '234', + permission: { + name: 'test.permission.2', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:2', + }, + { + id: '345', + permission: { + name: 'test.permission.3', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:3', + }, + { + id: '456', + permission: { + name: 'test.permission.4', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:4', + }, + ]); + + expect(mockApplyConditions).toHaveBeenCalledWith( + 'plugin-1', + [ + expect.objectContaining({ + id: '123', + resourceType: 'test-resource-1', + resourceRef: 'resource:1', + conditions: { rule: 'test-rule', params: ['yes'] }, + }), + expect.objectContaining({ + id: '345', + resourceType: 'test-resource-1', + resourceRef: 'resource:3', + conditions: { rule: 'test-rule', params: ['no'] }, + }), + ], + 'Bearer test-token', + ); + + expect(mockApplyConditions).toHaveBeenCalledWith( + 'plugin-2', + [ + expect.objectContaining({ + id: '234', + resourceType: 'test-resource-2', + resourceRef: 'resource:2', + conditions: { rule: 'test-rule', params: ['yes'] }, + }), + expect.objectContaining({ + id: '456', + resourceType: 'test-resource-2', + resourceRef: 'resource:4', + conditions: { rule: 'test-rule', params: ['no'] }, + }), + ], + 'Bearer test-token', + ); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { id: '123', result: AuthorizeResult.ALLOW }, + { id: '234', result: AuthorizeResult.ALLOW }, + { id: '345', result: AuthorizeResult.DENY }, + { id: '456', result: AuthorizeResult.DENY }, + ]); + }); + + it('leaves definitive results unchanged', async () => { + policy.handle + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-1', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['no'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-2', + resourceType: 'test-resource-2', + conditions: { rule: 'test-rule', params: ['no'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.ALLOW, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-1', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['yes'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-2', + resourceType: 'test-resource-2', + conditions: { rule: 'test-rule', params: ['yes'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.DENY, + }); + + const response = await request(app) + .post('/authorize') + .auth('test-token', { type: 'bearer' }) + .send([ + { + id: '123', + permission: { + name: 'test.permission.1', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:1', + }, + { + id: '234', + permission: { + name: 'test.permission.2', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:2', + }, + { + id: '345', + permission: { + name: 'test.permission.3', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:3', + }, + { + id: '456', + permission: { + name: 'test.permission.4', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:4', + }, + { + id: '567', + permission: { + name: 'test.permission.5', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:5', + }, + { + id: '678', + permission: { + name: 'test.permission.6', + attributes: {}, + }, + }, + ]); + + expect(mockApplyConditions).toHaveBeenCalledWith( + 'plugin-1', + [ + expect.objectContaining({ + id: '123', + resourceType: 'test-resource-1', + resourceRef: 'resource:1', + conditions: { rule: 'test-rule', params: ['no'] }, + }), + expect.objectContaining({ + id: '456', + resourceType: 'test-resource-1', + resourceRef: 'resource:4', + conditions: { rule: 'test-rule', params: ['yes'] }, + }), + ], + 'Bearer test-token', + ); + + expect(mockApplyConditions).toHaveBeenCalledWith( + 'plugin-2', + [ + expect.objectContaining({ + id: '234', + resourceType: 'test-resource-2', + resourceRef: 'resource:2', + conditions: { rule: 'test-rule', params: ['no'] }, + }), + expect.objectContaining({ + id: '567', + resourceType: 'test-resource-2', + resourceRef: 'resource:5', + conditions: { rule: 'test-rule', params: ['yes'] }, + }), + ], + 'Bearer test-token', + ); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { id: '123', result: AuthorizeResult.DENY }, + { id: '234', result: AuthorizeResult.DENY }, + { id: '345', result: AuthorizeResult.ALLOW }, + { id: '456', result: AuthorizeResult.ALLOW }, + { id: '567', result: AuthorizeResult.ALLOW }, + { id: '678', result: AuthorizeResult.DENY }, + ]); + }); + + it('leaves conditional results without resourceRefs unchanged', async () => { + policy.handle + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-1', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['yes'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-2', + resourceType: 'test-resource-2', + conditions: { rule: 'test-rule', params: ['yes'] }, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.ALLOW, + }) + .mockResolvedValueOnce({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-1', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['abc'] }, + }); + + const response = await request(app) + .post('/authorize') + .auth('test-token', { type: 'bearer' }) + .send([ + { + id: '123', + permission: { + name: 'test.permission.1', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:1', + }, + { + id: '234', + permission: { + name: 'test.permission.2', + resourceType: 'test-resource-2', + attributes: {}, + }, + resourceRef: 'resource:2', + }, + { + id: '345', + permission: { + name: 'test.permission.3', + resourceType: 'test-resource-1', + attributes: {}, + }, + resourceRef: 'resource:3', + }, + { + id: '456', + permission: { + name: 'test.permission.4', + resourceType: 'test-resource-1', + attributes: {}, + }, + }, + ]); + + expect(mockApplyConditions).toHaveBeenCalledWith( + 'plugin-1', + [ + expect.objectContaining({ + id: '123', + resourceType: 'test-resource-1', + resourceRef: 'resource:1', + conditions: { rule: 'test-rule', params: ['yes'] }, + }), + ], + 'Bearer test-token', + ); + + expect(mockApplyConditions).toHaveBeenCalledWith( + 'plugin-2', + [ + expect.objectContaining({ + id: '234', + resourceType: 'test-resource-2', + resourceRef: 'resource:2', + conditions: { rule: 'test-rule', params: ['yes'] }, + }), + ], + 'Bearer test-token', + ); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { id: '123', result: AuthorizeResult.ALLOW }, + { id: '234', result: AuthorizeResult.ALLOW }, + { id: '345', result: AuthorizeResult.ALLOW }, + { + id: '456', + result: AuthorizeResult.CONDITIONAL, + pluginId: 'plugin-1', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params: ['abc'] }, + }, + ]); + }); + + it.each<[ApplyConditionsResponseEntry['result'], string]>([ + [AuthorizeResult.ALLOW, 'yes'], + [AuthorizeResult.DENY, 'no'], ])( 'applies conditions and returns %s if resourceRef is supplied', - async result => { + async (result, params) => { + policy.handle.mockResolvedValue({ + result: AuthorizeResult.CONDITIONAL, + pluginId: 'test-plugin', + resourceType: 'test-resource-1', + conditions: { rule: 'test-rule', params }, + }); + mockApplyConditions.mockResolvedValueOnce([ { id: '123', @@ -255,21 +622,19 @@ describe('createRouter', () => { ]); expect(mockApplyConditions).toHaveBeenCalledWith( + 'test-plugin', [ expect.objectContaining({ id: '123', - result: AuthorizeResult.CONDITIONAL, - pluginId: 'test-plugin', resourceType: 'test-resource-1', resourceRef: 'test/resource', - conditions: { anyOf: [{ rule: 'test-rule', params: ['abc'] }] }, + conditions: { rule: 'test-rule', params }, }), expect.objectContaining({ id: '234', - pluginId: 'test-plugin', resourceType: 'test-resource-1', resourceRef: 'test/resource', - conditions: { anyOf: [{ rule: 'test-rule', params: ['abc'] }] }, + conditions: { rule: 'test-rule', params }, }), ], 'Bearer test-token', diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index 0e877bc816..58438afbb5 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -34,10 +34,13 @@ import { Identified, } from '@backstage/plugin-permission-common'; import { + ApplyConditionsRequestEntry, + ApplyConditionsResponseEntry, PermissionPolicy, - PolicyDecision, } from '@backstage/plugin-permission-node'; import { PermissionIntegrationClient } from './PermissionIntegrationClient'; +import { memoize } from 'lodash'; +import DataLoader from 'dataloader'; const requestSchema: z.ZodSchema[]> = z.array( z.object({ @@ -73,43 +76,6 @@ export interface RouterOptions { identity: IdentityClient; } -const applyPolicy = async ( - policy: PermissionPolicy, - requests: Identified[], - user: BackstageIdentityResponse | undefined, -): Promise[]> => { - return Promise.all( - requests.map(({ id, resourceRef, ...authorizeRequest }) => - policy.handle(authorizeRequest, user).then(decision => ({ - id, - ...(decision.result === AuthorizeResult.CONDITIONAL - ? { resourceRef } - : {}), - ...decision, - })), - ), - ); -}; - -const assertMatchingResourceTypes = ( - requests: AuthorizeRequest[], - decisions: PolicyDecision[], -) => { - requests.forEach((request, index) => { - const decision = decisions[index]; - - // Sanity check that any resource provided matches the one expected by the permission - if ( - decision.result === AuthorizeResult.CONDITIONAL && - decision.resourceType !== request.permission.resourceType - ) { - throw new Error( - `Invalid resource conditions returned from permission policy for permission ${request.permission.name}`, - ); - } - }); -}; - const handleRequest = async ( requests: Identified[], user: BackstageIdentityResponse | undefined, @@ -117,11 +83,46 @@ const handleRequest = async ( permissionIntegrationClient: PermissionIntegrationClient, authHeader?: string, ): Promise[]> => { - const decisions = await applyPolicy(policy, requests, user); + const applyConditionsLoaderFor = memoize((pluginId: string) => { + return new DataLoader< + ApplyConditionsRequestEntry, + ApplyConditionsResponseEntry + >(batch => + permissionIntegrationClient.applyConditions(pluginId, batch, authHeader), + ); + }); - assertMatchingResourceTypes(requests, decisions); + return Promise.all( + requests.map(({ id, resourceRef, ...request }) => + policy.handle(request, user).then(decision => { + if (decision.result !== AuthorizeResult.CONDITIONAL) { + return { + id, + ...decision, + }; + } - return permissionIntegrationClient.applyConditions(decisions, authHeader); + 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, + }); + }), + ), + ); }; /**