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 68ae5d8ff5..d1f25c1be0 100644 --- a/plugins/permission-backend/src/service/router.test.ts +++ b/plugins/permission-backend/src/service/router.test.ts @@ -165,11 +165,7 @@ describe('createRouter', () => { mockApplyConditions.mockResolvedValueOnce([ { id: '123', - result: AuthorizeResult.ALLOW, - }, - { - id: '123', - result: AuthorizeResult.DENY, + result: [AuthorizeResult.ALLOW, AuthorizeResult.DENY], }, ]); @@ -198,6 +194,21 @@ describe('createRouter', () => { ], }); + expect(mockApplyConditions).toHaveBeenCalledWith( + 'test-plugin', + expect.any(Object), + [ + { + conditions: { params: ['abc'], rule: 'test-rule' }, + id: '123', + pluginId: 'test-plugin', + resourceRefs: ['resource:1', 'resource:2'], + resourceType: 'test-resource-1', + result: 'CONDITIONAL', + }, + ], + ); + expect(response.status).toEqual(200); expect(policy.handle).toHaveBeenCalledWith( diff --git a/plugins/permission-backend/src/service/router.ts b/plugins/permission-backend/src/service/router.ts index f89a83ed51..1d6f1b36d8 100644 --- a/plugins/permission-backend/src/service/router.ts +++ b/plugins/permission-backend/src/service/router.ts @@ -26,7 +26,6 @@ import { isResourcePermission, PermissionAttributes, PermissionMessageBatch, - PolicyDecision, } from '@backstage/plugin-permission-common'; import { ApplyConditionsRequestEntry, @@ -125,9 +124,7 @@ const handleRequest = async ( auth: AuthService, userInfo: UserInfoService, ): Promise< - IdentifiedPermissionMessage< - EvaluatePermissionResponse | BulkDefinitivePolicyDecision - >[] + IdentifiedPermissionMessage[] > => { const applyConditionsLoaderFor = memoize((pluginId: string) => { return new DataLoader< @@ -182,20 +179,11 @@ const handleRequest = async ( } if (request.resourceRefs) { - const results = await Promise.all( - request.resourceRefs.map(resourceRef => - applyConditionsLoaderFor(decision.pluginId).load({ - id: request.id, - resourceRef, - ...decision, - }), - ), - ); - - return { + return applyConditionsLoaderFor(decision.pluginId).load({ id: request.id, - result: results.map(({ result }) => result), - }; + resourceRefs: request.resourceRefs, + ...decision, + }); } if (!request.resourceRef) { @@ -254,9 +242,7 @@ export async function createRouter( '/authorize', async ( req: Request, - res: Response< - PermissionMessageBatch - >, + res: Response>, ) => { const credentials = await httpAuth.credentials(req, { allow: ['user', 'none'], @@ -307,6 +293,8 @@ export async function createRouter( /** * @internal */ -type BulkDefinitivePolicyDecision = { - result: Array; -}; +type InternalEvaluatePermissionResponse = + | EvaluatePermissionResponse + | { + result: Array; + }; diff --git a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.test.ts index dbb5647157..973fb39d45 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 resourceRefs', () => { + 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', + resourceRefs: [ + '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..0d74f8eca5 100644 --- a/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts +++ b/plugins/permission-node/src/integration/createPermissionIntegrationRouter.ts @@ -58,12 +58,22 @@ const permissionCriteriaSchema: z.ZodSchema< const applyConditionsRequestSchema = z.object({ items: z.array( - z.object({ - id: z.string(), - resourceRef: z.string(), - resourceType: z.string(), - conditions: permissionCriteriaSchema, - }), + 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, + }), + ]), ), }); @@ -73,11 +83,20 @@ const applyConditionsRequestSchema = z.object({ * * @public */ -export type ApplyConditionsRequestEntry = IdentifiedPermissionMessage<{ - resourceRef: string; - resourceType: string; - conditions: PermissionCriteria; -}>; +export type ApplyConditionsRequestEntry = IdentifiedPermissionMessage< + | { + resourceRef: string; + resourceRefs?: undefined; + resourceType: string; + conditions: PermissionCriteria; + } + | { + resourceRef?: undefined; + resourceRefs: string[]; + resourceType: string; + conditions: PermissionCriteria; + } +>; /** * A batch of {@link ApplyConditionsRequestEntry} objects. @@ -94,8 +113,12 @@ export type ApplyConditionsRequest = { * * @public */ -export type ApplyConditionsResponseEntry = - IdentifiedPermissionMessage; +export type ApplyConditionsResponseEntry = IdentifiedPermissionMessage< + | DefinitivePolicyDecision + | { + result: Array; + } +>; /** * A batch of {@link ApplyConditionsResponseEntry} objects. @@ -158,6 +181,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 +511,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 +536,31 @@ export function createPermissionIntegrationRouter< requestedType, requests .filter(r => r.resourceType === requestedType) - .map(i => i.resourceRef), + .map(i => + typeof i.resourceRef === 'string' + ? [i.resourceRef] + : i.resourceRefs, + ) + .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: request.resourceRefs + ? request.resourceRefs.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), + ), })), }); },