Rename all the things

Signed-off-by: Joon Park <joonp@spotify.com>
backstage · Dec 17, 2021 · 0e8ec6d · 0e8ec6d
1 parent d1801d7
commit 0e8ec6d
Show file tree

Hide file tree

Showing 13 changed files with 131 additions and 89 deletions.
diff --git a/.changeset/real-geckos-work.md b/.changeset/real-geckos-work.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-permission-common': patch
+---
+
+Create PermissionAuthorizer interface for PermissionClients
diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts
@@ -304,11 +304,11 @@ export const apis = [
   createApiFactory({
     api: permissionApiRef,
     deps: {
-      discoveryApi: discoveryApiRef,
-      identityApi: identityApiRef,
-      configApi: configApiRef,
+      discovery: discoveryApiRef,
+      identity: identityApiRef,
+      config: configApiRef,
     },
-    factory: ({ configApi, discoveryApi, identityApi }) =>
-      IdentityPermissionApi.create({ configApi, discoveryApi, identityApi }),
+    factory: ({ config, discovery, identity }) =>
+      IdentityPermissionApi.create({ config, discovery, identity }),
   }),
 ];
diff --git a/packages/backend/src/index.ts b/packages/backend/src/index.ts
@@ -64,10 +64,10 @@ function makeCreateEnv(config: Config) {
   const reader = UrlReaders.default({ logger: root, config });
   const discovery = SingleHostDiscovery.fromConfig(config);
   const tokenManager = ServerTokenManager.fromConfig(config, { logger: root });
-  const permissions = new ServerPermissionClient({
-    discoveryApi: discovery,
-    configApi: config,
-    serverTokenManager: tokenManager,
+  const permissions = ServerPermissionClient.create({
+    discovery,
+    config,
+    tokenManager,
   });
 
   root.info(`Created UrlReader ${reader}`);

diff --git a/plugins/permission-common/api-report.md b/plugins/permission-common/api-report.md
@@ -68,16 +68,21 @@ export type PermissionAttributes = {
 };
 
 // @public
-export class PermissionClient {
-  constructor(options: { discoveryApi: DiscoveryApi; configApi: Config });
+export interface PermissionAuthorizer {
+  // (undocumented)
+  authorize(
+    requests: AuthorizeRequest[],
+    options?: AuthorizeRequestOptions,
+  ): Promise<AuthorizeResponse[]>;
+}
+
+// @public
+export class PermissionClient implements PermissionAuthorizer {
+  constructor(options: { discovery: DiscoveryApi; config: Config });
   authorize(
     requests: AuthorizeRequest[],
     options?: AuthorizeRequestOptions,
   ): Promise<AuthorizeResponse[]>;
-  // (undocumented)
-  protected readonly enabled: boolean;
-  // (undocumented)
-  protected shouldBypass(_options?: AuthorizeRequestOptions): Promise<boolean>;
 }
 
 // @public

diff --git a/plugins/permission-common/src/PermissionClient.test.ts b/plugins/permission-common/src/PermissionClient.test.ts
@@ -26,14 +26,14 @@ const server = setupServer();
 const token = 'fake-token';
 
 const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base';
-const discoveryApi: DiscoveryApi = {
+const discovery: DiscoveryApi = {
   async getBaseUrl() {
     return mockBaseUrl;
   },
 };
 const client: PermissionClient = new PermissionClient({
-  discoveryApi,
-  configApi: new ConfigReader({ permission: { enabled: true } }),
+  discovery,
+  config: new ConfigReader({ permission: { enabled: true } }),
 });
 
 const mockPermission: Permission = {
@@ -158,8 +158,8 @@ describe('PermissionClient', () => {
         },
       );
       const disabled = new PermissionClient({
-        discoveryApi,
-        configApi: new ConfigReader({ permission: { enabled: false } }),
+        discovery,
+        config: new ConfigReader({ permission: { enabled: false } }),
       });
       const response = await disabled.authorize([mockAuthorizeRequest]);
       expect(response[0]).toEqual(
@@ -180,8 +180,8 @@ describe('PermissionClient', () => {
         },
       );
       const disabled = new PermissionClient({
-        discoveryApi,
-        configApi: new ConfigReader({}),
+        discovery,
+        config: new ConfigReader({}),
       });
       const response = await disabled.authorize([mockAuthorizeRequest]);
       expect(response[0]).toEqual(

diff --git a/plugins/permission-common/src/PermissionClient.ts b/plugins/permission-common/src/PermissionClient.ts
@@ -29,7 +29,7 @@ import {
 } from './types/api';
 import { DiscoveryApi } from './types/discovery';
 import {
-  PermissionClientInterface,
+  PermissionAuthorizer,
   AuthorizeRequestOptions,
 } from './types/permission';
 
@@ -67,14 +67,14 @@ const responseSchema = z.array(
  * An isomorphic client for requesting authorization for Backstage permissions.
  * @public
  */
-export class PermissionClient implements PermissionClientInterface {
+export class PermissionClient implements PermissionAuthorizer {
   private readonly enabled: boolean;
-  private readonly discoveryApi: DiscoveryApi;
+  private readonly discovery: DiscoveryApi;
 
-  constructor(options: { discoveryApi: DiscoveryApi; configApi: Config }) {
-    this.discoveryApi = options.discoveryApi;
+  constructor(options: { discovery: DiscoveryApi; config: Config }) {
+    this.discovery = options.discovery;
     this.enabled =
-      options.configApi.getOptionalBoolean('permission.enabled') ?? false;
+      options.config.getOptionalBoolean('permission.enabled') ?? false;
   }
 
   /**
@@ -113,7 +113,7 @@ export class PermissionClient implements PermissionClientInterface {
       }),
     );
 
-    const permissionApi = await this.discoveryApi.getBaseUrl('permission');
+    const permissionApi = await this.discovery.getBaseUrl('permission');
     const response = await fetch(`${permissionApi}/authorize`, {
       method: 'POST',
       body: JSON.stringify(identifiedRequests),

diff --git a/plugins/permission-common/src/types/index.ts b/plugins/permission-common/src/types/index.ts
@@ -26,6 +26,6 @@ export type { DiscoveryApi } from './discovery';
 export type {
   PermissionAttributes,
   Permission,
-  PermissionClientInterface,
+  PermissionAuthorizer,
   AuthorizeRequestOptions,
 } from './permission';
diff --git a/plugins/permission-common/src/types/permission.ts b/plugins/permission-common/src/types/permission.ts
@@ -42,15 +42,19 @@ export type Permission = {
   resourceType?: string;
 };
 
-export interface PermissionClientInterface {
+/**
+ * A client interacting with the permission backend can implement this authorizer interface.
+ * @public
+ */
+export interface PermissionAuthorizer {
   authorize(
     requests: AuthorizeRequest[],
     options?: AuthorizeRequestOptions,
   ): Promise<AuthorizeResponse[]>;
 }
 
 /**
- * Options for authorization requests; currently only an optional auth token.
+ * Options for authorization requests.
  * @public
  */
 export type AuthorizeRequestOptions = {

diff --git a/plugins/permission-node/api-report.md b/plugins/permission-node/api-report.md
@@ -5,13 +5,14 @@
 ```ts
 import { AuthorizeRequest } from '@backstage/plugin-permission-common';
 import { AuthorizeRequestOptions } from '@backstage/plugin-permission-common';
+import { AuthorizeResponse } from '@backstage/plugin-permission-common';
 import { AuthorizeResult } from '@backstage/plugin-permission-common';
 import { BackstageIdentityResponse } from '@backstage/plugin-auth-backend';
 import { Config } from '@backstage/config';
-import { DiscoveryApi } from '@backstage/plugin-permission-common';
-import { PermissionClient } from '@backstage/plugin-permission-common';
+import { PermissionAuthorizer } from '@backstage/plugin-permission-common';
 import { PermissionCondition } from '@backstage/plugin-permission-common';
 import { PermissionCriteria } from '@backstage/plugin-permission-common';
+import { PluginEndpointDiscovery } from '@backstage/backend-common';
 import { Router } from 'express';
 import { TokenManager } from '@backstage/backend-common';
 
@@ -126,13 +127,17 @@ export type PolicyDecision =
   | ConditionalPolicyDecision;
 
 // @public
-export class ServerPermissionClient extends PermissionClient {
-  constructor(options: {
-    discoveryApi: DiscoveryApi;
-    configApi: Config;
-    serverTokenManager: TokenManager;
-  });
+export class ServerPermissionClient implements PermissionAuthorizer {
   // (undocumented)
-  shouldBypass(options?: AuthorizeRequestOptions): Promise<boolean>;
+  authorize(
+    requests: AuthorizeRequest[],
+    options?: AuthorizeRequestOptions,
+  ): Promise<AuthorizeResponse[]>;
+  // (undocumented)
+  static create(options: {
+    discovery: PluginEndpointDiscovery;
+    config: Config;
+    tokenManager: TokenManager;
+  }): ServerPermissionClient;
 }
 ```
diff --git a/plugins/permission-node/src/ServerPermissionClient.test.ts b/plugins/permission-node/src/ServerPermissionClient.test.ts
@@ -16,14 +16,17 @@
 
 import { ServerPermissionClient } from '.';
 import {
-  DiscoveryApi,
   Permission,
   Identified,
   AuthorizeRequest,
   AuthorizeResult,
 } from '@backstage/plugin-permission-common';
 import { ConfigReader } from '@backstage/config';
-import { getVoidLogger, ServerTokenManager } from '@backstage/backend-common';
+import {
+  getVoidLogger,
+  PluginEndpointDiscovery,
+  ServerTokenManager,
+} from '@backstage/backend-common';
 import { setupServer } from 'msw/node';
 import { RestContext, rest } from 'msw';
 
@@ -37,10 +40,13 @@ const mockAuthorizeHandler = jest.fn((req, res, { json }: RestContext) => {
   return res(json(responses));
 });
 const mockBaseUrl = 'http://backstage:9191/i-am-a-mock-base';
-const discoveryApi: DiscoveryApi = {
+const discovery: PluginEndpointDiscovery = {
   async getBaseUrl() {
     return mockBaseUrl;
   },
+  async getExternalBaseUrl() {
+    return mockBaseUrl;
+  },
 };
 const testPermission: Permission = {
   name: 'test.permission',
@@ -62,10 +68,10 @@ describe('ServerPermissionClient', () => {
   afterEach(() => server.resetHandlers());
 
   it('should bypass authorization if permissions are disabled', async () => {
-    const client = new ServerPermissionClient({
-      discoveryApi,
-      configApi: new ConfigReader({}),
-      serverTokenManager: ServerTokenManager.noop(),
+    const client = ServerPermissionClient.create({
+      discovery,
+      config: new ConfigReader({}),
+      tokenManager: ServerTokenManager.noop(),
     });
 
     await client.authorize([{ permission: testPermission }]);
@@ -75,10 +81,10 @@ describe('ServerPermissionClient', () => {
 
   it('should bypass authorization if permissions are enabled and request has valid server token', async () => {
     const tokenManager = ServerTokenManager.fromConfig(config, { logger });
-    const client = new ServerPermissionClient({
-      discoveryApi,
-      configApi: config,
-      serverTokenManager: tokenManager,
+    const client = ServerPermissionClient.create({
+      discovery,
+      config,
+      tokenManager,
     });
 
     await client.authorize([{ permission: testPermission }], {
@@ -90,10 +96,10 @@ describe('ServerPermissionClient', () => {
 
   it('should authorize normally if permissions are enabled and request does not have valid server token', async () => {
     const tokenManager = ServerTokenManager.fromConfig(config, { logger });
-    const client = new ServerPermissionClient({
-      discoveryApi,
-      configApi: config,
-      serverTokenManager: tokenManager,
+    const client = ServerPermissionClient.create({
+      discovery,
+      config,
+      tokenManager,
     });
 
     await client.authorize([{ permission: testPermission }], {
@@ -104,13 +110,12 @@ describe('ServerPermissionClient', () => {
   });
 
   it('should error if permissions are enabled but a no-op token manager is configured', async () => {
-    expect(
-      () =>
-        new ServerPermissionClient({
-          discoveryApi,
-          configApi: config,
-          serverTokenManager: ServerTokenManager.noop(),
-        }),
+    expect(() =>
+      ServerPermissionClient.create({
+        discovery,
+        config,
+        tokenManager: ServerTokenManager.noop(),
+      }),
     ).toThrowError(
       'You must configure at least one key in backend.auth.keys if permissions are enabled.',
     );