Revert "Cache user role in session object"

This reverts commit 8ed41c35a314580bb794fa0fff2e01b0bf7efcf7.

In discord, it was posted 2 systems are hitting 403 Forbidden for all
endpoints.

Reverting fixed the problem, until time is given to dive into this,
just revert.

One of the things wrong is this is missing an After/Want
xyz.openbmc_project.User.Manager.service.

Change-Id: I1766a6ec2dbc9fb52da3940b07ac002a1a6d269a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

3 files changed, 115 insertions, 305 deletions

diff --git a/include/dbus_privileges.hpp b/include/dbus_privileges.hpp
index 07a1216cdc..fca4a137d3 100644
--- a/include/dbus_privileges.hpp
+++ b/include/dbus_privileges.hpp
@@ -6,11 +6,9 @@
 #include "http_response.hpp"
 #include "logging.hpp"
 #include "routing/baserule.hpp"
-#include "user_role_map.hpp"
 #include "utils/dbus_utils.hpp"
 
 #include <boost/url/format.hpp>
-#include <sdbusplus/bus/match.hpp>
 #include <sdbusplus/unpack_properties.hpp>
 
 #include <memory>
@@ -18,6 +16,82 @@
 
 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;
+
+    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);
+    }
+
+    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;
+    }
+
+    // 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;
+
+    if (userGroups != nullptr)
+    {
+        // Populate session with user groups.
+        for (const auto& userGroup : *userGroups)
+        {
+            req.session->userGroups.emplace_back(userGroup);
+        }
+    }
+
+    return true;
+}
 
 inline bool
     isUserPrivileged(Request& req,
@@ -54,42 +128,62 @@ inline bool
         return false;
     }
 
+    req.userRole = req.session->userRole;
     return true;
 }
 
 template <typename CallbackFn>
-void validatePrivilege(Request& req,
-                       const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
-                       BaseRule& rule, CallbackFn&& callback)
+void afterGetUserInfo(Request& req,
+                      const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+                      BaseRule& rule, CallbackFn&& callback,
+                      const boost::system::error_code& ec,
+                      const dbus::utility::DBusPropertiesMap& userInfoMap)
 {
-    if (req.session == nullptr)
+    if (ec)
     {
+        BMCWEB_LOG_ERROR("GetUserInfo failed...");
+        asyncResp->res.result(
+            boost::beast::http::status::internal_server_error);
         return;
     }
-    std::string username = req.session->username;
-    UserFields props =
-        UserRoleMap::getInstance().getUserRole(req.session->username);
-    if (props.userRole)
-    {
-        req.session->userRole = props.userRole.value_or("");
-    }
-    if (props.passwordExpired)
-    {
-        req.session->isConfigureSelfOnly = *props.passwordExpired;
-    }
-    if (props.userGroups)
+
+    if (!populateUserInfo(req, asyncResp, userInfoMap))
     {
-        req.session->userGroups = std::move(*props.userGroups);
+        BMCWEB_LOG_ERROR("Failed to populate user information");
+        asyncResp->res.result(
+            boost::beast::http::status::internal_server_error);
+        return;
     }
 
     if (!isUserPrivileged(req, asyncResp, rule))
     {
         // User is not privileged
-        BMCWEB_LOG_WARNING("Insufficient Privilege");
+        BMCWEB_LOG_ERROR("Insufficient Privilege");
         asyncResp->res.result(boost::beast::http::status::forbidden);
         return;
     }
     callback(req);
 }
 
+template <typename CallbackFn>
+void validatePrivilege(Request& req,
+                       const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+                       BaseRule& rule, CallbackFn&& callback)
+{
+    if (req.session == nullptr)
+    {
+        return;
+    }
+    std::string username = req.session->username;
+    crow::connections::systemBus->async_method_call(
+        [&req, asyncResp, &rule, callback(std::forward<CallbackFn>(callback))](
+            const boost::system::error_code& ec,
+            const dbus::utility::DBusPropertiesMap& userInfoMap) mutable {
+        afterGetUserInfo(req, asyncResp, rule,
+                         std::forward<CallbackFn>(callback), ec, userInfoMap);
+        },
+        "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
+        "xyz.openbmc_project.User.Manager", "GetUserInfo", username);
+}
+
 } // namespace crow
diff --git a/include/sessions.hpp b/include/sessions.hpp
index 90a1de93de..cb7f78e78a 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -2,12 +2,10 @@
 
 #include "logging.hpp"
 #include "ossl_random.hpp"
-#include "user_role_map.hpp"
 #include "utility.hpp"
 #include "utils/ip_utils.hpp"
 
 #include <nlohmann/json.hpp>
-#include <sdbusplus/message.hpp>
 
 #include <algorithm>
 #include <csignal>
@@ -258,15 +256,11 @@ class SessionStore
             }
         }
 
-        std::string userRole = crow::UserRoleMap::getInstance()
-                                   .getUserRole(username)
-                                   .userRole.value_or("");
-
         auto session = std::make_shared<UserSession>(UserSession{
             uniqueId, sessionToken, std::string(username), csrfToken, clientId,
             redfish::ip_util::toString(clientIp),
             std::chrono::steady_clock::now(), persistence, false,
-            isConfigureSelfOnly, userRole});
+            isConfigureSelfOnly});
         auto it = authTokens.emplace(sessionToken, session);
         // Only need to write to disk if session isn't about to be destroyed.
         needWrite = persistence == PersistenceType::TIMEOUT;
diff --git a/include/user_role_map.hpp b/include/user_role_map.hpp
deleted file mode 100644
index b574c4e7af..0000000000
--- a/include/user_role_map.hpp
+++ /dev/null
@@ -1,278 +0,0 @@
-#pragma once
-
-#include "dbus_utility.hpp"
-#include "logging.hpp"
-#include "utils/dbus_utils.hpp"
-
-#include <boost/container/flat_map.hpp>
-#include <boost/url/format.hpp>
-#include <sdbusplus/bus/match.hpp>
-#include <sdbusplus/unpack_properties.hpp>
-
-#include <functional>
-#include <memory>
-#include <optional>
-#include <string>
-#include <variant>
-#include <vector>
-
-namespace crow
-{
-struct UserFields
-{
-    std::optional<std::string> userRole;
-    std::optional<bool> remote;
-    std::optional<bool> passwordExpired;
-    std::optional<std::vector<std::string>> userGroups;
-};
-
-struct UserRoleMap
-{
-  public:
-    static UserRoleMap& getInstance()
-    {
-        static UserRoleMap userRoleMap;
-        return userRoleMap;
-    }
-
-    UserFields getUserRole(std::string_view name)
-    {
-        auto it = roleMap.find(name);
-        if (it == roleMap.end())
-        {
-            BMCWEB_LOG_ERROR("User name {} is not found in the UserRoleMap.",
-                             name);
-            return {};
-        }
-        return it->second;
-    }
-    UserRoleMap(const UserRoleMap&) = delete;
-    UserRoleMap& operator=(const UserRoleMap&) = delete;
-    UserRoleMap(UserRoleMap&&) = delete;
-    UserRoleMap& operator=(UserRoleMap&&) = delete;
-    ~UserRoleMap() = default;
-
-  private:
-    static UserFields extractUserRole(
-        const dbus::utility::DBusInteracesMap& interfacesProperties)
-    {
-        UserFields fields;
-        for (const auto& interface : interfacesProperties)
-        {
-            for (const auto& property : interface.second)
-            {
-                if (property.first == "UserPrivilege")
-                {
-                    const std::string* role =
-                        std::get_if<std::string>(&property.second);
-                    if (role != nullptr)
-                    {
-                        fields.userRole = *role;
-                    }
-                }
-                else if (property.first == "UserGroups")
-                {
-                    const std::vector<std::string>* groups =
-                        std::get_if<std::vector<std::string>>(&property.second);
-                    if (groups != nullptr)
-                    {
-                        fields.userGroups = *groups;
-                    }
-                }
-                else if (property.first == "UserPasswordExpired")
-                {
-                    const bool* expired = std::get_if<bool>(&property.second);
-                    if (expired != nullptr)
-                    {
-                        fields.passwordExpired = *expired;
-                    }
-                }
-                else if (property.first == "RemoteUser")
-                {
-                    const bool* remote = std::get_if<bool>(&property.second);
-                    if (remote != nullptr)
-                    {
-                        fields.remote = *remote;
-                    }
-                }
-            }
-        }
-        return fields;
-    }
-
-    void userAdded(sdbusplus::message_t& m)
-    {
-        BMCWEB_LOG_DEBUG("User Added");
-        sdbusplus::message::object_path objPath;
-        dbus::utility::DBusInteracesMap interfacesProperties;
-
-        try
-        {
-            m.read(objPath, interfacesProperties);
-        }
-        catch (const sdbusplus::exception::SdBusError& e)
-        {
-            BMCWEB_LOG_ERROR(
-                "Failed to parse user add signal.ERROR={}REPLY_SIG={}",
-                e.what(), m.get_signature());
-            return;
-        }
-        BMCWEB_LOG_DEBUG("obj path = {}", objPath.str);
-
-        std::string name = objPath.filename();
-        if (name.empty())
-        {
-            return;
-        }
-        UserFields role = extractUserRole(interfacesProperties);
-
-        // Insert the newly added user name and the role
-        auto res = roleMap.emplace(name, role);
-        if (!res.second)
-        {
-            BMCWEB_LOG_ERROR(
-                "Insertion of the user=\"{}\" in the roleMap failed.", name);
-            return;
-        }
-    }
-
-    void userRemoved(sdbusplus::message_t& m)
-    {
-        BMCWEB_LOG_DEBUG("User Removed");
-        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={}REPLY_SIG={}", e.what(),
-                             m.get_signature());
-            return;
-        }
-
-        BMCWEB_LOG_DEBUG("obj path = {}", objPath.str);
-
-        std::string name = objPath.filename();
-        if (name.empty())
-        {
-            return;
-        }
-
-        roleMap.erase(name);
-    }
-
-    void userPropertiesChanged(sdbusplus::message_t& m)
-    {
-        BMCWEB_LOG_DEBUG("Properties Changed");
-        std::string interface;
-        dbus::utility::DBusPropertiesMap changedProperties;
-        try
-        {
-            m.read(interface, changedProperties);
-        }
-        catch (const sdbusplus::exception::SdBusError& e)
-        {
-            BMCWEB_LOG_ERROR("Failed to parse user properties changed signal.");
-            BMCWEB_LOG_ERROR("ERROR={}REPLY_SIG={}", e.what(),
-                             m.get_signature());
-            return;
-        }
-        dbus::utility::DBusInteracesMap map;
-        map.emplace_back("xyz.openbmc_project.User.Attributes",
-                         changedProperties);
-        const sdbusplus::message::object_path path(m.get_path());
-
-        BMCWEB_LOG_DEBUG("Object Path = \"{}\"", path.str);
-
-        std::string user = path.filename();
-        if (user.empty())
-        {
-            return;
-        }
-
-        BMCWEB_LOG_DEBUG("User Name = \"{}\"", user);
-
-        UserFields role = extractUserRole(map);
-
-        auto userProps = roleMap.find(user);
-        if (userProps == roleMap.end())
-        {
-            BMCWEB_LOG_CRITICAL("User {} not found", user);
-            return;
-        }
-        if (role.userRole)
-        {
-            userProps->second.userRole = role.userRole;
-        }
-        if (role.remote)
-        {
-            userProps->second.remote = role.remote;
-        }
-        if (role.userGroups)
-        {
-            userProps->second.userGroups = role.userGroups;
-        }
-        if (role.passwordExpired)
-        {
-            userProps->second.passwordExpired = role.passwordExpired;
-        }
-    }
-
-    void onGetManagedObjects(
-        const boost::system::error_code& ec,
-        const dbus::utility::ManagedObjectType& managedObjects)
-    {
-        if (ec)
-        {
-            BMCWEB_LOG_DEBUG("User manager call failed, ignoring");
-            return;
-        }
-
-        for (const auto& managedObj : managedObjects)
-        {
-            std::string name =
-                sdbusplus::message::object_path(managedObj.first).filename();
-            if (name.empty())
-            {
-                continue;
-            }
-            UserFields role = extractUserRole(managedObj.second);
-            roleMap.emplace(name, role);
-        }
-    }
-
-    static constexpr const char* userObjPath = "/xyz/openbmc_project/user";
-
-    UserRoleMap() :
-        userAddedSignal(
-            *crow::connections::systemBus,
-            sdbusplus::bus::match::rules::interfacesAdded(userObjPath),
-            std::bind_front(&UserRoleMap::userAdded, this)),
-        userRemovedSignal(
-            *crow::connections::systemBus,
-            sdbusplus::bus::match::rules::interfacesRemoved(userObjPath),
-            std::bind_front(&UserRoleMap::userRemoved, this)),
-        userPropertiesChangedSignal(
-            *crow::connections::systemBus,
-            sdbusplus::bus::match::rules::propertiesChangedNamespace(
-                userObjPath, "xyz.openbmc_project.User.Attributes"),
-            std::bind_front(&UserRoleMap::userPropertiesChanged, this))
-    {
-        dbus::utility::getManagedObjects(
-            "xyz.openbmc_project.User.Manager", {userObjPath},
-            std::bind_front(&UserRoleMap::onGetManagedObjects, this));
-    }
-
-    // Map of username -> role
-    boost::container::flat_map<std::string, UserFields, std::less<>> roleMap;
-
-    // These MUST be last, otherwise destruction can cause race conditions.
-    sdbusplus::bus::match_t userAddedSignal;
-    sdbusplus::bus::match_t userRemovedSignal;
-    sdbusplus::bus::match_t userPropertiesChangedSignal;
-};
-
-} // namespace crow