From 0dcbc5facb934907ab212eebaa556e4e191091b1 Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 19 Jun 2018 13:55:34 +0100 Subject: [PATCH] Send out confirmation emails on register and record confirmedAt date --- .../PasswordReset/PasswordResetHandler.coffee | 4 +- .../Security/OneTimeTokenHandler.coffee | 18 +-- .../User/UserEmailsConfirmationHandler.coffee | 42 ++++++ .../Features/User/UserEmailsController.coffee | 43 ++++-- .../User/UserRegistrationHandler.coffee | 2 +- .../coffee/Features/User/UserUpdater.coffee | 25 +++- services/web/app/coffee/models/User.coffee | 3 +- services/web/app/coffee/router.coffee | 4 + services/web/app/views/user/confirm_email.pug | 28 ++++ .../public/coffee/directives/asyncForm.coffee | 16 ++- .../acceptance/coffee/UserEmailsTests.coffee | 132 ++++++++++++++++++ .../PasswordResetHandlerTests.coffee | 10 +- .../Security/OneTimeTokenHandlerTests.coffee | 8 +- .../UserEmailsConfirmationHandlerTests.coffee | 125 +++++++++++++++++ .../User/UserEmailsControllerTests.coffee | 92 +++++++----- .../User/UserRegistrationHandlerTests.coffee | 4 +- .../unit/coffee/User/UserUpdaterTests.coffee | 39 +++++- 17 files changed, 516 insertions(+), 79 deletions(-) create mode 100644 services/web/app/coffee/Features/User/UserEmailsConfirmationHandler.coffee create mode 100644 services/web/app/views/user/confirm_email.pug create mode 100644 services/web/test/acceptance/coffee/UserEmailsTests.coffee create mode 100644 services/web/test/unit/coffee/User/UserEmailsConfirmationHandlerTests.coffee diff --git a/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee b/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee index 3947b63004..d383e8b669 100644 --- a/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee +++ b/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 diff --git a/services/web/app/coffee/Features/Security/OneTimeTokenHandler.coffee b/services/web/app/coffee/Features/Security/OneTimeTokenHandler.coffee index 3824703efb..c989ef9505 100644 --- a/services/web/app/coffee/Features/Security/OneTimeTokenHandler.coffee +++ b/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] diff --git a/services/web/app/coffee/Features/User/UserEmailsConfirmationHandler.coffee b/services/web/app/coffee/Features/User/UserEmailsConfirmationHandler.coffee new file mode 100644 index 0000000000..1fbad4a294 --- /dev/null +++ b/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 diff --git a/services/web/app/coffee/Features/User/UserEmailsController.coffee b/services/web/app/coffee/Features/User/UserEmailsController.coffee index 07433d2dde..f877cbe345 100644 --- a/services/web/app/coffee/Features/User/UserEmailsController.coffee +++ b/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 \ No newline at end of file diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee index 85cdabdbdf..df7fe93218 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/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}" diff --git a/services/web/app/coffee/Features/User/UserUpdater.coffee b/services/web/app/coffee/Features/User/UserUpdater.coffee index 22b31239bd..4de994b5c2 100644 --- a/services/web/app/coffee/Features/User/UserUpdater.coffee +++ b/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' diff --git a/services/web/app/coffee/models/User.coffee b/services/web/app/coffee/models/User.coffee index 71982ba40b..730b3668c6 100644 --- a/services/web/app/coffee/models/User.coffee +++ b/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 : ''} diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 339b4abe1c..6014fea661 100644 --- a/services/web/app/coffee/router.coffee +++ b/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(), diff --git a/services/web/app/views/user/confirm_email.pug b/services/web/app/views/user/confirm_email.pug new file mode 100644 index 0000000000..388a0a0252 --- /dev/null +++ b/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... diff --git a/services/web/public/coffee/directives/asyncForm.coffee b/services/web/public/coffee/directives/asyncForm.coffee index 15e3c9b3fe..acafec563d 100644 --- a/services/web/public/coffee/directives/asyncForm.coffee +++ b/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", () -> diff --git a/services/web/test/acceptance/coffee/UserEmailsTests.coffee b/services/web/test/acceptance/coffee/UserEmailsTests.coffee new file mode 100644 index 0000000000..8cdbea75fa --- /dev/null +++ b/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 diff --git a/services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee b/services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee index 261f5582dd..7ee82fbd7e 100644 --- a/services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee +++ b/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 diff --git a/services/web/test/unit/coffee/Security/OneTimeTokenHandlerTests.coffee b/services/web/test/unit/coffee/Security/OneTimeTokenHandlerTests.coffee index 1940f34d07..21e8d9f288 100644 --- a/services/web/test/unit/coffee/Security/OneTimeTokenHandlerTests.coffee +++ b/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 diff --git a/services/web/test/unit/coffee/User/UserEmailsConfirmationHandlerTests.coffee b/services/web/test/unit/coffee/User/UserEmailsConfirmationHandlerTests.coffee new file mode 100644 index 0000000000..22cd70c4bc --- /dev/null +++ b/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 + diff --git a/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee b/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee index 5b4a32fb38..d0e8076276 100644 --- a/services/web/test/unit/coffee/User/UserEmailsControllerTests.coffee +++ b/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,30 +50,29 @@ 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 + @UserUpdater.addEmailAddress.callsArgWith 2, null + @UserEmailsConfirmationHandler.sendConfirmationEmail = sinon.stub().yields() it 'adds new email', (done) -> - @UserUpdater.addEmailAddress.callsArgWith 2, null - @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 'sends an email confirmation', (done) -> + @UserEmailsController.add @req, + sendStatus: (code) => + code.should.equal 204 + assertCalledWith @UserEmailsConfirmationHandler.sendConfirmationEmail, @user._id, @newEmail + done() + it 'handles email parse error', (done) -> @EmailHelper.parseEmail.returns null @@ -80,14 +82,6 @@ describe "UserEmailsController", -> assertNotCalled @UserUpdater.addEmailAddress done() - it 'handles error', (done) -> - @UserUpdater.addEmailAddress.callsArgWith 2, new Error('Oups') - - @UserEmailsController.add @req, - sendStatus: (code) => - code.should.equal 500 - done() - describe 'remove', -> beforeEach -> @email = 'email_to_remove@bar.com' @@ -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() diff --git a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee index 3a8801bc08..e9a6c568dd 100644 --- a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee +++ b/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", -> diff --git a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee index 7b3be3df2b..6b9a1ecc85 100644 --- a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee +++ b/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() + +