[cpp-sp] branch main updated: Lots of bug fixes and primitive unit test.
Scott Cantor
cantor.2 at osu.edu
Tue Jun 3 19:32:18 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=9593e20f0480cee0779d3334d3cb637e37d5f566
The following commit(s) were added to refs/heads/main by this push:
new 9593e20f Lots of bug fixes and primitive unit test.
9593e20f is described below
commit 9593e20f0480cee0779d3334d3cb637e37d5f566
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Jun 3 15:32:15 2025 -0400
Lots of bug fixes and primitive unit test.
---
shibsp/RequestMapper.h | 2 +
shibsp/impl/AgentConfig.cpp | 5 +
shibsp/io/CookieManager.h | 2 +-
shibsp/io/impl/CookieManager.cpp | 10 +-
shibsp/session/AbstractSessionCache.h | 2 +
shibsp/session/SessionCache.h | 7 ++
shibsp/session/impl/AbstractSessionCache.cpp | 34 ++++---
shibsp/session/impl/MemorySessionCache.cpp | 17 +++-
tests/Makefile.am | 1 +
tests/data/session/impl/memory-shibboleth.ini | 20 ++++
tests/data/session/impl/request-map.xml | 4 +
tests/impl/XMLAccessControlTests.cpp | 5 +-
tests/impl/XMLRequestMapperTests.cpp | 3 +-
tests/session/impl/MemorySessionCacheTests.cpp | 122 +++++++++++++++++++++++++
14 files changed, 210 insertions(+), 24 deletions(-)
diff --git a/shibsp/RequestMapper.h b/shibsp/RequestMapper.h
index b23baf99..3d510ac2 100644
--- a/shibsp/RequestMapper.h
+++ b/shibsp/RequestMapper.h
@@ -29,6 +29,8 @@
#include <shibsp/util/Lockable.h>
+#include <tuple>
+
namespace shibsp {
class SHIBSP_API AccessControl;
diff --git a/shibsp/impl/AgentConfig.cpp b/shibsp/impl/AgentConfig.cpp
index 35464ed7..9e0a758a 100644
--- a/shibsp/impl/AgentConfig.cpp
+++ b/shibsp/impl/AgentConfig.cpp
@@ -327,6 +327,11 @@ void AgentInternalConfig::_term()
Category& log=Category::getInstance(SHIBSP_LOGCAT ".AgentConfig");
log.info("%s agent shutting down", PACKAGE_STRING);
+ SessionCache* cache = getAgent().getSessionCache(false);
+ if (cache) {
+ cache->stop();
+ }
+
AgentManager.deregisterFactories();
SessionCacheManager.deregisterFactories();
RemotingServiceManager.deregisterFactories();
diff --git a/shibsp/io/CookieManager.h b/shibsp/io/CookieManager.h
index ee3f5012..cc50716d 100644
--- a/shibsp/io/CookieManager.h
+++ b/shibsp/io/CookieManager.h
@@ -191,7 +191,7 @@ namespace shibsp {
private:
std::string computeCookieName(const SPRequest& request) const;
- void outputHeader(SPRequest& request, int maxAge) const;
+ void outputHeader(SPRequest& request, const char* value, int maxAge) const;
std::string m_defaultName;
std::string m_overrideProperty;
diff --git a/shibsp/io/impl/CookieManager.cpp b/shibsp/io/impl/CookieManager.cpp
index 7d388c49..f82eef90 100644
--- a/shibsp/io/impl/CookieManager.cpp
+++ b/shibsp/io/impl/CookieManager.cpp
@@ -136,9 +136,13 @@ string CookieManager::computeCookieName(const SPRequest& request) const
return cookieName;
}
-void CookieManager::outputHeader(SPRequest& request, int maxAge) const
+void CookieManager::outputHeader(SPRequest& request, const char* value, int maxAge) const
{
string header(computeCookieName(request));
+ header += '=';
+ if (value) {
+ header += value;
+ }
header += "; max-age=";
try {
header += boost::lexical_cast<string>(maxAge);
@@ -188,10 +192,10 @@ const char* CookieManager::getCookieValue(const SPRequest& request) const
void CookieManager::setCookie(SPRequest& request, const char* value) const
{
- outputHeader(request, request.getRequestSettings().first->getInt("cookieMaxAge", m_maxAge));
+ outputHeader(request, value, request.getRequestSettings().first->getInt("cookieMaxAge", m_maxAge));
}
void CookieManager::unsetCookie(SPRequest& request) const
{
- outputHeader(request, 0);
+ outputHeader(request, nullptr, 0);
}
diff --git a/shibsp/session/AbstractSessionCache.h b/shibsp/session/AbstractSessionCache.h
index 7a23ba7f..b2f92b5c 100644
--- a/shibsp/session/AbstractSessionCache.h
+++ b/shibsp/session/AbstractSessionCache.h
@@ -87,6 +87,8 @@ namespace shibsp {
*/
bool start();
+ void stop();
+
// SessionCache API
std::string create(SPRequest& request, DDF& session);
std::unique_lock<Session> find(SPRequest& request, bool checkTimeout, bool ignoreAddress);
diff --git a/shibsp/session/SessionCache.h b/shibsp/session/SessionCache.h
index fc51b242..eb3e328c 100644
--- a/shibsp/session/SessionCache.h
+++ b/shibsp/session/SessionCache.h
@@ -112,6 +112,13 @@ namespace shibsp {
*/
virtual bool start()=0;
+ /**
+ * Signals the implementation it should stop any background tasks.
+ *
+ * <p>This method is guaranteed to be called only once per process.</p>
+ */
+ virtual void stop()=0;
+
/**
* Creates a new session and stores it persistently while binding the session
* to the input request object.
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index e9f686e7..1f215946 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -101,7 +101,8 @@ SessionCacheSPI::~SessionCacheSPI()
{
}
-AbstractSessionCache::AbstractSessionCache(const ptree& pt) : m_log(Category::getInstance(SHIBSP_LOGCAT ".SessionCache"))
+AbstractSessionCache::AbstractSessionCache(const ptree& pt)
+ : m_log(Category::getInstance(SHIBSP_LOGCAT ".SessionCache")), m_shutdown(false)
{
load(pt);
@@ -121,12 +122,6 @@ AbstractSessionCache::AbstractSessionCache(const ptree& pt) : m_log(Category::ge
AbstractSessionCache::~AbstractSessionCache()
{
- // Notify and join with the cleanup thread.
- m_shutdown = true;
- m_shutdown_wait.notify_all();
- if (m_cleanup_thread.joinable()) {
- m_cleanup_thread.join();
- }
}
Category& AbstractSessionCache::log() const
@@ -146,6 +141,16 @@ bool AbstractSessionCache::start()
return false;
}
+void AbstractSessionCache::stop()
+{
+ // Notify and join with the cleanup thread.
+ m_shutdown = true;
+ m_shutdown_wait.notify_all();
+ if (m_cleanup_thread.joinable()) {
+ m_cleanup_thread.join();
+ }
+}
+
string AbstractSessionCache::create(SPRequest& request, DDF& session)
{
m_log.debug("creating new session");
@@ -179,8 +184,9 @@ string AbstractSessionCache::create(SPRequest& request, DDF& session)
if (attr != sessionObject->getAttributes().end()) {
issuer = const_cast<DDF&>(attr->second).first().string();
}
+
m_log.info("new session created: ID (%s), Issuer (%s), Address (%s)",
- key.c_str(), issuer ? issuer : "none", request.getRemoteAddr().c_str());
+ key.c_str(), issuer ? issuer : "unknown", request.getRemoteAddr().c_str());
// Drop a cookie with the session ID.
m_cookieManager->setCookie(request, key.c_str());
@@ -393,20 +399,20 @@ void* AbstractSessionCache::cleanup_fn(void* p)
pthread_sigmask(SIG_BLOCK, &sigmask, nullptr);
#endif
- mutex internal_mutex;
-
// Load our configuration details...
unsigned int cleanupInterval = pcache->getUnsignedInt(CLEANUP_INTERVAL_PROP_NAME, CLEANUP_INTERVAL_PROP_DEFAULT);
unsigned int inprocTimeout = pcache->getUnsignedInt(INPROC_TIMEOUT_PROP_NAME, INPROC_TIMEOUT_PROP_DEFAULT);
+ mutex internal_mutex;
unique_lock lock(internal_mutex);
- pcache->m_log.info("cleanup thread started...run every %u secs; timeout after %u secs", cleanupInterval, inprocTimeout);
+ pcache->m_log.info("cleanup thread started...run every %u secs, timeout after %u secs", cleanupInterval, inprocTimeout);
while (!pcache->m_shutdown) {
pcache->m_shutdown_wait.wait_for(lock, chrono::seconds(cleanupInterval));
if (pcache->m_shutdown) {
+ pcache->m_log.debug("cleanup thread shutting down");
break;
}
@@ -448,7 +454,7 @@ void* AbstractSessionCache::cleanup_fn(void* p)
}
}
- pcache->m_log.debug("cleanup thread completed");
+ pcache->m_log.debug("cleanup thread completed work");
}
pcache->m_log.info("cleanup thread exiting");
@@ -469,7 +475,9 @@ BasicSession::BasicSession(AbstractSessionCache& cache, DDF& obj)
DDF attrs = m_obj["attributes"];
DDF attr = attrs.first();
while (!attr.isnull()) {
- m_attributes[attr.name()] = attr;
+ if (attr.name()) {
+ m_attributes[attr.name()] = attr;
+ }
attr = attrs.next();
}
}
diff --git a/shibsp/session/impl/MemorySessionCache.cpp b/shibsp/session/impl/MemorySessionCache.cpp
index 1352bc6e..33d3b212 100644
--- a/shibsp/session/impl/MemorySessionCache.cpp
+++ b/shibsp/session/impl/MemorySessionCache.cpp
@@ -35,6 +35,7 @@ using namespace boost::property_tree;
using namespace std;
namespace {
+
class MemorySessionCache : public virtual AbstractSessionCache {
public:
MemorySessionCache(const ptree& pt);
@@ -54,6 +55,20 @@ namespace {
private:
duthomhas::csprng m_rng;
};
+
+ static inline string hexify(string& s) {
+ static char DIGITS[] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
+
+ string ret;
+
+ for (const char* ch = s.c_str(); *ch; ++ch) {
+ ret += (DIGITS[((unsigned char)(0xF0 & *ch)) >> 4 ]);
+ ret += (DIGITS[0x0F & *ch]);
+ }
+
+ return ret;
+ }
+
};
namespace shibsp {
@@ -72,7 +87,7 @@ MemorySessionCache::~MemorySessionCache()
string MemorySessionCache::cache_create(DDF& sessionData)
{
- return m_rng(string(16,0));
+ return hexify(m_rng(string(16,0)));
}
DDF MemorySessionCache::cache_read(
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7f9994ee..6a332cbb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -17,6 +17,7 @@ shibsptest_SOURCES = \
platform/iis/ModuleConfigTests.cpp \
remoting/impl/RemotingServiceTests.cpp \
remoting/impl/SecretSourceTests.cpp \
+ session/impl/MemorySessionCacheTests.cpp \
util/PropertyTreeTests.cpp \
util/BoostPropertySetTests.cpp \
util/ReloadableXMLFileTests.cpp
diff --git a/tests/data/session/impl/memory-shibboleth.ini b/tests/data/session/impl/memory-shibboleth.ini
new file mode 100644
index 00000000..ab381f79
--- /dev/null
+++ b/tests/data/session/impl/memory-shibboleth.ini
@@ -0,0 +1,20 @@
+[global]
+agentID = sp.example.org
+skipHandlers = true
+skipAttributes = true
+# Use "partial" for partial matching
+regexMatching = full
+
+[logging]
+type = console
+defaultLevel = INFO
+
+[logging-categories]
+Shibboleth.SessionCache = DEBUG
+
+[session-cache]
+type = memory
+cleanupInterval = 180
+
+[request-mapper]
+path = ./data/session/impl/request-map.xml
diff --git a/tests/data/session/impl/request-map.xml b/tests/data/session/impl/request-map.xml
new file mode 100644
index 00000000..733ee1e1
--- /dev/null
+++ b/tests/data/session/impl/request-map.xml
@@ -0,0 +1,4 @@
+<RequestMap applicationId="custom">
+ <Host name="sp.example.org">
+ </Host>
+</RequestMap>
diff --git a/tests/impl/XMLAccessControlTests.cpp b/tests/impl/XMLAccessControlTests.cpp
index f7f58558..9631e49e 100644
--- a/tests/impl/XMLAccessControlTests.cpp
+++ b/tests/impl/XMLAccessControlTests.cpp
@@ -91,7 +91,7 @@ public:
string getRemoteUser() const { return m_user; }
string getAuthType() const { return nullptr; }
long sendResponse(istream&, long status) { return status; }
- void clearHeader(const char*, const char*) {}
+ void clearHeader(const char* name) {}
void setHeader(const char*, const char*) {}
void setRemoteUser(const char*) {}
long returnDecline() { return 200; }
@@ -100,9 +100,6 @@ public:
bool isUseHeaders() const { return true; }
bool isUseVariables() const { return false; }
- void clearHeader(const char* name) {}
-
-
string m_user;
DummyRequestMap m_map;
};
diff --git a/tests/impl/XMLRequestMapperTests.cpp b/tests/impl/XMLRequestMapperTests.cpp
index 908038d9..d5ecd584 100644
--- a/tests/impl/XMLRequestMapperTests.cpp
+++ b/tests/impl/XMLRequestMapperTests.cpp
@@ -90,7 +90,7 @@ public:
string getRemoteUser() const { return m_user.c_str(); }
string getAuthType() const { return nullptr; }
long sendResponse(istream&, long status) { return status; }
- void clearHeader(const char*, const char*) {}
+ void clearHeader(const char*) {}
void setHeader(const char*, const char*) {}
void setRemoteUser(const char*) {}
long returnDecline() { return 200; }
@@ -98,7 +98,6 @@ public:
bool isUseHeaders() const {return true;}
bool isUseVariables() const { return false; }
- void clearHeader(const char* name) {}
string m_scheme;
string m_hostname;
diff --git a/tests/session/impl/MemorySessionCacheTests.cpp b/tests/session/impl/MemorySessionCacheTests.cpp
new file mode 100644
index 00000000..44c418a3
--- /dev/null
+++ b/tests/session/impl/MemorySessionCacheTests.cpp
@@ -0,0 +1,122 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * session/impl/MemorySessionCacheTests.cpp
+ *
+ * Unit tests for in-memory SessionCache back-end.
+ */
+
+#include "AbstractSPRequest.h"
+#include "Agent.h"
+#include "AgentConfig.h"
+#include "exceptions.h"
+#include "remoting/ddf.h"
+#include "session/SessionCache.h"
+
+#include <memory>
+#include <string>
+#include <boost/test/unit_test.hpp>
+#include <boost/property_tree/ini_parser.hpp>
+
+using namespace shibsp;
+using namespace boost::property_tree;
+using namespace std;
+
+#define DATA_PATH "./data/session/impl/"
+
+namespace {
+
+class DummyRequest : public AbstractSPRequest {
+public:
+ DummyRequest(const char* uri=nullptr) : AbstractSPRequest(SHIBSP_LOGCAT ".DummyRequest") {
+ setRequestURI(uri);
+ }
+ const char* getMethod() const { return nullptr; }
+ const char* getScheme() const { return m_scheme.c_str(); }
+ const char* getHostname() const { return m_hostname.c_str(); }
+ int getPort() const { return m_port; }
+ string getContentType() const { return ""; }
+ long getContentLength() const { return -1; }
+ const char* getQueryString() const { return m_query.c_str(); }
+ const char* getRequestBody() const { return nullptr; }
+ string getHeader(const char*) const { return nullptr; }
+ string getRemoteUser() const { return m_user.c_str(); }
+ string getAuthType() const { return nullptr; }
+ long sendResponse(istream&, long status) { return status; }
+ void clearHeader(const char* name) {}
+ void setHeader(const char* name, const char* value) {}
+ void setRemoteUser(const char*) {}
+ long returnDecline() { return 200; }
+ long returnOK() { return 200; }
+
+ bool isUseHeaders() const {return true;}
+ bool isUseVariables() const { return false; }
+
+ string m_scheme;
+ string m_hostname;
+ int m_port;
+ string m_query;
+ string m_user;
+ map<string,string> m_headers;
+};
+
+struct MemoryFixture
+{
+ MemoryFixture() : data_path(DATA_PATH) {
+ AgentConfig::getConfig().init(nullptr, (data_path + "memory-shibboleth.ini").c_str(), true);
+ }
+ ~MemoryFixture() {
+ AgentConfig::getConfig().term();
+ }
+
+ string data_path;
+};
+
+/////////////
+
+BOOST_FIXTURE_TEST_CASE(MemorySessionCache_tests, MemoryFixture)
+{
+ bool started = AgentConfig::getConfig().start();
+ BOOST_CHECK(started);
+
+ DDF obj(nullptr);
+ 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"];
+
+ 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());
+}
+
+}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list