diff --git a/services/web/app/coffee/Features/Security/RateLimiterMiddlewear.coffee b/services/web/app/coffee/Features/Security/RateLimiterMiddlewear.coffee new file mode 100644 index 0000000000..8e7f1f7bc5 --- /dev/null +++ b/services/web/app/coffee/Features/Security/RateLimiterMiddlewear.coffee @@ -0,0 +1,37 @@ +RateLimiter = require "../../infrastructure/RateLimiter" + +module.exports = RateLimiterMiddlewear = + ### + Do not allow more than opts.maxRequests from a single client in + opts.timeInterval. Pass an array of opts.params to segment this based on + parameters in the request URL, e.g.: + + app.get "/project/:project_id", RateLimiterMiddlewear.rateLimit(endpointName: "open-editor", params: ["project_id"]) + + will rate limit each project_id separately. + + Unique clients are identified by user_id if logged in, and IP address if not. + ### + rateLimit: (opts) -> + return (req, res, next) -> + if req.session.user? + user_id = req.session.user._id + else + user_id = req.ip + params = (opts.params or []).map (p) -> req.params[p] + params.push user_id + if !opts.endpointName? + throw new Error("no endpointName provided") + RateLimiter.addCount { + endpointName: opts.endpointName + timeInterval: opts.timeInterval or 60 + subjectName: params.join(":") + throttle: opts.maxRequests or 6 + }, (error, canContinue)-> + return next(error) if error? + if canContinue + next() + else + res.status(429) # Too many requests + res.write("Rate limit reached, please try again later") + res.end() \ No newline at end of file diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index a80f215cb8..b753fbafea 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -38,6 +38,7 @@ ConnectedUsersController = require("./Features/ConnectedUsers/ConnectedUsersCont DropboxRouter = require "./Features/Dropbox/DropboxRouter" dropboxHandler = require "./Features/Dropbox/DropboxHandler" Modules = require "./infrastructure/Modules" +RateLimiterMiddlewear = require('./Features/Security/RateLimiterMiddlewear') logger = require("logger-sharelatex") _ = require("underscore") @@ -91,7 +92,12 @@ module.exports = class Router app.get '/project', AuthenticationController.requireLogin(), ProjectController.projectListPage app.post '/project/new', AuthenticationController.requireLogin(), ProjectController.newProject - app.get '/Project/:Project_id', SecurityManager.requestCanAccessProject, ProjectController.loadEditor + app.get '/Project/:Project_id', RateLimiterMiddlewear.rateLimit({ + endpointName: "open-project" + params: ["Project_id"] + maxRequests: 10 + timeInterval: 60 + }), SecurityManager.requestCanAccessProject, ProjectController.loadEditor app.get '/Project/:Project_id/file/:File_id', SecurityManager.requestCanAccessProject, FileStoreController.getFile app.post '/project/:Project_id/settings', SecurityManager.requestCanModifyProject, ProjectController.updateProjectSettings diff --git a/services/web/test/UnitTests/coffee/Security/RateLimiterMiddlewearTests.coffee b/services/web/test/UnitTests/coffee/Security/RateLimiterMiddlewearTests.coffee new file mode 100644 index 0000000000..0a7b4dfbc5 --- /dev/null +++ b/services/web/test/UnitTests/coffee/Security/RateLimiterMiddlewearTests.coffee @@ -0,0 +1,82 @@ +SandboxedModule = require('sandboxed-module') +sinon = require('sinon') +require('chai').should() +modulePath = require('path').join __dirname, '../../../../app/js/Features/Security/RateLimiterMiddlewear' + +describe "RateLimiterMiddlewear", -> + beforeEach -> + @RateLimiterMiddlewear = SandboxedModule.require modulePath, requires: + '../../infrastructure/RateLimiter' : @RateLimiter = {} + @req = + params: {} + session: {} + @res = + status: sinon.stub() + write: sinon.stub() + end: sinon.stub() + @next = sinon.stub() + + describe "rateLimit", -> + beforeEach -> + @rateLimiter = @RateLimiterMiddlewear.rateLimit({ + endpointName: "test-endpoint" + params: ["project_id", "doc_id"] + timeInterval: 42 + maxRequests: 12 + }) + @req.params = { + project_id: @project_id = "project-id" + doc_id: @doc_id = "doc-id" + } + + describe "when under the rate limit with logged in user", -> + beforeEach -> + @req.session.user = { _id: @user_id = "user-id" } + @RateLimiter.addCount = sinon.stub().callsArgWith(1, null, true) + @rateLimiter(@req, @res, @next) + + it "should call the rate limiter backend with the user_id", -> + @RateLimiter.addCount + .calledWith({ + endpointName: "test-endpoint" + timeInterval: 42 + throttle: 12 + subjectName: "#{@project_id}:#{@doc_id}:#{@user_id}" + }) + .should.equal true + + it "should pass on to next()", -> + @next.called.should.equal true + + describe "when under the rate limit with anonymous user", -> + beforeEach -> + @req.ip = @ip = "1.2.3.4" + @RateLimiter.addCount = sinon.stub().callsArgWith(1, null, true) + @rateLimiter(@req, @res, @next) + + it "should call the rate limiter backend with the ip address", -> + @RateLimiter.addCount + .calledWith({ + endpointName: "test-endpoint" + timeInterval: 42 + throttle: 12 + subjectName: "#{@project_id}:#{@doc_id}:#{@ip}" + }) + .should.equal true + + it "should pass on to next()", -> + @next.called.should.equal true + + describe "when over the rate limit", -> + beforeEach -> + @req.session.user = { _id: @user_id = "user-id" } + @RateLimiter.addCount = sinon.stub().callsArgWith(1, null, false) + @rateLimiter(@req, @res, @next) + + it "should return a 429", -> + @res.status.calledWith(429).should.equal true + @res.end.called.should.equal true + + it "should not continue", -> + @next.called.should.equal false + \ No newline at end of file