backend-common: prevent expired backend tokens from being used

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/metal-items-mix.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': minor
+---
+
+**BREAKING**: Server-to-server tokens that are authenticated by the `ServerTokenManager` now must have an `exp` claim that has not expired. Tokens where the `exp` claim is in the past or missing are considered invalid and will throw an error. This is a followup to the deprecation from the `1.2` release of Backstage where perpetual tokens were deprecated. Be sure to update any usage of the `getToken()` method to have it be called every time a token is needed. Do not store tokens for later use.

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

@@ -171,7 +171,6 @@ describe('ServerTokenManager', () => {
 
     it('should throw for expired tokens, and re-issue new ones', async () => {
       jest.useFakeTimers();
-      const warn = jest.spyOn(logger, 'warn');
 
       const tokenManager = ServerTokenManager.fromConfig(configWithSecret, {
         logger,
@@ -179,14 +178,12 @@ describe('ServerTokenManager', () => {
 
       const { token: token1 } = await tokenManager.getToken();
       await expect(tokenManager.authenticate(token1)).resolves.not.toThrow();
-      expect(warn.mock.calls.length).toBe(0);
 
       // Right before the reissue timeout, it still returns the same token
       jest.advanceTimersByTime(9 * 60 * 1000);
       const { token: token1Again } = await tokenManager.getToken();
       expect(token1).toEqual(token1Again);
       await expect(tokenManager.authenticate(token1)).resolves.not.toThrow();
-      expect(warn.mock.calls.length).toBe(0);
 
       // Right after the reissue timeout, the old ones are still valid but returning a new token
       jest.advanceTimersByTime(2 * 60 * 1000);
@@ -194,14 +191,47 @@ describe('ServerTokenManager', () => {
       expect(token1).not.toEqual(token2);
       await expect(tokenManager.authenticate(token1)).resolves.not.toThrow();
       await expect(tokenManager.authenticate(token2)).resolves.not.toThrow();
-      expect(warn.mock.calls.length).toBe(0);
 
       // After expiry of the first one, it gets warnings but the newest one is still valid
       jest.advanceTimersByTime(52 * 60 * 1000);
-      await expect(tokenManager.authenticate(token1)).resolves.not.toThrow();
+      await expect(tokenManager.authenticate(token1)).rejects.toThrow(
+        'Invalid server token; caused by JWTExpired: "exp" claim timestamp check failed',
+      );
       await expect(tokenManager.authenticate(token2)).resolves.not.toThrow();
-      expect(warn.mock.calls[0][0]).toMatchInlineSnapshot(
-        '"#### DEPRECATION WARNING: #### Server-to-server token had an expired exp claim, support for this has been deprecated and will result in errors in a future release"',
+    });
+
+    it('should work with a manually crafted JWT', async () => {
+      const secret = 'a1b2c3';
+      const token = await new jose.SignJWT({})
+        .setProtectedHeader({ alg: 'HS256' })
+        .setSubject('backstage-server')
+        .setExpirationTime(Date.now() + 1000 * 60 * 60)
+        .sign(jose.base64url.decode(secret));
+
+      const tokenManager = ServerTokenManager.fromConfig(
+        new ConfigReader({
+          backend: { auth: { keys: [{ secret }] } },
+        }),
+        { logger },
+      );
+      await expect(tokenManager.authenticate(token)).resolves.toBeUndefined();
+    });
+
+    it('should reject tokens without exp claim', async () => {
+      const secret = 'a1b2c3';
+      const token = await new jose.SignJWT({})
+        .setProtectedHeader({ alg: 'HS256' })
+        .setSubject('backstage-server')
+        .sign(jose.base64url.decode(secret));
+
+      const tokenManager = ServerTokenManager.fromConfig(
+        new ConfigReader({
+          backend: { auth: { keys: [{ secret }] } },
+        }),
+        { logger },
+      );
+      await expect(tokenManager.authenticate(token)).rejects.toThrow(
+        'Invalid server token; caused by AuthenticationError: Server-to-server token had no exp claim',
       );
     });
   });

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

@@ -15,7 +15,7 @@
  */
 
 import { Config } from '@backstage/config';
-import { AuthenticationError, NotAllowedError } from '@backstage/errors';
+import { AuthenticationError } from '@backstage/errors';
 import { base64url, exportJWK, generateSecret, jwtVerify, SignJWT } from 'jose';
 import { DateTime, Duration } from 'luxon';
 import { Logger } from 'winston';
@@ -64,8 +64,6 @@ export class ServerTokenManager implements TokenManager {
   private signingKey: Uint8Array;
   private privateKeyPromise: Promise<void> | undefined;
   private currentTokenPromise: Promise<{ token: string }> | undefined;
-  private warnedForMissingExpClaim = false;
-  private warnedForExpiredExpClaim = false;
 
   /**
    * Creates a token manager that issues static dummy tokens and never fails
@@ -184,36 +182,20 @@ export class ServerTokenManager implements TokenManager {
         const {
           protectedHeader: { alg },
           payload: { sub, exp },
-        } = await jwtVerify(token, key, {
-          // TODO(freben): Holding on to tokens and reusing them is deprecated; remove this tolerance in a future release
-          clockTolerance: 3e9,
-        });
+        } = await jwtVerify(token, key);
 
         if (alg !== TOKEN_ALG) {
-          throw new NotAllowedError(`Illegal alg "${alg}"`);
+          throw new AuthenticationError(`Illegal alg "${alg}"`);
         }
 
         if (sub !== TOKEN_SUB) {
-          throw new NotAllowedError(`Illegal sub "${sub}"`);
+          throw new AuthenticationError(`Illegal sub "${sub}"`);
         }
 
-        // TODO(freben): Passing in tokens without an exp is deprecated; change this warning to an error in a future release
         if (typeof exp !== 'number') {
-          if (!this.warnedForMissingExpClaim) {
-            this.warnedForMissingExpClaim = true;
-            this.options.logger.warn(
-              `#### DEPRECATION WARNING: #### Server-to-server token had no exp claim, support for this has been deprecated and will result in errors in a future release`,
-            );
-          }
-        }
-        // TODO(freben): Holding on to tokens and reusing them is deprecated; remove this tolerance in a future release
-        else if (exp * 1000 < Date.now()) {
-          if (!this.warnedForExpiredExpClaim) {
-            this.warnedForExpiredExpClaim = true;
-            this.options.logger.warn(
-              `#### DEPRECATION WARNING: #### Server-to-server token had an expired exp claim, support for this has been deprecated and will result in errors in a future release`,
-            );
-          }
+          throw new AuthenticationError(
+            'Server-to-server token had no exp claim',
+          );
         }
         return;
       } catch (e) {
@@ -222,6 +204,6 @@ export class ServerTokenManager implements TokenManager {
       }
     }
 
-    throw new AuthenticationError(`Invalid server token: ${verifyError}`);
+    throw new AuthenticationError('Invalid server token', verifyError);
   }
 }