From f898c014ca2ec4eab0b9458c077f18897785fdb5 Mon Sep 17 00:00:00 2001
From: Joon Park <joonp@spotify.com>
Date: Fri, 17 Dec 2021 15:32:09 +0000
Subject: [PATCH] Add explicit instance variable to denote the given token
 manager's scope of authentication

Signed-off-by: Joon Park <joonp@spotify.com>
---
 packages/app-defaults/src/defaults/apis.ts                | 1 -
 packages/backend-common/api-report.md                     | 3 +++
 packages/backend-common/src/tokens/ServerTokenManager.ts  | 3 +++
 packages/backend-common/src/tokens/types.ts               | 6 ++++++
 .../src/search/DefaultCatalogCollator.test.ts             | 1 +
 plugins/permission-node/src/ServerPermissionClient.ts     | 8 +-------
 .../src/search/DefaultTechDocsCollator.test.ts            | 2 ++
 7 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/packages/app-defaults/src/defaults/apis.ts b/packages/app-defaults/src/defaults/apis.ts
index 3500afe32a..f26c298fb1 100644
--- a/packages/app-defaults/src/defaults/apis.ts
+++ b/packages/app-defaults/src/defaults/apis.ts
@@ -61,7 +61,6 @@ import {
   oidcAuthApiRef,
   bitbucketAuthApiRef,
   atlassianAuthApiRef,
-  identityApiRef,
 } from '@backstage/core-plugin-api';
 import {
   permissionApiRef,
diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md
index b929251890..c774e6baa3 100644
--- a/packages/backend-common/api-report.md
+++ b/packages/backend-common/api-report.md
@@ -502,6 +502,8 @@ export class ServerTokenManager implements TokenManager {
     token: string;
   }>;
   // (undocumented)
+  readonly isSecure: boolean;
+  // (undocumented)
   static noop(): TokenManager;
 }
 
@@ -572,6 +574,7 @@ export interface TokenManager {
   getToken: () => Promise<{
     token: string;
   }>;
+  isSecure: boolean;
 }
 
 // @public
diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts
index 7fbd18e8c3..82ef5b8401 100644
--- a/packages/backend-common/src/tokens/ServerTokenManager.ts
+++ b/packages/backend-common/src/tokens/ServerTokenManager.ts
@@ -21,6 +21,8 @@ import { TokenManager } from './types';
 import { Logger } from 'winston';
 
 class NoopTokenManager implements TokenManager {
+  public readonly isSecure: boolean = false;
+
   async getToken() {
     return { token: '' };
   }
@@ -37,6 +39,7 @@ class NoopTokenManager implements TokenManager {
 export class ServerTokenManager implements TokenManager {
   private readonly verificationKeys: JWKS.KeyStore;
   private readonly signingKey: JWK.Key;
+  public readonly isSecure: boolean = true;
 
   static noop(): TokenManager {
     return new NoopTokenManager();
diff --git a/packages/backend-common/src/tokens/types.ts b/packages/backend-common/src/tokens/types.ts
index 1fea018db9..2be1936885 100644
--- a/packages/backend-common/src/tokens/types.ts
+++ b/packages/backend-common/src/tokens/types.ts
@@ -20,6 +20,12 @@
  * @public
  */
 export interface TokenManager {
+  /**
+   * This property should be true when the token manager is expected to only
+   * authenticate tokens created by itself, or an equivalently-constructed
+   * instance.
+   */
+  isSecure: boolean;
   getToken: () => Promise<{ token: string }>;
   authenticate: (token: string) => Promise<void>;
 }
diff --git a/plugins/catalog-backend/src/search/DefaultCatalogCollator.test.ts b/plugins/catalog-backend/src/search/DefaultCatalogCollator.test.ts
index 1360ca2647..81f728b862 100644
--- a/plugins/catalog-backend/src/search/DefaultCatalogCollator.test.ts
+++ b/plugins/catalog-backend/src/search/DefaultCatalogCollator.test.ts
@@ -67,6 +67,7 @@ describe('DefaultCatalogCollator', () => {
       getExternalBaseUrl: jest.fn(),
     };
     mockTokenManager = {
+      isSecure: true,
       getToken: jest.fn().mockResolvedValue({ token: '' }),
       authenticate: jest.fn(),
     };
diff --git a/plugins/permission-node/src/ServerPermissionClient.ts b/plugins/permission-node/src/ServerPermissionClient.ts
index 51c9bcf04f..e9438a9162 100644
--- a/plugins/permission-node/src/ServerPermissionClient.ts
+++ b/plugins/permission-node/src/ServerPermissionClient.ts
@@ -16,7 +16,6 @@
 
 import {
   TokenManager,
-  ServerTokenManager,
   PluginEndpointDiscovery,
 } from '@backstage/backend-common';
 import { Config } from '@backstage/config';
@@ -50,12 +49,7 @@ export class ServerPermissionClient implements PermissionAuthorizer {
     const permissionEnabled =
       config.getOptionalBoolean('permission.enabled') ?? false;
 
-    if (
-      permissionEnabled &&
-      // TODO: Find a cleaner way of ensuring usage of SERVER token manager when
-      // permissions are enabled.
-      tokenManager instanceof ServerTokenManager.noop().constructor
-    ) {
+    if (permissionEnabled && !tokenManager.isSecure) {
       throw new Error(
         'You must configure at least one key in backend.auth.keys if permissions are enabled.',
       );
diff --git a/plugins/techdocs-backend/src/search/DefaultTechDocsCollator.test.ts b/plugins/techdocs-backend/src/search/DefaultTechDocsCollator.test.ts
index 5f0bed55dd..6f38d5291d 100644
--- a/plugins/techdocs-backend/src/search/DefaultTechDocsCollator.test.ts
+++ b/plugins/techdocs-backend/src/search/DefaultTechDocsCollator.test.ts
@@ -99,6 +99,7 @@ describe('DefaultTechDocsCollator with legacyPathCasing configuration', () => {
       getExternalBaseUrl: jest.fn(),
     };
     mockTokenManager = {
+      isSecure: true,
       getToken: jest.fn().mockResolvedValue({ token: '' }),
       authenticate: jest.fn(),
     };
@@ -165,6 +166,7 @@ describe('DefaultTechDocsCollator', () => {
       getExternalBaseUrl: jest.fn(),
     };
     mockTokenManager = {
+      isSecure: true,
       getToken: jest.fn().mockResolvedValue({ token: '' }),
       authenticate: jest.fn(),
     };