Merge pull request #20342 from mozilla/FXA-13402

refactor(passkey): restrict NewPasskey type to repository layer

Author: MagentaManifold

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

@@ -93,23 +93,20 @@ describe('PasskeyManager (Integration)', () => {
     it('persists the passkey with correct field values', async () => {
       const uid = await createTestAccount();
       // Pass lastUsedAt: null explicitly — PasskeyFactory may randomise it
-      const newPasskey = PasskeyFactory({
+      const passkey = PasskeyFactory({
         uid,
-        backupEligible: 1,
-        backupState: 0,
-        prfEnabled: 0,
+        backupEligible: true,
+        backupState: false,
+        prfEnabled: false,
         lastUsedAt: null,
       });
 
-      await manager.registerPasskey(newPasskey);
+      await manager.registerPasskey(passkey);
 
-      const found = await findPasskeyByCredentialId(
-        db,
-        newPasskey.credentialId
-      );
+      const found = await findPasskeyByCredentialId(db, passkey.credentialId);
       expect(found?.uid).toEqual(uid);
-      expect(found?.credentialId).toEqual(newPasskey.credentialId);
-      expect(found?.name).toBe(newPasskey.name);
+      expect(found?.credentialId).toEqual(passkey.credentialId);
+      expect(found?.name).toBe(passkey.name);
       expect(found?.backupEligible).toBe(true);
       expect(found?.backupState).toBe(false);
       expect(found?.prfEnabled).toBe(false);
@@ -146,7 +143,7 @@ describe('PasskeyManager (Integration)', () => {
   describe('updatePasskeyAfterAuth', () => {
     it('updates signCount, backupState, and lastUsedAt', async () => {
       const uid = await createTestAccount();
-      const passkey = PasskeyFactory({ uid, signCount: 0, backupState: 0 });
+      const passkey = PasskeyFactory({ uid, signCount: 0, backupState: false });
       await manager.registerPasskey(passkey);
 
       const updated = await manager.updatePasskeyAfterAuth(
@@ -166,7 +163,7 @@ describe('PasskeyManager (Integration)', () => {
 
     it('sets backupState to false when passed false', async () => {
       const uid = await createTestAccount();
-      const passkey = PasskeyFactory({ uid, backupState: 1 });
+      const passkey = PasskeyFactory({ uid, backupState: true });
       await manager.registerPasskey(passkey);
 
       await manager.updatePasskeyAfterAuth(
@@ -228,7 +225,7 @@ describe('PasskeyManager (Integration)', () => {
 
     it('succeeds when both stored and new signCount are zero', async () => {
       const uid = await createTestAccount();
-      const passkey = PasskeyFactory({ uid, signCount: 0, backupState: 0 });
+      const passkey = PasskeyFactory({ uid, signCount: 0, backupState: false });
       await manager.registerPasskey(passkey);
 
       // Authenticators that never increment always return 0 — this is valid per WebAuthn spec

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

@@ -74,22 +74,22 @@ describe('PasskeyManager', () => {
 
   describe('registerPasskey', () => {
     it('checks the limit and inserts the passkey', async () => {
-      const newPasskey = PasskeyFactory();
+      const passkey = PasskeyFactory();
 
       (PasskeyRepository.countPasskeysByUid as jest.Mock).mockResolvedValue(1);
       (PasskeyRepository.insertPasskey as jest.Mock).mockResolvedValue(
         undefined
       );
 
-      await manager.registerPasskey(newPasskey);
+      await manager.registerPasskey(passkey);
 
       expect(PasskeyRepository.countPasskeysByUid).toHaveBeenCalledWith(
         mockDb,
-        newPasskey.uid
+        passkey.uid
       );
       expect(PasskeyRepository.insertPasskey).toHaveBeenCalledWith(
         mockDb,
-        newPasskey
+        passkey
       );
     });
 

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

@@ -7,7 +7,6 @@ import {
   AccountDatabase,
   AccountDbProvider,
   Passkey,
-  NewPasskey,
 } from '@fxa/shared/db/mysql/account';
 import { LOGGER_PROVIDER } from '@fxa/shared/log';
 import { StatsD, StatsDService } from '@fxa/shared/metrics/statsd';
@@ -57,7 +56,7 @@ export class PasskeyManager {
    * @throws {AppError} (passkeyLimitReached) if the user has reached the maximum passkey count
    * @throws {AppError} (passkeyAlreadyRegistered) if credentialId is already registered
    */
-  async registerPasskey(passkey: NewPasskey): Promise<void> {
+  async registerPasskey(passkey: Passkey): Promise<void> {
     const uidHex = passkey.uid.toString('hex');
 
     await this.checkPasskeyCount(passkey.uid);

libs/accounts/passkey/src/lib/passkey.repository.ts

@@ -88,13 +88,20 @@ export async function findPasskeyByCredentialId(
  * Insert a new passkey record.
  *
  * @param db - Database instance
- * @param passkey - New passkey data to insert
+ * @param passkey - Passkey data to insert
  */
 export async function insertPasskey(
   db: AccountDatabase,
-  passkey: NewPasskey
+  passkey: Passkey
 ): Promise<void> {
-  await db.insertInto('passkeys').values(passkey).execute();
+  const newPasskey: NewPasskey = {
+    ...passkey,
+    transports: JSON.stringify(passkey.transports),
+    backupEligible: passkey.backupEligible ? 1 : 0,
+    backupState: passkey.backupState ? 1 : 0,
+    prfEnabled: passkey.prfEnabled ? 1 : 0,
+  };
+  await db.insertInto('passkeys').values(newPasskey).execute();
 }
 
 /**

libs/accounts/passkey/src/lib/passkey.service.spec.ts

@@ -51,7 +51,7 @@ describe('PasskeyService', () => {
     name: 'Test Passkey',
     createdAt: Date.now(),
     lastUsedAt: null,
-    transports: '[]',
+    transports: [],
     aaguid: Buffer.alloc(16),
   };
 
@@ -203,7 +203,7 @@ describe('PasskeyService', () => {
       aaguid: MOCK_AAGUID_ZEROS,
       backupEligible: false,
       backupState: false,
-      prfEnabled: false,
+      prfEnabled: true,
     };
 
     beforeEach(() => {
@@ -283,7 +283,7 @@ describe('PasskeyService', () => {
       ).toHaveBeenCalledWith(MOCK_CHALLENGE, MOCK_UID.toString('hex'));
     });
 
-    it('calls passkeyManager.registerPasskey with correct NewPasskey shape', async () => {
+    it('calls passkeyManager.registerPasskey with correct Passkey shape', async () => {
       await service.createPasskeyFromRegistrationResponse(
         MOCK_UID,
         mockResponse,
@@ -293,40 +293,9 @@ describe('PasskeyService', () => {
       expect(mockManager.registerPasskey).toHaveBeenCalledWith(
         expect.objectContaining({
           uid: MOCK_UID,
-          credentialId: MOCK_CREDENTIAL_ID,
-          publicKey: MOCK_PUBLIC_KEY,
-          signCount: 0,
-          transports: JSON.stringify(['internal']),
-          aaguid: MOCK_AAGUID_ZEROS,
+          createdAt: expect.any(Number),
           lastUsedAt: null,
-          backupEligible: 0,
-          backupState: 0,
-          prfEnabled: 0,
-        })
-      );
-    });
-
-    it('sets backupEligible=1, backupState=1 and prfEnabled=1 when flags are true', async () => {
-      mockVerifyWebauthnRegistrationResponse.mockResolvedValue({
-        verified: true,
-        data: {
           ...mockVerifiedData,
-          backupEligible: true,
-          backupState: true,
-          prfEnabled: true,
-        },
-      });
-      await service.createPasskeyFromRegistrationResponse(
-        MOCK_UID,
-        mockResponse,
-        MOCK_CHALLENGE
-      );
-
-      expect(mockManager.registerPasskey).toHaveBeenCalledWith(
-        expect.objectContaining({
-          backupEligible: 1,
-          backupState: 1,
-          prfEnabled: 1,
         })
       );
     });
@@ -350,7 +319,7 @@ describe('PasskeyService', () => {
           lastUsedAt: null,
           backupEligible: false,
           backupState: false,
-          prfEnabled: false,
+          prfEnabled: true,
         })
       );
     });

libs/accounts/passkey/src/lib/passkey.service.ts

@@ -10,7 +10,7 @@ import type {
 import { LOGGER_PROVIDER } from '@fxa/shared/log';
 import { StatsD, StatsDService } from '@fxa/shared/metrics/statsd';
 import * as Sentry from '@sentry/nestjs';
-import type { NewPasskey, Passkey } from '@fxa/shared/db/mysql/account';
+import type { Passkey } from '@fxa/shared/db/mysql/account';
 import type {
   AuthenticatorTransportFuture,
   PublicKeyCredentialCreationOptionsJSON,
@@ -200,17 +200,7 @@ export class PasskeyService {
       ...result.data,
     };
 
-    // TODO: update repository, manager, and service layers to accept/return a
-    // Passkey object directly instead of mapping to NewPasskey. See FXA-13402.
-    const newPasskey: NewPasskey = {
-      ...passkey,
-      transports: JSON.stringify(passkey.transports),
-      backupEligible: passkey.backupEligible ? 1 : 0,
-      backupState: passkey.backupState ? 1 : 0,
-      prfEnabled: passkey.prfEnabled ? 1 : 0,
-    };
-
-    await this.passkeyManager.registerPasskey(newPasskey);
+    await this.passkeyManager.registerPasskey(passkey);
 
     this.metrics.increment('passkey.registration.success');
     this.log?.log('passkey.registered', { uid: uidHex });

libs/shared/db/mysql/account/src/lib/factories.ts

@@ -8,7 +8,7 @@ import {
   Account,
   AccountCustomer,
   NewCart,
-  NewPasskey,
+  Passkey,
   PaypalCustomer,
   SessionToken,
   UnverifiedToken,
@@ -184,16 +184,16 @@ export const RecoveryPhoneFactory = (override?: Partial<RecoveryPhone>) => ({
   ...override,
 });
 
-export const PasskeyFactory = (override?: Partial<NewPasskey>): NewPasskey => ({
+export const PasskeyFactory = (override?: Partial<Passkey>): Passkey => ({
   uid: getHexBuffer(32),
   credentialId: getHexBuffer(faker.number.int({ min: 32, max: 128 })),
   publicKey: getHexBuffer(128),
   signCount: 0,
   transports: faker.helpers.arrayElement([
-    JSON.stringify(['internal']),
-    JSON.stringify(['usb']),
-    JSON.stringify(['internal', 'hybrid']),
-    JSON.stringify([]),
+    ['internal'],
+    ['usb'],
+    ['internal', 'hybrid'],
+    [],
   ]),
   aaguid: faker.datatype.boolean()
     ? getHexBuffer(32) // Real AAGUID (32 hex chars = 16 bytes)
@@ -207,8 +207,8 @@ export const PasskeyFactory = (override?: Partial<NewPasskey>): NewPasskey => ({
   ]),
   createdAt: faker.date.recent().getTime(),
   lastUsedAt: faker.datatype.boolean() ? faker.date.recent().getTime() : null,
-  backupEligible: faker.helpers.arrayElement([0, 1]),
-  backupState: faker.helpers.arrayElement([0, 1]),
-  prfEnabled: faker.helpers.arrayElement([0, 1]),
+  backupEligible: faker.datatype.boolean(),
+  backupState: faker.datatype.boolean(),
+  prfEnabled: faker.datatype.boolean(),
   ...override,
 });

libs/shared/db/mysql/account/src/lib/kysely-types.ts

@@ -248,7 +248,7 @@ export interface Passkeys {
   credentialId: Buffer;
   publicKey: Buffer;
   signCount: number;
-  transports: Json;
+  transports: JSONColumnType<string[]>;
   aaguid: Buffer;
   name: string;
   createdAt: number;

libs/shared/db/mysql/account/src/test/passkeys.sql

@@ -3,7 +3,7 @@ CREATE TABLE `passkeys` (
   `credentialId` varbinary(1023) NOT NULL,
   `publicKey` blob NOT NULL,
   `signCount` int unsigned NOT NULL DEFAULT '0',
-  `transports` varchar(255) COLLATE utf8mb4_bin DEFAULT NULL,
+  `transports` JSON NOT NULL,
   `aaguid` binary(16) DEFAULT NULL,
   `name` varchar(255) COLLATE utf8mb4_bin DEFAULT NULL,
   `createdAt` bigint(20) unsigned NOT NULL,