Merge pull request #17548 from overleaf/mf-promisify-render-set-password-form

[web] promisify renderSetPasswordForm

GitOrigin-RevId: 3a79a7fd23de2d7ff87a833204298aed6cc303a5

services/web/app/src/Features/PasswordReset/PasswordResetController.js

@@ -30,7 +30,6 @@ async function setNewUserPassword(req, res, next) {
   }
 
   passwordResetToken = passwordResetToken.trim()
-  delete req.session.resetToken
 
   const initiatorId = SessionManager.getLoggedInUserId(req.session)
   // password reset via tokens can be done while logged in, or not
@@ -145,6 +144,51 @@ async function requestReset(req, res, next) {
   }
 }
 
+async function renderSetPasswordForm(req, res, next) {
+  if (req.query.passwordResetToken != null) {
+    try {
+      const result =
+        await PasswordResetHandler.promises.getUserForPasswordResetToken(
+          req.query.passwordResetToken
+        )
+
+      const { user, remainingPeeks } = result || {}
+      if (!user || remainingPeeks <= 0) {
+        return res.redirect('/user/password/reset?error=token_expired')
+      }
+      req.session.resetToken = req.query.passwordResetToken
+      let emailQuery = ''
+
+      if (typeof req.query.email === 'string') {
+        const email = EmailsHelper.parseEmail(req.query.email)
+        if (email) {
+          emailQuery = `?email=${encodeURIComponent(email)}`
+        }
+      }
+
+      return res.redirect('/user/password/set' + emailQuery)
+    } catch (err) {
+      return res.redirect('/user/password/reset?error=token_expired')
+    }
+  }
+
+  if (req.session.resetToken == null) {
+    return res.redirect('/user/password/reset')
+  }
+
+  const email = EmailsHelper.parseEmail(req.query.email)
+
+  // clean up to avoid leaking the token in the session object
+  const passwordResetToken = req.session.resetToken
+  delete req.session.resetToken
+
+  res.render('user/setPassword', {
+    title: 'set_password',
+    email,
+    passwordResetToken,
+  })
+}
+
 module.exports = {
   renderRequestResetForm(req, res) {
     const errorQuery = req.query.error
@@ -159,38 +203,6 @@ module.exports = {
   },
 
   requestReset: expressify(requestReset),
-
-  renderSetPasswordForm(req, res) {
-    if (req.query.passwordResetToken != null) {
-      return PasswordResetHandler.getUserForPasswordResetToken(
-        req.query.passwordResetToken,
-        (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
-          let emailQuery = ''
-          if (typeof req.query.email === 'string') {
-            const email = EmailsHelper.parseEmail(req.query.email)
-            if (email) {
-              emailQuery = `?email=${encodeURIComponent(email)}`
-            }
-          }
-          return res.redirect('/user/password/set' + emailQuery)
-        }
-      )
-    }
-    if (req.session.resetToken == null) {
-      return res.redirect('/user/password/reset')
-    }
-    const email = EmailsHelper.parseEmail(req.query.email)
-    res.render('user/setPassword', {
-      title: 'set_password',
-      email,
-      passwordResetToken: req.session.resetToken,
-    })
-  },
-
+  renderSetPasswordForm: expressify(renderSetPasswordForm),
   setNewUserPassword: expressify(setNewUserPassword),
 }

services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js

@@ -39,11 +39,14 @@ describe('PasswordResetController', function () {
         setNewUserPassword: sinon
           .stub()
           .resolves({ found: true, reset: true, userID: this.user_id }),
+        getUserForPasswordResetToken: sinon
+          .stub()
+          .withArgs(this.token)
+          .resolves({
+            user: { _id: this.user_id },
+            remainingPeeks: 1,
+          }),
       },
-      getUserForPasswordResetToken: sinon
-        .stub()
-        .withArgs(this.token)
-        .yields(null, { user: { _id: this.user_id }, remainingPeeks: 1 }),
     }
     this.UserSessionsManager = {
       promises: {
@@ -259,15 +262,6 @@ describe('PasswordResetController', function () {
       this.PasswordResetController.setNewUserPassword(this.req, this.res)
     })
 
-    it('should clear the session.resetToken', function (done) {
-      this.res.sendStatus = code => {
-        code.should.equal(200)
-        this.req.session.should.not.have.property('resetToken')
-        done()
-      }
-      this.PasswordResetController.setNewUserPassword(this.req, this.res)
-    })
-
     it('should clear sessions', function (done) {
       this.res.sendStatus = code => {
         this.UserSessionsManager.promises.revokeAllUserSessions.callCount.should.equal(
@@ -374,10 +368,10 @@ describe('PasswordResetController', function () {
     describe('with expired token in query', function () {
       beforeEach(function () {
         this.req.query.passwordResetToken = this.token
-        this.PasswordResetHandler.getUserForPasswordResetToken = sinon
+        this.PasswordResetHandler.promises.getUserForPasswordResetToken = sinon
           .stub()
           .withArgs(this.token)
-          .yields(null, { user: { _id: this.user_id }, remainingPeeks: 0 })
+          .resolves({ user: { _id: this.user_id }, remainingPeeks: 0 })
       })
 
       it('should redirect to the reset request page with an error message', function (done) {
@@ -452,7 +446,15 @@ describe('PasswordResetController', function () {
 
         it('should render the page, passing the reset token', function (done) {
           this.res.render = (templatePath, options) => {
-            options.passwordResetToken.should.equal(this.req.session.resetToken)
+            options.passwordResetToken.should.equal(this.token)
+            done()
+          }
+          this.PasswordResetController.renderSetPasswordForm(this.req, this.res)
+        })
+
+        it('should clear the req.session.resetToken', function (done) {
+          this.res.render = (templatePath, options) => {
+            this.req.session.should.not.have.property('resetToken')
             done()
           }
           this.PasswordResetController.renderSetPasswordForm(this.req, this.res)