review fixes

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

.gitignore

@@ -170,6 +170,3 @@ knip.json
 # Schemathesis temporary files
 .hypothesis/
 .cassettes/
-
-# Locally generated keys
-*.key

docs/auth/service-to-service-auth.md

@@ -110,7 +110,8 @@ accordingly:
 ```yaml
 backend:
   auth:
-    keyStore:
+    # This is the new section for configuring plugin-to-plugin key storage
+    pluginKeyStore:
       type: static
       static:
         keys:
@@ -128,7 +129,7 @@ The first entry will always be used for signing, but any of the subsequent
 entries will also be used for token validation. This lets you have a period of
 time where tokens signed by the previous top entry are still accepted by
 receivers, by just inserting your new key pair as the top entry and leaving the
-old ones intact.
+old ones intact. You can remove old private keys however; those won't be used.
 
 ## Static Tokens
 

packages/backend-app-api/config.d.ts

@@ -34,7 +34,7 @@ export interface Config {
       dangerouslyDisableDefaultAuthPolicy?: boolean;
 
       /** Controls how to store keys for plugin-to-plugin auth */
-      keyStore?:
+      pluginKeyStore?:
         | { type: 'database' }
         | {
             type: 'static';
@@ -49,8 +49,10 @@ export interface Config {
                 publicKeyFile: string;
                 /**
                  * Path to the matching private key file in the PKCS#8 format. Should be an absolute path.
+                 *
+                 * The first array entry must specify a private key file, the rest must not.
                  */
-                privateKeyFile: string;
+                privateKeyFile?: string;
                 /**
                  * ID to uniquely identify this key within the JWK set.
                  */

packages/backend-app-api/src/services/implementations/auth/plugin/keys/StaticConfigPluginKeySource.test.ts

@@ -15,7 +15,7 @@
  */
 
 import { StaticConfigPluginKeySource } from './StaticConfigPluginKeySource';
-import { Config, ConfigReader } from '@backstage/config';
+import { ConfigReader } from '@backstage/config';
 import { createMockDirectory } from '@backstage/backend-test-utils';
 
 const privateKey = `
@@ -34,43 +34,39 @@ ZoUEY72HTvjIP9xSbLOhWhMREk84T0q91/m3v3sWq5EzHIWw1zRHQisKZw==
 `.trim();
 
 describe('StaticConfigPluginKeySource', () => {
-  let sourceConfig: Config;
-  const sourceDir = createMockDirectory();
-
-  beforeAll(() => {
-    sourceDir.setContent({
+  const mockDir = createMockDirectory({
+    content: {
       'public.pem': publicKey,
       'private.pem': privateKey,
-    });
-
-    const publicKeyPath = sourceDir.resolve('public.pem');
-    const privateKeyPath = sourceDir.resolve('private.pem');
-    sourceConfig = new ConfigReader({
-      type: 'static',
-      static: {
-        keys: [
-          {
-            publicKeyFile: publicKeyPath,
-            privateKeyFile: privateKeyPath,
-            keyId: '1',
-            algorithm: 'ES256',
-          },
-          {
-            publicKeyFile: publicKeyPath,
-            privateKeyFile: privateKeyPath,
-            keyId: '2',
-            // skipping explicit alg
-          },
-        ],
-      },
-    });
+    },
   });
+  const publicKeyPath = mockDir.resolve('public.pem');
+  const privateKeyPath = mockDir.resolve('private.pem');
 
   it('should provide keys from disk', async () => {
     const staticKeyStore = await StaticConfigPluginKeySource.create({
-      sourceConfig,
+      sourceConfig: new ConfigReader({
+        type: 'static',
+        static: {
+          keys: [
+            {
+              publicKeyFile: publicKeyPath,
+              privateKeyFile: privateKeyPath,
+              keyId: '1',
+              algorithm: 'ES256',
+            },
+            {
+              // intentionally only private key
+              // skipping explicit alg
+              publicKeyFile: publicKeyPath,
+              keyId: '2',
+            },
+          ],
+        },
+      }),
       keyDuration: { hours: 1 },
     });
+
     const keys = await staticKeyStore.listKeys();
     expect(keys.keys.length).toEqual(2);
     expect(keys.keys[0].key).toMatchObject({
@@ -89,4 +85,40 @@ describe('StaticConfigPluginKeySource', () => {
     });
     expect(pk.d).toBeDefined();
   });
+
+  it('throws an error for misconfigurations', async () => {
+    await expect(
+      StaticConfigPluginKeySource.create({
+        sourceConfig: new ConfigReader({
+          type: 'static',
+          static: {
+            keys: [],
+          },
+        }),
+        keyDuration: { hours: 1 },
+      }),
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"At least one key pair must be provided in static.keys, when the static key store type is used"`,
+    );
+
+    await expect(
+      StaticConfigPluginKeySource.create({
+        sourceConfig: new ConfigReader({
+          type: 'static',
+          static: {
+            keys: [
+              {
+                publicKeyFile: publicKeyPath,
+                keyId: '1',
+                algorithm: 'ES256',
+              },
+            ],
+          },
+        }),
+        keyDuration: { hours: 1 },
+      }),
+    ).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"Private key for signing must be provided in the first key pair in static.keys, when the static key store type is used"`,
+    );
+  });
 });

packages/backend-app-api/src/services/implementations/auth/plugin/keys/StaticConfigPluginKeySource.ts

@@ -24,13 +24,13 @@ import { PluginKeySource } from './types';
 
 export type KeyPair = {
   publicKey: JWK;
-  privateKey: JWK;
+  privateKey?: JWK;
   keyId: string;
 };
 
 export type StaticKeyConfig = {
   publicKeyFile: string;
-  privateKeyFile: string;
+  privateKeyFile?: string;
   keyId: string;
   algorithm: string;
 };
@@ -81,7 +81,7 @@ export class StaticConfigPluginKeySource implements PluginKeySource {
       .map(c => {
         const staticKeyConfig: StaticKeyConfig = {
           publicKeyFile: c.getString('publicKeyFile'),
-          privateKeyFile: c.getString('privateKeyFile'),
+          privateKeyFile: c.getOptionalString('privateKeyFile'),
           keyId: c.getString('keyId'),
           algorithm: c.getOptionalString('algorithm') ?? DEFAULT_ALGORITHM,
         };
@@ -97,6 +97,10 @@ export class StaticConfigPluginKeySource implements PluginKeySource {
       throw new Error(
         'At least one key pair must be provided in static.keys, when the static key store type is used',
       );
+    } else if (!keyPairs[0].privateKey) {
+      throw new Error(
+        'Private key for signing must be provided in the first key pair in static.keys, when the static key store type is used',
+      );
     }
 
     return new StaticConfigPluginKeySource(
@@ -106,7 +110,7 @@ export class StaticConfigPluginKeySource implements PluginKeySource {
   }
 
   async getPrivateSigningKey(): Promise<JWK> {
-    return this.keyPairs[0].privateKey;
+    return this.keyPairs[0].privateKey!;
   }
 
   async listKeys(): Promise<{ keys: KeyPayload[] }> {
@@ -122,11 +126,13 @@ export class StaticConfigPluginKeySource implements PluginKeySource {
       keyId,
       algorithm,
     );
-    const privateKey = await this.loadPrivateKeyFromFile(
-      options.privateKeyFile,
-      keyId,
-      algorithm,
-    );
+    const privateKey = options.privateKeyFile
+      ? await this.loadPrivateKeyFromFile(
+          options.privateKeyFile,
+          keyId,
+          algorithm,
+        )
+      : undefined;
 
     return { publicKey, privateKey, keyId };
   }

packages/backend-app-api/src/services/implementations/auth/plugin/keys/createPluginKeySource.test.ts

@@ -73,7 +73,7 @@ describe('createPluginKeySource', () => {
 
       const source = await createPluginKeySource({
         config: new ConfigReader({
-          backend: { auth: { keyStore: { type: 'database' } } },
+          backend: { auth: { pluginKeyStore: { type: 'database' } } },
         }),
         database: mockServices.database.mock({ getClient }),
         logger: mockServices.logger.mock(),
@@ -131,7 +131,7 @@ describe('createPluginKeySource', () => {
       config: new ConfigReader({
         backend: {
           auth: {
-            keyStore: {
+            pluginKeyStore: {
               type: 'static',
               static: {
                 keys: [

packages/backend-app-api/src/services/implementations/auth/plugin/keys/createPluginKeySource.ts

@@ -24,7 +24,7 @@ import { DatabasePluginKeySource } from './DatabasePluginKeySource';
 import { StaticConfigPluginKeySource } from './StaticConfigPluginKeySource';
 import { PluginKeySource } from './types';
 
-const CONFIG_ROOT_KEY = 'backend.auth.keyStore';
+const CONFIG_ROOT_KEY = 'backend.auth.pluginKeyStore';
 
 export async function createPluginKeySource(options: {
   config: RootConfigService;