Add an expiry time on server-to-server tokens

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

.changeset/olive-eggs-accept.md

@@ -0,0 +1,13 @@
+---
+'@backstage/backend-common': minor
+---
+
+**BREAKING**: 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.
+
+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
+for reuse. But this now has become even more important advice to heed, and you
+should verify that you do not hold on to and reuse tokens such as these in your
+own code.

packages/backend-common/api-report.md

@@ -599,18 +599,20 @@ export class ServerTokenManager implements TokenManager {
   // (undocumented)
   static fromConfig(
     config: Config,
-    options: {
-      logger: Logger;
-    },
+    options: ServerTokenManagerOptions,
   ): ServerTokenManager;
   // (undocumented)
   getToken(): Promise<{
     token: string;
   }>;
-  // (undocumented)
   static noop(): TokenManager;
 }
 
+// @public
+export interface ServerTokenManagerOptions {
+  logger: Logger;
+}
+
 // @public
 export type ServiceBuilder = {
   loadConfig(config: Config): ServiceBuilder;
@@ -669,10 +671,8 @@ export interface StatusCheckHandlerOptions {
 
 // @public
 export interface TokenManager {
-  // (undocumented)
-  authenticate: (token: string) => Promise<void>;
-  // (undocumented)
-  getToken: () => Promise<{
+  authenticate(token: string): Promise<void>;
+  getToken(): Promise<{
     token: string;
   }>;
 }

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

@@ -13,11 +13,12 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { getVoidLogger } from '../logging/voidLogger';
+
 import { ConfigReader } from '@backstage/config';
-import { ServerTokenManager } from './ServerTokenManager';
-import { Logger } from 'winston';
 import * as jose from 'jose';
+import { Logger } from 'winston';
+import { getVoidLogger } from '../logging/voidLogger';
+import { ServerTokenManager } from './ServerTokenManager';
 import { TokenManager } from './types';
 
 const emptyConfig = new ConfigReader({});
@@ -35,6 +36,7 @@ describe('ServerTokenManager', () => {
 
   afterEach(() => {
     process.env = env;
+    jest.useRealTimers();
   });
 
   describe('getToken', () => {
@@ -166,6 +168,28 @@ describe('ServerTokenManager', () => {
         /invalid server token/i,
       );
     });
+
+    it('should throw for expired tokens', async () => {
+      jest.useFakeTimers('modern');
+      jest.setSystemTime(new Date('2020-02-02T02:00:00.0000000Z'));
+
+      const tokenManager = ServerTokenManager.fromConfig(configWithSecret, {
+        logger,
+      });
+
+      const { token } = await tokenManager.getToken();
+      await expect(tokenManager.authenticate(token)).resolves.not.toThrow();
+
+      jest.setSystemTime(new Date('2020-02-02T02:59:00.0000000Z'));
+      await expect(tokenManager.authenticate(token)).resolves.not.toThrow();
+
+      jest.setSystemTime(new Date('2020-02-02T03:00:01.0000000Z'));
+      await expect(
+        tokenManager.authenticate(token),
+      ).rejects.toThrowErrorMatchingInlineSnapshot(
+        '"Invalid server token: JWTExpired: \\"exp\\" claim timestamp check failed"',
+      );
+    });
   });
 
   describe('fromConfig', () => {
@@ -242,7 +266,7 @@ describe('ServerTokenManager', () => {
     });
   });
 
-  describe('ServerTokenManager.noop', () => {
+  describe('noop', () => {
     let noopTokenManager: TokenManager;
 
     beforeEach(() => {

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

@@ -14,12 +14,19 @@
  * limitations under the License.
  */
 
-import { base64url, generateSecret, SignJWT, jwtVerify, exportJWK } from 'jose';
 import { Config } from '@backstage/config';
-import { AuthenticationError } from '@backstage/errors';
-import { TokenManager } from './types';
+import { AuthenticationError, NotAllowedError } from '@backstage/errors';
+import { base64url, exportJWK, generateSecret, jwtVerify, SignJWT } from 'jose';
 import { Logger } from 'winston';
+import { TokenManager } from './types';
 
+const TOKEN_ALG = 'HS256';
+const TOKEN_SUB = 'backstage-server';
+
+/**
+ * A token manager that issues static dummy tokens and never fails
+ * authentication. This can be useful for testing.
+ */
 class NoopTokenManager implements TokenManager {
   public readonly isInsecureServerTokenManager: boolean = true;
 
@@ -30,6 +37,18 @@ class NoopTokenManager implements TokenManager {
   async authenticate() {}
 }
 
+/**
+ * Options for {@link ServerTokenManager}.
+ *
+ * @public
+ */
+export interface ServerTokenManagerOptions {
+  /**
+   * The logger to use.
+   */
+  logger: Logger;
+}
+
 /**
  * Creates and validates tokens for use during backend-to-backend
  * authentication.
@@ -37,44 +56,48 @@ class NoopTokenManager implements TokenManager {
  * @public
  */
 export class ServerTokenManager implements TokenManager {
+  private readonly options: ServerTokenManagerOptions;
   private verificationKeys: Uint8Array[];
   private signingKey: Uint8Array;
   private privateKeyPromise?: Promise<void>;
-  private logger: Logger;
 
+  /**
+   * Creates a token manager that issues static dummy tokens and never fails
+   * authentication. This can be useful for testing.
+   */
   static noop(): TokenManager {
     return new NoopTokenManager();
   }
 
-  static fromConfig(config: Config, options: { logger: Logger }) {
-    const { logger } = options;
-
+  static fromConfig(config: Config, options: ServerTokenManagerOptions) {
     const keys = config.getOptionalConfigArray('backend.auth.keys');
     if (keys?.length) {
       return new ServerTokenManager(
         keys.map(key => key.getString('secret')),
-        logger,
+        options,
       );
     }
+
     if (process.env.NODE_ENV !== 'development') {
       throw new Error(
         'You must configure at least one key in backend.auth.keys for production.',
       );
     }
+
     // For development, if a secret has not been configured, we auto generate a secret instead of throwing.
-    logger.warn(
+    options.logger.warn(
       'Generated a secret for backend-to-backend authentication: DEVELOPMENT USE ONLY.',
     );
-    return new ServerTokenManager([], logger);
+    return new ServerTokenManager([], options);
   }
 
-  private constructor(secrets: string[], logger: Logger) {
+  private constructor(secrets: string[], options: ServerTokenManagerOptions) {
     if (!secrets.length && process.env.NODE_ENV !== 'development') {
       throw new Error(
         'No secrets provided when constructing ServerTokenManager',
       );
     }
-    this.logger = logger;
+    this.options = options;
     this.verificationKeys = secrets.map(s => base64url.decode(s));
     this.signingKey = this.verificationKeys[0];
   }
@@ -86,11 +109,13 @@ export class ServerTokenManager implements TokenManager {
         'Key generation is not supported outside of the dev environment',
       );
     }
+
     if (this.privateKeyPromise) {
       return this.privateKeyPromise;
     }
+
     const promise = (async () => {
-      const secret = await generateSecret('HS256');
+      const secret = await generateSecret(TOKEN_ALG);
       const jwk = await exportJWK(secret);
       this.verificationKeys.push(base64url.decode(jwk.k ?? ''));
       this.signingKey = this.verificationKeys[0];
@@ -98,13 +123,15 @@ export class ServerTokenManager implements TokenManager {
     })();
 
     try {
-      // If we fail to generate a new key, we need to clear the state so that
-      // the next caller will try to generate another key.
+      this.privateKeyPromise = promise;
       await promise;
     } catch (error) {
-      this.logger.error(`Failed to generate new key, ${error}`);
+      // If we fail to generate a new key, we need to clear the state so that
+      // the next caller will try to generate another key.
+      this.options.logger.error(`Failed to generate new key, ${error}`);
       delete this.privateKeyPromise;
     }
+
     return promise;
   }
 
@@ -112,26 +139,39 @@ export class ServerTokenManager implements TokenManager {
     if (!this.verificationKeys.length) {
       await this.generateKeys();
     }
-    const sub = 'backstage-server';
-    const jwt = await new SignJWT({ alg: 'HS256' })
-      .setProtectedHeader({ alg: 'HS256', sub: sub })
-      .setSubject('backstage-server')
+
+    const jwt = await new SignJWT({})
+      .setProtectedHeader({ alg: TOKEN_ALG })
+      .setSubject(TOKEN_SUB)
+      .setExpirationTime('1h')
       .sign(this.signingKey);
+
     return { token: jwt };
   }
 
   async authenticate(token: string): Promise<void> {
     let verifyError = undefined;
+
     for (const key of this.verificationKeys) {
       try {
-        await jwtVerify(token, key);
-        // If the verify succeeded, return
+        const result = await jwtVerify(token, key);
+        if (result.protectedHeader.alg !== TOKEN_ALG) {
+          throw new NotAllowedError(
+            `Illegal alg "${result.protectedHeader.alg}"`,
+          );
+        }
+        if (result.payload.sub !== TOKEN_SUB) {
+          throw new NotAllowedError(`Illegal sub "${result.payload.sub}"`);
+        }
+        // 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
         verifyError = e;
       }
     }
+
     throw new AuthenticationError(`Invalid server token: ${verifyError}`);
   }
 }

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

@@ -15,4 +15,5 @@
  */
 
 export { ServerTokenManager } from './ServerTokenManager';
+export type { ServerTokenManagerOptions } from './ServerTokenManager';
 export type { TokenManager } from './types';

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

@@ -20,6 +20,20 @@
  * @public
  */
 export interface TokenManager {
-  getToken: () => Promise<{ token: string }>;
-  authenticate: (token: string) => Promise<void>;
+  /**
+   * Fetches a valid token.
+   *
+   * @remarks
+   *
+   * Tokens are valid for roughly one hour; the actual deadline is set in the
+   * payload `exp` claim. Never hold on to tokens for reuse; always ask for a
+   * new one for each outgoing request. This ensures that you always get a
+   * valid, fresh one.
+   */
+  getToken(): Promise<{ token: string }>;
+
+  /**
+   * Validates a given token.
+   */
+  authenticate(token: string): Promise<void>;
 }