Refactor ServerPermissionClient away from inheritance

Signed-off-by: Joon Park <joonp@spotify.com>

plugins/permission-common/src/PermissionClient.ts

@@ -28,6 +28,10 @@ import {
   PermissionCondition,
 } from './types/api';
 import { DiscoveryApi } from './types/discovery';
+import {
+  PermissionClientInterface,
+  AuthorizeRequestOptions,
+} from './types/permission';
 
 const permissionCriteriaSchema: z.ZodSchema<
   PermissionCriteria<PermissionCondition>
@@ -59,20 +63,12 @@ const responseSchema = z.array(
     ),
 );
 
-/**
- * Options for authorization requests; currently only an optional auth token.
- * @public
- */
-export type AuthorizeRequestOptions = {
-  token?: string;
-};
-
 /**
  * An isomorphic client for requesting authorization for Backstage permissions.
  * @public
  */
-export class PermissionClient {
-  protected readonly enabled: boolean;
+export class PermissionClient implements PermissionClientInterface {
+  private readonly enabled: boolean;
   private readonly discoveryApi: DiscoveryApi;
 
   constructor(options: { discoveryApi: DiscoveryApi; configApi: Config }) {
@@ -106,7 +102,7 @@ export class PermissionClient {
     // but no resourceRef. That way clients who aren't prepared to handle filtering according
     // to conditions can be guaranteed that they won't unexpectedly get a CONDITIONAL response.
 
-    if (await this.shouldBypass(options)) {
+    if (!this.enabled) {
       return requests.map(_ => ({ result: AuthorizeResult.ALLOW }));
     }
 
@@ -141,12 +137,6 @@ export class PermissionClient {
     return identifiedRequests.map(request => responsesById[request.id]);
   }
 
-  protected async shouldBypass(
-    _options?: AuthorizeRequestOptions,
-  ): Promise<boolean> {
-    return !this.enabled;
-  }
-
   private getAuthorizationHeader(token?: string): Record<string, string> {
     return token ? { Authorization: `Bearer ${token}` } : {};
   }

plugins/permission-common/src/types/index.ts

@@ -23,4 +23,9 @@ export type {
   PermissionCriteria,
 } from './api';
 export type { DiscoveryApi } from './discovery';
-export type { PermissionAttributes, Permission } from './permission';
+export type {
+  PermissionAttributes,
+  Permission,
+  PermissionClientInterface,
+  AuthorizeRequestOptions,
+} from './permission';

plugins/permission-common/src/types/permission.ts

@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+import { AuthorizeRequest, AuthorizeResponse } from './api';
+
 /**
  * The attributes related to a given permission; these should be generic and widely applicable to
  * all permissions in the system.
@@ -39,3 +41,18 @@ export type Permission = {
   attributes: PermissionAttributes;
   resourceType?: string;
 };
+
+export interface PermissionClientInterface {
+  authorize(
+    requests: AuthorizeRequest[],
+    options?: AuthorizeRequestOptions,
+  ): Promise<AuthorizeResponse[]>;
+}
+
+/**
+ * Options for authorization requests; currently only an optional auth token.
+ * @public
+ */
+export type AuthorizeRequestOptions = {
+  token?: string;
+};

plugins/permission-node/src/ServerPermissionClient.ts

@@ -17,18 +17,25 @@
 import { TokenManager, ServerTokenManager } from '@backstage/backend-common';
 import { Config } from '@backstage/config';
 import {
+  AuthorizeRequest,
   AuthorizeRequestOptions,
+  AuthorizeResponse,
+  AuthorizeResult,
   DiscoveryApi,
   PermissionClient,
+  PermissionClientInterface,
 } from '@backstage/plugin-permission-common';
 
 /**
- * A server side {@link @backstage/plugin-permission-common#PermissionClient}
- * that allows all backend-to-backend requests.
+ * A thin wrapper around
+ * {@link @backstage/plugin-permission-common#PermissionClient} that allows all
+ * backend-to-backend requests.
  * @public
  */
-export class ServerPermissionClient extends PermissionClient {
+export class ServerPermissionClient implements PermissionClientInterface {
   private readonly serverTokenManager: TokenManager;
+  private readonly permissionClient: PermissionClient;
+  private readonly permissionEnabled: boolean;
 
   constructor(options: {
     discoveryApi: DiscoveryApi;
@@ -36,10 +43,12 @@ export class ServerPermissionClient extends PermissionClient {
     serverTokenManager: TokenManager;
   }) {
     const { discoveryApi, configApi, serverTokenManager } = options;
-    super({ discoveryApi, configApi });
+    this.permissionClient = new PermissionClient({ discoveryApi, configApi });
+    this.permissionEnabled =
+      options.configApi.getOptionalBoolean('permission.enabled') ?? false;
 
     if (
-      this.enabled &&
+      this.permissionEnabled &&
       // TODO: Find a cleaner way of ensuring usage of SERVER token manager when
       // permissions are enabled.
       serverTokenManager instanceof ServerTokenManager.noop().constructor
@@ -51,18 +60,21 @@ export class ServerPermissionClient extends PermissionClient {
     this.serverTokenManager = serverTokenManager;
   }
 
-  async shouldBypass(options?: AuthorizeRequestOptions): Promise<boolean> {
-    // Call super first in order to check if permissions are enabled before
-    // validating the server token. That way when permissions are disabled, the
-    // noop token manager can be used without fouling up the logic inside the
-    // ServerPermissionClient, because the code path won't be reached.
-    if (await super.shouldBypass(options)) {
-      return true;
+  async authorize(
+    requests: AuthorizeRequest[],
+    options?: AuthorizeRequestOptions,
+  ): Promise<AuthorizeResponse[]> {
+    // Check if permissions are enabled before validating the server token. That
+    // way when permissions are disabled, the noop token manager can be used
+    // without fouling up the logic inside the ServerPermissionClient, because
+    // the code path won't be reached.
+    if (
+      !this.permissionEnabled ||
+      (await this.isValidServerToken(options?.token))
+    ) {
+      return requests.map(_ => ({ result: AuthorizeResult.ALLOW }));
     }
-    if (await this.isValidServerToken(options?.token)) {
-      return true;
-    }
-    return false;
+    return this.permissionClient.authorize(requests, options);
   }
 
   private async isValidServerToken(