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>
diff --git a/include/dbus_privileges.hpp b/include/dbus_privileges.hpp
index 16aae5e..6602a5c 100644
--- a/include/dbus_privileges.hpp
+++ b/include/dbus_privileges.hpp
@@ -19,75 +19,49 @@
// 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;
-
- const bool success = sdbusplus::unpackPropertiesNoThrow(
- redfish::dbus_utils::UnpackErrorPrinter(), userInfoMap, "UserPrivilege",
- userRolePtr, "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)
{
return false;
}
- if (userRolePtr != nullptr)
- {
- req.session->userRole = *userRolePtr;
- BMCWEB_LOG_DEBUG("userName = {} userRole = {}", req.session->username,
- *userRolePtr);
- }
+ std::string userRole;
+ bool remoteUser = false;
+ std::optional<bool> passwordExpired;
+ std::optional<std::vector<std::string>> userGroups;
- if (remoteUser == nullptr)
+ const bool success = sdbusplus::unpackPropertiesNoThrow(
+ redfish::dbus_utils::UnpackErrorPrinter(), userInfoMap, "UserPrivilege",
+ userRole, "RemoteUser", remoteUser, "UserPasswordExpired",
+ passwordExpired, "UserGroups", userGroups);
+
+ if (!success)
{
- BMCWEB_LOG_ERROR("RemoteUser property missing or wrong type");
- asyncResp->res.result(
- boost::beast::http::status::internal_server_error);
+ BMCWEB_LOG_ERROR("Failed to unpack user properties.");
return false;
}
- bool expired = false;
- if (passwordExpired == nullptr)
+
+ if (!remoteUser && (!passwordExpired || !userGroups))
{
- 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;
- }
+ BMCWEB_LOG_ERROR(
+ "Missing UserPasswordExpired or UserGroups property for local user");
+ 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.clear();
+ if (userGroups)
{
- req.session->userGroups = *userGroups;
- }
- else
- {
- req.session->userGroups.clear();
+ req.session->userGroups.swap(*userGroups);
}
return true;
@@ -147,7 +121,7 @@
return;
}
- if (!populateUserInfo(req, asyncResp, userInfoMap))
+ if (!populateUserInfo(req, userInfoMap))
{
BMCWEB_LOG_ERROR("Failed to populate user information");
asyncResp->res.result(