diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee index 460b62da1d..1fde81f5c9 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee @@ -10,6 +10,7 @@ EditorRealTimeController = require("../Editor/EditorRealTimeController") NotificationsBuilder = require("../Notifications/NotificationsBuilder") AnalyticsManger = require("../Analytics/AnalyticsManager") AuthenticationController = require("../Authentication/AuthenticationController") +rateLimiter = require("../../infrastructure/RateLimiter") module.exports = CollaboratorsInviteController = @@ -22,7 +23,7 @@ module.exports = CollaboratorsInviteController = return next(err) res.json({invites: invites}) - _checkShouldInviteEmail: (email, callback=(err, shouldAllowInvite)->) -> + _checkShouldInviteEmail: (sendingUser, email, callback=(err, shouldAllowInvite)->) -> if Settings.restrictInvitesToExistingAccounts == true logger.log {email}, "checking if user exists with this email" UserGetter.getUser {email: email}, {_id: 1}, (err, user) -> @@ -30,7 +31,19 @@ module.exports = CollaboratorsInviteController = userExists = user? and user?._id? callback(null, userExists) else - callback(null, true) + UserGetter.getUser sendingUser._id, {features:1, _id:1}, (err, user)-> + if err? + return callback(err) + collabLimit = user?.features?.collaborators || 1 + if collabLimit == -1 + collabLimit = 20 + collabLimit = collabLimit * 10 + opts = + endpointName: "invite_to_project" + timeInterval: 60 * 30 + subjectName: sendingUser._id + throttle: collabLimit + rateLimiter.addCount opts, callback inviteToProject: (req, res, next) -> projectId = req.params.Project_id @@ -51,7 +64,7 @@ module.exports = CollaboratorsInviteController = if !email? or email == "" logger.log {projectId, email, sendingUserId}, "invalid email address" return res.sendStatus(400) - CollaboratorsInviteController._checkShouldInviteEmail email, (err, shouldAllowInvite)-> + CollaboratorsInviteController._checkShouldInviteEmail sendingUser, email, (err, shouldAllowInvite)-> if err? logger.err {err, email, projectId, sendingUserId}, "error checking if we can invite this email address" return next(err) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee index 4c7cc8c76a..8b130d27db 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee @@ -24,7 +24,13 @@ module.exports = RateLimiterMiddlewear.rateLimit({ endpointName: "invite-to-project" params: ["Project_id"] - maxRequests: 200 + maxRequests: 100 + timeInterval: 60 * 10 + }), + RateLimiterMiddlewear.rateLimit({ + endpointName: "invite-to-project-ip" + ipOnly:true + maxRequests: 100 timeInterval: 60 * 10 }), AuthenticationController.requireLogin(), diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 0a06a2a175..5360adb7a8 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -97,7 +97,7 @@ Thank you templates.projectInvite = - subject: _.template "<%= project.name %> - shared by <%= owner.email %>" + subject: _.template "<%= project.name.slice(0, 40) %> - shared by <%= owner.email %>" layout: BaseWithHeaderEmailLayout type:"notification" plainTextTemplate: _.template """ @@ -111,16 +111,16 @@ Thank you """ compiledTemplate: (opts) -> SingleCTAEmailBody({ - title: "#{ opts.project.name } – shared by #{ opts.owner.email }" + title: "#{ opts.project.name.slice(0, 40) } – shared by #{ opts.owner.email }" greeting: "Hi," - message: "#{ opts.owner.email } wants to share “#{ opts.project.name }” with you." + message: "#{ opts.owner.email } wants to share “#{ opts.project.name.slice(0, 40) }” with you." secondaryMessage: null ctaText: "View project" ctaURL: opts.inviteUrl gmailGoToAction: target: opts.inviteUrl name: "View project" - description: "Join #{ opts.project.name } at ShareLaTeX" + description: "Join #{ opts.project.name.slice(0, 40) } at ShareLaTeX" }) templates.completeJoinGroupAccount = diff --git a/services/web/app/coffee/Features/Security/RateLimiterMiddlewear.coffee b/services/web/app/coffee/Features/Security/RateLimiterMiddlewear.coffee index f486e94493..04b81581bf 100644 --- a/services/web/app/coffee/Features/Security/RateLimiterMiddlewear.coffee +++ b/services/web/app/coffee/Features/Security/RateLimiterMiddlewear.coffee @@ -19,12 +19,15 @@ module.exports = RateLimiterMiddlewear = user_id = AuthenticationController.getLoggedInUserId(req) || req.ip params = (opts.params or []).map (p) -> req.params[p] params.push user_id + subjectName = params.join(":") + if opts.ipOnly + subjectName = req.ip if !opts.endpointName? throw new Error("no endpointName provided") options = { endpointName: opts.endpointName timeInterval: opts.timeInterval or 60 - subjectName: params.join(":") + subjectName: subjectName throttle: opts.maxRequests or 6 } RateLimiter.addCount options, (error, canContinue)-> diff --git a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee index cf398e69da..bc1cb2e3b4 100644 --- a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee @@ -14,11 +14,20 @@ describe "CollaboratorsInviteController", -> @user = _id: 'id' @AnalyticsManger = recordEvent: sinon.stub() + @sendingUser = null @AuthenticationController = - getSessionUser: (req) => req.session.user + getSessionUser: (req) => + @sendingUser = req.session.user + return @sendingUser + + @RateLimiter = + addCount: sinon.stub + + @LimitationsManager = {} + @CollaboratorsInviteController = SandboxedModule.require modulePath, requires: "../Project/ProjectGetter": @ProjectGetter = {} - '../Subscription/LimitationsManager' : @LimitationsManager = {} + '../Subscription/LimitationsManager' : @LimitationsManager '../User/UserGetter': @UserGetter = {getUser: sinon.stub()} "./CollaboratorsHandler": @CollaboratorsHandler = {} "./CollaboratorsInviteHandler": @CollaboratorsInviteHandler = {} @@ -28,6 +37,7 @@ describe "CollaboratorsInviteController", -> "../Analytics/AnalyticsManager": @AnalyticsManger '../Authentication/AuthenticationController': @AuthenticationController 'settings-sharelatex': @settings = {} + "../../infrastructure/RateLimiter":@RateLimiter @res = new MockResponse() @req = new MockRequest() @@ -104,15 +114,10 @@ describe "CollaboratorsInviteController", -> describe 'when all goes well', -> beforeEach -> - @_checkShouldInviteEmail = sinon.stub( - @CollaboratorsInviteController, '_checkShouldInviteEmail' - ).callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail = sinon.stub().callsArgWith(2, null, true) @LimitationsManager.canAddXCollaborators = sinon.stub().callsArgWith(2, null, true) @CollaboratorsInviteController.inviteToProject @req, @res, @next - afterEach -> - @_checkShouldInviteEmail.restore() - it 'should produce json response', -> @res.json.callCount.should.equal 1 ({invite: @invite}).should.deep.equal(@res.json.firstCall.args[0]) @@ -122,8 +127,8 @@ describe "CollaboratorsInviteController", -> @LimitationsManager.canAddXCollaborators.calledWith(@project_id).should.equal true it 'should have called _checkShouldInviteEmail', -> - @_checkShouldInviteEmail.callCount.should.equal 1 - @_checkShouldInviteEmail.calledWith(@targetEmail).should.equal true + @CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal 1 + @CollaboratorsInviteController._checkShouldInviteEmail.calledWith(@sendingUser, @targetEmail).should.equal true it 'should have called inviteToProject', -> @CollaboratorsInviteHandler.inviteToProject.callCount.should.equal 1 @@ -136,22 +141,17 @@ describe "CollaboratorsInviteController", -> describe 'when the user is not allowed to add more collaborators', -> beforeEach -> - @_checkShouldInviteEmail = sinon.stub( - @CollaboratorsInviteController, '_checkShouldInviteEmail' - ).callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail = sinon.stub().callsArgWith(2, null, true) @LimitationsManager.canAddXCollaborators = sinon.stub().callsArgWith(2, null, false) @CollaboratorsInviteController.inviteToProject @req, @res, @next - afterEach -> - @_checkShouldInviteEmail.restore() - it 'should produce json response without an invite', -> @res.json.callCount.should.equal 1 ({invite: null}).should.deep.equal(@res.json.firstCall.args[0]) it 'should not have called _checkShouldInviteEmail', -> - @_checkShouldInviteEmail.callCount.should.equal 0 - @_checkShouldInviteEmail.calledWith(@targetEmail).should.equal false + @CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal 0 + @CollaboratorsInviteController._checkShouldInviteEmail.calledWith(@sendingUser, @targetEmail).should.equal false it 'should not have called inviteToProject', -> @CollaboratorsInviteHandler.inviteToProject.callCount.should.equal 0 @@ -159,23 +159,18 @@ describe "CollaboratorsInviteController", -> describe 'when canAddXCollaborators produces an error', -> beforeEach -> - @_checkShouldInviteEmail = sinon.stub( - @CollaboratorsInviteController, '_checkShouldInviteEmail' - ).callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail = sinon.stub().callsArgWith(2, null, true) @err = new Error('woops') @LimitationsManager.canAddXCollaborators = sinon.stub().callsArgWith(2, @err) @CollaboratorsInviteController.inviteToProject @req, @res, @next - afterEach -> - @_checkShouldInviteEmail.restore() - it 'should call next with an error', -> @next.callCount.should.equal 1 @next.calledWith(@err).should.equal true it 'should not have called _checkShouldInviteEmail', -> - @_checkShouldInviteEmail.callCount.should.equal 0 - @_checkShouldInviteEmail.calledWith(@targetEmail).should.equal false + @CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal 0 + @CollaboratorsInviteController._checkShouldInviteEmail.calledWith(@sendingUser, @targetEmail).should.equal false it 'should not have called inviteToProject', -> @CollaboratorsInviteHandler.inviteToProject.callCount.should.equal 0 @@ -183,16 +178,11 @@ describe "CollaboratorsInviteController", -> describe 'when inviteToProject produces an error', -> beforeEach -> - @_checkShouldInviteEmail = sinon.stub( - @CollaboratorsInviteController, '_checkShouldInviteEmail' - ).callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail = sinon.stub().callsArgWith(2, null, true) @err = new Error('woops') @CollaboratorsInviteHandler.inviteToProject = sinon.stub().callsArgWith(4, @err) @CollaboratorsInviteController.inviteToProject @req, @res, @next - afterEach -> - @_checkShouldInviteEmail.restore() - it 'should call next with an error', -> @next.callCount.should.equal 1 @next.calledWith(@err).should.equal true @@ -202,8 +192,8 @@ describe "CollaboratorsInviteController", -> @LimitationsManager.canAddXCollaborators.calledWith(@project_id).should.equal true it 'should have called _checkShouldInviteEmail', -> - @_checkShouldInviteEmail.callCount.should.equal 1 - @_checkShouldInviteEmail.calledWith(@targetEmail).should.equal true + @CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal 1 + @CollaboratorsInviteController._checkShouldInviteEmail.calledWith(@sendingUser, @targetEmail).should.equal true it 'should have called inviteToProject', -> @CollaboratorsInviteHandler.inviteToProject.callCount.should.equal 1 @@ -212,22 +202,17 @@ describe "CollaboratorsInviteController", -> describe 'when _checkShouldInviteEmail disallows the invite', -> beforeEach -> - @_checkShouldInviteEmail = sinon.stub( - @CollaboratorsInviteController, '_checkShouldInviteEmail' - ).callsArgWith(1, null, false) + @CollaboratorsInviteController._checkShouldInviteEmail = sinon.stub().callsArgWith(2, null, false) @LimitationsManager.canAddXCollaborators = sinon.stub().callsArgWith(2, null, true) @CollaboratorsInviteController.inviteToProject @req, @res, @next - afterEach -> - @_checkShouldInviteEmail.restore() - it 'should produce json response with no invite, and an error property', -> @res.json.callCount.should.equal 1 ({invite: null, error: 'cannot_invite_non_user'}).should.deep.equal(@res.json.firstCall.args[0]) it 'should have called _checkShouldInviteEmail', -> - @_checkShouldInviteEmail.callCount.should.equal 1 - @_checkShouldInviteEmail.calledWith(@targetEmail).should.equal true + @CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal 1 + @CollaboratorsInviteController._checkShouldInviteEmail.calledWith(@sendingUser, @targetEmail).should.equal true it 'should not have called inviteToProject', -> @CollaboratorsInviteHandler.inviteToProject.callCount.should.equal 0 @@ -235,22 +220,17 @@ describe "CollaboratorsInviteController", -> describe 'when _checkShouldInviteEmail produces an error', -> beforeEach -> - @_checkShouldInviteEmail = sinon.stub( - @CollaboratorsInviteController, '_checkShouldInviteEmail' - ).callsArgWith(1, new Error('woops')) + @CollaboratorsInviteController._checkShouldInviteEmail = sinon.stub().callsArgWith(2, new Error('woops')) @LimitationsManager.canAddXCollaborators = sinon.stub().callsArgWith(2, null, true) @CollaboratorsInviteController.inviteToProject @req, @res, @next - afterEach -> - @_checkShouldInviteEmail.restore() - it 'should call next with an error', -> @next.callCount.should.equal 1 @next.calledWith(@err).should.equal true it 'should have called _checkShouldInviteEmail', -> - @_checkShouldInviteEmail.callCount.should.equal 1 - @_checkShouldInviteEmail.calledWith(@targetEmail).should.equal true + @CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal 1 + @CollaboratorsInviteController._checkShouldInviteEmail.calledWith(@sendingUser, @targetEmail).should.equal true it 'should not have called inviteToProject', -> @CollaboratorsInviteHandler.inviteToProject.callCount.should.equal 0 @@ -260,14 +240,10 @@ describe "CollaboratorsInviteController", -> beforeEach -> @req.session.user = {_id: 'abc', email: 'me@example.com'} @req.body.email = 'me@example.com' - @_checkShouldInviteEmail = sinon.stub( - @CollaboratorsInviteController, '_checkShouldInviteEmail' - ).callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail = sinon.stub().callsArgWith(2, null, true) @LimitationsManager.canAddXCollaborators = sinon.stub().callsArgWith(2, null, true) @CollaboratorsInviteController.inviteToProject @req, @res, @next - afterEach -> - @_checkShouldInviteEmail.restore() it 'should reject action, return json response with error code', -> @res.json.callCount.should.equal 1 @@ -277,7 +253,7 @@ describe "CollaboratorsInviteController", -> @LimitationsManager.canAddXCollaborators.callCount.should.equal 0 it 'should not have called _checkShouldInviteEmail', -> - @_checkShouldInviteEmail.callCount.should.equal 0 + @CollaboratorsInviteController._checkShouldInviteEmail.callCount.should.equal 0 it 'should not have called inviteToProject', -> @CollaboratorsInviteHandler.inviteToProject.callCount.should.equal 0 @@ -702,13 +678,14 @@ describe "CollaboratorsInviteController", -> beforeEach -> @email = 'user@example.com' - @call = (callback) => - @CollaboratorsInviteController._checkShouldInviteEmail @email, callback + describe 'when we should be restricting to existing accounts', -> beforeEach -> @settings.restrictInvitesToExistingAccounts = true + @call = (callback) => + @CollaboratorsInviteController._checkShouldInviteEmail {}, @email, callback describe 'when user account is present', -> @@ -753,18 +730,46 @@ describe "CollaboratorsInviteController", -> expect(shouldAllow).to.equal undefined done() - describe 'when we should not be restricting', -> + describe 'when we should not be restricting on only registered users but do rate limit', -> beforeEach -> @settings.restrictInvitesToExistingAccounts = false + @sendingUser = + _id:"32312313" + features: + collaborators:17.8 + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @sendingUser) - it 'should callback with `true`', (done) -> - @call (err, shouldAllow) => - expect(err).to.equal null - expect(shouldAllow).to.equal true + it 'should callback with `true` when rate limit under', (done) -> + @RateLimiter.addCount = sinon.stub().callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail @sendingUser, @email, (err, result)=> + @RateLimiter.addCount.called.should.equal true + result.should.equal true done() - it 'should not have called getUser', (done) -> - @call (err, shouldAllow) => - @UserGetter.getUser.callCount.should.equal 0 + it 'should callback with `false` when rate limit hit', (done) -> + @RateLimiter.addCount = sinon.stub().callsArgWith(1, null, false) + @CollaboratorsInviteController._checkShouldInviteEmail @sendingUser, @email, (err, result)=> + @RateLimiter.addCount.called.should.equal true + result.should.equal false done() + + it 'should call rate limiter with 10x the collaborators', (done) -> + @RateLimiter.addCount = sinon.stub().callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail @sendingUser, @email, (err, result)=> + @RateLimiter.addCount.args[0][0].throttle.should.equal(178) + done() + + it 'should call rate limiter with 200 when collaborators is -1', (done) -> + @sendingUser.features.collaborators = -1 + @RateLimiter.addCount = sinon.stub().callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail @sendingUser, @email, (err, result)=> + @RateLimiter.addCount.args[0][0].throttle.should.equal(200) + done() + + it 'should call rate limiter with 10 when user has no collaborators set', (done) -> + delete @sendingUser.features + @RateLimiter.addCount = sinon.stub().callsArgWith(1, null, true) + @CollaboratorsInviteController._checkShouldInviteEmail @sendingUser, @email, (err, result)=> + @RateLimiter.addCount.args[0][0].throttle.should.equal(10) + done() \ No newline at end of file