Merge pull request ManageIQ#20087 from skateman/locked-account

Implemented account lockout policy backend

app/models/authenticator/base.rb

@@ -44,15 +44,17 @@ def authenticate(username, password, request = nil, options = {})
       options = options.dup
       options[:require_user] ||= false
       options[:authorize_only] ||= false
-      fail_message = _("Authentication failed")
 
       user_or_taskid = nil
 
       begin
         username = normalize_username(username)
         audit = {:event => audit_event, :userid => username}
 
-        authenticated = options[:authorize_only] || _authenticate(username, password, request)
+        # The fail_message might or might not come from the _authenticate method
+        authenticated, fail_message = options[:authorize_only] || _authenticate(username, password, request)
+        fail_message ||= _("Authentication failed") # Fall back to the default fail_message
+
         if authenticated
           audit_success(audit.merge(:message => "User #{username} successfully validated by #{self.class.proper_name}"))
 

app/models/authenticator/database.rb

@@ -13,7 +13,15 @@ def uses_stored_password?
     def _authenticate(username, password, _request)
       user = case_insensitive_find_by_userid(username)
 
-      user && user.authenticate_bcrypt(password)
+      return [false, _('Your account has been locked due to too many failed login attempts, please contact the administrator.')] if user&.locked?
+
+      if user&.authenticate_bcrypt(password) # Authenticate if the username matches
+        user.unlock! # Reset the number of failed logins
+        return true
+      end
+
+      user&.fail_login! # Increase the number of failed login attempts
+      [false, _("The username or password you entered is incorrect.")]
     end
 
     def authorize?

app/models/user.rb

@@ -53,6 +53,8 @@ class User < ApplicationRecord
   serialize     :settings, Hash   # Implement settings column as a hash
   default_value_for(:settings) { Hash.new }
 
+  default_value_for :failed_login_attempts, 0
+
   scope :with_same_userid, ->(id) { where(:userid => User.where(:id => id).pluck(:userid)) }
 
   def self.with_roles_excluding(identifier)
@@ -168,6 +170,20 @@ def change_password(oldpwd, newpwd)
     end
   end
 
+  def locked?
+    ::Settings.authentication.max_failed_login_attempts.positive? && failed_login_attempts >= ::Settings.authentication.max_failed_login_attempts
+  end
+
+  def unlock!
+    update!(:failed_login_attempts => 0)
+  end
+
+  def fail_login!
+    update!(:failed_login_attempts => failed_login_attempts + 1)
+
+    unlock_queue if locked?
+  end
+
   def ldap_group
     current_group.try(:description)
   end
@@ -366,4 +382,17 @@ def self.seed_data
     File.exist?(seed_file_name) ? YAML.load_file(seed_file_name) : []
   end
   private_class_method :seed_data
+
+  private
+
+  def unlock_queue
+    MiqQueue.put_or_update(
+      :class_name  => self.class.name,
+      :instance_id => id,
+      :method_name => 'unlock!',
+      :priority    => MiqQueue::MAX_PRIORITY
+    ) do |_msg, queue_options|
+      queue_options.merge(:deliver_on => Time.now.utc + ::Settings.authentication.locked_account_timeout.to_i)
+    end
+  end
 end

config/settings.yml

@@ -11,6 +11,8 @@
   :ldaphost:
   :ldapport: '389'
   :mode: database
+  :max_failed_login_attempts: 3
+  :locked_account_timeout: 2.minutes
   :search_timeout: 30
   :user_suffix:
   :user_type: userprincipalname

spec/models/authenticator/database_spec.rb

@@ -56,6 +56,24 @@ def authenticate
       it "updates lastlogon" do
         expect { authenticate }.to(change { alice.reload.lastlogon })
       end
+
+      it "resets failed login attempts" do
+        alice.update(:failed_login_attempts => 1)
+        authenticate
+        expect(alice.reload.failed_login_attempts).to eq(0)
+      end
+
+      context "with too many failed login attempts" do
+        before do
+          EvmSpecHelper.create_guid_miq_server_zone
+          alice.update(:failed_login_attempts => 4)
+          allow(alice).to receive(:unlock_queue)
+        end
+
+        it 'fails' do
+          expect { authenticate }.to raise_error(MiqException::MiqEVMLoginError, "Your account has been locked due to too many failed login attempts, please contact the administrator.")
+        end
+      end
     end
 
     context "with bad password" do
@@ -66,7 +84,13 @@ def authenticate
       end
 
       it "fails" do
-        expect { authenticate }.to raise_error(MiqException::MiqEVMLoginError, "Authentication failed")
+        expect { authenticate }.to raise_error(MiqException::MiqEVMLoginError, "The username or password you entered is incorrect.")
+      end
+
+      it "increases the number of failed logins" do
+        count = alice.failed_login_attempts
+        authenticate rescue nil
+        expect(alice.reload.failed_login_attempts).to eq(count + 1)
       end
 
       it "records one failing audit entry" do
@@ -81,7 +105,7 @@ def authenticate
 
       it "logs the failure" do
         allow($log).to receive(:warn).with(/Audit/)
-        expect($log).to receive(:warn).with(/Authentication failed$/)
+        expect($log).to receive(:warn).with(/The username or password you entered is incorrect.$/)
         authenticate rescue nil
       end
 
@@ -113,7 +137,7 @@ def authenticate
 
       it "logs the failure" do
         allow($log).to receive(:warn).with(/Audit/)
-        expect($log).to receive(:warn).with(/Authentication failed$/)
+        expect($log).to receive(:warn).with(/The username or password you entered is incorrect.$/)
         authenticate rescue nil
       end
     end

spec/models/user_spec.rb

@@ -145,6 +145,21 @@
     end
   end
 
+  describe '#fail_login!' do
+    let(:user) { FactoryBot.create(:user, :password => "smartvm", :failed_login_attempts => 0) }
+
+    it 'increases the number of failed login attempts' do
+      user.fail_login!
+      expect(user.reload.failed_login_attempts).to eq(1)
+    end
+
+    it 'queues an unlock task if the account is locked' do
+      allow(user).to receive(:locked?).and_return(true)
+      expect(user).to receive(:unlock_queue)
+      user.fail_login!
+    end
+  end
+
   context "#authorize_ldap" do
     before do
       @fq_user = "[email protected]"