diff --git a/services/web/app/src/Features/PasswordReset/PasswordResetController.js b/services/web/app/src/Features/PasswordReset/PasswordResetController.js index 0b477926b4..739217b997 100644 --- a/services/web/app/src/Features/PasswordReset/PasswordResetController.js +++ b/services/web/app/src/Features/PasswordReset/PasswordResetController.js @@ -165,8 +165,9 @@ module.exports = { if (req.query.passwordResetToken != null) { return PasswordResetHandler.getUserForPasswordResetToken( req.query.passwordResetToken, - (err, user, remainingUses) => { - if (err || !user || remainingUses <= 0) { + (err, result) => { + const { user, remainingPeeks } = result || {} + if (err || !user || remainingPeeks <= 0) { return res.redirect('/user/password/reset?error=token_expired') } req.session.resetToken = req.query.passwordResetToken diff --git a/services/web/app/src/Features/PasswordReset/PasswordResetHandler.js b/services/web/app/src/Features/PasswordReset/PasswordResetHandler.js index 669e955247..07c59a2f0e 100644 --- a/services/web/app/src/Features/PasswordReset/PasswordResetHandler.js +++ b/services/web/app/src/Features/PasswordReset/PasswordResetHandler.js @@ -46,50 +46,50 @@ function expirePasswordResetToken(token, callback) { }) } -function getUserForPasswordResetToken(token, callback) { - OneTimeTokenHandler.peekValueFromToken('password', token, (err, result) => { - const { data, remainingPeeks } = result || {} - if (err != null) { - if (err.name === 'NotFoundError') { - return callback(null, null) - } else { - return callback(err) - } - } - if (data == null || data.email == null) { - return callback(null, null, remainingPeeks) - } - UserGetter.getUserByMainEmail( - data.email, - { _id: 1, 'overleaf.id': 1, email: 1 }, - (err, user) => { - if (err != null) { - callback(err) - } else if (user == null) { - callback(null, null, 0) - } else if ( - data.user_id != null && - data.user_id === user._id.toString() - ) { - callback(null, user, remainingPeeks) - } else if ( - data.v1_user_id != null && - user.overleaf != null && - data.v1_user_id === user.overleaf.id - ) { - callback(null, user, remainingPeeks) - } else { - callback(null, null, 0) - } - } +async function getUserForPasswordResetToken(token) { + let result + try { + result = await OneTimeTokenHandler.promises.peekValueFromToken( + 'password', + token ) + } catch (err) { + if (err.name === 'NotFoundError') { + return + } else { + throw err + } + } + const { data, remainingPeeks } = result || {} + + if (data == null || data.email == null) { + return { user: null, remainingPeeks } + } + + const user = await UserGetter.promises.getUserByMainEmail(data.email, { + _id: 1, + 'overleaf.id': 1, + email: 1, }) + if (user == null) { + return { user: null, remainingPeeks: 0 } + } else if (data.user_id != null && data.user_id === user._id.toString()) { + return { user, remainingPeeks } + } else if ( + data.v1_user_id != null && + user.overleaf != null && + data.v1_user_id === user.overleaf.id + ) { + return { user, remainingPeeks } + } else { + return { user: null, remainingPeeks: 0 } + } } async function setNewUserPassword(token, password, auditLog) { - const user = await PasswordResetHandler.promises.getUserForPasswordResetToken( - token - ) + const result = + await PasswordResetHandler.promises.getUserForPasswordResetToken(token) + const { user } = result || {} if (!user) { return { @@ -98,7 +98,6 @@ async function setNewUserPassword(token, password, auditLog) { userId: null, } } - await UserAuditLogHandler.promises.addEntry( user._id, 'reset-password', @@ -122,16 +121,14 @@ const PasswordResetHandler = { setNewUserPassword: callbackify(setNewUserPassword), - getUserForPasswordResetToken, + getUserForPasswordResetToken: callbackify(getUserForPasswordResetToken), expirePasswordResetToken, } PasswordResetHandler.promises = { generateAndEmailResetToken, - getUserForPasswordResetToken: promisify( - PasswordResetHandler.getUserForPasswordResetToken - ), + getUserForPasswordResetToken, expirePasswordResetToken: promisify( PasswordResetHandler.expirePasswordResetToken ), diff --git a/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js b/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js index f953775e37..24a57ff5dd 100644 --- a/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js +++ b/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js @@ -43,7 +43,7 @@ describe('PasswordResetController', function () { getUserForPasswordResetToken: sinon .stub() .withArgs(this.token) - .yields(null, { _id: this.user_id }, 1), + .yields(null, { user: { _id: this.user_id }, remainingPeeks: 1 }), } this.UserSessionsManager = { promises: { @@ -377,7 +377,7 @@ describe('PasswordResetController', function () { this.PasswordResetHandler.getUserForPasswordResetToken = sinon .stub() .withArgs(this.token) - .yields(null, { _id: this.user_id }, 0) + .yields(null, { user: { _id: this.user_id }, remainingPeeks: 0 }) }) it('should redirect to the reset request page with an error message', function (done) { diff --git a/services/web/test/unit/src/PasswordReset/PasswordResetHandlerTests.js b/services/web/test/unit/src/PasswordReset/PasswordResetHandlerTests.js index 8e1b84838e..0a4801b188 100644 --- a/services/web/test/unit/src/PasswordReset/PasswordResetHandlerTests.js +++ b/services/web/test/unit/src/PasswordReset/PasswordResetHandlerTests.js @@ -13,6 +13,7 @@ describe('PasswordResetHandler', function () { this.OneTimeTokenHandler = { promises: { getNewToken: sinon.stub(), + peekValueFromToken: sinon.stub(), }, peekValueFromToken: sinon.stub(), expireToken: sinon.stub(), @@ -22,6 +23,7 @@ describe('PasswordResetHandler', function () { getUser: sinon.stub(), promises: { getUserByAnyEmail: sinon.stub(), + getUserByMainEmail: sinon.stub(), }, } this.EmailHandler = { promises: { sendEmail: sinon.stub() } } @@ -195,7 +197,7 @@ describe('PasswordResetHandler', function () { }) describe('when no data is found', function () { beforeEach(function () { - this.OneTimeTokenHandler.peekValueFromToken.yields(null, null) + this.OneTimeTokenHandler.promises.peekValueFromToken.resolves(null) }) it('should return found == false and reset == false', function () { @@ -217,14 +219,12 @@ describe('PasswordResetHandler', function () { describe('when the token has a user_id and email', function () { beforeEach(function () { - this.OneTimeTokenHandler.peekValueFromToken - .withArgs('password', this.token) - .yields(null, { - data: { - user_id: this.user._id, - email: this.email, - }, - }) + this.OneTimeTokenHandler.promises.peekValueFromToken.resolves({ + data: { + user_id: this.user._id, + email: this.email, + }, + }) this.AuthenticationManager.promises.setUserPassword .withArgs(this.user, this.password) .resolves(true) @@ -285,7 +285,7 @@ describe('PasswordResetHandler', function () { describe('when the email and user match', function () { describe('success', function () { beforeEach(function () { - this.UserGetter.getUserByMainEmail.yields(null, this.user) + this.UserGetter.promises.getUserByMainEmail.resolves(this.user) this.OneTimeTokenHandler.expireToken = sinon .stub() .callsArgWith(2, null) @@ -370,7 +370,7 @@ describe('PasswordResetHandler', function () { describe('via setUserPassword', function () { beforeEach(function () { this.PasswordResetHandler.promises.getUserForPasswordResetToken = - sinon.stub().withArgs(this.token).resolves(this.user) + sinon.stub().withArgs(this.token).resolves({ user: this.user }) this.AuthenticationManager.promises.setUserPassword .withArgs(this.user, this.password) .rejects() @@ -394,7 +394,7 @@ describe('PasswordResetHandler', function () { describe('via UserAuditLogHandler', function () { beforeEach(function () { this.PasswordResetHandler.promises.getUserForPasswordResetToken = - sinon.stub().withArgs(this.token).resolves(this.user) + sinon.stub().withArgs(this.token).resolves({ user: this.user }) this.UserAuditLogHandler.promises.addEntry.rejects( new Error('oops') ) @@ -423,14 +423,12 @@ describe('PasswordResetHandler', function () { describe('when the token has a v1_user_id and email', function () { beforeEach(function () { this.user.overleaf = { id: 184 } - this.OneTimeTokenHandler.peekValueFromToken - .withArgs('password', this.token) - .yields(null, { - data: { - v1_user_id: this.user.overleaf.id, - email: this.email, - }, - }) + this.OneTimeTokenHandler.promises.peekValueFromToken.resolves({ + data: { + v1_user_id: this.user.overleaf.id, + email: this.email, + }, + }) this.AuthenticationManager.promises.setUserPassword .withArgs(this.user, this.password) .resolves(true) @@ -493,9 +491,7 @@ describe('PasswordResetHandler', function () { describe('when the email and user match', function () { beforeEach(function () { - this.UserGetter.getUserByMainEmail - .withArgs(this.email) - .yields(null, this.user) + this.UserGetter.promises.getUserByMainEmail.resolves(this.user) }) it('should return reset == true and the user id', function (done) {