Ver Fonte

Merge pull request #14090 from overleaf/bg-best-reduce-rate-limiter-on-confirmation-email-resend

reduce rate limiter on confirmation emails

GitOrigin-RevId: 87743dd9dac483a68ff82f1185ae1156d60b0575
Brian Gough há 2 anos atrás
pai
commit
bf04275478

+ 1 - 1
services/web/app/src/router.js

@@ -187,7 +187,7 @@ const rateLimiters = {
     duration: 60,
   }),
   resendConfirmation: new RateLimiter('resend-confirmation', {
-    points: 10,
+    points: 1,
     duration: 60,
   }),
   sendChatMessage: new RateLimiter('send-chat-message', {

+ 18 - 4
services/web/frontend/js/features/project-list/components/notifications/groups/affiliation/reconfirm-affiliation.tsx

@@ -4,7 +4,10 @@ import { Button } from 'react-bootstrap'
 import Icon from '../../../../../../shared/components/icon'
 import getMeta from '../../../../../../utils/meta'
 import useAsync from '../../../../../../shared/hooks/use-async'
-import { postJSON } from '../../../../../../infrastructure/fetch-json'
+import {
+  FetchError,
+  postJSON,
+} from '../../../../../../infrastructure/fetch-json'
 import { UserEmailData } from '../../../../../../../../types/user-email'
 import { ExposedSettings } from '../../../../../../../../types/exposed-settings'
 import { Institution } from '../../../../../../../../types/institution'
@@ -21,7 +24,7 @@ function ReconfirmAffiliation({
 }: ReconfirmAffiliationProps) {
   const { t } = useTranslation()
   const { samlInitPath } = getMeta('ol-ExposedSettings') as ExposedSettings
-  const { isLoading, isError, isSuccess, runAsync } = useAsync()
+  const { error, isLoading, isError, isSuccess, runAsync } = useAsync()
   const [hasSent, setHasSent] = useState(false)
   const [isPending, setIsPending] = useState(false)
   const ssoEnabled = institution.ssoEnabled
@@ -48,6 +51,9 @@ function ReconfirmAffiliation({
     }
   }
 
+  const rateLimited =
+    error && error instanceof FetchError && error.response?.status === 429
+
   if (hasSent) {
     return (
       <div className="w-100">
@@ -73,7 +79,11 @@ function ReconfirmAffiliation({
         {isError && (
           <>
             <br />
-            <div>{t('generic_something_went_wrong')}</div>
+            <div>
+              {rateLimited
+                ? t('too_many_requests')
+                : t('generic_something_went_wrong')}
+            </div>
           </>
         )}
       </div>
@@ -116,7 +126,11 @@ function ReconfirmAffiliation({
       {isError && (
         <>
           <br />
-          <div>{t('generic_something_went_wrong')}</div>
+          <div>
+            {rateLimited
+              ? t('too_many_requests')
+              : t('generic_something_went_wrong')}
+          </div>
         </>
       )}
     </div>

+ 15 - 4
services/web/frontend/js/features/settings/components/emails/reconfirmation-info/reconfirmation-info-prompt.tsx

@@ -1,6 +1,6 @@
 import { useState, useEffect, useLayoutEffect } from 'react'
 import useAsync from '../../../../../shared/hooks/use-async'
-import { postJSON } from '../../../../../infrastructure/fetch-json'
+import { FetchError, postJSON } from '../../../../../infrastructure/fetch-json'
 import { Trans, useTranslation } from 'react-i18next'
 import { Institution } from '../../../../../../../types/institution'
 import { Button } from 'react-bootstrap'
@@ -24,7 +24,7 @@ function ReconfirmationInfoPrompt({
 }: ReconfirmationInfoPromptProps) {
   const { t } = useTranslation()
   const { samlInitPath } = getMeta('ol-ExposedSettings') as ExposedSettings
-  const { isLoading, isError, isSuccess, runAsync } = useAsync()
+  const { error, isLoading, isError, isSuccess, runAsync } = useAsync()
   const { state, setLoading: setUserEmailsContextLoading } =
     useUserEmailsContext()
   const [isPending, setIsPending] = useState(false)
@@ -59,6 +59,9 @@ function ReconfirmationInfoPrompt({
     }
   }
 
+  const rateLimited =
+    isError && error instanceof FetchError && error.response?.status === 429
+
   if (hasSent) {
     return (
       <div>
@@ -87,7 +90,11 @@ function ReconfirmationInfoPrompt({
         )}
         <br />
         {isError && (
-          <div className="text-danger">{t('generic_something_went_wrong')}</div>
+          <div className="text-danger">
+            {rateLimited
+              ? t('too_many_requests')
+              : t('generic_something_went_wrong')}
+          </div>
         )}
       </div>
     )
@@ -117,7 +124,11 @@ function ReconfirmationInfoPrompt({
         </Button>
         <br />
         {isError && (
-          <div className="text-danger">{t('generic_something_went_wrong')}</div>
+          <div className="text-danger">
+            {rateLimited
+              ? t('too_many_requests')
+              : t('generic_something_went_wrong')}
+          </div>
         )}
       </div>
     </>

+ 10 - 3
services/web/frontend/js/features/settings/components/emails/resend-confirmation-email-button.tsx

@@ -2,7 +2,7 @@ import { useEffect } from 'react'
 import { useTranslation } from 'react-i18next'
 import Icon from '../../../../shared/components/icon'
 import { Button } from 'react-bootstrap'
-import { postJSON } from '../../../../infrastructure/fetch-json'
+import { FetchError, postJSON } from '../../../../infrastructure/fetch-json'
 import useAsync from '../../../../shared/hooks/use-async'
 import { UserEmailData } from '../../../../../../types/user-email'
 import { useUserEmailsContext } from '../../context/user-email-context'
@@ -15,7 +15,7 @@ function ResendConfirmationEmailButton({
   email,
 }: ResendConfirmationEmailButtonProps) {
   const { t } = useTranslation()
-  const { isLoading, isError, runAsync } = useAsync()
+  const { error, isLoading, isError, runAsync } = useAsync()
   const { state, setLoading: setUserEmailsContextLoading } =
     useUserEmailsContext()
 
@@ -42,6 +42,9 @@ function ResendConfirmationEmailButton({
     )
   }
 
+  const rateLimited =
+    error && error instanceof FetchError && error.response?.status === 429
+
   return (
     <>
       <Button
@@ -54,7 +57,11 @@ function ResendConfirmationEmailButton({
       </Button>
       <br />
       {isError && (
-        <div className="text-danger">{t('generic_something_went_wrong')}</div>
+        <div className="text-danger">
+          {rateLimited
+            ? t('too_many_requests')
+            : t('generic_something_went_wrong')}
+        </div>
       )}
     </>
   )

+ 12 - 0
services/web/test/acceptance/src/helpers/UserHelper.js

@@ -13,6 +13,16 @@ const {
   UserAuditLogEntry,
 } = require('../../../../app/src/models/UserAuditLogEntry')
 
+// Import the rate limiter so we can clear it between tests
+
+const {
+  RateLimiter,
+} = require('../../../../app/src/infrastructure/RateLimiter')
+
+const rateLimiters = {
+  resendConfirmation: new RateLimiter('resend-confirmation'),
+}
+
 let globalUserNum = Settings.test.counterInit
 
 class UserHelper {
@@ -438,6 +448,8 @@ class UserHelper {
   }
 
   async confirmEmail(userId, email) {
+    // clear ratelimiting on resend confirmation endpoint
+    await rateLimiters.resendConfirmation.delete(userId)
     // UserHelper.createUser does not create a confirmation token
     let response = await this.fetch('/user/emails/resend_confirmation', {
       method: 'POST',