From 6dd105531f7d8b7e2b83381a24a798fad8c48ab9 Mon Sep 17 00:00:00 2001 From: Lauren Zugai Date: Thu, 16 Jan 2025 17:00:01 -0600 Subject: [PATCH] feat(sms): Set up flow to add recovery phone from Settings Because: * We want users to be able to add a recovery phone This commit: * Enables the "add" page if the feature flag is on and availability from GQL is true * Returns 'availability' with other recovery phone info in the GQL resolver * Displays recovery phone info in recovery phone row, * Hides 'change' and 'delete' buttons if user doesn't have a recovery phone set up, displays SIM swap link only if user doesn't have a recovery phone closes FXA-10370 --- .../src/lib/recovery-phone.service.ts | 2 - packages/fxa-auth-server/config/index.ts | 2 +- .../lib/routes/recovery-phone.ts | 13 +- .../src/gql/account.resolver.spec.ts | 41 +++- .../src/gql/account.resolver.ts | 55 +++++- .../src/gql/model/recoveryPhone.ts | 10 +- .../fxa-settings/src/components/App/gql.ts | 12 ++ .../FlowSetupRecoveryPhoneConfirmCode/en.ftl | 1 + .../index.stories.tsx | 3 +- .../index.test.tsx | 2 +- .../index.tsx | 12 +- .../PageRecoveryPhoneRemove/index.test.tsx | 2 + .../PageRecoveryPhoneRemove/index.tsx | 4 +- .../PageRecoveryPhoneSetup/index.stories.tsx | 53 ++++- .../PageRecoveryPhoneSetup/index.test.tsx | 113 +++++++++++ .../Settings/PageRecoveryPhoneSetup/index.tsx | 41 ++-- .../Settings/PageRecoveryPhoneSetup/mocks.tsx | 23 +++ .../Settings/Security/index.test.tsx | 1 + .../Settings/SubRow/index.stories.tsx | 11 +- .../components/Settings/SubRow/index.test.tsx | 42 +++- .../src/components/Settings/SubRow/index.tsx | 78 ++++---- .../UnitRowTwoStepAuth/index.stories.tsx | 185 ++++++------------ .../UnitRowTwoStepAuth/index.test.tsx | 179 ++++++++++++----- .../Settings/UnitRowTwoStepAuth/index.tsx | 53 ++--- .../Settings/UnitRowTwoStepAuth/mocks.tsx | 39 ++++ .../src/components/Settings/index.tsx | 5 + packages/fxa-settings/src/models/Account.ts | 47 +++++ .../src/models/contexts/AppContext.ts | 5 + .../src/models/contexts/SettingsContext.ts | 5 + .../SigninRecoveryPhoneCodeConfirm/mocks.tsx | 5 +- packages/fxa-settings/src/pages/mocks.tsx | 3 +- 31 files changed, 737 insertions(+), 310 deletions(-) create mode 100644 packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.test.tsx create mode 100644 packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/mocks.tsx create mode 100644 packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/mocks.tsx diff --git a/libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts b/libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts index e4d4466c215..eb87ead8541 100644 --- a/libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts +++ b/libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts @@ -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(); } diff --git a/packages/fxa-auth-server/config/index.ts b/packages/fxa-auth-server/config/index.ts index b40b28f6603..54bf1a93fe2 100644 --- a/packages/fxa-auth-server/config/index.ts +++ b/packages/fxa-auth-server/config/index.ts @@ -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, }, diff --git a/packages/fxa-auth-server/lib/routes/recovery-phone.ts b/packages/fxa-auth-server/lib/routes/recovery-phone.ts index 8e39992c84f..b2d0ef34c18 100644 --- a/packages/fxa-auth-server/lib/routes/recovery-phone.ts +++ b/packages/fxa-auth-server/lib/routes/recovery-phone.ts @@ -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', diff --git a/packages/fxa-graphql-api/src/gql/account.resolver.spec.ts b/packages/fxa-graphql-api/src/gql/account.resolver.spec.ts index e0b751f14da..575525ef7f1 100644 --- a/packages/fxa-graphql-api/src/gql/account.resolver.spec.ts +++ b/packages/fxa-graphql-api/src/gql/account.resolver.spec.ts @@ -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 () => { diff --git a/packages/fxa-graphql-api/src/gql/account.resolver.ts b/packages/fxa-graphql-api/src/gql/account.resolver.ts index c5a0c03be3a..0dbb3cc699e 100644 --- a/packages/fxa-graphql-api/src/gql/account.resolver.ts +++ b/packages/fxa-graphql-api/src/gql/account.resolver.ts @@ -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() diff --git a/packages/fxa-graphql-api/src/gql/model/recoveryPhone.ts b/packages/fxa-graphql-api/src/gql/model/recoveryPhone.ts index fc6f4b30130..454762c2a23 100644 --- a/packages/fxa-graphql-api/src/gql/model/recoveryPhone.ts +++ b/packages/fxa-graphql-api/src/gql/model/recoveryPhone.ts @@ -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; } diff --git a/packages/fxa-settings/src/components/App/gql.ts b/packages/fxa-settings/src/components/App/gql.ts index 8463797ad3c..48ebea1aff3 100644 --- a/packages/fxa-settings/src/components/App/gql.ts +++ b/packages/fxa-settings/src/components/App/gql.ts @@ -108,3 +108,15 @@ export const GET_BACKUP_CODES_STATUS = gql` } } `; + +export const GET_RECOVERY_PHONE = gql` + query GetRecoveryPhone { + account { + recoveryPhone { + available + exists + phoneNumber + } + } + } +`; diff --git a/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/en.ftl b/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/en.ftl index b947d8c7283..74efcb1126f 100644 --- a/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/en.ftl +++ b/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/en.ftl @@ -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 diff --git a/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.stories.tsx b/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.stories.tsx index ea67ae999e4..8d45d8c5f0e 100644 --- a/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.stories.tsx +++ b/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.stories.tsx @@ -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 = () => ( diff --git a/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.test.tsx b/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.test.tsx index 65b1dcac55a..f07e01be53a 100644 --- a/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.test.tsx @@ -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(); }); }); diff --git a/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.tsx b/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.tsx index e3eb40c458b..1ea48e7627e 100644 --- a/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.tsx +++ b/packages/fxa-settings/src/components/Settings/FlowSetupRecoveryPhoneConfirmCode/index.tsx @@ -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 = ({ > {resendStatus === ResendStatus.sent && !localizedErrorBannerMessage && ( - + )} {localizedErrorBannerMessage && ( ({ ...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', () => { diff --git a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneRemove/index.tsx b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneRemove/index.tsx index 1704035bd52..8aa1786dceb 100644 --- a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneRemove/index.tsx +++ b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneRemove/index.tsx @@ -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 }); diff --git a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.stories.tsx b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.stories.tsx index 8986202ae91..6c18623f10a 100644 --- a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.stories.tsx +++ b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.stories.tsx @@ -8,6 +8,9 @@ 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', @@ -15,18 +18,60 @@ export default { decorators: [withLocalization], } as Meta; -export const Step1 = () => ( +export const WithSuccessAddAndConfirm = () => ( - + {}, + confirmRecoveryPhone: () => {}, + } as unknown as Account, + })} + > + + ); -export const Step2 = () => ( +export const WithErrorOnAdd = () => ( - + { + throw AuthUiErrors.BACKEND_SERVICE_FAILURE; + }, + confirmRecoveryPhone: () => {}, + } as unknown as Account, + })} + > + + + + +); + +export const WithErrorOnConfirm = () => ( + + + {}, + confirmRecoveryPhone: () => { + throw AuthUiErrors.INVALID_OTP_CODE; + }, + } as unknown as Account, + })} + > + + ); diff --git a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.test.tsx b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.test.tsx new file mode 100644 index 00000000000..b4ecf8a5d54 --- /dev/null +++ b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.test.tsx @@ -0,0 +1,113 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import React from 'react'; +import { screen, waitFor } from '@testing-library/react'; +import { renderWithRouter } from '../../../models/mocks'; +import { Subject } from './mocks'; +import userEvent, { UserEvent } from '@testing-library/user-event'; + +jest.mock('../../../models/AlertBarInfo'); + +const phoneNumber = '1231231234'; +const phoneNumberWithCountryCode = '+11231231234'; +const otpCode = '123456'; + +const completeStepOne = async (user: UserEvent) => { + await waitFor(() => + user.type( + screen.getByRole('textbox', { name: /Enter phone number/i }), + phoneNumber + ) + ); + user.click(screen.getByRole('button', { name: /Send code/i })); +}; + +// Note, most unit tests are in component tests rendered for each step of the flow +describe('PageRecoveryPhoneSetup', () => { + it('renders step 1 by default', () => { + renderWithRouter(); + // renders step 1 component + expect( + screen.getByText(/You’ll get a text message from Mozilla/i) + ).toBeInTheDocument(); + }); + + it('add recovery phone with successful submission renders step 2', async () => { + const user = userEvent.setup(); + const addRecoveryPhone = jest.fn().mockResolvedValueOnce(undefined); + renderWithRouter(); + + await completeStepOne(user); + await waitFor(() => expect(addRecoveryPhone).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(addRecoveryPhone).toHaveBeenCalledWith(phoneNumberWithCountryCode) + ); + expect( + screen.queryByText(/You’ll get a text message from Mozilla/i) + ).not.toBeInTheDocument(); + expect(screen.getByText(/A six-digit code was sent/i)).toBeInTheDocument(); + }); + + it('at step 2, allows code resend', async () => { + const user = userEvent.setup(); + const confirmRecoveryPhone = jest.fn().mockResolvedValueOnce(undefined); + const addRecoveryPhone = jest + .fn() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined); + renderWithRouter( + + ); + + await completeStepOne(user); + await waitFor(() => { + expect( + screen.getByText(/A six-digit code was sent/i) + ).toBeInTheDocument(); + }); + + user.click( + screen.getByRole('button', { + name: /Resend code/i, + }) + ); + + await waitFor(() => expect(addRecoveryPhone).toHaveBeenCalledTimes(2)); + expect(addRecoveryPhone).toHaveBeenNthCalledWith( + 1, + phoneNumberWithCountryCode + ); + expect(addRecoveryPhone).toHaveBeenNthCalledWith( + 2, + phoneNumberWithCountryCode + ); + }); + + it('at step 2, allows code confirm', async () => { + const user = userEvent.setup(); + const confirmRecoveryPhone = jest.fn().mockResolvedValueOnce(undefined); + const addRecoveryPhone = jest.fn().mockResolvedValueOnce(undefined); + renderWithRouter( + + ); + + await completeStepOne(user); + await waitFor(() => { + expect( + screen.getByText(/A six-digit code was sent/i) + ).toBeInTheDocument(); + }); + await waitFor(async () => { + await user.type(screen.getByLabelText(/Enter 6-digit code/i), otpCode); + user.click(screen.getByRole('button', { name: /Confirm/i })); + }); + + await waitFor(() => expect(confirmRecoveryPhone).toHaveBeenCalledTimes(1)); + expect(confirmRecoveryPhone).toHaveBeenCalledWith( + otpCode, + phoneNumberWithCountryCode + ); + }); +}); diff --git a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.tsx b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.tsx index a922a1c8473..4928156156f 100644 --- a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.tsx +++ b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/index.tsx @@ -5,30 +5,24 @@ import React, { useState } from 'react'; import { useNavigateWithQuery as useNavigate } from '../../../lib/hooks/useNavigateWithQuery'; import { SETTINGS_PATH } from '../../../constants'; -import { useFtlMsgResolver } from '../../../models'; +import { useAccount, useFtlMsgResolver } from '../../../models'; import VerifiedSessionGuard from '../VerifiedSessionGuard'; import FlowSetupRecoveryPhoneConfirmCode from '../FlowSetupRecoveryPhoneConfirmCode'; import FlowSetupRecoveryPhoneSubmitNumber from '../FlowSetupRecoveryPhoneSubmitNumber'; +import { RouteComponentProps } from '@reach/router'; const numberOfSteps = 2; -type PageRecoveryPhoneSetupProps = { - testPhoneNumber?: string; - testStep?: 1 | 2; -}; - -// temporary props for storybook purposes -export const PageRecoveryPhoneSetup = ({ - testPhoneNumber, - testStep, -}: PageRecoveryPhoneSetupProps) => { +export const PageRecoveryPhoneSetup = (_: RouteComponentProps) => { const ftlMsgResolver = useFtlMsgResolver(); const navigate = useNavigate(); + const account = useAccount(); - const [currentStep, setCurrentStep] = useState(testStep || 1); - const [formattedPhoneNumber, setFormattedPhoneNumber] = useState( - testPhoneNumber || '' - ); + const [phoneNumber, setPhoneNumber] = useState(''); + // TODO, actually format this. Should get `national_format` back from Twilio? + const formattedPhoneNumber = phoneNumber; + + const [currentStep, setCurrentStep] = useState(1); const goHome = () => navigate(SETTINGS_PATH + '#two-step-authentication', { replace: true }); @@ -57,17 +51,22 @@ export const PageRecoveryPhoneSetup = ({ }; const sendCode = async () => { - // Placeholder until we have a proper SMS code sender + // Just retry adding the number and another code will send. Note that more than + // one code can be valid at the same time if the user clicks “resend code” to + // account for SMS transmission delay. (This will change in FXA-11039) + // try/catch is in the component that calls this function + await account.addRecoveryPhone(phoneNumber); }; const verifyRecoveryCode = async (code: string) => { - // Placeholder until we have a proper SMS code verifier + // try/catch is in the component that calls this function + await account.confirmRecoveryPhone(code, phoneNumber); }; - const verifyPhoneNumber = async (phoneNumber: string) => { - // Placeholder until we have a proper phone number verifier - // for now let's just make it available for the next step - await setFormattedPhoneNumber(phoneNumber); + const verifyPhoneNumber = async (phoneNumberInput: string) => { + // try/catch is in the component that calls this function + await account.addRecoveryPhone(phoneNumberInput); + setPhoneNumber(phoneNumberInput); }; return ( diff --git a/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/mocks.tsx b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/mocks.tsx new file mode 100644 index 00000000000..0324b701175 --- /dev/null +++ b/packages/fxa-settings/src/components/Settings/PageRecoveryPhoneSetup/mocks.tsx @@ -0,0 +1,23 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import React from 'react'; +import { MOCK_ACCOUNT, mockAppContext } from '../../../models/mocks'; +import { Account, AppContext } from '../../../models'; +import PageRecoveryPhoneSetup from '.'; +import { LocationProvider } from '@reach/router'; + +export const Subject = ({ account: accountOverrides = {} }) => { + const account = { + ...MOCK_ACCOUNT, + ...accountOverrides, + } as Account; + return ( + + + + + + ); +}; diff --git a/packages/fxa-settings/src/components/Settings/Security/index.test.tsx b/packages/fxa-settings/src/components/Settings/Security/index.test.tsx index 31d5f301715..af5abf27356 100644 --- a/packages/fxa-settings/src/components/Settings/Security/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/Security/index.test.tsx @@ -48,6 +48,7 @@ describe('Security', () => { emails: [], displayName: 'Jody', passwordCreated: 0, + recoveryPhone: { exists: true }, recoveryKey: { exists: true }, totp: { exists: true, verified: true }, backupCodes: { hasBackupCodes: true, count: 3 }, diff --git a/packages/fxa-settings/src/components/Settings/SubRow/index.stories.tsx b/packages/fxa-settings/src/components/Settings/SubRow/index.stories.tsx index 08d3ccd6948..194c88f42db 100644 --- a/packages/fxa-settings/src/components/Settings/SubRow/index.stories.tsx +++ b/packages/fxa-settings/src/components/Settings/SubRow/index.stories.tsx @@ -8,6 +8,7 @@ import SubRow, { BackupCodesSubRow, BackupPhoneSubRow } from './index'; import { action } from '@storybook/addon-actions'; import { withLocalization } from 'fxa-react/lib/storybooks'; import { CodeIcon } from '../../Icons'; +import { MOCK_FULL_PHONE_NUMBER } from '../../../pages/mocks'; export default { title: 'Components/Settings/SubRow', @@ -80,14 +81,14 @@ export const BackupPhoneUnavailableWithDescription: StoryFn = () => ( export const BackupPhoneAvailable: StoryFn = () => ( ); export const BackupPhoneAvailableWithDescription: StoryFn = () => ( ); @@ -96,7 +97,7 @@ export const BackupPhoneAvailableWithDelete: StoryFn = () => ( ); @@ -104,7 +105,7 @@ export const BackupPhoneAvailableWithDeleteAndDescription: StoryFn = () => ( ); @@ -112,6 +113,6 @@ export const BackupPhoneAvailableWithDeleteAndDescription: StoryFn = () => ( export const BackupPhoneAvailableNoDelete: StoryFn = () => ( ); diff --git a/packages/fxa-settings/src/components/Settings/SubRow/index.test.tsx b/packages/fxa-settings/src/components/Settings/SubRow/index.test.tsx index b4a431aca88..487a0a4449e 100644 --- a/packages/fxa-settings/src/components/Settings/SubRow/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/SubRow/index.test.tsx @@ -6,6 +6,10 @@ import React from 'react'; import { render, screen, fireEvent } from '@testing-library/react'; import SubRow, { BackupCodesSubRow, BackupPhoneSubRow } from './index'; import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; +import { + MOCK_FULL_PHONE_NUMBER, + MOCK_MASKED_PHONE_NUMBER, +} from '../../../pages/mocks'; describe('SubRow', () => { const defaultProps = { @@ -104,7 +108,7 @@ describe('BackupCodesSubRow', () => { describe('BackupPhoneSubRow', () => { const defaultProps = { onCtaClick: jest.fn(), - phoneNumber: '555-555-1234', + phoneNumber: MOCK_FULL_PHONE_NUMBER, }; it('renders correctly when phone number unavailable', () => { @@ -127,14 +131,30 @@ describe('BackupPhoneSubRow', () => { it('renders correctly when phone number is available and delete is not an option', () => { renderWithLocalizationProvider(); expect(screen.getByText('Recovery phone')).toBeInTheDocument(); - expect(screen.getByText('••• ••• 1234')).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Change' })).toBeInTheDocument(); + expect(screen.getByText(MOCK_MASKED_PHONE_NUMBER)).toBeInTheDocument(); + // Temporary until we work on the change flow for SMS phase 2, FXA-10995 + expect( + screen.queryByRole('button', { name: 'Change' }) + ).not.toBeInTheDocument(); expect( screen.getByText( 'If you want to remove your recovery phone, add backup authentication codes or disable two-step authentication first to avoid getting locked out of your account.' ) ).toBeInTheDocument(); - expect(screen.getByText(/Learn about SIM swap risk/)).toBeInTheDocument(); + expect( + screen.queryByText(/Learn about SIM swap risk/) + ).not.toBeInTheDocument(); + }); + + it('renders correctly when user does not have a verified session (phone number is already masked)', () => { + renderWithLocalizationProvider( + + ); + expect(screen.getByText('Recovery phone')).toBeInTheDocument(); + expect(screen.getByText(MOCK_MASKED_PHONE_NUMBER)).toBeInTheDocument(); }); it('renders correctly when phone number is available and delete is an option', () => { @@ -142,8 +162,11 @@ describe('BackupPhoneSubRow', () => { ); expect(screen.getByText('Recovery phone')).toBeInTheDocument(); - expect(screen.getByText('••• ••• 1234')).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Change' })).toBeInTheDocument(); + expect(screen.getByText(MOCK_MASKED_PHONE_NUMBER)).toBeInTheDocument(); + // Temporary until we work on the change flow for SMS phase 2, FXA-10995 + expect( + screen.queryByRole('button', { name: 'Change' }) + ).not.toBeInTheDocument(); const deleteButtons = screen.getAllByTitle(/Remove/); expect(deleteButtons).toHaveLength(2); expect( @@ -151,10 +174,13 @@ describe('BackupPhoneSubRow', () => { 'This is the easier recovery method if you canʼt use your authenticator app.' ) ).toBeInTheDocument(); - expect(screen.getByText(/Learn about SIM swap risk/)).toBeInTheDocument(); + expect( + screen.queryByText(/Learn about SIM swap risk/) + ).not.toBeInTheDocument(); }); - it('calls onCtaClick when CTA button is clicked', () => { + // Temporary skip we work on the change flow for SMS phase 2, FXA-10995 + it.skip('calls onCtaClick when CTA button is clicked', () => { renderWithLocalizationProvider(); fireEvent.click(screen.getByRole('button', { name: 'Change' })); expect(defaultProps.onCtaClick).toHaveBeenCalled(); diff --git a/packages/fxa-settings/src/components/Settings/SubRow/index.tsx b/packages/fxa-settings/src/components/Settings/SubRow/index.tsx index 9eb3f6dbb45..d46aca4857b 100644 --- a/packages/fxa-settings/src/components/Settings/SubRow/index.tsx +++ b/packages/fxa-settings/src/components/Settings/SubRow/index.tsx @@ -22,7 +22,8 @@ import LinkExternal, { type SubRowProps = { ctaGleanId: string; - ctaMessage: string; + // temporarily this prop is optional until we enable 'Change' in phase 2, FXA-10995 + ctaMessage?: string; icon: React.ReactNode; idPrefix: string; isEnabled: boolean; @@ -73,8 +74,8 @@ const SubRow = ({ return linkExternalProps ? ( {linkExternalProps.children} @@ -118,13 +119,16 @@ const SubRow = ({

)} - + {/* temporary check until we enable changing in SMS phase 2, FXA-10995 */} + {ctaMessage && ( + + )} {onDeleteClick && localizedDeleteIconTitle && ( <>
@@ -228,43 +232,51 @@ export const BackupPhoneSubRow = ({ ); const message = hasPhoneNumber ? ( - // We will likely want to only retrieve the last 4 digits of the phone number from the backend - // but adding a slice here just in case to ensure only the last 4 digits are displayed - // u2022 is a bullet point character - // This format works for a North American phone number, but may need to be adjusted for other formats - // durring next phases of SMS feature rollout + // If the user's session is not verified, an already masked phone number is returned. + // If it is verified, the full phone number is returned but here we want a client-side mask. + // We may want to get `national_format` back from Twilio. // Phone numbers should always be displayed left-to-right, *including* in rtl languages - // • is a bullet point character (\u2022) -

{`••• ••• ${phoneNumber.slice(-4)}`}

+

+ {phoneNumber.includes('•') + ? phoneNumber + : `••••••${phoneNumber.slice(-4)}`} +

) : (

No recovery phone number available

 ); const ctaMessage = hasPhoneNumber - ? ftlMsgResolver.getMsg('tfa-row-backup-phone-change-cta', 'Change') + ? // Temporary until we enable changing in phase 2, FXA-10995 + // ? ftlMsgResolver.getMsg('tfa-row-backup-phone-change-cta', 'Change') + undefined : ftlMsgResolver.getMsg('tfa-row-backup-phone-add-cta', 'Add'); const ctaGleanId = hasPhoneNumber ? 'account_pref_two_step_auth_phone_change_submit' : 'account_pref_two_step_auth_phone_add_submit'; - const localizedDeleteIconTitle = ftlMsgResolver.getMsg( - 'tfa-row-backup-phone-delete-title-v2', - 'Remove recovery phone' - ); + // Do not display 'delete' button if user does not have a phone number + const localizedDeleteIconTitle = hasPhoneNumber + ? ftlMsgResolver.getMsg( + 'tfa-row-backup-phone-delete-title-v2', + 'Remove recovery phone' + ) + : undefined; - const linkExternalProps = { - // TODO add a link to the knowledge base article once it is available - href: '', - children: ftlMsgResolver.getMsg( - 'tfa-row-backup-phone-sim-swap-risk-link', - 'Learn about SIM swap risk' - ), - gleanDataAttrs: { - id: 'account_pref_two_step_auth_phone_learn_more_link', - }, - }; + const linkExternalProps = !hasPhoneNumber + ? { + // TODO add a link to the knowledge base article once it is available + href: '', + children: ftlMsgResolver.getMsg( + 'tfa-row-backup-phone-sim-swap-risk-link', + 'Learn about SIM swap risk' + ), + gleanDataAttrs: { + id: 'account_pref_two_step_auth_phone_learn_more_link', + }, + } + : undefined; return ( ( - - - -); +export const TFAEnabledWithBackupCodesRemainingAndRecoveryPhoneUnavailable = + () => + createSubject({ + recoveryPhone: { exists: false, phoneNumber: null, available: false }, + }); -export const TFAEnabledNoCodesRemaining = () => ( - - - -); +export const TFAEnabledNoCodesRemaining = () => + createSubject({ + backupCodes: { hasBackupCodes: false, count: 0 }, + }); -export const TFADisabled = () => ( - - - -); +export const TFADisabled = () => + createSubject({ + totp: { exists: false, verified: false }, + backupCodes: { count: 0 }, + }); -export const DisabledNoPassword = () => ( - - - -); +export const DisabledNoPassword = () => + createSubject({ + hasPassword: false, + totp: { enabled: false }, + backupCodes: { count: 0 }, + }); -export const TwoFAEnabledWithBackupCodesNoBackupPhone = () => ( - - action('Add clicked') }} - /> - -); +export const TwoFAEnabledWithBackupCodesNoBackupPhone = () => + createSubject({ + recoveryPhone: { exists: false, phoneNumber: null, available: true }, + }); -export const TwoFAEnabledWithBackupPhoneNoBackupCodes = () => ( - - action('Change clicked'), - }} - /> - -); +export const TwoFAEnabledWithBackupPhoneNoBackupCodes = () => + createSubject({ + recoveryPhone: { + exists: true, + phoneNumber: MOCK_FULL_PHONE_NUMBER, + available: true, + }, + backupCodes: { hasBackupCodes: false, count: 0 }, + }); -export const TwoFAEnabledWithBackupCodesAndBackupPhone = () => ( - - action('Change clicked'), - onDeleteClick: () => action('Delete clicked'), - showDescription: true, - }} - /> - -); +export const TwoFAEnabledWithBackupCodesAndBackupPhone = () => + createSubject({ + recoveryPhone: { + exists: true, + phoneNumber: MOCK_FULL_PHONE_NUMBER, + available: true, + }, + }); // if backup codes run out and user does not replace them -export const TwoFAEnabledNoBackupCodesNoBackupPhone = () => ( - - action('Add clicked'), - }} - /> - -); +// Recovery phone not shown since `available` is based on region and recovery codes +export const TwoFAEnabledNoBackupCodesNoBackupPhone = () => + createSubject({ + backupCodes: { hasBackupCodes: false, count: 0 }, + recoveryPhone: { exists: false, phoneNumber: null, available: false }, + }); + +// User is currently in an unsupported region, but already has a previously added backup phone +export const TwoFAEnabledWithBackupPhoneAndUnsupportedCurrentRegion = () => + createSubject({ + recoveryPhone: { + exists: true, + phoneNumber: MOCK_FULL_PHONE_NUMBER, + available: false, + }, + backupCodes: { hasBackupCodes: true, count: 1 }, + }); diff --git a/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.test.tsx b/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.test.tsx index d49dafc1d5a..9251fb67c27 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.test.tsx @@ -4,30 +4,19 @@ import React from 'react'; import { screen, fireEvent, waitFor } from '@testing-library/react'; -import { UnitRowTwoStepAuth } from '.'; +import { renderWithRouter } from '../../../models/mocks'; +import { createSubject } from './mocks'; import { - renderWithRouter, - mockAppContext, - mockSettingsContext, -} from '../../../models/mocks'; -import { Account, AppContext } from '../../../models'; -import { SettingsContext } from '../../../models/contexts/SettingsContext'; + MOCK_FULL_PHONE_NUMBER, + MOCK_MASKED_PHONE_NUMBER, +} from '../../../pages/mocks'; jest.mock('../../../models/AlertBarInfo'); -const account = { - hasPassword: true, - backupCodes: { hasBackupCodes: true, count: 3 }, - totp: { exists: true, verified: true }, - disableTwoStepAuth: jest.fn().mockResolvedValue(true), -} as unknown as Account; describe('UnitRowTwoStepAuth', () => { - it('renders when Two-step authentication is enabled', async () => { - renderWithRouter( - - - - ); + it('renders when two-step authentication is enabled', async () => { + renderWithRouter(createSubject()); + expect( screen.getByTestId('two-step-unit-row-header').textContent ).toContain('Two-step authentication'); @@ -37,17 +26,14 @@ describe('UnitRowTwoStepAuth', () => { expect(screen.getByRole('button', { name: 'Disable' })).toBeVisible(); }); - it('renders as expected when Two-step authentication is not enabled', () => { - const account = { - hasPassword: true, - totp: { exists: false, verified: false }, - backupCodes: { hasBackupCodes: false, count: 0 }, - } as unknown as Account; + it('renders when two-step authentication is not enabled', () => { renderWithRouter( - - - + createSubject({ + totp: { exists: false, verified: false }, + backupCodes: { hasBackupCodes: false, count: 0 }, + }) ); + expect( screen.getByTestId('two-step-unit-row-header').textContent ).toContain('Two-step authentication'); @@ -59,15 +45,36 @@ describe('UnitRowTwoStepAuth', () => { ); }); + it('renders disabled state when account has no password', async () => { + renderWithRouter( + createSubject({ + hasPassword: false, + totp: { exists: false, verified: false }, + backupCodes: { hasBackupCodes: false, count: 0 }, + }) + ); + + const mainButton = await screen.findByText('Add'); + expect(mainButton).toBeDisabled(); + expect(mainButton).toHaveAttribute( + 'title', + 'Set a password to sync and use certain account security features.' + ); + expect( + screen.getByTestId('two-step-unit-row-header-value').textContent + ).toContain('Disabled'); + expect( + screen.queryByTestId('backup-authentication-codes-sub-row') + ).not.toBeInTheDocument(); + }); + it('renders view as not enabled after disabling TOTP', async () => { - const context = mockAppContext({ account }); - const settingsContext = mockSettingsContext(); + const disableTwoStepAuthMock = jest.fn().mockResolvedValue(true); + renderWithRouter( - - - - - + createSubject({ + disableTwoStepAuth: disableTwoStepAuthMock, + }) ); fireEvent.click(screen.getByRole('button', { name: 'Disable' })); @@ -78,38 +85,102 @@ describe('UnitRowTwoStepAuth', () => { ).toBeInTheDocument() ); - // using test id here because the modal cta has the same name as the row button fireEvent.click(screen.getByTestId('modal-confirm')); await waitFor(() => - expect(settingsContext.alertBarInfo?.success).toBeCalledTimes(1) + expect(disableTwoStepAuthMock).toHaveBeenCalledTimes(1) ); }); - it('renders disabled state when account has no password', async () => { - const account = { - hasPassword: false, - totp: { exists: false, verified: false }, - backupCodes: { hasBackupCodes: false, count: 0 }, - } as unknown as Account; - + it('renders with no backup codes and no recovery phone', () => { renderWithRouter( - - - + createSubject({ + backupCodes: { hasBackupCodes: false, count: 0 }, + recoveryPhone: { exists: false, phoneNumber: null, available: false }, + }) ); - const mainButton = await screen.findByText('Add'); - expect(mainButton).toBeDisabled(); - expect(mainButton).toHaveAttribute( - 'title', - 'Set a password to sync and use certain account security features.' - ); expect( screen.getByTestId('two-step-unit-row-header-value').textContent - ).toContain('Disabled'); + ).toContain('Enabled'); expect( screen.queryByTestId('backup-authentication-codes-sub-row') + ).toBeInTheDocument(); + expect( + screen.queryByTestId('backup-recovery-phone-sub-row') ).not.toBeInTheDocument(); }); + + it('renders with backup phone but no backup codes', () => { + renderWithRouter( + createSubject({ + recoveryPhone: { + exists: true, + phoneNumber: MOCK_FULL_PHONE_NUMBER, + available: true, + }, + backupCodes: { hasBackupCodes: false, count: 0 }, + }) + ); + + expect( + screen.getByTestId('two-step-unit-row-header-value').textContent + ).toContain('Enabled'); + expect( + screen.getByTestId('backup-recovery-phone-sub-row').textContent + ).toContain(MOCK_MASKED_PHONE_NUMBER); + expect( + screen.queryByTestId('backup-authentication-codes-sub-row') + ).toBeInTheDocument(); + }); + + it('renders with backup codes and backup phone', () => { + renderWithRouter( + createSubject({ + recoveryPhone: { + exists: true, + phoneNumber: MOCK_FULL_PHONE_NUMBER, + available: true, + }, + backupCodes: { hasBackupCodes: true, count: 3 }, + }) + ); + + expect( + screen.getByTestId('two-step-unit-row-header-value').textContent + ).toContain('Enabled'); + expect( + screen.getByTestId('backup-recovery-phone-sub-row').textContent + ).toContain(MOCK_MASKED_PHONE_NUMBER); + expect( + screen.getByTestId('backup-authentication-codes-sub-row').textContent + ).toContain('3'); + // There are two because they are conditionally rendered based on the container size + expect(screen.getAllByTitle(/Remove/)).toHaveLength(2); + }); + + it('renders with backup phone added but currently unsupported recovery phone region', () => { + renderWithRouter( + createSubject({ + recoveryPhone: { + exists: true, + phoneNumber: MOCK_FULL_PHONE_NUMBER, + available: false, + }, + backupCodes: { hasBackupCodes: true, count: 1 }, + }) + ); + + expect( + screen.getByTestId('two-step-unit-row-header-value').textContent + ).toContain('Enabled'); + expect( + screen.getByTestId('backup-recovery-phone-sub-row').textContent + ).toContain(MOCK_MASKED_PHONE_NUMBER); + expect( + screen.getByTestId('backup-authentication-codes-sub-row').textContent + ).toContain('1'); + // There are two because they are conditionally rendered based on the container size + expect(screen.getAllByTitle(/Remove/)).toHaveLength(2); + }); }); diff --git a/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.tsx b/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.tsx index 680315d8ec7..fdc3e474fd8 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.tsx @@ -8,36 +8,31 @@ import { useBooleanState } from 'fxa-react/lib/hooks'; import Modal from '../Modal'; import UnitRow, { UnitRowProps } from '../UnitRow'; import VerifiedSessionGuard from '../VerifiedSessionGuard'; -import { useAccount, useAlertBar, useFtlMsgResolver } from '../../../models'; +import { + useAccount, + useAlertBar, + useConfig, + useFtlMsgResolver, +} from '../../../models'; import { SETTINGS_PATH } from '../../../constants'; import GleanMetrics from '../../../lib/glean'; import { FtlMsg } from 'fxa-react/lib/utils'; -import { - BackupCodesSubRow, - BackupPhoneSubRow, - BackupPhoneSubRowProps, -} from '../SubRow'; +import { BackupCodesSubRow, BackupPhoneSubRow } from '../SubRow'; import { useNavigateWithQuery as useNavigate } from '../../../lib/hooks/useNavigateWithQuery'; const route = `${SETTINGS_PATH}/two_step_authentication`; const replaceCodesRoute = `${route}/replace_codes`; -// These props are temporary for storybook purposes -// until recovery phone feature is enabled. -type UnitRowTwoStepAuthProps = { - backupPhoneSubRowProps?: BackupPhoneSubRowProps; -}; - -export const UnitRowTwoStepAuth = ({ - backupPhoneSubRowProps, -}: UnitRowTwoStepAuthProps) => { +export const UnitRowTwoStepAuth = () => { const alertBar = useAlertBar(); const account = useAccount(); const navigate = useNavigate(); const { backupCodes: { count }, totp: { exists, verified }, + recoveryPhone, } = account; + const config = useConfig(); const [disable2FAModalRevealed, revealDisable2FAModal, hideDisable2FAModal] = useBooleanState(); const ftlMsgResolver = useFtlMsgResolver(); @@ -155,17 +150,27 @@ export const UnitRowTwoStepAuth = ({ onCtaClick={() => { navigate(replaceCodesRoute); }} + key={1} /> ); - } - if (backupPhoneSubRowProps?.onCtaClick) { - subRows.push( - - ); + if ( + (config.featureFlags?.enableAdding2FABackupPhone === true && + recoveryPhone.available === true) || + recoveryPhone.exists === true + ) { + subRows.push( + { + navigate(`${SETTINGS_PATH}/recovery_phone/setup`); + }} + onDeleteClick={() => { + navigate(`${SETTINGS_PATH}/recovery_phone/remove`); + }} + phoneNumber={recoveryPhone.phoneNumber || ''} + key={2} + /> + ); + } } return subRows; diff --git a/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/mocks.tsx b/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/mocks.tsx new file mode 100644 index 00000000000..a9bd183da6d --- /dev/null +++ b/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/mocks.tsx @@ -0,0 +1,39 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import React from 'react'; +import { mockAppContext, mockSettingsContext } from '../../../models/mocks'; +import { Account, AppContext } from '../../../models'; +import UnitRowTwoStepAuth from '.'; +import { SettingsContext } from '../../../models/contexts/SettingsContext'; +import { getDefault } from '../../../lib/config'; + +export const createSubject = ( + accountOverrides = {}, + settingsOverrides = {} +) => { + const account = { + hasPassword: true, + backupCodes: { hasBackupCodes: true, count: 3 }, + totp: { exists: true, verified: true }, + recoveryPhone: { exists: false, phoneNumber: null, available: false }, + ...accountOverrides, + } as unknown as Account; + const config = { + ...getDefault(), + featureFlags: { + enableAdding2FABackupPhone: true, + }, + }; + const appContext = mockAppContext({ account, config }); + const settingsContext = mockSettingsContext(settingsOverrides); + + return ( + + + + + + ); +}; diff --git a/packages/fxa-settings/src/components/Settings/index.tsx b/packages/fxa-settings/src/components/Settings/index.tsx index dc29eb97245..11ed8f837ed 100644 --- a/packages/fxa-settings/src/components/Settings/index.tsx +++ b/packages/fxa-settings/src/components/Settings/index.tsx @@ -27,6 +27,7 @@ import PageSecondaryEmailVerify from './PageSecondaryEmailVerify'; import { PageDisplayName } from './PageDisplayName'; import PageTwoStepAuthentication from './PageTwoStepAuthentication'; import { Page2faReplaceRecoveryCodes } from './Page2faReplaceRecoveryCodes'; +import { PageRecoveryPhoneSetup } from './PageRecoveryPhoneSetup'; import { PageDeleteAccount } from './PageDeleteAccount'; import { ScrollToTop } from './ScrollToTop'; import { SETTINGS_PATH } from '../../constants'; @@ -167,6 +168,10 @@ export const Settings = ({ /> + {config.featureFlags?.enableAdding2FABackupPhone === true && + account.recoveryPhone.available === true && ( + + )} ) : ( <> diff --git a/packages/fxa-settings/src/models/Account.ts b/packages/fxa-settings/src/models/Account.ts index 7a7a53ba833..1bc01316616 100644 --- a/packages/fxa-settings/src/models/Account.ts +++ b/packages/fxa-settings/src/models/Account.ts @@ -28,6 +28,7 @@ import { GET_LOCAL_SIGNED_IN_STATUS, GET_TOTP_STATUS, GET_BACKUP_CODES_STATUS, + GET_RECOVERY_PHONE, } from '../components/App/gql'; import { AccountAvatar, @@ -114,6 +115,11 @@ export interface AccountData { linkedAccounts: LinkedAccount[]; totp: AccountTotp; backupCodes: AccountBackupCodes; + recoveryPhone: { + exists: boolean; + phoneNumber: string | null; + available: boolean; + }; subscriptions: Subscription[]; securityEvents: SecurityEvent[]; } @@ -197,6 +203,11 @@ export const GET_ACCOUNT = gql` hasBackupCodes count } + recoveryPhone { + exists + phoneNumber + available + } subscriptions { created productName @@ -404,6 +415,10 @@ export class Account implements AccountData { return this.data.backupCodes; } + get recoveryPhone() { + return this.data.recoveryPhone; + } + get attachedClients() { return this.data.attachedClients; } @@ -428,6 +443,7 @@ export class Account implements AccountData { | 'recovery' | 'securityEvents' | 'backupCodes' + | 'recoveryPhone' ) { let query = GET_ACCOUNT; switch (field) { @@ -443,6 +459,9 @@ export class Account implements AccountData { case 'backupCodes': query = GET_BACKUP_CODES_STATUS; break; + case 'recoveryPhone': + query = GET_RECOVERY_PHONE; + break; } await this.withLoadingStatus( this.apolloClient.query({ @@ -1171,6 +1190,9 @@ export class Account implements AccountData { await this.withLoadingStatus( this.authClient.verifyTotpCode(sessionToken()!, code) ); + // We must requery for this because this endpoint checks + // for if recovery codes exist + await this.refresh('recoveryPhone'); const cache = this.apolloClient.cache; cache.modify({ id: cache.identify({ __typename: 'Account' }), @@ -1385,4 +1407,29 @@ export class Account implements AccountData { return result; } + + async addRecoveryPhone(phoneNumber: string) { + await this.withLoadingStatus( + this.authClient.recoveryPhoneCreate(sessionToken()!, phoneNumber) + ); + } + + async confirmRecoveryPhone(code: string, phoneNumber: string) { + await this.withLoadingStatus( + this.authClient.recoveryPhoneConfirmSetup(sessionToken()!, code) + ); + const cache = this.apolloClient.cache; + cache.modify({ + id: cache.identify({ __typename: 'Account' }), + fields: { + recoveryPhone() { + return { + exists: true, + phoneNumber, + available: true, + }; + }, + }, + }); + } } diff --git a/packages/fxa-settings/src/models/contexts/AppContext.ts b/packages/fxa-settings/src/models/contexts/AppContext.ts index ac5249c67f1..93022c249d5 100644 --- a/packages/fxa-settings/src/models/contexts/AppContext.ts +++ b/packages/fxa-settings/src/models/contexts/AppContext.ts @@ -100,6 +100,11 @@ export function defaultAppContext(context?: AppContextValue) { }, linkedAccounts: [], securityEvents: [], + recoveryPhone: { + exists: false, + phoneNumber: null, + available: false, + }, }; const session = { verified: true, diff --git a/packages/fxa-settings/src/models/contexts/SettingsContext.ts b/packages/fxa-settings/src/models/contexts/SettingsContext.ts index 10ed917c20b..db4ae64e261 100644 --- a/packages/fxa-settings/src/models/contexts/SettingsContext.ts +++ b/packages/fxa-settings/src/models/contexts/SettingsContext.ts @@ -62,6 +62,11 @@ export const INITIAL_SETTINGS_QUERY = gql` hasBackupCodes count } + recoveryPhone { + exists + phoneNumber + available + } subscriptions { created productName diff --git a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhoneCodeConfirm/mocks.tsx b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhoneCodeConfirm/mocks.tsx index b16d8eeeaf4..444af081b0a 100644 --- a/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhoneCodeConfirm/mocks.tsx +++ b/packages/fxa-settings/src/pages/Signin/SigninRecoveryPhoneCodeConfirm/mocks.tsx @@ -4,7 +4,7 @@ import React, { useState } from 'react'; import ConfirmRecoveryCode from '.'; -import { MOCK_PHONE_NUMBER } from '../../mocks'; +import { MOCK_MASKED_PHONE_NUMBER } from '../../mocks'; import { LocationProvider } from '@reach/router'; import { SigninRecoveryPhoneCodeConfirmProps } from '.'; @@ -15,7 +15,6 @@ export const Subject = ({ verifyCode = mockVerifyCode, resendCode = mockResendCode, }: Partial) => { - const maskedPhoneNumber = MOCK_PHONE_NUMBER; const [errorMessage, setErrorMessage] = useState(''); const clearBanners = () => { @@ -25,7 +24,7 @@ export const Subject = ({ return (