From 7135b4304e4ae368fef5e9446065b86a32a8b3cf Mon Sep 17 00:00:00 2001 From: Joe VLcek Date: Tue, 29 Aug 2017 17:52:17 -0400 Subject: [PATCH] If the userid is not found in the DB do a case insensitive search https://bugzilla.redhat.com/show_bug.cgi?id=1486041 --- app/models/authenticator/base.rb | 16 ++++++++----- app/models/authenticator/database.rb | 2 +- app/models/authenticator/httpd.rb | 8 ++----- spec/models/authenticator/database_spec.rb | 27 ++++++++++++++++++++++ 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/app/models/authenticator/base.rb b/app/models/authenticator/base.rb index 787502484e8..d7c125e3c74 100644 --- a/app/models/authenticator/base.rb +++ b/app/models/authenticator/base.rb @@ -89,7 +89,7 @@ def authenticate(username, password, request = nil, options = {}) if task.nil? || MiqTask.status_error?(task.status) || MiqTask.status_timeout?(task.status) raise MiqException::MiqEVMLoginError, fail_message end - user_or_taskid = User.find_by_userid(task.userid) + user_or_taskid = case_insensitive_find_by_userid(task.userid) end if user_or_taskid.kind_of?(User) @@ -149,8 +149,7 @@ def authorize(taskid, username, *args) def find_or_initialize_user(identity, username) userid = userid_for(identity, username) - user = User.find_by_userid(userid) - user ||= User.in_my_region.where('lower(userid) = ?', userid).order(:lastlogon).last + user = case_insensitive_find_by_userid(userid) user ||= User.new(:userid => userid) [userid, user] end @@ -168,12 +167,12 @@ def authenticate_with_http_basic(username, password, request = nil, options = {} end def lookup_by_identity(username) - User.find_by_userid(username) + case_insensitive_find_by_userid(username) end # FIXME: LDAP def find_by_principalname(username) - unless (user = User.find_by_userid(username)) + unless (user = case_insensitive_find_by_userid(username)) if username.include?('\\') parts = username.split('\\') username = "#{parts.last}@#{parts.first}" @@ -181,7 +180,7 @@ def find_by_principalname(username) suffix = config[:user_suffix] username = "#{username}@#{suffix}" end - user = User.find_by_userid(username) + user = case_insensitive_find_by_userid(username) end [user, username] end @@ -200,6 +199,11 @@ def failure_reason(_username, _request) nil end + def case_insensitive_find_by_userid(username) + user = User.find_by_userid(username) + user || User.in_my_region.where('lower(userid) = ?', username.downcase).order(:lastlogon).last + end + def userid_for(_identity, username) username end diff --git a/app/models/authenticator/database.rb b/app/models/authenticator/database.rb index b662993a26f..438841d8c69 100644 --- a/app/models/authenticator/database.rb +++ b/app/models/authenticator/database.rb @@ -11,7 +11,7 @@ def uses_stored_password? private def _authenticate(username, password, _request) - user = User.find_by_userid(username) + user = case_insensitive_find_by_userid(username) user && user.authenticate_bcrypt(password) end diff --git a/app/models/authenticator/httpd.rb b/app/models/authenticator/httpd.rb index ece32f75a2a..d529eaa4721 100644 --- a/app/models/authenticator/httpd.rb +++ b/app/models/authenticator/httpd.rb @@ -73,15 +73,11 @@ def find_or_initialize_user(identity, username) private def find_userid_as_upn(upn_username) - user = User.find_by_userid(upn_username) - user || User.in_my_region.where('lower(userid) = ?', upn_username).order(:lastlogon).last + case_insensitive_find_by_userid(upn_username) end def find_userid_as_username(identity, username) - userid = userid_for(identity, username) - user = User.find_by_userid(userid) - user ||= User.in_my_region.where('lower(userid) = ?', userid).order(:lastlogon).last - user + case_insensitive_find_by_userid(userid_for(identity, username)) end def find_userid_as_distinguished_name(user_attrs) diff --git a/spec/models/authenticator/database_spec.rb b/spec/models/authenticator/database_spec.rb index 5415bbe93e4..3f3355d0273 100644 --- a/spec/models/authenticator/database_spec.rb +++ b/spec/models/authenticator/database_spec.rb @@ -1,6 +1,7 @@ describe Authenticator::Database do subject { Authenticator::Database.new({}) } let!(:alice) { FactoryGirl.create(:user, :userid => 'alice', :password => 'secret') } + let!(:vincent) { FactoryGirl.create(:user, :userid => 'Vincent', :password => 'secret') } describe '#uses_stored_password?' do it "is true" do @@ -98,5 +99,31 @@ def authenticate authenticate rescue nil end end + + context "with mixed case username" do + let(:username) { 'vInCeNt' } + + it "succeeds" do + expect(authenticate).to eq(vincent) + end + + it "records two successful audit entries" do + expect(AuditEvent).to receive(:success).with( + :event => 'authenticate_database', + :userid => 'vInCeNt', + :message => "User vincent successfully validated by EVM", + ) + expect(AuditEvent).to receive(:success).with( + :event => 'authenticate_database', + :userid => 'vInCeNt', + :message => "Authentication successful for user vincent", + ) + expect(AuditEvent).not_to receive(:failure) + authenticate + end + it "updates lastlogon" do + expect(-> { authenticate }).to change { vincent.reload.lastlogon } + end + end end end