Преглед изворни кода

Merge pull request #672 from sharelatex/ja-refactor-email-templating

Refactor email templating
James Allen пре 8 година
родитељ
комит
46fbf0c641
21 измењених фајлова са 791 додато и 295 уклоњено
  1. 10 6
      services/web/app/coffee/Features/Email/Bodies/SingleCTAEmailBody.coffee
  2. 10 6
      services/web/app/coffee/Features/Email/Bodies/ol-SingleCTAEmailBody.coffee
  3. 96 152
      services/web/app/coffee/Features/Email/EmailBuilder.coffee
  4. 2 2
      services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee
  5. 35 19
      services/web/app/coffee/Features/Security/OneTimeTokenHandler.coffee
  6. 36 0
      services/web/app/coffee/Features/User/UserEmailsConfirmationHandler.coffee
  7. 34 9
      services/web/app/coffee/Features/User/UserEmailsController.coffee
  8. 1 1
      services/web/app/coffee/Features/User/UserRegistrationHandler.coffee
  9. 22 3
      services/web/app/coffee/Features/User/UserUpdater.coffee
  10. 1 1
      services/web/app/coffee/infrastructure/mongojs.coffee
  11. 2 1
      services/web/app/coffee/models/User.coffee
  12. 4 0
      services/web/app/coffee/router.coffee
  13. 28 0
      services/web/app/views/user/confirm_email.pug
  14. 11 5
      services/web/public/coffee/directives/asyncForm.coffee
  15. 194 0
      services/web/test/acceptance/coffee/UserEmailsTests.coffee
  16. 5 5
      services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee
  17. 80 45
      services/web/test/unit/coffee/Security/OneTimeTokenHandlerTests.coffee
  18. 125 0
      services/web/test/unit/coffee/User/UserEmailsConfirmationHandlerTests.coffee
  19. 58 34
      services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee
  20. 2 2
      services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee
  21. 35 4
      services/web/test/unit/coffee/User/UserUpdaterTests.coffee

+ 10 - 6
services/web/app/coffee/Features/Email/Bodies/SingleCTAEmailBody.coffee

@@ -4,13 +4,17 @@ settings = require "settings-sharelatex"
 module.exports = _.template """
 <table class="row" style="border-collapse: collapse; border-spacing: 0; display: table; padding: 0; position: relative; text-align: left; vertical-align: top; width: 100%;"><tbody><tr style="padding: 0; text-align: left; vertical-align: top;">
 	<th class="small-12 large-12 columns first last" style="Margin: 0 auto; color: #0a0a0a; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0 auto; padding: 0; padding-bottom: 16px; padding-left: 16px; padding-right: 16px; text-align: left; width: 564px;"><table style="border-collapse: collapse; border-spacing: 0; padding: 0; text-align: left; vertical-align: top; width: 100%;"><tr style="padding: 0; text-align: left; vertical-align: top;"><th style="Margin: 0; color: #0a0a0a; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0; padding: 0; text-align: left;">
-		<h3 class="avoid-auto-linking" style="Margin: 0; color: inherit; font-family: Baskerville, 'Baskerville Old Face', Georgia, serif; font-size: 24px; font-weight: normal; line-height: 1.3; margin: 0; padding: 0; text-align: left; word-wrap: normal;">
-			<%= title %>
-		</h3>
+		<% if (title) { %>
+			<h3 class="avoid-auto-linking" style="Margin: 0; color: inherit; font-family: Baskerville, 'Baskerville Old Face', Georgia, serif; font-size: 24px; font-weight: normal; line-height: 1.3; margin: 0; padding: 0; text-align: left; word-wrap: normal;">
+				<%= title %>
+			</h3>
+		<% } %>
 		<table class="spacer" style="border-collapse: collapse; border-spacing: 0; padding: 0; text-align: left; vertical-align: top; width: 100%;"><tbody><tr style="padding: 0; text-align: left; vertical-align: top;"><td height="20px" style="-moz-hyphens: auto; -webkit-hyphens: auto; Margin: 0; border-collapse: collapse !important; color: #0a0a0a; font-family: Helvetica, Arial, sans-serif; font-size: 20px; font-weight: normal; hyphens: auto; line-height: 20px; margin: 0; mso-line-height-rule: exactly; padding: 0; text-align: left; vertical-align: top; word-wrap: break-word;">&#xA0;</td></tr></tbody></table> 
-		<p style="Margin: 0; Margin-bottom: 10px; color: #0a0a0a; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0; margin-bottom: 10px; padding: 0; text-align: left;">
-			<%= greeting %>
-		</p>
+		<% if (greeting) { %>
+			<p style="Margin: 0; Margin-bottom: 10px; color: #0a0a0a; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0; margin-bottom: 10px; padding: 0; text-align: left;">
+				<%= greeting %>
+			</p>
+		<% } %>
 		<p class="avoid-auto-linking" style="Margin: 0; Margin-bottom: 10px; color: #0a0a0a; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0; margin-bottom: 10px; padding: 0; text-align: left;">
 			<%= message %>
 		</p>

+ 10 - 6
services/web/app/coffee/Features/Email/Bodies/ol-SingleCTAEmailBody.coffee

@@ -4,13 +4,17 @@ settings = require "settings-sharelatex"
 module.exports = _.template """
 <table class="row" style="border-collapse: collapse; border-spacing: 0; display: table; padding: 0; position: relative; text-align: left; vertical-align: top; width: 100%;"><tbody><tr style="padding: 0; text-align: left; vertical-align: top;">
 	<th class="small-12 large-12 columns first last" style="Margin: 0 auto; color: #5D6879; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0 auto; padding: 0; padding-bottom: 16px; padding-left: 16px; padding-right: 16px; text-align: left; width: 564px;"><table style="border-collapse: collapse; border-spacing: 0; padding: 0; text-align: left; vertical-align: top; width: 100%;"><tr style="padding: 0; text-align: left; vertical-align: top;"><th style="Margin: 0; color: #5D6879; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0; padding: 0; text-align: left;">
-		<h3 class="avoid-auto-linking" style="Margin: 0; color: #5D6879; font-family: Georgia, serif; font-size: 24px; font-weight: normal; line-height: 1.3; margin: 0; padding: 0; text-align: left; word-wrap: normal;">
-			<%= title %>
-		</h3>
+		<% if (title) { %>
+			<h3 class="avoid-auto-linking" style="Margin: 0; color: #5D6879; font-family: Georgia, serif; font-size: 24px; font-weight: normal; line-height: 1.3; margin: 0; padding: 0; text-align: left; word-wrap: normal;">
+				<%= title %>
+			</h3>
+		<% } %>
 		<table class="spacer" style="border-collapse: collapse; border-spacing: 0; padding: 0; text-align: left; vertical-align: top; width: 100%;"><tbody><tr style="padding: 0; text-align: left; vertical-align: top;"><td height="20px" style="-moz-hyphens: auto; -webkit-hyphens: auto; Margin: 0; border-collapse: collapse !important; color: #5D6879; font-family: Helvetica, Arial, sans-serif; font-size: 20px; font-weight: normal; hyphens: auto; line-height: 20px; margin: 0; mso-line-height-rule: exactly; padding: 0; text-align: left; vertical-align: top; word-wrap: break-word;">&#xA0;</td></tr></tbody></table>
-		<p style="Margin: 0; Margin-bottom: 10px; color: #5D6879; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0; margin-bottom: 10px; padding: 0; text-align: left;">
-			<%= greeting %>
-		</p>
+		<% if (greeting) { %>
+			<p style="Margin: 0; Margin-bottom: 10px; color: #5D6879; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0; margin-bottom: 10px; padding: 0; text-align: left;">
+				<%= greeting %>
+			</p>
+		<% } %>
 		<p class="avoid-auto-linking" style="Margin: 0; Margin-bottom: 10px; color: #5D6879; font-family: Helvetica, Arial, sans-serif; font-size: 16px; font-weight: normal; line-height: 1.3; margin: 0; margin-bottom: 10px; padding: 0; text-align: left;">
 			<%= message %>
 		</p>

+ 96 - 152
services/web/app/coffee/Features/Email/EmailBuilder.coffee

@@ -1,5 +1,6 @@
 _ = require('underscore')
 settings = require("settings-sharelatex")
+marked = require('marked')
 
 PersonalEmailLayout = require("./Layouts/PersonalEmailLayout")
 NotificationEmailLayout = require("./Layouts/NotificationEmailLayout")
@@ -7,169 +8,113 @@ BaseWithHeaderEmailLayout = require("./Layouts/" + settings.brandPrefix + "BaseW
 
 SingleCTAEmailBody = require("./Bodies/" + settings.brandPrefix + "SingleCTAEmailBody")
 
-templates = {}
-
-templates.registered =
-	subject:  _.template "Activate your #{settings.appName} Account"
-	layout: PersonalEmailLayout
-	type: "notification"
-	plainTextTemplate: _.template """
-Congratulations, you've just had an account created for you on #{settings.appName} with the email address "<%= to %>".
+CTAEmailTemplate = (content) ->
+	content.greeting ?= () -> 'Hi,'
+	content.secondaryMessage ?= () -> ""
+	return {
+		subject: (opts) -> content.subject(opts),
+		layout: BaseWithHeaderEmailLayout,
+		plainTextTemplate: (opts) -> """
+#{content.greeting(opts)}
+
+#{content.message(opts).trim()}
+
+#{content.ctaText(opts)}: #{content.ctaURL(opts)}
+
+#{content.secondaryMessage?(opts).trim() or ""}
+
+Regards,
+The #{settings.appName} Team - #{settings.siteUrl}
+		"""
+		compiledTemplate: (opts) ->
+			SingleCTAEmailBody({
+				title: content.title?(opts)
+				greeting: content.greeting(opts)
+				message: marked(content.message(opts).trim())
+				secondaryMessage: marked(content.secondaryMessage(opts).trim())
+				ctaText: content.ctaText(opts)
+				ctaURL: content.ctaURL(opts)
+				gmailGoToAction: content.gmailGoToAction?(opts)
+			})
+	}
 
-Click here to set your password and log in: <%= setNewPasswordUrl %>
-
-If you have any questions or problems, please contact #{settings.adminEmail}
-"""
-	compiledTemplate: _.template """
-<p>Congratulations, you've just had an account created for you on #{settings.appName} with the email address "<%= to %>".</p>
+templates = {}
 
-<p><a href="<%= setNewPasswordUrl %>">Click here to set your password and log in.</a></p>
+templates.registered = CTAEmailTemplate({
+	subject: () -> "Activate your #{settings.appName} Account"
+	message: (opts) -> """
+Congratulations, you've just had an account created for you on #{settings.appName} with the email address '#{opts.to}'.
 
-<p>If you have any questions or problems, please contact <a href="mailto:#{settings.adminEmail}">#{settings.adminEmail}</a>.</p>
+Click here to set your password and log in:
 """
-
-
-templates.canceledSubscription =
-	subject:  _.template "ShareLaTeX thoughts"
-	layout: PersonalEmailLayout
-	type:"lifecycle"
-	plainTextTemplate: _.template """
-Hi <%= first_name %>,
-
-I'm sorry to see you cancelled your ShareLaTeX premium account. Would you mind giving me some advice on what the site is lacking at the moment via this survey?:
-
-https://sharelatex.typeform.com/to/f5lBiZ
-
-Thank you in advance.
-
-Henry
-
-ShareLaTeX Co-founder
+	secondaryMessage: () -> "If you have any questions or problems, please contact #{settings.adminEmail}"
+	ctaText: () -> "Set password"
+	ctaURL: (opts) -> opts.setNewPasswordUrl
+})
+
+templates.canceledSubscription = CTAEmailTemplate({
+	subject: () -> "#{settings.appName} thoughts"
+	message: () -> """
+I'm sorry to see you cancelled your #{settings.appName} premium account. Would you mind giving us some feedback on what the site is lacking at the moment via this quick survey?
 """
-	compiledTemplate: _.template '''
-<p>Hi <%= first_name %>,</p>
-
-<p>I'm sorry to see you cancelled your ShareLaTeX premium account. Would you mind giving me some advice on what the site is lacking at the moment via <a href="https://sharelatex.typeform.com/to/f5lBiZ">this survey</a>?</p>
-
-<p>Thank you in advance.</p>
-
-<p>
-Henry <br>
-ShareLaTeX Co-founder
-</p>
-'''
-
-
-templates.passwordResetRequested =
-	subject:  _.template "Password Reset - #{settings.appName}"
-	layout: BaseWithHeaderEmailLayout
-	type:"notification"
-	plainTextTemplate: _.template """
-Password Reset
-
-We got a request to reset your #{settings.appName} password.
-
-Click this link to reset your password: <%= setNewPasswordUrl %>
-
+	secondaryMessage: () -> "Thank you in advance!"
+	ctaText: () -> "Leave Feedback"
+	ctaURL: (opts) -> "https://sharelatex.typeform.com/to/f5lBiZ"
+})
+
+templates.passwordResetRequested = CTAEmailTemplate({
+	subject: () -> "Password Reset - #{settings.appName}"
+	title: () -> "Password Reset"
+	message: () -> "We got a request to reset your #{settings.appName} password."
+	secondaryMessage: () -> """
 If you ignore this message, your password won't be changed.
 
 If you didn't request a password reset, let us know.
-
-Thank you
-
-#{settings.appName} - <%= siteUrl %>
-"""
-	compiledTemplate: (opts) ->
-		SingleCTAEmailBody({
-			title: "Password Reset"
-			greeting: "Hi,"
-			message: "We got a request to reset your #{settings.appName} password."
-			secondaryMessage: "If you ignore this message, your password won't be changed.<br>If you didn't request a password reset, let us know."
-			ctaText: "Reset password"
-			ctaURL: opts.setNewPasswordUrl
-			gmailGoToAction: null
-		})
-
-
-templates.projectInvite =
-	subject: _.template "<%= project.name %> - shared by <%= owner.email %>"
-	layout: BaseWithHeaderEmailLayout
-	type:"notification"
-	plainTextTemplate: _.template """
-Hi, <%= owner.email %> wants to share '<%= project.name %>' with you.
-
-Follow this link to view the project: <%= inviteUrl %>
-
-Thank you
-
-#{settings.appName} - <%= siteUrl %>
-"""
-	compiledTemplate: (opts) ->
-		SingleCTAEmailBody({
-			title: "#{ opts.project.name } &ndash; shared by #{ opts.owner.email }"
-			greeting: "Hi,"
-			message: "#{ opts.owner.email } wants to share &ldquo;#{ opts.project.name }&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"
-		})
-
-
-templates.verifyEmailToJoinTeam =
-	subject: _.template "<%= inviterName %> has invited you to join a team on #{settings.appName}"
-	layout: BaseWithHeaderEmailLayout
-	type:"notification"
-	plainTextTemplate: _.template """
-
-Please click the button below to join the team and enjoy the benefits of an upgraded  <%= appName %> account.
-
-<%= acceptInviteUrl %>
-
-Thank You
-
-#{settings.appName} - <%= siteUrl %>
 """
-	compiledTemplate: (opts) ->
-		SingleCTAEmailBody({
-			title: "#{opts.inviterName} has invited you to join a team on #{settings.appName}"
-			greeting: "Hi,"
-			message: "Please click the button below to join the team and enjoy the benefits of an upgraded  #{ opts.appName } account."
-			secondaryMessage: null
-			ctaText: "Verify now"
-			ctaURL: opts.acceptInviteUrl
-			gmailGoToAction: null
-		})
-
-templates.testEmail =
-	subject: _.template "A Test Email from ShareLaTeX"
-	layout: BaseWithHeaderEmailLayout
-	type:"notification"
-	plainTextTemplate: _.template """
-Hi,
-
-This is a test email sent from ShareLaTeX.
-
-#{settings.appName} - <%= siteUrl %>
-"""
-	compiledTemplate: (opts) ->
-		SingleCTAEmailBody({
-			title: "A Test Email from ShareLaTeX"
-			greeting: "Hi,"
-			message: "This is a test email sent from ShareLaTeX"
-			secondaryMessage: null
-			ctaText: "Open ShareLaTeX"
-			ctaURL: "/"
-			gmailGoToAction: null
-		})
-
+	ctaText: () -> "Reset password"
+	ctaURL: (opts) -> opts.setNewPasswordUrl
+})
+
+templates.confirmEmail = CTAEmailTemplate({
+	subject: () -> "Confirm Email - #{settings.appName}"
+	title: () -> "Confirm Email"
+	message: () -> "Please confirm your email on #{settings.appName}."
+	ctaText: () -> "Confirm Email"
+	ctaURL: (opts) -> opts.confirmEmailUrl
+})
+
+templates.projectInvite = CTAEmailTemplate({
+	subject: (opts) -> "#{opts.project.name} - shared by #{opts.owner.email}"
+	title: (opts) -> "#{ opts.project.name } - shared by #{ opts.owner.email }"
+	message: (opts) -> "#{ opts.owner.email } wants to share '#{ opts.project.name }' with you."
+	ctaText: () -> "View project"
+	ctaURL: (opts) -> opts.inviteUrl
+	gmailGoToAction: (opts) ->
+		target: opts.inviteUrl
+		name: "View project"
+		description: "Join #{ opts.project.name } at #{ settings.appName }"
+})
+
+templates.verifyEmailToJoinTeam = CTAEmailTemplate({
+	subject: (opts) -> "#{ opts.inviterName } has invited you to join a team on #{settings.appName}"
+	title: (opts) -> "#{opts.inviterName} has invited you to join a team on #{settings.appName}"
+	message: (opts) -> "Please click the button below to join the team and enjoy the benefits of an upgraded #{ settings.appName } account."
+	ctaText: (opts) -> "Join now"
+	ctaURL: (opts) -> opts.acceptInviteUrl
+})
+
+templates.testEmail = CTAEmailTemplate({
+	subject: () -> "A Test Email from #{settings.appName}"
+	title: () -> "A Test Email from #{settings.appName}"
+	greeting: () -> "Hi,"
+	message: () -> "This is a test Email from #{settings.appName}"
+	ctaText: () -> "Open #{settings.appName}"
+	ctaURL: () -> settings.siteUrl
+})
 
 module.exports =
 	templates: templates
-
+	CTAEmailTemplate: CTAEmailTemplate
 	buildEmail: (templateName, opts)->
 		template = templates[templateName]
 		opts.siteUrl = settings.siteUrl
@@ -180,5 +125,4 @@ module.exports =
 			subject : template.subject(opts)
 			html: template.layout(opts)
 			text: template?.plainTextTemplate?(opts)
-			type:template.type
 		}

+ 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

+ 35 - 19
services/web/app/coffee/Features/Security/OneTimeTokenHandler.coffee

@@ -1,34 +1,50 @@
 Settings = require('settings-sharelatex')
-RedisWrapper = require("../../infrastructure/RedisWrapper")
-rclient = RedisWrapper.client("one_time_token")
 crypto = require("crypto")
 logger = require("logger-sharelatex")
+{db} = require "../../infrastructure/mongojs"
+Errors = require "../Errors/Errors"
 
 ONE_HOUR_IN_S = 60 * 60
 
-buildKey = (token)-> return "password_token:#{token}"
-
 module.exports =
-
-	getNewToken: (value, options = {}, callback)->
+	getNewToken: (use, data, options = {}, callback = (error, data) ->)->
 		# 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"
+		createdAt = new Date()
+		expiresAt = new Date(createdAt.getTime() + expiresIn * 1000)
 		token = crypto.randomBytes(32).toString("hex")
-		multi = rclient.multi()
-		multi.set buildKey(token), value
-		multi.expire buildKey(token), expiresIn
-		multi.exec (err)->
-			callback(err, token)
+		logger.log {data, expiresIn, token_start: token.slice(0,8)}, "generating token for #{use}"
+		db.tokens.insert {
+			use: use
+			token: token,
+			data: data,
+			createdAt: createdAt,
+			expiresAt: expiresAt
+		}, (error) ->
+			return callback(error) if error?
+			callback null, token
 
-	getValueFromTokenAndExpire: (token, callback)->
-		logger.log token:token, "getting user id from password token"
-		multi = rclient.multi()
-		multi.get buildKey(token)
-		multi.del buildKey(token)
-		multi.exec (err, results)->
-			callback err, results?[0]
+	getValueFromTokenAndExpire: (use, token, callback = (error, data) ->)->
+		logger.log token_start: token.slice(0,8), "getting data from #{use} token"
+		now = new Date()
+		db.tokens.findAndModify {
+			query: {
+				use: use,
+				token: token,
+				expiresAt: { $gt: now },
+				usedAt: { $exists: false }
+			},
+			update: {
+				$set: {
+					usedAt: now
+				}
+			}
+		}, (error, token) ->
+			return callback(error) if error?
+			if !token?
+				return callback(new Errors.NotFoundError('no token found'))
+			return callback null, token.data
 

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

@@ -0,0 +1,36 @@
+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 =
+	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?
+		data = {user_id, email}
+		OneTimeTokenHandler.getNewToken 'email_confirmation', data, {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} = 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'

+ 1 - 1
services/web/app/coffee/infrastructure/mongojs.coffee

@@ -1,6 +1,6 @@
 Settings = require "settings-sharelatex"
 mongojs = require "mongojs"
-db = mongojs(Settings.mongo.url, ["projects", "users", "userstubs"])
+db = mongojs(Settings.mongo.url, ["projects", "users", "userstubs", "tokens"])
 module.exports =
 	db: db
 	ObjectId: mongojs.ObjectId

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

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

@@ -0,0 +1,194 @@
+expect = require("chai").expect
+async = require("async")
+User = require "./helpers/User"
+request = require "./helpers/request"
+settings = require "settings-sharelatex"
+{db, ObjectId} = require("../../../app/js/infrastructure/mongojs")
+
+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) =>
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
+						# There should only be one confirmation token at the moment
+						expect(tokens.length).to.equal 1
+						expect(tokens[0].data.email).to.equal 'newly-added-email@example.com'
+						expect(tokens[0].data.user_id).to.equal @user._id
+						token = tokens[0].token
+						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) =>
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
+						# Token should be deleted after use
+						expect(tokens.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) =>
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
+						# There should only be one confirmation token at the moment
+						expect(tokens.length).to.equal 1
+						expect(tokens[0].data.email).to.equal @email
+						expect(tokens[0].data.user_id).to.equal @user._id
+						token1 = tokens[0].token
+						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) =>
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
+						# The first token has been used, so this should be token2 now
+						expect(tokens.length).to.equal 1
+						expect(tokens[0].data.email).to.equal @email
+						expect(tokens[0].data.user_id).to.equal @user2._id
+						token2 = tokens[0].token
+						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
+
+	describe "with an expired token", ->
+		it 'should not confirm the email', (done) ->
+			token = null
+			async.series [
+				(cb) =>
+					@user.request { 
+						method: 'POST',
+						url: '/user/emails',
+						json:
+							email: @email = 'expired-token-email@example.com'
+					}, (error, response, body) =>
+						return done(error) if error?
+						expect(response.statusCode).to.equal 204
+						cb()
+				(cb) =>
+					db.tokens.find {
+						use: 'email_confirmation',
+						usedAt: { $exists: false }
+					}, (error, tokens) =>
+						# There should only be one confirmation token at the moment
+						expect(tokens.length).to.equal 1
+						expect(tokens[0].data.email).to.equal @email
+						expect(tokens[0].data.user_id).to.equal @user._id
+						token = tokens[0].token
+						cb()
+				(cb) =>
+					db.tokens.update {
+						token: token
+					}, {
+						$set: { 
+							expiresAt: new Date(Date.now() - 1000000)
+						}
+					}, 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 404
+						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

+ 80 - 45
services/web/test/unit/coffee/Security/OneTimeTokenHandlerTests.coffee

@@ -5,64 +5,99 @@ path = require('path')
 sinon = require('sinon')
 modulePath = path.join __dirname, "../../../../app/js/Features/Security/OneTimeTokenHandler"
 expect = require("chai").expect
+Errors = require "../../../../app/js/Features/Errors/Errors"
+tk = require("timekeeper")
 
 describe "OneTimeTokenHandler", ->
-
 	beforeEach ->
-		@value = "user id here"
-		@stubbedToken = require("crypto").randomBytes(32)
-
-		@settings = 
-			redis:
-				web:{}
-		@redisMulti =
-			set:sinon.stub()
-			get:sinon.stub()
-			del:sinon.stub()
-			expire:sinon.stub()
-			exec:sinon.stub()
-		self = @
+		tk.freeze Date.now() # freeze the time for these tests
+		@stubbedToken = "mock-token"
+		@callback = sinon.stub()
 		@OneTimeTokenHandler = SandboxedModule.require modulePath, requires:
-			"../../infrastructure/RedisWrapper" :
-				client: =>
-					auth:->
-					multi: -> return self.redisMulti
-
 			"settings-sharelatex":@settings
 			"logger-sharelatex": log:->
 			"crypto": randomBytes: () => @stubbedToken
+			"../Errors/Errors": Errors
+			"../../infrastructure/mongojs": db: @db = tokens: {}
 
+	afterEach ->
+		tk.reset()
 
 	describe "getNewToken", ->
+		beforeEach ->
+			@db.tokens.insert = sinon.stub().yields()
+
+		describe 'normally', ->
+			beforeEach ->
+				@OneTimeTokenHandler.getNewToken 'password', 'mock-data-to-store', @callback
+
+			it "should insert a generated token with a 1 hour expiry", ->
+				@db.tokens.insert
+					.calledWith({
+						use: 'password'
+						token: @stubbedToken,
+						createdAt: new Date(),
+						expiresAt: new Date(Date.now() + 60 * 60 * 1000)
+						data: 'mock-data-to-store'
+					})
+					.should.equal true
+
+			it 'should call the callback with the token', ->
+				@callback.calledWith(null, @stubbedToken).should.equal true
+
+		describe 'with an optional expiresIn parameter', ->
+			beforeEach ->
+				@OneTimeTokenHandler.getNewToken 'password', 'mock-data-to-store', { expiresIn: 42 }, @callback
+
+			it "should insert a generated token with a custom expiry", ->
+				@db.tokens.insert
+					.calledWith({
+						use: 'password'
+						token: @stubbedToken,
+						createdAt: new Date(),
+						expiresAt: new Date(Date.now() + 42 * 1000)
+						data: 'mock-data-to-store'
+					})
+					.should.equal true
 
-		it "should set a new token into redis with a ttl", (done)->
-			@redisMulti.exec.callsArgWith(0) 
-			@OneTimeTokenHandler.getNewToken @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)=>
-				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) =>
-				@redisMulti.expire.calledWith("password_token:#{@stubbedToken.toString("hex")}", @ttl).should.equal true
-				done()
+			it 'should call the callback with the token', ->
+				@callback.calledWith(null, @stubbedToken).should.equal true
 
 	describe "getValueFromTokenAndExpire", ->
+		describe 'successfully', ->
+			beforeEach ->
+				@db.tokens.findAndModify = sinon.stub().yields(null, { data: 'mock-data' })
+				@OneTimeTokenHandler.getValueFromTokenAndExpire 'password', 'mock-token', @callback
+
+			it 'should expire the token', ->
+				@db.tokens.findAndModify
+					.calledWith({
+						query: {
+							use: 'password'
+							token: 'mock-token',
+							expiresAt: { $gt: new Date() },
+							usedAt: { $exists: false }
+						},
+						update: {
+							$set: { usedAt: new Date() }
+						}
+					})
+					.should.equal true
+
+			it 'should return the data', ->
+				@callback.calledWith(null, 'mock-data').should.equal true
+
+		describe 'when a valid token is not found', ->
+			beforeEach ->
+				@db.tokens.findAndModify = sinon.stub().yields(null, null)
+				@OneTimeTokenHandler.getValueFromTokenAndExpire 'password', 'mock-token', @callback
+
+			it 'should return a NotFoundError', ->
+				@callback
+					.calledWith(sinon.match.instanceOf(Errors.NotFoundError))
+					.should.equal true
+
+
 
-		it "should get and delete the token", (done)->
-			@redisMulti.exec.callsArgWith(0, null, [@value]) 
-			@OneTimeTokenHandler.getValueFromTokenAndExpire @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
-				done()
 
 

+ 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',
+						{@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, 
+				{@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, 
+					{@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, 
+					{@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()
+
+