Apply account locking to password reset page

If an attacker has stolen a user's session, they could previously brute
force attack the user's password reset page.

This change applies the existing Devise account lock out feature. It
would lock the user account after 10 attempts.

The attacker/user would be logged out and unable to log back in for 10
minutes.

The administrator could unlock the account at any time.

Normally, the user is sent unlock instructions, however, I think in this
scenario we should assume that the attacker has been able to change the
user's email address. We suppress the email to the user.

https://gitlab.com/gitlab-org/gitlab/-/issues/339154

Changelog: security

app/controllers/profiles/passwords_controller.rb

@@ -47,6 +47,8 @@ class Profiles::PasswordsController < Profiles::ApplicationController
     password_attributes[:password_automatically_set] = false
 
     unless @user.password_automatically_set || @user.valid_password?(user_params[:current_password])
+      handle_invalid_current_password_attempt!
+
       redirect_to edit_profile_password_path, alert: _('You must provide a valid current password')
       return
     end
@@ -85,6 +87,12 @@ class Profiles::PasswordsController < Profiles::ApplicationController
     render_404 unless @user.allow_password_authentication?
   end
 
+  def handle_invalid_current_password_attempt!
+    Gitlab::AppLogger.info(message: 'Invalid current password when attempting to update user password', username: @user.username, ip: request.remote_ip)
+
+    @user.increment_failed_attempts!
+  end
+
   def user_params
     params.require(:user).permit(:current_password, :password, :password_confirmation)
   end

spec/features/profiles/password_spec.rb

@@ -78,40 +78,80 @@ RSpec.describe 'Profile > Password' do
     end
   end
 
-  context 'Change passowrd' do
+  context 'Change password' do
+    let(:new_password) { '22233344' }
+
     before do
       sign_in(user)
       visit(edit_profile_password_path)
     end
 
-    it 'does not change user passowrd without old one' do
-      page.within '.update-password' do
-        fill_passwords('22233344', '22233344')
+    shared_examples 'user enters an incorrect current password' do
+      subject do
+        page.within '.update-password' do
+          fill_in 'user_current_password', with: user_current_password
+          fill_passwords(new_password, new_password)
+        end
       end
 
-      page.within '.flash-container' do
-        expect(page).to have_content 'You must provide a valid current password'
-      end
-    end
+      it 'handles the invalid password attempt, and prompts the user to try again', :aggregate_failures do
+        expect(Gitlab::AppLogger).to receive(:info)
+          .with(message: 'Invalid current password when attempting to update user password', username: user.username, ip: user.current_sign_in_ip)
+
+        subject
+
+        user.reload
 
-    it 'does not change password with invalid old password' do
-      page.within '.update-password' do
-        fill_in 'user_current_password', with: 'invalid'
-        fill_passwords('password', 'confirmation')
+        expect(user.failed_attempts).to eq(1)
+        expect(user.valid_password?(new_password)).to eq(false)
+        expect(current_path).to eq(edit_profile_password_path)
+
+        page.within '.flash-container' do
+          expect(page).to have_content('You must provide a valid current password')
+        end
       end
 
-      page.within '.flash-container' do
-        expect(page).to have_content 'You must provide a valid current password'
+      it 'locks the user account when user passes the maximum attempts threshold', :aggregate_failures do
+        user.update!(failed_attempts: User.maximum_attempts.pred)
+
+        subject
+
+        expect(current_path).to eq(new_user_session_path)
+
+        page.within '.flash-container' do
+          expect(page).to have_content('Your account is locked.')
+        end
       end
     end
 
-    it 'changes user password' do
-      page.within '.update-password' do
-        fill_in "user_current_password", with: user.password
-        fill_passwords('22233344', '22233344')
+    context 'when current password is blank' do
+      let(:user_current_password) { nil }
+
+      it_behaves_like 'user enters an incorrect current password'
+    end
+
+    context 'when current password is incorrect' do
+      let(:user_current_password) {'invalid' }
+
+      it_behaves_like 'user enters an incorrect current password'
+    end
+
+    context 'when the password reset is successful' do
+      subject do
+        page.within '.update-password' do
+          fill_in "user_current_password", with: user.password
+          fill_passwords(new_password, new_password)
+        end
       end
 
-      expect(current_path).to eq new_user_session_path
+      it 'changes the password, logs the user out and prompts them to sign in again', :aggregate_failures do
+        expect { subject }.to change { user.reload.valid_password?(new_password) }.to(true)
+        expect(current_path).to eq new_user_session_path
+
+        page.within '.flash-container' do
+          expect(page).to have_content('Password was successfully updated. Please sign in again.')
+        end
+      end
     end
   end