diff --git a/app/models/authenticator/base.rb b/app/models/authenticator/base.rb index 5d366270168..a4687e1d8e2 100644 --- a/app/models/authenticator/base.rb +++ b/app/models/authenticator/base.rb @@ -65,7 +65,7 @@ def authenticate(username, password, request = nil, options = {}) audit = {:event => audit_event, :userid => username} # The fail_message might or might not come from the _authenticate method - authenticated, fail_message = options[:authorize_only] || _authenticate(username, password, request) + authenticated, fail_message = options[:authorize_only] || _authenticate(username, password, request: request, lookup_scope: options[:lookup_scope]) fail_message ||= _("Authentication failed") # Fall back to the default fail_message if authenticated @@ -76,7 +76,7 @@ def authenticate(username, password, request = nil, options = {}) else # If role_mode == database we will only use the external system for authentication. Also, the user must exist in our database # otherwise we will fail authentication - user_or_taskid = lookup_by_identity(username, request) + user_or_taskid = lookup_by_identity(username, request: request, lookup_scope: options[:lookup_scope]) user_or_taskid ||= autocreate_user(username) unless user_or_taskid @@ -200,8 +200,8 @@ def authenticate_with_http_basic(username, password, request = nil, options = {} [!!result, username] end - def lookup_by_identity(username, *_args) - case_insensitive_find_by_userid(username) + def lookup_by_identity(username, request: nil, lookup_scope: nil) + case_insensitive_find_by_userid(username, lookup_scope: lookup_scope) end # FIXME: LDAP @@ -242,9 +242,15 @@ def failure_reason(_username, _request) nil end - def case_insensitive_find_by_userid(username) - user = User.lookup_by_userid(username) - user || User.in_my_region.where(:lower_userid => username.downcase).order(:lastlogon).last + def case_insensitive_find_by_userid(username, lookup_scope: nil) + user_key = username.downcase + @lookup ||= {} + @lookup[user_key] ||= {} + return @lookup[user_key][lookup_scope] if @lookup[username][lookup_scope] + + base = User.send(User::LOOKUP_SCOPES[lookup_scope]) + user = base.lookup_by_userid(username) + @lookup[user_key][lookup_scope] = user || base.in_my_region.where(:lower_userid => user_key).order(:lastlogon).last end def userid_for(_identity, username) diff --git a/app/models/authenticator/database.rb b/app/models/authenticator/database.rb index 12d513d6519..9a025685e01 100644 --- a/app/models/authenticator/database.rb +++ b/app/models/authenticator/database.rb @@ -10,8 +10,8 @@ def uses_stored_password? private - def _authenticate(username, password, _request) - user = case_insensitive_find_by_userid(username) + def _authenticate(username, password, request: nil, lookup_scope: nil) + user = case_insensitive_find_by_userid(username, lookup_scope: lookup_scope) return [false, _('Your account has been locked due to too many failed login attempts, please contact the administrator.')] if user&.locked? diff --git a/app/models/authenticator/httpd.rb b/app/models/authenticator/httpd.rb index 39cea08e54b..fa0fd65f52d 100644 --- a/app/models/authenticator/httpd.rb +++ b/app/models/authenticator/httpd.rb @@ -41,7 +41,7 @@ def user_authorizable_with_system_token? ext_auth_is_oidc? || ext_auth_is_saml? end - def _authenticate(_username, _password, request) + def _authenticate(_username, _password, request: nil, lookup_scope: nil) request.present? && request.headers['X-REMOTE-USER'].present? end @@ -85,29 +85,30 @@ def find_or_initialize_user(identity, username) [upn_username, user] end - def lookup_by_identity(username, request = nil) + def lookup_by_identity(username, request: nil, lookup_scope: nil) if request user_attrs, _membership_list = user_details_from_headers(username, request) upn_username = username_to_upn_name(user_attrs) - user = find_userid_as_upn(upn_username) - user ||= find_userid_as_distinguished_name(user_attrs) + user = find_userid_as_upn(upn_username, lookup_scope: lookup_scope) + user ||= find_userid_as_distinguished_name(user_attrs, lookup_scope: lookup_scope) end - user || case_insensitive_find_by_userid(username) + user || case_insensitive_find_by_userid(username, lookup_scope: lookup_scope) end private - def find_userid_as_upn(upn_username) - case_insensitive_find_by_userid(upn_username) + def find_userid_as_upn(upn_username, lookup_scope: nil) + case_insensitive_find_by_userid(upn_username, lookup_scope: lookup_scope) end def find_userid_as_username(identity, username) case_insensitive_find_by_userid(userid_for(identity, username)) end - def find_userid_as_distinguished_name(user_attrs) + def find_userid_as_distinguished_name(user_attrs, lookup_scope: nil) dn_domain = user_attrs[:domain].downcase.split(".").map { |s| "dc=#{s}" }.join(",") - user = User.in_my_region.where("userid LIKE ?", "%=#{user_attrs[:username]},%,#{dn_domain}").last + base = User.send(User::LOOKUP_SCOPES[lookup_scope]) + user = base.in_my_region.where("userid LIKE ?", "%=#{user_attrs[:username]},%,#{dn_domain}").last user end diff --git a/app/models/authenticator/ldap.rb b/app/models/authenticator/ldap.rb index a7e1517a368..c165337d898 100644 --- a/app/models/authenticator/ldap.rb +++ b/app/models/authenticator/ldap.rb @@ -66,7 +66,7 @@ def normalize_username(username) miq_ldap.normalize(miq_ldap.fqusername(username)) end - def _authenticate(username, password, _request) + def _authenticate(username, password, request: nil, lookup_scope: nil) password.present? && ldap_bind(username, password) end diff --git a/app/models/user.rb b/app/models/user.rb index 5ee6d06e0af..b0bf4ca1371 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -58,6 +58,25 @@ class User < ApplicationRecord scope :with_same_userid, ->(id) { where(:userid => User.where(:id => id).pluck(:userid)) } + # This just creates the following Hash: + # + # { + # :api_includes => :api_includes + # } + # + # Where :default_scoped is the value for anything that doesn't match a key in + # the original array. The "lookup hash" was chossen since it is much faster + # to do a hash table lookup than a `Array.includes?(KEY)`. + LOOKUP_SCOPES = Hash.new(:default_scoped).merge({:api_includes => :api_includes}).freeze + + scope :api_includes, -> { + eager_load(:current_group => [ + :miq_user_role, + :tenant, + :entitlement + ]) + } + def self.with_roles_excluding(identifier) where.not(:id => User.unscope(:select).joins(:miq_groups => :miq_product_features) .where(:miq_product_features => {:identifier => identifier}) @@ -263,8 +282,8 @@ def self.authenticate_with_http_basic(username, password, request = nil, options authenticator(username).authenticate_with_http_basic(username, password, request, options) end - def self.lookup_by_identity(username) - authenticator(username).lookup_by_identity(username) + def self.lookup_by_identity(username, lookup_scope: :none) + authenticator(username).lookup_by_identity(username, lookup_scope: lookup_scope) end def self.authorize_user(userid) diff --git a/spec/models/authenticator/httpd_spec.rb b/spec/models/authenticator/httpd_spec.rb index 1bb4c4a90e2..925a50b04f4 100644 --- a/spec/models/authenticator/httpd_spec.rb +++ b/spec/models/authenticator/httpd_spec.rb @@ -107,19 +107,19 @@ end it "finds existing users as username" do - expect(subject.lookup_by_identity('alice', request)).to eq(alice) + expect(subject.lookup_by_identity('alice', request: request)).to eq(alice) end it "finds existing users as UPN" do - expect(subject.lookup_by_identity('cheshire', request)).to eq(cheshire) + expect(subject.lookup_by_identity('cheshire', request: request)).to eq(cheshire) end it "finds existing users as distinguished name" do - expect(subject.lookup_by_identity('towmater', request)).to eq(towmater_dn) + expect(subject.lookup_by_identity('towmater', request: request)).to eq(towmater_dn) end it "doesn't create new users" do - expect(subject.lookup_by_identity('bob', request)).to be_nil + expect(subject.lookup_by_identity('bob', request: request)).to be_nil end end