[cpp-sp] branch main updated: Fix some session calls and implement session key advice from Hub.
Codeberg
noreply at shibboleth.net
Wed Sep 16 16:55:41 UTC 2026
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/854b1b01576d17efdb93094793ac529919da9105
The following commit(s) were added to refs/heads/main by this push:
new 854b1b01 Fix some session calls and implement session key advice from Hub.
854b1b01 is described below
commit 854b1b01576d17efdb93094793ac529919da9105
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Wed Sep 16 12:55:30 2026 -0400
Fix some session calls and implement session key advice from Hub.
---
shibsp/Agent.cpp | 6 +++---
shibsp/handler/impl/LogoutConsumer.cpp | 2 +-
shibsp/handler/impl/LogoutInitiator.cpp | 2 +-
shibsp/handler/impl/TokenConsumer.cpp | 2 +-
shibsp/session/AbstractSessionCache.h | 2 +-
shibsp/session/SessionCache.h | 18 +++++++++++-------
shibsp/session/SessionCacheSPI.h | 6 +++++-
shibsp/session/impl/AbstractSessionCache.cpp | 6 +++---
shibsp/session/impl/FilesystemSessionCache.cpp | 9 +++++----
shibsp/session/impl/MemorySessionCache.cpp | 8 +++++---
shibsp/session/impl/StorageServiceSessionCache.cpp | 4 ++--
tests/session/impl/FilesystemSessionCacheTests.cpp | 8 ++++----
tests/session/impl/MemorySessionCacheTests.cpp | 6 +++---
13 files changed, 45 insertions(+), 34 deletions(-)
diff --git a/shibsp/Agent.cpp b/shibsp/Agent.cpp
index 6b888167..c6255243 100644
--- a/shibsp/Agent.cpp
+++ b/shibsp/Agent.cpp
@@ -205,7 +205,7 @@ pair<bool,long> Agent::doAuthentication(SPRequest& request, bool handler) const
bool sessionExists = false;
try {
- unique_lock<Session> session = request.getSession(); // don't cache it but enforce policy
+ unique_lock<Session> session = request.getSession(); // enforce policy
sessionExists = session.owns_lock();
// Lock will release here.
}
@@ -295,7 +295,7 @@ pair<bool,long> Agent::doAuthorization(SPRequest& request) const
if (settings.second) {
unique_lock<Session> session;
try {
- session = request.getSession(false, false); // ignore timeout and do not cache
+ session = request.getSession(false, true); // ignore timeout and address check
}
catch (const exception& e) {
request.warn("unable to obtain session to pass to access control provider: %s", e.what());
@@ -338,7 +338,7 @@ pair<bool,long> Agent::doExport(SPRequest& request, bool requireSession) const
unique_lock<Session> session;
try {
- session = request.getSession(false, false); // ignore timeout and address check here
+ session = request.getSession(false, true); // ignore timeout and address check here
}
catch (const exception& e) {
request.warn("unable to obtain session to export to request: %s", e.what());
diff --git a/shibsp/handler/impl/LogoutConsumer.cpp b/shibsp/handler/impl/LogoutConsumer.cpp
index 70e91f38..4f58164f 100644
--- a/shibsp/handler/impl/LogoutConsumer.cpp
+++ b/shibsp/handler/impl/LogoutConsumer.cpp
@@ -92,7 +92,7 @@ pair<bool,long> LogoutConsumer::run(SPRequest& request, bool isHandler) const
unique_lock<Session> session;
try {
- session = request.getSession(false, true); // don't cache it and ignore all checks
+ session = request.getSession(false, true); // ignore all checks
}
catch (const exception& ex) {
request.error("error accessing current session: %s", ex.what());
diff --git a/shibsp/handler/impl/LogoutInitiator.cpp b/shibsp/handler/impl/LogoutInitiator.cpp
index 59e8f2cd..d891437f 100644
--- a/shibsp/handler/impl/LogoutInitiator.cpp
+++ b/shibsp/handler/impl/LogoutInitiator.cpp
@@ -62,7 +62,7 @@ pair<bool,long> LogoutInitiator::run(SPRequest& request, bool isHandler) const
unique_lock<Session> session;
try {
- session = request.getSession(false, true); // don't cache it and ignore all checks
+ session = request.getSession(false, true); // ignore all checks
}
catch (const exception& ex) {
request.error("error accessing current session: %s", ex.what());
diff --git a/shibsp/handler/impl/TokenConsumer.cpp b/shibsp/handler/impl/TokenConsumer.cpp
index 23435e37..2d411666 100644
--- a/shibsp/handler/impl/TokenConsumer.cpp
+++ b/shibsp/handler/impl/TokenConsumer.cpp
@@ -146,7 +146,7 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
SessionCache* cache = request.getAgent().getSessionCache();
DDF sessionData = output["session"];
// Ownership of sessionData transfers on input to create call (will be detached from output).
- cache->create(request, sessionData);
+ cache->create(request, output["random"].string(), sessionData);
const char* sessionHook = request.getRequestSettings().first->getString(RequestMapper::SESSION_HOOK_PROP_NAME);
diff --git a/shibsp/session/AbstractSessionCache.h b/shibsp/session/AbstractSessionCache.h
index a44fa5f2..e201512a 100644
--- a/shibsp/session/AbstractSessionCache.h
+++ b/shibsp/session/AbstractSessionCache.h
@@ -125,7 +125,7 @@ namespace shibsp {
void stop();
// SessionCache API
- std::string create(SPRequest& request, DDF& data);
+ std::string create(SPRequest& request, const char* candidateSessionID, DDF& data);
std::unique_lock<Session> find(SPRequest& request, bool checkTimeout, bool ignoreAddress);
std::unique_lock<Session> find(const char* applicationId, const char* key, unsigned int version=1);
bool update(SPRequest& request, std::unique_lock<Session>& session, DDF& data, const char* reason=nullptr);
diff --git a/shibsp/session/SessionCache.h b/shibsp/session/SessionCache.h
index 5062d7ec..7a04d5ef 100644
--- a/shibsp/session/SessionCache.h
+++ b/shibsp/session/SessionCache.h
@@ -141,22 +141,26 @@ namespace shibsp {
* Creates a new session and stores it persistently while binding the session
* to the input request object.
*
- * <p>The input DDF must be a structure containing at least one member named
- * "attributes" which must be a list, but may be empty. It may contain other
- * members, which will be stored but not examined. The ownership of the DDF
- * will be assumed by this method regardless of the outcome.</p>
+ * <p>The input DDF must be a structure containing at least one member named "attributes"
+ * which must be a list, but may be empty. It may contain other members, which will be
+ * stored but not processed. The ownership of the DDF will be assumed by this method
+ * regardless of the outcome.</p>
*
* <p>An exception is raised in the event of an error.</p>
*
- * <p>The new session's version is implicitly 1 (the first) as it is assumed the
- * session is brand new.</p>
+ * <p>The new session's version is implicitly 1 (the first) as it is assumed the session
+ * is brand new.</p>
+ *
+ * <p>The candidateSessionID parameter may be null and the implementation is not obligated to use
+ * the value so the return value may or may not match the parameter value if it exists.</p>
*
* @param request request to bind the session to
+ * @param candidateSessionID candidate session key, if any
* @param data session data obtained from the hub
*
* @return the newly created session ID
*/
- virtual std::string create(SPRequest& request, DDF& data)=0;
+ virtual std::string create(SPRequest& request, const char* candidateSessionID, DDF& data)=0;
/**
* Locates an existing session bound to a request.
diff --git a/shibsp/session/SessionCacheSPI.h b/shibsp/session/SessionCacheSPI.h
index 2b97773b..83de8eca 100644
--- a/shibsp/session/SessionCacheSPI.h
+++ b/shibsp/session/SessionCacheSPI.h
@@ -57,12 +57,16 @@ namespace shibsp {
*
* <p>The caller retains ownership of the input data.</p>
*
+ * <p>The candidateSessionID parameter may be null and the implementation is not obligated to use
+ * the value so the return value may or may not match the parameter value if it exists.</p>
+ *
* @param request agent request, if available
+ * @param candidateSessionID candidate session key, if any
* @param sessionData data to store in record of session
*
* @return session key/ID created, this MUST be URL-safe
*/
- virtual std::string cache_create(SPRequest* request, DDF& sessionData)=0;
+ virtual std::string cache_create(SPRequest* request, const char* candidateSessionID, DDF& sessionData)=0;
/**
* Read a session record from the underlying storage medium and return its data.
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index 59752e67..1634b7a3 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -338,7 +338,7 @@ pair<string,unsigned int> AbstractSessionCache::parseCookieValue(const char* val
return make_pair(string(value, sep), atoi(sep +1));
}
-string AbstractSessionCache::create(SPRequest& request, DDF& data)
+string AbstractSessionCache::create(SPRequest& request, const char* candidateSessionID, DDF& data)
{
request.debug("creating new session");
@@ -366,7 +366,7 @@ string AbstractSessionCache::create(SPRequest& request, DDF& data)
string key;
try {
request.debug("writing new session to persistent store");
- key = cache_create(&request, data);
+ key = cache_create(&request, candidateSessionID, data);
}
catch (const exception&) {
// Should be logged by the SPI.
@@ -963,7 +963,7 @@ bool BasicSession::isValid(SPRequest* request, unsigned int lifetime, unsigned i
// Pass a zero to bypass timeout enforcement as we know as well or better than the back-end...
if (!m_cache.cache_touch(request, getID(), getVersion(), 0)) {
AbstractSessionCache::log(request, m_cache.logger(), Priority::SHIB_WARN,
- "session (%) missing in persistent store, invalidating locally", getID());
+ "session (%s) missing in persistent store, invalidating locally", getID());
return false;
}
}
diff --git a/shibsp/session/impl/FilesystemSessionCache.cpp b/shibsp/session/impl/FilesystemSessionCache.cpp
index b7f41bfc..cd334657 100644
--- a/shibsp/session/impl/FilesystemSessionCache.cpp
+++ b/shibsp/session/impl/FilesystemSessionCache.cpp
@@ -61,7 +61,7 @@ namespace {
bool start();
void stop();
- string cache_create(SPRequest* request, DDF& sessionData);
+ string cache_create(SPRequest* request, const char* candidateSessionID, DDF& sessionData);
DDF cache_read(
SPRequest* request,
const char* applicationId,
@@ -196,13 +196,12 @@ void FilesystemSessionCache::stop()
#endif
}
-string FilesystemSessionCache::cache_create(SPRequest* request, DDF& sessionData)
+string FilesystemSessionCache::cache_create(SPRequest* request, const char* candidateSessionID, DDF& sessionData)
{
- string key;
+ string key(candidateSessionID ? candidateSessionID : AgentConfig::getConfig().generateRandom(16));
string path;
int attempts = 0;
do {
- key = AgentConfig::getConfig().generateRandom(16);
path = m_dir + key;
computeVersionedFilename(path, 1);
// We attempt an exclusive open to "reserve" the new session file name.
@@ -237,6 +236,8 @@ string FilesystemSessionCache::cache_create(SPRequest* request, DDF& sessionData
} else {
log(DEBUG_MARK, "error opening new session file (%s), errno=%d", path.c_str(), e);
}
+
+ key = AgentConfig::getConfig().generateRandom(16);
} while (++attempts < 3);
log(ERROR_MARK, "failed to write new session after 3 attempts to generate a unique key");
diff --git a/shibsp/session/impl/MemorySessionCache.cpp b/shibsp/session/impl/MemorySessionCache.cpp
index 08e73820..2ba365eb 100644
--- a/shibsp/session/impl/MemorySessionCache.cpp
+++ b/shibsp/session/impl/MemorySessionCache.cpp
@@ -54,7 +54,7 @@ namespace {
bool start();
void stop();
- string cache_create(SPRequest* request, DDF& sessionData);
+ string cache_create(SPRequest* request, const char* candidateSessionID, DDF& sessionData);
DDF cache_read(
SPRequest* request,
const char* applicationId,
@@ -143,17 +143,19 @@ void MemorySessionCache::stop()
}
#endif
}
-string MemorySessionCache::cache_create(SPRequest* request, DDF& sessionData)
+string MemorySessionCache::cache_create(SPRequest* request, const char* candidateSessionID, DDF& sessionData)
{
lock_guard<mutex> locker(m_lock);
+ string key(candidateSessionID ? candidateSessionID : AgentConfig::getConfig().generateRandom(16));
+
int attempts = 0;
do {
- string key = AgentConfig::getConfig().generateRandom(16);
if (m_storage.find(key) == m_storage.end()) {
m_storage[key] = make_pair(sessionData.copy(), time(nullptr));
return key;
}
+ key = AgentConfig::getConfig().generateRandom(16);
} while (++attempts < 3);
log(ERROR_MARK, "failed to write new session after 3 attempts to generate a unique key");
diff --git a/shibsp/session/impl/StorageServiceSessionCache.cpp b/shibsp/session/impl/StorageServiceSessionCache.cpp
index 584a0d8f..3bd0b72d 100644
--- a/shibsp/session/impl/StorageServiceSessionCache.cpp
+++ b/shibsp/session/impl/StorageServiceSessionCache.cpp
@@ -46,7 +46,7 @@ namespace {
StorageServiceSessionCache(const ptree& pt);
~StorageServiceSessionCache();
- string cache_create(SPRequest* request, DDF& sessionData);
+ string cache_create(SPRequest* request, const char*, DDF& sessionData);
DDF cache_read(
SPRequest* request,
const char* applicationId,
@@ -92,7 +92,7 @@ StorageServiceSessionCache::~StorageServiceSessionCache()
{
}
-string StorageServiceSessionCache::cache_create(SPRequest* request, DDF& sessionData)
+string StorageServiceSessionCache::cache_create(SPRequest* request, const char*, DDF& sessionData)
{
const RemotingService* remoting = AgentConfig::getConfig().getAgent().getRemotingService();
DDF in = remoting->build("session-cache", nullptr, request ? request->getRequestID() : nullptr);
diff --git a/tests/session/impl/FilesystemSessionCacheTests.cpp b/tests/session/impl/FilesystemSessionCacheTests.cpp
index 099db45f..0c74456f 100644
--- a/tests/session/impl/FilesystemSessionCacheTests.cpp
+++ b/tests/session/impl/FilesystemSessionCacheTests.cpp
@@ -116,7 +116,7 @@ BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_invalid_attributes, FilesystemFix
SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
exceptionCheck checker("Error while processing session attributes for storage.");
- BOOST_CHECK_EXCEPTION(cache->create(request, child), SessionException, checker.check_message);
+ BOOST_CHECK_EXCEPTION(cache->create(request, nullptr, child), SessionException, checker.check_message);
}
BOOST_FIXTURE_TEST_CASE(FilessystemSessionCache_notonorafter, FilesystemFixture)
@@ -132,7 +132,7 @@ BOOST_FIXTURE_TEST_CASE(FilessystemSessionCache_notonorafter, FilesystemFixture)
DDF child = obj["session"];
SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
- string key = cache->create(request, child);
+ string key = cache->create(request, nullptr, child);
BOOST_CHECK(obj["session"].isnull());
BOOST_CHECK_EQUAL(key.c_str(), child.name());
@@ -177,7 +177,7 @@ BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_tests, FilesystemFixture)
SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
- string key = cache->create(request, child);
+ string key = cache->create(request, nullptr, child);
BOOST_CHECK(obj["session"].isnull());
BOOST_CHECK_EQUAL(key.c_str(), child.name());
@@ -236,7 +236,7 @@ BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_testUpdate, FilesystemFixture)
SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
- string key = cache->create(request, child);
+ string key = cache->create(request, nullptr, child);
BOOST_CHECK(obj["session"].isnull());
BOOST_CHECK_EQUAL(key.c_str(), child.name());
diff --git a/tests/session/impl/MemorySessionCacheTests.cpp b/tests/session/impl/MemorySessionCacheTests.cpp
index c40a4834..55c4bbdb 100644
--- a/tests/session/impl/MemorySessionCacheTests.cpp
+++ b/tests/session/impl/MemorySessionCacheTests.cpp
@@ -91,7 +91,7 @@ BOOST_FIXTURE_TEST_CASE(MemorySessionCache_notonorafter, MemoryFixture)
DDF child = obj["session"];
SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
- string key = cache->create(request, child);
+ string key = cache->create(request, nullptr, child);
BOOST_CHECK(obj["session"].isnull());
BOOST_CHECK_EQUAL(key.c_str(), child.name());
@@ -122,7 +122,7 @@ BOOST_FIXTURE_TEST_CASE(MemorySessionCache_tests, MemoryFixture)
SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
- string key = cache->create(request, child);
+ string key = cache->create(request, nullptr, child);
BOOST_CHECK(obj["session"].isnull());
BOOST_CHECK_EQUAL(key.c_str(), child.name());
@@ -185,7 +185,7 @@ BOOST_FIXTURE_TEST_CASE(MemorySessionCache_testUpdate, MemoryFixture)
SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
- string key = cache->create(request, child);
+ string key = cache->create(request, nullptr, child);
BOOST_CHECK(obj["session"].isnull());
BOOST_CHECK_EQUAL(key.c_str(), child.name());
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list