UX: show error when hitting the rate limit on password reset

app/assets/javascripts/discourse/controllers/password-reset.js.es6

@@ -41,8 +41,7 @@ export default Ember.Controller.extend(PasswordValidation, {
           second_factor_token: this.get("secondFactor"),
           second_factor_method: this.get("secondFactorMethod")
         }
-      })
-        .then(result => {
+      }).then(result => {
           if (result.success) {
             this.set("successMessage", result.message);
             this.set("redirectTo", result.redirect_to);
@@ -83,8 +82,12 @@ export default Ember.Controller.extend(PasswordValidation, {
             }
           }
         })
-        .catch(error => {
-          throw new Error(error);
+        .catch(e => {
+          if (e.jqXHR && e.jqXHR.status === 429) {
+            this.set("errorMessage", I18n.t("user.second_factor.rate_limit"));
+          } else {
+            throw new Error(e);
+          }
         });
     },
 

app/assets/javascripts/discourse/templates/password-reset.hbs

@@ -18,7 +18,15 @@
       <form>
         {{#if secondFactorRequired}}
           {{#second-factor-form secondFactorMethod=secondFactorMethod backupEnabled=backupEnabled}}
-            {{text-field value=secondFactor id="second-factor" autocorrect="off" autocapitalize="off" autofocus="autofocus" secondFactorMethod=secondFactorMethod}}
+            {{text-field
+                id="second-factor"
+                value=secondFactor
+                autocorrect="off"
+                autocapitalize="off"
+                autofocus="autofocus"
+                maxlength="6"
+                secondFactorMethod=secondFactorMethod
+            }}
           {{/second-factor-form}}
           {{d-button action="submit" class='btn-primary' label='submit'}}
         {{else}}

app/controllers/users_controller.rb

@@ -453,12 +453,11 @@ class UsersController < ApplicationController
     token = params[:token]
 
     if EmailToken.valid_token_format?(token)
-      @user =
-        if request.put?
-          EmailToken.confirm(token)
-        else
-          EmailToken.confirmable(token)&.user
-        end
+      @user = if request.put?
+        EmailToken.confirm(token)
+      else
+        EmailToken.confirmable(token)&.user
+      end
 
       if @user
         secure_session["password-#{token}"] = @user.id
@@ -468,9 +467,15 @@ class UsersController < ApplicationController
       end
     end
 
-    totp_enabled = @user&.totp_enabled?
+    second_factor_token = params[:second_factor_token]
+    second_factor_method = params[:second_factor_method].to_i
 
-    if !totp_enabled || @user.authenticate_second_factor(params[:second_factor_token], params[:second_factor_method].to_i)
+    if second_factor_token.present? && second_factor_token[/\d{6}/] && UserSecondFactor.methods[second_factor_method]
+      RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
+      second_factor_authenticated = @user&.authenticate_second_factor(second_factor_token, second_factor_method)
+    end
+
+    if second_factor_authenticated || !@user&.totp_enabled?
       secure_session["second-factor-#{token}"] = "true"
     end
 
@@ -479,13 +484,10 @@ class UsersController < ApplicationController
     if !@user
       @error = I18n.t('password_reset.no_token')
     elsif request.put?
-      @invalid_password = params[:password].blank? || params[:password].length > User.max_password_length
-
       if !valid_second_factor
-        RateLimiter.new(nil, "second-factor-min-#{request.remote_ip}", 3, 1.minute).performed!
         @user.errors.add(:user_second_factors, :invalid)
         @error = I18n.t('login.invalid_second_factor_code')
-      elsif @invalid_password
+      elsif @invalid_password = params[:password].blank? || params[:password].size > User.max_password_length
         @user.errors.add(:password, :invalid)
       else
         @user.password = params[:password]
@@ -547,6 +549,8 @@ class UsersController < ApplicationController
         end
       end
     end
+  rescue RateLimiter::LimitExceeded => e
+    render_rate_limit_error(e)
   end
 
   def confirm_email_token

config/locales/client.en.yml

@@ -771,6 +771,7 @@ en:
         enable: "Enable <a href>two factor authentication</a> for enhanced account security"
         confirm_password_description: "Please confirm your password to continue"
         label: "Code"
+        rate_limit: "Please wait before trying another authentication code."
         enable_description: |
           Scan this QR code in a supported app (<a href="https://www.google.com/search?q=authenticator+apps+for+android" target="_blank">Android</a> – <a href="https://www.google.com/search?q=authenticator+apps+for+ios" target="_blank">iOS</a> and enter your authentication code.
         disable_description: "Please enter the authentication code from your app"

spec/requests/users_controller_spec.rb

@@ -250,6 +250,35 @@ describe UsersController do
         expect(UserAuthToken.where(id: user_token.id).count).to eq(1)
       end
 
+      context "rate limiting" do
+
+        before { RateLimiter.clear_all!; RateLimiter.enable }
+        after  { RateLimiter.disable }
+
+        it "rate limits reset passwords" do
+          freeze_time
+
+          token = user.email_tokens.create!(email: user.email).token
+
+          3.times do
+            put "/u/password-reset/#{token}", params: {
+              second_factor_token: 123456,
+              second_factor_method: 1
+            }
+
+            expect(response.status).to eq(200)
+          end
+
+          put "/u/password-reset/#{token}", params: {
+            second_factor_token: 123456,
+            second_factor_method: 1
+          }
+
+          expect(response.status).to eq(429)
+        end
+
+      end
+
       context '2 factor authentication required' do
         let!(:second_factor) { Fabricate(:user_second_factor_totp, user: user) }