Fix error display when email mfa is locked (#188)

ValueMelody · Nov 3, 2024 · d663ab5 · d663ab5
1 parent af71cbe
commit d663ab5
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 32 deletions.
diff --git a/server/src/__tests__/normal/identity-mfa.test.tsx b/server/src/__tests__/normal/identity-mfa.test.tsx
@@ -1712,6 +1712,48 @@ describe(
       },
     )
 
+    test(
+      'could protect against threshold',
+      async () => {
+        process.env.EMAIL_MFA_EMAIL_THRESHOLD = 1 as unknown as string
+
+        await insertUsers(
+          db,
+          false,
+        )
+        await enrollEmailMfa(db)
+        const params = await prepareFollowUpParams(db)
+
+        const res = await app.request(
+          `${routeConfig.IdentityRoute.AuthorizeEmailMfa}${params}`,
+          {},
+          mock(db),
+        )
+        expect(res.status).toBe(200)
+        const html = await res.text()
+        const dom = new JSDOM(html)
+        const document = dom.window.document
+        expect(document.getElementsByTagName('select').length).toBe(1)
+        expect(document.getElementsByName('code').length).toBe(1)
+        expect(document.getElementsByTagName('form').length).toBe(1)
+
+        const res1 = await app.request(
+          `${routeConfig.IdentityRoute.AuthorizeEmailMfa}${params}`,
+          {},
+          mock(db),
+        )
+        expect(res1.status).toBe(200)
+        const html1 = await res1.text()
+        const dom1 = new JSDOM(html1)
+        const document1 = dom1.window.document
+        expect(document1.getElementsByTagName('select').length).toBe(1)
+        expect(document1.getElementsByName('code').length).toBe(0)
+        expect(document1.getElementsByTagName('form').length).toBe(0)
+
+        process.env.EMAIL_MFA_EMAIL_THRESHOLD = 10 as unknown as string
+      },
+    )
+
     test(
       'could disable locale selector',
       async () => {
@@ -1808,7 +1850,7 @@ describe(
         const res3 = await sendRequest()
         expect(res3.status).toBe(200)
 
-        process.env.EMAIL_MFA_EMAIL_THRESHOLD = 5 as unknown as string
+        process.env.EMAIL_MFA_EMAIL_THRESHOLD = 10 as unknown as string
       },
     )
 

diff --git a/server/src/handlers/identity/mfa.tsx b/server/src/handlers/identity/mfa.tsx
@@ -49,7 +49,12 @@ const handleSendEmailMfa = async (
     c.env.KV,
     authCode,
   )
-  if (!authCodeBody) return false
+  if (!authCodeBody) {
+    return {
+      result: false,
+      reason: localeConfig.Error.WrongAuthCode,
+    }
+  }
 
   const requireEmailMfa = enableEmailMfa || authCodeBody.user.mfaTypes.includes(userModel.MfaType.Email)
   const couldFallbackAsOtp = allowOtpSwitchToEmailMfa(
@@ -71,7 +76,12 @@ const handleSendEmailMfa = async (
   )
 
   if (threshold) {
-    if (attempts >= threshold) throw new errorConfig.Forbidden(localeConfig.Error.EmailMfaLocked)
+    if (attempts >= threshold) {
+      return {
+        result: false,
+        reason: localeConfig.Error.EmailMfaLocked,
+      }
+    }
 
     await kvService.setEmailMfaEmailAttempts(
       c.env.KV,
@@ -95,7 +105,7 @@ const handleSendEmailMfa = async (
     )
   }
 
-  return true
+  return { result: true }
 }
 
 const allowSmsSwitchToEmailMfa = (
@@ -515,12 +525,19 @@ export const getAuthorizeEmailMfa = async (c: Context<typeConfig.Context>) => {
     queryDto.code,
     queryDto.locale,
   )
-  if (!emailRes) return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${queryDto.locale}`)
+  if (!emailRes || (!emailRes.result && emailRes.reason === localeConfig.Error.WrongAuthCode)) {
+    return c.redirect(`${routeConfig.IdentityRoute.AuthCodeExpired}?locale=${queryDto.locale}`)
+  }
 
   return c.html(<AuthorizeEmailMfaView
     logoUrl={logoUrl}
     queryDto={queryDto}
     locales={enableLocaleSelector ? locales : [queryDto.locale]}
+    error={
+      !emailRes.result && emailRes.reason === localeConfig.Error.EmailMfaLocked
+        ? localeConfig.requestError.emailMfaLocked
+        : undefined
+    }
   />)
 }
 
@@ -580,7 +597,13 @@ export const postResendEmailMfa = async (c: Context<typeConfig.Context>) => {
     bodyDto.code,
     bodyDto.locale || locales[0],
   )
-  if (!emailRes) throw new errorConfig.Forbidden(localeConfig.Error.WrongAuthCode)
+  if (!emailRes || (!emailRes.result && emailRes.reason === localeConfig.Error.WrongAuthCode)) {
+    throw new errorConfig.Forbidden(localeConfig.Error.WrongAuthCode)
+  }
+
+  if (!emailRes.result && emailRes.reason === localeConfig.Error.EmailMfaLocked) {
+    throw new errorConfig.Forbidden(localeConfig.Error.EmailMfaLocked)
+  }
 
   return c.json({ success: true })
 }
diff --git a/server/src/views/AuthorizeEmailMfa.tsx b/server/src/views/AuthorizeEmailMfa.tsx
@@ -15,42 +15,50 @@ import Field from 'views/components/Field'
 import SubmitButton from 'views/components/SubmitButton'
 
 const AuthorizeEmailMfa = ({
-  queryDto, logoUrl, locales,
+  queryDto, logoUrl, locales, error,
 }: {
   queryDto: identityDto.GetAuthorizeFollowUpReqDto;
   logoUrl: string;
   locales: typeConfig.Locale[];
+  error?: { en: string; fr: string };
 }) => {
   return (
     <Layout
       locales={locales}
       logoUrl={logoUrl}
       locale={queryDto.locale}
     >
-      <h1 class='w-text text-center'>{localeConfig.authorizeEmailMfa.title[queryDto.locale]}</h1>
+      {!error && (
+        <h1 class='w-text text-center'>{localeConfig.authorizeEmailMfa.title[queryDto.locale]}</h1>
+      )}
       <SubmitError />
-      <form
-        onsubmit='return handleSubmit(event)'
-      >
-        <section class='flex-col gap-4'>
-          <Field
-            type='text'
-            required={false}
-            name='code'
-          />
-          <button
-            id='resend-btn'
-            type='button'
-            class='button-text'
-            onclick='resendCode()'
-          >
-            {localeConfig.authorizeEmailMfa.resend[queryDto.locale]}
-          </button>
-          <SubmitButton
-            title={localeConfig.authorizeEmailMfa.verify[queryDto.locale]}
-          />
-        </section>
-      </form>
+      {error && <SubmitError
+        show
+        message={error[queryDto.locale]} />}
+      {!error && (
+        <form
+          onsubmit='return handleSubmit(event)'
+        >
+          <section class='flex-col gap-4'>
+            <Field
+              type='text'
+              required={false}
+              name='code'
+            />
+            <button
+              id='resend-btn'
+              type='button'
+              class='button-text'
+              onclick='resendCode()'
+            >
+              {localeConfig.authorizeEmailMfa.resend[queryDto.locale]}
+            </button>
+            <SubmitButton
+              title={localeConfig.authorizeEmailMfa.verify[queryDto.locale]}
+            />
+          </section>
+        </form>
+      )}
       {html`
         <script>
           ${resetErrorScript.resetCodeError()}
@@ -71,6 +79,10 @@ const AuthorizeEmailMfa = ({
                 var resendBtn = document.getElementById("resend-btn")
                 resendBtn.disabled = true;
                 resendBtn.innerHTML = "${localeConfig.authorizeEmailMfa.resent[queryDto.locale]}"
+              } else {
+                return response.text().then(text => {
+                  throw new Error(text);
+                });
               }
             })
             .catch((error) => {

diff --git a/server/src/views/AuthorizeSmsMfa.tsx b/server/src/views/AuthorizeSmsMfa.tsx
@@ -171,6 +171,10 @@ const AuthorizeSmsMfa = ({
                   document.getElementById('code-row').classList.remove('hidden');
                   document.getElementById('resend-btn').classList.remove('hidden');
                   document.getElementById('submit-button').innerHTML = "${localeConfig.authorizeSmsMfa.verify[queryDto.locale]}"
+                } else {
+                  return response.text().then(text => {
+                    throw new Error(text);
+                  });
                 }
               })
               .catch((error) => {

diff --git a/server/src/views/components/SubmitError.tsx b/server/src/views/components/SubmitError.tsx
@@ -1,9 +1,16 @@
-const SubmitError = () => {
+const SubmitError = ({
+  show = false,
+  message,
+}: {
+  show?: boolean;
+  message?: string;
+}) => {
   return (
     <div
       id='submit-error'
-      class='alert mt-2 hidden'
+      class={`alert mt-2 ${show ? '' : 'hidden'}`}
     >
+      {message ?? ''}
     </div>
   )
 }