diff options
author | Jonathan Doman <jonathan.doman@intel.com> | 2023-08-19 01:27:54 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2023-10-27 20:20:54 +0300 |
commit | 522377dcb85082da598403e104a44d621b4c2bb4 (patch) | |
tree | 1b7544710f1a3e41857802549222dc9b628fc196 /include | |
parent | a974c132c8eaa591acaa92019f9b0088b4815ff1 (diff) | |
download | bmcweb-522377dcb85082da598403e104a44d621b4c2bb4.tar.xz |
Refactor populateUserInfo
- No need to set error code in asyncResp since caller already does that.
Then we can remove the asyncResp param altogether.
- Check if session is valid before unpacking properties to avoid
unnecessary work.
- Use std::optional instead of pointers for slighter cleaner code.
- Enforce required properties for local users based on D-Bus interface
documentation (UserGroups must be provided for local users).
Change-Id: I770d3556a0d62182b6abd72bfa3f8d62e2a105d1
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
Diffstat (limited to 'include')
-rw-r--r-- | include/dbus_privileges.hpp | 68 |
1 files changed, 21 insertions, 47 deletions
diff --git a/include/dbus_privileges.hpp b/include/dbus_privileges.hpp index 16aae5e62a..6602a5c2fa 100644 --- a/include/dbus_privileges.hpp +++ b/include/dbus_privileges.hpp @@ -19,75 +19,49 @@ namespace crow // Populate session with user information. inline bool populateUserInfo(Request& req, - const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, const dbus::utility::DBusPropertiesMap& userInfoMap) { - const std::string* userRolePtr = nullptr; - const bool* remoteUser = nullptr; - const bool* passwordExpired = nullptr; - const std::vector<std::string>* userGroups = nullptr; + if (req.session == nullptr) + { + return false; + } + + std::string userRole; + bool remoteUser = false; + std::optional<bool> passwordExpired; + std::optional<std::vector<std::string>> userGroups; const bool success = sdbusplus::unpackPropertiesNoThrow( redfish::dbus_utils::UnpackErrorPrinter(), userInfoMap, "UserPrivilege", - userRolePtr, "RemoteUser", remoteUser, "UserPasswordExpired", + userRole, "RemoteUser", remoteUser, "UserPasswordExpired", passwordExpired, "UserGroups", userGroups); if (!success) { BMCWEB_LOG_ERROR("Failed to unpack user properties."); - asyncResp->res.result( - boost::beast::http::status::internal_server_error); return false; } - if (req.session == nullptr) + if (!remoteUser && (!passwordExpired || !userGroups)) { + BMCWEB_LOG_ERROR( + "Missing UserPasswordExpired or UserGroups property for local user"); return false; } - if (userRolePtr != nullptr) - { - req.session->userRole = *userRolePtr; - BMCWEB_LOG_DEBUG("userName = {} userRole = {}", req.session->username, - *userRolePtr); - } - - if (remoteUser == nullptr) - { - BMCWEB_LOG_ERROR("RemoteUser property missing or wrong type"); - asyncResp->res.result( - boost::beast::http::status::internal_server_error); - return false; - } - bool expired = false; - if (passwordExpired == nullptr) - { - if (!*remoteUser) - { - BMCWEB_LOG_ERROR("UserPasswordExpired property is expected for" - " local user but is missing or wrong type"); - asyncResp->res.result( - boost::beast::http::status::internal_server_error); - return false; - } - } - else - { - expired = *passwordExpired; - } + req.session->userRole = userRole; + BMCWEB_LOG_DEBUG("userName = {} userRole = {}", req.session->username, + userRole); // Set isConfigureSelfOnly based on D-Bus results. This // ignores the results from both pamAuthenticateUser and the // value from any previous use of this session. - req.session->isConfigureSelfOnly = expired; + req.session->isConfigureSelfOnly = passwordExpired.value_or(false); - if (userGroups != nullptr) - { - req.session->userGroups = *userGroups; - } - else + req.session->userGroups.clear(); + if (userGroups) { - req.session->userGroups.clear(); + req.session->userGroups.swap(*userGroups); } return true; @@ -147,7 +121,7 @@ void afterGetUserInfo(Request& req, return; } - if (!populateUserInfo(req, asyncResp, userInfoMap)) + if (!populateUserInfo(req, userInfoMap)) { BMCWEB_LOG_ERROR("Failed to populate user information"); asyncResp->res.result( |