Skip to content

Commit

Permalink
Merge pull request ManageIQ#15904 from jvlcek/bz1486041_mixed_case_db
Browse files Browse the repository at this point in the history
If the userid is not found in the DB do a case insensitive search
  • Loading branch information
gtanzillo authored Sep 5, 2017
2 parents b0955e8 + 7135b43 commit 63e85c8
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 13 deletions.
16 changes: 10 additions & 6 deletions app/models/authenticator/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -168,20 +167,20 @@ 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}"
elsif !username.include?('@') && MiqLdap.using_ldap?
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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/models/authenticator/database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions app/models/authenticator/httpd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions spec/models/authenticator/database_spec.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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

0 comments on commit 63e85c8

Please sign in to comment.