fix(wallet): require magic code when setting phone for first time

Author: dukenv0307

src/pages/EnablePayments/shared/useWalletPhoneMagicCode.ts

@@ -11,21 +11,23 @@ import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
 
 /**
- * Shared magic-code handling for the wallet KYC personal-details flows. Changing an existing phone number is protected
- * by a magic code because it is used for card 3DS verification, so both flows send the user to a dedicated
- * confirmation screen to enter the code before the change is submitted.
+ * Shared magic-code handling for the wallet KYC personal-details flows. Setting a phone number, whether for the first
+ * time or changing an existing one, is protected by a magic code because it is used for card 3DS verification, so
+ * both flows send the user to a dedicated confirmation screen to enter the code before the change is submitted.
  */
 function useWalletPhoneMagicCode() {
     const [privatePersonalDetails] = useOnyx(ONYXKEYS.PRIVATE_PERSONAL_DETAILS);
 
-    // Submits the personal details, first routing to the magic-code screen when an existing phone number is changed.
+    // Submits the personal details, first routing to the magic-code screen when a phone number is being set to a new
+    // value. Setting a phone for the first time must be protected too: it is used for card 3DS verification, so an
+    // attacker who gains account access before any phone is on file could otherwise set one without a code.
     const submitPersonalDetails = (personalDetails: UpdatePersonalDetailsForWalletParams) => {
         // The stored phone number keeps its country code, so normalize it the same way as the submitted one before
         // comparing, otherwise an unchanged phone would look like a change and wrongly prompt for a magic code.
         const storedPhoneNumber = privatePersonalDetails?.phoneNumber;
         const normalizedStoredPhoneNumber = (storedPhoneNumber && parsePhoneNumber(storedPhoneNumber, {regionCode: CONST.COUNTRY.US}).number?.significant) ?? '';
-        const hasPhoneNumberChanged = !!normalizedStoredPhoneNumber && personalDetails.phoneNumber !== normalizedStoredPhoneNumber;
-        if (hasPhoneNumberChanged) {
+        const isSettingPhoneNumber = !!personalDetails.phoneNumber && personalDetails.phoneNumber !== normalizedStoredPhoneNumber;
+        if (isSettingPhoneNumber) {
             Navigation.navigate(ROUTES.SETTINGS_ENABLE_PAYMENTS_CONFIRM_MAGIC_CODE.getRoute());
             return;
         }

tests/unit/useWalletPhoneMagicCodeTest.tsx

@@ -83,16 +83,15 @@ describe('useWalletPhoneMagicCode', () => {
         expect(mockNavigate).not.toHaveBeenCalled();
     });
 
-    it('submits directly without routing to the magic-code screen when no phone number is saved yet', async () => {
+    it('routes to the magic-code screen when a phone number is set for the first time (none saved yet)', async () => {
         const {result} = await renderWalletPhoneMagicCode();
 
-        const params = buildParams();
         act(() => {
-            result.current.submitPersonalDetails(params);
+            result.current.submitPersonalDetails(buildParams());
         });
 
-        expect(mockUpdatePersonalDetails).toHaveBeenCalledWith(params);
-        expect(mockNavigate).not.toHaveBeenCalled();
+        expect(mockUpdatePersonalDetails).not.toHaveBeenCalled();
+        expect(mockNavigate).toHaveBeenCalledWith(ROUTES.SETTINGS_ENABLE_PAYMENTS_CONFIRM_MAGIC_CODE.getRoute());
     });
 
     it('routes to the magic-code screen and holds the submission when an existing phone number is changed', async () => {
@@ -109,4 +108,16 @@ describe('useWalletPhoneMagicCode', () => {
         expect(mockUpdatePersonalDetails).not.toHaveBeenCalled();
         expect(mockNavigate).toHaveBeenCalledWith(ROUTES.SETTINGS_ENABLE_PAYMENTS_CONFIRM_MAGIC_CODE.getRoute());
     });
+
+    it('submits directly without routing to the magic-code screen when no phone number is provided', async () => {
+        const {result} = await renderWalletPhoneMagicCode();
+
+        const params = buildParams({phoneNumber: ''});
+        act(() => {
+            result.current.submitPersonalDetails(params);
+        });
+
+        expect(mockUpdatePersonalDetails).toHaveBeenCalledWith(params);
+        expect(mockNavigate).not.toHaveBeenCalled();
+    });
 });