backend-common: TokenManager throws for invalid tokens rather than returning boolean

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

docs/tutorials/authenticate-api-requests.md

@@ -95,9 +95,8 @@ async function main() {
         req.cookies['token'];
 
       // Authenticate all requests originating from backends by default
-      const isValidServerToken = await authEnv.tokenManager.validateServerToken(
-        token,
-      );
+      const isValidServerToken =
+        authEnv.tokenManager.validateServerToken(token);
       if (!isValidServerToken) {
         req.user = await identity.authenticate(token);
       }

packages/backend-common/api-report.md

@@ -539,7 +539,7 @@ export class ServerTokenManager implements TokenManager {
   // (undocumented)
   static noop(): ServerTokenManager;
   // (undocumented)
-  validateServerToken(token: string): Promise<boolean>;
+  validateServerToken(token: string): void;
 }
 
 // @public (undocumented)
@@ -608,7 +608,7 @@ export interface TokenManager {
     token: string;
   }>;
   // (undocumented)
-  validateServerToken: (token: string) => Promise<boolean>;
+  validateServerToken: (token: string) => void;
 }
 
 // @public

packages/backend-common/src/tokens/ServerTokenManager.test.ts

@@ -39,31 +39,29 @@ describe('ServerTokenManager', () => {
   });
 
   describe('validateServerToken', () => {
-    it('should return true if token is valid', async () => {
+    it('should not throw if token is valid', async () => {
       const tokenManager = ServerTokenManager.fromConfig(configWithSecret);
       const { token } = await tokenManager.getServerToken();
-      const isValidServerToken = await tokenManager.validateServerToken(token);
-      expect(isValidServerToken).toBe(true);
+      expect(() => tokenManager.validateServerToken(token)).not.toThrow();
     });
 
-    it('should return false if token is invalid', async () => {
+    it('should throw if token is invalid', () => {
       const tokenManager = ServerTokenManager.fromConfig(configWithSecret);
-      const isValidServerToken = await tokenManager.validateServerToken(
-        'random-string',
-      );
-      expect(isValidServerToken).toBe(false);
+      expect(() =>
+        tokenManager.validateServerToken('random-string'),
+      ).toThrowError(/invalid server token/i);
     });
 
-    it('should return true for server tokens created by a different instance using the same secret', async () => {
+    it('should validate server tokens created by a different instance using the same secret', async () => {
       const tokenManager1 = ServerTokenManager.fromConfig(configWithSecret);
       const tokenManager2 = ServerTokenManager.fromConfig(configWithSecret);
 
       const { token } = await tokenManager1.getServerToken();
 
-      expect(await tokenManager2.validateServerToken(token)).toBe(true);
+      expect(() => tokenManager2.validateServerToken(token)).not.toThrow();
     });
 
-    it('should return false for server tokens created by a different instance using a different secret', async () => {
+    it('should throw for server tokens created using a different secret', async () => {
       const tokenManager1 = ServerTokenManager.fromConfig(
         new ConfigReader({ backend: { auth: { secret: 'a1b2c3' } } }),
       );
@@ -73,20 +71,33 @@ describe('ServerTokenManager', () => {
 
       const { token } = await tokenManager1.getServerToken();
 
-      expect(await tokenManager2.validateServerToken(token)).toBe(false);
+      expect(() => tokenManager2.validateServerToken(token)).toThrowError(
+        /invalid server token/i,
+      );
+    });
+  });
+
+  describe('ServerTokenManager.noop', () => {
+    let noopTokenManager: ServerTokenManager;
+
+    beforeEach(() => {
+      noopTokenManager = ServerTokenManager.noop();
     });
 
-    it('should always return true if using noop TokenManager', async () => {
-      const tokenManager = ServerTokenManager.noop();
-      const { token } = await tokenManager.getServerToken();
-      const isValidServerToken0 = await tokenManager.validateServerToken(token);
-      const isValidServerToken1 = await tokenManager.validateServerToken(
-        'random-string',
-      );
-      const isValidServerToken2 = await tokenManager.validateServerToken('');
-      expect(isValidServerToken0).toBe(true);
-      expect(isValidServerToken1).toBe(true);
-      expect(isValidServerToken2).toBe(true);
+    it('should accept tokens it generates', async () => {
+      const { token } = await noopTokenManager.getServerToken();
+
+      expect(() => noopTokenManager.validateServerToken(token)).not.toThrow();
+    });
+
+    it('should accept arbitrary strings', async () => {
+      expect(() =>
+        noopTokenManager.validateServerToken('random-string'),
+      ).not.toThrow();
+    });
+
+    it('should accept empty strings', async () => {
+      expect(() => noopTokenManager.validateServerToken('')).not.toThrow();
     });
   });
 });

packages/backend-common/src/tokens/ServerTokenManager.ts

@@ -16,6 +16,7 @@
 
 import { JWK, JWT } from 'jose';
 import { Config } from '@backstage/config';
+import { AuthenticationError } from '@backstage/errors';
 import { TokenManager } from './types';
 
 export class ServerTokenManager implements TokenManager {
@@ -46,16 +47,16 @@ export class ServerTokenManager implements TokenManager {
     return { token: jwt };
   }
 
-  async validateServerToken(token: string): Promise<boolean> {
+  validateServerToken(token: string): void {
     if (this.key === JWK.None) {
-      return true;
+      return;
     }
 
     try {
       JWT.verify(token, this.key);
-      return true;
+      return;
     } catch (e) {
-      return false;
+      throw new AuthenticationError('Invalid server token');
     }
   }
 }

packages/backend-common/src/tokens/types.ts

@@ -16,5 +16,5 @@
 
 export interface TokenManager {
   getServerToken: () => Promise<{ token: string }>;
-  validateServerToken: (token: string) => Promise<boolean>;
+  validateServerToken: (token: string) => void;
 }

plugins/catalog-backend/src/search/DefaultCatalogCollator.test.ts

@@ -68,7 +68,7 @@ describe('DefaultCatalogCollator', () => {
     };
     mockTokenManager = {
       getServerToken: jest.fn().mockResolvedValue({ token: '' }),
-      validateServerToken: jest.fn().mockResolvedValue(true),
+      validateServerToken: jest.fn(),
     };
     collator = new DefaultCatalogCollator({
       discovery: mockDiscoveryApi,

plugins/techdocs-backend/src/search/DefaultTechDocsCollator.test.ts

@@ -100,7 +100,7 @@ describe('DefaultTechDocsCollator with legacyPathCasing configuration', () => {
     };
     mockTokenManager = {
       getServerToken: jest.fn().mockResolvedValue({ token: '' }),
-      validateServerToken: jest.fn().mockResolvedValue(true),
+      validateServerToken: jest.fn(),
     };
     const mockConfig = new ConfigReader({
       techdocs: {
@@ -166,7 +166,7 @@ describe('DefaultTechDocsCollator', () => {
     };
     mockTokenManager = {
       getServerToken: jest.fn().mockResolvedValue({ token: '' }),
-      validateServerToken: jest.fn().mockResolvedValue(true),
+      validateServerToken: jest.fn(),
     };
     collator = DefaultTechDocsCollator.fromConfig(new ConfigReader({}), {
       discovery: mockDiscoveryApi,