warn about the exp

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

.changeset/olive-eggs-accept.md

@@ -1,10 +1,13 @@
 ---
-'@backstage/backend-common': minor
+'@backstage/backend-common': patch
 ---
 
-**BREAKING**: Server-to-server authentication tokens issued from a
+**DEPRECATION**: Server-to-server authentication tokens issued from a
 `TokenManager` (specifically, `ServerTokenManager`) now has an expiry time set,
-for one hour in the future from when issued. This improves security.
+for one hour in the future from when issued. This improves security. The ability
+to pass in and validate tokens that either have a missing `exp` claim, or an
+`exp` claim that expired in the past, is now deprecated. Trying to do so will
+lead to logged warnings, and in a future release will instead lead to errors.
 
 It was always the case that users of this interface were expected to call its
 `getToken()` method for every outgoing call and never hold on to any given token

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

@@ -171,6 +171,7 @@ 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,
@@ -178,12 +179,14 @@ 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);
@@ -191,15 +194,15 @@ 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, the newest one is still valid
+      // 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),
-      ).rejects.toThrowErrorMatchingInlineSnapshot(
-        '"Invalid server token: JWTExpired: \\"exp\\" claim timestamp check failed"',
-      );
+      await expect(tokenManager.authenticate(token1)).resolves.not.toThrow();
       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"',
+      );
     });
   });
 

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

@@ -64,6 +64,8 @@ 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
@@ -179,17 +181,40 @@ export class ServerTokenManager implements TokenManager {
 
     for (const key of this.verificationKeys) {
       try {
-        const result = await jwtVerify(token, key);
-        if (result.protectedHeader.alg !== TOKEN_ALG) {
-          throw new NotAllowedError(
-            `Illegal alg "${result.protectedHeader.alg}"`,
-          );
+        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,
+        });
+
+        if (alg !== TOKEN_ALG) {
+          throw new NotAllowedError(`Illegal alg "${alg}"`);
         }
-        if (result.payload.sub !== TOKEN_SUB) {
-          throw new NotAllowedError(`Illegal sub "${result.payload.sub}"`);
+
+        if (sub !== TOKEN_SUB) {
+          throw new NotAllowedError(`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`,
+            );
+          }
         }
-        // TODO(freben): Reject missing payload.exp in the future as well
-        // The jose library does NOT throw if exp is not set in the token, but DOES throw if exp is set and expired
         return;
       } catch (e) {
         // Catch the verify exception and continue