diff --git a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 b/app/assets/javascripts/discourse/controllers/password-reset.js.es6 index b012e29ed01..2caf6e6b708 100644 --- a/app/assets/javascripts/discourse/controllers/password-reset.js.es6 +++ b/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); + } }); }, diff --git a/app/assets/javascripts/discourse/templates/password-reset.hbs b/app/assets/javascripts/discourse/templates/password-reset.hbs index 660c370a94b..5a762a985ae 100644 --- a/app/assets/javascripts/discourse/templates/password-reset.hbs +++ b/app/assets/javascripts/discourse/templates/password-reset.hbs @@ -18,7 +18,15 @@
{{#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}} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f3c837bf30a..2153f47b78f 100644 --- a/app/controllers/users_controller.rb +++ b/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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 918e12d8b0f..7bf0b3e45ba 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -771,6 +771,7 @@ en: enable: "Enable two factor authentication 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 (AndroidiOS and enter your authentication code. disable_description: "Please enter the authentication code from your app" diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 67c96d6bde3..106cf80e932 100644 --- a/spec/requests/users_controller_spec.rb +++ b/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) }