mozilla · vpomerleau · Jan 28, 2025 · Jan 16, 2025 · vpomerleau · Jan 27, 2025
@@ -80,7 +80,6 @@ export class RecoveryPhoneService {
     if (!this.isSuccessfulSmsSend(msg)) {
       return false;
     }
-
     await this.recoveryPhoneManager.storeUnconfirmed(
       uid,
       code,
@@ -168,7 +167,6 @@ export class RecoveryPhoneService {
    * @returns True if successful
    */
   public async removePhoneNumber(uid: string) {
-
     if (!this.config.enabled) {
       throw new RecoveryPhoneNotEnabled();
     }

@@ -2182,7 +2182,7 @@ const convictConf = convict({
       },
       maxMessageLength: {
         default: 60,
-        doc: 'Max allows sms message lenght',
+        doc: 'Max allows sms message length',
         env: 'RECOVERY_PHONE__SMS__MAX_MESSAGE_LENGTH',
         format: Number,
       },

@@ -212,7 +212,6 @@ class RecoveryPhoneHandler {
     try {
       success = await this.recoveryPhoneService.removePhoneNumber(uid);
     } catch (error) {
-
       if (error instanceof RecoveryPhoneNotEnabled) {
         throw AppError.featureNotEnabled();
       }
@@ -229,7 +228,7 @@ class RecoveryPhoneHandler {
         'RecoveryPhoneService',
         'destroy',
         { uid },
-        error,
+        error
       );
     }
 
@@ -274,16 +273,6 @@ class RecoveryPhoneHandler {
         available,
       };
     } catch (error) {
-      if (error instanceof RecoveryPhoneNotEnabled) {
-        // In this case we won't throw an AppError. Unlike other endpoints,
-        // this drives whether or not the feature shows up in the UI, so
-        // if the recovery phone services isn't enabled, we can simply
-        // return available false.
-        return {
-          available: false,
-        };
-      }
-
       throw AppError.backendServiceFailure(
         'RecoveryPhoneService',
         'destroy',

@@ -20,6 +20,7 @@ import { ProfileClientService } from '../backend/profile-client.service';
 import { AccountResolver } from './account.resolver';
 import { NotifierService, NotifierSnsService } from '@fxa/shared/notifier';
 import { RecoveryPhoneService } from '@fxa/accounts/recovery-phone';
+import { GraphQLResolveInfo } from 'graphql';
 
 let USER_1: any;
 let SESSION_1: any;
@@ -202,18 +203,44 @@ describe('#integration - AccountResolver', () => {
         const linkedAccounts = resolver.linkedAccounts(user!);
         expect(linkedAccounts).toEqual([]);
       });
-
       it('resolves recovery phone number', async () => {
+        authClient.recoveryPhoneAvailable = jest
+          .fn()
+          .mockResolvedValue({ available: true });
         recoveryPhoneService.hasConfirmed = jest
           .fn()
-          .mockResolvedValue({ exists: false });
+          .mockResolvedValue({ exists: true, phoneNumber: '+11234567890' });
+
         const user = await Account.findByUid(USER_1.uid);
-        const result = await resolver.recoveryPhone(user!);
+        // Make the private method public for testing in favor of mocking 'info',
+        // which is a very large object that's challenging to mock
+        Object.defineProperty(
+          resolver,
+          'shouldIncludeRecoveryPhoneAvailability',
+          {
+            value: jest.fn().mockReturnValue(true),
+          }
+        );
+
+        const result = await resolver.recoveryPhone(
+          'token',
+          headers,
+          user!,
+          {} as unknown as GraphQLResolveInfo
+        );
 
-        // Data shouldn't exist because no number has been registered yet.
-        expect(result).toEqual({ exists: false });
-        expect(recoveryPhoneService.hasConfirmed).toBeCalledTimes(1);
-        expect(recoveryPhoneService.hasConfirmed).toBeCalledWith(USER_1.uid);
+        expect(recoveryPhoneService.hasConfirmed).toHaveBeenCalledWith(
+          user!.uid
+        );
+        expect(authClient.recoveryPhoneAvailable).toHaveBeenCalledWith(
+          'token',
+          headers
+        );
+        expect(result).toStrictEqual({
+          phoneNumber: '+11234567890',
+          exists: true,
+          available: true,
+        });
       });
 
       it('resolves linked accounts when loaded', async () => {

@@ -91,6 +91,7 @@ import { EmailBounceStatusPayload } from './dto/payload/email-bounce';
 import { NotifierService } from '@fxa/shared/notifier';
 import { MozLoggerService } from '@fxa/shared/mozlog';
 import { RecoveryPhoneService } from '@fxa/accounts/recovery-phone';
+import { RecoveryPhone } from './model/recoveryPhone';
 
 function snakeToCamel(str: string) {
   return str.replace(/(_\w)/g, (m: string) => m[1].toUpperCase());
@@ -148,6 +149,22 @@ export class AccountResolver {
     return simplified.fields.hasOwnProperty('securityEvents');
   }
 
+  private shouldIncludeRecoveryPhoneAvailability(
+    info: GraphQLResolveInfo
+  ): boolean {
+    // Introspect the query to determine if we should check recovery phone availability
+    const parsed: ResolveTree = parseResolveInfo(info) as ResolveTree;
+    const simplified = simplifyParsedResolveInfoFragmentWithType(
+      parsed,
+      info.returnType
+    ) as {
+      fields: {
+        available?: boolean;
+      };
+    };
+    return !!simplified.fields.hasOwnProperty('available');
+  }
+
   @Mutation((returns) => CreateTotpPayload, {
     description:
       'Create a new randomly generated TOTP token for a user if they do not currently have one.',
@@ -849,9 +866,41 @@ export class AccountResolver {
     return [];
   }
 
-  @ResolveField()
-  public async recoveryPhone(@Parent() account: Account) {
-    return this.recoveryPhoneService.hasConfirmed(account.uid);
+  @ResolveField(() => RecoveryPhone)
+  public async recoveryPhone(
+    @GqlSessionToken() sessionToken: string,
+    @GqlXHeaders() headers: Headers,
+    @Parent() account: Account,
+    @Info() info: GraphQLResolveInfo
+  ) {
+    const includeAvailability =
+      this.shouldIncludeRecoveryPhoneAvailability(info);
+
+    try {
+      const recoveryPhone = await this.recoveryPhoneService.hasConfirmed(
+        account.uid
+      );
+
+      if (includeAvailability) {
+        // This queries the auth-server endpoint instead of directly due to
+        // this endpoint needing maxmind
+        const { available } = await this.authAPI.recoveryPhoneAvailable(
+          sessionToken,
+          headers
+        );
+        return { ...recoveryPhone, available };
+      }
+
+      return recoveryPhone;
+    } catch (_) {
+      // The service either failed, or we have the backend config for this off.
+      // Return some reasonable defaults.
+      return {
+        exists: false,
+        ...(includeAvailability ? { available: false } : {}),
+        phoneNumber: null,
+      };
+    }
   }
 
   @ResolveField()

@@ -13,7 +13,15 @@ export class RecoveryPhone {
 
   @Field({
     nullable: true,
-    description: 'The registered recovery phone number',
+    description:
+      'The registered recovery phone number. If the user does not have a verified session, this field will return the last 4 digits of the phone number with a mask on the rest.',
   })
   public phoneNumber!: string;
+
+  @Field({
+    nullable: true,
+    description:
+      'Returns true if the user is eligible to set up a recovery phone.',
+  })
+  public available!: boolean;
 }
@@ -108,3 +108,15 @@ export const GET_BACKUP_CODES_STATUS = gql`
     }
   }
 `;
+
+export const GET_RECOVERY_PHONE = gql`
+  query GetRecoveryPhone {
+    account {
+      recoveryPhone {
+        available
+        exists
+        phoneNumber
+      }
+    }
+  }
+`;
@@ -13,4 +13,5 @@ flow-setup-phone-confirm-code-button = Confirm
 # followed by a button to resend a code
 flow-setup-phone-confirm-code-expired = Code expired?
 flow-setup-phone-confirm-code-resend-code-button = Resend code
+flow-setup-phone-confirm-code-resend-code-success = Code sent
 flow-setup-phone-confirm-code-success-message-v2 = Recovery phone added
@@ -9,6 +9,7 @@ import SettingsLayout from '../SettingsLayout';
 import { action } from '@storybook/addon-actions';
 import { AuthUiErrors } from '../../../lib/auth-errors/auth-errors';
 import FlowSetupRecoveryPhoneConfirmCode from '.';
+import { MOCK_FULL_PHONE_NUMBER } from '../../../pages/mocks';
 
 export default {
   title: 'Components/Settings/FlowSetupRecoveryPhoneConfirmCode',
@@ -43,7 +44,7 @@ const verifyRecoveryCodeFailure = async (code: string) => {
   return Promise.reject(AuthUiErrors.UNEXPECTED_ERROR);
 };
 
-const formattedPhoneNumber = '+1 123-456-3019';
+const formattedPhoneNumber = MOCK_FULL_PHONE_NUMBER;
 
 export const Success = () => (
   <SettingsLayout>

@@ -87,7 +87,7 @@ describe('FlowSetupRecoveryPhoneConfirmCode', () => {
 
     await waitFor(() => {
       expect(mockSendCode).toHaveBeenCalledTimes(1);
-      expect(screen.getByText(/Resend code/i)).toBeInTheDocument();
+      expect(screen.getByText(/Code sent/i)).toBeInTheDocument();
     });
   });
 

@@ -6,7 +6,7 @@ import React, { useState } from 'react';
 import FlowContainer from '../FlowContainer';
 import ProgressBar from '../ProgressBar';
 import { FtlMsg } from 'fxa-react/lib/utils';
-import Banner, { ResendCodeSuccessBanner } from '../../Banner';
+import Banner from '../../Banner';
 import FormVerifyTotp from '../../FormVerifyTotp';
 import { BackupRecoveryPhoneCodeImage } from '../../images';
 import { getLocalizedErrorMessage } from '../../../lib/error-utils';
@@ -95,7 +95,15 @@ export const FlowSetupRecoveryPhoneConfirmCode = ({
     >
       <ProgressBar {...{ currentStep, numberOfSteps }} />
       {resendStatus === ResendStatus.sent && !localizedErrorBannerMessage && (
-        <ResendCodeSuccessBanner />
+        <Banner
+          type="success"
+          content={{
+            localizedHeading: ftlMsgResolver.getMsg(
+              'flow-setup-phone-confirm-code-resend-code-success',
+              'Code sent'
+            ),
+          }}
+        />
       )}
       {localizedErrorBannerMessage && (
         <Banner

@@ -14,6 +14,7 @@ import {
 } from '../../../models/mocks';
 import { SettingsContext } from '../../../models/contexts/SettingsContext';
 import { useAlertBar } from '../../../models';
+import { MOCK_FULL_PHONE_NUMBER } from '../../../pages/mocks';
 
 jest.mock('../../../models', () => ({
   ...jest.requireActual('../../../models'),
@@ -31,6 +32,7 @@ jest.mock('@reach/router', () => ({
 
 const account = {
   removeRecoveryPhone: jest.fn().mockResolvedValue({}),
+  recoveryPhone: { phoneNumber: MOCK_FULL_PHONE_NUMBER },
 } as unknown as Account;
 
 describe('PageRecoveryPhoneRemove', () => {

@@ -30,8 +30,8 @@ const PageRecoveryPhoneRemove = (props: RouteComponentProps) => {
   const alertBar = useAlertBar();
   const ftlMsgResolver = useFtlMsgResolver();
 
-  // TODO, actually get this number back and format it
-  const formattedFullPhoneNumber = '+1 ••• ••••';
+  // TODO, we may want national_format back from Twilio
+  const formattedFullPhoneNumber = account.recoveryPhone.phoneNumber!;
 
   const goHome = () => navigate(SETTINGS_PATH + '#security', { replace: true });
 

@@ -8,25 +8,70 @@ import { withLocalization } from 'fxa-react/lib/storybooks';
 import { Meta } from '@storybook/react';
 import SettingsLayout from '../SettingsLayout';
 import { LocationProvider } from '@reach/router';
+import { Account, AppContext } from '../../../models';
+import { MOCK_ACCOUNT, mockAppContext } from '../../../models/mocks';
+import { AuthUiErrors } from '../../../lib/auth-errors/auth-errors';
 
 export default {
   title: 'Pages/Settings/RecoveryPhoneSetup',
   component: PageRecoveryPhoneSetup,
   decorators: [withLocalization],
 } as Meta;
 
-export const Step1 = () => (
+export const WithSuccessAddAndConfirm = () => (
   <LocationProvider>
     <SettingsLayout>
-      <PageRecoveryPhoneSetup testStep={1} />
+      <AppContext.Provider
+        value={mockAppContext({
+          account: {
+            ...MOCK_ACCOUNT,
+            addRecoveryPhone: () => {},
+            confirmRecoveryPhone: () => {},
+          } as unknown as Account,
+        })}
+      >
+        <PageRecoveryPhoneSetup />
+      </AppContext.Provider>
     </SettingsLayout>
   </LocationProvider>
 );
 
-export const Step2 = () => (
+export const WithErrorOnAdd = () => (
   <LocationProvider>
     <SettingsLayout>
-      <PageRecoveryPhoneSetup testStep={2} testPhoneNumber="+1 123-456-7890" />
+      <AppContext.Provider
+        value={mockAppContext({
+          account: {
+            ...MOCK_ACCOUNT,
+            addRecoveryPhone: () => {
+              throw AuthUiErrors.BACKEND_SERVICE_FAILURE;
+            },
+            confirmRecoveryPhone: () => {},
+          } as unknown as Account,
+        })}
+      >
+        <PageRecoveryPhoneSetup />
+      </AppContext.Provider>
+    </SettingsLayout>
+  </LocationProvider>
+);
+
+export const WithErrorOnConfirm = () => (
+  <LocationProvider>
+    <SettingsLayout>
+      <AppContext.Provider
+        value={mockAppContext({
+          account: {
+            ...MOCK_ACCOUNT,
+            addRecoveryPhone: () => {},
+            confirmRecoveryPhone: () => {
+              throw AuthUiErrors.INVALID_OTP_CODE;
+            },
+          } as unknown as Account,
+        })}
+      >
+        <PageRecoveryPhoneSetup />
+      </AppContext.Provider>
     </SettingsLayout>
   </LocationProvider>
 );