Merge pull request #18363 from mozilla/fxa-remove-unconfirmed

feat(sms): Revoke old recovery phone codes when sending new code
mozilla · Feb 12, 2025 · 5a43625 · 5a43625
2 parents 1868f1e + 794c4f0
commit 5a43625
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 2 deletions.
diff --git a/libs/accounts/recovery-phone/src/lib/recovery-phone.service.spec.ts b/libs/accounts/recovery-phone/src/lib/recovery-phone.service.spec.ts
@@ -114,7 +114,7 @@ describe('RecoveryPhoneService', () => {
     expect(result).toBeTruthy();
   });
 
-  it('Should send new code for setup phone number ', async () => {
+  it('Should send new code for setup phone number', async () => {
     mockOtpManager.generateCode.mockReturnValue(code);
     mockRecoveryPhoneManager.getAllUnconfirmed.mockResolvedValue([
       'this:is:the:code123',
@@ -448,6 +448,48 @@ describe('RecoveryPhoneService', () => {
       mockRecoveryPhoneManager.getConfirmedPhoneNumber.mockRejectedValue(error);
       expect(service.sendCode(uid)).rejects.toThrow(error);
     });
+
+    it('Should send new code for setup phone number', async () => {
+      mockOtpManager.generateCode.mockReturnValue(code);
+      mockRecoveryPhoneManager.getAllUnconfirmed.mockResolvedValue([
+        'this:is:the:code123',
+        'this:is:the:code456',
+      ]);
+
+      mockRecoveryPhoneManager.getConfirmedPhoneNumber.mockResolvedValueOnce({
+        phoneNumber,
+      });
+      mockOtpManager.generateCode.mockResolvedValueOnce(code);
+      mockSmsManager.sendSMS.mockResolvedValue({ status: 'success' });
+
+      const result = await service.sendCode(uid);
+
+      expect(result).toBeTruthy();
+      expect(mockRecoveryPhoneManager.getConfirmedPhoneNumber).toBeCalledWith(
+        uid
+      );
+      expect(mockRecoveryPhoneManager.storeUnconfirmed).toBeCalledWith(
+        uid,
+        code,
+        phoneNumber,
+        false
+      );
+      expect(mockOtpManager.generateCode).toBeCalled();
+      expect(mockSmsManager.sendSMS).toBeCalledWith({
+        to: phoneNumber,
+        body: code,
+      });
+
+      expect(mockRecoveryPhoneManager.removeCode).toBeCalledWith(
+        uid,
+        'code123'
+      );
+      expect(mockRecoveryPhoneManager.removeCode).toBeCalledWith(
+        uid,
+        'code456'
+      );
+      expect(mockRecoveryPhoneManager.getAllUnconfirmed).toBeCalledWith(uid);
+    });
   });
 
   describe('available', () => {

diff --git a/libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts b/libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts
@@ -331,7 +331,8 @@ export class RecoveryPhoneService {
   }
 
   /**
-   * Sends an totp code to a user
+   * Sends a new totp code to a user and revokes any previous unconfirmed codes.
+   *
    * @param uid Account id
    * @param getFormattedMessage Optional template function to format the message
    * @returns True if message didn't fail to send.
@@ -344,6 +345,17 @@ export class RecoveryPhoneService {
       throw new RecoveryPhoneNotEnabled();
     }
 
+    // Invalidate and remove any or all previous unconfirmed code entries
+    const unconfirmedKeys = await this.recoveryPhoneManager.getAllUnconfirmed(
+      uid
+    );
+    for (const key of unconfirmedKeys) {
+      const oldCode = key.split(':').pop();
+      if (oldCode) {
+        await this.recoveryPhoneManager.removeCode(uid, oldCode);
+      }
+    }
+
     const { phoneNumber } =
       await this.recoveryPhoneManager.getConfirmedPhoneNumber(uid);
     const code = await this.otpCode.generateCode();