Fix authorization for LDAP users
Modified the code to make an asynchronous call to GetUserInfo to get
the user role for authorization.
For local users, DBus matches are used to store user role map hot in
memory. Hence, bmcweb has to know whether a user is a local user or
LDAP user to get the role. To avoid this, removed the existing DBus
matches and modified the code to call GetUserInfo to get the role of
local users as well as LDAP users.
Tested:
- Created a local user having admin privilege and verified that he is
able to restart the system
/redfish/v1/Systems/system/Actions/ComputerSystem.Reset
-d '{"ResetType": "GracefulRestart"}'
- Created a local user having user privilege and verified that he is
unauthorized to restart the system
/redfish/v1/Systems/system/Actions/ComputerSystem.Reset
-d '{"ResetType": "GracefulRestart"}'
- Created a remote user having admin privilege and verified that he is
able to restart the system
/redfish/v1/Systems/system/Actions/ComputerSystem.Reset
-d '{"ResetType": "GracefulRestart"}'
- Created a remote user having user privilege and verified that he is
unauthorized to restart the system
/redfish/v1/Systems/system/Actions/ComputerSystem.Reset
-d '{"ResetType": "GracefulRestart"}'
- Tested Redfish ConfigureSelf privilege
Signed-off-by: RAJESWARAN THILLAIGOVINDAN <rajeswgo@in.ibm.com>
Change-Id: Ic3e46a0c0aff2cf456c98048350e58e302011c57
diff --git a/http/app.h b/http/app.h
index fd01340..74fc5c6 100644
--- a/http/app.h
+++ b/http/app.h
@@ -54,7 +54,7 @@
router.handleUpgrade(req, res, std::move(adaptor));
}
- void handle(const Request& req, Response& res)
+ void handle(Request& req, Response& res)
{
router.handle(req, res);
}
diff --git a/http/http_request.h b/http/http_request.h
index caae93a..ff09bf1 100644
--- a/http/http_request.h
+++ b/http/http_request.h
@@ -26,6 +26,8 @@
std::shared_ptr<crow::persistent_data::UserSession> session;
+ std::string userRole{};
+
Request(
boost::beast::http::request<boost::beast::http::string_body>& reqIn) :
req(reqIn),
diff --git a/http/routing.h b/http/routing.h
index 7846924..f194ad1 100644
--- a/http/routing.h
+++ b/http/routing.h
@@ -1177,7 +1177,7 @@
}
}
- void handle(const Request& req, Response& res)
+ void handle(Request& req, Response& res)
{
if (static_cast<size_t>(req.method()) >= perMethods.size())
return;
@@ -1250,48 +1250,61 @@
<< static_cast<uint32_t>(req.method()) << " / "
<< rules[ruleIndex]->getMethods();
- redfish::Privileges userPrivileges;
- if (req.session != nullptr)
- {
- // Get the user role from the session.
- const std::string& userRole =
- persistent_data::UserRoleMap::getInstance().getUserRole(
- req.session->username);
-
- BMCWEB_LOG_DEBUG << "USER ROLE=" << userRole;
-
- // Get the user privileges from the role
- userPrivileges = redfish::getUserPrivileges(userRole);
- }
-
- if (!rules[ruleIndex]->checkPrivileges(userPrivileges))
- {
- res.result(boost::beast::http::status::forbidden);
- res.end();
- return;
- }
-
- // any uncaught exceptions become 500s
- try
+ if (req.session == nullptr)
{
rules[ruleIndex]->handle(req, res, found.second);
- }
- catch (std::exception& e)
- {
- BMCWEB_LOG_ERROR << "An uncaught exception occurred: " << e.what();
- res.result(boost::beast::http::status::internal_server_error);
- res.end();
return;
}
- catch (...)
- {
- BMCWEB_LOG_ERROR
- << "An uncaught exception occurred. The type was unknown "
- "so no information was available.";
- res.result(boost::beast::http::status::internal_server_error);
- res.end();
- return;
- }
+
+ crow::connections::systemBus->async_method_call(
+ [&req, &res, &rules, ruleIndex, found](
+ const boost::system::error_code ec,
+ std::map<std::string, std::variant<bool, std::string,
+ std::vector<std::string>>>
+ userInfo) {
+ if (ec)
+ {
+ BMCWEB_LOG_ERROR << "GetUserInfo failed...";
+ res.result(
+ boost::beast::http::status::internal_server_error);
+ res.end();
+ return;
+ }
+
+ const std::string* userRolePtr = nullptr;
+ auto userInfoIter = userInfo.find("UserPrivilege");
+ if (userInfoIter != userInfo.end())
+ {
+ userRolePtr =
+ std::get_if<std::string>(&userInfoIter->second);
+ }
+
+ std::string userRole{};
+ if (userRolePtr != nullptr)
+ {
+ userRole = *userRolePtr;
+ BMCWEB_LOG_DEBUG << "userName = " << req.session->username
+ << " userRole = " << *userRolePtr;
+ }
+
+ // Get the user privileges from the role
+ redfish::Privileges userPrivileges =
+ redfish::getUserPrivileges(userRole);
+
+ if (!rules[ruleIndex]->checkPrivileges(userPrivileges))
+ {
+ res.result(boost::beast::http::status::forbidden);
+ res.end();
+ return;
+ }
+
+ req.userRole = userRole;
+
+ rules[ruleIndex]->handle(req, res, found.second);
+ },
+ "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
+ "xyz.openbmc_project.User.Manager", "GetUserInfo",
+ req.session->username);
}
void debugPrint()
diff --git a/include/sessions.hpp b/include/sessions.hpp
index f461521..f197bff 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -31,250 +31,6 @@
SINGLE_REQUEST // User times out once this request is completed.
};
-constexpr char const* userService = "xyz.openbmc_project.User.Manager";
-constexpr char const* userObjPath = "/xyz/openbmc_project/user";
-constexpr char const* userAttrIface = "xyz.openbmc_project.User.Attributes";
-constexpr char const* dbusPropertiesIface = "org.freedesktop.DBus.Properties";
-
-struct UserRoleMap
-{
- using GetManagedPropertyType =
- boost::container::flat_map<std::string,
- std::variant<std::string, bool>>;
-
- using InterfacesPropertiesType =
- boost::container::flat_map<std::string, GetManagedPropertyType>;
-
- using GetManagedObjectsType = std::vector<
- std::pair<sdbusplus::message::object_path, InterfacesPropertiesType>>;
-
- static UserRoleMap& getInstance()
- {
- static UserRoleMap userRoleMap;
- return userRoleMap;
- }
-
- UserRoleMap(const UserRoleMap&) = delete;
- UserRoleMap& operator=(const UserRoleMap&) = delete;
-
- std::string getUserRole(std::string_view name)
- {
- auto it = roleMap.find(std::string(name));
- if (it == roleMap.end())
- {
- BMCWEB_LOG_ERROR << "User name " << name
- << " is not found in the UserRoleMap.";
- return "";
- }
- return it->second;
- }
-
- std::string
- extractUserRole(const InterfacesPropertiesType& interfacesProperties)
- {
- auto iface = interfacesProperties.find(userAttrIface);
- if (iface == interfacesProperties.end())
- {
- return {};
- }
-
- auto& properties = iface->second;
- auto property = properties.find("UserPrivilege");
- if (property == properties.end())
- {
- return {};
- }
-
- const std::string* role = std::get_if<std::string>(&property->second);
- if (role == nullptr)
- {
- BMCWEB_LOG_ERROR << "UserPrivilege property value is null";
- return {};
- }
-
- return *role;
- }
-
- private:
- void userAdded(sdbusplus::message::message& m)
- {
- sdbusplus::message::object_path objPath;
- InterfacesPropertiesType interfacesProperties;
-
- try
- {
- m.read(objPath, interfacesProperties);
- }
- catch (const sdbusplus::exception::SdBusError& e)
- {
- BMCWEB_LOG_ERROR << "Failed to parse user add signal."
- << "ERROR=" << e.what()
- << "REPLY_SIG=" << m.get_signature();
- return;
- }
- BMCWEB_LOG_DEBUG << "obj path = " << objPath.str;
-
- std::size_t lastPos = objPath.str.rfind("/");
- if (lastPos == std::string::npos)
- {
- return;
- };
-
- std::string name = objPath.str.substr(lastPos + 1);
- std::string role = this->extractUserRole(interfacesProperties);
-
- // Insert the newly added user name and the role
- auto res = roleMap.emplace(name, role);
- if (res.second == false)
- {
- BMCWEB_LOG_ERROR << "Insertion of the user=\"" << name
- << "\" in the roleMap failed.";
- return;
- }
- }
-
- void userRemoved(sdbusplus::message::message& m)
- {
- sdbusplus::message::object_path objPath;
-
- try
- {
- m.read(objPath);
- }
- catch (const sdbusplus::exception::SdBusError& e)
- {
- BMCWEB_LOG_ERROR << "Failed to parse user delete signal.";
- BMCWEB_LOG_ERROR << "ERROR=" << e.what()
- << "REPLY_SIG=" << m.get_signature();
- return;
- }
-
- BMCWEB_LOG_DEBUG << "obj path = " << objPath.str;
-
- std::size_t lastPos = objPath.str.rfind("/");
- if (lastPos == std::string::npos)
- {
- return;
- };
-
- // User name must be atleast 1 char in length.
- if ((lastPos + 1) >= objPath.str.length())
- {
- return;
- }
-
- std::string name = objPath.str.substr(lastPos + 1);
-
- roleMap.erase(name);
- }
-
- void userPropertiesChanged(sdbusplus::message::message& m)
- {
- std::string interface;
- GetManagedPropertyType changedProperties;
- m.read(interface, changedProperties);
- const std::string path = m.get_path();
-
- BMCWEB_LOG_DEBUG << "Object Path = \"" << path << "\"";
-
- std::size_t lastPos = path.rfind("/");
- if (lastPos == std::string::npos)
- {
- return;
- };
-
- // User name must be at least 1 char in length.
- if ((lastPos + 1) == path.length())
- {
- return;
- }
-
- std::string user = path.substr(lastPos + 1);
-
- BMCWEB_LOG_DEBUG << "User Name = \"" << user << "\"";
-
- auto index = changedProperties.find("UserPrivilege");
- if (index == changedProperties.end())
- {
- return;
- }
-
- const std::string* role = std::get_if<std::string>(&index->second);
- if (role == nullptr)
- {
- return;
- }
- BMCWEB_LOG_DEBUG << "Role = \"" << *role << "\"";
-
- auto it = roleMap.find(user);
- if (it == roleMap.end())
- {
- BMCWEB_LOG_ERROR << "User Name = \"" << user
- << "\" is not found. But, received "
- "propertiesChanged signal";
- return;
- }
- it->second = *role;
- }
-
- UserRoleMap() :
- userAddedSignal(
- *crow::connections::systemBus,
- sdbusplus::bus::match::rules::interfacesAdded(userObjPath),
- [this](sdbusplus::message::message& m) {
- BMCWEB_LOG_DEBUG << "User Added";
- this->userAdded(m);
- }),
- userRemovedSignal(
- *crow::connections::systemBus,
- sdbusplus::bus::match::rules::interfacesRemoved(userObjPath),
- [this](sdbusplus::message::message& m) {
- BMCWEB_LOG_DEBUG << "User Removed";
- this->userRemoved(m);
- }),
- userPropertiesChangedSignal(
- *crow::connections::systemBus,
- sdbusplus::bus::match::rules::path_namespace(userObjPath) +
- sdbusplus::bus::match::rules::type::signal() +
- sdbusplus::bus::match::rules::member("PropertiesChanged") +
- sdbusplus::bus::match::rules::interface(dbusPropertiesIface) +
- sdbusplus::bus::match::rules::argN(0, userAttrIface),
- [this](sdbusplus::message::message& m) {
- BMCWEB_LOG_DEBUG << "Properties Changed";
- this->userPropertiesChanged(m);
- })
- {
- crow::connections::systemBus->async_method_call(
- [this](boost::system::error_code ec,
- GetManagedObjectsType& managedObjects) {
- if (ec)
- {
- BMCWEB_LOG_DEBUG << "User manager call failed, ignoring";
- return;
- }
-
- for (auto& managedObj : managedObjects)
- {
- std::size_t lastPos = managedObj.first.str.rfind("/");
- if (lastPos == std::string::npos)
- {
- continue;
- };
- std::string name = managedObj.first.str.substr(lastPos + 1);
- std::string role = extractUserRole(managedObj.second);
- roleMap.emplace(name, role);
- }
- },
- userService, userObjPath, "org.freedesktop.DBus.ObjectManager",
- "GetManagedObjects");
- }
-
- boost::container::flat_map<std::string, std::string> roleMap;
- sdbusplus::bus::match_t userAddedSignal;
- sdbusplus::bus::match_t userRemovedSignal;
- sdbusplus::bus::match_t userPropertiesChangedSignal;
-};
-
struct UserSession
{
std::string uniqueId;
diff --git a/redfish-core/include/node.hpp b/redfish-core/include/node.hpp
index fddeaa0..9086f1e 100644
--- a/redfish-core/include/node.hpp
+++ b/redfish-core/include/node.hpp
@@ -178,9 +178,9 @@
*/
inline bool isAllowedWithoutConfigureSelf(const crow::Request& req)
{
- const std::string& userRole =
- crow::persistent_data::UserRoleMap::getInstance().getUserRole(
- req.session->username);
+ const std::string& userRole = req.userRole;
+ BMCWEB_LOG_DEBUG << "isAllowedWithoutConfigureSelf for the role "
+ << req.userRole;
Privileges effectiveUserPrivileges =
redfish::getUserPrivileges(userRole);
effectiveUserPrivileges.resetSinglePrivilege("ConfigureSelf");
diff --git a/src/webserver_main.cpp b/src/webserver_main.cpp
index a2da120..901c180 100644
--- a/src/webserver_main.cpp
+++ b/src/webserver_main.cpp
@@ -108,10 +108,6 @@
redfish::RedfishService redfish(app);
- // Keep the user role map hot in memory and
- // track the changes using match object
- crow::persistent_data::UserRoleMap::getInstance();
-
app.run();
io->run();