diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js index c28a26e590..3546972099 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js @@ -69,7 +69,7 @@ const CollaboratorsInviteController = { const maxRequests = 10 * collabLimit const points = Math.floor(RATE_LIMIT_POINTS / maxRequests) try { - await rateLimiter.consume(userId, points) + await rateLimiter.consume(userId, points, { method: 'userId' }) } catch (err) { if (err instanceof Error) { throw err diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index 99cda1601e..99bf62728f 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/services/web/app/src/Features/Compile/CompileController.js @@ -281,7 +281,7 @@ module.exports = CompileController = { const projectId = req.params.Project_id const rateLimit = function (callback) { pdfDownloadRateLimiter - .consume(req.ip) + .consume(req.ip, 1, { method: 'ip' }) .then(() => { callback(null, true) }) diff --git a/services/web/app/src/Features/Compile/CompileManager.js b/services/web/app/src/Features/Compile/CompileManager.js index c480644a8e..c65c4fff9e 100644 --- a/services/web/app/src/Features/Compile/CompileManager.js +++ b/services/web/app/src/Features/Compile/CompileManager.js @@ -310,7 +310,7 @@ module.exports = CompileManager = { Metrics.inc(`auto-compile-${compileGroup}`) const rateLimiter = getAutoCompileRateLimiter(compileGroup) rateLimiter - .consume('global') + .consume('global', 1, { method: 'global' }) .then(() => { callback(null, true) }) diff --git a/services/web/app/src/Features/Email/EmailSender.js b/services/web/app/src/Features/Email/EmailSender.js index fd62810bb3..ad23ae0de7 100644 --- a/services/web/app/src/Features/Email/EmailSender.js +++ b/services/web/app/src/Features/Email/EmailSender.js @@ -116,7 +116,7 @@ async function checkCanSendEmail(options) { return true } try { - await rateLimiter.consume(options.sendingUser_id) + await rateLimiter.consume(options.sendingUser_id, 1, { method: 'userId' }) } catch (err) { if (err instanceof Error) { throw err diff --git a/services/web/app/src/Features/Security/LoginRateLimiter.js b/services/web/app/src/Features/Security/LoginRateLimiter.js index bdeba035af..a80ec3285a 100644 --- a/services/web/app/src/Features/Security/LoginRateLimiter.js +++ b/services/web/app/src/Features/Security/LoginRateLimiter.js @@ -8,7 +8,7 @@ const rateLimiter = new RateLimiter('login', { function processLoginRequest(email, callback) { rateLimiter - .consume(email) + .consume(email, 1, { method: 'email' }) .then(() => { callback(null, true) }) diff --git a/services/web/app/src/Features/Security/RateLimiterMiddleware.js b/services/web/app/src/Features/Security/RateLimiterMiddleware.js index 10b28cd61c..6a99ac610b 100644 --- a/services/web/app/src/Features/Security/RateLimiterMiddleware.js +++ b/services/web/app/src/Features/Security/RateLimiterMiddleware.js @@ -16,32 +16,33 @@ const settings = require('@overleaf/settings') * will rate limit each project_id separately. * * Unique clients are identified by user_id if logged in, and IP address if not. + * The method label is used to identify this in our metrics. */ function rateLimit(rateLimiter, opts = {}) { const getUserId = opts.getUserId || (req => SessionManager.getLoggedInUserId(req.session)) return function (req, res, next) { - const userId = getUserId(req) || req.ip + const clientId = opts.ipOnly ? req.ip : getUserId(req) || req.ip + const method = clientId === req.ip ? 'ip' : 'userId' + if ( settings.smokeTest && settings.smokeTest.userId && - settings.smokeTest.userId.toString() === userId.toString() + settings.smokeTest.userId.toString() === clientId.toString() ) { // ignore smoke test user return next() } - let key - if (opts.ipOnly) { - key = req.ip - } else { + let key = clientId + if (!opts.ipOnly) { const params = (opts.params || []).map(p => req.params[p]) - params.push(userId) + params.push(clientId) key = params.join(':') } rateLimiter - .consume(key) + .consume(key, 1, { method }) .then(() => next()) .catch(err => { if (err instanceof Error) { diff --git a/services/web/app/src/Features/Subscription/TeamInvitesController.js b/services/web/app/src/Features/Subscription/TeamInvitesController.js index 2677d234bf..a3766ef9e4 100644 --- a/services/web/app/src/Features/Subscription/TeamInvitesController.js +++ b/services/web/app/src/Features/Subscription/TeamInvitesController.js @@ -264,7 +264,9 @@ async function resendInvite(req, res, next) { } try { - await rateLimiters.resendGroupInvite.consume(userEmail) + await rateLimiters.resendGroupInvite.consume(userEmail, 1, { + method: 'email', + }) const existingUser = await UserGetter.promises.getUserByAnyEmail(userEmail) diff --git a/services/web/app/src/infrastructure/RateLimiter.js b/services/web/app/src/infrastructure/RateLimiter.js index 46137995db..825aca1ed5 100644 --- a/services/web/app/src/infrastructure/RateLimiter.js +++ b/services/web/app/src/infrastructure/RateLimiter.js @@ -34,7 +34,7 @@ class RateLimiter { }) } - async consume(key, points = 1, options = {}) { + async consume(key, points = 1, options = { method: 'unknown' }) { if (Settings.disableRateLimits) { // Return a fake result in case it's used somewhere return { @@ -57,7 +57,10 @@ class RateLimiter { if (err.consumedPoints - points <= this._rateLimiter.points) { logger.warn({ path: this.name, key }, 'rate limit exceeded') } - Metrics.inc('rate-limit-hit', 1, { path: this.name }) + Metrics.inc('rate-limit-hit', 1, { + path: this.name, + method: options.method, + }) throw err } }