Merge pull request #4327 from overleaf/jpa-pw-reset-captcha

[misc] add captcha on password reset requests

GitOrigin-RevId: 9a23b9c9dee2c56345e9c1846861c05c25126802

services/web/app/src/Features/Captcha/CaptchaMiddleware.js

@@ -24,11 +24,7 @@ module.exports = CaptchaMiddleware = {
       ) {
         return next()
       }
-      const inviteAndCaptchaDisabled =
-        action === 'invite' && Settings.recaptcha.disabled.invite
-      const registerAndCaptchaDisabled =
-        action === 'register' && Settings.recaptcha.disabled.register
-      if (inviteAndCaptchaDisabled || registerAndCaptchaDisabled) {
+      if (Settings.recaptcha.disabled[action]) {
         return next()
       }
       const response = req.body['g-recaptcha-response']

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

@@ -1,5 +1,4 @@
 const PasswordResetHandler = require('./PasswordResetHandler')
-const RateLimiter = require('../../infrastructure/RateLimiter')
 const AuthenticationController = require('../Authentication/AuthenticationController')
 const UserGetter = require('../User/UserGetter')
 const UserUpdater = require('../User/UserUpdater')
@@ -61,22 +60,10 @@ module.exports = {
   },
 
   requestReset(req, res, next) {
-    const email = req.body.email.trim().toLowerCase()
-    const opts = {
-      endpointName: 'password_reset_rate_limit',
-      timeInterval: 60,
-      subjectName: req.ip,
-      throttle: 6,
-    }
-    RateLimiter.addCount(opts, (err, canContinue) => {
-      if (err != null) {
-        return next(
-          new OError('rate-limit password reset failed').withCause(err)
-        )
-      }
-      if (!canContinue) {
-        return res.status(429).send({
-          message: req.i18n.translate('rate_limit_hit_wait'),
+    const email = EmailsHelper.parseEmail(req.body.email)
+    if (!email) {
+      return res.status(400).send({
+        message: req.i18n.translate('must_be_email_address'),
       })
     }
     PasswordResetHandler.generateAndEmailResetToken(email, (err, status) => {
@@ -99,7 +86,6 @@ module.exports = {
         })
       }
     })
-    })
   },
 
   renderSetPasswordForm(req, res) {

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

@@ -1,9 +1,18 @@
 const PasswordResetController = require('./PasswordResetController')
 const AuthenticationController = require('../Authentication/AuthenticationController')
+const CaptchaMiddleware = require('../../Features/Captcha/CaptchaMiddleware')
+const RateLimiterMiddleware = require('../Security/RateLimiterMiddleware')
 const { Joi, validate } = require('../../infrastructure/Validation')
 
 module.exports = {
   apply(webRouter, apiRouter) {
+    const rateLimit = RateLimiterMiddleware.rateLimit({
+      endpointName: 'password_reset_rate_limit',
+      ipOnly: true,
+      maxRequests: 6,
+      timeInterval: 60,
+    })
+
     webRouter.get(
       '/user/password/reset',
       PasswordResetController.renderRequestResetForm
@@ -15,6 +24,8 @@ module.exports = {
           email: Joi.string().required(),
         }),
       }),
+      rateLimit,
+      CaptchaMiddleware.validateCaptcha('passwordReset'),
       PasswordResetController.requestReset
     )
     AuthenticationController.addEndpointToLoginWhitelist('/user/password/reset')
@@ -31,6 +42,7 @@ module.exports = {
           passwordResetToken: Joi.string().required(),
         }),
       }),
+      rateLimit,
       PasswordResetController.setNewUserPassword
     )
     AuthenticationController.addEndpointToLoginWhitelist('/user/password/set')
@@ -42,6 +54,8 @@ module.exports = {
           email: Joi.string().required(),
         }),
       }),
+      rateLimit,
+      CaptchaMiddleware.validateCaptcha('passwordReset'),
       PasswordResetController.requestReset
     )
   },

services/web/app/views/user/passwordReset.pug

@@ -4,6 +4,18 @@ block vars
 	- metadata = { viewport: true }
 
 block content
+	- var showCaptcha = settings.recaptcha && settings.recaptcha.siteKey && !(settings.recaptcha.disabled && settings.recaptcha.disabled.passwordReset)
+
+	if showCaptcha
+		script(type="text/javascript", nonce=scriptNonce, src="https://www.recaptcha.net/recaptcha/api.js?render=explicit")
+		div(
+			id="recaptcha"
+			class="g-recaptcha"
+			data-sitekey=settings.recaptcha.siteKey
+			data-size="invisible"
+			data-badge="inline"
+		)
+
 	main.content.content-alt
 		.container
 			.row
@@ -17,6 +29,8 @@ block content
 							name="passwordResetForm"
 							action="/user/password/reset",
 							method="POST",
+							captcha=(showCaptcha ? '' : false),
+							captcha-action-name=(showCaptcha ? "passwordReset" : false),
 							ng-cloak
 						)
 							input(type="hidden", name="_csrf", value=csrfToken)

services/web/app/views/user/reconfirm.pug

@@ -2,6 +2,18 @@ extends ../layout
 
 block content
 	- var email = reconfirm_email ? reconfirm_email : ""
+	- var showCaptcha = settings.recaptcha && settings.recaptcha.siteKey && !(settings.recaptcha.disabled && settings.recaptcha.disabled.passwordReset)
+
+	if showCaptcha
+		script(type="text/javascript", nonce=scriptNonce, src="https://www.recaptcha.net/recaptcha/api.js?render=explicit")
+		div(
+			id="recaptcha"
+			class="g-recaptcha"
+			data-sitekey=settings.recaptcha.siteKey
+			data-size="invisible"
+			data-badge="inline"
+		)
+
 	main.content.content-alt
 		.container
 			.row
@@ -19,6 +31,8 @@ block content
 							ng-cloak
 							ng-init="email='"+email+"'"
 							aria-label=translate('request_reconfirmation_email')
+							captcha=(showCaptcha ? '' : false),
+							captcha-action-name=(showCaptcha ? "passwordReset" : false),
 						)
 							input(type="hidden", name="_csrf", value=csrfToken)
 							form-messages(for="reconfirmAccountForm" role="alert")

services/web/app/views/user/setPassword.pug

@@ -32,6 +32,8 @@ block content
 												| Request a new password reset email
 									.alert.alert-danger(ng-switch-when="400")
 											| #{translate('invalid_password')}
+									.alert.alert-danger(ng-switch-when="429")
+											| #{translate('rate_limit_hit_wait')}
 									.alert.alert-danger(ng-switch-default)
 											| #{translate('error_performing_request')}
 

services/web/config/settings.defaults.js

@@ -558,6 +558,7 @@ module.exports = {
     disabled: {
       invite: true,
       login: true,
+      passwordReset: true,
       register: true,
     },
   },

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

@@ -38,7 +38,6 @@ describe('PasswordResetController', function () {
           .resolves({ found: true, reset: true, userID: this.user_id }),
       },
     }
-    this.RateLimiter = { addCount: sinon.stub() }
     this.UserSessionsManager = {
       promises: {
         revokeAllUserSessions: sinon.stub().resolves(),
@@ -53,7 +52,6 @@ describe('PasswordResetController', function () {
       requires: {
         '@overleaf/settings': this.settings,
         './PasswordResetHandler': this.PasswordResetHandler,
-        '../../infrastructure/RateLimiter': this.RateLimiter,
         '../Authentication/AuthenticationController': (this.AuthenticationController = {
           getLoggedInUserId: sinon.stub(),
           finishLogin: sinon.stub(),
@@ -70,23 +68,7 @@ describe('PasswordResetController', function () {
   })
 
   describe('requestReset', function () {
-    it('should error if the rate limit is hit', function (done) {
-      this.PasswordResetHandler.generateAndEmailResetToken.callsArgWith(
-        1,
-        null,
-        'primary'
-      )
-      this.RateLimiter.addCount.callsArgWith(1, null, false)
-      this.PasswordResetController.requestReset(this.req, this.res)
-      this.PasswordResetHandler.generateAndEmailResetToken
-        .calledWith(this.email)
-        .should.equal(false)
-      this.res.statusCode.should.equal(429)
-      done()
-    })
-
     it('should tell the handler to process that email', function (done) {
-      this.RateLimiter.addCount.callsArgWith(1, null, true)
       this.PasswordResetHandler.generateAndEmailResetToken.callsArgWith(
         1,
         null,
@@ -101,7 +83,6 @@ describe('PasswordResetController', function () {
     })
 
     it('should send a 500 if there is an error', function (done) {
-      this.RateLimiter.addCount.callsArgWith(1, null, true)
       this.PasswordResetHandler.generateAndEmailResetToken.callsArgWith(
         1,
         new Error('error')
@@ -113,7 +94,6 @@ describe('PasswordResetController', function () {
     })
 
     it("should send a 404 if the email doesn't exist", function (done) {
-      this.RateLimiter.addCount.callsArgWith(1, null, true)
       this.PasswordResetHandler.generateAndEmailResetToken.callsArgWith(
         1,
         null,
@@ -125,7 +105,6 @@ describe('PasswordResetController', function () {
     })
 
     it('should send a 404 if the email is registered as a secondard email', function (done) {
-      this.RateLimiter.addCount.callsArgWith(1, null, true)
       this.PasswordResetHandler.generateAndEmailResetToken.callsArgWith(
         1,
         null,
@@ -139,7 +118,6 @@ describe('PasswordResetController', function () {
     it('should normalize the email address', function (done) {
       this.email = '  UPperCaseEMAILWithSpacesAround@example.Com '
       this.req.body.email = this.email
-      this.RateLimiter.addCount.callsArgWith(1, null, true)
       this.PasswordResetHandler.generateAndEmailResetToken.callsArgWith(
         1,
         null,