From 4f8a905e9b1f599243c615549280cee66665dc3f Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 15 Apr 2021 16:23:41 +0200 Subject: [PATCH] Merge pull request #3909 from overleaf/jel-reconfirm-email-template Add reconfirm email template GitOrigin-RevId: 2488c79c25a7148f601e3e3e2021cdbee4be7b4c --- .../app/src/Features/Email/EmailBuilder.js | 26 +++ .../Features/Security/OneTimeTokenHandler.js | 42 ++-- .../User/UserEmailsConfirmationHandler.js | 31 ++- .../src/Features/User/UserEmailsController.js | 28 +++ .../UserAffiliationsReconfirmController.js | 8 +- .../test/unit/src/Email/EmailBuilderTests.js | 37 ++++ .../src/User/UserEmailsControllerTests.js | 181 +++++++++++------- 7 files changed, 255 insertions(+), 98 deletions(-) diff --git a/services/web/app/src/Features/Email/EmailBuilder.js b/services/web/app/src/Features/Email/EmailBuilder.js index f27a35401b..972fa6c64b 100644 --- a/services/web/app/src/Features/Email/EmailBuilder.js +++ b/services/web/app/src/Features/Email/EmailBuilder.js @@ -283,6 +283,32 @@ templates.projectInvite = ctaTemplate({ } }) +templates.reconfirmEmail = ctaTemplate({ + subject() { + return `Reconfirm Email - ${settings.appName}` + }, + title() { + return 'Reconfirm Email' + }, + message(opts) { + return [ + `Please reconfirm your email address, ${opts.to}, on your ${settings.appName} account.` + ] + }, + secondaryMessage() { + return [ + 'If you did not request this, you can simply ignore this message.', + `If you have any questions or trouble confirming your email address, please get in touch with our support team at ${settings.adminEmail}.` + ] + }, + ctaText() { + return 'Reconfirm Email' + }, + ctaURL(opts) { + return opts.confirmEmailUrl + } +}) + templates.verifyEmailToJoinTeam = ctaTemplate({ subject(opts) { return `${_.escape( diff --git a/services/web/app/src/Features/Security/OneTimeTokenHandler.js b/services/web/app/src/Features/Security/OneTimeTokenHandler.js index 70957e1922..946336cd6d 100644 --- a/services/web/app/src/Features/Security/OneTimeTokenHandler.js +++ b/services/web/app/src/Features/Security/OneTimeTokenHandler.js @@ -1,31 +1,17 @@ -/* eslint-disable - node/handle-callback-err, - max-len, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -const Settings = require('settings-sharelatex') const crypto = require('crypto') -const logger = require('logger-sharelatex') const { db } = require('../../infrastructure/mongodb') const Errors = require('../Errors/Errors') +const { promisifyAll } = require('../../util/promises') const ONE_HOUR_IN_S = 60 * 60 -module.exports = { +const OneTimeTokenHandler = { getNewToken(use, data, options, callback) { // options is optional - if (options == null) { + if (!options) { options = {} } - if (callback == null) { + if (!callback) { callback = function (error, data) {} } if (typeof options === 'function') { @@ -36,7 +22,7 @@ module.exports = { const createdAt = new Date() const expiresAt = new Date(createdAt.getTime() + expiresIn * 1000) const token = crypto.randomBytes(32).toString('hex') - return db.tokens.insertOne( + db.tokens.insertOne( { use, token, @@ -45,20 +31,20 @@ module.exports = { expiresAt }, function (error) { - if (error != null) { + if (error) { return callback(error) } - return callback(null, token) + callback(null, token) } ) }, getValueFromTokenAndExpire(use, token, callback) { - if (callback == null) { + if (!callback) { callback = function (error, data) {} } const now = new Date() - return db.tokens.findOneAndUpdate( + db.tokens.findOneAndUpdate( { use, token, @@ -71,15 +57,19 @@ module.exports = { } }, function (error, result) { - if (error != null) { + if (error) { return callback(error) } const token = result.value - if (token == null) { + if (!token) { return callback(new Errors.NotFoundError('no token found')) } - return callback(null, token.data) + callback(null, token.data) } ) } } + +OneTimeTokenHandler.promises = promisifyAll(OneTimeTokenHandler) + +module.exports = OneTimeTokenHandler diff --git a/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js b/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js index ba36fcc0e2..6013312161 100644 --- a/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js +++ b/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js @@ -5,10 +5,11 @@ const settings = require('settings-sharelatex') const Errors = require('../Errors/Errors') const UserUpdater = require('./UserUpdater') const UserGetter = require('./UserGetter') -const { promisify } = require('util') +const { callbackify, promisify } = require('util') // Reject email confirmation tokens after 90 days const TOKEN_EXPIRY_IN_S = 90 * 24 * 60 * 60 +const TOKEN_USE = 'email_confirmation' function sendConfirmationEmail(userId, email, emailTemplate, callback) { if (arguments.length === 3) { @@ -28,7 +29,7 @@ function sendConfirmationEmail(userId, email, emailTemplate, callback) { } const data = { user_id: userId, email } OneTimeTokenHandler.getNewToken( - 'email_confirmation', + TOKEN_USE, data, { expiresIn: TOKEN_EXPIRY_IN_S }, function (err, token) { @@ -45,12 +46,36 @@ function sendConfirmationEmail(userId, email, emailTemplate, callback) { ) } +async function sendReconfirmationEmail(userId, email) { + email = EmailHelper.parseEmail(email) + if (!email) { + throw new Error('invalid email') + } + + const data = { user_id: userId, email } + const token = await OneTimeTokenHandler.promises.getNewToken( + TOKEN_USE, + data, + { expiresIn: TOKEN_EXPIRY_IN_S } + ) + + const emailOptions = { + to: email, + confirmEmailUrl: `${settings.siteUrl}/user/emails/confirm?token=${token}`, + sendingUser_id: userId + } + + await EmailHandler.promises.sendEmail('reconfirmEmail', emailOptions) +} + const UserEmailsConfirmationHandler = { sendConfirmationEmail, + sendReconfirmationEmail: callbackify(sendReconfirmationEmail), + confirmEmailFromToken(token, callback) { OneTimeTokenHandler.getValueFromTokenAndExpire( - 'email_confirmation', + TOKEN_USE, token, function (error, data) { if (error) { diff --git a/services/web/app/src/Features/User/UserEmailsController.js b/services/web/app/src/Features/User/UserEmailsController.js index dbe81451df..9b04d3e9a3 100644 --- a/services/web/app/src/Features/User/UserEmailsController.js +++ b/services/web/app/src/Features/User/UserEmailsController.js @@ -87,6 +87,32 @@ function resendConfirmation(req, res, next) { }) } +function sendReconfirmation(req, res, next) { + const userId = AuthenticationController.getLoggedInUserId(req) + const email = EmailHelper.parseEmail(req.body.email) + if (!email) { + return res.sendStatus(400) + } + UserGetter.getUserByAnyEmail(email, { _id: 1 }, function (error, user) { + if (error) { + return next(error) + } + if (!user || user._id.toString() !== userId) { + return res.sendStatus(422) + } + UserEmailsConfirmationHandler.sendReconfirmationEmail( + userId, + email, + function (error) { + if (error) { + return next(error) + } + res.sendStatus(200) + } + ) + }) +} + const UserEmailsController = { list(req, res, next) { const userId = AuthenticationController.getLoggedInUserId(req) @@ -176,6 +202,8 @@ const UserEmailsController = { resendConfirmation, + sendReconfirmation, + showConfirm(req, res, next) { res.render('user/confirm_email', { token: req.query.token, diff --git a/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsReconfirmController.js b/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsReconfirmController.js index fa292b37e7..aa8c6049fc 100644 --- a/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsReconfirmController.js +++ b/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsReconfirmController.js @@ -4,7 +4,7 @@ import getMeta from '../../../utils/meta' export default App.controller( 'UserAffiliationsReconfirmController', - function ($scope, UserAffiliationsDataService, $window) { + function ($scope, $http, $window) { const samlInitPath = ExposedSettings.samlInitPath $scope.reconfirm = {} $scope.ui = $scope.ui || {} // $scope.ui inherited on settings page @@ -30,7 +30,11 @@ export default App.controller( function sendReconfirmEmail(email) { $scope.ui.hasError = false $scope.ui.isMakingRequest = true - UserAffiliationsDataService.resendConfirmationEmail(email) + $http + .post('/user/emails/send-reconfirmation', { + email, + _csrf: window.csrfToken + }) .then(() => { $scope.reconfirm[email].reconfirmationSent = true }) diff --git a/services/web/test/unit/src/Email/EmailBuilderTests.js b/services/web/test/unit/src/Email/EmailBuilderTests.js index 5c4a547da2..bde69edb17 100644 --- a/services/web/test/unit/src/Email/EmailBuilderTests.js +++ b/services/web/test/unit/src/Email/EmailBuilderTests.js @@ -307,6 +307,43 @@ describe('EmailBuilder', function () { }) }) + describe('reconfirmEmail', function () { + before(function () { + this.emailAddress = 'example@overleaf.com' + this.userId = 'abc123' + this.opts = { + to: this.emailAddress, + confirmEmailUrl: `${this.settings.siteUrl}/user/emails/confirm?token=aToken123`, + sendingUser_id: this.userId + } + this.email = this.EmailBuilder.buildEmail('reconfirmEmail', this.opts) + }) + + it('should build the email', function () { + expect(this.email.html).to.exist + expect(this.email.text).to.exist + }) + + describe('HTML email', function () { + it('should include a CTA button and a fallback CTA link', function () { + const dom = cheerio.load(this.email.html) + const buttonLink = dom('a:contains("Reconfirm Email")') + expect(buttonLink.length).to.equal(1) + expect(buttonLink.attr('href')).to.equal(this.opts.confirmEmailUrl) + const fallback = dom('.force-overleaf-style').last() + expect(fallback.length).to.equal(1) + const fallbackLink = fallback.html() + expect(fallbackLink).to.contain(this.opts.confirmEmailUrl) + }) + }) + + describe('plain text email', function () { + it('should contain the CTA link', function () { + expect(this.email.text).to.contain(this.opts.confirmEmailUrl) + }) + }) + }) + describe('verifyEmailToJoinTeam', function () { before(function () { this.emailAddress = 'example@overleaf.com' diff --git a/services/web/test/unit/src/User/UserEmailsControllerTests.js b/services/web/test/unit/src/User/UserEmailsControllerTests.js index a59f89e66c..94f85fd858 100644 --- a/services/web/test/unit/src/User/UserEmailsControllerTests.js +++ b/services/web/test/unit/src/User/UserEmailsControllerTests.js @@ -18,6 +18,7 @@ describe('UserEmailsController', function () { this.UserGetter = { getUserFullEmails: sinon.stub(), + getUserByAnyEmail: sinon.stub(), promises: { getUser: sinon.stub().resolves(this.user) } @@ -45,13 +46,7 @@ describe('UserEmailsController', function () { this.EmailHelper = { parseEmail: sinon.stub() } this.endorseAffiliation = sinon.stub().yields() this.InstitutionsAPI = { - endorseAffiliation: this.endorseAffiliation, - getInstitutionViaDomain: sinon - .stub() - .withArgs('overleaf.com') - .resolves({ sso_enabled: true }) - .withArgs('example.com') - .resolves({ sso_enabled: false }) + endorseAffiliation: this.endorseAffiliation } this.HttpErrorHandler = { conflict: sinon.stub() } this.UserEmailsController = SandboxedModule.require(modulePath, { @@ -69,6 +64,7 @@ describe('UserEmailsController', function () { }), '../Helpers/EmailHelper': this.EmailHelper, './UserEmailsConfirmationHandler': (this.UserEmailsConfirmationHandler = { + sendReconfirmationEmail: sinon.stub(), promises: { sendConfirmationEmail: sinon.stub().resolves() } @@ -439,8 +435,13 @@ describe('UserEmailsController', function () { }) }) }) + describe('resendConfirmation', function () { beforeEach(function () { + this.EmailHelper.parseEmail.returnsArg(0) + this.UserGetter.getUserByAnyEmail.yields(undefined, { + _id: this.user._id + }) this.req = { body: {} } @@ -452,74 +453,120 @@ describe('UserEmailsController', function () { .stub() .yields() }) - describe('when institution SSO is released', function () { + + it('should send the email', function (done) { + this.req = { + body: { + email: 'test@example.com' + } + } + this.UserEmailsController.sendReconfirmation( + this.req, + this.res, + this.next + ) + expect(this.UserEmailsConfirmationHandler.sendReconfirmationEmail).to.have + .been.calledOnce + done() + }) + + it('should return 422 if email not valid', function (done) { + this.req = { + body: {} + } + this.UserEmailsController.resendConfirmation( + this.req, + this.res, + this.next + ) + expect(this.UserEmailsConfirmationHandler.sendConfirmationEmail).to.not + .have.been.called + expect(this.res.sendStatus.lastCall.args[0]).to.equal(422) + done() + }) + describe('email on another user account', function () { beforeEach(function () { - this.Features.hasFeature.withArgs('saml').returns(true) - }) - describe('for an institution SSO email', function () { - beforeEach(function () { - this.req.body.email = 'with-sso@overleaf.com' - }) - it('should not send the email', function () { - this.UserEmailsController.resendConfirmation( - this.req, - this.res, - () => { - this.UserEmailsConfirmationHandler.sendConfirmationEmail.should - .not.have.been.called.once - } - ) + this.UserGetter.getUserByAnyEmail.yields(undefined, { + _id: 'another-user-id' }) }) - describe('for a non-institution SSO email', function () { - beforeEach(function () { - this.req.body.email = 'without-sso@example.com' - }) - it('should send the email', function () { - this.UserEmailsController.resendConfirmation( - this.req, - this.res, - () => { - this.UserEmailsConfirmationHandler.sendConfirmationEmail.should - .have.been.called.once - } - ) - }) + it('should return 422', function (done) { + this.req = { + body: { + email: 'test@example.com' + } + } + this.UserEmailsController.resendConfirmation( + this.req, + this.res, + this.next + ) + expect(this.UserEmailsConfirmationHandler.sendConfirmationEmail).to.not + .have.been.called + expect(this.res.sendStatus.lastCall.args[0]).to.equal(422) + done() }) }) - describe('when institution SSO is not released', function () { + }) + + describe('sendReconfirmation', function () { + beforeEach(function () { + this.res.sendStatus = sinon.stub() + this.UserGetter.getUserByAnyEmail.yields(undefined, { + _id: this.user._id + }) + this.EmailHelper.parseEmail.returnsArg(0) + }) + it('should send the email', function (done) { + this.req = { + body: { + email: 'test@example.com' + } + } + this.UserEmailsController.sendReconfirmation( + this.req, + this.res, + this.next + ) + expect(this.UserEmailsConfirmationHandler.sendReconfirmationEmail).to.have + .been.calledOnce + done() + }) + it('should return 400 if email not valid', function (done) { + this.req = { + body: {} + } + this.UserEmailsController.sendReconfirmation( + this.req, + this.res, + this.next + ) + expect(this.UserEmailsConfirmationHandler.sendReconfirmationEmail).to.not + .have.been.called + expect(this.res.sendStatus.lastCall.args[0]).to.equal(400) + done() + }) + describe('email on another user account', function () { beforeEach(function () { - this.Features.hasFeature.withArgs('saml').returns(false) - }) - describe('for an institution SSO email', function () { - beforeEach(function () { - this.req.body.email = 'with-sso@overleaf.com' - }) - it('should send the email', function () { - this.UserEmailsController.resendConfirmation( - this.req, - this.res, - () => { - this.UserEmailsConfirmationHandler.sendConfirmationEmail.should - .have.been.called.once - } - ) + this.UserGetter.getUserByAnyEmail.yields(undefined, { + _id: 'another-user-id' }) }) - describe('for a non-institution SSO email', function () { - beforeEach(function () { - this.req.body.email = 'without-sso@example.com' - }) - it('should send the email', function () { - this.UserEmailsController.resendConfirmation( - this.req, - this.res, - () => { - this.UserEmailsConfirmationHandler.sendConfirmationEmail.should - .have.been.called.once - } - ) - }) + it('should return 422', function (done) { + this.req = { + body: { + email: 'test@example.com' + } + } + this.UserEmailsController.sendReconfirmation( + this.req, + this.res, + this.next + ) + expect(this.UserEmailsConfirmationHandler.sendReconfirmationEmail).to + .not.have.been.called + expect(this.res.sendStatus.lastCall.args[0]).to.equal(422) + done() }) }) })