diff --git a/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js b/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js
index b800e7db30..18fdf33b08 100644
--- a/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js
+++ b/services/web/app/src/Features/User/UserEmailsConfirmationHandler.js
@@ -7,6 +7,7 @@ const UserUpdater = require('./UserUpdater')
const UserGetter = require('./UserGetter')
const { callbackify, promisify } = require('util')
const crypto = require('crypto')
+const SessionManager = require('../Authentication/SessionManager')
// Reject email confirmation tokens after 90 days
const TOKEN_EXPIRY_IN_S = 90 * 24 * 60 * 60
@@ -85,58 +86,51 @@ async function sendReconfirmationEmail(userId, email) {
await EmailHandler.promises.sendEmail('reconfirmEmail', emailOptions)
}
+async function confirmEmailFromToken(req, token) {
+ const { data } = await OneTimeTokenHandler.promises.peekValueFromToken(
+ TOKEN_USE,
+ token
+ )
+ if (!data) {
+ throw new Errors.NotFoundError('no token found')
+ }
+
+ const loggedInUserId = SessionManager.getLoggedInUserId(req.session)
+ // user_id may be stored as an ObjectId or string
+ const userId = data.user_id?.toString()
+ const email = data.email
+ if (!userId || email !== EmailHelper.parseEmail(email)) {
+ throw new Errors.NotFoundError('invalid data')
+ }
+ if (loggedInUserId !== userId) {
+ throw new Errors.ForbiddenError('logged in user does not match token user')
+ }
+ const user = await UserGetter.promises.getUser(userId, { emails: 1 })
+ if (!user) {
+ throw new Errors.NotFoundError('user not found')
+ }
+ const emailExists = user.emails.some(emailData => emailData.email === email)
+ if (!emailExists) {
+ throw new Errors.NotFoundError('email missing for user')
+ }
+
+ await OneTimeTokenHandler.promises.expireToken(TOKEN_USE, token)
+ await UserUpdater.promises.confirmEmail(userId, email)
+
+ return { userId, email }
+}
+
const UserEmailsConfirmationHandler = {
sendConfirmationEmail,
sendReconfirmationEmail: callbackify(sendReconfirmationEmail),
- confirmEmailFromToken(token, callback) {
- OneTimeTokenHandler.getValueFromTokenAndExpire(
- TOKEN_USE,
- token,
- function (error, data) {
- if (error) {
- return callback(error)
- }
- if (!data) {
- return callback(new Errors.NotFoundError('no token found'))
- }
- const userId = data.user_id
- const email = data.email
-
- if (!userId || email !== EmailHelper.parseEmail(email)) {
- return callback(new Errors.NotFoundError('invalid data'))
- }
- UserGetter.getUser(userId, { emails: 1 }, function (error, user) {
- if (error) {
- return callback(error)
- }
- if (!user) {
- return callback(new Errors.NotFoundError('user not found'))
- }
- const emailExists = user.emails.some(
- emailData => emailData.email === email
- )
- if (!emailExists) {
- return callback(new Errors.NotFoundError('email missing for user'))
- }
- UserUpdater.confirmEmail(userId, email, function (error) {
- if (error) {
- return callback(error)
- }
- callback(null, { userId, email })
- })
- })
- }
- )
- },
+ confirmEmailFromToken: callbackify(confirmEmailFromToken),
}
UserEmailsConfirmationHandler.promises = {
sendConfirmationEmail: promisify(sendConfirmationEmail),
- confirmEmailFromToken: promisify(
- UserEmailsConfirmationHandler.confirmEmailFromToken
- ),
+ confirmEmailFromToken,
sendConfirmationCode,
}
diff --git a/services/web/app/src/Features/User/UserEmailsController.js b/services/web/app/src/Features/User/UserEmailsController.js
index 297e0f86ca..523f4b6ebe 100644
--- a/services/web/app/src/Features/User/UserEmailsController.js
+++ b/services/web/app/src/Features/User/UserEmailsController.js
@@ -156,6 +156,13 @@ async function primaryEmailCheck(req, res) {
AsyncFormHelper.redirect(req, res, '/project')
}
+async function showConfirm(req, res, next) {
+ res.render('user/confirm_email', {
+ token: req.query.token,
+ title: 'confirm_email',
+ })
+}
+
const UserEmailsController = {
list(req, res, next) {
const userId = SessionManager.getLoggedInUserId(req.session)
@@ -254,12 +261,7 @@ const UserEmailsController = {
primaryEmailCheck: expressify(primaryEmailCheck),
- showConfirm(req, res, next) {
- res.render('user/confirm_email', {
- token: req.query.token,
- title: 'confirm_email',
- })
- },
+ showConfirm: expressify(showConfirm),
confirm(req, res, next) {
const { token } = req.body
@@ -269,10 +271,18 @@ const UserEmailsController = {
})
}
UserEmailsConfirmationHandler.confirmEmailFromToken(
+ req,
token,
function (error, userData) {
if (error) {
- if (error instanceof Errors.NotFoundError) {
+ if (error instanceof Errors.ForbiddenError) {
+ res.status(403).json({
+ message: {
+ key: 'confirm-email-wrong-user',
+ text: `We can’t confirm this email. You must be logged in with the Overleaf account that requested the new secondary email.`,
+ },
+ })
+ } else if (error instanceof Errors.NotFoundError) {
res.status(404).json({
message: req.i18n.translate('confirmation_token_invalid'),
})
diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js
index ca59aa73ee..8704a5c169 100644
--- a/services/web/app/src/router.js
+++ b/services/web/app/src/router.js
@@ -303,9 +303,14 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
UserController.promises.ensureAffiliationMiddleware,
UserEmailsController.list
)
- webRouter.get('/user/emails/confirm', UserEmailsController.showConfirm)
+ webRouter.get(
+ '/user/emails/confirm',
+ AuthenticationController.requireLogin(),
+ UserEmailsController.showConfirm
+ )
webRouter.post(
'/user/emails/confirm',
+ AuthenticationController.requireLogin(),
RateLimiterMiddleware.rateLimit(rateLimiters.confirmEmail),
UserEmailsController.confirm
)
diff --git a/services/web/app/views/user/confirm_email.pug b/services/web/app/views/user/confirm_email.pug
index 9527785b26..d45b5b8dba 100644
--- a/services/web/app/views/user/confirm_email.pug
+++ b/services/web/app/views/user/confirm_email.pug
@@ -4,10 +4,17 @@ block content
main.content.content-alt#main-content
.container
.row
- .col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4
+ .col-md-8.col-md-offset-2.col-lg-6.col-lg-offset-3
.card
- .page-header
+ .page-header(data-ol-hide-on-error-message="confirm-email-wrong-user")
h1 #{translate("confirm_email")}
+ form(
+ method="POST"
+ action="/logout"
+ id="logoutForm"
+ )
+ input(type="hidden", name="_csrf", value=csrfToken)
+ input(type="hidden", name="redirect", value=currentUrlWithQueryParams)
form(
data-ol-async-form,
data-ol-auto-submit,
@@ -21,11 +28,20 @@ block content
div(data-ol-not-sent)
+formMessages()
+ div(data-ol-custom-form-message="confirm-email-wrong-user" hidden)
+ h1.h3 #{translate("we_cant_confirm_this_email")}
+ p !{translate("to_confirm_email_address_you_must_be_logged_in_with_the_requesting_account")}
+ p !{translate("you_are_currently_logged_in_as", {email: getUserEmail()})}
+ .actions
+ button.btn-primary.btn.btn-block(
+ form="logoutForm"
+ ) #{translate('log_in_with_a_different_account')}
.actions
button.btn-primary.btn.btn-block(
type='submit',
data-ol-disabled-inflight
+ data-ol-hide-on-error-message="confirm-email-wrong-user"
)
span(data-ol-inflight="idle")
| #{translate('confirm')}
diff --git a/services/web/locales/en.json b/services/web/locales/en.json
index ebb86054d7..834f6fba59 100644
--- a/services/web/locales/en.json
+++ b/services/web/locales/en.json
@@ -1029,6 +1029,7 @@
"log_in_first_to_proceed": "You will need to log in first to proceed.",
"log_in_now": "Log in now",
"log_in_with": "Log in with __provider__",
+ "log_in_with_a_different_account": "Log in with a different account",
"log_in_with_email": "Log in with __email__",
"log_in_with_existing_institution_email": "Please log in with your existing __appName__ account in order to get your __appName__ and __institutionName__ institutional accounts linked.",
"log_in_with_primary_email_address": "This will be the email address to use if you log in with an email address and password. Important __appName__ notifications will be sent to this email address.",
@@ -1913,6 +1914,7 @@
"to_add_email_accounts_need_to_be_linked_2": "To add this email, your <0>__appName__ and <0>__institutionName__ accounts will need to be linked.",
"to_add_more_collaborators": "To add more collaborators or turn on link sharing, please ask the project owner",
"to_change_access_permissions": "To change access permissions, please ask the project owner",
+ "to_confirm_email_address_you_must_be_logged_in_with_the_requesting_account": "To confirm an email address, you must be logged in with the Overleaf account that requested the new secondary email.",
"to_confirm_transfer_enter_email_address": "To accept the invitation, enter the email address linked to your account.",
"to_confirm_unlink_all_users_enter_email": "To confirm you want to unlink all users, enter your email address:",
"to_fix_this_you_can": "To fix this, you can:",
@@ -2121,6 +2123,7 @@
"visual_editor": "Visual Editor",
"visual_editor_is_only_available_for_tex_files": "Visual Editor is only available for TeX files",
"want_change_to_apply_before_plan_end": "If you wish this change to apply before the end of your current billing period, please contact us.",
+ "we_cant_confirm_this_email": "We can’t confirm this email",
"we_cant_find_any_sections_or_subsections_in_this_file": "We can’t find any sections or subsections in this file",
"we_do_not_share_personal_information": "See our <0>Privacy Notice for details of how we treat your personal data",
"we_logged_you_in": "We have logged you in.",
@@ -2179,6 +2182,7 @@
"you_are_a_manager_of_commons_at_institution_x": "You are a <0>manager of the Overleaf Commons subscription at <0>__institutionName__",
"you_are_a_manager_of_publisher_x": "You are a <0>manager of <0>__publisherName__",
"you_are_a_manager_of_x_plan_as_member_of_group_subscription_y_administered_by_z": "You are a <1>manager of the <0>__planName__ group subscription <1>__groupName__ administered by <1>__adminEmail__",
+ "you_are_currently_logged_in_as": "You are currently logged in as __email__.",
"you_are_on_a_paid_plan_contact_support_to_find_out_more": "You’re on an __appName__ Paid plan. <0>Contact support to find out more.",
"you_are_on_x_plan_as_a_confirmed_member_of_institution_y": "You are on our <0>__planName__ plan as a <1>confirmed member of <1>__institutionName__",
"you_are_on_x_plan_as_member_of_group_subscription_y_administered_by_z": "You are on our <0>__planName__ plan as a <1>member of the group subscription <1>__groupName__ administered by <1>__adminEmail__",
diff --git a/services/web/test/unit/src/User/UserEmailsConfirmationHandlerTests.js b/services/web/test/unit/src/User/UserEmailsConfirmationHandlerTests.js
index 585a3cb149..f01a1c3250 100644
--- a/services/web/test/unit/src/User/UserEmailsConfirmationHandlerTests.js
+++ b/services/web/test/unit/src/User/UserEmailsConfirmationHandlerTests.js
@@ -24,20 +24,6 @@ const EmailHelper = require('../../../../app/src/Features/Helpers/EmailHelper')
describe('UserEmailsConfirmationHandler', function () {
beforeEach(function () {
- this.UserEmailsConfirmationHandler = SandboxedModule.require(modulePath, {
- requires: {
- '@overleaf/settings': (this.settings = {
- siteUrl: 'https://emails.example.com',
- }),
- '../Security/OneTimeTokenHandler': (this.OneTimeTokenHandler = {}),
- './UserUpdater': (this.UserUpdater = {}),
- './UserGetter': (this.UserGetter = {
- getUser: sinon.stub().yields(null, this.mockUser),
- }),
- '../Email/EmailHandler': (this.EmailHandler = {}),
- '../Helpers/EmailHelper': EmailHelper,
- },
- })
this.mockUser = {
_id: 'mock-user-id',
email: 'mock@example.com',
@@ -45,6 +31,31 @@ describe('UserEmailsConfirmationHandler', function () {
}
this.user_id = this.mockUser._id
this.email = this.mockUser.email
+ this.req = {}
+ this.UserEmailsConfirmationHandler = SandboxedModule.require(modulePath, {
+ requires: {
+ '@overleaf/settings': (this.settings = {
+ siteUrl: 'https://emails.example.com',
+ }),
+ '../Security/OneTimeTokenHandler': (this.OneTimeTokenHandler = {
+ promises: {},
+ }),
+ './UserUpdater': (this.UserUpdater = {
+ promises: {},
+ }),
+ './UserGetter': (this.UserGetter = {
+ getUser: sinon.stub().yields(null, this.mockUser),
+ promises: {
+ getUser: sinon.stub().resolves(this.mockUser),
+ },
+ }),
+ '../Email/EmailHandler': (this.EmailHandler = {}),
+ '../Helpers/EmailHelper': EmailHelper,
+ '../Authentication/SessionManager': (this.SessionManager = {
+ getLoggedInUserId: sinon.stub().returns(this.mockUser._id),
+ }),
+ },
+ })
return (this.callback = sinon.stub())
})
@@ -127,122 +138,139 @@ describe('UserEmailsConfirmationHandler', function () {
describe('confirmEmailFromToken', function () {
beforeEach(function () {
- this.OneTimeTokenHandler.getValueFromTokenAndExpire = sinon
+ this.OneTimeTokenHandler.promises.peekValueFromToken = sinon
.stub()
- .yields(null, { user_id: this.user_id, email: this.email })
- return (this.UserUpdater.confirmEmail = sinon.stub().yields())
+ .resolves({ data: { user_id: this.user_id, email: this.email } })
+ this.OneTimeTokenHandler.promises.expireToken = sinon.stub().resolves()
+ this.UserUpdater.promises.confirmEmail = sinon.stub().resolves()
})
describe('successfully', function () {
- beforeEach(function () {
- return this.UserEmailsConfirmationHandler.confirmEmailFromToken(
- (this.token = 'mock-token'),
- this.callback
+ beforeEach(async function () {
+ await this.UserEmailsConfirmationHandler.promises.confirmEmailFromToken(
+ this.req,
+ (this.token = 'mock-token')
)
})
- it('should call getValueFromTokenAndExpire', function () {
- return this.OneTimeTokenHandler.getValueFromTokenAndExpire
+ it('should call peekValueFromToken', function () {
+ return this.OneTimeTokenHandler.promises.peekValueFromToken
+ .calledWith('email_confirmation', this.token)
+ .should.equal(true)
+ })
+
+ it('should call expireToken', function () {
+ return this.OneTimeTokenHandler.promises.expireToken
.calledWith('email_confirmation', this.token)
.should.equal(true)
})
it('should confirm the email of the user_id', function () {
- return this.UserUpdater.confirmEmail
+ return this.UserUpdater.promises.confirmEmail
.calledWith(this.user_id, this.email)
.should.equal(true)
})
-
- it('should call the callback', function () {
- return this.callback.called.should.equal(true)
- })
})
describe('with an expired token', function () {
beforeEach(function () {
- this.OneTimeTokenHandler.getValueFromTokenAndExpire = sinon
+ this.OneTimeTokenHandler.promises.peekValueFromToken = sinon
.stub()
- .yields(null, null)
- return this.UserEmailsConfirmationHandler.confirmEmailFromToken(
- (this.token = 'mock-token'),
- this.callback
- )
+ .rejects(new Errors.NotFoundError('no token found'))
})
- it('should call the callback with a NotFoundError', function () {
- return this.callback
- .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
- .should.equal(true)
+ it('should reject with a NotFoundError', async function () {
+ await expect(
+ this.UserEmailsConfirmationHandler.promises.confirmEmailFromToken(
+ this.req,
+ (this.token = 'mock-token')
+ )
+ ).to.be.rejectedWith(Errors.NotFoundError)
})
})
describe('with no user_id in the token', function () {
beforeEach(function () {
- this.OneTimeTokenHandler.getValueFromTokenAndExpire = sinon
+ this.OneTimeTokenHandler.promises.peekValueFromToken = sinon
.stub()
- .yields(null, { email: this.email })
- return this.UserEmailsConfirmationHandler.confirmEmailFromToken(
- (this.token = 'mock-token'),
- this.callback
- )
+ .resolves({ data: { email: this.email } })
})
- it('should call the callback with a NotFoundError', function () {
- return this.callback
- .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
- .should.equal(true)
+ it('should reject with a NotFoundError', async function () {
+ await expect(
+ this.UserEmailsConfirmationHandler.promises.confirmEmailFromToken(
+ this.req,
+ (this.token = 'mock-token')
+ )
+ ).to.be.rejectedWith(Errors.NotFoundError)
})
})
describe('with no email in the token', function () {
beforeEach(function () {
- this.OneTimeTokenHandler.getValueFromTokenAndExpire = sinon
+ this.OneTimeTokenHandler.promises.peekValueFromToken = sinon
.stub()
- .yields(null, { user_id: this.user_id })
- return this.UserEmailsConfirmationHandler.confirmEmailFromToken(
- (this.token = 'mock-token'),
- this.callback
- )
+ .resolves({ data: { user_id: this.user_id } })
})
- it('should call the callback with a NotFoundError', function () {
- return this.callback
- .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
- .should.equal(true)
+ it('should reject with a NotFoundError', async function () {
+ await expect(
+ this.UserEmailsConfirmationHandler.promises.confirmEmailFromToken(
+ this.req,
+ (this.token = 'mock-token')
+ )
+ ).to.be.rejectedWith(Errors.NotFoundError)
})
})
describe('with no user found', function () {
beforeEach(function () {
- this.UserGetter.getUser.yields(null, null)
- return this.UserEmailsConfirmationHandler.confirmEmailFromToken(
- (this.token = 'mock-token'),
- this.callback
- )
+ this.UserGetter.promises.getUser.resolves(null)
})
- it('should call the callback with a NotFoundError', function () {
- return this.callback
- .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
- .should.equal(true)
+ it('should reject with a NotFoundError', async function () {
+ await expect(
+ this.UserEmailsConfirmationHandler.promises.confirmEmailFromToken(
+ this.req,
+ (this.token = 'mock-token')
+ )
+ ).to.be.rejectedWith(Errors.NotFoundError)
})
})
describe('with secondary email missing on user', function () {
beforeEach(function () {
- this.OneTimeTokenHandler.getValueFromTokenAndExpire = sinon
+ this.OneTimeTokenHandler.promises.peekValueFromToken = sinon
.stub()
- .yields(null, { user_id: this.user_id, email: 'deleted@email.com' })
- return this.UserEmailsConfirmationHandler.confirmEmailFromToken(
- (this.token = 'mock-token'),
- this.callback
- )
+ .resolves({
+ data: { user_id: this.user_id, email: 'deleted@email.com' },
+ })
})
- it('should call the callback with a NotFoundError', function () {
- return this.callback
- .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
- .should.equal(true)
+ it('should reject with a NotFoundError', async function () {
+ await expect(
+ this.UserEmailsConfirmationHandler.promises.confirmEmailFromToken(
+ this.req,
+ (this.token = 'mock-token')
+ )
+ ).to.be.rejectedWith(Errors.NotFoundError)
+ })
+ })
+
+ describe('when the logged in user is not the token user', function () {
+ beforeEach(function () {
+ this.SessionManager.getLoggedInUserId = sinon
+ .stub()
+ .returns('other-user-id')
+ })
+
+ it('should reject with a ForbiddenError', async function () {
+ await expect(
+ this.UserEmailsConfirmationHandler.promises.confirmEmailFromToken(
+ this.req,
+ (this.token = 'mock-token')
+ )
+ ).to.be.rejectedWith(Errors.ForbiddenError)
})
})
})
diff --git a/services/web/test/unit/src/User/UserEmailsControllerTests.js b/services/web/test/unit/src/User/UserEmailsControllerTests.js
index b16f91498a..cf81d6b4ff 100644
--- a/services/web/test/unit/src/User/UserEmailsControllerTests.js
+++ b/services/web/test/unit/src/User/UserEmailsControllerTests.js
@@ -440,7 +440,7 @@ describe('UserEmailsController', function () {
it('should confirm the email from the token', function () {
this.UserEmailsConfirmationHandler.confirmEmailFromToken
- .calledWith(this.token)
+ .calledWith(this.req, this.token)
.should.equal(true)
})