[cpp-sp] branch main updated: Implement and test API for updating sessions.
Scott Cantor
cantor.2 at osu.edu
Mon Sep 22 16:14:27 UTC 2025
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch main
in repository cpp-sp.
View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=57d8f2d3bfcadeddff7d8a9826d12a7b61d00e21
The following commit(s) were added to refs/heads/main by this push:
new 57d8f2d3 Implement and test API for updating sessions.
57d8f2d3 is described below
commit 57d8f2d3bfcadeddff7d8a9826d12a7b61d00e21
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Sep 22 12:14:19 2025 -0400
Implement and test API for updating sessions.
---
shibsp/remoting/ddf.h | 17 ++++
shibsp/session/AbstractSessionCache.h | 33 ++++++-
shibsp/session/SessionCache.h | 42 ++++++++-
shibsp/session/impl/AbstractSessionCache.cpp | 95 ++++++++++++++++---
tests/impl/XMLAccessControlTests.cpp | 3 +
tests/impl/XMLRequestMapperTests.cpp | 3 +
tests/session/impl/FilesystemSessionCacheTests.cpp | 92 +++++++++++++++++-
tests/session/impl/MemorySessionCacheTests.cpp | 104 ++++++++++++++++++---
8 files changed, 354 insertions(+), 35 deletions(-)
diff --git a/shibsp/remoting/ddf.h b/shibsp/remoting/ddf.h
index aa49d73b..1ce82409 100644
--- a/shibsp/remoting/ddf.h
+++ b/shibsp/remoting/ddf.h
@@ -153,8 +153,25 @@ namespace shibsp {
class SHIBSP_API DDFJanitor
{
public:
+ /**
+ * Assume ownership of the supplied object.
+ *
+ * @param obj object to take ownership of
+ */
DDFJanitor(DDF& obj) : m_obj(obj) {}
+
+ /**
+ * Free the owned object unless previously released.
+ */
~DDFJanitor() { m_obj.destroy(); }
+
+ /**
+ * Release ownership of the stored object and return it.
+ *
+ * @return the stored object, now owned by caller
+ */
+ DDF release() { DDF ret = m_obj; m_obj = DDF(); return ret; }
+
private:
DDF& m_obj;
DDFJanitor(const DDFJanitor&);
diff --git a/shibsp/session/AbstractSessionCache.h b/shibsp/session/AbstractSessionCache.h
index 4fecf852..1c0176e9 100644
--- a/shibsp/session/AbstractSessionCache.h
+++ b/shibsp/session/AbstractSessionCache.h
@@ -65,11 +65,37 @@ namespace shibsp {
const char* getApplicationID() const;
const char* getClientAddress(const char* fanily) const;
const std::map<std::string,DDF>& getAttributes() const;
+ DDF getOpaqueData() const;
time_t getCreation() const;
time_t getLastAccess() const;
- // Perform validation of a local session based on policy and checks for revocation.
- // Address checking is notably handled elsewhere.
+ /**
+ * Returns a clone of the underlying data in the session.
+ *
+ * @return a clone of the session's data, owned by caller
+ */
+ DDF cloneData() const;
+
+ /**
+ * Replace the session's data with an updated version.
+ *
+ * <p>This must be called while holding the exclusive lock on the object.
+ * Ownership of the input object is transferred to this object and the
+ * original data will be freed.</p>
+ *
+ * @param data new data
+ */
+ void updateData(DDF& data);
+
+ /**
+ * Perform validation of a local session based on policy and checks for revocation.
+ *
+ * @param request optional session carrying request if available
+ * @param lifetime session lifetime policy, 0 if none
+ * @param timeout session timeout policy, 0 if none
+ *
+ * @return true iff the session remains valid
+ */
bool isValid(SPRequest* request, unsigned int lifetime, unsigned int timeout);
private:
@@ -99,9 +125,10 @@ namespace shibsp {
void stop();
// SessionCache API
- std::string create(SPRequest& request, DDF& session);
+ std::string create(SPRequest& request, 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);
void remove(SPRequest& request);
void remove(const char* key);
diff --git a/shibsp/session/SessionCache.h b/shibsp/session/SessionCache.h
index f012192e..5062d7ec 100644
--- a/shibsp/session/SessionCache.h
+++ b/shibsp/session/SessionCache.h
@@ -94,6 +94,15 @@ namespace shibsp {
* @return an immutable map of attribute data keyed by attribute ID
*/
virtual const std::map<std::string,DDF>& getAttributes() const=0;
+
+ /**
+ * Returns the opaque hub-supplied data for the session.
+ *
+ * <p>The data remains owned by the session.</p>
+ *
+ * @return opaque data
+ */
+ virtual DDF getOpaqueData() const=0;
};
/**
@@ -132,8 +141,10 @@ namespace shibsp {
* Creates a new session and stores it persistently while binding the session
* to the input request object.
*
- * <p>The second parameter's ownership is 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 examined. 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>
*
@@ -141,11 +152,11 @@ namespace shibsp {
* session is brand new.</p>
*
* @param request request to bind the session to
- * @param session session data obtained from the hub
+ * @param data session data obtained from the hub
*
* @return the newly created session ID
*/
- virtual std::string create(SPRequest& request, DDF& session)=0;
+ virtual std::string create(SPRequest& request, DDF& data)=0;
/**
* Locates an existing session bound to a request.
@@ -175,6 +186,29 @@ namespace shibsp {
*/
virtual std::unique_lock<Session> find(const char* applicationId, const char* key, unsigned int version=1)=0;
+ /**
+ * Updates an existing session bound to a request with new data.
+ *
+ * <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>This operation will succeed only if the existing session's version remains the current one,
+ * and will increment its version.</p>
+ *
+ * <p>The return value is false in the event that a version mismatch occurs, indicating the session
+ * was updated independently. Any other error will raise an exception.</p>
+ *
+ * @param request request from client containing session
+ * @param session session to update
+ * @param data updated session data to store in place of (not in addition to) the existing data
+ * @param reason indication of purpose for update for logging
+ *
+ * @return true iff the session was updated successfully, false if the session was updated independently
+ */
+ virtual bool update(SPRequest& request, std::unique_lock<Session>& session, DDF& data, const char* reason=nullptr)=0;
+
/**
* Removes an existing session bound to a request.
*
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index 84db1886..28f9cd28 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -314,27 +314,27 @@ pair<string,unsigned int> AbstractSessionCache::parseCookieValue(const char* val
return make_pair(string(value, sep), atoi(sep +1));
}
-string AbstractSessionCache::create(SPRequest& request, DDF& session)
+string AbstractSessionCache::create(SPRequest& request, DDF& data)
{
m_log.debug("creating new session");
// Isolate from parent.
- session.remove();
+ data.remove();
// Add additional fields managed by agent.
// The version is absent, implying 1, as the common case.
// The attributes member should be present from hub.
- session.addmember("ts").longinteger(time(nullptr));
- session.addmember("app_id").string(request.getRequestSettings().first->getString(
+ data.addmember("ts").longinteger(time(nullptr));
+ data.addmember("app_id").string(request.getRequestSettings().first->getString(
RequestMapper::APPLICATION_ID_PROP_NAME, RequestMapper::APPLICATION_ID_PROP_DEFAULT));
- session.addmember(getAddressFamily(request.getRemoteAddr())).string(request.getRemoteAddr());
+ data.addmember(getAddressFamily(request.getRemoteAddr())).string(request.getRemoteAddr());
const AttributeConfiguration& attrConfig = request.getAgent().getAttributeConfiguration(
request.getRequestSettings().first->getString(RequestMapper::ATTRIBUTE_CONFIG_ID_PROP_NAME));
- DDF attrs = session["attributes"];
+ DDF attrs = data["attributes"];
if (!attrConfig.processAttributes(attrs)) {
m_log.warn("error processing session attributes for storage/use");
- session.destroy();
+ data.destroy();
throw SessionException("Error while processing session attributes for storage.");
}
@@ -342,16 +342,16 @@ string AbstractSessionCache::create(SPRequest& request, DDF& session)
string key;
try {
m_log.debug("writing new session to persistent store");
- key = cache_create(&request, session);
+ key = cache_create(&request, data);
}
catch (const exception&) {
// Should be logged by the SPI.
- session.destroy();
+ data.destroy();
throw;
}
- session.name(key.c_str());
- unique_ptr<BasicSession> sessionObject(new BasicSession(*this, session));
+ data.name(key.c_str());
+ unique_ptr<BasicSession> sessionObject(new BasicSession(*this, data));
const char* issuer = nullptr;
const auto& attr = sessionObject->getAttributes().find(m_issuerAttribute);
@@ -571,6 +571,63 @@ unique_lock<Session> AbstractSessionCache::_find(
return unique_lock<Session>(*ref);
}
+bool AbstractSessionCache::update(SPRequest& request, unique_lock<Session>& session, DDF& data, const char* reason)
+{
+ // On input we hold an exclusive lock on the relevant session.
+
+ DDF newData;
+
+ try {
+ // Validate and reformat attribute data.
+ const AttributeConfiguration& attrConfig = request.getAgent().getAttributeConfiguration(
+ request.getRequestSettings().first->getString(RequestMapper::ATTRIBUTE_CONFIG_ID_PROP_NAME));
+ DDF attrs = data["attributes"];
+ if (!attrConfig.processAttributes(attrs)) {
+ m_log.warn("error processing updated session attributes for storage/use");
+ data.destroy();
+ throw SessionException("Error while processing updated session attributes for storage.");
+ }
+
+ m_log.info("updating session (%s), version (%u), reason (%s)",
+ session.mutex()->getID(), session.mutex()->getVersion(), reason ? reason : "(unspecified)");
+
+ // The update requires that we copy the existing DDF from the original session and then
+ // replace members with "like" names.
+ newData = dynamic_cast<BasicSession*>(session.mutex())->cloneData();
+ DDFJanitor janitor(newData);
+ DDF child = data.first();
+ while (!child.isnull()) {
+ // This call defends against an empty name, and handles cleanup of an existing member by the same name.
+ newData.add(child);
+ child = data.next();
+ }
+ // Free whatever's left of the input.
+ data.destroy();
+
+ // Attempt to update the back-end.
+ if (cache_update(&request, session.mutex()->getID(), session.mutex()->getVersion(), newData)) {
+ // On success, the version field will have been updated and we need to overwrite the local copy of
+ // this session's data.
+ dynamic_cast<BasicSession*>(session.mutex())->updateData(newData);
+ janitor.release();
+
+ // We need to update our cookie since by definition we incremented the version.
+ string new_cookieval(session.mutex()->getID());
+ computeVersionedFilename(new_cookieval, session.mutex()->getVersion());
+ m_cookieManager->setCookie(request, new_cookieval.c_str());
+ return true;
+ }
+ else {
+ return false;
+ }
+ }
+ catch (const exception& e) {
+ // Should be logged by the SPI.
+ data.destroy();
+ throw;
+ }
+}
+
void AbstractSessionCache::remove(SPRequest& request)
{
const char* cookieval = m_cookieManager->getCookieValue(request);
@@ -760,6 +817,22 @@ const std::map<std::string,DDF>& BasicSession::getAttributes() const
return m_attributes;
}
+DDF BasicSession::getOpaqueData() const
+{
+ return m_obj["opaque"];
+}
+
+DDF BasicSession::cloneData() const
+{
+ return m_obj.copy();
+}
+
+void BasicSession::updateData(DDF& data)
+{
+ m_obj.destroy();
+ m_obj = data;
+}
+
bool BasicSession::isValid(SPRequest* request, unsigned int lifetime, unsigned int timeout)
{
time_t now = time(nullptr);
diff --git a/tests/impl/XMLAccessControlTests.cpp b/tests/impl/XMLAccessControlTests.cpp
index 63217b9d..be183714 100644
--- a/tests/impl/XMLAccessControlTests.cpp
+++ b/tests/impl/XMLAccessControlTests.cpp
@@ -72,6 +72,9 @@ public:
const map<string,DDF>& getAttributes() const {
return m_attributes;
}
+ DDF getOpaqueData() const {
+ return DDF();
+ }
map<string,DDF> m_attributes;
};
diff --git a/tests/impl/XMLRequestMapperTests.cpp b/tests/impl/XMLRequestMapperTests.cpp
index f1e3e2af..304a130e 100644
--- a/tests/impl/XMLRequestMapperTests.cpp
+++ b/tests/impl/XMLRequestMapperTests.cpp
@@ -74,6 +74,9 @@ public:
const map<string,DDF>& getAttributes() const {
return m_attributes;
}
+ DDF getOpaqueData() const {
+ return DDF();
+ }
map<string,DDF> m_attributes;
};
diff --git a/tests/session/impl/FilesystemSessionCacheTests.cpp b/tests/session/impl/FilesystemSessionCacheTests.cpp
index c1a51e9f..20be541c 100644
--- a/tests/session/impl/FilesystemSessionCacheTests.cpp
+++ b/tests/session/impl/FilesystemSessionCacheTests.cpp
@@ -72,6 +72,25 @@ struct FilesystemFixture
std::remove(trackingfile.c_str());
}
+ DDF createTestData(const char* opaque) {
+ DDF obj(nullptr);
+ obj.addmember("session.opaque").string(opaque);
+ DDF attrs = obj.addmember("session.attributes").list();
+
+ DDF issuer("Shib-Identity-Provider");
+ issuer.list();
+ issuer.add(DDF(nullptr).string("https://idp.example.org"));
+ attrs.add(issuer);
+
+ DDF affiliation("affiliation");
+ affiliation.list();
+ affiliation.add(DDF(nullptr).string("member"));
+ affiliation.add(DDF(nullptr).string("student"));
+ attrs.add(affiliation);
+
+ return obj;
+ }
+
string data_path;
};
@@ -80,7 +99,7 @@ BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_invalid_attributes, FilesystemFix
bool started = AgentConfig::getConfig().start();
BOOST_CHECK(started);
- DDF obj(nullptr);
+ DDF obj = DDF(nullptr);
DDFJanitor janitor(obj);
obj.addmember("session.attributes"); // not a list
@@ -99,7 +118,7 @@ BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_tests, FilesystemFixture)
bool started = AgentConfig::getConfig().start();
BOOST_CHECK(started);
- DDF obj(nullptr);
+ DDF obj = createTestData("foo");
DDFJanitor janitor(obj);
obj.addmember("session.opaque").string("foo");
@@ -167,4 +186,73 @@ BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_tests, FilesystemFixture)
BOOST_CHECK(!session);
}
+BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_testUpdate, FilesystemFixture)
+{
+ bool started = AgentConfig::getConfig().start();
+ BOOST_CHECK(started);
+
+ DDF obj = createTestData("foo");
+ DDFJanitor janitor(obj);
+
+ DummyRequest request("https://sp.example.org/secure/index.html");
+ DDF child = obj["session"];
+
+ SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
+
+ string key = cache->create(request, child);
+
+ BOOST_CHECK(obj["session"].isnull());
+ BOOST_CHECK_EQUAL(key.c_str(), child.name());
+
+ // Bind session to request with cookie.
+ string cookieName("__Host-shibsession_73702e6578616d706c652e6f7267637573746f6d");
+ string cookie(cookieName);
+ cookie = cookie + '=' + key + ".1";
+ request.m_requestHeaders["Cookie"] = cookie;
+
+ unique_lock<Session> session = cache->find(request, true, false);
+ BOOST_CHECK(session);
+
+ // Clear old response headers.
+ request.m_responseHeaders.clear();
+
+ if (session) {
+ BOOST_CHECK_EQUAL(session.mutex()->getVersion(), 1);
+ DDF opaque = session.mutex()->getOpaqueData();
+ BOOST_CHECK_EQUAL(opaque.string(), "foo");
+
+ // Explicit update.
+ DDF obj2 = createTestData("bar");
+ DDFJanitor janitor2(obj2);
+
+ child = obj2["session"];
+ BOOST_CHECK(cache->update(request, session, child, "unit testing"));
+
+ opaque = session.mutex()->getOpaqueData();
+ BOOST_CHECK_EQUAL(opaque.string(), "bar");
+ BOOST_CHECK_EQUAL(session.mutex()->getVersion(), 2);
+
+ session.unlock();
+ }
+
+ session = cache->find(request, true, false);
+ BOOST_CHECK(session);
+ if (session) {
+ BOOST_CHECK_EQUAL(session.mutex()->getVersion(), 2);
+ string header = cookieName + '=' + key + ".2";
+ header += "; Path=/; Secure=1; HttpOnly=1; SameSite=None";
+ BOOST_CHECK_EQUAL(request.m_responseHeaders["Set-Cookie"], header);
+ session.unlock();
+ }
+
+ cache->remove(request);
+
+ string header = cookieName;
+ header += "=; Max-Age=0; Path=/; Secure=1; HttpOnly=1; SameSite=None";
+ BOOST_CHECK_EQUAL(request.m_responseHeaders["Set-Cookie"], header);
+
+ session = cache->find("custom", key.c_str());
+ BOOST_CHECK(!session);
+}
+
}
diff --git a/tests/session/impl/MemorySessionCacheTests.cpp b/tests/session/impl/MemorySessionCacheTests.cpp
index 403a81f4..a538c2ac 100644
--- a/tests/session/impl/MemorySessionCacheTests.cpp
+++ b/tests/session/impl/MemorySessionCacheTests.cpp
@@ -49,6 +49,25 @@ struct MemoryFixture
AgentConfig::getConfig().term();
}
+ DDF createTestData(const char* opaque) {
+ DDF obj(nullptr);
+ obj.addmember("session.opaque").string(opaque);
+ DDF attrs = obj.addmember("session.attributes").list();
+
+ DDF issuer("Shib-Identity-Provider");
+ issuer.list();
+ issuer.add(DDF(nullptr).string("https://idp.example.org"));
+ attrs.add(issuer);
+
+ DDF affiliation("affiliation");
+ affiliation.list();
+ affiliation.add(DDF(nullptr).string("member"));
+ affiliation.add(DDF(nullptr).string("student"));
+ attrs.add(affiliation);
+
+ return obj;
+ }
+
string data_path;
};
@@ -59,23 +78,9 @@ BOOST_FIXTURE_TEST_CASE(MemorySessionCache_tests, MemoryFixture)
bool started = AgentConfig::getConfig().start();
BOOST_CHECK(started);
- DDF obj(nullptr);
+ DDF obj = createTestData("foo");
DDFJanitor janitor(obj);
- obj.addmember("session.opaque").string("foo");
- DDF attrs = obj.addmember("session.attributes").list();
-
- DDF issuer("Shib-Identity-Provider");
- issuer.list();
- issuer.add(DDF(nullptr).string("https://idp.example.org"));
- attrs.add(issuer);
-
- DDF affiliation("affiliation");
- affiliation.list();
- affiliation.add(DDF(nullptr).string("member"));
- affiliation.add(DDF(nullptr).string("student"));
- attrs.add(affiliation);
-
DummyRequest request("https://sp.example.org/secure/index.html");
DDF child = obj["session"];
@@ -127,4 +132,73 @@ BOOST_FIXTURE_TEST_CASE(MemorySessionCache_tests, MemoryFixture)
BOOST_CHECK(!session);
}
+BOOST_FIXTURE_TEST_CASE(MemorySessionCache_testUpdate, MemoryFixture)
+{
+ bool started = AgentConfig::getConfig().start();
+ BOOST_CHECK(started);
+
+ DDF obj = createTestData("foo");
+ DDFJanitor janitor(obj);
+
+ DummyRequest request("https://sp.example.org/secure/index.html");
+ DDF child = obj["session"];
+
+ SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
+
+ string key = cache->create(request, child);
+
+ BOOST_CHECK(obj["session"].isnull());
+ BOOST_CHECK_EQUAL(key.c_str(), child.name());
+
+ // Bind session to request with cookie.
+ string cookieName("__Host-shibsession_73702e6578616d706c652e6f7267637573746f6d");
+ string cookie(cookieName);
+ cookie = cookie + '=' + key + ".1";
+ request.m_requestHeaders["Cookie"] = cookie;
+
+ unique_lock<Session> session = cache->find(request, true, false);
+ BOOST_CHECK(session);
+
+ // Clear old response headers.
+ request.m_responseHeaders.clear();
+
+ if (session) {
+ BOOST_CHECK_EQUAL(session.mutex()->getVersion(), 1);
+ DDF opaque = session.mutex()->getOpaqueData();
+ BOOST_CHECK_EQUAL(opaque.string(), "foo");
+
+ // Explicit update.
+ DDF obj2 = createTestData("bar");
+ DDFJanitor janitor2(obj2);
+
+ child = obj2["session"];
+ BOOST_CHECK(cache->update(request, session, child));
+
+ opaque = session.mutex()->getOpaqueData();
+ BOOST_CHECK_EQUAL(opaque.string(), "bar");
+ BOOST_CHECK_EQUAL(session.mutex()->getVersion(), 2);
+
+ session.unlock();
+ }
+
+ session = cache->find(request, true, false);
+ BOOST_CHECK(session);
+ if (session) {
+ BOOST_CHECK_EQUAL(session.mutex()->getVersion(), 2);
+ string header = cookieName + '=' + key + ".2";
+ header += "; Path=/; Secure=1; HttpOnly=1; SameSite=None";
+ BOOST_CHECK_EQUAL(request.m_responseHeaders["Set-Cookie"], header);
+ session.unlock();
+ }
+
+ cache->remove(request);
+
+ string header = cookieName;
+ header += "=; Max-Age=0; Path=/; Secure=1; HttpOnly=1; SameSite=None";
+ BOOST_CHECK_EQUAL(request.m_responseHeaders["Set-Cookie"], header);
+
+ session = cache->find("custom", key.c_str());
+ BOOST_CHECK(!session);
+}
+
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list