Merge pull request #19442 from mozilla/FXA-12390

task(settings): Debounce sending of code on click and render

Author: dschom

packages/fxa-settings/src/components/Settings/MfaGuard/error-boundary.test.tsx

@@ -5,21 +5,23 @@
 import React from 'react';
 import { render, screen } from '@testing-library/react';
 import { MfaErrorBoundary } from './error-boundary';
-import { JwtTokenCache } from '../../../lib/cache';
+import { JwtTokenCache, MfaOtpRequestCache } from '../../../lib/cache';
 
 const mockScope = 'test';
 const mockSessionToken = 'session-xyz';
 const mockJwt = 'jwt-123';
 
 describe('MfaErrorBoundary', () => {
-  let removeSpy: jest.SpyInstance;
+  let removeJwtSpy: jest.SpyInstance;
+  let removeOtpSpy: jest.SpyInstance;
 
   beforeEach(() => {
-    removeSpy = jest.spyOn(JwtTokenCache, 'removeToken');
+    removeJwtSpy = jest.spyOn(JwtTokenCache, 'removeToken');
+    removeOtpSpy = jest.spyOn(MfaOtpRequestCache, 'remove');
   });
 
   afterEach(() => {
-    removeSpy.mockReset();
+    removeJwtSpy.mockReset();
   });
 
   it('renders children when no error occurs', () => {
@@ -72,6 +74,7 @@ describe('MfaErrorBoundary', () => {
     );
 
     expect(screen.getByText('fallback')).toBeInTheDocument();
-    expect(removeSpy).toHaveBeenCalledWith(mockSessionToken, mockScope);
+    expect(removeJwtSpy).toHaveBeenCalledWith(mockSessionToken, mockScope);
+    expect(removeOtpSpy).toHaveBeenCalledWith(mockSessionToken, mockScope);
   });
 });

packages/fxa-settings/src/components/Settings/MfaGuard/error-boundary.tsx

@@ -4,7 +4,7 @@
 
 import { Component, ReactNode } from 'react';
 import { MfaScope } from '../../../lib/types';
-import { JwtTokenCache } from '../../../lib/cache';
+import { JwtTokenCache, MfaOtpRequestCache } from '../../../lib/cache';
 
 /**
  * Error Boundary Implementation.
@@ -58,6 +58,10 @@ export class MfaErrorBoundary extends Component<
         this.props.sessionToken,
         this.props.requiredScope
       );
+      MfaOtpRequestCache.remove(
+        this.props.sessionToken,
+        this.props.requiredScope
+      );
     } else {
       // Causes error to bubble up to the next error boundary
       throw error;

packages/fxa-settings/src/components/Settings/MfaGuard/index.test.tsx

@@ -3,11 +3,11 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import React from 'react';
-import { screen, waitFor } from '@testing-library/react';
+import { act, screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { mockAppContext, renderWithRouter } from '../../../models/mocks';
 import { MfaGuard } from './index';
-import { JwtTokenCache } from '../../../lib/cache';
+import { JwtTokenCache, MfaOtpRequestCache } from '../../../lib/cache';
 import { AuthUiErrors } from '../../../lib/auth-errors/auth-errors';
 import { AppContext } from '../../../models';
 
@@ -56,9 +56,8 @@ async function submitCode(otp: string = mockOtp) {
 
 describe('MfaGuard', () => {
   beforeEach(() => {
-    if (JwtTokenCache.hasToken(mockSessionToken, mockScope)) {
-      JwtTokenCache.removeToken(mockSessionToken, mockScope);
-    }
+    JwtTokenCache.removeToken(mockSessionToken, mockScope);
+    MfaOtpRequestCache.remove(mockSessionToken, mockScope);
     jest.clearAllMocks();
   });
 
@@ -127,7 +126,7 @@ describe('MfaGuard', () => {
   it('clears error banner on input change', async () => {
     renderWithRouter(
       <AppContext.Provider value={mockAppContext()}>
-        <MfaGuard requiredScope={mockScope}>
+        <MfaGuard requiredScope={mockScope} debounceIntervalMs={0}>
           <div>secured</div>
         </MfaGuard>
       </AppContext.Provider>
@@ -149,7 +148,7 @@ describe('MfaGuard', () => {
   it('shows resend success banner and hides error banner on resend success', async () => {
     renderWithRouter(
       <AppContext.Provider value={mockAppContext()}>
-        <MfaGuard requiredScope={mockScope}>
+        <MfaGuard requiredScope={mockScope} debounceIntervalMs={0}>
           <div>secured</div>
         </MfaGuard>
       </AppContext.Provider>
@@ -179,7 +178,7 @@ describe('MfaGuard', () => {
   it('shows error banner and hide success banner on resend error', async () => {
     renderWithRouter(
       <AppContext.Provider value={mockAppContext()}>
-        <MfaGuard requiredScope={mockScope}>
+        <MfaGuard requiredScope={mockScope} debounceIntervalMs={0}>
           <div>secured</div>
         </MfaGuard>
       </AppContext.Provider>
@@ -209,7 +208,7 @@ describe('MfaGuard', () => {
 
     renderWithRouter(
       <AppContext.Provider value={mockAppContext()}>
-        <MfaGuard requiredScope={mockScope}>
+        <MfaGuard requiredScope={mockScope} debounceIntervalMs={0}>
           <div>secured</div>
         </MfaGuard>
       </AppContext.Provider>
@@ -226,7 +225,11 @@ describe('MfaGuard', () => {
 
     renderWithRouter(
       <AppContext.Provider value={mockAppContext()}>
-        <MfaGuard requiredScope={mockScope} onDismissCallback={mockOnDismiss}>
+        <MfaGuard
+          requiredScope={mockScope}
+          onDismissCallback={mockOnDismiss}
+          debounceIntervalMs={0}
+        >
           <div>secured</div>
         </MfaGuard>
       </AppContext.Provider>
@@ -236,4 +239,39 @@ describe('MfaGuard', () => {
 
     expect(mockOnDismiss).toHaveBeenCalledTimes(1);
   });
+
+  it('debounces OTP resend requests', async () => {
+    renderWithRouter(
+      <AppContext.Provider value={mockAppContext()}>
+        <MfaGuard requiredScope={mockScope} debounceIntervalMs={100}>
+          <div>secured</div>
+        </MfaGuard>
+      </AppContext.Provider>
+    );
+
+    // Should be debounced! The dialog just rendered and a code went out...
+    await userEvent.click(
+      screen.getByRole('button', { name: 'Email new code.' })
+    );
+    await act(async () => {
+      await new Promise((r) => setTimeout(r, 101));
+    });
+
+    await userEvent.click(
+      screen.getByRole('button', { name: 'Email new code.' })
+    );
+    // Should be debounced! The resend request above was just clicked...
+    await userEvent.click(
+      screen.getByRole('button', { name: 'Email new code.' })
+    );
+
+    await act(async () => {
+      await new Promise((r) => setTimeout(r, 101));
+    });
+    await userEvent.click(
+      screen.getByRole('button', { name: 'Email new code.' })
+    );
+
+    expect(mockAuthClient.mfaRequestOtp).toHaveBeenCalledTimes(3);
+  });
 });

packages/fxa-settings/src/components/Settings/MfaGuard/index.tsx

@@ -6,7 +6,6 @@ import React, {
   ReactNode,
   useCallback,
   useEffect,
-  useRef,
   useState,
   useSyncExternalStore,
 } from 'react';
@@ -22,6 +21,7 @@ import {
 import Modal from '../ModalMfaProtected';
 import {
   JwtTokenCache,
+  MfaOtpRequestCache,
   sessionToken as getSessionToken,
 } from '../../../lib/cache';
 import { MfaScope } from '../../../lib/types';
@@ -39,26 +39,23 @@ export const MfaGuard = ({
   children,
   requiredScope,
   onDismissCallback = async () => {},
+  debounceIntervalMs = 3000,
 }: {
   children: ReactNode;
   requiredScope: MfaScope;
   onDismissCallback?: () => Promise<void>;
+  debounceIntervalMs?: number;
 }) => {
   // Let errors be handled by error boundaries in async contexts
   const handleError = useErrorHandler();
-
   const config = useConfig();
-
-  const hasSentConfirmationCode = useRef(false);
-
   const [localizedErrorBannerMessage, setLocalizedErrorBannerMessage] =
     useState<string | undefined>(undefined);
 
   const [resendCodeLoading, setResendCodeLoading] = useState(false);
   const [showResendSuccessBanner, setShowResendSuccessBanner] = useState(false);
 
   const resetStates = useCallback(() => {
-    hasSentConfirmationCode.current = false;
     setLocalizedErrorBannerMessage(undefined);
     setShowResendSuccessBanner(false);
   }, []);
@@ -94,21 +91,34 @@ export const MfaGuard = ({
     throw new Error('Invalid state. Missing jwt cache.');
   }
 
+  const debounce = useCallback(
+    (limitInMs: number) => {
+      const lastRequest = MfaOtpRequestCache.get(sessionToken, requiredScope);
+      return lastRequest != null && Date.now() - lastRequest < limitInMs;
+    },
+    [sessionToken, requiredScope]
+  );
+
   // Modal Setup
   useEffect(() => {
     (async () => {
       // To avoid requesting multiple OTPs on mount
-      if (
-        hasSentConfirmationCode.current ||
-        JwtTokenCache.hasToken(sessionToken, requiredScope)
-      ) {
+      if (JwtTokenCache.hasToken(sessionToken, requiredScope)) {
         return;
       }
+
+      // Avoid bombarding the user with emails just because they open
+      // the dialog
+      const limitInMs = config.mfa.otp.expiresInMinutes * 60 * 1000;
+      if (debounce(limitInMs)) {
+        return;
+      }
+
       try {
-        hasSentConfirmationCode.current = true;
+        MfaOtpRequestCache.set(sessionToken, requiredScope);
         await authClient.mfaRequestOtp(sessionToken, requiredScope);
       } catch (err) {
-        hasSentConfirmationCode.current = false;
+        MfaOtpRequestCache.remove(sessionToken, requiredScope);
         if (err.code === 401) {
           handleError(err);
           return;
@@ -127,6 +137,8 @@ export const MfaGuard = ({
     alertBar,
     ftlMsgResolver,
     onDismiss,
+    config.mfa.otp.expiresInMinutes,
+    debounce,
   ]);
 
   const onSubmitOtp = async (code: string) => {
@@ -151,12 +163,19 @@ export const MfaGuard = ({
   };
 
   const handleResendCode = async () => {
+    // Stop users from hammering the resend button...
+    if (debounce(debounceIntervalMs)) {
+      return;
+    }
+
     setResendCodeLoading(true);
     try {
+      MfaOtpRequestCache.set(sessionToken, requiredScope);
       await authClient.mfaRequestOtp(sessionToken, requiredScope);
       setLocalizedErrorBannerMessage(undefined);
       setShowResendSuccessBanner(true);
     } catch (err) {
+      MfaOtpRequestCache.remove(sessionToken, requiredScope);
       setShowResendSuccessBanner(false);
       if (err.code === 401) {
         handleError(err);

packages/fxa-settings/src/lib/cache.ts

@@ -404,3 +404,60 @@ export class JwtNotFoundError extends Error {
     super(message);
   }
 }
+
+/**
+ * Manages OTP request cache
+ *
+ * The main purpose of this cache is to track when we email users
+ * MFA OTP codes. This info can then be used to debounce sends.
+ */
+export class MfaOtpRequestCache {
+  /** Key where data is held in persistent storage */
+  private static readonly storageKey = 'mfa_otp_requests';
+
+  /** Internal state, access is protected by getters / setters below. */
+  private static _state?: Record<string, number>;
+
+  /** Gets the current state with backing in persistent storage */
+  private static get state(): Record<string, number> {
+    if (this._state != null) {
+      return this._state;
+    }
+
+    // Fallback to stored state, if stored state is invalid, then
+    // assume fresh slate, and create new state object
+    this._state = storage.get(this.storageKey);
+    if (this._state == null) {
+      this._state = {};
+    }
+
+    return this._state;
+  }
+
+  private static set state(val: Record<string, number>) {
+    this._state = val;
+    this.store();
+  }
+
+  static getKey(sessionToken: string, scope: MfaScope) {
+    return `${sessionToken}-${scope}`;
+  }
+
+  private static store() {
+    storage.set(this.storageKey, this._state);
+  }
+
+  static set(sessionToken: string, requiredScope: MfaScope) {
+    this.state[this.getKey(sessionToken, requiredScope)] = Date.now();
+    this.store();
+  }
+
+  static remove(sessionToken: string, requiredScope: MfaScope) {
+    delete this.state[this.getKey(sessionToken, requiredScope)];
+    this.store();
+  }
+
+  static get(sessionToken: string, requiredScope: MfaScope) {
+    return this.state[this.getKey(sessionToken, requiredScope)];
+  }
+}