Merge branch 'release/16.6' into release/17.0

app/controllers/concerns/accounts/user_password_change.rb

@@ -42,8 +42,15 @@ module Accounts::UserPasswordChange
     # auth sources in the admin UI, so this shouldn't normally happen.
     return if redirect_if_password_change_not_allowed(user)
 
+    # Check if user is locked due to too many failed attempts
+    if user.failed_too_many_recent_login_attempts?
+      flash_and_log_invalid_credentials(is_logged_in: !show_user_name)
+      return render_password_change(user, nil, show_user_name:)
+    end
+
     # Ensure the current password is validated
     unless user.check_password?(params[:password], update_legacy:)
+      user.log_failed_login
       flash_and_log_invalid_credentials(is_logged_in: !show_user_name)
       return render_password_change(user, nil, show_user_name:)
     end
@@ -53,6 +60,9 @@ module Accounts::UserPasswordChange
 
     # Yield the success to the caller
     if call.success?
+      # Reset failed login count on successful password change
+      User.reset_failed_login_count_for(user)
+
       response = yield call
 
       call.apply_flash_message!(flash)

spec/controllers/account_controller_spec.rb

@@ -515,6 +515,171 @@ RSpec.describe AccountController, :skip_2fa_stage do
         expect(response).to have_http_status :not_found
       end
     end
+
+    context "with brute force protection",
+            with_settings: { brute_force_block_minutes: 30, brute_force_block_after_failed_logins: 20 } do
+      shared_let(:user) { create(:user, login: "testuser", password: "ValidPass123!", password_confirmation: "ValidPass123!") }
+
+      describe "blocks password change attempts after too many failures" do
+        before do
+          user.update_columns(
+            failed_login_count: 20,
+            last_failed_login_on: 1.minute.ago
+          )
+
+          post :change_password,
+               params: {
+                 password_change_user: user.login,
+                 password: "ValidPass123!",
+                 new_password: "NewPass123!",
+                 new_password_confirmation: "NewPass123!"
+               }
+        end
+
+        it "blocks the attempt even with correct password" do
+          expect(response).to have_http_status :unprocessable_entity
+        end
+
+        it "does not change the password" do
+          user.reload
+          expect(user.check_password?("ValidPass123!")).to be true
+          expect(user.check_password?("NewPass123!")).to be false
+        end
+
+        it "shows an error message" do
+          if Setting.brute_force_block_after_failed_logins.to_i > 0
+            expected_message = I18n.t(:notice_account_invalid_credentials_or_blocked)
+          else
+            expected_message = I18n.t(:notice_account_invalid_credentials)
+          end
+          expect(flash[:error]).to eq(expected_message)
+        end
+      end
+
+      describe "logs failed password attempts" do
+        before do
+          user.update_columns(
+            failed_login_count: 0,
+            last_failed_login_on: nil
+          )
+
+          post :change_password,
+               params: {
+                 password_change_user: user.login,
+                 password: "WrongPassword!",
+                 new_password: "NewPass123!",
+                 new_password_confirmation: "NewPass123!"
+               }
+        end
+
+        it "increments failed login count" do
+          user.reload
+          expect(user.failed_login_count).to eq(1)
+        end
+
+        it "updates last failed login timestamp" do
+          user.reload
+          expect(user.last_failed_login_on).to be_within(1.second).of(Time.zone.now)
+        end
+
+        it "does not change the password" do
+          user.reload
+          expect(user.check_password?("ValidPass123!")).to be true
+          expect(user.check_password?("NewPass123!")).to be false
+        end
+      end
+
+      describe "accumulates multiple failed attempts" do
+        it "blocks after reaching the threshold" do
+          user.update_columns(
+            failed_login_count: 0,
+            last_failed_login_on: nil
+          )
+
+          # Make 20 failed attempts
+          20.times do
+            post :change_password,
+                 params: {
+                   password_change_user: user.login,
+                   password: "WrongPassword!",
+                   new_password: "NewPass123!",
+                   new_password_confirmation: "NewPass123!"
+                 }
+          end
+
+          user.reload
+          expect(user.failed_login_count).to eq(20)
+
+          # Next attempt should be blocked even with correct password
+          post :change_password,
+               params: {
+                 password_change_user: user.login,
+                 password: "ValidPass123!",
+                 new_password: "NewPass123!",
+                 new_password_confirmation: "NewPass123!"
+               }
+
+          user.reload
+          expect(user.check_password?("ValidPass123!")).to be true
+          expect(user.check_password?("NewPass123!")).to be false
+        end
+      end
+
+      describe "resets failed login count on successful password change" do
+        before do
+          user.update_columns(
+            failed_login_count: 5,
+            last_failed_login_on: 1.minute.ago
+          )
+
+          post :change_password,
+               params: {
+                 password_change_user: user.login,
+                 password: "ValidPass123!",
+                 new_password: "NewPass123!",
+                 new_password_confirmation: "NewPass123!"
+               }
+        end
+
+        it "resets the failed login count to zero" do
+          user.reload
+          expect(user.failed_login_count).to eq(0)
+        end
+
+        it "changes the password successfully" do
+          user.reload
+          expect(user.check_password?("NewPass123!")).to be true
+          expect(user.check_password?("ValidPass123!")).to be false
+        end
+      end
+
+      describe "allows password change after block time expires" do
+        before do
+          user.update_columns(
+            failed_login_count: 20,
+            last_failed_login_on: 31.minutes.ago
+          )
+
+          post :change_password,
+               params: {
+                 password_change_user: user.login,
+                 password: "ValidPass123!",
+                 new_password: "NewPass123!",
+                 new_password_confirmation: "NewPass123!"
+               }
+        end
+
+        it "allows the password change" do
+          user.reload
+          expect(user.check_password?("NewPass123!")).to be true
+        end
+
+        it "resets the failed login count" do
+          user.reload
+          expect(user.failed_login_count).to eq(0)
+        end
+      end
+    end
   end
 
   describe "POST #lost_password" do

spec/controllers/my_controller_spec.rb

@@ -120,6 +120,87 @@ RSpec.describe MyController do
         assert User.try_to_login(user.login, "adminADMIN!New")
       end
     end
+
+    describe "with brute force protection",
+             with_settings: { brute_force_block_minutes: 30, brute_force_block_after_failed_logins: 20 } do
+      describe "blocks password change attempts after too many failures" do
+        before do
+          user.update_columns(
+            failed_login_count: 20,
+            last_failed_login_on: 1.minute.ago
+          )
+
+          post :change_password,
+               params: {
+                 password: "adminADMIN!",
+                 new_password: "adminADMIN!New",
+                 new_password_confirmation: "adminADMIN!New"
+               }
+        end
+
+        it "blocks the attempt even with correct password" do
+          expect(response).to have_http_status :unprocessable_entity
+        end
+
+        it "does not change the password" do
+          user.reload
+          expect(user.check_password?("adminADMIN!")).to be true
+          expect(user.check_password?("adminADMIN!New")).to be false
+        end
+      end
+
+      describe "logs failed password attempts" do
+        before do
+          user.update_columns(
+            failed_login_count: 0,
+            last_failed_login_on: nil
+          )
+
+          post :change_password,
+               params: {
+                 password: "WrongPassword!",
+                 new_password: "adminADMIN!New",
+                 new_password_confirmation: "adminADMIN!New"
+               }
+        end
+
+        it "increments failed login count" do
+          user.reload
+          expect(user.failed_login_count).to eq(1)
+        end
+
+        it "updates last failed login timestamp" do
+          user.reload
+          expect(user.last_failed_login_on).to be_within(1.second).of(Time.zone.now)
+        end
+      end
+
+      describe "resets failed login count on successful password change" do
+        before do
+          user.update_columns(
+            failed_login_count: 5,
+            last_failed_login_on: 1.minute.ago
+          )
+
+          post :change_password,
+               params: {
+                 password: "adminADMIN!",
+                 new_password: "adminADMIN!New",
+                 new_password_confirmation: "adminADMIN!New"
+               }
+        end
+
+        it "resets the failed login count to zero" do
+          user.reload
+          expect(user.failed_login_count).to eq(0)
+        end
+
+        it "changes the password successfully" do
+          user.reload
+          expect(user.check_password?("adminADMIN!New")).to be true
+        end
+      end
+    end
   end
 
   describe "account" do