Merge pull request #765 from MasterKale/feat/improved-metadataservice-logging

Improve MetadataService logging

Author: MasterKale

packages/server/src/helpers/index.ts

@@ -15,3 +15,4 @@ export * from './verifySignature.ts';
 export * from './iso/index.ts';
 export * from '../metadata/verifyMDSBlob.ts';
 export * as cose from './cose.ts';
+export { type SimpleWebAuthnLogger } from './logging.ts';

packages/server/src/helpers/logging.test.ts

@@ -0,0 +1,23 @@
+import { assertEquals } from '@std/assert';
+import { assertSpyCallArg, spy } from '@std/testing/mock';
+
+import { buildLoggerAllMethods } from './logging.ts';
+
+Deno.test('should define default methods for undefined methods', () => {
+  const logger = buildLoggerAllMethods({});
+
+  assertEquals(typeof logger.debug, 'function');
+  assertEquals(typeof logger.info, 'function');
+  assertEquals(typeof logger.warn, 'function');
+  assertEquals(typeof logger.error, 'function');
+});
+
+Deno.test('should use provided logger methods', () => {
+  const _debugSpy = spy();
+  const logger = buildLoggerAllMethods({ debug: _debugSpy });
+
+  logger.debug('SimpleWebAuthn');
+
+  assertEquals(logger.debug, _debugSpy);
+  assertSpyCallArg(_debugSpy, 0, 0, 'SimpleWebAuthn');
+});

packages/server/src/helpers/logging.ts

@@ -1,20 +1,62 @@
-// const defaultLogger = debug('SimpleWebAuthn');
-
 /**
- * Generate an instance of a `debug` logger that extends off of the "simplewebauthn" namespace for
- * consistent naming.
- *
- * See https://www.npmjs.com/package/debug for information on how to control logging output when
- * using @simplewebauthn/server
+ * A basic logging interface that enables projects to capture logging output from SimpleWebAuthn
+ * using whatever logging method is appropriate for the project. Logging levels can be defined
+ * independently to only capture desired levels.
  *
- * Example:
+ * For example, a project using `console` statements to capture logs can use the following
+ * implementation of this interface:
  *
+ * ```ts
+ * const ConsoleLogger: SimpleWebAuthnLogger = {
+ *   // debug(message: string, ...args: unknown[]) { console.debug(message, ...args); },
+ *   info(message: string, ...args: unknown[]) { console.info(message, ...args); },
+ *   warn(message: string, ...args: unknown[]) { console.warn(message, ...args); },
+ *   error(message: string, ...args: unknown[]) { console.error(message, ...args); },
+ * };
  * ```
- * const log = getLogger('mds');
- * log('hello'); // simplewebauthn:mds hello +0ms
- * ```
  */
-export function getLogger(_name: string): (message: string, ..._rest: unknown[]) => void {
-  // This is a noop for now while I search for a better debug logger technique
-  return (_message, ..._rest) => {};
+export interface SimpleWebAuthnLogger {
+  debug?: (message: string, ...args: unknown[]) => void;
+  info?: (message: string, ...args: unknown[]) => void;
+  warn?: (message: string, ...args: unknown[]) => void;
+  error?: (message: string, ...args: unknown[]) => void;
+}
+
+/**
+ * A logger instance that doesn't do anything. Useful as a default argument when no custom instance
+ * of the `SimpleWebAuthnLogger` interface is specified.
+ */
+export const DefaultNoopLogger: Required<SimpleWebAuthnLogger> = {
+  debug() {},
+  info() {},
+  warn() {},
+  error() {},
+};
+
+/**
+ * Generate an instance of SimpleWebAuthnLogger that defines all methods. Any logging method not
+ * defined on `logger` will be a no-op.
+ */
+export function buildLoggerAllMethods(
+  logger: SimpleWebAuthnLogger,
+): Required<SimpleWebAuthnLogger> {
+  const toReturn: Required<SimpleWebAuthnLogger> = { ...DefaultNoopLogger };
+
+  if (logger.debug) {
+    toReturn.debug = logger.debug;
+  }
+
+  if (logger.info) {
+    toReturn.info = logger.info;
+  }
+
+  if (logger.warn) {
+    toReturn.warn = logger.warn;
+  }
+
+  if (logger.error) {
+    toReturn.error = logger.error;
+  }
+
+  return toReturn;
 }

packages/server/src/services/metadataService.test.ts

@@ -1,6 +1,6 @@
 import { assertEquals, assertRejects } from '@std/assert';
 import { afterEach, beforeEach, describe, it } from '@std/testing/bdd';
-import { assertSpyCallArg, assertSpyCalls, type Stub, stub } from '@std/testing/mock';
+import { assertSpyCallArg, assertSpyCalls, spy, type Stub, stub } from '@std/testing/mock';
 
 import { _fetchInternals } from '../helpers/fetch.ts';
 
@@ -95,6 +95,25 @@ describe('Method: getStatement()', () => {
   });
 });
 
+describe('Behavior: logging', () => {
+  it('should use provided logger', async () => {
+    const _debugSpy = spy();
+
+    await MetadataService.initialize({
+      mdsServers: [],
+      statements: [],
+      logger: { debug: _debugSpy },
+    });
+
+    /**
+     * NOTE TO FUTURE SELF: As of June 2026 the content of the logging is less important here than
+     * the fact that the service used the provided logger to communicate internal goings on.
+     */
+    assertSpyCallArg(_debugSpy, 0, 0, 'MetadataService is REFRESHING');
+    assertSpyCallArg(_debugSpy, 1, 0, 'MetadataService is READY');
+  });
+});
+
 const localStatementAAGUID = '91dfead7-959e-4475-ad26-9b0d482be089';
 const localStatement: MetadataStatement = {
   legalHeader: 'https://fidoalliance.org/metadata/metadata-statement-legal-header/',

packages/server/src/services/metadataService.ts

@@ -1,7 +1,11 @@
 import { convertAAGUIDToString } from '../helpers/convertAAGUIDToString.ts';
 import type { MetadataBLOBPayloadEntry, MetadataStatement } from '../metadata/mdsTypes.ts';
 import { verifyMDSBlob } from '../metadata/verifyMDSBlob.ts';
-import { getLogger } from '../helpers/logging.ts';
+import {
+  buildLoggerAllMethods,
+  DefaultNoopLogger,
+  type SimpleWebAuthnLogger,
+} from '../helpers/logging.ts';
 import { fetch } from '../helpers/fetch.ts';
 import type { Uint8Array_ } from '../types/index.ts';
 
@@ -44,8 +48,6 @@ enum SERVICE_STATE {
  */
 export type VerificationMode = 'permissive' | 'strict';
 
-const log = getLogger('MetadataService');
-
 interface MetadataService {
   /**
    * Prepare the service to handle remote MDS servers and/or cache local metadata statements.
@@ -65,6 +67,7 @@ interface MetadataService {
     mdsServers?: string[];
     statements?: MetadataStatement[];
     verificationMode?: VerificationMode;
+    logger?: SimpleWebAuthnLogger;
   }): Promise<void>;
   /**
    * Get a metadata statement for a given AAGUID.
@@ -86,18 +89,27 @@ export class BaseMetadataService implements MetadataService {
   private statementCache: { [aaguid: string]: CachedBLOBEntry } = {};
   private state: SERVICE_STATE = SERVICE_STATE.DISABLED;
   private verificationMode: VerificationMode = 'strict';
+  private logger: Required<SimpleWebAuthnLogger> = DefaultNoopLogger;
 
   async initialize(
     opts: {
       mdsServers?: string[];
       statements?: MetadataStatement[];
       verificationMode?: VerificationMode;
+      logger?: SimpleWebAuthnLogger;
     } = {},
   ): Promise<void> {
     // Reset statement cache
     this.statementCache = {};
 
-    const { mdsServers = [defaultURLMDS], statements, verificationMode } = opts;
+    const {
+      mdsServers = [defaultURLMDS],
+      statements,
+      verificationMode,
+      logger = DefaultNoopLogger,
+    } = opts;
+
+    this.logger = buildLoggerAllMethods(logger);
 
     this.setState(SERVICE_STATE.REFRESHING);
 
@@ -124,7 +136,7 @@ export class BaseMetadataService implements MetadataService {
         }
       });
 
-      log(`Cached ${statementsAdded} local statements`);
+      this.logger.info(`Cached ${statementsAdded} local statements`);
     }
 
     /**
@@ -149,15 +161,15 @@ export class BaseMetadataService implements MetadataService {
           await this.verifyBlob(blob, cachedMDS);
         } catch (err) {
           // Notify of the error and move on
-          log(`Could not download BLOB from ${url}:`, err);
+          this.logger.error(`Could not download BLOB from ${url}:`, err);
           numServers -= 1;
         }
       }
 
       // Calculate the difference to get the total number of new statements we successfully added
       const newCacheCount = Object.keys(this.statementCache).length;
       const cacheDiff = newCacheCount - currentCacheCount;
-      log(
+      this.logger.info(
         `Cached ${cacheDiff} statements from ${numServers} metadata server(s)`,
       );
     }
@@ -288,7 +300,7 @@ export class BaseMetadataService implements MetadataService {
         // TODO (Feb 2026): It'd be more actionable for devs if a specific error was raised here,
         // then this message was logged higher up when it can include the array index of the stale
         // blob.
-        log(
+        this.logger.warn(
           `⚠️ This MDS blob (serial: ${payload.no}) contains stale data as of ${parsedNextUpdate.toISOString()}. Please consider re-initializing MetadataService with a newer MDS blob.`,
         );
       }
@@ -337,11 +349,11 @@ export class BaseMetadataService implements MetadataService {
     this.state = newState;
 
     if (newState === SERVICE_STATE.DISABLED) {
-      log('MetadataService is DISABLED');
+      this.logger.debug('MetadataService is DISABLED');
     } else if (newState === SERVICE_STATE.REFRESHING) {
-      log('MetadataService is REFRESHING');
+      this.logger.debug('MetadataService is REFRESHING');
     } else if (newState === SERVICE_STATE.READY) {
-      log('MetadataService is READY');
+      this.logger.debug('MetadataService is READY');
     }
   }
 }