permission-backend: extract client for calling other plugins to apply conditions

Also adds response validation

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

plugins/permission-backend/package.json

@@ -30,7 +30,8 @@
     "express": "^4.17.1",
     "express-promise-router": "^4.1.0",
     "winston": "^3.2.1",
-    "yn": "^4.0.0"
+    "yn": "^4.0.0",
+    "zod": "^3.11.6"
   },
   "devDependencies": {
     "@backstage/cli": "^0.9.0",

plugins/permission-backend/src/service/PermissionIntegrationClient.test.ts

@@ -0,0 +1,173 @@
+/*
+ * Copyright 2021 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { RestContext, rest } from 'msw';
+import { setupServer } from 'msw/node';
+import { PluginEndpointDiscovery } from '@backstage/backend-common';
+import { AuthorizeResult } from '@backstage/plugin-permission-common';
+import { PermissionIntegrationClient } from './PermissionIntegrationClient';
+
+const server = setupServer();
+
+const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base';
+const discovery: PluginEndpointDiscovery = {
+  async getBaseUrl() {
+    return mockBaseUrl;
+  },
+  async getExternalBaseUrl() {
+    throw new Error('Not implemented.');
+  },
+};
+
+const client: PermissionIntegrationClient = new PermissionIntegrationClient({
+  discovery,
+});
+
+const mockConditions = {
+  not: {
+    allOf: [
+      { rule: 'RULE_1', params: [] },
+      { rule: 'RULE_2', params: ['abc'] },
+    ],
+  },
+};
+
+describe('PermissionIntegrationClient', () => {
+  beforeAll(() => server.listen({ onUnhandledRequest: 'error' }));
+  afterAll(() => server.close());
+  afterEach(() => server.resetHandlers());
+
+  describe('applyConditions', () => {
+    const mockApplyConditionsHandler = jest.fn(
+      (_req, res, { json }: RestContext) => {
+        return res(json({ result: AuthorizeResult.ALLOW }));
+      },
+    );
+
+    beforeEach(() => {
+      server.use(
+        rest.post(
+          `${mockBaseUrl}/permissions/apply-conditions`,
+          mockApplyConditionsHandler,
+        ),
+      );
+    });
+
+    afterEach(() => {
+      jest.clearAllMocks();
+    });
+
+    it('should make a POST request to the correct endpoint', async () => {
+      await client.applyConditions('testResource1', {
+        pluginId: 'test-plugin',
+        resourceType: 'test-resource',
+        conditions: mockConditions,
+      });
+
+      expect(mockApplyConditionsHandler).toHaveBeenCalled();
+    });
+
+    it('should include a request body', async () => {
+      await client.applyConditions('testResource1', {
+        pluginId: 'test-plugin',
+        resourceType: 'test-resource',
+        conditions: mockConditions,
+      });
+
+      expect(mockApplyConditionsHandler).toHaveBeenCalledWith(
+        expect.objectContaining({
+          body: {
+            resourceRef: 'testResource1',
+            resourceType: 'test-resource',
+            conditions: mockConditions,
+          },
+        }),
+        expect.anything(),
+        expect.anything(),
+      );
+    });
+
+    it('should return the response from the fetch request', async () => {
+      const response = await client.applyConditions('testResource1', {
+        pluginId: 'test-plugin',
+        resourceType: 'test-resource',
+        conditions: mockConditions,
+      });
+
+      expect(response).toEqual(
+        expect.objectContaining({ result: AuthorizeResult.ALLOW }),
+      );
+    });
+
+    it('should not include authorization headers if no token is supplied', async () => {
+      await client.applyConditions('testResource1', {
+        pluginId: 'test-plugin',
+        resourceType: 'test-resource',
+        conditions: mockConditions,
+      });
+
+      const request = mockApplyConditionsHandler.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.applyConditions(
+        'testResource1',
+        {
+          pluginId: 'test-plugin',
+          resourceType: 'test-resource',
+          conditions: mockConditions,
+        },
+        'Bearer fake-token',
+      );
+
+      const request = mockApplyConditionsHandler.mock.calls[0][0];
+      expect(request.headers.get('authorization')).toEqual('Bearer fake-token');
+    });
+
+    it('should forward response errors', async () => {
+      mockApplyConditionsHandler.mockImplementationOnce(
+        (_req, res, { status }: RestContext) => {
+          return res(status(401));
+        },
+      );
+
+      await expect(
+        client.applyConditions('testResource1', {
+          pluginId: 'test-plugin',
+          resourceType: 'test-resource',
+          conditions: mockConditions,
+        }),
+      ).rejects.toThrowError(/request failed with 401/i);
+    });
+
+    it('should reject invalid responses', async () => {
+      mockApplyConditionsHandler.mockImplementationOnce(
+        (_req, res, { json }: RestContext) => {
+          return res(json({ outcome: AuthorizeResult.ALLOW }));
+        },
+      );
+
+      await expect(
+        client.applyConditions('testResource1', {
+          pluginId: 'test-plugin',
+          resourceType: 'test-resource',
+          conditions: mockConditions,
+        }),
+      ).rejects.toThrowError(/invalid input/i);
+    });
+  });
+});

plugins/permission-backend/src/service/PermissionIntegrationClient.ts

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2021 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import fetch from 'cross-fetch';
+import { z } from 'zod';
+import { ResponseError } from '@backstage/errors';
+import { PluginEndpointDiscovery } from '@backstage/backend-common';
+import {
+  AuthorizeResult,
+  PermissionCondition,
+  PermissionCriteria,
+} from '@backstage/plugin-permission-common';
+import {
+  ApplyConditionsRequest,
+  ApplyConditionsResponse,
+} from '@backstage/plugin-permission-node';
+
+const responseSchema = z.object({
+  result: z.literal(AuthorizeResult.ALLOW).or(z.literal(AuthorizeResult.DENY)),
+});
+
+export class PermissionIntegrationClient {
+  private readonly discovery: PluginEndpointDiscovery;
+
+  constructor(options: { discovery: PluginEndpointDiscovery }) {
+    this.discovery = options.discovery;
+  }
+
+  async applyConditions(
+    resourceRef: string,
+    conditions: {
+      pluginId: string;
+      resourceType: string;
+      conditions: PermissionCriteria<PermissionCondition>;
+    },
+    authHeader?: string,
+  ): Promise<ApplyConditionsResponse> {
+    const endpoint = `${await this.discovery.getBaseUrl(
+      conditions.pluginId,
+    )}/permissions/apply-conditions`;
+
+    const request: ApplyConditionsRequest = {
+      resourceRef,
+      resourceType: conditions.resourceType,
+      conditions: conditions.conditions,
+    };
+
+    const response = await fetch(endpoint, {
+      method: 'POST',
+      body: JSON.stringify(request),
+      headers: {
+        ...(authHeader ? { authorization: authHeader } : {}),
+        'content-type': 'application/json',
+      },
+    });
+
+    if (!response.ok) {
+      throw await ResponseError.fromResponse(response);
+    }
+
+    return responseSchema.parse(await response.json());
+  }
+}

plugins/permission-backend/src/service/router.ts

@@ -19,7 +19,6 @@ import {
   SingleHostDiscovery,
   PluginEndpointDiscovery,
 } from '@backstage/backend-common';
-import fetch from 'cross-fetch';
 import express, { Request, Response } from 'express';
 import Router from 'express-promise-router';
 import { Logger } from 'winston';
@@ -28,19 +27,15 @@ import {
   IdentityClient,
 } from '@backstage/plugin-auth-backend';
 import { Config } from '@backstage/config';
-import { ConflictError, ResponseError } from '@backstage/errors';
+import { ConflictError } from '@backstage/errors';
 import {
   AuthorizeResult,
   AuthorizeResponse,
   AuthorizeRequest,
   Identified,
-  PermissionCondition,
-  PermissionCriteria,
 } from '@backstage/plugin-permission-common';
-import {
-  ApplyConditionsRequest,
-  PermissionPolicy,
-} from '@backstage/plugin-permission-node';
+import { PermissionPolicy } from '@backstage/plugin-permission-node';
+import { PermissionIntegrationClient } from './PermissionIntegrationClient';
 
 export interface RouterOptions {
   logger: Logger;
@@ -48,49 +43,11 @@ export interface RouterOptions {
   policy: PermissionPolicy;
 }
 
-// TODO(permission-backend): probably move this to a separate client
-const applyConditions = async (
-  resourceRef: string,
-  conditions: {
-    pluginId: string;
-    resourceType: string;
-    conditions: PermissionCriteria<PermissionCondition>;
-  },
-  discoveryApi: PluginEndpointDiscovery,
-  authHeader?: string,
-): Promise<{ result: AuthorizeResult.ALLOW | AuthorizeResult.DENY }> => {
-  const endpoint = `${await discoveryApi.getBaseUrl(
-    conditions.pluginId,
-  )}/permissions/apply-conditions`;
-
-  const request: ApplyConditionsRequest = {
-    resourceRef,
-    resourceType: conditions.resourceType,
-    conditions: conditions.conditions,
-  };
-
-  const response = await fetch(endpoint, {
-    method: 'POST',
-    body: JSON.stringify(request),
-    headers: {
-      ...(authHeader ? { authorization: authHeader } : {}),
-      'content-type': 'application/json',
-    },
-  });
-
-  if (!response.ok) {
-    throw await ResponseError.fromResponse(response);
-  }
-
-  // TODO(permission-backend): validate response
-  return await response.json();
-};
-
 const handleRequest = async (
   { id, resourceRef, ...request }: Identified<AuthorizeRequest>,
   user: BackstageIdentity | undefined,
   policy: PermissionPolicy,
-  discoveryApi: PluginEndpointDiscovery,
+  permissionIntegrationClient: PermissionIntegrationClient,
   authHeader?: string,
 ): Promise<Identified<AuthorizeResponse>> => {
   const response = await policy.handle(request, user);
@@ -106,10 +63,9 @@ const handleRequest = async (
     if (resourceRef) {
       return {
         id,
-        ...(await applyConditions(
+        ...(await permissionIntegrationClient.applyConditions(
           resourceRef,
           response.conditions,
-          discoveryApi,
           authHeader,
         )),
       };
@@ -135,6 +91,10 @@ export async function createRouter(
     issuer: await discovery.getExternalBaseUrl('auth'),
   });
 
+  const permissionIntegrationClient = new PermissionIntegrationClient({
+    discovery,
+  });
+
   const router = Router();
   router.use(express.json());
 
@@ -160,7 +120,7 @@ export async function createRouter(
               request,
               user,
               policy,
-              discovery,
+              permissionIntegrationClient,
               req.header('authorization'),
             ),
           ),