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>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b55b7c3..c050853 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -190,6 +190,7 @@
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 77a5bbb..cf32e07 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 @@
Middleware() { read_data(); }
~Middleware() {
- if (PersistentData::session_store->needs_write()) {
+ if (PersistentData::SessionStore::getInstance().needs_write()) {
write_data();
}
}
@@ -68,11 +68,12 @@
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 @@
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 @@
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 6ff8a0e..0000000
--- 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 fdd1200..b0fe112 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -73,20 +73,10 @@
}
};
-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 @@
// 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 @@
} // 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 a40469f..f151e4f 100644
--- a/include/token_authorization_middleware.hpp
+++ b/include/token_authorization_middleware.hpp
@@ -76,7 +76,7 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
});
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 1ba091b..4d88629 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 48872f8..9071067 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -21,7 +21,6 @@
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 @@
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 5cf667a..ef3b7af 100644
--- a/redfish-core/lib/chassis.hpp
+++ b/redfish-core/lib/chassis.hpp
@@ -100,7 +100,6 @@
*/
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 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 32b697a..3bbcff4 100644
--- a/redfish-core/lib/ethernet.hpp
+++ b/redfish-core/lib/ethernet.hpp
@@ -737,7 +737,6 @@
*/
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 @@
/*
* 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 438de41..d7bf101 100644
--- a/redfish-core/lib/network_protocol.hpp
+++ b/redfish-core/lib/network_protocol.hpp
@@ -22,8 +22,7 @@
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 @@
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 7de6d3b..9e793df 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 @@
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 @@
}
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 @@
// 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 @@
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 @@
// 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 @@
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 8f45834..47acc85 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 @@
EXPECT_EQ("404", std::string(buf + 9, buf + 12));
}
- server.stop();
+ app.stop();
}
// Tests that Base64 basic strings work
@@ -51,7 +70,7 @@
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 @@
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 @@
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 @@
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 6454ed3..55152d6 100644
--- a/src/webserver_main.cpp
+++ b/src/webserver_main.cpp
@@ -47,8 +47,6 @@
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