task(auth): Have totp/destroy remove recovery phone

Because:
- When a user removes 2fa-totp, we want to also destroy any registered recovery phone.
- When a user removes 2fa-totp, we want to also destroy an existing backup recovery codes (ie 2fa back up codes).

This Commit:
- If recovery phone was registered, removes it during call to /totp/destroy
- If backup codes exist, removes them during call to /totp/destroy
- Adds ability to remove recovery codes to backup-code.manager
- Some cleanup:
  - Renames `recovery-phone` to `account-recovery-phone` in `account/recovery-phone/project.json` to be consistent.
  - Add tag 'scope:shared:lib' to `account/recovery-phone/project.json`
  - Add tag 'scope:shared:lib' to `account/recovery-phone/project.json`
  - Removes entry for  `accounts/recovery-phone` in `tsconfig.base.json`, because`@fxa/accounts/recovery-phone` already declared.
  - Removes entry for `accounts/two-factor` in `tsconfig.base.json`, because `@fxa/accounts/two-factor` already declared.
  - Changes call in recovery-phone.repository > removePhoneNumber, from `executeTakeFirstOrThrow` to `executeTakeFirst`, which avoids a potentially unhandled error.
  - Changes `Container.set('BackupCodeManager', backupCodeManager)` to `Container.set(BackupCodeManager, backupCodeManager)`, which is more consistent.

Author: dschom

libs/accounts/recovery-phone/project.json

@@ -1,9 +1,9 @@
 {
-  "name": "recovery-phone",
+  "name": "accounts-recovery-phone",
   "$schema": "../../../node_modules/nx/schemas/project-schema.json",
   "sourceRoot": "libs/accounts/recovery-phone/src",
   "projectType": "library",
-  "tags": [],
+  "tags": ["scope:shared:lib"],
   "targets": {
     "build": {
       "executor": "@nx/esbuild:esbuild",

libs/accounts/recovery-phone/src/lib/recovery-phone.manager.in.spec.ts

@@ -6,12 +6,14 @@ import {
   AccountDatabase,
   AccountDbProvider,
   testAccountDatabaseSetup,
+  RecoveryPhoneFactory,
 } from '@fxa/shared/db/mysql/account';
 import { Test } from '@nestjs/testing';
 
 describe('RecoveryPhoneManager', () => {
   let recoveryPhoneManager: RecoveryPhoneManager;
   let db: AccountDatabase;
+  const dateMock = jest.spyOn(global.Date, 'now');
 
   // Taken from: https://www.twilio.com/docs/lookup/v2-api#code-lookup-with-data-packages
   const mockLookUpData: PhoneNumberLookupData = {
@@ -43,6 +45,7 @@ describe('RecoveryPhoneManager', () => {
   };
 
   beforeAll(async () => {
+    dateMock.mockImplementation(() => 1739227529776);
     db = await testAccountDatabaseSetup([
       'accounts',
       'recoveryPhones',
@@ -67,6 +70,7 @@ describe('RecoveryPhoneManager', () => {
 
   afterAll(async () => {
     await db.destroy();
+    dateMock.mockReset();
   });
 
   it('should get a recovery phone', async () => {
@@ -201,6 +205,7 @@ describe('RecoveryPhoneManager', () => {
     );
 
     const expectedData = JSON.stringify({
+      createdAt: 1739227529776,
       phoneNumber,
       isSetup,
       lookupData: JSON.stringify(mockLookUpData),

libs/accounts/recovery-phone/src/lib/recovery-phone.repository.ts

@@ -29,7 +29,9 @@ export async function removePhoneNumber(
   const result = await db
     .deleteFrom('recoveryPhones')
     .where('uid', '=', uid)
-    .executeTakeFirstOrThrow();
+    // TBD: Didn't seem like handled an error coming off this call, so
+    // removed the orThrow...
+    .executeTakeFirst();
 
   return result.numDeletedRows === BigInt(1);
 }

libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts

@@ -186,6 +186,12 @@ export class RecoveryPhoneService {
       uid
     );
 
+    // TBD: Random, obs. why do we do this? It seems like a potential edge case, what if the user
+    // consumes all the recovery codes? This could then return false...
+    //
+    // Just curious, about the rationale surrounding why this would block a user from removing their
+    // phone.
+    //
     if (!hasRecoveryCodes) {
       throw new RecoveryNumberRemoveMissingBackupCodes(uid);
     }

libs/accounts/two-factor/project.json

@@ -3,7 +3,7 @@
   "$schema": "../../../node_modules/nx/schemas/project-schema.json",
   "sourceRoot": "libs/accounts/two-factor/src",
   "projectType": "library",
-  "tags": [],
+  "tags": ["scope:shared:lib"],
   "targets": {
     "build": {
       "executor": "@nx/esbuild:esbuild",

libs/accounts/two-factor/src/lib/backup-code.manager.in.spec.ts

@@ -92,4 +92,16 @@ describe('BackupCodeManager', () => {
       backupCodeManager.getCountForUserId(getMockUid())
     ).rejects.toThrow('Database error');
   });
+
+  it('should delete codes', async () => {
+    const mockUid1 = getMockUid();
+    const mockUid2 = getMockUid();
+    await createRecoveryCode(db, mockUid1);
+
+    const result1 = await backupCodeManager.deleteRecoveryCodes(mockUid1);
+    const result2 = await backupCodeManager.deleteRecoveryCodes(mockUid2);
+
+    expect(result1).toBeTruthy();
+    expect(result2).toBeFalsy();
+  });
 });

libs/accounts/two-factor/src/lib/backup-code.manager.ts

@@ -7,7 +7,10 @@ import {
   AccountDbProvider,
 } from '@fxa/shared/db/mysql/account';
 import { Inject, Injectable } from '@nestjs/common';
-import { getRecoveryCodes } from './backup-code.repository';
+import {
+  deleteRecoveryCodes,
+  getRecoveryCodes,
+} from './backup-code.repository';
 
 @Injectable()
 export class BackupCodeManager {
@@ -34,4 +37,15 @@ export class BackupCodeManager {
       count: recoveryCodes.length,
     };
   }
+
+  /**
+   * Removes recover codes for a given uid.
+   *
+   * @param uid - The uid in hexadecimal string format.
+   * @returns True if one or more codes were removed
+   */
+  async deleteRecoveryCodes(uid: string) {
+    const success = await deleteRecoveryCodes(this.db, Buffer.from(uid, 'hex'));
+    return success;
+  }
 }

libs/accounts/two-factor/src/lib/backup-code.repository.ts

@@ -10,3 +10,12 @@ export async function getRecoveryCodes(db: AccountDatabase, uid: Buffer) {
     .selectAll()
     .execute();
 }
+
+export async function deleteRecoveryCodes(db: AccountDatabase, uid: Buffer) {
+  const result = await db
+    .deleteFrom('recoveryCodes')
+    .where('uid', '=', uid)
+    .executeTakeFirst();
+
+  return result.numDeletedRows === BigInt(1);
+}

libs/shared/db/mysql/account/src/index.ts

@@ -9,6 +9,7 @@ export {
   AccountFactory,
   AccountCustomerFactory,
   PaypalCustomerFactory,
+  RecoveryPhoneFactory,
 } from './lib/factories';
 export { setupAccountDatabase, AccountDbProvider } from './lib/setup';
 export { testAccountDatabaseSetup } from './lib/tests';

packages/fxa-auth-server/bin/key_server.js

@@ -130,7 +130,7 @@ async function run(config) {
   Container.set(AccountManager, accountManager);
 
   const backupCodeManager = new BackupCodeManager(accountDatabase);
-  Container.set('BackupCodeManager', backupCodeManager);
+  Container.set(BackupCodeManager, backupCodeManager);
 
   // Set currencyHelper before stripe and paypal helpers, so they can use it.
   try {