Move Sessions to non Node structure
This commit, in line with 7e860f1550c8686eec42f7a75bc5f2ef51e756ad moves
the session service over to the normal BMCWEB routes. This is
relatively painless, with the exception of the fact that the previous
classes held members of the other classes in their struct. This was an
attempt at a design pattern from very early on that never really worked
in practice, so it was largely abandoned, and now this is cleaning up
the last remains of it.
This commit accomplishes this by making two critical changes, first,
Delete /redfish/v1/SessionService/Sessions/<sessionId> no longer returns
the structure of the session that was deleted, instead returns 204
unmodified, which is very similar to what we do in other cases. While
this is a breaking change, it's not clear what a user would even do with
a returned deleted session, so it seems really unlikely to break anyone.
This commit also creates a separate method to fill in a session object
with a given session details, such that the POST and GET methods can
share a single implementation. This is more efficient than the old way,
as it prevents a double lookup from the session store.
Tested:
Tested redfish validator on system. No new failures (UUID failure still
present)
Change-Id: If5d2b2c5a21af05ed0cb02a15bd1c1c976b8da12
Signed-off-by: Ed Tanous <edtanous@google.com>
diff --git a/redfish-core/lib/redfish_sessions.hpp b/redfish-core/lib/redfish_sessions.hpp
index f7cb7a7..7590268 100644
--- a/redfish-core/lib/redfish_sessions.hpp
+++ b/redfish-core/lib/redfish_sessions.hpp
@@ -26,301 +26,252 @@
class SessionCollection;
-class Sessions : public Node
+void fillSessionObject(crow::Response& res,
+ const persistent_data::UserSession& session)
{
- public:
- Sessions(App& app) :
- Node(app, "/redfish/v1/SessionService/Sessions/<str>/", std::string())
- {
- 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"}, {"ConfigureSelf"}}},
- {boost::beast::http::verb::post, {{"ConfigureManager"}}}};
- }
-
- private:
- void doGet(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request&,
- const std::vector<std::string>& params) override
- {
- // Note that control also reaches here via doPost and doDelete.
- auto session =
- persistent_data::SessionStore::getInstance().getSessionByUid(
- params[0]);
-
- if (session == nullptr)
- {
- messages::resourceNotFound(asyncResp->res, "Session", params[0]);
- return;
- }
-
- asyncResp->res.jsonValue["Id"] = session->uniqueId;
- asyncResp->res.jsonValue["UserName"] = session->username;
- asyncResp->res.jsonValue["@odata.id"] =
- "/redfish/v1/SessionService/Sessions/" + session->uniqueId;
- asyncResp->res.jsonValue["@odata.type"] = "#Session.v1_3_0.Session";
- asyncResp->res.jsonValue["Name"] = "User Session";
- asyncResp->res.jsonValue["Description"] = "Manager User Session";
- asyncResp->res.jsonValue["ClientOriginIPAddress"] = session->clientIp;
+ res.jsonValue["Id"] = session.uniqueId;
+ res.jsonValue["UserName"] = session.username;
+ res.jsonValue["@odata.id"] =
+ "/redfish/v1/SessionService/Sessions/" + session.uniqueId;
+ res.jsonValue["@odata.type"] = "#Session.v1_3_0.Session";
+ res.jsonValue["Name"] = "User Session";
+ res.jsonValue["Description"] = "Manager User Session";
+ res.jsonValue["ClientOriginIPAddress"] = session.clientIp;
#ifdef BMCWEB_ENABLE_IBM_MANAGEMENT_CONSOLE
- asyncResp->res.jsonValue["Oem"]["OpenBMC"]["@odata.type"] =
- "#OemSession.v1_0_0.Session";
- asyncResp->res.jsonValue["Oem"]["OpenBMC"]["ClientID"] =
- session->clientId;
+ res.jsonValue["Oem"]["OpenBMC"]["@odata.type"] =
+ "#OemSession.v1_0_0.Session";
+ res.jsonValue["Oem"]["OpenBMC"]["ClientID"] = session.clientId;
#endif
- }
+}
- void doDelete(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request& req,
- const std::vector<std::string>& params) override
- {
- // Need only 1 param which should be id of session to be deleted
- if (params.size() != 1)
- {
- // This should be handled by crow and never happen
- BMCWEB_LOG_ERROR << "Session DELETE has been called with invalid "
- "number of params";
-
- messages::generalError(asyncResp->res);
- return;
- }
-
- auto session =
- persistent_data::SessionStore::getInstance().getSessionByUid(
- params[0]);
-
- if (session == nullptr)
- {
- messages::resourceNotFound(asyncResp->res, "Session", params[0]);
- return;
- }
-
- // Perform a proper ConfigureSelf authority check. If a
- // session is being used to DELETE some other user's session,
- // then the ConfigureSelf privilege does not apply. In that
- // case, perform the authority check again without the user's
- // ConfigureSelf privilege.
- if (session->username != req.session->username)
- {
- Privileges effectiveUserPrivileges =
- redfish::getUserPrivileges(req.userRole);
-
- if (!effectiveUserPrivileges.isSupersetOf({{"ConfigureUsers"}}))
- {
- messages::insufficientPrivilege(asyncResp->res);
- return;
- }
- }
-
- // DELETE should return representation of object that will be removed
- doGet(asyncResp, req, params);
-
- persistent_data::SessionStore::getInstance().removeSession(session);
- }
-
- /**
- * This allows SessionCollection to reuse this class' doGet method, to
- * maintain consistency of returned data, as Collection's doPost should
- * return data for created member which should match member's doGet
- * result in 100%
- */
- friend SessionCollection;
-};
-
-class SessionCollection : public Node
+inline void requestRoutesSession(App& app)
{
- public:
- SessionCollection(App& app) :
- Node(app, "/redfish/v1/SessionService/Sessions/"), memberSession(app)
- {
- 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, {}}};
- }
+ BMCWEB_ROUTE(app, "/redfish/v1/SessionService/Sessions/<str>/")
+ .privileges({{"Login"}})
+ .methods(boost::beast::http::verb::get)(
+ [](const crow::Request& /*req*/,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& sessionId) -> void {
+ // Note that control also reaches here via doPost and doDelete.
+ auto session = persistent_data::SessionStore::getInstance()
+ .getSessionByUid(sessionId);
- private:
- void doGet(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request&, const std::vector<std::string>&) override
- {
- std::vector<const std::string*> sessionIds =
- persistent_data::SessionStore::getInstance().getUniqueIds(
- false, persistent_data::PersistenceType::TIMEOUT);
+ if (session == nullptr)
+ {
+ messages::resourceNotFound(asyncResp->res, "Session",
+ sessionId);
+ return;
+ }
- asyncResp->res.jsonValue["Members@odata.count"] = sessionIds.size();
- asyncResp->res.jsonValue["Members"] = nlohmann::json::array();
- for (const std::string* uid : sessionIds)
- {
- asyncResp->res.jsonValue["Members"].push_back(
- {{"@odata.id", "/redfish/v1/SessionService/Sessions/" + *uid}});
- }
- asyncResp->res.jsonValue["Members@odata.count"] = sessionIds.size();
- asyncResp->res.jsonValue["@odata.type"] =
- "#SessionCollection.SessionCollection";
- asyncResp->res.jsonValue["@odata.id"] =
- "/redfish/v1/SessionService/Sessions/";
- asyncResp->res.jsonValue["Name"] = "Session Collection";
- asyncResp->res.jsonValue["Description"] = "Session Collection";
- }
+ fillSessionObject(asyncResp->res, *session);
+ });
- void doPost(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request& req,
- const std::vector<std::string>&) override
- {
- std::string username;
- std::string password;
- std::optional<nlohmann::json> oemObject;
- std::string clientId;
- if (!json_util::readJson(req, asyncResp->res, "UserName", username,
- "Password", password, "Oem", oemObject))
- {
- return;
- }
+ BMCWEB_ROUTE(app, "/redfish/v1/SessionService/Sessions/<str>/")
+ .privileges({{"ConfigureManager"}, {"ConfigureSelf"}})
+ .methods(boost::beast::http::verb::delete_)(
+ [](const crow::Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
+ const std::string& sessionId) -> void {
+ auto session = persistent_data::SessionStore::getInstance()
+ .getSessionByUid(sessionId);
- if (password.empty() || username.empty() ||
- asyncResp->res.result() != boost::beast::http::status::ok)
- {
- if (username.empty())
- {
- messages::propertyMissing(asyncResp->res, "UserName");
- }
+ if (session == nullptr)
+ {
+ messages::resourceNotFound(asyncResp->res, "Session",
+ sessionId);
+ return;
+ }
- if (password.empty())
- {
- messages::propertyMissing(asyncResp->res, "Password");
- }
+ // Perform a proper ConfigureSelf authority check. If a
+ // session is being used to DELETE some other user's session,
+ // then the ConfigureSelf privilege does not apply. In that
+ // case, perform the authority check again without the user's
+ // ConfigureSelf privilege.
+ if (session->username != req.session->username)
+ {
+ Privileges effectiveUserPrivileges =
+ redfish::getUserPrivileges(req.userRole);
- return;
- }
+ if (!effectiveUserPrivileges.isSupersetOf(
+ {{"ConfigureUsers"}}))
+ {
+ messages::insufficientPrivilege(asyncResp->res);
+ return;
+ }
+ }
- int pamrc = pamAuthenticateUser(username, password);
- bool isConfigureSelfOnly = pamrc == PAM_NEW_AUTHTOK_REQD;
- if ((pamrc != PAM_SUCCESS) && !isConfigureSelfOnly)
- {
- messages::resourceAtUriUnauthorized(asyncResp->res,
- std::string(req.url),
- "Invalid username or password");
- return;
- }
+ persistent_data::SessionStore::getInstance().removeSession(
+ session);
+ asyncResp->res.result(boost::beast::http::status::no_content);
+ });
+
+ BMCWEB_ROUTE(app, "/redfish/v1/SessionService/Sessions/")
+ .privileges({{"Login"}})
+ .methods(boost::beast::http::verb::get)(
+ [](const crow::Request& /*req*/,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) -> void {
+ std::vector<const std::string*> sessionIds =
+ persistent_data::SessionStore::getInstance().getUniqueIds(
+ false, persistent_data::PersistenceType::TIMEOUT);
+
+ asyncResp->res.jsonValue["Members@odata.count"] =
+ sessionIds.size();
+ asyncResp->res.jsonValue["Members"] = nlohmann::json::array();
+ for (const std::string* uid : sessionIds)
+ {
+ asyncResp->res.jsonValue["Members"].push_back(
+ {{"@odata.id",
+ "/redfish/v1/SessionService/Sessions/" + *uid}});
+ }
+ asyncResp->res.jsonValue["Members@odata.count"] =
+ sessionIds.size();
+ asyncResp->res.jsonValue["@odata.type"] =
+ "#SessionCollection.SessionCollection";
+ asyncResp->res.jsonValue["@odata.id"] =
+ "/redfish/v1/SessionService/Sessions/";
+ asyncResp->res.jsonValue["Name"] = "Session Collection";
+ asyncResp->res.jsonValue["Description"] = "Session Collection";
+ });
+
+ BMCWEB_ROUTE(app, "/redfish/v1/SessionService/Sessions/")
+ .privileges({})
+ .methods(boost::beast::http::verb::post)(
+ [](const crow::Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) -> void {
+ std::string username;
+ std::string password;
+ std::optional<nlohmann::json> oemObject;
+ std::string clientId;
+ if (!json_util::readJson(req, asyncResp->res, "UserName",
+ username, "Password", password, "Oem",
+ oemObject))
+ {
+ return;
+ }
+
+ if (password.empty() || username.empty() ||
+ asyncResp->res.result() != boost::beast::http::status::ok)
+ {
+ if (username.empty())
+ {
+ messages::propertyMissing(asyncResp->res, "UserName");
+ }
+
+ if (password.empty())
+ {
+ messages::propertyMissing(asyncResp->res, "Password");
+ }
+
+ return;
+ }
+
+ int pamrc = pamAuthenticateUser(username, password);
+ bool isConfigureSelfOnly = pamrc == PAM_NEW_AUTHTOK_REQD;
+ if ((pamrc != PAM_SUCCESS) && !isConfigureSelfOnly)
+ {
+ messages::resourceAtUriUnauthorized(
+ asyncResp->res, std::string(req.url),
+ "Invalid username or password");
+ return;
+ }
#ifdef BMCWEB_ENABLE_IBM_MANAGEMENT_CONSOLE
- if (oemObject)
- {
- std::optional<nlohmann::json> bmcOem;
- if (!json_util::readJson(*oemObject, asyncResp->res, "OpenBMC",
- bmcOem))
- {
- return;
- }
- if (!json_util::readJson(*bmcOem, asyncResp->res, "ClientID",
- clientId))
- {
- BMCWEB_LOG_ERROR << "Could not read ClientId";
- return;
- }
- }
+ if (oemObject)
+ {
+ std::optional<nlohmann::json> bmcOem;
+ if (!json_util::readJson(*oemObject, asyncResp->res,
+ "OpenBMC", bmcOem))
+ {
+ return;
+ }
+ if (!json_util::readJson(*bmcOem, asyncResp->res,
+ "ClientID", clientId))
+ {
+ BMCWEB_LOG_ERROR << "Could not read ClientId";
+ return;
+ }
+ }
#endif
- // User is authenticated - create session
- std::shared_ptr<persistent_data::UserSession> session =
- persistent_data::SessionStore::getInstance().generateUserSession(
- username, req.ipAddress.to_string(), clientId,
- persistent_data::PersistenceType::TIMEOUT, isConfigureSelfOnly);
- asyncResp->res.addHeader("X-Auth-Token", session->sessionToken);
- asyncResp->res.addHeader("Location",
- "/redfish/v1/SessionService/Sessions/" +
- session->uniqueId);
- asyncResp->res.result(boost::beast::http::status::created);
- if (session->isConfigureSelfOnly)
- {
- messages::passwordChangeRequired(
- asyncResp->res,
- "/redfish/v1/AccountService/Accounts/" + session->username);
- }
- memberSession.doGet(asyncResp, req, {session->uniqueId});
- }
+ // User is authenticated - create session
+ std::shared_ptr<persistent_data::UserSession> session =
+ persistent_data::SessionStore::getInstance()
+ .generateUserSession(
+ username, req.ipAddress.to_string(), clientId,
+ persistent_data::PersistenceType::TIMEOUT,
+ isConfigureSelfOnly);
+ asyncResp->res.addHeader("X-Auth-Token", session->sessionToken);
+ asyncResp->res.addHeader(
+ "Location",
+ "/redfish/v1/SessionService/Sessions/" + session->uniqueId);
+ asyncResp->res.result(boost::beast::http::status::created);
+ if (session->isConfigureSelfOnly)
+ {
+ messages::passwordChangeRequired(
+ asyncResp->res, "/redfish/v1/AccountService/Accounts/" +
+ session->username);
+ }
- /**
- * Member session to ensure consistency between collection's doPost and
- * member's doGet, as they should return 100% matching data
- */
- Sessions memberSession;
-};
+ fillSessionObject(asyncResp->res, *session);
+ });
-class SessionService : public Node
-{
- public:
- SessionService(App& app) : Node(app, "/redfish/v1/SessionService/")
- {
+ BMCWEB_ROUTE(app, "/redfish/v1/SessionService/")
+ .privileges({{"Login"}})
+ .methods(boost::beast::http::verb::get)(
+ [](const crow::Request& /* req */,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) -> void {
+ asyncResp->res.jsonValue["@odata.type"] =
+ "#SessionService.v1_0_2.SessionService";
+ asyncResp->res.jsonValue["@odata.id"] =
+ "/redfish/v1/SessionService/";
+ asyncResp->res.jsonValue["Name"] = "Session Service";
+ asyncResp->res.jsonValue["Id"] = "SessionService";
+ asyncResp->res.jsonValue["Description"] = "Session Service";
+ asyncResp->res.jsonValue["SessionTimeout"] =
+ persistent_data::SessionStore::getInstance()
+ .getTimeoutInSeconds();
+ asyncResp->res.jsonValue["ServiceEnabled"] = true;
- 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"}}}};
- }
+ asyncResp->res.jsonValue["Sessions"] = {
+ {"@odata.id", "/redfish/v1/SessionService/Sessions"}};
+ });
- private:
- void doGet(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request&, const std::vector<std::string>&) override
- {
- asyncResp->res.jsonValue["@odata.type"] =
- "#SessionService.v1_0_2.SessionService";
- asyncResp->res.jsonValue["@odata.id"] = "/redfish/v1/SessionService/";
- asyncResp->res.jsonValue["Name"] = "Session Service";
- asyncResp->res.jsonValue["Id"] = "SessionService";
- asyncResp->res.jsonValue["Description"] = "Session Service";
- asyncResp->res.jsonValue["SessionTimeout"] =
- persistent_data::SessionStore::getInstance().getTimeoutInSeconds();
- asyncResp->res.jsonValue["ServiceEnabled"] = true;
+ BMCWEB_ROUTE(app, "/redfish/v1/SessionService/")
+ .privileges({{"ConfigureManager"}})
+ .methods(boost::beast::http::verb::patch)(
+ [](const crow::Request& req,
+ const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) -> void {
+ std::optional<int64_t> sessionTimeout;
+ if (!json_util::readJson(req, asyncResp->res, "SessionTimeout",
+ sessionTimeout))
+ {
+ return;
+ }
- asyncResp->res.jsonValue["Sessions"] = {
- {"@odata.id", "/redfish/v1/SessionService/Sessions"}};
- }
+ if (sessionTimeout)
+ {
+ // The mininum & maximum allowed values for session timeout
+ // are 30 seconds and 86400 seconds respectively as per the
+ // session service schema mentioned at
+ // https://redfish.dmtf.org/schemas/v1/SessionService.v1_1_7.json
- void doPatch(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const crow::Request& req,
- const std::vector<std::string>&) override
- {
- std::optional<int64_t> sessionTimeout;
- if (!json_util::readJson(req, asyncResp->res, "SessionTimeout",
- sessionTimeout))
- {
- return;
- }
-
- if (sessionTimeout)
- {
- // The mininum & maximum allowed values for session timeout are 30
- // seconds and 86400 seconds respectively as per the session service
- // schema mentioned at
- // https://redfish.dmtf.org/schemas/v1/SessionService.v1_1_7.json
-
- if (*sessionTimeout <= 86400 && *sessionTimeout >= 30)
- {
- std::chrono::seconds sessionTimeoutInseconds(*sessionTimeout);
- persistent_data::SessionStore::getInstance()
- .updateSessionTimeout(sessionTimeoutInseconds);
- messages::propertyValueModified(
- asyncResp->res, "SessionTimeOut",
- std::to_string(*sessionTimeout));
- }
- else
- {
- messages::propertyValueNotInList(
- asyncResp->res, std::to_string(*sessionTimeout),
- "SessionTimeOut");
- }
- }
- }
-};
+ if (*sessionTimeout <= 86400 && *sessionTimeout >= 30)
+ {
+ std::chrono::seconds sessionTimeoutInseconds(
+ *sessionTimeout);
+ persistent_data::SessionStore::getInstance()
+ .updateSessionTimeout(sessionTimeoutInseconds);
+ messages::propertyValueModified(
+ asyncResp->res, "SessionTimeOut",
+ std::to_string(*sessionTimeout));
+ }
+ else
+ {
+ messages::propertyValueNotInList(
+ asyncResp->res, std::to_string(*sessionTimeout),
+ "SessionTimeOut");
+ }
+ }
+ });
+}
} // namespace redfish