Merge pull request #396 from sharelatex/ho-rate-limit-email-send-2

Ho rate limit email send 2

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)

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(),

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 } &ndash; shared by #{ opts.owner.email }"
+			title: "#{ opts.project.name.slice(0, 40) } &ndash; shared by #{ opts.owner.email }"
 			greeting: "Hi,"
-			message: "#{ opts.owner.email } wants to share &ldquo;#{ opts.project.name }&rdquo; with you."
+			message: "#{ opts.owner.email } wants to share &ldquo;#{ opts.project.name.slice(0, 40) }&rdquo; 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 =

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)->

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` 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 callback with `true`', (done) ->
-				@call (err, shouldAllow) =>
-					expect(err).to.equal null
-					expect(shouldAllow).to.equal true
+			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 not have called getUser', (done) ->
-				@call (err, shouldAllow) =>
-					@UserGetter.getUser.callCount.should.equal 0
+			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()