Redact legacy migration verifier and preserve V2 TOTP in V3

src/services/assistedKeys.ts/ss-signer/signerV3.ts

@@ -1035,11 +1035,20 @@ export class SignerV3 {
     let [docV3] = await signerV3Model.find({ id: xfp }); // won't be available typically, unless previous migration
     // request failed at updateCosignersToSignerMap step
     if (!docV3) {
+      // V2 stores the TOTP key inside verifier.twoFAKey, while V3 stores the
+      // key as a string. Passing the V2 subdocument to Mongoose stringifies it.
+      const storedVerifier = policy?.verification?.verifier;
+      const verifier = typeof storedVerifier === "string" ? storedVerifier : storedVerifier?.twoFAKey;
+      if (typeof verifier !== "string" || !verifier) throw new Error("Invalid legacy signer verifier");
+      const policyV3 = {
+        ...(typeof policy.toObject === "function" ? policy.toObject() : policy),
+        verification: { method: policy.verification.method, verifier },
+      };
       const signerInstance = new signerV3Model({
         id: xfp,
         isBIP85,
         xIndex,
-        policy,
+        policy: policyV3,
       });
 
       await signerInstance.save((err) => {
@@ -1059,7 +1068,12 @@ export class SignerV3 {
         bhXpub: xpub,
         masterFingerprint,
         derivationPath,
-        verification: policy.verification,
+        // This migration reuses an existing verifier. Older Keeper clients only
+        // consume migrationSuccessful, so never send the stored TOTP secret back.
+        verification: {
+          method: policy.verification.method,
+          verifier: null,
+        },
       },
     };
   };

tests/unit/signerV3.test.ts

@@ -2,6 +2,7 @@ import SignerV3 from "../../src/services/assistedKeys.ts/ss-signer/signerV3";
 import dbV2 from "../../src/databases/dbV2";
 import bitHyveWallet from "../../src/wallet/bithyve";
 import {
+  CosignersMapUpdateAction,
   VerificationType,
   PermittedAction,
 } from "../../src/interfaces/signer";
@@ -1654,4 +1655,117 @@ describe("SignerV3", () => {
       ).rejects.toThrow("Policy mismatch: spending restrictions do not match");
     });
   });
+
+  describe("migrateSignersV2ToV3", () => {
+    it("migrates a legacy signer without returning its stored verifier", async () => {
+      const storedVerifierKey = "JBSWY3DPEHPK3PXP";
+      const verificationToken = authenticator.generate(storedVerifierKey);
+      const policy = {
+        verification: {
+          method: VerificationType.TWO_FA,
+          verifier: { twoFAKey: storedVerifierKey },
+        },
+        restrictions: { none: false, maxTransactionAmount: 10000 },
+        toObject: jest.fn(() => ({
+          verification: { method: VerificationType.TWO_FA, verifier: { twoFAKey: storedVerifierKey } },
+          restrictions: { none: false, maxTransactionAmount: 10000 },
+        })),
+      };
+      const save = jest.fn((callback) => callback && callback());
+      const signerV3Model: any = jest.fn().mockImplementation((data) => ({
+        ...data,
+        save,
+      }));
+      signerV3Model.find = jest.fn().mockResolvedValue([]);
+      (dbV2.getSignerV2Model as jest.Mock).mockReturnValue({
+        find: jest.fn().mockResolvedValue([{ xIndex: mockSigner.xIndex, policy }]),
+      });
+      (dbV2.getSignerV3Model as jest.Mock).mockReturnValue(signerV3Model);
+      const insertMany = jest.fn().mockResolvedValue([]);
+      const cosignersMapModel: any = jest.fn().mockImplementation((data) => data);
+      cosignersMapModel.find = jest.fn().mockResolvedValue([]);
+      cosignersMapModel.insertMany = insertMany;
+      (dbV2.getCoSignersToSignerMapV3Model as jest.Mock).mockReturnValue(cosignersMapModel);
+      const derive = jest.spyOn(bitHyveWallet, "getRandomXpub").mockReturnValue({
+        xpub: mockSigner.xpub,
+        xfp: mockSigner.xfp,
+        masterFingerprint: mockSigner.masterFingerprint,
+        derivationPath: mockSigner.derivationPath,
+        xIndex: mockSigner.xIndex,
+      });
+      const mapUpdates = [{
+        cosignersId: "unit-test-cosigners",
+        signerId: mockSigner.xfp,
+        action: CosignersMapUpdateAction.ADD,
+      }];
+
+      try {
+        const result = await SignerV3.migrateSignersV2ToV3(
+          "unit-test-vault",
+          "unit-test-app",
+          mapUpdates
+        );
+
+        expect(result.migrationSuccessful).toBe(true);
+        expect(result.setupData.verification).toEqual({
+          method: VerificationType.TWO_FA,
+          verifier: null,
+        });
+        expect(JSON.stringify(result)).not.toContain(storedVerifierKey);
+        const savedPolicy = signerV3Model.mock.calls[0][0].policy;
+        expect(policy.toObject).toHaveBeenCalled();
+        expect(savedPolicy.verification).toEqual({ method: VerificationType.TWO_FA, verifier: storedVerifierKey });
+        expect(savedPolicy.restrictions).toEqual(policy.restrictions);
+        expect(authenticator.verify({ secret: savedPolicy.verification.verifier, token: verificationToken })).toBe(true);
+        expect(policy.verification.verifier).toEqual({ twoFAKey: storedVerifierKey });
+        expect(save).toHaveBeenCalled();
+        expect(insertMany).toHaveBeenCalled();
+      } finally {
+        derive.mockRestore();
+      }
+    });
+
+    it("keeps the verifier redacted when retrying after a V3 signer was saved", async () => {
+      const storedVerifierKey = "UNIT_TEST_RETRY_TOTP_SECRET";
+      const policy = {
+        verification: {
+          method: VerificationType.TWO_FA,
+          verifier: { twoFAKey: storedVerifierKey },
+        },
+      };
+      const signerV3Model: any = jest.fn();
+      signerV3Model.find = jest.fn().mockResolvedValue([{ id: mockSigner.xfp, policy }]);
+      (dbV2.getSignerV2Model as jest.Mock).mockReturnValue({
+        find: jest.fn().mockResolvedValue([{ xIndex: mockSigner.xIndex, policy }]),
+      });
+      (dbV2.getSignerV3Model as jest.Mock).mockReturnValue(signerV3Model);
+      const cosignersMapModel: any = jest.fn().mockImplementation((data) => data);
+      cosignersMapModel.find = jest.fn().mockResolvedValue([]);
+      cosignersMapModel.insertMany = jest.fn().mockResolvedValue([]);
+      (dbV2.getCoSignersToSignerMapV3Model as jest.Mock).mockReturnValue(cosignersMapModel);
+      const derive = jest.spyOn(bitHyveWallet, "getRandomXpub").mockReturnValue({
+        xpub: mockSigner.xpub,
+        xfp: mockSigner.xfp,
+        masterFingerprint: mockSigner.masterFingerprint,
+        derivationPath: mockSigner.derivationPath,
+        xIndex: mockSigner.xIndex,
+      });
+
+      try {
+        const result = await SignerV3.migrateSignersV2ToV3(
+          "unit-test-vault",
+          "unit-test-app",
+          [{ cosignersId: "unit-test-cosigners", signerId: mockSigner.xfp, action: CosignersMapUpdateAction.ADD }]
+        );
+
+        expect(result.migrationSuccessful).toBe(true);
+        expect(result.setupData.verification).toEqual({ method: VerificationType.TWO_FA, verifier: null });
+        expect(JSON.stringify(result)).not.toContain(storedVerifierKey);
+        expect(signerV3Model).not.toHaveBeenCalled();
+        expect(cosignersMapModel.insertMany).toHaveBeenCalled();
+      } finally {
+        derive.mockRestore();
+      }
+    });
+  });
 });