From af321b324c36910d08f3b1bf1172c6c49f65a00c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Thu, 4 May 2023 22:10:14 +0200
Subject: [PATCH] address comments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fredrik Adelöw <freben@gmail.com>
---
 plugins/auth-backend/src/database/AuthDatabase.ts  |  1 +
 .../auth-backend/src/identity/KeyStores.test.ts    | 12 ++++++++++--
 plugins/auth-backend/src/identity/KeyStores.ts     | 14 ++++----------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/plugins/auth-backend/src/database/AuthDatabase.ts b/plugins/auth-backend/src/database/AuthDatabase.ts
index 9c112e0aac..97e4236754 100644
--- a/plugins/auth-backend/src/database/AuthDatabase.ts
+++ b/plugins/auth-backend/src/database/AuthDatabase.ts
@@ -39,6 +39,7 @@ export class AuthDatabase {
     return new AuthDatabase(database);
   }
 
+  /** @internal */
   static forTesting(): AuthDatabase {
     const config = new ConfigReader({
       backend: {
diff --git a/plugins/auth-backend/src/identity/KeyStores.test.ts b/plugins/auth-backend/src/identity/KeyStores.test.ts
index d186db6c46..6c034eb64b 100644
--- a/plugins/auth-backend/src/identity/KeyStores.test.ts
+++ b/plugins/auth-backend/src/identity/KeyStores.test.ts
@@ -20,6 +20,7 @@ import { DatabaseKeyStore } from './DatabaseKeyStore';
 import { FirestoreKeyStore } from './FirestoreKeyStore';
 import { KeyStores } from './KeyStores';
 import { MemoryKeyStore } from './MemoryKeyStore';
+import { getVoidLogger } from '@backstage/backend-common';
 
 describe('KeyStores', () => {
   const defaultConfigOptions = {
@@ -33,7 +34,10 @@ describe('KeyStores', () => {
 
   it('reads auth section from config', async () => {
     const configSpy = jest.spyOn(defaultConfig, 'getOptionalConfig');
-    const keyStore = await KeyStores.fromConfig(defaultConfig);
+    const keyStore = await KeyStores.fromConfig(defaultConfig, {
+      logger: getVoidLogger(),
+      database: AuthDatabase.forTesting(),
+    });
 
     expect(keyStore).toBeInstanceOf(MemoryKeyStore);
     expect(configSpy).toHaveBeenCalledWith('auth.keyStore');
@@ -46,6 +50,7 @@ describe('KeyStores', () => {
 
   it('can handle without auth config', async () => {
     const keyStore = await KeyStores.fromConfig(new ConfigReader({}), {
+      logger: getVoidLogger(),
       database: AuthDatabase.forTesting(),
     });
     expect(keyStore).toBeInstanceOf(DatabaseKeyStore);
@@ -72,7 +77,10 @@ describe('KeyStores', () => {
       },
     };
     const config = new ConfigReader(configOptions);
-    const keyStore = await KeyStores.fromConfig(config);
+    const keyStore = await KeyStores.fromConfig(config, {
+      logger: getVoidLogger(),
+      database: AuthDatabase.forTesting(),
+    });
 
     expect(keyStore).toBeInstanceOf(FirestoreKeyStore);
     expect(createSpy).toHaveBeenCalledWith(
diff --git a/plugins/auth-backend/src/identity/KeyStores.ts b/plugins/auth-backend/src/identity/KeyStores.ts
index d000a96879..4cd8f91842 100644
--- a/plugins/auth-backend/src/identity/KeyStores.ts
+++ b/plugins/auth-backend/src/identity/KeyStores.ts
@@ -26,7 +26,7 @@ import { MemoryKeyStore } from './MemoryKeyStore';
 import { KeyStore } from './types';
 
 type Options = {
-  logger?: Logger;
+  logger: Logger;
   database: AuthDatabase;
 };
 
@@ -37,21 +37,15 @@ export class KeyStores {
    *
    * @returns a KeyStore store
    */
-  static async fromConfig(
-    config: Config,
-    options?: Options,
-  ): Promise<KeyStore> {
-    const { logger, database } = options ?? {};
+  static async fromConfig(config: Config, options: Options): Promise<KeyStore> {
+    const { logger, database } = options;
 
     const ks = config.getOptionalConfig('auth.keyStore');
     const provider = ks?.getOptionalString('provider') ?? 'database';
 
-    logger?.info(`Configuring "${provider}" as KeyStore provider`);
+    logger.info(`Configuring "${provider}" as KeyStore provider`);
 
     if (provider === 'database') {
-      if (!database) {
-        throw new Error('This KeyStore provider requires a database');
-      }
       return new DatabaseKeyStore(await database.get());
     }