From d120af630cce059a243d32a46d291b0ff77c343b Mon Sep 17 00:00:00 2001 From: rtriesscheijn Date: Mon, 16 Oct 2023 13:25:34 +0200 Subject: [PATCH] feat: read static key store config safely Signed-off-by: rtriesscheijn --- plugins/auth-backend/package.json | 1 + .../auth-backend/src/identity/KeyStores.ts | 8 +-- .../src/identity/StaticKeyStore.test.ts | 52 +++++++++---------- .../src/identity/StaticKeyStore.ts | 30 +++++++---- yarn.lock | 1 + 5 files changed, 51 insertions(+), 41 deletions(-) diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 355c8eb554..32c3e86cf3 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -88,6 +88,7 @@ "@backstage/backend-defaults": "workspace:^", "@backstage/backend-test-utils": "workspace:^", "@backstage/cli": "workspace:^", + "@backstage/core-app-api": "workspace:^", "@types/body-parser": "^1.19.0", "@types/cookie-parser": "^1.4.2", "@types/express-session": "^1.17.2", diff --git a/plugins/auth-backend/src/identity/KeyStores.ts b/plugins/auth-backend/src/identity/KeyStores.ts index a19606c9a3..eecf94edc5 100644 --- a/plugins/auth-backend/src/identity/KeyStores.ts +++ b/plugins/auth-backend/src/identity/KeyStores.ts @@ -23,7 +23,7 @@ import { DatabaseKeyStore } from './DatabaseKeyStore'; import { FirestoreKeyStore } from './FirestoreKeyStore'; import { MemoryKeyStore } from './MemoryKeyStore'; import { KeyStore } from './types'; -import { StaticKeyStore, StaticKeyStoreConfig } from './StaticKeyStore'; +import { StaticKeyStore } from './StaticKeyStore'; type Options = { logger: LoggerService; @@ -76,12 +76,12 @@ export class KeyStores { } if (provider === 'static') { - const settings = ks?.getOptional('static'); + const settings = ks?.getConfig(provider); if (settings === undefined) { - throw new Error(`Missing configuration for static keyStore provider`); + throw new Error(`Missing configuration for static key store provider`); } - await StaticKeyStore.create(settings); + await StaticKeyStore.fromConfig(settings); } throw new Error(`Unknown KeyStore provider: ${provider}`); diff --git a/plugins/auth-backend/src/identity/StaticKeyStore.test.ts b/plugins/auth-backend/src/identity/StaticKeyStore.test.ts index e0882efa31..d56698189e 100644 --- a/plugins/auth-backend/src/identity/StaticKeyStore.test.ts +++ b/plugins/auth-backend/src/identity/StaticKeyStore.test.ts @@ -14,11 +14,12 @@ * limitations under the License. */ import { promises as fs } from 'fs'; -import { StaticKeyStore, StaticKeyStoreConfig } from './StaticKeyStore'; +import { StaticKeyStore } from './StaticKeyStore'; import { AnyJWK } from './types'; +import { ConfigReader } from '@backstage/core-app-api'; const publicKeyPath = '/mnt/public.pem'; -const privateKeyPath = 'mnt/private.pem'; +const privateKeyPath = '/mnt/private.pem'; const privateKey = ` -----BEGIN PRIVATE KEY----- MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgR8Ja2ppMEgOm1KeY @@ -34,14 +35,28 @@ ZoUEY72HTvjIP9xSbLOhWhMREk84T0q91/m3v3sWq5EzHIWw1zRHQisKZw== -----END PUBLIC KEY----- `.trim(); +const config = new ConfigReader({ + keys: [ + { + publicKeyFile: publicKeyPath, + privateKeyFile: privateKeyPath, + keyId: '1', + algorithm: 'ES256', + }, + { + publicKeyFile: publicKeyPath, + privateKeyFile: privateKeyPath, + keyId: '2', + algorithm: 'ES256', + }, + ], +}); + jest.mock('fs/promises'); describe('StaticKeyStore', () => { beforeEach(() => { jest.resetAllMocks(); - }); - - it('should provide keys from disk', async () => { fs.readFile = jest.fn().mockImplementation(async (path: string, _: any) => { if (path === publicKeyPath) { return Promise.resolve(publicKey); @@ -52,25 +67,10 @@ describe('StaticKeyStore', () => { throw new Error('Unexpected path'); }); + }); - const staticKeyStoreConfig: StaticKeyStoreConfig = { - keys: [ - { - publicKeyFile: publicKeyPath, - privateKeyFile: privateKeyPath, - keyId: '1', - algorithm: 'ES256', - }, - { - publicKeyFile: publicKeyPath, - privateKeyFile: privateKeyPath, - keyId: '2', - algorithm: 'ES256', - }, - ], - }; - const staticKeyStore = await StaticKeyStore.create(staticKeyStoreConfig); - + it('should provide keys from disk', async () => { + const staticKeyStore = await StaticKeyStore.fromConfig(config); const keys = await staticKeyStore.listKeys(); expect(keys.items.length).toEqual(2); expect(keys.items[0].key).toMatchObject({ @@ -91,8 +91,7 @@ describe('StaticKeyStore', () => { }); it('should not allow users to add keys', async () => { - const staticKeyStoreConfig: StaticKeyStoreConfig = { keys: [] }; - const staticKeyStore = await StaticKeyStore.create(staticKeyStoreConfig); + const staticKeyStore = await StaticKeyStore.fromConfig(config); const key: AnyJWK = { use: 'sig', @@ -106,8 +105,7 @@ describe('StaticKeyStore', () => { }); it('should not allow users to remove keys', async () => { - const staticKeyStoreConfig: StaticKeyStoreConfig = { keys: [] }; - const staticKeyStore = await StaticKeyStore.create(staticKeyStoreConfig); + const staticKeyStore = await StaticKeyStore.fromConfig(config); expect(() => staticKeyStore.removeKeys(['1'])).toThrow( 'Cannot remove keys from the static key store', ); diff --git a/plugins/auth-backend/src/identity/StaticKeyStore.ts b/plugins/auth-backend/src/identity/StaticKeyStore.ts index 6859d1f445..11ec1585d2 100644 --- a/plugins/auth-backend/src/identity/StaticKeyStore.ts +++ b/plugins/auth-backend/src/identity/StaticKeyStore.ts @@ -17,6 +17,7 @@ import { AnyJWK, KeyStore, StoredKey } from './types'; import { exportJWK, importPKCS8, importSPKI, JWK } from 'jose'; import { KeyLike } from 'jose/dist/types/types'; import { promises as fs } from 'fs'; +import { Config } from '@backstage/config'; export type KeyPair = { publicKey: JWK; @@ -27,11 +28,7 @@ export type StaticKeyConfig = { publicKeyFile: string; privateKeyFile: string; keyId: string; - algorithm?: string; -}; - -export type StaticKeyStoreConfig = { - keys: StaticKeyConfig[]; + algorithm: string; }; const DEFAULT_ALGORITHM = 'ES256'; @@ -68,15 +65,28 @@ export class StaticKeyStore implements KeyStore { private readonly createdAt: Date; private constructor(keyPairs: KeyPair[]) { + if (keyPairs.length === 0) { + throw new Error('Should provide at least one key pair'); + } + this.keyPairs = keyPairs; this.createdAt = new Date(); } - public static async create( - config: StaticKeyStoreConfig, - ): Promise { + public static async fromConfig(config: Config): Promise { + const keyConfigs = config.getConfigArray('keys').map(c => { + const staticKeyConfig: StaticKeyConfig = { + publicKeyFile: c.getString('publicKeyFile'), + privateKeyFile: c.getString('privateKeyFile'), + keyId: c.getString('keyId'), + algorithm: c.getOptionalString('algorithm') ?? DEFAULT_ALGORITHM, + }; + + return staticKeyConfig; + }); + const keyPairs = await Promise.all( - config.keys.map(async k => await this.loadKeyPair(k)), + keyConfigs.map(async k => await this.loadKeyPair(k)), ); return new StaticKeyStore(keyPairs); @@ -117,7 +127,7 @@ export class StaticKeyStore implements KeyStore { } private static async loadKeyPair(options: StaticKeyConfig): Promise { - const algorithm = options.algorithm ?? DEFAULT_ALGORITHM; + const algorithm = options.algorithm; const keyId = options.keyId; const publicKey = await this.loadPublicKeyFromFile( options.publicKeyFile, diff --git a/yarn.lock b/yarn.lock index 2533a51f32..c4aa235464 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5006,6 +5006,7 @@ __metadata: "@backstage/catalog-model": "workspace:^" "@backstage/cli": "workspace:^" "@backstage/config": "workspace:^" + "@backstage/core-app-api": "workspace:^" "@backstage/errors": "workspace:^" "@backstage/plugin-auth-backend-module-gcp-iap-provider": "workspace:^" "@backstage/plugin-auth-backend-module-github-provider": "workspace:^"