From 03fd89fd7717d21f02c3bf0fc1ac0ccb9bcfb4c9 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 18 Feb 2026 13:06:36 +0100 Subject: [PATCH] [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 --- .../app/src/Features/Helpers/EmailHelper.mjs | 16 +++++ .../Features/Security/LoginRateLimiter.mjs | 4 +- .../acceptance/src/AuthenticationTests.mjs | 61 +++++++++++++++++-- 3 files changed, 75 insertions(+), 6 deletions(-) diff --git a/services/web/app/src/Features/Helpers/EmailHelper.mjs b/services/web/app/src/Features/Helpers/EmailHelper.mjs index 34e96b91d9..8e25afacc6 100644 --- a/services/web/app/src/Features/Helpers/EmailHelper.mjs +++ b/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, } diff --git a/services/web/app/src/Features/Security/LoginRateLimiter.mjs b/services/web/app/src/Features/Security/LoginRateLimiter.mjs index 505d6b3b6a..4fb032f222 100644 --- a/services/web/app/src/Features/Security/LoginRateLimiter.mjs +++ b/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 diff --git a/services/web/test/acceptance/src/AuthenticationTests.mjs b/services/web/test/acceptance/src/AuthenticationTests.mjs index aea57f3ea6..d3acb19de3 100644 --- a/services/web/test/acceptance/src/AuthenticationTests.mjs +++ b/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, + }) + }) }) })