permission-backend: move dataloader to router

Signed-off-by: MT Lewis <mtlewis@users.noreply.github.com>

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<void>(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);
-    });
   });
 });

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<T extends PolicyDecision = PolicyDecision> = 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<Identified<AuthorizeResponse>[]> {
-    const loaderFor = memoize(
-      (pluginId: string) =>
-        new DataLoader<
-          Identified<ConditionalPolicyDecision & { resourceRef?: string }>,
-          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<ConditionalPolicyDecision>[],
+    decisions: readonly ApplyConditionsRequestEntry[],
     authHeader?: string,
   ): Promise<ApplyConditionsResponse> {
     const endpoint = `${await this.discovery.getBaseUrl(

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<typeof PermissionIntegrationClient>['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<ApplyConditionsResponseEntry['result']>([
-        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',

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<Identified<AuthorizeRequest>[]> = z.array(
   z.object({
@@ -73,43 +76,6 @@ export interface RouterOptions {
   identity: IdentityClient;
 }
 
-const applyPolicy = async (
-  policy: PermissionPolicy,
-  requests: Identified<AuthorizeRequest>[],
-  user: BackstageIdentityResponse | undefined,
-): Promise<Identified<PolicyDecision & { resourceRef?: string }>[]> => {
-  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<AuthorizeRequest>[],
   user: BackstageIdentityResponse | undefined,
@@ -117,11 +83,46 @@ const handleRequest = async (
   permissionIntegrationClient: PermissionIntegrationClient,
   authHeader?: string,
 ): Promise<Identified<AuthorizeResponse>[]> => {
-  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,
+        });
+      }),
+    ),
+  );
 };
 
 /**