[cpp-sp] branch main updated: Integrate WIP on Hub-backed session cache.
Codeberg
noreply at shibboleth.net
Mon Dec 1 20:42:46 UTC 2025
This is an automated email from the git hooks/post-receive script.
codeberg pushed a commit to branch main
in repository cpp-sp.
View the commit online:
https://codeberg.org/Shibboleth/cpp-sp/commit/99afb9e935f375e74d00db551331213915040074
The following commit(s) were added to refs/heads/main by this push:
new 99afb9e9 Integrate WIP on Hub-backed session cache.
99afb9e9 is described below
commit 99afb9e935f375e74d00db551331213915040074
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Dec 1 15:42:33 2025 -0500
Integrate WIP on Hub-backed session cache.
---
Projects/vc22/shibsp.vcxproj | 1 +
Projects/vc22/shibsp.vcxproj.filters | 3 +
shibsp/Makefile.am | 1 +
shibsp/session/SessionCacheSPI.h | 8 +-
shibsp/session/impl/AbstractSessionCache.cpp | 6 +-
shibsp/session/impl/FilesystemSessionCache.cpp | 8 +-
shibsp/session/impl/MemorySessionCache.cpp | 52 ++--
shibsp/session/impl/StorageServiceSessionCache.cpp | 266 ++++++++++++---------
8 files changed, 201 insertions(+), 144 deletions(-)
diff --git a/Projects/vc22/shibsp.vcxproj b/Projects/vc22/shibsp.vcxproj
index 0ab66ff9..d87b2dfa 100644
--- a/Projects/vc22/shibsp.vcxproj
+++ b/Projects/vc22/shibsp.vcxproj
@@ -175,6 +175,7 @@
<ClCompile Include="..\..\shibsp\session\impl\AbstractSessionCache.cpp" />
<ClCompile Include="..\..\shibsp\session\impl\FilesystemSessionCache.cpp" />
<ClCompile Include="..\..\shibsp\session\impl\MemorySessionCache.cpp" />
+ <ClCompile Include="..\..\shibsp\session\impl\StorageServiceSessionCache.cpp" />
<ClCompile Include="..\..\shibsp\util\BoostPropertySet.cpp" />
<ClCompile Include="..\..\shibsp\util\CGIParser.cpp" />
<ClCompile Include="..\..\shibsp\util\DirectoryWalker.cpp" />
diff --git a/Projects/vc22/shibsp.vcxproj.filters b/Projects/vc22/shibsp.vcxproj.filters
index 2e50d4ef..e2484017 100644
--- a/Projects/vc22/shibsp.vcxproj.filters
+++ b/Projects/vc22/shibsp.vcxproj.filters
@@ -371,6 +371,9 @@
<ClCompile Include="..\..\shibsp\session\impl\MemorySessionCache.cpp">
<Filter>Source Files\Session</Filter>
</ClCompile>
+ <ClCompile Include="..\..\shibsp\session\impl\StorageServiceSessionCache.cpp">
+ <Filter>Source Files\Session</Filter>
+ </ClCompile>
<ClCompile Include="..\..\shibsp\remoting\impl\CurlHTTPRemotingService.cpp">
<Filter>Source Files\Remoting</Filter>
</ClCompile>
diff --git a/shibsp/Makefile.am b/shibsp/Makefile.am
index 4ac1855e..1bced9a8 100644
--- a/shibsp/Makefile.am
+++ b/shibsp/Makefile.am
@@ -136,6 +136,7 @@ libshibsp_la_SOURCES = \
session/impl/AbstractSessionCache.cpp \
session/impl/FilesystemSessionCache.cpp \
session/impl/MemorySessionCache.cpp \
+ session/impl/StorageServiceSessionCache.cpp \
util/BoostPropertySet.cpp \
util/CGIParser.cpp \
util/DirectoryWalker.cpp \
diff --git a/shibsp/session/SessionCacheSPI.h b/shibsp/session/SessionCacheSPI.h
index 407690db..2b97773b 100644
--- a/shibsp/session/SessionCacheSPI.h
+++ b/shibsp/session/SessionCacheSPI.h
@@ -113,12 +113,12 @@ namespace shibsp {
* @param agent request, if available
* @param key session key/ID
* @param version old session version
- * @param data updated session data
+ * @param sessionData updated session data
*
* @return true iff the session was updated to a version one greater than the input version,
* false to signal a version collision such that a newer version was added behind us
*/
- virtual bool cache_update(SPRequest* request, const char* key, unsigned int version, DDF& data)=0;
+ virtual bool cache_update(SPRequest* request, const char* key, unsigned int version, DDF& sessionData)=0;
/**
* Informs the storage medium that a session was used at the current point in time.
@@ -133,9 +133,7 @@ namespace shibsp {
*
* @return true iff the session remains valid/available
*/
- virtual bool cache_touch(
- SPRequest* request, const char* key, unsigned int version=1, unsigned int timeout=0
- )=0;
+ virtual bool cache_touch(SPRequest* request, const char* key, unsigned int version=1, unsigned int timeout=0)=0;
/**
* Delete a session record from the underlying storage medium.
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index 73cdf1ab..4f30e41b 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -80,7 +80,7 @@ void SHIBSP_API shibsp::registerSessionCaches()
{
AgentConfig::getConfig().SessionCacheManager.registerFactory(FILESYSTEM_SESSION_CACHE, FilesystemSessionCacheFactory);
AgentConfig::getConfig().SessionCacheManager.registerFactory(MEMORY_SESSION_CACHE, MemorySessionCacheFactory);
- //AgentConfig::getConfig().SessionCacheManager.registerFactory(STORAGESERVICE_SESSION_CACHE, StorageServiceSessionCacheFactory);
+ AgentConfig::getConfig().SessionCacheManager.registerFactory(STORAGESERVICE_SESSION_CACHE, StorageServiceSessionCacheFactory);
}
Session::Session()
@@ -113,6 +113,10 @@ bool AbstractSessionCache::isSessionDataValid(DDF& sessionData)
// Must have a non-empty string "app_id" member
// Must have a positive longinteger "ts" member
+ if (sessionData.isstruct()) {
+ return false;
+ }
+
const char* appId = sessionData["app_id"].string();
if (!appId || !*appId) {
return false;
diff --git a/shibsp/session/impl/FilesystemSessionCache.cpp b/shibsp/session/impl/FilesystemSessionCache.cpp
index 176e4f6d..f105b1ab 100644
--- a/shibsp/session/impl/FilesystemSessionCache.cpp
+++ b/shibsp/session/impl/FilesystemSessionCache.cpp
@@ -380,7 +380,7 @@ DDF FilesystemSessionCache::cache_read(
return obj;
}
-bool FilesystemSessionCache::cache_update(SPRequest* request, const char* key, unsigned int version, DDF& data)
+bool FilesystemSessionCache::cache_update(SPRequest* request, const char* key, unsigned int version, DDF& sessionData)
{
// The essence of this operation is to grab the "next" version by reserving a new session file under the
// correct name for the new version.
@@ -430,8 +430,8 @@ bool FilesystemSessionCache::cache_update(SPRequest* request, const char* key, u
}
// Ensure the new version is set accurately.
- data.addmember("ver").integer(version);
- os << data;
+ sessionData.addmember("ver").integer(version);
+ os << sessionData;
if (os) {
m_spilog.debug("stored new version of session to file (%s)", path.c_str());
return true;
@@ -508,7 +508,7 @@ void FilesystemSessionCache::cache_remove(SPRequest* request, const char* key)
}
}
else {
- m_spilog.debug("removed session file for (%s), version (%u)", key, version);
+ m_spilog.debug("removed file for session (%s), version (%u)", key, version);
}
}
}
diff --git a/shibsp/session/impl/MemorySessionCache.cpp b/shibsp/session/impl/MemorySessionCache.cpp
index 779ac8a1..f24d86f0 100644
--- a/shibsp/session/impl/MemorySessionCache.cpp
+++ b/shibsp/session/impl/MemorySessionCache.cpp
@@ -64,7 +64,7 @@ namespace {
unsigned int timeout=0,
const char* client_addr=nullptr
);
- bool cache_update(SPRequest* request, const char* key, unsigned int version, DDF& data);
+ bool cache_update(SPRequest* request, const char* key, unsigned int version, DDF& sessionData);
bool cache_touch(SPRequest* request, const char* key, unsigned int version=1, unsigned int timeout=0);
void cache_remove(SPRequest* request, const char* key);
@@ -231,6 +231,31 @@ DDF MemorySessionCache::cache_read(
return entry->second.first.copy();
}
+bool MemorySessionCache::cache_update(SPRequest* request, const char* key, unsigned int version, DDF& sessionData)
+{
+ lock_guard<mutex> locker(m_lock);
+
+ auto entry = m_storage.find(key);
+ if (entry == m_storage.end()) {
+ return false;
+ }
+
+ unsigned int oldver = entry->second.first.getmember("ver").integer();
+ if (oldver == 0) {
+ oldver = 1;
+ }
+
+ if (version != oldver) {
+ return false;
+ }
+
+ sessionData.addmember("ver").integer(++version);
+ entry->second.first.destroy();
+ entry->second.first = sessionData.copy();
+ entry->second.second = time(nullptr);
+ return true;
+}
+
bool MemorySessionCache::cache_touch(SPRequest* request, const char* key, unsigned int version, unsigned int timeout)
{
lock_guard<mutex> locker(m_lock);
@@ -256,31 +281,6 @@ bool MemorySessionCache::cache_touch(SPRequest* request, const char* key, unsign
return false;
}
-bool MemorySessionCache::cache_update(SPRequest* request, const char* key, unsigned int version, DDF& data)
-{
- lock_guard<mutex> locker(m_lock);
-
- auto entry = m_storage.find(key);
- if (entry == m_storage.end()) {
- return false;
- }
-
- unsigned int oldver = entry->second.first.getmember("ver").integer();
- if (oldver == 0) {
- oldver = 1;
- }
-
- if (version != oldver) {
- return false;
- }
-
- data.addmember("ver").integer(++version);
- entry->second.first.destroy();
- entry->second.first = data.copy();
- entry->second.second = time(nullptr);
- return true;
-}
-
void MemorySessionCache::cache_remove(SPRequest* request, const char* key)
{
lock_guard<mutex> locker(m_lock);
diff --git a/shibsp/session/impl/StorageServiceSessionCache.cpp b/shibsp/session/impl/StorageServiceSessionCache.cpp
index 7769a35d..73bf5003 100644
--- a/shibsp/session/impl/StorageServiceSessionCache.cpp
+++ b/shibsp/session/impl/StorageServiceSessionCache.cpp
@@ -89,49 +89,30 @@ StorageServiceSessionCache::~StorageServiceSessionCache()
string StorageServiceSessionCache::cache_create(SPRequest* request, DDF& sessionData)
{
- DDF in("storage");
+ DDF in("session-cache");
DDFJanitor injanitor(in);
-
- ostringstream sink;
- sink << sessionData;
-
+ DDF sessionCopy = sessionData.copy();
+ in.add(sessionCopy.name("session"));
in.addmember("op").string("C");
- in.addmember("context").string("sessions");
- in.addmember("value").string(sink.str());
- in.addmember("exp").longinteger(time(nullptr) + m_storageTimeout);
-
- // Extremely unlikely to collide but we try 3 times just in case.
+ // setting needs to be forwarded to Hub to preserve last access time
+ in.addmember("storage_timeout").longinteger(m_storageTimeout);
- int attempts = 0;
- do {
- string key = hex_encode(m_rng(string(16,0)));
- in.addmember("key").string(key);
-
- try {
- DDF out = AgentConfig::getConfig().getAgent().getRemotingService()->send(in);
- out.destroy();
- return key;
- }
- catch (const OperationException& e) {
- const char* event = e.getProperty(AgentException::EVENT_PROP_NAME);
- if (!event || strcmp(event, "DuplicateRecord")) {
- m_spilog.error("failure storing session via Hub: event (%s): %s",
- event ? event : "null", e.what());
- throw;
- }
- }
- catch (const exception& e) {
- m_spilog.error("exception attempting to store session via hub: %s", e.what());
- throw;
+ try {
+ DDF out = AgentConfig::getConfig().getAgent().getRemotingService()->send(in);
+ DDFJanitor outJanitor(out);
+ const char* key = out["key"].string();
+ if (!key || !*key) {
+ m_spilog.error("no session key returned from create operation");
+ throw OperationException("No session key returned from create operation.");
}
-
- } while (++attempts < 3);
-
- m_spilog.error("failed to store new session via Hub after 3 attempts to generate a unique key");
- throw IOException("Exhausted attempts to generate a unique session key.");
+ return string(key);
+ }
+ catch (const exception& e) {
+ m_spilog.error("exception attempting to store session via Hub: %s", e.what());
+ throw;
+ }
}
-/*
DDF StorageServiceSessionCache::cache_read(
SPRequest* request,
const char* applicationId,
@@ -142,47 +123,67 @@ DDF StorageServiceSessionCache::cache_read(
const char* client_addr
)
{
- auto entry = m_storage.find(key);
- if (entry == m_storage.end()) {
- m_lock.unlock();
- return DDF();
- }
+ DDF in("session-cache");
+ DDFJanitor injanitor(in);
- time_t now = time(nullptr);
+ in.addmember("op").string("R");
+ // setting needs to be forwarded to Hub to recover last access time
+ in.addmember("storage_timeout").integer(m_storageTimeout);
+ in.addmember("key").string(key);
- if (timeout ) {
- if (entry->second.second + timeout < now) {
- if (m_spilog.isInfoEnabled()) {
- string ts(date::format("%FT%TZ", chrono::system_clock::from_time_t(entry->second.second)));
- m_spilog.info("session (%s) expired for inactivity, timeout (%lu), last access (%s)", key, timeout, ts.c_str());
- }
- cache_remove(request, key);
+ // Policy settings to enforce on Hub.
+ if (lifetime) {
+ in.addmember("lifetime").integer(lifetime);
+ }
+ if (timeout) {
+ in.addmember("timeout").integer(timeout);
+ }
+
+ DDF out;
+ try {
+ out = AgentConfig::getConfig().getAgent().getRemotingService()->send(in);
+ }
+ catch (const OperationException& e) {
+ // Check for policy events.
+ const char* event = e.getProperty(AgentException::EVENT_PROP_NAME);
+ if (event && !strcmp(event, "InvalidSession")) {
+ m_spilog.warn("stored session (%s) was invalid", key);
+ return DDF();
+ }
+ else if (event && !strcmp(event, "ExpiredSession")) {
+ m_spilog.warn("session (%s) expired due to lifetime or inactivity", key);
return DDF();
}
+ m_spilog.error("exception attempting to update session (%s) via Hub: %s", key, e.what());
+ throw;
+ }
+ catch (const exception& e) {
+ m_spilog.error("exception attempting to read session (%s) from Hub: %s", key, e.what());
+ throw;
+ }
+
+ DDFJanitor outjanitor(out);
+
+ DDF sessionData = out["session"];
+ if (sessionData.isnull()) {
+ return sessionData;
}
- const char* appId = entry->second.first["app_id"].string();
+ if (!isSessionDataValid(sessionData)) {
+ m_spilog.error("session data returned from Hub for (%s) was invalid", key);
+ throw IOException("Session data was invalid.");
+ }
+
+ // Check application. We know the member exists due to the previous check.
+ const char* appId = sessionData["app_id"].string();
if (strcmp(applicationId, appId)) {
m_spilog.warn("session (%s) issued for application (%s), accessed via application (%s)", key, appId, applicationId);
return DDF();
}
- if (lifetime) {
- time_t start = entry->second.first["ts"].longinteger();
- if (start + lifetime < now) {
- if (m_spilog.isInfoEnabled()) {
- string created(date::format("%FT%TZ", chrono::system_clock::from_time_t(start)));
- string expired(date::format("%FT%TZ", chrono::system_clock::from_time_t(start + lifetime)));
- m_spilog.info("session (%s) has expired, created (%s), expired (%s)", key, created.c_str(), expired.c_str());
- }
- cache_remove(request, key);
- return DDF();
- }
- }
-
if (client_addr) {
const char* family = getAddressFamily(client_addr);
- const char* addr = entry->second.first[family].string();
+ const char* addr = sessionData[family].string();
if (addr) {
if (!isAddressMatch(client_addr, addr)) {
m_spilog.info("session (%s) use invalid, bound to address (%s), accessed from (%s)", key, addr, client_addr);
@@ -193,86 +194,135 @@ DDF StorageServiceSessionCache::cache_read(
// We have to rebind the session to a new address family, requiring an update to the session.
m_spilog.info("attempting update of session (%s) to rebind to new address (%s)", key, client_addr);
- // Fill in the new address and attempt the update.
- entry->second.first.addmember(family).string(client_addr);
- unsigned int oldver = entry->second.first.getmember("ver").integer();
- entry->second.first.addmember("ver").integer(oldver == 0 ? 2 : oldver + 1);
+ // Fill in the new address and attempt the update. We can do this using the "existing" output
+ // because we have the object janitor'd and the cache_update copies the object internally.
+ sessionData.addmember(family).string(client_addr);
+ try {
+ if (!cache_update(request, key, version, sessionData)) {
+ // This signals a version mismatch in which the original session we were asked to read
+ // has already been updated by another thread or process. In this case, we recurse the
+ // read attempt. This has to terminate eventually...right?
+
+ // We bump the version once, under the assunption it shouldn't be likely that it's been
+ // updated behind us more than once...
+ return cache_read(request, applicationId, key, version + 1, lifetime, timeout, client_addr);
+ }
+ }
+ catch (const exception& ex) {
+ // This is an outright error attempting the update, so we just fail hard.
+ m_spilog.error("exception attempting to update session (%s): %s", key, ex.what());
+ throw;
+ }
}
- }
+ }
- entry->second.second = now;
- return entry->second.first.copy();
+ // Otherwise return the detached session structure.
+ return sessionData.remove();
}
-bool StorageServiceSessionCache::cache_touch(SPRequest* request, const char* key, unsigned int version, unsigned int timeout)
+bool StorageServiceSessionCache::cache_update(SPRequest* request, const char* key, unsigned int version, DDF& sessionData)
{
- auto entry = m_storage.find(key);
- if (entry != m_storage.end()) {
+ DDF in("session-cache");
+ DDFJanitor injanitor(in);
- time_t now = time(nullptr);
+ DDF sessionCopy = sessionData.copy();
+ in.add(sessionCopy.name("session"));
+ in.addmember("op").string("U");
+ // setting needs to be forwarded to Hub to set last access time
+ in.addmember("storage_timeout").integer(m_storageTimeout);
+ in.addmember("key").string(key);
+ in.addmember("ver").integer(version);
- if (timeout && entry->second.second + timeout < now) {
- if (m_spilog.isInfoEnabled()) {
- string ts(date::format("%FT%TZ", chrono::system_clock::from_time_t(entry->second.second)));
- m_spilog.info("session (%s) expired for inactivity, timeout (%lu), last access (%s)", key, timeout, ts.c_str());
- }
- m_storage.erase(entry);
+ DDF out;
+ try {
+ out = AgentConfig::getConfig().getAgent().getRemotingService()->send(in);
+ }
+ catch (const OperationException& e) {
+ // Check for VersionMismatch event.
+ const char* event = e.getProperty(AgentException::EVENT_PROP_NAME);
+ if (event && !strcmp(event, "VersionMismatch")) {
return false;
}
+ m_spilog.error("exception attempting to update session (%s) via Hub: %s", key, e.what());
+ throw;
+ }
+ catch (const exception& e) {
+ m_spilog.error("exception attempting to update session (%s) via Hub: %s", key, e.what());
+ throw;
+ }
+
+ DDFJanitor outjanitor(out);
- entry->second.second = now;
- return true;
+ long newver = out["ver"].integer();
+ if (newver <= version) {
+ m_spilog.error("missing/unexpected version returned from Hub from update of session (%s)", key);
+ throw IOException("Missing/unexpected version returned from Hub from session update.");
}
- return false;
+ // Ensure the new version is set accurately.
+ sessionData.addmember("ver").integer(newver);
+ return true;
}
-bool StorageServiceSessionCache::cache_update(SPRequest* request, const char* key, unsigned int version, DDF& data)
+bool StorageServiceSessionCache::cache_touch(SPRequest* request, const char* key, unsigned int version, unsigned int timeout)
{
- auto entry = m_storage.find(key);
- if (entry == m_storage.end()) {
- return false;
+ DDF in("session-cache");
+ DDFJanitor injanitor(in);
+ in.addmember("op").string("T");
+ in.addmember("key").string(key);
+ // setting needs to be forwarded to Hub to recover last access time
+ in.addmember("storage_timeout").integer(m_storageTimeout);
+ // Policy settings to enforce on Hub.
+ if (timeout) {
+ in.addmember("timeout").integer(timeout);
}
- unsigned int oldver = entry->second.first.getmember("ver").integer();
- if (oldver == 0) {
- oldver = 1;
+ DDF out;
+ try {
+ out = AgentConfig::getConfig().getAgent().getRemotingService()->send(in);
+ }
+ catch (const OperationException& e) {
+ // Check for policy events.
+ const char* event = e.getProperty(AgentException::EVENT_PROP_NAME);
+ if (event && !strcmp(event, "InvalidSession")) {
+ m_spilog.warn("stored session (%s) was invalid", key);
+ return false;
+ }
+ else if (event && !strcmp(event, "ExpiredSession")) {
+ m_spilog.warn("session (%s) expired due to lifetime or inactivity", key);
+ return false;
+ }
+ m_spilog.error("exception attempting to touch session (%s) via Hub: %s", key, e.what());
+ throw;
+ }
+ catch (const exception& e) {
+ m_spilog.error("exception attempting to touch session (%s) via Hub: %s", key, e.what());
+ throw;
}
- if (version != oldver) {
+ DDFJanitor outjanitor(out);
+
+ if (out.getmember("ver").integer() < version) {
return false;
}
-
- data.addmember("ver").integer(++version);
- entry->second.first.destroy();
- entry->second.first = data.copy();
- entry->second.second = time(nullptr);
return true;
}
-*/
void StorageServiceSessionCache::cache_remove(SPRequest* request, const char* key)
{
- DDF in("storage");
+ DDF in("session-cache");
DDFJanitor injanitor(in);
in.addmember("op").string("D");
- in.addmember("context").string("sessions");
in.addmember("key").string(key);
try {
DDF out = AgentConfig::getConfig().getAgent().getRemotingService()->send(in);
out.destroy();
- }
- catch (const OperationException& e) {
- const char* event = e.getProperty(AgentException::EVENT_PROP_NAME);
- m_spilog.error("failure deleting session via Hub: event (%s): %s", event ? event : "null", e.what());
- throw;
+ m_spilog.debug("removed session from storage via Hub (%s)", key);
}
catch (const exception& e) {
m_spilog.error("exception attempting to delete session via hub: %s", e.what());
throw;
}
-
- m_spilog.debug("removed session from storage (%s)", key);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list