Make SessionStore a proper singleton

- SessionStore class now has a proper singleton structure
- session_storage_singleton.hpp is removed
- from_json(..) function for SessionStore is changed to a specialized
template
- minor cosmetic fixes added
- Move the template class usages of Crow App over to a non-template 
parameter


Change-Id: Ic9effd5b7bac089a84c80a0caa97bd46d4984416
Signed-off-by: Borawski.Lukasz <lukasz.borawski@intel.com>
Signed-off-by: Ed Tanous <ed.tanous@intel.com>

13 files changed, 115 insertions, 87 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index b55b7c3123..c050853ccc 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -190,6 +190,7 @@ if (${BMCWEB_BUILD_UT})
          src/kvm_websocket_test.cpp src/msan_test.cpp
          src/ast_video_puller_test.cpp
          src/openbmc_jtag_rest_test.cpp
+         redfish-core/ut/privileges_test.cpp
 	 ${CMAKE_BINARY_DIR}/include/bmcweb/blns.hpp) # big list of naughty
                                                       # strings
     add_custom_command (OUTPUT ${CMAKE_BINARY_DIR}/include/bmcweb/blns.hpp
diff --git a/include/persistent_data_middleware.hpp b/include/persistent_data_middleware.hpp
index 77a5bbbf0a..cf32e071e5 100644
--- a/include/persistent_data_middleware.hpp
+++ b/include/persistent_data_middleware.hpp
@@ -2,9 +2,9 @@
 
 #include <nlohmann/json.hpp>
 #include <pam_authenticate.hpp>
+#include <sessions.hpp>
 #include <webassets.hpp>
 #include <random>
-#include "session_storage_singleton.hpp"
 #include <crow/app.h>
 #include <crow/http_request.h>
 #include <crow/http_response.h>
@@ -28,7 +28,7 @@ class Middleware {
   Middleware() { read_data(); }
 
   ~Middleware() {
-    if (PersistentData::session_store->needs_write()) {
+    if (PersistentData::SessionStore::getInstance().needs_write()) {
       write_data();
     }
   }
@@ -68,11 +68,12 @@ class Middleware {
         if (jSessions != data.end()) {
           if (jSessions->is_object()) {
             for (const auto& elem : *jSessions) {
-              std::shared_ptr<UserSession> newSession = std::make_shared<UserSession>();
+              std::shared_ptr<UserSession> newSession =
+                  std::make_shared<UserSession>();
 
               if (newSession->fromJson(elem)) {
-                session_store->auth_tokens.emplace(newSession->unique_id,
-                                                   newSession);
+                SessionStore::getInstance().auth_tokens.emplace(
+                    newSession->unique_id, newSession);
               }
             }
           }
@@ -97,7 +98,7 @@ class Middleware {
   void write_data() {
     std::ofstream persistent_file(filename);
     nlohmann::json data;
-    data["sessions"] = PersistentData::session_store->auth_tokens;
+    data["sessions"] = PersistentData::SessionStore::getInstance().auth_tokens;
     data["system_uuid"] = system_uuid;
     data["revision"] = json_revision;
     persistent_file << data;
@@ -106,5 +107,5 @@ class Middleware {
   std::string system_uuid;
 };
 
-}  // namespaec PersistentData
+}  // namespace PersistentData
 }  // namespace crow
diff --git a/include/session_storage_singleton.hpp b/include/session_storage_singleton.hpp
deleted file mode 100644
index 6ff8a0e303..0000000000
--- a/include/session_storage_singleton.hpp
+++ /dev/null
@@ -1,10 +0,0 @@
-#pragma once
-#include "sessions.hpp"
-
-namespace crow {
-namespace PersistentData {
-
-static std::shared_ptr<SessionStore> session_store;
-
-}  // namespace PersistentData
-}  // namespace crow
diff --git a/include/sessions.hpp b/include/sessions.hpp
index fdd12000f4..b0fe1122b4 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -73,20 +73,10 @@ struct UserSession {
   }
 };
 
-void to_json(nlohmann::json& j, const std::shared_ptr<UserSession> p) {
-  if (p->persistence != PersistenceType::SINGLE_REQUEST) {
-    j = {{"unique_id", p->unique_id},
-         {"session_token", p->session_token},
-         {"username", p->username},
-         {"csrf_token", p->csrf_token}};
-  }
-}
-
 class Middleware;
 
 class SessionStore {
  public:
-  SessionStore() : timeout_in_minutes(60) {}
   std::shared_ptr<UserSession> generate_user_session(
       const boost::string_view username,
       PersistenceType persistence = PersistenceType::TIMEOUT) {
@@ -184,7 +174,17 @@ class SessionStore {
   // structure, which is private
   friend Middleware;
 
+  static SessionStore& getInstance() {
+    static SessionStore sessionStore;
+    return sessionStore;
+  }
+
+  SessionStore(const SessionStore&) = delete;
+  SessionStore& operator=(const SessionStore&) = delete;
+
  private:
+  SessionStore() : timeout_in_minutes(60) {}
+
   void apply_session_timeouts() {
     auto time_now = std::chrono::steady_clock::now();
     if (time_now - last_timeout_update > std::chrono::minutes(1)) {
@@ -211,3 +211,21 @@ class SessionStore {
 
 }  // namespace PersistentData
 }  // namespace crow
+
+// to_json(...) definition for objects of UserSession type
+namespace nlohmann {
+template <>
+struct adl_serializer<std::shared_ptr<crow::PersistentData::UserSession>> {
+  static void to_json(
+      nlohmann::json& j,
+      const std::shared_ptr<crow::PersistentData::UserSession>& p) {
+    if (p->persistence !=
+        crow::PersistentData::PersistenceType::SINGLE_REQUEST) {
+      j = nlohmann::json{{"unique_id", p->unique_id},
+                         {"session_token", p->session_token},
+                         {"username", p->username},
+                         {"csrf_token", p->csrf_token}};
+    }
+  }
+};
+}  // namespace nlohmann
diff --git a/include/token_authorization_middleware.hpp b/include/token_authorization_middleware.hpp
index a40469fea2..f151e4ff63 100644
--- a/include/token_authorization_middleware.hpp
+++ b/include/token_authorization_middleware.hpp
@@ -76,7 +76,7 @@ class Middleware {
     if (ctx.session != nullptr &&
         ctx.session->persistence ==
             crow::PersistentData::PersistenceType::SINGLE_REQUEST) {
-      PersistentData::session_store->remove_session(ctx.session);
+      PersistentData::SessionStore::getInstance().remove_session(ctx.session);
     }
   }
 
@@ -114,7 +114,7 @@ class Middleware {
     // needed.
     // This whole flow needs to be revisited anyway, as we can't be
     // calling directly into pam for every request
-    return PersistentData::session_store->generate_user_session(
+    return PersistentData::SessionStore::getInstance().generate_user_session(
         user, crow::PersistentData::PersistenceType::SINGLE_REQUEST);
   }
 
@@ -123,7 +123,9 @@ class Middleware {
     CROW_LOG_DEBUG << "[AuthMiddleware] Token authentication";
 
     boost::string_view token = auth_header.substr(strlen("Token "));
-    auto session = PersistentData::session_store->login_session_by_token(token);
+    auto session =
+        PersistentData::SessionStore::getInstance().login_session_by_token(
+            token);
     return session;
   }
 
@@ -135,7 +137,9 @@ class Middleware {
     if (token.empty()) {
       return nullptr;
     }
-    auto session = PersistentData::session_store->login_session_by_token(token);
+    auto session =
+        PersistentData::SessionStore::getInstance().login_session_by_token(
+            token);
     return session;
   }
 
@@ -161,7 +165,8 @@ class Middleware {
         cookie_value.substr(start_index, end_index - start_index);
 
     const std::shared_ptr<crow::PersistentData::UserSession> session =
-        PersistentData::session_store->login_session_by_token(auth_key);
+        PersistentData::SessionStore::getInstance().login_session_by_token(
+            auth_key);
     if (session == nullptr) {
       return nullptr;
     }
@@ -303,8 +308,8 @@ void request_routes(Crow<Middlewares...>& app) {
           if (!pam_authenticate_user(username, password)) {
             res.result(boost::beast::http::status::unauthorized);
           } else {
-            auto session =
-                PersistentData::session_store->generate_user_session(username);
+            auto session = PersistentData::SessionStore::getInstance()
+                               .generate_user_session(username);
 
             if (looks_like_ibm) {
               // IBM requires a very specific login structure, and doesn't
@@ -341,18 +346,17 @@ void request_routes(Crow<Middlewares...>& app) {
       });
 
   CROW_ROUTE(app, "/logout")
-      .methods("POST"_method)(
-          [&](const crow::request& req, crow::response& res) {
-            auto& session =
-                app.template get_context<TokenAuthorization::Middleware>(req)
-                    .session;
-            if (session != nullptr) {
-              PersistentData::session_store->remove_session(session);
-            }
-            res.end();
-            return;
-
-          });
+      .methods(
+          "POST"_method)([&](const crow::request& req, crow::response& res) {
+        auto& session =
+            app.template get_context<TokenAuthorization::Middleware>(req)
+                .session;
+        if (session != nullptr) {
+          PersistentData::SessionStore::getInstance().remove_session(session);
+        }
+        res.end();
+        return;
+      });
 }
 }  // namespace TokenAuthorization
 }  // namespace crow
diff --git a/include/webserver_common.hpp b/include/webserver_common.hpp
index 1ba091b9c9..4d88629d26 100644
--- a/include/webserver_common.hpp
+++ b/include/webserver_common.hpp
@@ -15,6 +15,7 @@
 */
 #pragma once
 
+#include "security_headers_middleware.hpp"
 #include "token_authorization_middleware.hpp"
 #include "security_headers_middleware.hpp"
 #include "webserver_common.hpp"
@@ -22,4 +23,3 @@
 using CrowApp = crow::App<crow::PersistentData::Middleware,
                           crow::TokenAuthorization::Middleware,
                           crow::SecurityHeadersMiddleware>;
-
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 48872f8ec8..9071067eb3 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -21,7 +21,6 @@ namespace redfish {
 
 class AccountService : public Node {
  public:
-  template <typename CrowApp>
   AccountService(CrowApp& app) : Node(app, "/redfish/v1/AccountService/") {
     Node::json["@odata.id"] = "/redfish/v1/AccountService";
     Node::json["@odata.type"] = "#AccountService.v1_1_0.AccountService";
@@ -37,7 +36,8 @@ class AccountService : public Node {
     Node::json["Roles"]["@odata.id"] = "/redfish/v1/AccountService/Roles";
 
     entityPrivileges = {
-        {boost::beast::http::verb::get, {{"ConfigureUsers"}, {"ConfigureManager"}}},
+        {boost::beast::http::verb::get,
+         {{"ConfigureUsers"}, {"ConfigureManager"}}},
         {boost::beast::http::verb::head, {{"Login"}}},
         {boost::beast::http::verb::patch, {{"ConfigureUsers"}}},
         {boost::beast::http::verb::put, {{"ConfigureUsers"}}},
diff --git a/redfish-core/lib/chassis.hpp b/redfish-core/lib/chassis.hpp
index 5cf667a00b..ef3b7afc63 100644
--- a/redfish-core/lib/chassis.hpp
+++ b/redfish-core/lib/chassis.hpp
@@ -100,7 +100,6 @@ class OnDemandChassisProvider {
  */
 class ChassisCollection : public Node {
  public:
-  template <typename CrowApp>
   ChassisCollection(CrowApp &app) : Node(app, "/redfish/v1/Chassis/") {
     Node::json["@odata.type"] = "#ChassisCollection.ChassisCollection";
     Node::json["@odata.id"] = "/redfish/v1/Chassis";
@@ -155,10 +154,6 @@ class ChassisCollection : public Node {
  */
 class Chassis : public Node {
  public:
-  /*
-   * Default Constructor
-   */
-  template <typename CrowApp>
   Chassis(CrowApp &app)
       : Node(app, "/redfish/v1/Chassis/<str>/", std::string()) {
     Node::json["@odata.type"] = "#Chassis.v1_4_0.Chassis";
diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp
index 32b697a695..3bbcff4fa5 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -737,7 +737,6 @@ class OnDemandEthernetProvider {
  */
 class EthernetCollection : public Node {
  public:
-  template <typename CrowApp>
   // TODO(Pawel) Remove line from below, where we assume that there is only one
   // manager called openbmc This shall be generic, but requires to update
   // GetSubroutes method
@@ -809,7 +808,6 @@ class EthernetInterface : public Node {
   /*
    * Default Constructor
    */
-  template <typename CrowApp>
   // TODO(Pawel) Remove line from below, where we assume that there is only one
   // manager called openbmc This shall be generic, but requires to update
   // GetSubroutes method
diff --git a/redfish-core/lib/network_protocol.hpp b/redfish-core/lib/network_protocol.hpp
index 438de41efc..d7bf1016ad 100644
--- a/redfish-core/lib/network_protocol.hpp
+++ b/redfish-core/lib/network_protocol.hpp
@@ -22,8 +22,7 @@ namespace redfish {
 class NetworkProtocol : public Node {
  public:
   NetworkProtocol(CrowApp& app)
-      : Node(app,
-             "/redfish/v1/Managers/openbmc/NetworkProtocol") {
+      : Node(app, "/redfish/v1/Managers/openbmc/NetworkProtocol") {
     Node::json["@odata.type"] =
         "#ManagerNetworkProtocol.v1_1_0.ManagerNetworkProtocol";
     Node::json["@odata.id"] = "/redfish/v1/Managers/openbmc/NetworkProtocol";
@@ -36,12 +35,13 @@ class NetworkProtocol : public Node {
     Node::json["Status"]["HealthRollup"] = "OK";
     Node::json["Status"]["State"] = "Enabled";
 
-    entityPrivileges = {{boost::beast::http::verb::get, {{"Login"}}},
-                        {boost::beast::http::verb::head, {{"Login"}}},
-                        {boost::beast::http::verb::patch, {{"ConfigureManager"}}},
-                        {boost::beast::http::verb::put, {{"ConfigureManager"}}},
-                        {boost::beast::http::verb::delete_, {{"ConfigureManager"}}},
-                        {boost::beast::http::verb::post, {{"ConfigureManager"}}}};
+    entityPrivileges = {
+        {boost::beast::http::verb::get, {{"Login"}}},
+        {boost::beast::http::verb::head, {{"Login"}}},
+        {boost::beast::http::verb::patch, {{"ConfigureManager"}}},
+        {boost::beast::http::verb::put, {{"ConfigureManager"}}},
+        {boost::beast::http::verb::delete_, {{"ConfigureManager"}}},
+        {boost::beast::http::verb::post, {{"ConfigureManager"}}}};
   }
 
  private:
diff --git a/redfish-core/lib/redfish_sessions.hpp b/redfish-core/lib/redfish_sessions.hpp
index 7de6d3ba9a..9e793df049 100644
--- a/redfish-core/lib/redfish_sessions.hpp
+++ b/redfish-core/lib/redfish_sessions.hpp
@@ -17,7 +17,7 @@
 
 #include "error_messages.hpp"
 #include "node.hpp"
-#include "session_storage_singleton.hpp"
+#include "persistent_data_middleware.hpp"
 
 namespace redfish {
 
@@ -45,7 +45,8 @@ class Sessions : public Node {
   void doGet(crow::response& res, const crow::request& req,
              const std::vector<std::string>& params) override {
     auto session =
-        crow::PersistentData::session_store->get_session_by_uid(params[0]);
+        crow::PersistentData::SessionStore::getInstance().get_session_by_uid(
+            params[0]);
 
     if (session == nullptr) {
       messages::addMessageToErrorJson(
@@ -81,7 +82,8 @@ class Sessions : public Node {
     }
 
     auto session =
-        crow::PersistentData::session_store->get_session_by_uid(params[0]);
+        crow::PersistentData::SessionStore::getInstance().get_session_by_uid(
+            params[0]);
 
     if (session == nullptr) {
       messages::addMessageToErrorJson(
@@ -95,7 +97,7 @@ class Sessions : public Node {
     // DELETE should return representation of object that will be removed
     doGet(res, req, params);
 
-    crow::PersistentData::session_store->remove_session(session);
+    crow::PersistentData::SessionStore::getInstance().remove_session(session);
   }
 
   /**
@@ -132,7 +134,7 @@ class SessionCollection : public Node {
   void doGet(crow::response& res, const crow::request& req,
              const std::vector<std::string>& params) override {
     std::vector<const std::string*> session_ids =
-        crow::PersistentData::session_store->get_unique_ids(
+        crow::PersistentData::SessionStore::getInstance().get_unique_ids(
             false, crow::PersistentData::PersistenceType::TIMEOUT);
 
     Node::json["Members@odata.count"] = session_ids.size();
@@ -161,7 +163,8 @@ class SessionCollection : public Node {
 
     // User is authenticated - create session for him
     auto session =
-        crow::PersistentData::session_store->generate_user_session(username);
+        crow::PersistentData::SessionStore::getInstance().generate_user_session(
+            username);
     res.add_header("X-Auth-Token", session->session_token);
 
     // Return data for created session
@@ -301,7 +304,8 @@ class SessionService : public Node {
     Node::json["Id"] = "SessionService";
     Node::json["Description"] = "Session Service";
     Node::json["SessionTimeout"] =
-        crow::PersistentData::session_store->get_timeout_in_seconds();
+        crow::PersistentData::SessionStore::getInstance()
+            .get_timeout_in_seconds();
     Node::json["ServiceEnabled"] = true;
 
     entityPrivileges = {
diff --git a/src/token_authorization_middleware_test.cpp b/src/token_authorization_middleware_test.cpp
index 8f458349e5..47acc857fd 100644
--- a/src/token_authorization_middleware_test.cpp
+++ b/src/token_authorization_middleware_test.cpp
@@ -1,18 +1,37 @@
 #include "token_authorization_middleware.hpp"
-#include <crow/app.h>
+#include <condition_variable>
+#include <future>
+#include <mutex>
+#include "webserver_common.hpp"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 using namespace crow;
-using namespace std;
 
-// Tests that static urls are correctly passed
-TEST(TokenAuthentication, TestBasicReject) {
-  App<crow::PersistentData::Middleware, crow::TokenAuthorization::Middleware>
-      app;
-  decltype(app)::server_t server(&app, "127.0.0.1", 45451);
-  CROW_ROUTE(app, "/")([]() { return boost::beast::http::status::ok; });
-  auto _ = async(launch::async, [&] { server.run(); });
+class TokenAuth : public ::testing::Test {
+ public:
+  TokenAuth()
+      : lk(std::unique_lock<std::mutex>(m)),
+        io(std::make_shared<boost::asio::io_service>()) {}
+
+  std::mutex m;
+  std::condition_variable cv;
+  std::unique_lock<std::mutex> lk;
+  std::shared_ptr<boost::asio::io_service> io;
+  int testPort = 45451;
+};
+
+TEST_F(TokenAuth, SpecialResourcesAreAcceptedWithoutAuth) {
+  CrowApp app(io);
+  crow::TokenAuthorization::request_routes(app);
+  CROW_ROUTE(app, "/redfish/v1")
+  ([]() { return boost::beast::http::status::ok; });
+  auto _ = std::async(std::launch::async, [&] {
+    app.port(testPort).run();
+    cv.notify_one();
+    io->run();
+  });
+
   asio::io_service is;
   std::string sendmsg;
 
@@ -42,7 +61,7 @@ TEST(TokenAuthentication, TestBasicReject) {
     EXPECT_EQ("404", std::string(buf + 9, buf + 12));
   }
 
-  server.stop();
+  app.stop();
 }
 
 // Tests that Base64 basic strings work
@@ -51,7 +70,7 @@ TEST(TokenAuthentication, TestRejectedResource) {
       app;
   app.bindaddr("127.0.0.1").port(45451);
   CROW_ROUTE(app, "/")([]() { return boost::beast::http::status::ok; });
-  auto _ = async(launch::async, [&] { app.run(); });
+  auto _ = async(std::launch::async, [&] { app.run(); });
 
   asio::io_service is;
   static char buf[2048];
@@ -81,7 +100,7 @@ TEST(TokenAuthentication, TestGetLoginUrl) {
       app;
   app.bindaddr("127.0.0.1").port(45451);
   CROW_ROUTE(app, "/")([]() { return boost::beast::http::status::ok; });
-  auto _ = async(launch::async, [&] { app.run(); });
+  auto _ = async(std::launch::async, [&] { app.run(); });
 
   asio::io_service is;
   static char buf[2048];
@@ -111,7 +130,7 @@ TEST(TokenAuthentication, TestPostBadLoginUrl) {
       app;
   app.bindaddr("127.0.0.1").port(45451);
   CROW_ROUTE(app, "/")([]() { return boost::beast::http::status::ok; });
-  auto _ = async(launch::async, [&] { app.run(); });
+  auto _ = async(std::launch::async, [&] { app.run(); });
 
   asio::io_service is;
   std::array<char, 2048> buf;
@@ -195,7 +214,7 @@ TEST(TokenAuthentication, TestSuccessfulLogin) {
       app;
   app.bindaddr("127.0.0.1").port(45451);
   CROW_ROUTE(app, "/")([]() { return boost::beast::http::status::ok; });
-  auto _ = async(launch::async, [&] { app.run(); });
+  auto _ = async(std::launch::async, [&] { app.run(); });
 
   asio::io_service is;
   std::array<char, 2048> buf;
diff --git a/src/webserver_main.cpp b/src/webserver_main.cpp
index 6454ed32ec..55152d6033 100644
--- a/src/webserver_main.cpp
+++ b/src/webserver_main.cpp
@@ -47,8 +47,6 @@ int main(int argc, char** argv) {
   crow::logger::setLogLevel(crow::LogLevel::DEBUG);
 
   auto io = std::make_shared<boost::asio::io_service>();
-  crow::PersistentData::session_store =
-      std::make_shared<crow::PersistentData::SessionStore>();
   CrowApp app(io);
 
 #ifdef CROW_ENABLE_SSL