From be5b958ed0acf42c8ac51f92f781b7f7ce293ee7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 8 Apr 2024 14:14:01 +0200 Subject: [PATCH] backend-app-api: fix plugin token public key expiration Signed-off-by: Patrik Oldsberg --- .../auth/PluginTokenHandler.test.ts | 82 +++++++++++++++++++ .../auth/PluginTokenHandler.ts | 48 ++++++----- 2 files changed, 110 insertions(+), 20 deletions(-) create mode 100644 packages/backend-app-api/src/services/implementations/auth/PluginTokenHandler.test.ts diff --git a/packages/backend-app-api/src/services/implementations/auth/PluginTokenHandler.test.ts b/packages/backend-app-api/src/services/implementations/auth/PluginTokenHandler.test.ts new file mode 100644 index 0000000000..2f23203768 --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/auth/PluginTokenHandler.test.ts @@ -0,0 +1,82 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { mockServices } from '@backstage/backend-test-utils'; +import { PluginTokenHandler } from './PluginTokenHandler'; +import { decodeJwt } from 'jose'; + +describe('PluginTokenHandler', () => { + beforeEach(() => { + jest.useRealTimers(); + }); + + it('issues token with correct expiration of token and generated key', async () => { + jest.useFakeTimers({ + now: new Date(0), + }); + + const addKeyMock = jest.fn(); + const handler = PluginTokenHandler.create({ + discovery: mockServices.discovery(), + keyDurationSeconds: 10, + logger: mockServices.logger.mock(), + ownPluginId: 'test', + publicKeyStore: { + addKey: addKeyMock, + listKeys: jest.fn(), + }, + }); + + const { token } = await handler.issueToken({ + pluginId: 'test', + targetPluginId: 'other', + }); + const payload = decodeJwt(token); + expect(payload.iat).toBe(0); + expect(payload.exp).toBe(10); + + expect(addKeyMock).toHaveBeenCalledTimes(1); + expect(addKeyMock).toHaveBeenCalledWith({ + id: expect.any(String), + key: expect.any(Object), + expiresAt: new Date(30_000), + }); + + jest.advanceTimersByTime(5_000); + await handler.issueToken({ + pluginId: 'test', + targetPluginId: 'other', + }); + expect(addKeyMock).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(10_000); + await handler.issueToken({ + pluginId: 'test', + targetPluginId: 'other', + }); + expect(addKeyMock).toHaveBeenCalledTimes(2); + + expect(addKeyMock.mock.calls[0][0].id).not.toBe( + addKeyMock.mock.calls[1][0].id, + ); + + expect(addKeyMock).toHaveBeenNthCalledWith(2, { + id: expect.any(String), + key: expect.any(Object), + expiresAt: new Date(45_000), + }); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/auth/PluginTokenHandler.ts b/packages/backend-app-api/src/services/implementations/auth/PluginTokenHandler.ts index 7e9ac4220b..15eaac6b1c 100644 --- a/packages/backend-app-api/src/services/implementations/auth/PluginTokenHandler.ts +++ b/packages/backend-app-api/src/services/implementations/auth/PluginTokenHandler.ts @@ -24,7 +24,6 @@ import { SignJWT, decodeProtectedHeader, } from 'jose'; -import { DateTime } from 'luxon'; import { v4 as uuid } from 'uuid'; import { InternalKey, KeyStore } from './types'; import { AuthenticationError } from '@backstage/errors'; @@ -32,6 +31,13 @@ import { jwtVerify } from 'jose'; import { tokenTypes } from '@backstage/plugin-auth-node'; import { JwksClient } from './JwksClient'; +/** + * The margin for how many times longer we make the public key available + * compared to how long we use the private key to sign new tokens. + */ +const KEY_EXPIRATION_MARGIN_FACTOR = 3; +const SECONDS_IN_MS = 1000; + const ALLOWED_PLUGIN_ID_PATTERN = /^[a-z0-9_-]+$/i; type Options = { @@ -71,12 +77,12 @@ export class PluginTokenHandler { } private constructor( - readonly logger: LoggerService, - readonly ownPluginId: string, - readonly publicKeyStore: KeyStore, - readonly keyDurationSeconds: number, - readonly algorithm: string, - readonly discovery: DiscoveryService, + private readonly logger: LoggerService, + private readonly ownPluginId: string, + private readonly publicKeyStore: KeyStore, + private readonly keyDurationSeconds: number, + private readonly algorithm: string, + private readonly discovery: DiscoveryService, ) {} async verifyToken( @@ -129,10 +135,13 @@ export class PluginTokenHandler { const sub = pluginId; const aud = targetPluginId; - const iat = Math.floor(Date.now() / 1000); + const iat = Math.floor(Date.now() / SECONDS_IN_MS); const ourExp = iat + this.keyDurationSeconds; const exp = onBehalfOf - ? Math.min(ourExp, Math.floor(onBehalfOf.expiresAt.getTime() / 1000)) + ? Math.min( + ourExp, + Math.floor(onBehalfOf.expiresAt.getTime() / SECONDS_IN_MS), + ) : ourExp; const claims = { sub, aud, iat, exp, obo: onBehalfOf?.token }; @@ -223,22 +232,16 @@ export class PluginTokenHandler { private async getKey(): Promise { // Make sure that we only generate one key at a time if (this.privateKeyPromise) { - if ( - this.keyExpiry && - DateTime.fromJSDate(this.keyExpiry) > DateTime.local() - ) { + if (this.keyExpiry && this.keyExpiry.getTime() > Date.now()) { return this.privateKeyPromise; } this.logger.info(`Signing key has expired, generating new key`); delete this.privateKeyPromise; } - const keyExpiry = DateTime.utc() - .plus({ - seconds: this.keyDurationSeconds, - }) - .toJSDate(); - this.keyExpiry = keyExpiry; + this.keyExpiry = new Date( + Date.now() + this.keyDurationSeconds * SECONDS_IN_MS, + ); const promise = (async () => { // This generates a new signing key to be used to sign tokens until the next key rotation @@ -260,7 +263,12 @@ export class PluginTokenHandler { await this.publicKeyStore.addKey({ id: kid, key: publicKey as InternalKey, - expiresAt: keyExpiry, + expiresAt: new Date( + Date.now() + + this.keyDurationSeconds * + SECONDS_IN_MS * + KEY_EXPIRATION_MARGIN_FACTOR, + ), }); // At this point we are allowed to start using the new key