WIP - fix(passkey): Use correct challenge value in redis

.claude/settings.local.json

@@ -0,0 +1,8 @@
+{
+  "permissions": {
+    "allow": [
+      "Bash(node:*)",
+      "Bash(head -5 grep -r \"require\\\\|import\" /Users/dschomburg/Code/fxa/node_modules/@simplewebauthn/server/esm/registration/verifyRegistrationResponse.js)"
+    ]
+  }
+}

libs/accounts/passkey/src/lib/passkey.challenge.manager.in.spec.ts

@@ -10,6 +10,7 @@ import { StatsDService } from '@fxa/shared/metrics/statsd';
 import { PasskeyChallengeManager } from './passkey.challenge.manager';
 import { PasskeyConfig } from './passkey.config';
 import { PASSKEY_CHALLENGE_REDIS } from './passkey.provider';
+import { generateRandomChallenge } from './webauthn-adapter';
 
 const mockLogger = {
   log: jest.fn(),
@@ -73,9 +74,10 @@ afterAll(async () => {
 });
 
 describe('PasskeyChallengeManager (integration)', () => {
-  describe('generateRegistrationChallenge', () => {
+  describe('storeRegistrationChallenge', () => {
     it('stores challenge with uid and type=registration', async () => {
-      const challenge = await manager.generateRegistrationChallenge('deadbeef');
+      const challenge = generateRandomChallenge();
+      await manager.storeRegistrationChallenge(challenge, 'deadbeef');
 
       const stored = await manager.consumeRegistrationChallenge(
         challenge,
@@ -90,7 +92,8 @@ describe('PasskeyChallengeManager (integration)', () => {
 
   describe('generateAuthenticationChallenge', () => {
     it('stores challenge with no uid and type=authentication', async () => {
-      const challenge = await manager.generateAuthenticationChallenge();
+      const challenge = generateRandomChallenge();
+      await manager.storeAuthenticationChallenge(challenge);
 
       const stored = await manager.consumeAuthenticationChallenge(challenge);
 
@@ -102,7 +105,8 @@ describe('PasskeyChallengeManager (integration)', () => {
 
   describe('generateUpgradeChallenge', () => {
     it('stores challenge with uid and type=upgrade', async () => {
-      const challenge = await manager.generateUpgradeChallenge('cafebabe');
+      const challenge = generateRandomChallenge();
+      await manager.storeUpgradeChallenge(challenge, 'cafebabe');
 
       const stored = await manager.consumeUpgradeChallenge(
         challenge,
@@ -117,7 +121,8 @@ describe('PasskeyChallengeManager (integration)', () => {
 
   describe('consumeChallenge', () => {
     it('returns null on second consume (single-use enforcement)', async () => {
-      const challenge = await manager.generateRegistrationChallenge('deadbeef');
+      const challenge = generateRandomChallenge();
+      await manager.storeRegistrationChallenge(challenge, 'deadbeef');
 
       await manager.consumeRegistrationChallenge(challenge, 'deadbeef');
 
@@ -150,8 +155,8 @@ describe('PasskeyChallengeManager (integration)', () => {
       const moduleRef = await buildTestModule(redis, shortConfig, mockLogger);
 
       const shortManager = moduleRef.get(PasskeyChallengeManager);
-      const challenge =
-        await shortManager.generateRegistrationChallenge('deadbeef');
+      const challenge = generateRandomChallenge();
+      await shortManager.storeRegistrationChallenge(challenge, 'deadbeef');
 
       // Wait longer than the TTL to ensure the challenge expires
       await new Promise((resolve) => setTimeout(resolve, 1100));
@@ -166,7 +171,8 @@ describe('PasskeyChallengeManager (integration)', () => {
 
   describe('deleteChallenge', () => {
     it('removes the key from Redis', async () => {
-      const challenge = await manager.generateRegistrationChallenge('deadbeef');
+      const challenge = generateRandomChallenge();
+      await manager.storeRegistrationChallenge(challenge, 'deadbeef');
 
       await manager.deleteChallenge('registration', challenge, 'deadbeef');
 

libs/accounts/passkey/src/lib/passkey.challenge.manager.spec.ts

@@ -7,6 +7,7 @@ import {
   StoredChallenge,
 } from './passkey.challenge.manager';
 import { PasskeyConfig } from './passkey.config';
+import { generateRandomChallenge } from './webauthn-adapter';
 
 const mockRedis = {
   set: jest.fn(),
@@ -56,20 +57,14 @@ describe('PasskeyChallengeManager', () => {
     );
   });
 
-  describe('generateRegistrationChallenge', () => {
-    it('returns a base64url-encoded challenge string', async () => {
-      mockRedis.set.mockResolvedValue('OK');
-      const result = await manager.generateRegistrationChallenge('deadbeef');
-
-      expect(result).toBe(MOCK_CHALLENGE);
-    });
-
+  describe('storeRegistrationChallenge', () => {
     it('calls redis.set with the correct key and TTL', async () => {
       mockRedis.set.mockResolvedValue('OK');
-      await manager.generateRegistrationChallenge('deadbeef');
+      const challenge = generateRandomChallenge();
+      await manager.storeRegistrationChallenge(challenge, 'deadbeef');
 
       expect(mockRedis.set).toHaveBeenCalledWith(
-        `passkey:challenge:registration:${MOCK_CHALLENGE}:deadbeef`,
+        `passkey:challenge:registration:${challenge}:deadbeef`,
         expect.any(String),
         'EX',
         CHALLENGE_TIMEOUT_MS / 1000
@@ -78,7 +73,8 @@ describe('PasskeyChallengeManager', () => {
 
     it('increments statsd counter for generated challenges', async () => {
       mockRedis.set.mockResolvedValue('OK');
-      await manager.generateRegistrationChallenge('deadbeef');
+      const challenge = generateRandomChallenge();
+      await manager.storeRegistrationChallenge(challenge, 'deadbeef');
 
       expect(mockStatsd.increment).toHaveBeenCalledWith(
         'passkey.challenge.generated',
@@ -91,14 +87,15 @@ describe('PasskeyChallengeManager', () => {
       const dateNowSpy = jest.spyOn(Date, 'now').mockReturnValue(fakeNow);
 
       mockRedis.set.mockResolvedValue('OK');
-      await manager.generateRegistrationChallenge('deadbeef');
+      const challenge = generateRandomChallenge();
+      await manager.storeRegistrationChallenge(challenge, 'deadbeef');
 
       dateNowSpy.mockRestore();
 
       const [, rawJson] = mockRedis.set.mock.calls[0];
       const stored: StoredChallenge = JSON.parse(rawJson);
 
-      expect(stored.challenge).toBe(MOCK_CHALLENGE);
+      expect(stored.challenge).toBe(challenge);
       expect(stored.type).toBe('registration');
       expect(stored.uid).toBe('deadbeef');
       expect(stored.createdAt).toBe(fakeNow);
@@ -107,21 +104,23 @@ describe('PasskeyChallengeManager', () => {
 
     it('throws if redis.set rejects', async () => {
       mockRedis.set.mockRejectedValue(new Error('Redis connection lost'));
+      const challenge = generateRandomChallenge();
       await expect(
-        manager.generateRegistrationChallenge('deadbeef')
+        manager.storeRegistrationChallenge(challenge, 'deadbeef')
       ).rejects.toThrow('Redis connection lost');
     });
   });
 
   describe('generateAuthenticationChallenge', () => {
     it('stores the challenge with type=authentication and no uid', async () => {
       mockRedis.set.mockResolvedValue('OK');
-      await manager.generateAuthenticationChallenge();
+      const challenge = generateRandomChallenge();
+      await manager.storeAuthenticationChallenge(challenge);
 
       const [key, rawJson] = mockRedis.set.mock.calls[0];
       const stored: StoredChallenge = JSON.parse(rawJson);
 
-      expect(key).toBe(`passkey:challenge:authentication:${MOCK_CHALLENGE}`);
+      expect(key).toBe(`passkey:challenge:authentication:${challenge}`);
       expect(stored.type).toBe('authentication');
       expect(stored.uid).toBeUndefined();
     });
@@ -130,12 +129,13 @@ describe('PasskeyChallengeManager', () => {
   describe('generateUpgradeChallenge', () => {
     it('stores the challenge with type=upgrade and uid', async () => {
       mockRedis.set.mockResolvedValue('OK');
-      await manager.generateUpgradeChallenge('cafebabe');
+      const challenge = generateRandomChallenge();
+      await manager.storeUpgradeChallenge(challenge, 'cafebabe');
 
       const [key, rawJson] = mockRedis.set.mock.calls[0];
       const stored: StoredChallenge = JSON.parse(rawJson);
 
-      expect(key).toBe(`passkey:challenge:upgrade:${MOCK_CHALLENGE}:cafebabe`);
+      expect(key).toBe(`passkey:challenge:upgrade:${challenge}:cafebabe`);
       expect(stored.type).toBe('upgrade');
       expect(stored.uid).toBe('cafebabe');
     });

libs/accounts/passkey/src/lib/passkey.challenge.manager.ts

@@ -2,7 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import { randomBytes } from 'crypto';
 import { Injectable, Inject, LoggerService } from '@nestjs/common';
 import { LOGGER_PROVIDER } from '@fxa/shared/log';
 import { StatsD, StatsDService } from '@fxa/shared/metrics/statsd';
@@ -117,17 +116,20 @@ export class PasskeyChallengeManager {
    * @param uid - Hex-encoded uid of the user.
    * @returns Base64url-encoded 32-byte challenge string.
    */
-  async generateRegistrationChallenge(uid: string): Promise<string> {
-    return this.generateChallenge('registration', uid);
+  async storeRegistrationChallenge(
+    challenge: string,
+    uid: string
+  ): Promise<void> {
+    await this.storeChallenge('registration', challenge, uid);
   }
 
   /**
    * Generates an authentication challenge for the WebAuthn assertion ceremony.
    *
    * @returns Base64url-encoded 32-byte challenge string.
    */
-  async generateAuthenticationChallenge(): Promise<string> {
-    return this.generateChallenge('authentication');
+  async storeAuthenticationChallenge(challenge: string): Promise<void> {
+    await this.storeChallenge('authentication', challenge);
   }
 
   /**
@@ -136,8 +138,8 @@ export class PasskeyChallengeManager {
    * @param uid - Hex-encoded uid of the user.
    * @returns Base64url-encoded 32-byte challenge string.
    */
-  async generateUpgradeChallenge(uid: string): Promise<string> {
-    return this.generateChallenge('upgrade', uid);
+  async storeUpgradeChallenge(challenge: string, uid: string): Promise<void> {
+    await this.storeChallenge('upgrade', challenge, uid);
   }
 
   /**
@@ -241,11 +243,11 @@ export class PasskeyChallengeManager {
     }
   }
 
-  private async generateChallenge(
+  private async storeChallenge(
     type: ChallengeType,
+    challenge: string,
     uid?: string
-  ): Promise<string> {
-    const challenge = randomBytes(32).toString('base64url');
+  ): Promise<void> {
     const now = Date.now();
     const timeout = this.config.challengeTimeout;
     const ttlSeconds = Math.ceil(timeout / 1000);
@@ -263,8 +265,6 @@ export class PasskeyChallengeManager {
 
     this.log?.debug?.('passkey.challenge.generated', { type, uid });
     this.statsd?.increment('passkey.challenge.generated', { type });
-
-    return challenge;
   }
 
   private buildKey(