Move ClientID parameter out of OEM
In 2022.2, Redfish added support for the Context parameter on the
Session Resource. This parameter has the same function that the
OemSession.ClientId field served. This commit moves all the existing
ClientId code to produce Context as well.
Functionally, this has one important difference, in that Context in
Redfish is optionally provided by the user, which means we need to omit
it if not given by the user. The old implementation left it set to
empty string ("").
Because of this, a few minor interfaces need to change to use
std::optional. Existing uses of clientId are moved to using
value_or("") to keep the same behavior as before.
Tested:
curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\":
\"0penBmc\"}" https://192.168.7.2/redfish/v1/SessionService/Sessions
Returns a Session object with no Context key present
curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\":
\"0penBmc\", \"Context\": \"Foobar\"}"
https://192.168.7.2/redfish/v1/SessionService/Sessions
Returns a Session object with:
"Context": "Foobar"
Subsequent Gets of /redfish/v1/SessionService/Sessions/<sid>
return the same session objects, both with and without Context.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4df358623f93f3e6cb659e99970ad909cefebc62
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 025c143..d1266f3 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -264,11 +264,10 @@
return true;
}
sslUser.resize(lastChar);
- std::string unsupportedClientId;
sessionIsFromTransport = true;
userSession = persistent_data::SessionStore::getInstance()
.generateUserSession(
- sslUser, req->ipAddress, unsupportedClientId,
+ sslUser, req->ipAddress, std::nullopt,
persistent_data::PersistenceType::TIMEOUT);
if (userSession != nullptr)
{
diff --git a/include/authentication.hpp b/include/authentication.hpp
index 6a9e6ad..84875a9 100644
--- a/include/authentication.hpp
+++ b/include/authentication.hpp
@@ -84,9 +84,8 @@
// needed.
// This whole flow needs to be revisited anyway, as we can't be
// calling directly into pam for every request
- std::string unsupportedClientId;
return persistent_data::SessionStore::getInstance().generateUserSession(
- user, clientIp, unsupportedClientId,
+ user, clientIp, std::nullopt,
persistent_data::PersistenceType::SINGLE_REQUEST, isConfigureSelfOnly);
}
#endif
diff --git a/include/ibm/management_console_rest.hpp b/include/ibm/management_console_rest.hpp
index 0a8b146..97ac497 100644
--- a/include/ibm/management_console_rest.hpp
+++ b/include/ibm/management_console_rest.hpp
@@ -448,9 +448,10 @@
segInfo.push_back(std::make_pair(lockFlags, segmentLength));
}
- lockRequestStructure.push_back(
- make_tuple(req.session->uniqueId, req.session->clientId, lockType,
- resourceId, segInfo));
+
+ lockRequestStructure.push_back(make_tuple(
+ req.session->uniqueId, req.session->clientId.value_or(""), lockType,
+ resourceId, segInfo));
}
// print lock request into journal
@@ -557,8 +558,8 @@
// validate the request ids
auto varReleaselock = crow::ibm_mc_lock::Lock::getInstance().releaseLock(
- listTransactionIds,
- std::make_pair(req.session->clientId, req.session->uniqueId));
+ listTransactionIds, std::make_pair(req.session->clientId.value_or(""),
+ req.session->uniqueId));
if (!varReleaselock.first)
{
diff --git a/include/login_routes.hpp b/include/login_routes.hpp
index 0ff313c..df910e4 100644
--- a/include/login_routes.hpp
+++ b/include/login_routes.hpp
@@ -176,11 +176,10 @@
}
else
{
- std::string unsupportedClientId;
auto session =
persistent_data::SessionStore::getInstance()
.generateUserSession(
- username, req.ipAddress, unsupportedClientId,
+ username, req.ipAddress, std::nullopt,
persistent_data::PersistenceType::TIMEOUT,
isConfigureSelfOnly);
diff --git a/include/persistent_data.hpp b/include/persistent_data.hpp
index de5d678..7478713 100644
--- a/include/persistent_data.hpp
+++ b/include/persistent_data.hpp
@@ -239,9 +239,10 @@
session["username"] = p.second->username;
session["csrf_token"] = p.second->csrfToken;
session["client_ip"] = p.second->clientIp;
-#ifdef BMCWEB_ENABLE_IBM_MANAGEMENT_CONSOLE
- session["client_id"] = p.second->clientId;
-#endif
+ if (p.second->clientId)
+ {
+ session["client_id"] = *p.second->clientId;
+ }
sessions.push_back(std::move(session));
}
}
diff --git a/include/sessions.hpp b/include/sessions.hpp
index 79b344a..94a0755 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -8,6 +8,7 @@
#include <utils/ip_utils.hpp>
#include <csignal>
+#include <optional>
#include <random>
#ifdef BMCWEB_ENABLE_IBM_MANAGEMENT_CONSOLE
#include <ibm/locks.hpp>
@@ -33,7 +34,7 @@
std::string sessionToken;
std::string username;
std::string csrfToken;
- std::string clientId;
+ std::optional<std::string> clientId;
std::string clientIp;
std::chrono::time_point<std::chrono::steady_clock> lastUpdated;
PersistenceType persistence{PersistenceType::TIMEOUT};
@@ -88,12 +89,10 @@
{
userSession->username = *thisValue;
}
-#ifdef BMCWEB_ENABLE_IBM_MANAGEMENT_CONSOLE
else if (element.key() == "client_id")
{
userSession->clientId = *thisValue;
}
-#endif
else if (element.key() == "client_ip")
{
userSession->clientIp = *thisValue;
@@ -204,7 +203,7 @@
std::shared_ptr<UserSession> generateUserSession(
const std::string_view username,
const boost::asio::ip::address& clientIp,
- const std::string_view clientId,
+ const std::optional<std::string>& clientId,
PersistenceType persistence = PersistenceType::TIMEOUT,
bool isConfigureSelfOnly = false)
{
@@ -255,8 +254,8 @@
}
auto session = std::make_shared<UserSession>(UserSession{
- uniqueId, sessionToken, std::string(username), csrfToken,
- std::string(clientId), redfish::ip_util::toString(clientIp),
+ uniqueId, sessionToken, std::string(username), csrfToken, clientId,
+ redfish::ip_util::toString(clientIp),
std::chrono::steady_clock::now(), persistence, false,
isConfigureSelfOnly});
auto it = authTokens.emplace(sessionToken, session);
diff --git a/redfish-core/lib/redfish_sessions.hpp b/redfish-core/lib/redfish_sessions.hpp
index 37527dd..d1314a5 100644
--- a/redfish-core/lib/redfish_sessions.hpp
+++ b/redfish-core/lib/redfish_sessions.hpp
@@ -34,14 +34,19 @@
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["@odata.type"] = "#Session.v1_5_0.Session";
res.jsonValue["Name"] = "User Session";
res.jsonValue["Description"] = "Manager User Session";
res.jsonValue["ClientOriginIPAddress"] = session.clientIp;
+ if (session.clientId)
+ {
+ res.jsonValue["Context"] = *session.clientId;
+ }
+// The below implementation is deprecated in leiu of Session.Context
#ifdef BMCWEB_ENABLE_IBM_MANAGEMENT_CONSOLE
res.jsonValue["Oem"]["OpenBMC"]["@odata.type"] =
"#OemSession.v1_0_0.Session";
- res.jsonValue["Oem"]["OpenBMC"]["ClientID"] = session.clientId;
+ res.jsonValue["Oem"]["OpenBMC"]["ClientID"] = session.clientId.value_or("");
#endif
}
@@ -187,9 +192,10 @@
std::string username;
std::string password;
std::optional<nlohmann::json> oemObject;
- std::string clientId;
+ std::optional<std::string> clientId;
if (!json_util::readJsonPatch(req, asyncResp->res, "UserName", username,
- "Password", password, "Oem", oemObject))
+ "Password", password, "Context", clientId,
+ "Oem", oemObject))
{
return;
}
@@ -226,11 +232,23 @@
{
return;
}
- if (!json_util::readJson(*bmcOem, asyncResp->res, "ClientID", clientId))
+
+ std::optional<std::string> oemClientId;
+ if (!json_util::readJson(*bmcOem, asyncResp->res, "ClientID",
+ oemClientId))
{
BMCWEB_LOG_ERROR << "Could not read ClientId";
return;
}
+ if (oemClientId)
+ {
+ if (clientId)
+ {
+ messages::propertyValueConflict(*oemClientId, *clientId);
+ return;
+ }
+ clientId = *oemClientId;
+ }
}
#endif