From d1c9e09cfd487cb93be6bf4ad7f01b3ba9d9e827 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 5 Aug 2020 11:59:47 -0500 Subject: [PATCH 1/3] [BaseController::Authentication] Use :api_includes Use the new `:lookup_scope` argument for `.authenticate` and `.lookup_by_identity` to reduce the number of user based queries on every request. --- app/controllers/api/base_controller/authentication.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/base_controller/authentication.rb b/app/controllers/api/base_controller/authentication.rb index 8b45ec6db0..9daa35cd6d 100644 --- a/app/controllers/api/base_controller/authentication.rb +++ b/app/controllers/api/base_controller/authentication.rb @@ -87,7 +87,7 @@ def api_token_mgr end def auth_user(userid) - auth_user_obj = User.lookup_by_identity(userid) + auth_user_obj = User.lookup_by_identity(userid, lookup_scope: :api_includes) authorize_user_group(auth_user_obj) validate_user_identity(auth_user_obj) User.current_user = auth_user_obj @@ -154,7 +154,7 @@ def authenticate_with_jwt def basic_authentication(username, password) timeout = ::Settings.api.authentication_timeout.to_i_with_method - user = User.authenticate(username, password, request, :require_user => true, :timeout => timeout) + user = User.authenticate(username, password, request, :require_user => true, :timeout => timeout, :lookup_scope => :api_includes) auth_user(user.userid) rescue MiqException::MiqEVMLoginError => e raise AuthenticationError, e.message From 4810302c90ee0bae1d26ca8585a11eb38c887ec0 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Wed, 5 Aug 2020 12:01:25 -0500 Subject: [PATCH 2/3] [BaseController::Logger] Use miq_user_role.name While `virtual_delegates` can be used to improve performance via a single attribute in a query, we already bring back the `miq_user_role` (via the lookup_scope), so it is better to just use that. And currently virtual_delegates don't support already loaded relationships. --- app/controllers/api/base_controller/logger.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/base_controller/logger.rb b/app/controllers/api/base_controller/logger.rb index 85b70f416c..c44008073a 100644 --- a/app/controllers/api/base_controller/logger.rb +++ b/app/controllers/api/base_controller/logger.rb @@ -23,7 +23,7 @@ def log_api_auth group = User.current_user.current_group log_request("Authorization", :user => User.current_user.userid, :group => group.description, - :role => group.miq_user_role_name, + :role => group.miq_user_role.name, :tenant => group.tenant.name) end end From db287afeddc70c015e22295c5b77a99bb9d6e3b8 Mon Sep 17 00:00:00 2001 From: Nick LaMuro Date: Mon, 31 Aug 2020 19:54:29 -0500 Subject: [PATCH 3/3] Various user_or_id fixes There are a few places in the API where we re-fetch the User when it is already available. These fixes allow for making use of the existing user that already exists instead of calling another lookup. The two places involved are: - When authenticating using .basic_authentication - When generating a token for the user (on login) --- app/controllers/api/auth_controller.rb | 2 +- app/controllers/api/base_controller/authentication.rb | 11 ++++++++--- lib/services/api/user_token_service.rb | 10 +++++++--- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/auth_controller.rb b/app/controllers/api/auth_controller.rb index 2266fd567f..66127a141e 100644 --- a/app/controllers/api/auth_controller.rb +++ b/app/controllers/api/auth_controller.rb @@ -5,7 +5,7 @@ class AuthController < BaseController def show requester_type = fetch_and_validate_requester_type token_service = Environment.user_token_service - auth_token = token_service.generate_token(User.current_user.userid, requester_type) + auth_token = token_service.generate_token(User.current_user, requester_type) token_info = token_service.token_mgr(requester_type).token_get_info(auth_token) res = { :auth_token => auth_token, diff --git a/app/controllers/api/base_controller/authentication.rb b/app/controllers/api/base_controller/authentication.rb index 9daa35cd6d..a64afa2606 100644 --- a/app/controllers/api/base_controller/authentication.rb +++ b/app/controllers/api/base_controller/authentication.rb @@ -86,8 +86,13 @@ def api_token_mgr Environment.user_token_service.token_mgr('api') end - def auth_user(userid) - auth_user_obj = User.lookup_by_identity(userid, lookup_scope: :api_includes) + def auth_user(user_or_id) + auth_user_obj = if user_or_id.kind_of?(User) + user_or_id + else + User.lookup_by_identity(user_or_id, lookup_scope: :api_includes) + end + authorize_user_group(auth_user_obj) validate_user_identity(auth_user_obj) User.current_user = auth_user_obj @@ -155,7 +160,7 @@ def authenticate_with_jwt def basic_authentication(username, password) timeout = ::Settings.api.authentication_timeout.to_i_with_method user = User.authenticate(username, password, request, :require_user => true, :timeout => timeout, :lookup_scope => :api_includes) - auth_user(user.userid) + auth_user(user) rescue MiqException::MiqEVMLoginError => e raise AuthenticationError, e.message end diff --git a/lib/services/api/user_token_service.rb b/lib/services/api/user_token_service.rb index fb6dd1b389..eca720a10a 100644 --- a/lib/services/api/user_token_service.rb +++ b/lib/services/api/user_token_service.rb @@ -23,9 +23,13 @@ def api_config @api_config ||= ::Settings[base_config[:module]].to_hash end - def generate_token(userid, requester_type, token_ttl: nil) - userid = userid.downcase - validate_userid(userid) + def generate_token(user_or_id, requester_type, token_ttl: nil) + if user_or_id.kind_of?(User) + userid = user_or_id.userid.downcase + else + userid = user_or_id.downcase + validate_userid(userid) + end validate_requester_type(requester_type) # Additional Requester type token ttl's for authentication