Add Support for privilege check in handleUpgrade

This commit enables privilege check for user(s) in case of upgraded
connections.
Currently users with no privileges will also be able to access
Websockets connections (Ex: KVM).

The privilege check was already in place for normal connections (i.e.
router->handle()). This commit lifts off the privilege check code and
moves it into a common function (validatePrivilege()), which can be used
both by handle() and handleUpgrade() and register required callback to
be called.

Also, the const qualifier for Request in the handleUpgrade() function's
signature is removed to enable setting "isConfigureSelf" field of
request. The signature of handleUpgrade() is made identical to handle()

Tested:
 - websocket_test.py Passed
 - Admin and Operator users are able to access KVM on WebUI
 - Readonly User was unable to access KVM on WebUI

Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
Change-Id: I6f743c27e7e6077f1c6c56e6958922027e4404e8

1 files changed, 78 insertions, 45 deletions

diff --git a/http/routing.hpp b/http/routing.hpp
index 40218fbf68..e86c77417f 100644
--- a/http/routing.hpp
+++ b/http/routing.hpp
@@ -57,6 +57,7 @@ class BaseRule
     virtual void handle(const Request& /*req*/,
                         const std::shared_ptr<bmcweb::AsyncResp>&,
                         const RoutingParams&) = 0;
+#ifndef BMCWEB_ENABLE_SSL
     virtual void
         handleUpgrade(const Request& /*req*/,
                       const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
@@ -64,7 +65,7 @@ class BaseRule
     {
         asyncResp->res.result(boost::beast::http::status::not_found);
     }
-#ifdef BMCWEB_ENABLE_SSL
+#else
     virtual void handleUpgrade(
         const Request& /*req*/,
         const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
@@ -348,6 +349,7 @@ class WebSocketRule : public BaseRule
         asyncResp->res.result(boost::beast::http::status::not_found);
     }
 
+#ifndef BMCWEB_ENABLE_SSL
     void handleUpgrade(const Request& req,
                        const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/,
                        boost::asio::ip::tcp::socket&& adaptor) override
@@ -361,7 +363,7 @@ class WebSocketRule : public BaseRule
                 closeHandler, errorHandler);
         myConnection->start();
     }
-#ifdef BMCWEB_ENABLE_SSL
+#else
     void handleUpgrade(const Request& req,
                        const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/,
                        boost::beast::ssl_stream<boost::asio::ip::tcp::socket>&&
@@ -1243,20 +1245,10 @@ class Router
         return findRoute;
     }
 
-    static void
-        isUserPrivileged(const boost::system::error_code& ec, Request& req,
-                         const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
-                         BaseRule& rule, const RoutingParams& params,
-                         const dbus::utility::DBusPropertiesMap& userInfoMap)
+    static bool isUserPrivileged(
+        Request& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+        BaseRule& rule, const dbus::utility::DBusPropertiesMap& userInfoMap)
     {
-        if (ec)
-        {
-            BMCWEB_LOG_ERROR << "GetUserInfo failed...";
-            asyncResp->res.result(
-                boost::beast::http::status::internal_server_error);
-            return;
-        }
-
         std::string userRole{};
         const std::string* userRolePtr = nullptr;
         const bool* remoteUser = nullptr;
@@ -1271,7 +1263,7 @@ class Router
         {
             asyncResp->res.result(
                 boost::beast::http::status::internal_server_error);
-            return;
+            return false;
         }
 
         if (userRolePtr != nullptr)
@@ -1286,7 +1278,7 @@ class Router
             BMCWEB_LOG_ERROR << "RemoteUser property missing or wrong type";
             asyncResp->res.result(
                 boost::beast::http::status::internal_server_error);
-            return;
+            return false;
         }
         bool expired = false;
         if (passwordExpired == nullptr)
@@ -1298,7 +1290,7 @@ class Router
                        " local user but is missing or wrong type";
                 asyncResp->res.result(
                     boost::beast::http::status::internal_server_error);
-                return;
+                return false;
             }
         }
         else
@@ -1334,15 +1326,60 @@ class Router
                                         "redfish", "v1", "AccountService",
                                         "Accounts", req.session->username));
             }
-            return;
+            return false;
         }
 
         req.userRole = userRole;
-        rule.handle(req, asyncResp, params);
+
+        return true;
+    }
+
+    template <typename CallbackFn>
+    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 (ec)
+        {
+            BMCWEB_LOG_ERROR << "GetUserInfo failed...";
+            asyncResp->res.result(
+                boost::beast::http::status::internal_server_error);
+            return;
+        }
+
+        if (!Router::isUserPrivileged(req, asyncResp, rule, userInfoMap))
+        {
+            // User is not privileged
+            BMCWEB_LOG_ERROR << "Insufficient Privilege";
+            asyncResp->res.result(boost::beast::http::status::forbidden);
+            return;
+        }
+        callback();
+    }
+
+    template <typename CallbackFn>
+    void validatePrivilege(Request& req,
+                           const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+                           BaseRule& rule, CallbackFn&& callback)
+    {
+        crow::connections::systemBus->async_method_call(
+            [this, &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",
+            req.session->username);
     }
 
     template <typename Adaptor>
-    void handleUpgrade(const Request& req,
+    void handleUpgrade(Request& req,
                        const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
                        Adaptor&& adaptor)
     {
@@ -1372,24 +1409,28 @@ class Router
             throw std::runtime_error("Trie internal structure corrupted!");
         }
 
-        if ((rules[ruleIndex]->getMethods() &
-             (1U << static_cast<size_t>(*verb))) == 0)
+        BaseRule& rule = *rules[ruleIndex];
+        size_t methods = rule.getMethods();
+        if ((methods & (1U << static_cast<size_t>(*verb))) == 0)
         {
-            BMCWEB_LOG_DEBUG << "Rule found but method mismatch: "
-                             << req.url().encoded_path() << " with "
-                             << req.methodString() << "("
-                             << static_cast<uint32_t>(*verb) << ") / "
-                             << rules[ruleIndex]->getMethods();
+            BMCWEB_LOG_DEBUG
+                << "Rule found but method mismatch: "
+                << req.url().encoded_path() << " with " << req.methodString()
+                << "(" << static_cast<uint32_t>(*verb) << ") / " << methods;
             asyncResp->res.result(boost::beast::http::status::not_found);
             return;
         }
 
-        BMCWEB_LOG_DEBUG << "Matched rule (upgrade) '" << rules[ruleIndex]->rule
-                         << "' " << static_cast<uint32_t>(*verb) << " / "
-                         << rules[ruleIndex]->getMethods();
+        BMCWEB_LOG_DEBUG << "Matched rule (upgrade) '" << rule.rule << "' "
+                         << static_cast<uint32_t>(*verb) << " / " << methods;
 
-        rules[ruleIndex]->handleUpgrade(req, asyncResp,
-                                        std::forward<Adaptor>(adaptor));
+        // TODO(ed) This should be able to use std::bind_front, but it doesn't
+        // appear to work with the std::move on adaptor.
+        validatePrivilege(req, asyncResp, rule,
+                          [&rule, &req, asyncResp,
+                           adaptor(std::forward<Adaptor>(adaptor))]() mutable {
+            rule.handleUpgrade(req, asyncResp, std::move(adaptor));
+        });
     }
 
     void handle(Request& req,
@@ -1457,19 +1498,11 @@ class Router
             rule.handle(req, asyncResp, params);
             return;
         }
-        std::string username = req.session->username;
-
-        crow::connections::systemBus->async_method_call(
-            [req{std::move(req)}, asyncResp, &rule, params](
-
-                const boost::system::error_code& ec,
-                const dbus::utility::DBusPropertiesMap& userInfoMap
 
-                ) mutable {
-            isUserPrivileged(ec, req, asyncResp, rule, params, userInfoMap);
-            },
-            "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
-            "xyz.openbmc_project.User.Manager", "GetUserInfo", username);
+        validatePrivilege(req, asyncResp, rule,
+                          std::bind_front(&BaseRule::handle, std::ref(rule),
+                                          std::ref(req), asyncResp,
+                                          std::ref(params)));
     }
 
     void debugPrint()