permission-common: batching of permissions

Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>

plugins/permission-common/config.d.ts

@@ -23,5 +23,10 @@ export interface Config {
      * @visibility frontend
      */
     enabled?: boolean;
+
+    /**
+     * @visibility frontend
+     */
+    EXPERIMENTAL_enableBatchedRequests?: boolean;
   };
 }

plugins/permission-common/src/PermissionClient.test.ts

@@ -17,7 +17,10 @@
 import { RestContext, rest } from 'msw';
 import { setupServer } from 'msw/node';
 import { ConfigReader } from '@backstage/config';
-import { PermissionClient } from './PermissionClient';
+import {
+  BatchedAuthorizePermissionRequest,
+  PermissionClient,
+} from './PermissionClient';
 import {
   EvaluatePermissionRequest,
   AuthorizeResult,
@@ -36,10 +39,7 @@ const discovery: DiscoveryApi = {
     return mockBaseUrl;
   },
 };
-const client: PermissionClient = new PermissionClient({
-  discovery,
-  config: new ConfigReader({ permission: { enabled: true } }),
-});
+let client: PermissionClient;
 
 const mockPermission = createPermission({
   name: 'test.permission',
@@ -53,6 +53,13 @@ describe('PermissionClient', () => {
   afterEach(() => server.resetHandlers());
 
   describe('authorize', () => {
+    beforeAll(() => {
+      client = new PermissionClient({
+        discovery,
+        config: new ConfigReader({ permission: { enabled: true } }),
+      });
+    });
+
     const mockAuthorizeConditional = {
       permission: mockPermission,
       resourceRef: 'foo:bar',
@@ -211,7 +218,270 @@ describe('PermissionClient', () => {
     });
   });
 
+  describe('authorize (batched)', () => {
+    beforeAll(() => {
+      client = new PermissionClient({
+        discovery,
+        config: new ConfigReader({
+          permission: {
+            enabled: true,
+            EXPERIMENTAL_enableBatchedRequests: true,
+          },
+        }),
+      });
+    });
+
+    const mockAuthorizeConditional = {
+      permission: mockPermission,
+      resourceRef: 'foo:bar',
+    };
+
+    const mockAuthorizeHandler = jest.fn();
+
+    beforeEach(() => {
+      mockAuthorizeHandler.mockReset();
+      server.use(rest.post(`${mockBaseUrl}/authorize`, mockAuthorizeHandler));
+
+      mockAuthorizeHandler.mockImplementation(
+        (req, res, { json }: RestContext) => {
+          const responses = req.body.items.map(
+            (a: BatchedAuthorizePermissionRequest) => ({
+              id: a.id,
+              result: [AuthorizeResult.ALLOW],
+            }),
+          );
+
+          return res(json({ items: responses }));
+        },
+      );
+    });
+
+    afterEach(() => {
+      jest.clearAllMocks();
+    });
+
+    it('should fetch entities from correct endpoint', async () => {
+      await client.authorize([mockAuthorizeConditional]);
+      expect(mockAuthorizeHandler).toHaveBeenCalled();
+    });
+
+    it('should include a request body', async () => {
+      await client.authorize([mockAuthorizeConditional]);
+
+      const request = mockAuthorizeHandler.mock.calls[0][0];
+
+      expect(request.body).toEqual({
+        items: [
+          expect.objectContaining({
+            permission: mockPermission,
+            resourceRefs: ['foo:bar'],
+          }),
+        ],
+      });
+    });
+
+    it('should return the response from the fetch request', async () => {
+      const response = await client.authorize([mockAuthorizeConditional]);
+      expect(response[0]).toEqual(
+        expect.objectContaining({ result: AuthorizeResult.ALLOW }),
+      );
+    });
+
+    it('should not include authorization headers if no token is supplied', async () => {
+      await client.authorize([mockAuthorizeConditional]);
+
+      const request = mockAuthorizeHandler.mock.calls[0][0];
+      expect(request.headers.has('authorization')).toEqual(false);
+    });
+
+    it('should include correctly-constructed authorization header if token is supplied', async () => {
+      await client.authorize([mockAuthorizeConditional], { token });
+
+      const request = mockAuthorizeHandler.mock.calls[0][0];
+      expect(request.headers.get('authorization')).toEqual('Bearer fake-token');
+    });
+
+    it('should forward response errors', async () => {
+      mockAuthorizeHandler.mockImplementationOnce(
+        (_req, res, { status }: RestContext) => {
+          return res(status(401));
+        },
+      );
+      await expect(
+        client.authorize([mockAuthorizeConditional], { token }),
+      ).rejects.toThrow(/request failed with 401/i);
+    });
+
+    it('should reject responses with missing ids', async () => {
+      mockAuthorizeHandler.mockImplementationOnce(
+        (_req, res, { json }: RestContext) => {
+          return res(
+            json({
+              items: [{ id: 'wrong-id', result: [AuthorizeResult.ALLOW] }],
+            }),
+          );
+        },
+      );
+      await expect(
+        client.authorize([mockAuthorizeConditional], { token }),
+      ).rejects.toThrow(/items in response do not match request/i);
+    });
+
+    it('should reject invalid responses', async () => {
+      mockAuthorizeHandler.mockImplementationOnce(
+        (req, res, { json }: RestContext) => {
+          const responses = req.body.items.map(
+            (a: IdentifiedPermissionMessage<EvaluatePermissionRequest>) => ({
+              id: a.id,
+              outcome: AuthorizeResult.ALLOW,
+            }),
+          );
+
+          return res(json({ items: responses }));
+        },
+      );
+      await expect(
+        client.authorize([mockAuthorizeConditional], { token }),
+      ).rejects.toThrow(/invalid_type/i);
+    });
+
+    it('should allow all when permission.enabled is false', async () => {
+      const disabled = new PermissionClient({
+        discovery,
+        config: new ConfigReader({ permission: { enabled: false } }),
+      });
+      const response = await disabled.authorize([mockAuthorizeConditional]);
+      expect(response[0]).toEqual(
+        expect.objectContaining({ result: AuthorizeResult.ALLOW }),
+      );
+      expect(mockAuthorizeHandler).not.toHaveBeenCalled();
+    });
+
+    it('should allow all when permission.enabled is not configured', async () => {
+      const disabled = new PermissionClient({
+        discovery,
+        config: new ConfigReader({}),
+      });
+      const response = await disabled.authorize([mockAuthorizeConditional]);
+      expect(response[0]).toEqual(
+        expect.objectContaining({ result: AuthorizeResult.ALLOW }),
+      );
+      expect(mockAuthorizeHandler).not.toHaveBeenCalled();
+    });
+
+    it('should properly map the permissions', async () => {
+      const mockPermission2 = createPermission({
+        name: 'test.permission2',
+        attributes: {},
+        resourceType: 'foo',
+      });
+
+      const mockPermission3 = createPermission({
+        name: 'test.permission3',
+        attributes: {},
+      });
+
+      mockAuthorizeHandler.mockImplementationOnce(
+        (req, res, { json }: RestContext) => {
+          return res(
+            json({
+              items: [
+                {
+                  id: req.body.items[0].id,
+                  result: [AuthorizeResult.ALLOW, AuthorizeResult.DENY],
+                },
+                {
+                  id: req.body.items[1].id,
+                  result: [AuthorizeResult.DENY],
+                },
+                {
+                  id: req.body.items[2].id,
+                  result: [AuthorizeResult.DENY, AuthorizeResult.ALLOW],
+                },
+              ],
+            }),
+          );
+        },
+      );
+
+      const response = await client.authorize([
+        {
+          permission: mockPermission,
+          resourceRef: 'foo:bar', // allow
+        },
+        {
+          permission: mockPermission3, // deny
+        },
+        {
+          permission: mockPermission,
+          resourceRef: 'foo:car', // deny
+        },
+        {
+          permission: mockPermission3, // deny
+        },
+        {
+          permission: mockPermission2,
+          resourceRef: 'r2', // deny
+        },
+        {
+          permission: mockPermission2,
+          resourceRef: 'r1', // allow
+        },
+      ]);
+
+      expect(mockAuthorizeHandler.mock.calls[0][0].body).toEqual({
+        items: [
+          {
+            permission: {
+              type: 'resource',
+              name: 'test.permission',
+              attributes: {},
+              resourceType: 'foo',
+            },
+            resourceRefs: ['foo:bar', 'foo:car'],
+            id: expect.any(String),
+          },
+          {
+            permission: {
+              type: 'basic',
+              name: 'test.permission3',
+              attributes: {},
+            },
+            resourceRefs: [],
+            id: expect.any(String),
+          },
+          {
+            permission: {
+              type: 'resource',
+              name: 'test.permission2',
+              attributes: {},
+              resourceType: 'foo',
+            },
+            resourceRefs: ['r2', 'r1'],
+            id: expect.any(String),
+          },
+        ],
+      });
+
+      expect(response).toEqual([
+        { result: 'ALLOW' },
+        { result: 'DENY' },
+        { result: 'DENY' },
+        { result: 'DENY' },
+        { result: 'DENY' },
+        { result: 'ALLOW' },
+      ]);
+    });
+  });
+
   describe('authorizeConditional', () => {
+    beforeAll(() => {
+      client = new PermissionClient({
+        discovery,
+        config: new ConfigReader({ permission: { enabled: true } }),
+      });
+    });
+
     const mockResourceAuthorizeConditional = {
       permission: mockPermission,
     };
@@ -420,7 +690,7 @@ describe('PermissionClient', () => {
             }),
           );
 
-          return res(json(responses));
+          return res(json({ items: responses }));
         },
       );
       const disabled = new PermissionClient({

plugins/permission-common/src/PermissionClient.ts

@@ -29,9 +29,10 @@ import {
   AuthorizePermissionRequest,
   AuthorizePermissionResponse,
   QueryPermissionResponse,
+  IdentifiedPermissionMessage,
 } from './types/api';
 import { DiscoveryApi } from './types/discovery';
-import { AuthorizeRequestOptions } from './types/permission';
+import { AuthorizeRequestOptions, Permission } from './types/permission';
 
 const permissionCriteriaSchema: z.ZodSchema<
   PermissionCriteria<PermissionCondition>
@@ -108,11 +109,17 @@ export type PermissionClientRequestOptions = {
 export class PermissionClient implements PermissionEvaluator {
   private readonly enabled: boolean;
   private readonly discovery: DiscoveryApi;
+  private readonly enableBatchedRequests: boolean;
 
   constructor(options: { discovery: DiscoveryApi; config: Config }) {
     this.discovery = options.discovery;
     this.enabled =
       options.config.getOptionalBoolean('permission.enabled') ?? false;
+
+    this.enableBatchedRequests =
+      options.config.getOptionalBoolean(
+        'permission.EXPERIMENTAL_enableBatchedRequests',
+      ) ?? false;
   }
 
   /**
@@ -122,6 +129,10 @@ export class PermissionClient implements PermissionEvaluator {
     requests: AuthorizePermissionRequest[],
     options?: PermissionClientRequestOptions,
   ): Promise<AuthorizePermissionResponse[]> {
+    if (this.enableBatchedRequests) {
+      return this.makeBatchedRequest(requests, options);
+    }
+
     return this.makeRequest(
       requests,
       authorizePermissionResponseSchema,
@@ -183,7 +194,71 @@ export class PermissionClient implements PermissionEvaluator {
     return request.items.map(query => responsesById[query.id]);
   }
 
+  private async makeBatchedRequest(
+    queries: AuthorizePermissionRequest[],
+    options?: AuthorizeRequestOptions,
+  ) {
+    if (!this.enabled) {
+      return queries.map(_ => ({ result: AuthorizeResult.ALLOW as const }));
+    }
+
+    const request: Record<string, BatchedAuthorizePermissionRequest> = {};
+
+    for (const query of queries) {
+      const { permission, resourceRef } = query;
+
+      request[permission.name] ||= {
+        permission,
+        resourceRefs: [],
+        id: uuid.v4(),
+      };
+
+      if (resourceRef) {
+        request[permission.name].resourceRefs.push(resourceRef);
+      }
+    }
+
+    const rawRequest = { items: Object.values(request) };
+    const permissionApi = await this.discovery.getBaseUrl('permission');
+    const response = await fetch(`${permissionApi}/authorize`, {
+      method: 'POST',
+      body: JSON.stringify(rawRequest),
+      headers: {
+        ...this.getAuthorizationHeader(options?.token),
+        'content-type': 'application/json',
+      },
+    });
+    if (!response.ok) {
+      throw await ResponseError.fromResponse(response);
+    }
+
+    const responseBody = await response.json();
+
+    const parsedResponse = responseSchema(
+      z.object({
+        result: z.array(
+          z.literal(AuthorizeResult.ALLOW).or(z.literal(AuthorizeResult.DENY)),
+        ),
+      }),
+      new Set(rawRequest.items.map(({ id }) => id)),
+    ).parse(responseBody);
+
+    return queries.map(query => {
+      const { id } = request[query.permission.name];
+
+      const item = parsedResponse.items.find(i => i.id === id)!;
+      return {
+        result: query.resourceRef ? item.result.shift()! : item.result[0],
+      };
+    });
+  }
+
   private getAuthorizationHeader(token?: string): Record<string, string> {
     return token ? { Authorization: `Bearer ${token}` } : {};
   }
 }
+
+export type BatchedAuthorizePermissionRequest = IdentifiedPermissionMessage<{
+  permission: Permission;
+  resourceRefs: string[];
+}>;

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<EvaluatePermissionRequest>;