From 5fcbd869604b11bc92ccf338cdf91bfef57fa22a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 3 May 2022 17:23:23 +0200 Subject: [PATCH 1/4] Add an expiry time on server-to-server tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/olive-eggs-accept.md | 13 +++ packages/backend-common/api-report.md | 16 ++-- .../src/tokens/ServerTokenManager.test.ts | 32 ++++++- .../src/tokens/ServerTokenManager.ts | 84 ++++++++++++++----- packages/backend-common/src/tokens/index.ts | 1 + packages/backend-common/src/tokens/types.ts | 18 +++- 6 files changed, 128 insertions(+), 36 deletions(-) create mode 100644 .changeset/olive-eggs-accept.md diff --git a/.changeset/olive-eggs-accept.md b/.changeset/olive-eggs-accept.md new file mode 100644 index 0000000000..81b1069f15 --- /dev/null +++ b/.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. diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 961973b144..a66c8de735 100644 --- a/packages/backend-common/api-report.md +++ b/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; - // (undocumented) - getToken: () => Promise<{ + authenticate(token: string): Promise; + getToken(): Promise<{ token: string; }>; } diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index 0ff7bfb316..57435fe486 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.test.ts +++ b/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(() => { diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index 484b99ca4d..ba6a276421 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/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; - 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 { 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}`); } } diff --git a/packages/backend-common/src/tokens/index.ts b/packages/backend-common/src/tokens/index.ts index 43ff12e597..fe1b56df80 100644 --- a/packages/backend-common/src/tokens/index.ts +++ b/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'; diff --git a/packages/backend-common/src/tokens/types.ts b/packages/backend-common/src/tokens/types.ts index 1fea018db9..2fa771b7e2 100644 --- a/packages/backend-common/src/tokens/types.ts +++ b/packages/backend-common/src/tokens/types.ts @@ -20,6 +20,20 @@ * @public */ export interface TokenManager { - getToken: () => Promise<{ token: string }>; - authenticate: (token: string) => Promise; + /** + * 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; } From 4874ebc2afa42531397bbce4cbeda43b21bdcff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 4 May 2022 15:26:41 +0200 Subject: [PATCH 2/4] add caching as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/tokens/ServerTokenManager.test.ts | 29 +++++++++----- .../src/tokens/ServerTokenManager.ts | 38 +++++++++++++++---- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index 57435fe486..2827634aac 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.test.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.test.ts @@ -169,26 +169,37 @@ describe('ServerTokenManager', () => { ); }); - it('should throw for expired tokens', async () => { - jest.useFakeTimers('modern'); - jest.setSystemTime(new Date('2020-02-02T02:00:00.0000000Z')); + it('should throw for expired tokens, and re-issue new ones', async () => { + jest.useFakeTimers(); const tokenManager = ServerTokenManager.fromConfig(configWithSecret, { logger, }); - const { token } = await tokenManager.getToken(); - await expect(tokenManager.authenticate(token)).resolves.not.toThrow(); + const { token: token1 } = await tokenManager.getToken(); + await expect(tokenManager.authenticate(token1)).resolves.not.toThrow(); - jest.setSystemTime(new Date('2020-02-02T02:59:00.0000000Z')); - await expect(tokenManager.authenticate(token)).resolves.not.toThrow(); + // Less than ten minutes before expiry, it still returns the same token + jest.advanceTimersByTime(49 * 60 * 1000); + const { token: token1Again } = await tokenManager.getToken(); + expect(token1).toEqual(token1Again); + await expect(tokenManager.authenticate(token1)).resolves.not.toThrow(); - jest.setSystemTime(new Date('2020-02-02T03:00:01.0000000Z')); + // Right before the expiry, the old ones are still valid but returning a new token + jest.advanceTimersByTime(10 * 60 * 1000); + const { token: token2 } = await tokenManager.getToken(); + expect(token1).not.toEqual(token2); + await expect(tokenManager.authenticate(token1)).resolves.not.toThrow(); + await expect(tokenManager.authenticate(token2)).resolves.not.toThrow(); + + // After expiry, the newest one is still valid + jest.advanceTimersByTime(2 * 60 * 1000); await expect( - tokenManager.authenticate(token), + tokenManager.authenticate(token1), ).rejects.toThrowErrorMatchingInlineSnapshot( '"Invalid server token: JWTExpired: \\"exp\\" claim timestamp check failed"', ); + await expect(tokenManager.authenticate(token2)).resolves.not.toThrow(); }); }); diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index ba6a276421..65354f270f 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -17,11 +17,13 @@ import { Config } from '@backstage/config'; import { AuthenticationError, NotAllowedError } from '@backstage/errors'; import { base64url, exportJWK, generateSecret, jwtVerify, SignJWT } from 'jose'; +import { DateTime, Duration } from 'luxon'; import { Logger } from 'winston'; import { TokenManager } from './types'; const TOKEN_ALG = 'HS256'; const TOKEN_SUB = 'backstage-server'; +const TOKEN_EXPIRY = Duration.fromObject({ hours: 1 }); /** * A token manager that issues static dummy tokens and never fails @@ -57,9 +59,10 @@ export interface ServerTokenManagerOptions { */ export class ServerTokenManager implements TokenManager { private readonly options: ServerTokenManagerOptions; - private verificationKeys: Uint8Array[]; + private readonly verificationKeys: Uint8Array[]; private signingKey: Uint8Array; - private privateKeyPromise?: Promise; + private privateKeyPromise: Promise | undefined; + private currentTokenPromise: Promise<{ token: string }> | undefined; /** * Creates a token manager that issues static dummy tokens and never fails @@ -140,13 +143,32 @@ export class ServerTokenManager implements TokenManager { await this.generateKeys(); } - const jwt = await new SignJWT({}) - .setProtectedHeader({ alg: TOKEN_ALG }) - .setSubject(TOKEN_SUB) - .setExpirationTime('1h') - .sign(this.signingKey); + if (this.currentTokenPromise) { + return this.currentTokenPromise; + } - return { token: jwt }; + const result = Promise.resolve().then(async () => { + const jwt = await new SignJWT({}) + .setProtectedHeader({ alg: TOKEN_ALG }) + .setSubject(TOKEN_SUB) + .setExpirationTime(DateTime.now().plus(TOKEN_EXPIRY).toUnixInteger()) + .sign(this.signingKey); + return { token: jwt }; + }); + + this.currentTokenPromise = result; + + result + .then(() => { + setTimeout(() => { + this.currentTokenPromise = undefined; + }, TOKEN_EXPIRY.minus({ minutes: 5 }).toMillis()); + }) + .catch(() => { + this.currentTokenPromise = undefined; + }); + + return result; } async authenticate(token: string): Promise { From 6dd45cbee6f29fd8899252664ff4b8c16f2b6616 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 9 May 2022 14:51:26 +0200 Subject: [PATCH 3/4] separate const for reissue time, shorten reissue time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/tokens/ServerTokenManager.test.ts | 12 ++++++------ .../backend-common/src/tokens/ServerTokenManager.ts | 9 ++++++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index 2827634aac..30d3571606 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.test.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.test.ts @@ -179,21 +179,21 @@ describe('ServerTokenManager', () => { const { token: token1 } = await tokenManager.getToken(); await expect(tokenManager.authenticate(token1)).resolves.not.toThrow(); - // Less than ten minutes before expiry, it still returns the same token - jest.advanceTimersByTime(49 * 60 * 1000); + // 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(); - // Right before the expiry, the old ones are still valid but returning a new token - jest.advanceTimersByTime(10 * 60 * 1000); + // Right after the reissue timeout, the old ones are still valid but returning a new token + jest.advanceTimersByTime(2 * 60 * 1000); const { token: token2 } = await tokenManager.getToken(); expect(token1).not.toEqual(token2); await expect(tokenManager.authenticate(token1)).resolves.not.toThrow(); await expect(tokenManager.authenticate(token2)).resolves.not.toThrow(); - // After expiry, the newest one is still valid - jest.advanceTimersByTime(2 * 60 * 1000); + // After expiry of the first one, the newest one is still valid + jest.advanceTimersByTime(52 * 60 * 1000); await expect( tokenManager.authenticate(token1), ).rejects.toThrowErrorMatchingInlineSnapshot( diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index 65354f270f..1915330d26 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -23,7 +23,8 @@ import { TokenManager } from './types'; const TOKEN_ALG = 'HS256'; const TOKEN_SUB = 'backstage-server'; -const TOKEN_EXPIRY = Duration.fromObject({ hours: 1 }); +const TOKEN_EXPIRY_AFTER = Duration.fromObject({ hours: 1 }); +const TOKEN_REISSUE_AFTER = Duration.fromObject({ minutes: 10 }); /** * A token manager that issues static dummy tokens and never fails @@ -151,7 +152,9 @@ export class ServerTokenManager implements TokenManager { const jwt = await new SignJWT({}) .setProtectedHeader({ alg: TOKEN_ALG }) .setSubject(TOKEN_SUB) - .setExpirationTime(DateTime.now().plus(TOKEN_EXPIRY).toUnixInteger()) + .setExpirationTime( + DateTime.now().plus(TOKEN_EXPIRY_AFTER).toUnixInteger(), + ) .sign(this.signingKey); return { token: jwt }; }); @@ -162,7 +165,7 @@ export class ServerTokenManager implements TokenManager { .then(() => { setTimeout(() => { this.currentTokenPromise = undefined; - }, TOKEN_EXPIRY.minus({ minutes: 5 }).toMillis()); + }, TOKEN_REISSUE_AFTER.toMillis()); }) .catch(() => { this.currentTokenPromise = undefined; From 2cd671bfddbc0b1f6546322d148672e8f4bee0db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 9 May 2022 15:39:05 +0200 Subject: [PATCH 4/4] warn about the exp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/olive-eggs-accept.md | 9 ++-- .../src/tokens/ServerTokenManager.test.ts | 15 ++++--- .../src/tokens/ServerTokenManager.ts | 43 +++++++++++++++---- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/.changeset/olive-eggs-accept.md b/.changeset/olive-eggs-accept.md index 81b1069f15..66d44d9c3e 100644 --- a/.changeset/olive-eggs-accept.md +++ b/.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 diff --git a/packages/backend-common/src/tokens/ServerTokenManager.test.ts b/packages/backend-common/src/tokens/ServerTokenManager.test.ts index 30d3571606..6a72b99908 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.test.ts +++ b/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"', + ); }); }); diff --git a/packages/backend-common/src/tokens/ServerTokenManager.ts b/packages/backend-common/src/tokens/ServerTokenManager.ts index 1915330d26..329c2f164c 100644 --- a/packages/backend-common/src/tokens/ServerTokenManager.ts +++ b/packages/backend-common/src/tokens/ServerTokenManager.ts @@ -64,6 +64,8 @@ export class ServerTokenManager implements TokenManager { private signingKey: Uint8Array; private privateKeyPromise: Promise | 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