Merge pull request #20127 from mozilla/fxa-13170

vbudhram · web-flow · commit 98a9c126c41e · 2026-03-04T23:05:48.000-05:00
fix(fxa-settings): Handle missing account data in localStorage
diff --git a/packages/fxa-settings/src/components/Settings/index.test.tsx b/packages/fxa-settings/src/components/Settings/index.test.tsx
@@ -192,11 +192,43 @@ describe('Settings App', () => {
     });
   });
 
+  it('redirects to root when account data is unavailable in localStorage', async () => {
+    const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
+
+    // Create an account object where accessing primaryEmail throws,
+    // simulating the Account.data getter failure when localStorage has no data
+    const brokenAccount = {
+      ...MOCK_ACCOUNT,
+    } as unknown as Account;
+    Object.defineProperty(brokenAccount, 'primaryEmail', {
+      get() {
+        throw new Error('Account data not loaded from localStorage');
+      },
+    });
+
+    renderWithRouter(
+      <AppContext.Provider
+        value={mockAppContext({ account: brokenAccount })}
+      >
+        <Subject />
+      </AppContext.Provider>,
+      { route: SETTINGS_PATH }
+    );
+
+    await waitFor(() => {
+      expect(mockNavigateWithQuery).toHaveBeenCalledWith('/');
+    });
+    expect(warnSpy).toHaveBeenCalledWith(
+      'Account data unavailable, redirecting to sign-in'
+    );
+    warnSpy.mockRestore();
+  });
+
   it('redirects to root if account is not verified', async () => {
     // this warning is expected, so we don't want to see it in the test output
     const warnSpy = jest.spyOn(console, 'warn').mockImplementation((msg) => {
       if (
-        msg === 'Account or email verification is require to access /settings!'
+        msg === 'Account or email verification is required to access /settings!'
       )
         return;
     });
@@ -230,7 +262,7 @@ describe('Settings App', () => {
     // this warning is expected, so we don't want to see it in the test output
     const warnSpy = jest.spyOn(console, 'warn').mockImplementation((msg) => {
       if (
-        msg === 'Account or email verification is require to access /settings!'
+        msg === 'Account or email verification is required to access /settings!'
       )
         return;
     });
@@ -261,7 +293,7 @@ describe('Settings App', () => {
     // this warning is expected, so we don't want to see it in the test output
     const warnSpy = jest.spyOn(console, 'warn').mockImplementation((msg) => {
       if (
-        msg === 'Account or email verification is require to access /settings!'
+        msg === 'Account or email verification is required to access /settings!'
       )
         return;
     });
diff --git a/packages/fxa-settings/src/components/Settings/index.tsx b/packages/fxa-settings/src/components/Settings/index.tsx
@@ -148,14 +148,19 @@ export const Settings = ({
     return <AppErrorDialog data-testid="error-dialog" />;
   }
 
-  // If the account email isn't verified or the user is an unverified session state,
-  // kick back to root to prompt for verification. This should only happen if the user
-  // tries to access /settings directly without entering a confirmation code on
-  // confirm_signup_code page or signin_token_code page.
-  if (account.primaryEmail.verified === false || sessionVerified === false) {
-    console.warn(
-      'Account or email verification is require to access /settings!'
-    );
+  // Redirect to root if the account or session is unverified. The try-catch
+  // handles the case where account data may be missing from localStorage
+  // (e.g. WKWebView storage eviction on iOS).
+  try {
+    if (account.primaryEmail.verified === false || sessionVerified === false) {
+      console.warn(
+        'Account or email verification is required to access /settings!'
+      );
+      navigateWithQuery('/');
+      return <LoadingSpinner fullScreen />;
+    }
+  } catch {
+    console.warn('Account data unavailable, redirecting to sign-in');
     navigateWithQuery('/');
     return <LoadingSpinner fullScreen />;
   }
@@ -178,6 +183,8 @@ export const Settings = ({
     return <LoadingSpinner fullScreen />;
   }
 
+  const hasPassword = account.hasPassword;
+
   return (
     <SettingsLayout>
       <Head />
@@ -191,7 +198,7 @@ export const Settings = ({
           <MfaGuardPage2faSetup path="/two_step_authentication" />
           <MfaGuardPage2faChange path="/two_step_authentication/change" />
           <MfaGuardPage2faReplaceBackupCodes path="/two_step_authentication/replace_codes" />
-          {account.hasPassword ? (
+          {hasPassword ? (
             <>
               <MfaGuardPageRecoveryKeyCreate path="/account_recovery" />
               <MfaGuardedPageChangePassword path="/change_password" />
diff --git a/packages/fxa-settings/src/lib/account-storage.ts b/packages/fxa-settings/src/lib/account-storage.ts
@@ -162,6 +162,30 @@ export function getCurrentAccountUid(): string | null {
   return storage().get('currentAccountUid') || null;
 }
 
+/**
+ * Find a valid account UID when currentAccountUid is missing.
+ * Returns the most recently logged-in account with a session token, or null.
+ * Used as a recovery path when WKWebView storage eviction loses the UID
+ * but account data still exists in the accounts map.
+ */
+export function findLastActiveAccountUid(): string | null {
+  const accounts = storage().get('accounts') || {};
+  let bestUid: string | null = null;
+  let bestLastLogin = -1;
+
+  for (const uid of Object.keys(accounts)) {
+    const account = accounts[uid];
+    if (account?.sessionToken && account.uid) {
+      const lastLogin = account.lastLogin ?? 0;
+      if (lastLogin > bestLastLogin) {
+        bestLastLogin = lastLogin;
+        bestUid = uid;
+      }
+    }
+  }
+  return bestUid;
+}
+
 export function getAccountData(uid?: string): UnifiedAccountData | null {
   const accountUid = uid || getCurrentAccountUid();
   if (!accountUid) return null;
diff --git a/packages/fxa-settings/src/models/Account.ts b/packages/fxa-settings/src/models/Account.ts
@@ -32,8 +32,10 @@ import {
 } from '../lib/interfaces';
 import { createSaltV2 } from 'fxa-auth-client/lib/salt';
 import { getHandledError } from '../lib/error-utils';
+import * as Sentry from '@sentry/browser';
 import {
   getFullAccountData,
+  getCurrentAccountUid,
   updateExtendedAccountState,
   updateBasicAccountData,
   ExtendedAccountState,
@@ -308,7 +310,18 @@ export class Account implements AccountData {
           securityEvents: [],
         } as AccountData;
       }
-      throw new Error('Account data not loaded from localStorage');
+      const error = new Error('Account data not loaded from localStorage');
+      Sentry.captureException(error, {
+        tags: { isIOS: /FxiOS/.test(navigator.userAgent) },
+        extra: {
+          currentAccountUid: getCurrentAccountUid(),
+          hasAccountsKey: !!localStorage.getItem('__fxa_storage.accounts'),
+          pathname: window.location.pathname,
+          search: window.location.search,
+          entrypoint: new URLSearchParams(window.location.search).get('entrypoint'),
+        },
+      });
+      throw error;
     }
 
     return {
diff --git a/packages/fxa-settings/src/models/contexts/AccountStateContext.tsx b/packages/fxa-settings/src/models/contexts/AccountStateContext.tsx
@@ -21,11 +21,14 @@ import {
   SecurityEvent,
 } from '../Account';
 import { useLocalStorageSync } from '../../lib/hooks/useLocalStorageSync';
+import * as Sentry from '@sentry/browser';
 import {
   getAccountData,
   updateAccountData,
   clearExtendedAccountState,
   getCurrentAccountUid,
+  setCurrentAccountUid,
+  findLastActiveAccountUid,
   UnifiedAccountData,
 } from '../../lib/account-storage';
 
@@ -168,14 +171,36 @@ function unifiedToAccountState(
   };
 }
 
+/**
+ * Recover the current account UID when it is missing from localStorage.
+ * This primarily affects Firefox iOS where WKWebView storage eviction can
+ * lose currentAccountUid while account data still exists.
+ *
+ * Recovery order: URL ?uid= param, then fallback UID, then scan for an
+ * active session in accounts storage.
+ */
+function recoverAccountUid(fallbackUid?: string | null): string | null {
+  const urlUid = new URLSearchParams(window.location.search).get('uid');
+  const recovered = urlUid || fallbackUid || findLastActiveAccountUid();
+  if (recovered) {
+    setCurrentAccountUid(recovered);
+  }
+  return recovered;
+}
+
 export function AccountStateProvider({
   children,
   initialState,
 }: AccountStateProviderProps) {
   // useLocalStorageSync triggers re-renders on storage changes;
   // we read from getAccountData() rather than the return value directly
   useLocalStorageSync('accounts');
-  const currentAccountUid = useLocalStorageSync('currentAccountUid') as string | undefined;
+  let currentAccountUid = useLocalStorageSync('currentAccountUid') as string | undefined;
+
+  // Recover UID when missing (iOS WKWebView storage eviction)
+  if (!currentAccountUid) {
+    currentAccountUid = recoverAccountUid() ?? undefined;
+  }
 
   const accountState = unifiedToAccountState(
     getAccountData(currentAccountUid),
@@ -185,8 +210,24 @@ export function AccountStateProvider({
   // Serialize AccountState -> UnifiedAccountData for localStorage:
   // Set -> Array, Error -> {message,name}, exclude derived 'primaryEmail'
   const setAccountDataCallback = useCallback((data: Partial<AccountState>) => {
-    const uid = getCurrentAccountUid();
-    if (!uid) return;
+    let uid = getCurrentAccountUid();
+
+    // Recover UID when currentAccountUid is missing
+    if (!uid) {
+      uid = recoverAccountUid(data.uid);
+    }
+
+    if (!uid) {
+      Sentry.captureMessage('setAccountData called with no currentAccountUid', {
+        level: 'warning',
+        extra: {
+          pathname: window.location.pathname,
+          search: window.location.search,
+          hasAccountsKey: !!localStorage.getItem('__fxa_storage.accounts'),
+        },
+      });
+      return;
+    }
 
     const { uid: dataUid, email: dataEmail, primaryEmail, ...rest } = data;
     const storageData: Partial<UnifiedAccountData> = {
