Forráskód Böngészése

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

rate limit emails sent sharing projects by users
Henry Oswald 9 éve
szülő
commit
21411e83f3

+ 2 - 1
services/web/app/coffee/Features/Collaborators/CollaboratorsEmailHandler.coffee

@@ -11,7 +11,7 @@ module.exports = CollaboratorsEmailHandler =
 			"user_first_name=#{encodeURIComponent(project.owner_ref.first_name)}"
 			"user_first_name=#{encodeURIComponent(project.owner_ref.first_name)}"
 		].join("&")
 		].join("&")
 
 
-	notifyUserOfProjectInvite: (project_id, email, invite, callback)->
+	notifyUserOfProjectInvite: (project_id, email, invite, sendingUser, callback)->
 		Project
 		Project
 			.findOne(_id: project_id )
 			.findOne(_id: project_id )
 			.select("name owner_ref")
 			.select("name owner_ref")
@@ -24,4 +24,5 @@ module.exports = CollaboratorsEmailHandler =
 						name: project.name
 						name: project.name
 					inviteUrl: CollaboratorsEmailHandler._buildInviteUrl(project, invite)
 					inviteUrl: CollaboratorsEmailHandler._buildInviteUrl(project, invite)
 					owner: project.owner_ref
 					owner: project.owner_ref
+					sendingUser_id: sendingUser._id
 				EmailHandler.sendEmail "projectInvite", emailOptions, callback
 				EmailHandler.sendEmail "projectInvite", emailOptions, callback

+ 1 - 1
services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee

@@ -53,7 +53,7 @@ module.exports = CollaboratorsInviteHandler =
 
 
 	_sendMessages: (projectId, sendingUser, invite, callback=(err)->) ->
 	_sendMessages: (projectId, sendingUser, invite, callback=(err)->) ->
 		logger.log {projectId, inviteId: invite._id}, "sending notification and email for invite"
 		logger.log {projectId, inviteId: invite._id}, "sending notification and email for invite"
-		CollaboratorsEmailHandler.notifyUserOfProjectInvite projectId, invite.email, invite, (err)->
+		CollaboratorsEmailHandler.notifyUserOfProjectInvite projectId, invite.email, invite, sendingUser, (err)->
 			return callback(err) if err?
 			return callback(err) if err?
 			CollaboratorsInviteHandler._trySendInviteNotification projectId, sendingUser, invite, (err)->
 			CollaboratorsInviteHandler._trySendInviteNotification projectId, sendingUser, invite, (err)->
 				return callback(err) if err?
 				return callback(err) if err?

+ 32 - 17
services/web/app/coffee/Features/Email/EmailSender.coffee

@@ -4,7 +4,7 @@ Settings = require('settings-sharelatex')
 nodemailer = require("nodemailer")
 nodemailer = require("nodemailer")
 sesTransport = require('nodemailer-ses-transport')
 sesTransport = require('nodemailer-ses-transport')
 sgTransport = require('nodemailer-sendgrid-transport')
 sgTransport = require('nodemailer-sendgrid-transport')
-
+rateLimiter = require('../../infrastructure/RateLimiter')
 _ = require("underscore")
 _ = require("underscore")
 
 
 if Settings.email? and Settings.email.fromAddress?
 if Settings.email? and Settings.email.fromAddress?
@@ -39,24 +39,39 @@ if nm_client?
 else
 else
 	logger.warn "Failed to create email transport. Please check your settings. No email will be sent."
 	logger.warn "Failed to create email transport. Please check your settings. No email will be sent."
 
 
+checkCanSendEmail = (options, callback)->
+	if !options.sendingUser_id? #email not sent from user, not rate limited
+		callback(null, true)
+	opts = 
+		endpointName: "send_email"
+		timeInterval: 60 * 60 * 3
+		subjectName: options.sendingUser_id
+		throttle: 100
+	rateLimiter.addCount opts, callback
 
 
 module.exports =
 module.exports =
 	sendEmail : (options, callback = (error) ->)->
 	sendEmail : (options, callback = (error) ->)->
 		logger.log receiver:options.to, subject:options.subject, "sending email"
 		logger.log receiver:options.to, subject:options.subject, "sending email"
-		metrics.inc "email"
-		options =
-			to: options.to
-			from: defaultFromAddress
-			subject: options.subject
-			html: options.html
-			text: options.text
-			replyTo: options.replyTo || Settings.email.replyToAddress
-			socketTimeout: 30 * 1000
-		if Settings.email.textEncoding?
-			opts.textEncoding = textEncoding
-		client.sendMail options, (err, res)->
+		checkCanSendEmail options, (err, canContinue)->
 			if err?
 			if err?
-				logger.err err:err, "error sending message"
-			else
-				logger.log "Message sent to #{options.to}"
-			callback(err)
+				return callback(err)
+			if !canContinue
+				logger.log sendingUser_id:options.sendingUser_id, to:options.to, subject:options.subject, canContinue:canContinue, "rate limit hit for sending email, not sending"
+				return callback("rate limit hit sending email")
+			metrics.inc "email"
+			options =
+				to: options.to
+				from: defaultFromAddress
+				subject: options.subject
+				html: options.html
+				text: options.text
+				replyTo: options.replyTo || Settings.email.replyToAddress
+				socketTimeout: 30 * 1000
+			if Settings.email.textEncoding?
+				opts.textEncoding = textEncoding
+			client.sendMail options, (err, res)->
+				if err?
+					logger.err err:err, "error sending message"
+				else
+					logger.log "Message sent to #{options.to}"
+				callback(err)

+ 2 - 2
services/web/test/UnitTests/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee

@@ -185,7 +185,7 @@ describe "CollaboratorsInviteHandler", ->
 	describe '_sendMessages', ->
 	describe '_sendMessages', ->
 
 
 		beforeEach ->
 		beforeEach ->
-			@CollaboratorsEmailHandler.notifyUserOfProjectInvite = sinon.stub().callsArgWith(3, null)
+			@CollaboratorsEmailHandler.notifyUserOfProjectInvite = sinon.stub().callsArgWith(4, null)
 			@CollaboratorsInviteHandler._trySendInviteNotification = sinon.stub().callsArgWith(3, null)
 			@CollaboratorsInviteHandler._trySendInviteNotification = sinon.stub().callsArgWith(3, null)
 			@call = (callback) =>
 			@call = (callback) =>
 				@CollaboratorsInviteHandler._sendMessages @projectId, @sendingUser, @fakeInvite, callback
 				@CollaboratorsInviteHandler._sendMessages @projectId, @sendingUser, @fakeInvite, callback
@@ -213,7 +213,7 @@ describe "CollaboratorsInviteHandler", ->
 		describe 'when CollaboratorsEmailHandler.notifyUserOfProjectInvite produces an error', ->
 		describe 'when CollaboratorsEmailHandler.notifyUserOfProjectInvite produces an error', ->
 
 
 			beforeEach ->
 			beforeEach ->
-				@CollaboratorsEmailHandler.notifyUserOfProjectInvite = sinon.stub().callsArgWith(3, new Error('woops'))
+				@CollaboratorsEmailHandler.notifyUserOfProjectInvite = sinon.stub().callsArgWith(4, new Error('woops'))
 
 
 			it 'should produce an error', (done) ->
 			it 'should produce an error', (done) ->
 				@call (err, invite) =>
 				@call (err, invite) =>

+ 30 - 0
services/web/test/UnitTests/coffee/Email/EmailSenderTests.coffee

@@ -10,6 +10,9 @@ describe "EmailSender", ->
 
 
 	beforeEach ->
 	beforeEach ->
 
 
+		@RateLimiter =
+			addCount:sinon.stub()
+
 		@settings =
 		@settings =
 			email:
 			email:
 				transport: "ses"
 				transport: "ses"
@@ -21,11 +24,15 @@ describe "EmailSender", ->
 
 
 		@sesClient =
 		@sesClient =
 			sendMail: sinon.stub()
 			sendMail: sinon.stub()
+
 		@ses =
 		@ses =
 			createTransport: => @sesClient
 			createTransport: => @sesClient
+
+
 		@sender = SandboxedModule.require modulePath, requires:
 		@sender = SandboxedModule.require modulePath, requires:
 			'nodemailer': @ses
 			'nodemailer': @ses
 			"settings-sharelatex":@settings
 			"settings-sharelatex":@settings
+			'../../infrastructure/RateLimiter':@RateLimiter
 			"logger-sharelatex":
 			"logger-sharelatex":
 				log:->
 				log:->
 				warn:->
 				warn:->
@@ -84,6 +91,29 @@ describe "EmailSender", ->
 				args.replyTo.should.equal @opts.replyTo
 				args.replyTo.should.equal @opts.replyTo
 				done()
 				done()
 
 
+
+		it "should not send an email when the rate limiter says no", (done)->
+			@opts.sendingUser_id = "12321312321"
+			@RateLimiter.addCount.callsArgWith(1, null, false)
+			@sender.sendEmail @opts, =>
+				@sesClient.sendMail.called.should.equal false
+				done()
+
+		it "should send the email when the rate limtier says continue",  (done)->
+			@sesClient.sendMail.callsArgWith(1)
+			@opts.sendingUser_id = "12321312321"
+			@RateLimiter.addCount.callsArgWith(1, null, true)
+			@sender.sendEmail @opts, =>
+				@sesClient.sendMail.called.should.equal true
+				done()
+
+		it "should not check the rate limiter when there is no sendingUser_id", (done)->
+			@sesClient.sendMail.callsArgWith(1)
+			@sender.sendEmail @opts, =>
+				@sesClient.sendMail.called.should.equal true
+				@RateLimiter.addCount.called.should.equal false
+				done()
+
 		describe 'with plain-text email content', () ->
 		describe 'with plain-text email content', () ->
 
 
 			beforeEach ->
 			beforeEach ->