Refactor session storage
Session storage had a few bugs, and a number of old practices. This
moves the session storage closer to the best practices. It enforces
the use of a factory function for generating new sessions, as well as
using get_ptr when reading the sessions out.
Change-Id: Ia252076f21e47b99f8057190349355838fdd787d
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
diff --git a/include/persistent_data_middleware.hpp b/include/persistent_data_middleware.hpp
index cf32e07..334a84d 100644
--- a/include/persistent_data_middleware.hpp
+++ b/include/persistent_data_middleware.hpp
@@ -46,36 +46,47 @@
if (persistent_file.is_open()) {
// call with exceptions disabled
auto data = nlohmann::json::parse(persistent_file, nullptr, false);
- if (!data.is_discarded()) {
- auto jRevision = data.find("revision");
- auto jUuid = data.find("system_uuid");
- auto jSessions = data.find("sessions");
+ if (data.is_discarded()) {
+ CROW_LOG_ERROR << "Error parsing persistent data in json file.";
+ } else {
+ for (const auto& item : data.items()) {
+ if (item.key() == "revision") {
+ file_revision = 0;
- file_revision = 0;
- if (jRevision != data.end()) {
- if (jRevision->is_number_integer()) {
- file_revision = jRevision->get<int>();
- }
- }
-
- system_uuid = "";
- if (jUuid != data.end()) {
- if (jUuid->is_string()) {
- system_uuid = jUuid->get<std::string>();
- }
- }
-
- if (jSessions != data.end()) {
- if (jSessions->is_object()) {
- for (const auto& elem : *jSessions) {
- std::shared_ptr<UserSession> newSession =
- std::make_shared<UserSession>();
-
- if (newSession->fromJson(elem)) {
- SessionStore::getInstance().auth_tokens.emplace(
- newSession->unique_id, newSession);
- }
+ const uint64_t* uintPtr = item.value().get_ptr<const uint64_t*>();
+ if (uintPtr == nullptr) {
+ CROW_LOG_ERROR << "Failed to read revision flag";
+ } else {
+ file_revision = *uintPtr;
}
+ } else if (item.key() == "system_uuid") {
+ const std::string* jSystemUuid =
+ item.value().get_ptr<const std::string*>();
+ if (jSystemUuid != nullptr) {
+ system_uuid = *jSystemUuid;
+ }
+ } else if (item.key() == "sessions") {
+ for (const auto& elem : item.value()) {
+ std::shared_ptr<UserSession> newSession =
+ UserSession::fromJson(elem);
+
+ if (newSession == nullptr) {
+ CROW_LOG_ERROR
+ << "Problem reading session from persistent store";
+ continue;
+ }
+
+ CROW_LOG_DEBUG << "Restored session: " << newSession->csrf_token
+ << " " << newSession->unique_id << " "
+ << newSession->session_token;
+ SessionStore::getInstance().auth_tokens.emplace(
+ newSession->session_token, newSession);
+ }
+ } else {
+ // Do nothing in the case of extra fields. We may have cases where
+ // fields are added in the future, and we want to at least attempt
+ // to gracefully support downgrades in that case, even if we don't
+ // officially support it
}
}
}
@@ -97,14 +108,14 @@
void write_data() {
std::ofstream persistent_file(filename);
- nlohmann::json data;
- data["sessions"] = PersistentData::SessionStore::getInstance().auth_tokens;
- data["system_uuid"] = system_uuid;
- data["revision"] = json_revision;
+ nlohmann::json data{
+ {"sessions", PersistentData::SessionStore::getInstance().auth_tokens},
+ {"system_uuid", system_uuid},
+ {"revision", json_revision}};
persistent_file << data;
}
- std::string system_uuid;
+ std::string system_uuid{""};
};
} // namespace PersistentData
diff --git a/include/sessions.hpp b/include/sessions.hpp
index b0fe112..b4e86c7 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -36,40 +36,42 @@
*
* @param[in] j JSON object from which data should be loaded
*
- * @return true if data has been loaded properly, false otherwise
+ * @return a shared pointer if data has been loaded properly, nullptr otherwise
*/
- bool fromJson(const nlohmann::json& j) {
- auto jUid = j.find("unique_id");
- auto jToken = j.find("session_token");
- auto jUsername = j.find("username");
- auto jCsrf = j.find("csrf_token");
-
- // Verify existence
- if (jUid == j.end() || jToken == j.end() || jUsername == j.end() ||
- jCsrf == j.end()) {
- return false;
+ static std::shared_ptr<UserSession> fromJson(const nlohmann::json& j) {
+ std::shared_ptr<UserSession> userSession = std::make_shared<UserSession>();
+ for (const auto& element : j.items()) {
+ const std::string* thisValue =
+ element.value().get_ptr<const std::string*>();
+ if (thisValue == nullptr) {
+ CROW_LOG_ERROR << "Error reading persistent store. Property "
+ << element.key() << " was not of type string";
+ return nullptr;
+ }
+ if (element.key() == "unique_id") {
+ userSession->unique_id = *thisValue;
+ } else if (element.key() == "session_token") {
+ userSession->session_token = *thisValue;
+ } else if (element.key() == "csrf_token") {
+ userSession->csrf_token = *thisValue;
+ } else if (element.key() == "username") {
+ userSession->username = *thisValue;
+ } else {
+ CROW_LOG_ERROR << "Got unexpected property reading persistent file: "
+ << element.key();
+ return nullptr;
+ }
}
- // Verify types
- if (!jUid->is_string() || !jToken->is_string() || !jUsername->is_string() ||
- !jCsrf->is_string()) {
- return false;
- }
-
- unique_id = jUid->get<std::string>();
- session_token = jToken->get<std::string>();
- username = jUsername->get<std::string>();
- csrf_token = jCsrf->get<std::string>();
-
- // For now, sessions that were persisted through a reboot get their timer
- // reset. This could probably be overcome with a better understanding of
- // wall clock time and steady timer time, possibly persisting values with
+ // For now, sessions that were persisted through a reboot get their idle
+ // timer reset. This could probably be overcome with a better understanding
+ // of wall clock time and steady timer time, possibly persisting values with
// wall clock time instead of steady timer, but the tradeoffs of all the
// corner cases involved are non-trivial, so this is done temporarily
- last_updated = std::chrono::steady_clock::now();
- persistence = PersistenceType::TIMEOUT;
+ userSession->last_updated = std::chrono::steady_clock::now();
+ userSession->persistence = PersistenceType::TIMEOUT;
- return true;
+ return userSession;
}
};