Просмотр исходного кода

Send out confirmation emails on register and record confirmedAt date

James Allen 8 лет назад
Родитель
Сommit
0dcbc5facb

+ 2 - 2
services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee

@@ -14,7 +14,7 @@ module.exports =
 			if !user? or user.holdingAccount
 				logger.err email:email, "user could not be found for password reset"
 				return callback(null, false)
-			OneTimeTokenHandler.getNewToken user._id, (err, token)->
+			OneTimeTokenHandler.getNewToken 'password', user._id, (err, token)->
 				if err then return callback(err)
 				emailOptions =
 					to : email
@@ -24,7 +24,7 @@ module.exports =
 					callback null, true
 
 	setNewUserPassword: (token, password, callback = (error, found, user_id) ->)->
-		OneTimeTokenHandler.getValueFromTokenAndExpire token, (err, user_id)->
+		OneTimeTokenHandler.getValueFromTokenAndExpire 'password', token, (err, user_id)->
 			if err then return callback(err)
 			if !user_id?
 				return callback null, false, null

+ 9 - 9
services/web/app/coffee/Features/Security/OneTimeTokenHandler.coffee

@@ -6,29 +6,29 @@ logger = require("logger-sharelatex")
 
 ONE_HOUR_IN_S = 60 * 60
 
-buildKey = (token)-> return "password_token:#{token}"
+buildKey = (use, token)-> return "#{use}_token:#{token}"
 
 module.exports =
 
-	getNewToken: (value, options = {}, callback)->
+	getNewToken: (use, value, options = {}, callback)->
 		# options is optional
 		if typeof options == "function"
 			callback = options
 			options = {}
 		expiresIn = options.expiresIn or ONE_HOUR_IN_S
-		logger.log value:value, "generating token for password reset"
 		token = crypto.randomBytes(32).toString("hex")
+		logger.log {value, expiresIn, token_start: token.slice(0,8)}, "generating token for #{use}"
 		multi = rclient.multi()
-		multi.set buildKey(token), value
-		multi.expire buildKey(token), expiresIn
+		multi.set buildKey(use, token), value
+		multi.expire buildKey(use, token), expiresIn
 		multi.exec (err)->
 			callback(err, token)
 
-	getValueFromTokenAndExpire: (token, callback)->
-		logger.log token:token, "getting user id from password token"
+	getValueFromTokenAndExpire: (use, token, callback)->
+		logger.log token_start: token.slice(0,8), "getting value from #{use} token"
 		multi = rclient.multi()
-		multi.get buildKey(token)
-		multi.del buildKey(token)
+		multi.get buildKey(use, token)
+		multi.del buildKey(use, token)
 		multi.exec (err, results)->
 			callback err, results?[0]
 

+ 42 - 0
services/web/app/coffee/Features/User/UserEmailsConfirmationHandler.coffee

@@ -0,0 +1,42 @@
+EmailHelper = require "../Helpers/EmailHelper"
+EmailHandler = require "../Email/EmailHandler"
+OneTimeTokenHandler = require "../Security/OneTimeTokenHandler"
+settings = require 'settings-sharelatex'
+Errors = require "../Errors/Errors"
+logger = require "logger-sharelatex"
+UserUpdater = require "./UserUpdater"
+
+ONE_YEAR_IN_S = 365 * 24 * 60 * 60
+
+module.exports = UserEmailsConfirmationHandler =
+	serializeData: (user_id, email) ->
+		JSON.stringify({user_id, email})
+
+	deserializeData: (data) ->
+		JSON.parse(data)
+
+	sendConfirmationEmail: (user_id, email, emailTemplate, callback = (error) ->) ->
+		if arguments.length == 3
+			callback = emailTemplate
+			emailTemplate = 'confirmEmail'
+		email = EmailHelper.parseEmail(email)
+		return callback(new Error('invalid email')) if !email?
+		value = UserEmailsConfirmationHandler.serializeData(user_id, email)
+		OneTimeTokenHandler.getNewToken 'email_confirmation', value, {expiresIn: ONE_YEAR_IN_S}, (err, token)->
+			return callback(err) if err?
+			emailOptions =
+				to: email
+				confirmEmailUrl: "#{settings.siteUrl}/user/emails/confirm?token=#{token}"
+			EmailHandler.sendEmail emailTemplate, emailOptions, callback
+
+	confirmEmailFromToken: (token, callback = (error) ->) ->
+		logger.log {token_start: token.slice(0,8)}, 'confirming email from token'
+		OneTimeTokenHandler.getValueFromTokenAndExpire 'email_confirmation', token, (error, data) ->
+			return callback(error) if error?
+			if !data?
+				return callback(new Errors.NotFoundError('no token found'))
+			{user_id, email} = UserEmailsConfirmationHandler.deserializeData(data)
+			logger.log {data, user_id, email, token_start: token.slice(0,8)}, 'found data for email confirmation'
+			if !user_id? or email != EmailHelper.parseEmail(email)
+				return callback(new Errors.NotFoundError('invalid data'))
+			UserUpdater.confirmEmail user_id, email, callback

+ 34 - 9
services/web/app/coffee/Features/User/UserEmailsController.coffee

@@ -2,42 +2,67 @@ AuthenticationController = require('../Authentication/AuthenticationController')
 UserGetter = require("./UserGetter")
 UserUpdater = require("./UserUpdater")
 EmailHelper = require("../Helpers/EmailHelper")
+UserEmailsConfirmationHandler = require "./UserEmailsConfirmationHandler"
 logger = require("logger-sharelatex")
+Errors = require "../Errors/Errors"
 
 module.exports = UserEmailsController =
 
-	list: (req, res) ->
+	list: (req, res, next) ->
 		userId = AuthenticationController.getLoggedInUserId(req)
 		UserGetter.getUserFullEmails userId, (error, fullEmails) ->
-			return res.sendStatus 500 if error?
+			return next(error) if error?
 			res.json fullEmails
 
 
-	add: (req, res) ->
+	add: (req, res, next) ->
 		userId = AuthenticationController.getLoggedInUserId(req)
 		email = EmailHelper.parseEmail(req.body.email)
 		return res.sendStatus 422 unless email?
 
 		UserUpdater.addEmailAddress userId, email, (error)->
-			return res.sendStatus 500 if error?
-			res.sendStatus 200
+			return next(error) if error?
+			UserEmailsConfirmationHandler.sendConfirmationEmail userId, email, (err) ->
+				return next(error) if error?
+				res.sendStatus 204
 
 
-	remove: (req, res) ->
+	remove: (req, res, next) ->
 		userId = AuthenticationController.getLoggedInUserId(req)
 		email = EmailHelper.parseEmail(req.body.email)
 		return res.sendStatus 422 unless email?
 
 		UserUpdater.removeEmailAddress userId, email, (error)->
-			return res.sendStatus 500 if error?
+			return next(error) if error?
 			res.sendStatus 200
 
 
-	setDefault: (req, res) ->
+	setDefault: (req, res, next) ->
 		userId = AuthenticationController.getLoggedInUserId(req)
 		email = EmailHelper.parseEmail(req.body.email)
 		return res.sendStatus 422 unless email?
 
 		UserUpdater.setDefaultEmailAddress userId, email, (error)->
-			return res.sendStatus 500 if error?
+			return next(error) if error?
 			res.sendStatus 200
+
+	showConfirm: (req, res, next) ->
+		res.render 'user/confirm_email', {
+			token: req.query.token,
+			title: 'confirm_email'
+		}
+
+	confirm: (req, res, next) ->
+		token = req.body.token
+		if !token?
+			return res.sendStatus 422
+		UserEmailsConfirmationHandler.confirmEmailFromToken token, (error) ->
+			if error?
+				if error instanceof Errors.NotFoundError
+					res.status(404).json({
+						message: 'Sorry, your confirmation token is invalid or has expired. Please request a new email confirmation link.'
+					})
+				else
+					next(error)
+			else
+				res.sendStatus 200

+ 1 - 1
services/web/app/coffee/Features/User/UserRegistrationHandler.coffee

@@ -74,7 +74,7 @@ module.exports = UserRegistrationHandler =
 				logger.log {email}, "user already exists, resending welcome email"
 
 			ONE_WEEK = 7 * 24 * 60 * 60 # seconds
-			OneTimeTokenHandler.getNewToken user._id, { expiresIn: ONE_WEEK }, (err, token)->
+			OneTimeTokenHandler.getNewToken 'password', user._id, { expiresIn: ONE_WEEK }, (err, token)->
 				return callback(err) if err?
 				
 				setNewPasswordUrl = "#{settings.siteUrl}/user/activate?token=#{token}&user_id=#{user._id}"

+ 22 - 3
services/web/app/coffee/Features/User/UserUpdater.coffee

@@ -5,6 +5,8 @@ db = mongojs.db
 async = require("async")
 ObjectId = mongojs.ObjectId
 UserGetter = require("./UserGetter")
+EmailHelper = require "../Helpers/EmailHelper"
+Errors = require "../Errors/Errors"
 
 module.exports = UserUpdater =
 	updateUser: (query, update, callback = (error) ->) ->
@@ -53,7 +55,6 @@ module.exports = UserUpdater =
 					return callback(error)
 				callback()
 
-
 	# remove one of the user's email addresses. The email cannot be the user's
 	# default email address
 	removeEmailAddress: (userId, email, callback) ->
@@ -63,7 +64,7 @@ module.exports = UserUpdater =
 			if error?
 				logger.err error:error, 'problem removing users email'
 				return callback(error)
-			if res.nMatched == 0
+			if res.n == 0
 				return callback(new Error('Cannot remove default email'))
 			callback()
 
@@ -77,10 +78,28 @@ module.exports = UserUpdater =
 			if error?
 				logger.err error:error, 'problem setting default emails'
 				return callback(error)
-			if res.nMatched == 0
+			if res.n == 0 # TODO: Check n or nMatched?
 				return callback(new Error('Default email does not belong to user'))
 			callback()
 
+	confirmEmail: (userId, email, callback) ->
+		email = EmailHelper.parseEmail(email)
+		return callback(new Error('invalid email')) if !email?
+		logger.log {userId, email}, 'confirming user email'
+		query =
+			_id: userId
+			'emails.email': email
+		update =
+			$set:
+				'emails.$.confirmedAt': new Date()
+		@updateUser query, update, (error, res) ->
+			return callback(error) if error?
+			logger.log {res, userId, email}, "tried to confirm email"
+			if res.n == 0
+				return callback(new Errors.NotFoundError('user id and email do no match'))
+			callback()
+
+
 [
 	'updateUser'
 	'changeEmailAddress'

+ 2 - 1
services/web/app/coffee/models/User.coffee

@@ -10,7 +10,8 @@ UserSchema = new Schema
 	email             : {type : String, default : ''}
 	emails: [{
 		email: { type : String, default : '' },
-		createdAt: { type : Date, default: () -> new Date() }
+		createdAt: { type : Date, default: () -> new Date() },
+		confirmedAt: { type: Date }
 	}],
 	first_name        : {type : String, default : ''}
 	last_name         : {type : String, default : ''}

+ 4 - 0
services/web/app/coffee/router.coffee

@@ -120,6 +120,10 @@ module.exports = class Router
 		webRouter.post '/user/emails/default',
 			AuthenticationController.requireLogin(),
 			UserEmailsController.setDefault
+		webRouter.get '/user/emails/confirm',
+			UserEmailsController.showConfirm
+		webRouter.post '/user/emails/confirm',
+			UserEmailsController.confirm
 
 		webRouter.get  '/user/sessions',
 			AuthenticationController.requireLogin(),

+ 28 - 0
services/web/app/views/user/confirm_email.pug

@@ -0,0 +1,28 @@
+extends ../layout
+
+block content
+	.content.content-alt
+		.container
+			.row
+				.col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4
+					.card
+						.page-header
+							h1 #{translate("confirm_email")}
+						form(
+							async-form="confirm-email",
+							name="confirmEmailForm"
+							action="/user/emails/confirm",
+							method="POST",
+							id="confirmEmailForm",
+							auto-submit="true",
+							ng-cloak
+						)
+							input(type="hidden", name="_csrf", value=csrfToken)
+							input(type="hidden", name="token", value=token)
+							form-messages(for="confirmEmailForm")
+								.alert.alert-success(ng-show="confirmEmailForm.response.success")
+									| Thank you, your email is now confirmed
+							p.text-center(ng-show="!confirmEmailForm.response.success && !confirmEmailForm.response.error")
+								i.fa.fa-fw.fa-spin.fa-spinner
+								| 
+								| Confirming your email...

+ 11 - 5
services/web/public/coffee/directives/asyncForm.coffee

@@ -15,11 +15,6 @@ define [
 				scope[attrs.name].response = response = {}
 				scope[attrs.name].inflight = false
 
-				element.on "submit", (e) ->
-					e.preventDefault()
-					validateCaptchaIfEnabled (response) ->
-						submitRequest response
-
 				validateCaptchaIfEnabled = (callback = (response) ->) ->
 					if attrs.captcha?
 						validateCaptcha callback
@@ -84,6 +79,17 @@ define [
 									text: data.message?.text or data.message or "Something went wrong talking to the server :(. Please try again."
 									type: 'error'
 							ga('send', 'event', formName, 'failure', data.message)
+
+				submit = () ->
+					validateCaptchaIfEnabled (response) ->
+						submitRequest response
+
+				element.on "submit", (e) ->
+					e.preventDefault()
+					submit()
+
+				if attrs.autoSubmit
+					submit()
 		}
 
 	App.directive "formMessages", () ->

+ 132 - 0
services/web/test/acceptance/coffee/UserEmailsTests.coffee

@@ -0,0 +1,132 @@
+expect = require("chai").expect
+async = require("async")
+User = require "./helpers/User"
+request = require "./helpers/request"
+settings = require "settings-sharelatex"
+rclient = require("redis-sharelatex").createClient(settings.redis.web)
+
+describe "UserEmails", ->
+	beforeEach (done) ->
+		@timeout(20000)
+		@user = new User()
+		@user.login done
+
+	describe 'confirming an email', ->
+		it 'should confirm the email', (done) ->
+			token = null
+			async.series [
+				(cb) =>
+					@user.request { 
+						method: 'POST',
+						url: '/user/emails',
+						json:
+							email: 'newly-added-email@example.com'
+					}, (error, response, body) =>
+						return done(error) if error?
+						expect(response.statusCode).to.equal 204
+						cb()
+				(cb) =>
+					@user.request { url: '/user/emails', json: true }, (error, response, body) ->
+						expect(response.statusCode).to.equal 200
+						expect(body[0].confirmedAt).to.not.exist
+						expect(body[1].confirmedAt).to.not.exist
+						cb()
+				(cb) =>
+					rclient.keys 'email_confirmation_token:*', (error, keys) ->
+						# There should only be one confirmation token at the moment
+						expect(keys.length).to.equal 1
+						token = keys[0].split(':')[1]
+						cb()
+				(cb) =>
+					@user.request { 
+						method: 'POST',
+						url: '/user/emails/confirm',
+						json:
+							token: token
+					}, (error, response, body) =>
+						return done(error) if error?
+						expect(response.statusCode).to.equal 200
+						cb()
+				(cb) =>
+					@user.request { url: '/user/emails', json: true }, (error, response, body) ->
+						expect(response.statusCode).to.equal 200
+						expect(body[0].confirmedAt).to.not.exist
+						expect(body[1].confirmedAt).to.exist
+						cb()
+				(cb) =>
+					rclient.keys 'email_confirmation_token:*', (error, keys) ->
+						# Token should be deleted after use
+						expect(keys.length).to.equal 0
+						cb()
+			], done
+
+		it 'should not allow confirmation of the email if the user has changed', (done) ->
+			token1 = null
+			token2 = null
+			@user2 = new User()
+			@email = 'duplicate-email@example.com'
+			async.series [
+				(cb) => @user2.login cb
+				(cb) =>
+					# Create email for first user
+					@user.request { 
+						method: 'POST',
+						url: '/user/emails',
+						json: {@email}
+					}, cb
+				(cb) =>
+					rclient.keys 'email_confirmation_token:*', (error, keys) ->
+						# There should only be one confirmation token at the moment,
+						# for the first user
+						expect(keys.length).to.equal 1
+						token1 = keys[0].split(':')[1]
+						cb()
+				(cb) =>
+					# Delete the email from the first user
+					@user.request { 
+						method: 'DELETE',
+						url: '/user/emails',
+						json: {@email}
+					}, cb
+				(cb) =>
+					# Create email for second user
+					@user2.request { 
+						method: 'POST',
+						url: '/user/emails',
+						json: {@email}
+					}, cb
+				(cb) =>
+					# Original confirmation token should no longer work
+					@user.request { 
+						method: 'POST',
+						url: '/user/emails/confirm',
+						json:
+							token: token1
+					}, (error, response, body) =>
+						return done(error) if error?
+						expect(response.statusCode).to.equal 404
+						cb()
+				(cb) =>
+					rclient.keys 'email_confirmation_token:*', (error, keys) ->
+						# The first token has been used, so this should be token2 now
+						expect(keys.length).to.equal 1
+						token2 = keys[0].split(':')[1]
+						cb()
+				(cb) =>
+					# Second user should be able to confirm the email
+					@user2.request { 
+						method: 'POST',
+						url: '/user/emails/confirm',
+						json:
+							token: token2
+					}, (error, response, body) =>
+						return done(error) if error?
+						expect(response.statusCode).to.equal 200
+						cb()
+				(cb) =>
+					@user2.request { url: '/user/emails', json: true }, (error, response, body) ->
+						expect(response.statusCode).to.equal 200
+						expect(body[0].confirmedAt).to.not.exist
+						expect(body[1].confirmedAt).to.exist
+						cb()
+			], done

+ 5 - 5
services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee

@@ -41,7 +41,7 @@ describe "PasswordResetHandler", ->
 
 		it "should check the user exists", (done)->
 			@UserGetter.getUserByMainEmail.callsArgWith(1)
-			@OneTimeTokenHandler.getNewToken.callsArgWith(1)
+			@OneTimeTokenHandler.getNewToken.yields()
 			@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
 				exists.should.equal false
 				done()
@@ -50,7 +50,7 @@ describe "PasswordResetHandler", ->
 		it "should send the email with the token", (done)->
 
 			@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
-			@OneTimeTokenHandler.getNewToken.callsArgWith(1, null, @token)
+			@OneTimeTokenHandler.getNewToken.yields(null, @token)
 			@EmailHandler.sendEmail.callsArgWith(2)
 			@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
 				@EmailHandler.sendEmail.called.should.equal true
@@ -63,7 +63,7 @@ describe "PasswordResetHandler", ->
 		it "should return exists = false for a holdingAccount", (done) ->
 			@user.holdingAccount = true
 			@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
-			@OneTimeTokenHandler.getNewToken.callsArgWith(1)
+			@OneTimeTokenHandler.getNewToken.yields()
 			@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
 				exists.should.equal false
 				done()
@@ -71,14 +71,14 @@ describe "PasswordResetHandler", ->
 	describe "setNewUserPassword", ->
 
 		it "should return false if no user id can be found", (done)->
-			@OneTimeTokenHandler.getValueFromTokenAndExpire.callsArgWith(1)
+			@OneTimeTokenHandler.getValueFromTokenAndExpire.yields()
 			@PasswordResetHandler.setNewUserPassword @token, @password, (err, found) =>
 				found.should.equal false
 				@AuthenticationManager.setUserPassword.called.should.equal false
 				done()		
 
 		it "should set the user password", (done)->
-			@OneTimeTokenHandler.getValueFromTokenAndExpire.callsArgWith(1, null, @user_id)
+			@OneTimeTokenHandler.getValueFromTokenAndExpire.yields(null, @user_id)
 			@AuthenticationManager.setUserPassword.callsArgWith(2)
 			@PasswordResetHandler.setNewUserPassword @token, @password, (err, found, user_id) =>
 				found.should.equal true

+ 4 - 4
services/web/test/unit/coffee/Security/OneTimeTokenHandlerTests.coffee

@@ -37,21 +37,21 @@ describe "OneTimeTokenHandler", ->
 
 		it "should set a new token into redis with a ttl", (done)->
 			@redisMulti.exec.callsArgWith(0) 
-			@OneTimeTokenHandler.getNewToken @value, (err, token) =>
+			@OneTimeTokenHandler.getNewToken 'password', @value, (err, token) =>
 				@redisMulti.set.calledWith("password_token:#{@stubbedToken.toString("hex")}", @value).should.equal true
 				@redisMulti.expire.calledWith("password_token:#{@stubbedToken.toString("hex")}", 60 * 60).should.equal true
 				done()
 
 		it "should return if there was an error", (done)->
 			@redisMulti.exec.callsArgWith(0, "error")
-			@OneTimeTokenHandler.getNewToken @value, (err, token)=>
+			@OneTimeTokenHandler.getNewToken 'password', @value, (err, token)=>
 				err.should.exist
 				done()
 
 		it "should allow the expiry time to be overridden", (done) ->
 			@redisMulti.exec.callsArgWith(0) 
 			@ttl = 42
-			@OneTimeTokenHandler.getNewToken @value, {expiresIn: @ttl}, (err, token) =>
+			@OneTimeTokenHandler.getNewToken 'password', @value, {expiresIn: @ttl}, (err, token) =>
 				@redisMulti.expire.calledWith("password_token:#{@stubbedToken.toString("hex")}", @ttl).should.equal true
 				done()
 
@@ -59,7 +59,7 @@ describe "OneTimeTokenHandler", ->
 
 		it "should get and delete the token", (done)->
 			@redisMulti.exec.callsArgWith(0, null, [@value]) 
-			@OneTimeTokenHandler.getValueFromTokenAndExpire @stubbedToken, (err, value)=>
+			@OneTimeTokenHandler.getValueFromTokenAndExpire 'password', @stubbedToken, (err, value)=>
 				value.should.equal @value
 				@redisMulti.get.calledWith("password_token:#{@stubbedToken}").should.equal true
 				@redisMulti.del.calledWith("password_token:#{@stubbedToken}").should.equal true

+ 125 - 0
services/web/test/unit/coffee/User/UserEmailsConfirmationHandlerTests.coffee

@@ -0,0 +1,125 @@
+should = require('chai').should()
+SandboxedModule = require('sandboxed-module')
+assert = require('assert')
+path = require('path')
+sinon = require('sinon')
+modulePath = path.join __dirname, "../../../../app/js/Features/User/UserEmailsConfirmationHandler"
+expect = require("chai").expect
+Errors = require "../../../../app/js/Features/Errors/Errors"
+EmailHelper = require "../../../../app/js/Features/Helpers/EmailHelper"
+
+describe "UserEmailsConfirmationHandler", ->
+	beforeEach ->
+		@UserEmailsConfirmationHandler = SandboxedModule.require modulePath, requires:
+			"settings-sharelatex": @settings =
+				siteUrl: "emails.example.com"
+			"logger-sharelatex": @logger = { log: sinon.stub() }
+			"../Security/OneTimeTokenHandler": @OneTimeTokenHandler = {}
+			"../Errors/Errors": Errors
+			"./UserUpdater": @UserUpdater = {}
+			"../Email/EmailHandler": @EmailHandler = {}
+			"../Helpers/EmailHelper": EmailHelper
+		@user_id = "mock-user-id"
+		@email = "mock@example.com"
+		@callback = sinon.stub()
+
+	describe "sendConfirmationEmail", ->
+		beforeEach ->
+			@OneTimeTokenHandler.getNewToken = sinon.stub().yields(null, @token = "new-token")
+			@EmailHandler.sendEmail = sinon.stub().yields()
+
+		describe 'successfully', ->
+			beforeEach ->
+				@UserEmailsConfirmationHandler.sendConfirmationEmail @user_id, @email, @callback
+
+			it "should generate a token for the user which references their id and email", ->
+				@OneTimeTokenHandler.getNewToken
+					.calledWith(
+						'email_confirmation',
+						JSON.stringify({@user_id, @email}),
+						{ expiresIn: 365 * 24 * 60 * 60 }
+					)
+					.should.equal true
+
+			it 'should send an email to the user', ->
+				@EmailHandler.sendEmail
+					.calledWith('confirmEmail', {
+						to: @email,
+						confirmEmailUrl: 'emails.example.com/user/emails/confirm?token=new-token'
+					})
+					.should.equal true
+
+			it 'should call the callback', ->
+				@callback.called.should.equal true
+
+		describe 'with invalid email', ->
+			beforeEach ->
+				@UserEmailsConfirmationHandler.sendConfirmationEmail @user_id, '!"£$%^&*()', @callback
+
+			it 'should return an error', ->
+				@callback.calledWith(sinon.match.instanceOf(Error)).should.equal true
+
+		describe 'a custom template', ->
+			beforeEach ->
+				@UserEmailsConfirmationHandler.sendConfirmationEmail @user_id, @email, 'myCustomTemplate', @callback
+
+			it 'should send an email with the given template', ->
+				@EmailHandler.sendEmail
+					.calledWith('myCustomTemplate')
+					.should.equal true
+
+	describe "confirmEmailFromToken", ->
+		beforeEach ->
+			@OneTimeTokenHandler.getValueFromTokenAndExpire = sinon.stub().yields(
+				null, 
+				JSON.stringify({@user_id, @email})
+			)
+			@UserUpdater.confirmEmail = sinon.stub().yields()
+
+		describe "successfully", ->
+			beforeEach ->
+				@UserEmailsConfirmationHandler.confirmEmailFromToken @token = 'mock-token', @callback
+
+			it "should call getValueFromTokenAndExpire", ->
+				@OneTimeTokenHandler.getValueFromTokenAndExpire
+					.calledWith('email_confirmation', @token)
+					.should.equal true
+
+			it "should confirm the email of the user_id", ->
+				@UserUpdater.confirmEmail
+					.calledWith(@user_id, @email)
+					.should.equal true
+
+			it "should call the callback", ->
+				@callback.called.should.equal true
+
+		describe 'with an expired token', ->
+			beforeEach ->
+				@OneTimeTokenHandler.getValueFromTokenAndExpire = sinon.stub().yields(null, null)
+				@UserEmailsConfirmationHandler.confirmEmailFromToken @token = 'mock-token', @callback
+
+			it "should call the callback with a NotFoundError", ->
+				@callback.calledWith(sinon.match.instanceOf(Errors.NotFoundError)).should.equal true
+
+		describe 'with no user_id in the token', ->
+			beforeEach ->
+				@OneTimeTokenHandler.getValueFromTokenAndExpire = sinon.stub().yields(
+					null, 
+					JSON.stringify({@email})
+				)
+				@UserEmailsConfirmationHandler.confirmEmailFromToken @token = 'mock-token', @callback
+
+			it "should call the callback with a NotFoundError", ->
+				@callback.calledWith(sinon.match.instanceOf(Errors.NotFoundError)).should.equal true
+
+		describe 'with no email in the token', ->
+			beforeEach ->
+				@OneTimeTokenHandler.getValueFromTokenAndExpire = sinon.stub().yields(
+					null, 
+					JSON.stringify({@user_id})
+				)
+				@UserEmailsConfirmationHandler.confirmEmailFromToken @token = 'mock-token', @callback
+
+			it "should call the callback with a NotFoundError", ->
+				@callback.calledWith(sinon.match.instanceOf(Errors.NotFoundError)).should.equal true
+

+ 58 - 34
services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee

@@ -8,6 +8,7 @@ modulePath = "../../../../app/js/Features/User/UserEmailsController.js"
 SandboxedModule = require('sandboxed-module')
 MockRequest = require "../helpers/MockRequest"
 MockResponse = require "../helpers/MockResponse"
+Errors = require("../../../../app/js/Features/Errors/Errors")
 
 describe "UserEmailsController", ->
 	beforeEach ->
@@ -30,6 +31,8 @@ describe "UserEmailsController", ->
 			"./UserGetter": @UserGetter
 			"./UserUpdater": @UserUpdater
 			"../Helpers/EmailHelper": @EmailHelper
+			"./UserEmailsConfirmationHandler": @UserEmailsConfirmationHandler = {}
+			"../Errors/Errors": Errors
 			"logger-sharelatex":
 				log: -> console.log(arguments)
 				err: ->
@@ -47,45 +50,36 @@ describe "UserEmailsController", ->
 					assertCalledWith @UserGetter.getUserFullEmails, @user._id
 					done()
 
-		it 'handles error', (done) ->
-			@UserGetter.getUserFullEmails.callsArgWith 1, new Error('Oups')
-
-			@UserEmailsController.list @req, 
-				sendStatus: (code) =>
-					code.should.equal 500
-					done()
-
 	describe 'Add', ->
 		beforeEach ->
 			@newEmail = 'new_email@baz.com'
 			@req.body.email = @newEmail
 			@EmailHelper.parseEmail.returns @newEmail
-
-		it 'adds new email', (done) ->
 			@UserUpdater.addEmailAddress.callsArgWith 2, null
+			@UserEmailsConfirmationHandler.sendConfirmationEmail = sinon.stub().yields()
 
+		it 'adds new email', (done) ->
 			@UserEmailsController.add @req, 
 				sendStatus: (code) =>
-					code.should.equal 200
+					code.should.equal 204
 					assertCalledWith @EmailHelper.parseEmail, @newEmail
 					assertCalledWith @UserUpdater.addEmailAddress, @user._id, @newEmail
 					done()
 
-		it 'handles email parse error', (done) ->
-			@EmailHelper.parseEmail.returns null
-
+		it 'sends an email confirmation', (done) ->
 			@UserEmailsController.add @req, 
 				sendStatus: (code) =>
-					code.should.equal 422
-					assertNotCalled @UserUpdater.addEmailAddress
+					code.should.equal 204
+					assertCalledWith @UserEmailsConfirmationHandler.sendConfirmationEmail, @user._id, @newEmail
 					done()
 
-		it 'handles error', (done) ->
-			@UserUpdater.addEmailAddress.callsArgWith 2, new Error('Oups')
+		it 'handles email parse error', (done) ->
+			@EmailHelper.parseEmail.returns null
 
 			@UserEmailsController.add @req, 
 				sendStatus: (code) =>
-					code.should.equal 500
+					code.should.equal 422
+					assertNotCalled @UserUpdater.addEmailAddress
 					done()
 
 	describe 'remove', ->
@@ -113,15 +107,6 @@ describe "UserEmailsController", ->
 					assertNotCalled @UserUpdater.removeEmailAddress
 					done()
 
-		it 'handles error', (done) ->
-			@UserUpdater.removeEmailAddress.callsArgWith 2, new Error('Oups')
-
-			@UserEmailsController.remove @req, 
-				sendStatus: (code) =>
-					code.should.equal 500
-					done()
-
-
 	describe 'setDefault', ->
 		beforeEach ->
 			@email = "email_to_set_default@bar.com"
@@ -147,11 +132,50 @@ describe "UserEmailsController", ->
 					assertNotCalled @UserUpdater.setDefaultEmailAddress
 					done()
 
-		it 'handles error', (done) ->
-			@UserUpdater.setDefaultEmailAddress.callsArgWith 2, new Error('Oups')
+	describe 'confirm', ->
+		beforeEach ->
+			@UserEmailsConfirmationHandler.confirmEmailFromToken = sinon.stub().yields()
+			@res =
+				sendStatus: sinon.stub()
+				json: sinon.stub()
+			@res.status = sinon.stub().returns(@res)
+			@next = sinon.stub()
+			@token = 'mock-token'
+			@req.body = token: @token
+
+		describe 'successfully', ->
+			beforeEach ->
+				@UserEmailsController.confirm @req, @res, @next
+
+			it 'should confirm the email from the token', ->
+				@UserEmailsConfirmationHandler.confirmEmailFromToken
+					.calledWith(@token)
+					.should.equal true
+
+			it 'should return a 200 status', ->
+				@res.sendStatus.calledWith(200).should.equal true
+
+		describe 'without a token', ->
+			beforeEach ->
+				@req.body.token = null
+				@UserEmailsController.confirm @req, @res, @next
+
+			it 'should return a 422 status', ->
+				@res.sendStatus.calledWith(422).should.equal true
+
+		describe 'when confirming fails', ->
+			beforeEach ->
+				@UserEmailsConfirmationHandler.confirmEmailFromToken = sinon.stub().yields(
+					new Errors.NotFoundError('not found')
+				)
+				@UserEmailsController.confirm @req, @res, @next
+
+			it 'should return a 404 error code with a message', ->
+				@res.status.calledWith(404).should.equal true
+				@res.json.calledWith({
+					message: 'Sorry, your confirmation token is invalid or has expired. Please request a new email confirmation link.'
+				}).should.equal true
+
+
 
-			@UserEmailsController.setDefault @req, 
-				sendStatus: (code) =>
-					code.should.equal 500
-					done()
 

+ 2 - 2
services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee

@@ -152,7 +152,7 @@ describe "UserRegistrationHandler", ->
 		beforeEach ->
 			@email = "email@example.com"
 			@crypto.randomBytes = sinon.stub().returns({toString: () => @password = "mock-password"})
-			@OneTimeTokenHandler.getNewToken.callsArgWith(2, null, @token = "mock-token")
+			@OneTimeTokenHandler.getNewToken.yields(null, @token = "mock-token")
 			@handler.registerNewUser = sinon.stub()
 			@callback = sinon.stub()
 		
@@ -171,7 +171,7 @@ describe "UserRegistrationHandler", ->
 			it "should generate a new password reset token", ->
 				
 				@OneTimeTokenHandler.getNewToken
-					.calledWith(@user_id, expiresIn: 7 * 24 * 60 * 60)
+					.calledWith('password', @user_id, expiresIn: 7 * 24 * 60 * 60)
 					.should.equal true
 
 			it "should send a registered email", ->

+ 35 - 4
services/web/test/unit/coffee/User/UserUpdaterTests.coffee

@@ -5,11 +5,12 @@ path = require('path')
 sinon = require('sinon')
 modulePath = path.join __dirname, "../../../../app/js/Features/User/UserUpdater"
 expect = require("chai").expect
+tk = require('timekeeper')
 
 describe "UserUpdater", ->
 
 	beforeEach ->
-
+		tk.freeze(Date.now())
 		@settings = {}
 		@mongojs = 
 			db:{}
@@ -32,6 +33,9 @@ describe "UserUpdater", ->
 			email:"hello@world.com"
 		@newEmail = "bob@bob.com"
 
+	afterEach ->
+		tk.reset()
+
 	describe 'changeEmailAddress', ->
 		beforeEach ->
 			@UserGetter.getUserEmail.callsArgWith(1, null, @stubbedUser.email)
@@ -103,7 +107,7 @@ describe "UserUpdater", ->
 				done()
 
 		it 'handle missed update', (done)->
-			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, nMatched: 0)
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, n: 0)
 
 			@UserUpdater.removeEmailAddress @stubbedUser._id, @newEmail, (err)=>
 				should.exist(err)
@@ -111,7 +115,7 @@ describe "UserUpdater", ->
 
 	describe 'setDefaultEmailAddress', ->
 		it 'set default', (done)->
-			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, nMatched: 1)
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, n: 1)
 
 			@UserUpdater.setDefaultEmailAddress @stubbedUser._id, @newEmail, (err)=>
 				should.not.exist(err)
@@ -129,10 +133,37 @@ describe "UserUpdater", ->
 				done()
 
 		it 'handle missed update', (done)->
-			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, nMatched: 0)
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, n: 0)
 
 			@UserUpdater.setDefaultEmailAddress @stubbedUser._id, @newEmail, (err)=>
 				should.exist(err)
 				done()
 
+	describe 'confirmEmail', ->
+		it 'should update the email record', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, n: 1)
+
+			@UserUpdater.confirmEmail @stubbedUser._id, @newEmail, (err)=>
+				should.not.exist(err)
+				@UserUpdater.updateUser.calledWith(
+					{ _id: @stubbedUser._id, 'emails.email': @newEmail },
+					$set: { 'emails.$.confirmedAt': new Date() }
+				).should.equal true
+				done()
+
+		it 'handle error', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, new Error('nope'))
+
+			@UserUpdater.confirmEmail @stubbedUser._id, @newEmail, (err)=>
+				should.exist(err)
+				done()
+
+		it 'handle missed update', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, n: 0)
+
+			@UserUpdater.confirmEmail @stubbedUser._id, @newEmail, (err)=>
+				should.exist(err)
+				done()
+
+