Browse Source

[web] validate and parse email using EmailHelper when ratelimiting (#31622)

* [web] validate and parse email using EmailHelper when ratelimiting

* [web] use a fake email for rate-limiting ldap logins in Server Pro

GitOrigin-RevId: 27ea7724319e06c4d64ac81e1155dcab558da99c
Jakob Ackermann 5 tháng trước cách đây
mục cha
commit
03fd89fd77

+ 16 - 0
services/web/app/src/Features/Helpers/EmailHelper.mjs

@@ -1,4 +1,5 @@
 import emailAddresses from 'email-addresses'
+import { z } from '../../infrastructure/Validation.mjs'
 
 const { parseOneAddress } = emailAddresses
 
@@ -7,11 +8,25 @@ const EMAIL_REGEXP =
   // eslint-disable-next-line no-useless-escape
   /^([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/
 
+const emailSchema = z
+  .string()
+  .transform(email => parseEmail(email))
+  .refine(v => v !== null, { message: 'Invalid email address' })
+
+/**
+ * @param {string} email
+ * @return {string|null}
+ */
 function getDomain(email) {
   email = parseEmail(email)
   return email ? email.split('@').pop() : null
 }
 
+/**
+ * @param {string} email
+ * @param {boolean} parseRfcAddress
+ * @return {string|null}
+ */
 function parseEmail(email, parseRfcAddress = false) {
   if (typeof email !== 'string' || !email) {
     return null
@@ -39,6 +54,7 @@ function parseEmail(email, parseRfcAddress = false) {
 }
 
 export default {
+  emailSchema,
   getDomain,
   parseEmail,
 }

+ 3 - 1
services/web/app/src/Features/Security/LoginRateLimiter.mjs

@@ -1,6 +1,7 @@
 import { RateLimiter } from '../../infrastructure/RateLimiter.mjs'
 import { callbackify } from '@overleaf/promise-utils'
 import Settings from '@overleaf/settings'
+import EmailHelper from '../Helpers/EmailHelper.mjs'
 
 const rateLimiterLoginEmail = new RateLimiter(
   'login',
@@ -11,8 +12,9 @@ const rateLimiterLoginEmail = new RateLimiter(
 )
 
 async function processLoginRequest(email) {
+  email = EmailHelper.emailSchema.parse(email)
   try {
-    await rateLimiterLoginEmail.consume(email.trim().toLowerCase(), 1, {
+    await rateLimiterLoginEmail.consume(email, 1, {
       method: 'email',
     })
     return true

+ 56 - 5
services/web/test/acceptance/src/AuthenticationTests.mjs

@@ -113,26 +113,35 @@ describe('Authentication', function () {
   })
 
   describe('rate-limit', function () {
+    const RATE_LIMIT = 10
+
     beforeEach('fetchCsrfToken', async function () {
       await user.login()
       await user.logout()
       await user.getCsrfToken()
     })
-    const tryLogin = async (i = 0) => {
+    async function tryLogin(i = 0, prefix = '') {
+      const { statusCode } = await tryLoginWithEmail(
+        `${prefix}${user.email}${' '.repeat(i)}`
+      )
+      return statusCode
+    }
+    async function tryLoginWithEmail(email) {
       const {
         response: { statusCode },
+        body,
       } = await user.doRequest('POST', {
         url: Settings.enableLegacyLogin ? '/login/legacy' : '/login',
         json: {
-          email: `${user.email}${' '.repeat(i)}`,
+          email,
           password: 'wrong-password',
           'g-recaptcha-response': 'valid',
         },
       })
-      return statusCode
+      return { statusCode, body }
     }
     it('should return 429 after 10 unsuccessful login attempts', async function () {
-      for (let i = 0; i < 10; i++) {
+      for (let i = 0; i < RATE_LIMIT; i++) {
         const statusCode = await tryLogin()
         expect(statusCode).to.equal(401)
       }
@@ -142,7 +151,7 @@ describe('Authentication', function () {
       }
     })
     it('ignore extra spaces in email address', async function () {
-      for (let i = 0; i < 10; i++) {
+      for (let i = 0; i < RATE_LIMIT; i++) {
         const statusCode = await tryLogin(i)
         expect(statusCode).to.equal(401)
       }
@@ -151,5 +160,47 @@ describe('Authentication', function () {
         expect(statusCode).to.equal(429)
       }
     })
+    it('normalizes the email address', async function () {
+      for (let i = 0; i < RATE_LIMIT / 2; i++) {
+        const statusCode = await tryLogin(i, 'x') // lower
+        expect(statusCode).to.equal(401)
+      }
+      for (let i = 0; i < RATE_LIMIT / 2; i++) {
+        const statusCode = await tryLogin(i, 'X') // upper
+        expect(statusCode).to.equal(401)
+      }
+      // combined, they exceed the rate-limit
+      for (let i = 0; i < 10; i++) {
+        const statusCode = await tryLogin(i, 'x')
+        expect(statusCode).to.equal(429)
+      }
+    })
+    it('should return 400 with bad email (missing @)', async function () {
+      const { statusCode, body } = await tryLoginWithEmail('foo')
+      expect(statusCode).to.equal(400)
+      expect(body).to.deep.equal({
+        name: 'ZodValidationError',
+        details: [
+          { code: 'custom', path: [], message: 'Invalid email address' },
+        ],
+        statusCode: 400,
+      })
+    })
+    it('should return 400 with bad email (number)', async function () {
+      const { statusCode, body } = await tryLoginWithEmail(1)
+      expect(statusCode).to.equal(400)
+      expect(body).to.deep.equal({
+        name: 'ZodValidationError',
+        details: [
+          {
+            expected: 'string',
+            code: 'invalid_type',
+            path: [],
+            message: 'Invalid input: expected string, received number',
+          },
+        ],
+        statusCode: 400,
+      })
+    })
   })
 })