Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If the userid is not found in the DB do a case insensitive search #15904

Merged
merged 1 commit into from
Sep 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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