[cpp-sp] branch main updated: SessionCache WIP with changes to external interface.

Scott Cantor cantor.2 at osu.edu
Wed May 21 13:37:37 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=45d835c70893d3c48c182ee32e19e6d3c5e5c70e

The following commit(s) were added to refs/heads/main by this push:
     new 45d835c7 SessionCache WIP with changes to external interface.
45d835c7 is described below

commit 45d835c70893d3c48c182ee32e19e6d3c5e5c70e
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed May 21 09:37:33 2025 -0400

    SessionCache WIP with changes to external interface.
---
 Projects/vc22/shibsp.vcxproj                       |   4 +-
 Projects/vc22/shibsp.vcxproj.filters               |  11 +-
 apache/mod_shib_24.cpp                             |  18 +-
 shibsp/AbstractSPRequest.cpp                       |  30 +--
 shibsp/AbstractSPRequest.h                         |   5 +-
 shibsp/Agent.cpp                                   |  53 ++---
 shibsp/Makefile.am                                 |   3 +-
 shibsp/SPRequest.h                                 |   7 +-
 shibsp/handler/impl/AdminLogoutInitiator.cpp       |  24 +--
 shibsp/handler/impl/AttributeCheckerHandler.cpp    |  14 +-
 shibsp/handler/impl/LocalLogoutInitiator.cpp       |  13 +-
 shibsp/handler/impl/SessionHandler.cpp             |  18 +-
 shibsp/io/CookieManager.h                          |   9 +
 shibsp/io/impl/CookieManager.cpp                   |  25 +++
 shibsp/remoting/ddf.h                              |   2 +
 .../remoting/impl/AbstractHTTPRemotingService.cpp  |   1 -
 shibsp/remoting/impl/ddf.cpp                       |   6 +
 shibsp/session/AbstractSessionCache.h              |  37 ++--
 shibsp/session/SessionCache.h                      | 134 +++++-------
 shibsp/session/SessionCacheSPI.h                   |  20 +-
 shibsp/session/impl/AbstractSessionCache.cpp       | 234 ++++++++++++++++++---
 shibsp/session/impl/FilesystemSessionCache.cpp     |  15 +-
 22 files changed, 404 insertions(+), 279 deletions(-)

diff --git a/Projects/vc22/shibsp.vcxproj b/Projects/vc22/shibsp.vcxproj
index 5bc447f6..1d935686 100644
--- a/Projects/vc22/shibsp.vcxproj
+++ b/Projects/vc22/shibsp.vcxproj
@@ -43,9 +43,6 @@
     <ClInclude Include="..\..\shibsp\handler\LogoutInitiator.h" />
     <ClInclude Include="..\..\shibsp\handler\RemotedHandler.h" />
     <ClInclude Include="..\..\shibsp\handler\SecuredHandler.h" />
-    <ClInclude Include="..\..\shibsp\impl\StorageServiceSessionCache.h" />
-    <ClInclude Include="..\..\shibsp\impl\XMLApplication.h" />
-    <ClInclude Include="..\..\shibsp\impl\XMLServiceProvider.h" />
     <ClInclude Include="..\..\shibsp\internal.h" />
     <ClInclude Include="..\..\shibsp\io\CookieManager.h" />
     <ClInclude Include="..\..\shibsp\io\GenericRequest.h" />
@@ -67,6 +64,7 @@
     <ClInclude Include="..\..\shibsp\RequestMapper.h" />
     <ClInclude Include="..\..\shibsp\session\AbstractSessionCache.h" />
     <ClInclude Include="..\..\shibsp\session\SessionCache.h" />
+    <ClInclude Include="..\..\shibsp\session\SessionCacheSPI.h" />
     <ClInclude Include="..\..\shibsp\SPRequest.h" />
     <ClInclude Include="..\..\shibsp\util\BoostPropertySet.h" />
     <ClInclude Include="..\..\shibsp\util\CGIParser.h" />
diff --git a/Projects/vc22/shibsp.vcxproj.filters b/Projects/vc22/shibsp.vcxproj.filters
index 40d623d6..a3cd1ae9 100644
--- a/Projects/vc22/shibsp.vcxproj.filters
+++ b/Projects/vc22/shibsp.vcxproj.filters
@@ -174,15 +174,6 @@
     <ClInclude Include="..\..\shibsp\internal.h">
       <Filter>Header Files\Impl</Filter>
     </ClInclude>
-    <ClInclude Include="..\..\shibsp\impl\StorageServiceSessionCache.h">
-      <Filter>Header Files\Impl</Filter>
-    </ClInclude>
-    <ClInclude Include="..\..\shibsp\impl\XMLApplication.h">
-      <Filter>Header Files\Impl</Filter>
-    </ClInclude>
-    <ClInclude Include="..\..\shibsp\impl\XMLServiceProvider.h">
-      <Filter>Header Files\Impl</Filter>
-    </ClInclude>
     <ClInclude Include="..\..\shibsp\logging\impl\AbstractLoggingService.h">
       <Filter>Header Files\Impl</Filter>
     </ClInclude>
@@ -406,4 +397,4 @@
       <Filter>Resource Files</Filter>
     </CustomBuild>
   </ItemGroup>
-</Project>
\ No newline at end of file
+</Project>
diff --git a/apache/mod_shib_24.cpp b/apache/mod_shib_24.cpp
index 42fb67a6..17fbc455 100644
--- a/apache/mod_shib_24.cpp
+++ b/apache/mod_shib_24.cpp
@@ -1202,8 +1202,7 @@ extern "C" authz_status shib_session_check_authz(request_rec* r, const char*, co
         return sta.second;
 
     try {
-        Session* session = sta.first->getSession(false, true, false);
-        lock_guard<Session> slocker(*session, adopt_lock);
+        unique_lock<Session> session = sta.first->getSession(false, true);
         if (session) {
             sta.first->log(Priority::SHIB_DEBUG, "htaccess: accepting shib-session/valid-user based on active session");
             return AUTHZ_GRANTED;
@@ -1299,9 +1298,8 @@ extern "C" authz_status shib_acclass_check_authz(request_rec* r, const char* req
     const htAccessControl& hta = dynamic_cast<const ApacheRequestMapper*>(sta.first->getRequestSettings().first)->getHTAccessControl();
 
     try {
-        Session* session = sta.first->getSession(false, true, false);
-        lock_guard<Session> slocker(*session, adopt_lock);
-        if (session && hta.doShibAttr(*sta.first, session,
+        unique_lock<Session> session = sta.first->getSession(false, true);
+        if (session && hta.doShibAttr(*sta.first, session.mutex(),
                 sta.first->getAgent().getString("legacy-classref-attribute", "Shib-AuthnContext-Class"),
                 require_line) == AccessControl::shib_acl_true)
             return AUTHZ_GRANTED;
@@ -1323,11 +1321,10 @@ extern "C" authz_status shib_attr_check_authz(request_rec* r, const char* requir
     const htAccessControl& hta = dynamic_cast<const ApacheRequestMapper*>(sta.first->getRequestSettings().first)->getHTAccessControl();
 
     try {
-        Session* session = sta.first->getSession(false, true, false);
-        lock_guard<Session> slocker(*session, adopt_lock);
+        unique_lock<Session> session = sta.first->getSession(false, true);
         if (session) {
             const char* rule = ap_getword_conf(r->pool, &require_line);
-            if (rule && hta.doShibAttr(*sta.first, session, rule, require_line) == AccessControl::shib_acl_true)
+            if (rule && hta.doShibAttr(*sta.first, session.mutex(), rule, require_line) == AccessControl::shib_acl_true)
                 return AUTHZ_GRANTED;
         }
         return session ? AUTHZ_DENIED : AUTHZ_DENIED_NO_USER;
@@ -1348,11 +1345,10 @@ extern "C" authz_status shib_plugin_check_authz(request_rec* r, const char* requ
     const htAccessControl& hta = dynamic_cast<const ApacheRequestMapper*>(sta.first->getRequestSettings().first)->getHTAccessControl();
 
     try {
-        Session* session = sta.first->getSession(false, true, false);
-        lock_guard<Session> slocker(*session, adopt_lock);
+        unique_lock<Session> session = sta.first->getSession(false, true);
         if (session) {
             const char* config = ap_getword_conf(r->pool, &require_line);
-            if (config && hta.doAccessControl(*sta.first, session, config) == AccessControl::shib_acl_true)
+            if (config && hta.doAccessControl(*sta.first, session.mutex(), config) == AccessControl::shib_acl_true)
                 return AUTHZ_GRANTED;
         }
         return session ? AUTHZ_DENIED : AUTHZ_DENIED_NO_USER;
diff --git a/shibsp/AbstractSPRequest.cpp b/shibsp/AbstractSPRequest.cpp
index 610de8ff..a324cbc9 100644
--- a/shibsp/AbstractSPRequest.cpp
+++ b/shibsp/AbstractSPRequest.cpp
@@ -52,14 +52,13 @@ SPRequest::~SPRequest()
 AbstractSPRequest::AbstractSPRequest(const char* category)
     : m_log(Category::getInstance(category)),
         m_agent(AgentConfig::getConfig().getAgent()),
-        m_mapper(nullptr), m_sessionTried(false), m_session(nullptr)
+        m_mapper(nullptr)
 {
 }
 
 AbstractSPRequest::~AbstractSPRequest()
 {
-    if (m_session)
-        m_session->unlock();
+    // TODO: wrap this in a proper lock wrapper?
     if (m_mapper)
         m_mapper->unlock_shared();
 }
@@ -84,30 +83,9 @@ RequestMapper::Settings AbstractSPRequest::getRequestSettings() const
     return m_settings;
 }
 
-Session* AbstractSPRequest::getSession(bool checkTimeout, bool ignoreAddress, bool cache)
+unique_lock<Session> AbstractSPRequest::getSession(bool checkTimeout, bool ignoreAddress)
 {
-    // Only attempt this once.
-    if (cache && m_sessionTried)
-        return m_session;
-    else if (cache)
-        m_sessionTried = true;
-
-    // Need address checking and timeout settings.
-    time_t timeout = 3600;
-    if (checkTimeout || !ignoreAddress) {
-        if (checkTimeout) {
-            timeout = getRequestSettings().first->getUnsignedInt("timeout", 3600);
-        }
-        ignoreAddress = !getRequestSettings().first->getBool("consistentAddress", true);
-    }
-
-    // The cache will either silently pass a session or nullptr back, or throw an exception out.
-    Session* session = getAgent().getSessionCache()->find(
-        *this, (ignoreAddress ? nullptr : getRemoteAddr().c_str()), (checkTimeout ? &timeout : nullptr)
-        );
-    if (cache)
-        m_session = session;
-    return session;
+    return getAgent().getSessionCache()->find(*this, checkTimeout, ignoreAddress);
 }
 
 void AbstractSPRequest::setRequestURI(const char* uri)
diff --git a/shibsp/AbstractSPRequest.h b/shibsp/AbstractSPRequest.h
index 52f38671..21510434 100644
--- a/shibsp/AbstractSPRequest.h
+++ b/shibsp/AbstractSPRequest.h
@@ -62,7 +62,8 @@ namespace shibsp {
         // Virtual function overrides.
         const Agent& getAgent() const;
         RequestMapper::Settings getRequestSettings() const;
-        Session* getSession(bool checkTimeout=true, bool ignoreAddress=false, bool cache=true);
+        std::unique_lock<Session> getSession(bool checkTimeout=true, bool ignoreAddress=false);
+        Session* getCachedSession(bool checkTimeout=true, bool ignoreAddress=false);
         const char* getRequestURI() const;
         const char* getRequestURL() const;
         std::string getRemoteAddr() const;
@@ -92,8 +93,6 @@ namespace shibsp {
         Agent& m_agent;
         mutable RequestMapper* m_mapper;
         mutable RequestMapper::Settings m_settings;
-        mutable bool m_sessionTried;
-        mutable Session* m_session;
         std::string m_uri;
         mutable std::string m_url;
         mutable std::string m_handlerURL;
diff --git a/shibsp/Agent.cpp b/shibsp/Agent.cpp
index 54adddf0..7a268a51 100644
--- a/shibsp/Agent.cpp
+++ b/shibsp/Agent.cpp
@@ -184,11 +184,9 @@ pair<bool,long> Agent::doAuthentication(SPRequest& request, bool handler) const
 
         bool sessionExists = false;
         try {
-            Session* session = request.getSession(true, false, false);   // don't cache it
-            if (session) {
-                sessionExists = true;
-                session->unlock();
-            }
+            unique_lock<Session> session = request.getSession();   // don't cache it but enforce policy
+            sessionExists = session.owns_lock();
+            // Lock will release here.
         }
         catch (const exception& e) {
             log.warn("error during session lookup: %s", e.what());
@@ -259,8 +257,7 @@ pair<bool,long> Agent::doAuthorization(SPRequest& request) const
 {
     Category& log = Category::getInstance(SHIBSP_LOGCAT ".Agent");
 
-    Session* session = nullptr;
-    unique_lock<Session> slocker;
+    unique_lock<Session> session;
     string targetURL = request.getRequestURL();
 
     try {
@@ -281,11 +278,7 @@ pair<bool,long> Agent::doAuthorization(SPRequest& request) const
         // Do we have an access control plugin?
         if (settings.second) {
             try {
-                session = request.getSession(false, false, false);  // ignore timeout and do not cache
-                if (session) {
-                    unique_lock<Session> slocker2(*session, adopt_lock);
-                    slocker.swap(slocker2); // assign to lock popper
-                }
+                session = request.getSession(false, false);  // ignore timeout and do not cache
             }
             catch (const exception& e) {
                 log.warn("unable to obtain session to pass to access control provider: %s", e.what());
@@ -294,7 +287,7 @@ pair<bool,long> Agent::doAuthorization(SPRequest& request) const
 #ifdef HAVE_CXX14
             shared_lock<AccessControl> acllock(*settings.second);
 #endif
-            switch (settings.second->authorized(request, session)) {
+            switch (settings.second->authorized(request, session.mutex())) {
                 case AccessControl::shib_acl_true:
                     log.debug("access control provider granted access");
                     return make_pair(true, request.returnOK());
@@ -304,7 +297,7 @@ pair<bool,long> Agent::doAuthorization(SPRequest& request) const
                     log.warn("access control provider denied access");
                     AgentException ex("Access to resource denied.");
                     ex.setStatusCode(HTTPResponse::SHIBSP_HTTP_STATUS_FORBIDDEN);
-                    return make_pair(true, handleError(log, request, session, &ex, false));
+                    return make_pair(true, handleError(log, request, session.mutex(), &ex, false));
                 }
 
                 default:
@@ -325,19 +318,14 @@ pair<bool,long> Agent::doExport(SPRequest& request, bool requireSession) const
 {
     Category& log = Category::getInstance(SHIBSP_LOGCAT ".Agent");
 
-    Session* session = nullptr;
-    unique_lock<Session> slocker;
+    unique_lock<Session> session;
     string targetURL = request.getRequestURL();
 
     try {
         RequestMapper::Settings settings = request.getRequestSettings();
 
         try {
-            session = request.getSession(false, false, false);  // ignore timeout and do not cache
-            if (session) {
-                unique_lock<Session> slocker2(*session, adopt_lock);
-                slocker.swap(slocker2); // assign to lock popper
-            }
+            session = request.getSession(false, false);  // ignore timeout and do not cache
         }
         catch (const exception& e) {
             log.warn("unable to obtain session to export to request: %s", e.what());
@@ -357,25 +345,25 @@ pair<bool,long> Agent::doExport(SPRequest& request, bool requireSession) const
             }
         }
 
-        request.setHeader("Shib-Session-ID", session->getID());
-        request.setHeader("Shib-Application-ID", session->getApplicationID());
+        request.setHeader("Shib-Session-ID", session.mutex()->getID());
+        request.setHeader("Shib-Application-ID", session.mutex()->getApplicationID());
 
         unsigned int lifetime = settings.first->getUnsignedInt("lifetime", 28800);
-        request.setHeader( "Shib-Session-Expires", boost::lexical_cast<string>(session->getCreation() + lifetime).c_str());
+        request.setHeader( "Shib-Session-Expires", boost::lexical_cast<string>(session.mutex()->getCreation() + lifetime).c_str());
         unsigned int timeout = settings.first->getUnsignedInt("timeout", 3600);
         if (timeout > 0) {
-            request.setHeader( "Shib-Session-Inactivity", boost::lexical_cast<string>(session->getLastAccess() + timeout).c_str());
+            request.setHeader( "Shib-Session-Inactivity", boost::lexical_cast<string>(session.mutex()->getLastAccess() + timeout).c_str());
         }
 
         // Export the attributes.
         request.getAgent().getAttributeConfiguration(
             request.getRequestSettings().first->getString("attributeConfigID")
-            ).exportAttributes(request, *session);
+            ).exportAttributes(request, *(session.mutex()));
 
         return make_pair(false,0L);
     }
     catch (exception& e) {
-        return make_pair(true, handleError(log, request, session, &e));
+        return make_pair(true, handleError(log, request, session.mutex(), &e));
     }
 }
 
@@ -435,17 +423,12 @@ pair<bool,long> Agent::doHandler(SPRequest& request) const
         throw ConfigurationException("Configured Shibboleth handler failed to process the request.");
     }
     catch (exception& e) {
-        Session* session = nullptr;
+        unique_lock<Session> session;
         try {
-            session = request.getSession(false, true, false);   // do not cache
+            session = request.getSession(false, true);   // do not cache
         }
         catch (const exception&) {
         }
-        if (session) {
-            lock_guard<Session> slocker(*session, adopt_lock); // pop existing lock on exit
-            return make_pair(true, handleError(log, request, session, &e));
-        } else {
-            return make_pair(true, handleError(log, request, nullptr, &e));
-        }
+        return make_pair(true, handleError(log, request, session.mutex(), &e));
     }
 }
diff --git a/shibsp/Makefile.am b/shibsp/Makefile.am
index 4f8c1c1e..c94599bb 100644
--- a/shibsp/Makefile.am
+++ b/shibsp/Makefile.am
@@ -70,7 +70,8 @@ reminclude_HEADERS = \
 
 sessioninclude_HEADERS = \
 	session/AbstractSessionCache.h \
-	session/SessionCache.h
+	session/SessionCache.h \
+	session/SessionCacheSPI.h
 	
 utilinclude_HEADERS = \
 	util/BoostPropertySet.h \
diff --git a/shibsp/SPRequest.h b/shibsp/SPRequest.h
index 5182bae5..099d3f65 100644
--- a/shibsp/SPRequest.h
+++ b/shibsp/SPRequest.h
@@ -84,13 +84,12 @@ namespace shibsp {
 
         /**
          * Returns a locked Session associated with the request.
-         *
+         * 
          * @param checkTimeout  true iff the last-used timestamp should be updated and any timeout policy enforced
          * @param ignoreAddress true iff all address checking should be ignored, regardless of policy
-         * @param cache         true iff the request should hold the Session lock itself and unlock during cleanup
-         * @return pointer to Session, or nullptr
+         * @return a locked mutex wrapper around the Session (or an empty wrapper if no Session exists)
          */
-        virtual Session* getSession(bool checkTimeout=true, bool ignoreAddress=false, bool cache=true)=0;
+        virtual std::unique_lock<Session> getSession(bool checkTimeout=true, bool ignoreAddress=false)=0;
 
         /**
          * Returns the effective base Handler URL for a resource,
diff --git a/shibsp/handler/impl/AdminLogoutInitiator.cpp b/shibsp/handler/impl/AdminLogoutInitiator.cpp
index abe588d5..83062326 100644
--- a/shibsp/handler/impl/AdminLogoutInitiator.cpp
+++ b/shibsp/handler/impl/AdminLogoutInitiator.cpp
@@ -84,9 +84,11 @@ pair<bool,long> AdminLogoutInitiator::run(SPRequest& request, bool isHandler) co
         return make_pair(true, request.sendResponse(msg, HTTPResponse::SHIBSP_HTTP_STATUS_BADREQUEST));
     }
 
-    Session* session = nullptr;
+    const char* applicationId = request.getRequestSettings().first->getString("applicationId", "default");
+
+    unique_lock<Session> session;
     try {
-        session = AgentConfig::getConfig().getAgent().getSessionCache()->find(request, sessionId);
+        session = AgentConfig::getConfig().getAgent().getSessionCache()->find(applicationId, sessionId);
     }
     catch (const std::exception& ex) {
         m_log.error("error accessing designated session: %s", ex.what());
@@ -94,23 +96,19 @@ pair<bool,long> AdminLogoutInitiator::run(SPRequest& request, bool isHandler) co
 
     // With no session, we return a 404 after "revoking" the session just to be safe.
     if (!session) {
-        AgentConfig::getConfig().getAgent().getSessionCache()->remove(
-            request.getRequestSettings().first->getString("applicationId", "default"), sessionId);
+        AgentConfig::getConfig().getAgent().getSessionCache()->remove(applicationId, sessionId);
         istringstream msg("NOT FOUND");
         return make_pair(true, request.sendResponse(msg, HTTPResponse::SHIBSP_HTTP_STATUS_NOTFOUND));
     }
 
-    time_t revocationExp = session->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800);
-
-    unique_lock<Session> sessionLocker(*session, adopt_lock);
+    time_t revocationExp = session.mutex()->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800);
 
     bool doSAML = false;
 
     // Do back channel notification.
-    vector<string> sessions(1, session->getID());
+    vector<string> sessions(1, session.mutex()->getID());
     if (!notifyBackChannel(request, sessions, true)) {
-        sessionLocker.unlock();
-        session = nullptr;
+        session.unlock();
         AgentConfig::getConfig().getAgent().getSessionCache()->remove(
             request.getRequestSettings().first->getString("applicationId", "default"), sessionId, revocationExp);
         
@@ -119,10 +117,8 @@ pair<bool,long> AdminLogoutInitiator::run(SPRequest& request, bool isHandler) co
     }
 
     if (!doSAML) {
-        sessionLocker.unlock();
-        session = nullptr;
-        AgentConfig::getConfig().getAgent().getSessionCache()->remove(
-            request.getRequestSettings().first->getString("applicationId", "default"), sessionId, revocationExp);
+        session.unlock();
+        AgentConfig::getConfig().getAgent().getSessionCache()->remove(applicationId, sessionId, revocationExp);
 
         istringstream msg("OK");
         return make_pair(true, request.sendResponse(msg, HTTPResponse::SHIBSP_HTTP_STATUS_OK));
diff --git a/shibsp/handler/impl/AttributeCheckerHandler.cpp b/shibsp/handler/impl/AttributeCheckerHandler.cpp
index 98601289..b51dc514 100644
--- a/shibsp/handler/impl/AttributeCheckerHandler.cpp
+++ b/shibsp/handler/impl/AttributeCheckerHandler.cpp
@@ -115,9 +115,9 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
         returnURL = request.getRequestSettings().first->getString("homeURL", "/");
     }
        
-    Session* session = nullptr;
+    unique_lock<Session> session;
     try {
-        session = request.getSession(true, false, false);
+        session = request.getSession();
         if (!session)
             request.log(Priority::SHIB_WARN, "AttributeChecker found session unavailable immediately after creation");
     }
@@ -125,12 +125,10 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
         request.log(Priority::SHIB_WARN, string("AttributeChecker caught exception accessing session immediately after creation: ") + ex.what());
     }
 
-    unique_lock<Session> sessionLocker(*session, adopt_lock);
-
     bool checked = false;
     if (session) {
         if (!m_attributes.empty()) {
-            const auto& indexed = session->getAttributes();
+            const auto& indexed = session.mutex()->getAttributes();
             // Lambda returns true if the candidate attribute ID is NOT in the session.
             auto absent = [&indexed](const string& id) {
                 return indexed.find(id) == indexed.end();
@@ -141,7 +139,7 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
             checked = find_if(m_attributes.begin(), m_attributes.end(), absent) == m_attributes.end();
         }
         else {
-            checked = (m_acl && m_acl->authorized(request, session) == AccessControl::shib_acl_true);
+            checked = (m_acl && m_acl->authorized(request, session.mutex()) == AccessControl::shib_acl_true);
         }
     }
 
@@ -152,8 +150,8 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
     }
 
     if (m_flushSession && session) {
-        time_t revocationExp = session->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800);
-        sessionLocker.unlock(); // unlock the session
+        time_t revocationExp = session.mutex()->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800);
+        session.unlock();
         flushSession(request, revocationExp);
     }
 
diff --git a/shibsp/handler/impl/LocalLogoutInitiator.cpp b/shibsp/handler/impl/LocalLogoutInitiator.cpp
index c73d1395..f447e934 100644
--- a/shibsp/handler/impl/LocalLogoutInitiator.cpp
+++ b/shibsp/handler/impl/LocalLogoutInitiator.cpp
@@ -72,24 +72,21 @@ pair<bool,long> LocalLogoutInitiator::run(SPRequest& request, bool isHandler) co
         return ret;
 
     // When out of process, we run natively.
-    Session* session = nullptr;
+    unique_lock<Session> session;
     try {
-        session = request.getSession(false, true, false);  // don't cache it and ignore all checks
+        session = request.getSession(false, true);  // don't cache it and ignore all checks
     }
     catch (const std::exception& ex) {
         m_log.error("error accessing current session: %s", ex.what());
     }
 
     if (session) {
-        // Guard the session in case of exception.
-        unique_lock<Session> locker(*session, adopt_lock);
-
         // Do back channel notification.
         bool result;
-        vector<string> sessions(1, session->getID());
+        vector<string> sessions(1, session.mutex()->getID());
         result = notifyBackChannel(request, sessions, true);
-        time_t revocationExp = session->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800);
-        locker.unlock();    // unlock the session
+        time_t revocationExp = session.mutex()->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800);
+        session.unlock();
         request.getAgent().getSessionCache()->remove(request, revocationExp);
         if (!result) {
             //return sendLogoutPage(request, "partial");
diff --git a/shibsp/handler/impl/SessionHandler.cpp b/shibsp/handler/impl/SessionHandler.cpp
index 30819c6e..c8508497 100644
--- a/shibsp/handler/impl/SessionHandler.cpp
+++ b/shibsp/handler/impl/SessionHandler.cpp
@@ -136,15 +136,15 @@ pair<bool,long> SessionHandler::doJSON(SPRequest& request) const
 {
     stringstream s;
 
-    Session* session = nullptr;
+    unique_lock<Session> session;
     try {
-        session = request.getSession(); // caches the locked session in the request so it's unlocked automatically
+        session = request.getSession();
         if (!session) {
             s << "{}" << endl;
             return make_pair(true, request.sendResponse(s));
         }
     }
-    catch (exception& ex) {
+    catch (const exception& ex) {
         m_log.info("exception accessing user session: %s", ex.what());
         s << "{}" << endl;
         return make_pair(true, request.sendError(s));
@@ -152,7 +152,7 @@ pair<bool,long> SessionHandler::doJSON(SPRequest& request) const
 
     s << "{ ";
     s << "\"expiration\": ";
-    s << ((session->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800) - time(nullptr)) / 60);
+    s << ((session.mutex()->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800) - time(nullptr)) / 60);
 
     /*
         attributes: [ { "name": "foo", "values" : count } ]
@@ -162,7 +162,7 @@ pair<bool,long> SessionHandler::doJSON(SPRequest& request) const
         ]
     */
 
-    const map<string,DDF>& attributes = session->getAttributes();
+    const map<string,DDF>& attributes = session.mutex()->getAttributes();
     if (!attributes.empty()) {
         s << ", \"attributes\": [ ";
         string key;
@@ -228,9 +228,9 @@ pair<bool,long> SessionHandler::doHTML(SPRequest& request) const
     stringstream s;
     s << "<html><head><title>Session Summary</title></head><body><pre>" << endl;
 
-    Session* session = nullptr;
+    unique_lock<Session> session;
     try {
-        session = request.getSession(); // caches the locked session in the request so it's unlocked automatically
+        session = request.getSession();
         if (!session) {
             s << "A valid session was not found.</pre></body></html>" << endl;
             return make_pair(true, request.sendResponse(s));
@@ -245,12 +245,12 @@ pair<bool,long> SessionHandler::doHTML(SPRequest& request) const
     s << "<u>Miscellaneous</u>" << endl;
 
     s << "<strong>Session Expiration (barring inactivity):</strong> ";
-    s << ((session->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800) - time(nullptr)) / 60) << " minute(s)" << endl;
+    s << ((session.mutex()->getCreation() + request.getRequestSettings().first->getUnsignedInt("lifetime", 28800) - time(nullptr)) / 60) << " minute(s)" << endl;
     s << endl << "<u>Attributes</u>" << endl;
 
     string key;
     int count=0;
-    const map<string,DDF>& attributes = session->getAttributes();
+    const map<string,DDF>& attributes = session.mutex()->getAttributes();
     for (map<string,DDF>::const_iterator a = attributes.begin(); a != attributes.end(); ++a) {
         if (a->first != key) {
             if (a != attributes.begin()) {
diff --git a/shibsp/io/CookieManager.h b/shibsp/io/CookieManager.h
index afc0de39..ee3f5012 100644
--- a/shibsp/io/CookieManager.h
+++ b/shibsp/io/CookieManager.h
@@ -144,6 +144,15 @@ namespace shibsp {
          */
         void setSameSite(samesite_t sameSiteValue);
 
+        /**
+         * Sets default SameSite attribute for cookies created by this object.
+         * 
+         * <p>Defaults to absent.</p>
+         * 
+         * @param sameSiteValue attribute value
+         */
+        void setSameSite(const char* sameSiteValue);
+        
         /**
          * Adds a cookie with the specified value to the outgoing response for the supplied request.
          * 
diff --git a/shibsp/io/impl/CookieManager.cpp b/shibsp/io/impl/CookieManager.cpp
index fb1eb078..fe378cea 100644
--- a/shibsp/io/impl/CookieManager.cpp
+++ b/shibsp/io/impl/CookieManager.cpp
@@ -31,6 +31,10 @@
 using namespace shibsp;
 using namespace std;
 
+#ifndef HAVE_STRCASECMP
+# define _stricmp strcasecmp
+#endif
+
 CookieManager::CookieManager(const char* defaultName)
     : m_defaultName(defaultName),
         m_overrideProperty(nullptr),
@@ -78,6 +82,27 @@ void CookieManager::setHttpOnly(bool httpOnly)
     m_httpOnly = httpOnly;
 }
 
+void CookieManager::setSameSite(const char* value)
+{
+    if (value) {
+        if (!strcasecmp(value, "None")) {
+            setSameSite(SAMESITE_NONE);
+        }
+        else if (!strcasecmp(value, "Lax")) {
+            setSameSite(SAMESITE_LAX);
+        }
+        else if (!strcasecmp(value, "Strict")) {
+            setSameSite(SAMESITE_STRICT);
+        }
+        else {
+            setSameSite(SAMESITE_ABSENT);
+        }
+    }
+    else {
+        setSameSite(SAMESITE_ABSENT);
+    }
+}
+
 void CookieManager::setSameSite(samesite_t value)
 {
     m_sameSiteValue = value;
diff --git a/shibsp/remoting/ddf.h b/shibsp/remoting/ddf.h
index ed1f25c8..aa49d73b 100644
--- a/shibsp/remoting/ddf.h
+++ b/shibsp/remoting/ddf.h
@@ -25,6 +25,7 @@
 
 #include <cstdio>
 #include <iostream>
+#include <string>
 
 namespace shibsp {
 
@@ -82,6 +83,7 @@ namespace shibsp {
         DDF& empty();
         DDF& string(char* val, bool copyit=true, bool safe=true);
         DDF& string(const char* val);
+        DDF& string(const std::string& val);
         DDF& unsafe_string(const char* val);
         DDF& string(long val);
         DDF& string(double val);
diff --git a/shibsp/remoting/impl/AbstractHTTPRemotingService.cpp b/shibsp/remoting/impl/AbstractHTTPRemotingService.cpp
index e97dd2ce..a9513de7 100644
--- a/shibsp/remoting/impl/AbstractHTTPRemotingService.cpp
+++ b/shibsp/remoting/impl/AbstractHTTPRemotingService.cpp
@@ -104,7 +104,6 @@ DDF AbstractHTTPRemotingService::send(const DDF& in) const
 {
     DDF output = AbstractRemotingService::send(in);
     if (!m_authCachingCookie.empty()) {
-        // TODO: Check for auth cache cookie value coming back and stash off using a write lock.
         const char* latestValue = output.getmember("cached_auth").string();
         if (latestValue) {
             m_authcachelock->lock_shared();
diff --git a/shibsp/remoting/impl/ddf.cpp b/shibsp/remoting/impl/ddf.cpp
index a8d44ad4..0e7fd5ba 100644
--- a/shibsp/remoting/impl/ddf.cpp
+++ b/shibsp/remoting/impl/ddf.cpp
@@ -393,6 +393,12 @@ DDF& DDF::string(const char* val)
     return string(const_cast<char*>(val), true);
 }
 
+DDF& DDF::string(const std::string& val)
+{
+    // Yes, this is horrendous, but we're copying the buffer so the cast shouldn't hurt anything.
+    return string(const_cast<char*>(val.c_str()), true);
+}
+
 DDF& DDF::unsafe_string(const char* val)
 {
     return string(const_cast<char*>(val), true, false);
diff --git a/shibsp/session/AbstractSessionCache.h b/shibsp/session/AbstractSessionCache.h
index 69917480..d0696519 100644
--- a/shibsp/session/AbstractSessionCache.h
+++ b/shibsp/session/AbstractSessionCache.h
@@ -24,6 +24,7 @@
 #include <logging/Category.h>
 #include <remoting/ddf.h>
 #include <session/SessionCache.h>
+#include <session/SessionCacheSPI.h>
 #include <util/BoostPropertySet.h>
 
 #include <condition_variable>
@@ -38,6 +39,7 @@ namespace shibsp {
 
     class SHIBSP_API AbstractSessionCache;
     class SHIBSP_API Attribute;
+    class SHIBSP_API CookieManager;
 
     class SHIBSP_API BasicSession : public virtual Session
     {
@@ -51,31 +53,26 @@ namespace shibsp {
 
         const char* getID() const;
         const char* getApplicationID() const;
-        const char* getClientAddress(const char* family) const;
-        void setClientAddress(const char* client_addr);
+        const char* getClientAddress() const;
         const std::map<std::string,DDF>& getAttributes() const;
         time_t getCreation() const;
         time_t getLastAccess() const;
 
-        void validate(const char* applicationId, const char* client_addr, time_t* timeout);
-
-        // Allows the cache to bind sessions to multiple client address
-        // families based on whatever this function returns.
-        static const char* getAddressFamily(const char* addr);
+        bool isValid(const char* applicationId, time_t lifetime, time_t timeout, const char* client_addr);
 
     private:
         DDF m_obj;
         std::map<std::string,DDF> m_attributes;
 
         AbstractSessionCache& m_cache;
-        time_t m_creation,m_lastAccess;
+        time_t m_creation,m_lastAccess,m_lastAccessReported;
         // TODO: possibly convert to a shared lock where possible?
         // I used exclusive because it avoided lock "upgrades"
         // when mutating or deleting sessions.
         std::mutex m_lock;
     };
 
-    class SHIBSP_API AbstractSessionCache : public virtual SessionCache, public virtual BoostPropertySet {
+    class SHIBSP_API AbstractSessionCache : public virtual SessionCache, public virtual SessionCacheSPI, public virtual BoostPropertySet {
         public:
             /**
              * Starts background cleanup thread for in-memory hashtable of sessions.
@@ -84,11 +81,12 @@ namespace shibsp {
              */
             bool start();
 
-            static const char CLEANUP_INTERVAL_PROP_NAME[];
-            static const char INPROC_TIMEOUT_PROP_NAME[];
-
-            static unsigned int CLEANUP_INTERVAL_PROP_DEFAULT;
-            static unsigned int INPROC_TIMEOUT_PROP_DEFAULT;
+            // SessiomCache API
+            std::string create(SPRequest& request, DDF session);
+            std::unique_lock<Session> find(SPRequest& request, bool checkTimeout, bool ignoreAddress);
+            std::unique_lock<Session> find(const char* applicationId, const char* key);
+            void remove(SPRequest& request, time_t revocationExp=0);
+            void remove(const char* applicationId, const char* key, time_t revocationExp=0);
 
         protected:
             /**
@@ -111,6 +109,10 @@ namespace shibsp {
         private:
             static void* cleanup_fn(void*);
             void dormant(const std::string& key);
+            // Wrapper for finding sessions via varied inputs.
+            std::unique_lock<Session> _find(
+                const char* applicationID, const char* key, time_t lifetime, time_t timeout, const char* client_addr
+                );
 
             Category& m_log;
 #if defined(HAVE_CXX17)
@@ -121,12 +123,11 @@ namespace shibsp {
             std::mutex m_lock;
 #endif
             std::map<std::string,std::unique_ptr<BasicSession>> m_hashtable;
-
-            bool m_shutdown;
+            std::unique_ptr<CookieManager> m_cookieManager;
             std::condition_variable m_shutdown_wait;
             std::thread m_cleanup_thread;
-                
-        friend class BasicSession;
+            std::string m_issuerAttribute;
+            bool m_shutdown;
         };    
 };
 
diff --git a/shibsp/session/SessionCache.h b/shibsp/session/SessionCache.h
index b6f01d83..d6c57429 100644
--- a/shibsp/session/SessionCache.h
+++ b/shibsp/session/SessionCache.h
@@ -111,58 +111,67 @@ namespace shibsp {
          */
         virtual bool start()=0;
 
-#ifndef SHIBSP_LITE
         /**
-         * Inserts a new session into the cache and binds the session to the outgoing
-         * client response.
+         * 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>
+         * 
+         * @param request request to bind the session to
+         * @param session session data obtained from the hub
+         * 
+         * @return the newly created session ID
+         */
+        virtual std::string create(SPRequest& request, DDF session)=0;
+
+        /**
+         * Locates an existing session bound to a request.
          *
-         * <p>The newly created session ID is placed into the first parameter.</p>
+         * <p>If a bound session is found to have expired, be invalid, etc., and if the request
+         * can be used to "clear" the session from subsequent client requests, then it may be cleared.</p>
          *
-         * <p>The SSO tokens and Attributes remain owned by the caller and are copied by the cache.</p>
+         * @param request       request from client
+         * @param checkTimeout  true iff the timeout policy should be enforced before returning session
+         * @param ignoreAddress true iff address checking should be ignored, regardless of request's policy
+         * 
+         * @return locked Session (or an unbound wrapper)
+         */
+        virtual std::unique_lock<Session> find(SPRequest& request, bool checkTimeout, bool ignoreAddress)=0;
+
+        /**
+         * Locates an existing session by its key/ID.
          *
-         * @param sessionID         reference to string to capture newly inserted session ID
-         * @param request           request that initiated session
-         * @param expires           expiration time of session
-         * @param issuer            issuing metadata of assertion issuer, if known
-         * @param protocol          protocol family used to initiate the session
-         * @param nameid            principal identifier, normalized to SAML 2, if any
-         * @param authn_instant     UTC timestamp of authentication at IdP, if known
-         * @param session_index     index of session between principal and IdP, if any
-         * @param authncontext_class    method/category of authentication event, if known
-         * @param authncontext_decl specifics of authentication event, if known
-         * @param tokens            assertions to cache with session, if any
-         * @param attributes        optional array of resolved Attributes to cache with session
+         * @param applicationID current application ID
+         * @param key           session key to locate
+         * 
+         * @return locked Session (or an unbound wrapper)
          */
-        virtual void insert(
-            std::string& sessionID,
-            const SPRequest& request,
-            time_t expires,
-            const opensaml::saml2md::EntityDescriptor* issuer=nullptr,
-            const XMLCh* protocol=nullptr,
-            const opensaml::saml2::NameID* nameid=nullptr,
-            const XMLCh* authn_instant=nullptr,
-            const XMLCh* session_index=nullptr,
-            const XMLCh* authncontext_class=nullptr,
-            const XMLCh* authncontext_decl=nullptr,
-            const std::vector<const opensaml::Assertion*>* tokens=nullptr,
-            const std::vector<Attribute*>* attributes=nullptr
-            )=0;
+        virtual std::unique_lock<Session> find(const char* applicationId, const char* key)=0;
 
         /**
-         * Determines whether the Session bound to a client request matches a set of input criteria.
+         * Removes an existing session bound to a request.
+         *
+         * <p>Revocation may be supported by some implementations.</p>
          *
-         * @param request       request in which to locate Session
-         * @param issuer        required source of session(s)
-         * @param nameid        required name identifier
-         * @param indexes       session indexes
-         * @return  true iff the Session exists and matches the input criteria
+         * @param request       request from client containing session
+         * @param revocationExp optional indicator for length of time to track revocation of this session
          */
-        virtual bool matches(
-            const SPRequest& request,
-            const opensaml::saml2md::EntityDescriptor* issuer,
-            const opensaml::saml2::NameID& nameid,
-            const std::set<std::string>* indexes
-            )=0;
+        virtual void remove(SPRequest& request, time_t revocationExp=0)=0;
+
+       /**
+        * Removes an existing session identified by its application and ID.
+        *
+        * <p>Revocation may be supported by some implementations.</p>
+        *
+        * @param bucketID      application associated with session
+        * @param key           session key/ID
+        * @param revocationExp optional indicator for length of time to track revocation of this session
+        */
+        virtual void remove(const char* applicationId, const char* key, time_t revocationExp=0)=0;
+
+#ifndef SHIBSP_LITE
+        // TODO: legacy API to be removed, keeping for reference during rewrite...
 
         /**
         * Returns active sessions that match particular parameters and records the logout
@@ -194,7 +203,6 @@ namespace shibsp {
          * Executes a test of the cache's general health.
          */
         virtual void test()=0;
-#endif
 
         /**
          * Returns the ID of the session bound to the specified client request, if possible.
@@ -204,32 +212,6 @@ namespace shibsp {
          */
         virtual std::string active(const SPRequest& request)=0;
 
-        /**
-         * Locates an existing session bound to a request.
-         *
-         * <p>If the client address is supplied, then a check will be performed against
-         * the address recorded in the record.</p>
-         *
-         * <p>If a bound session is found to have expired, be invalid, etc., and if the request
-         * can be used to "clear" the session from subsequent client requests, then it may be cleared.</p>
-         *
-         * @param request       request from client bound to session
-         * @param client_addr   network address of client (if known)
-         * @param timeout       inactivity timeout to enforce (0 for none, nullptr to bypass check/update of last access)
-         * @return  pointer to locked Session, or nullptr
-         */
-        virtual Session* find(SPRequest& request, const char* client_addr=nullptr, time_t* timeout=nullptr)=0;
-
-        /**
-         * Deletes an existing session bound to a request.
-         *
-         * <p>Revocation may be supported by some implementations.</p>
-         *
-         * @param request       request from client containing session
-         * @param revocationExp optional indicator for length of time to track revocation of this session
-         */
-        virtual void remove(SPRequest& request, time_t revocationExp=0)=0;
-
         /**
         * Locates an existing session by ID.
         *
@@ -238,17 +220,7 @@ namespace shibsp {
         * @return  pointer to locked Session, or nullptr
         */
         virtual Session* find(const char* bucketID, const char* key)=0;
-
-        /**
-        * Deletes an existing session.
-        *
-        * <p>Revocation may be supported by some implementations.</p>
-        *
-        * @param bucketID      bucket for session
-        * @param key           session key
-        * @param revocationExp optional indicator for length of time to track revocation of this session
-        */
-        virtual void remove(const char* bucketID, const char* key, time_t revocationExp=0)=0;
+#endif
     };
 
     /** SessionCache implementation backed by the file system. */
diff --git a/shibsp/session/SessionCacheSPI.h b/shibsp/session/SessionCacheSPI.h
index 3cbe54f4..d45ddd71 100644
--- a/shibsp/session/SessionCacheSPI.h
+++ b/shibsp/session/SessionCacheSPI.h
@@ -57,22 +57,26 @@ namespace shibsp {
          * Read a session record from the underlying storage medium and return its data.
          * 
          * <p>To the extent possible, the implementation SHOULD ensure that the underlying
-         * storage of the session reflects its use as of the time of this call such that
-         * subsequent calls to this method will be actioned based on a time of last use that
-         * is no older than the current time.</p>
+         * storage of the session (if returned) reflects its use as of the time of this call
+         * such that subsequent calls to this method will be actioned based on a time of last
+         * use that is no older than the current time.</p>
          * 
          * <p>The caller owns the resulting data object.</p>
          * 
-         * @param key session key/ID
-         * @param lifetime if positive, the time since its creation the session may be valid
-         * @param timeout if positive, a timeout duration to enforce against the estimated time of last use
+         * @param applicationId application ID
+         * @param key           session key/ID
+         * @param lifetime      if positive, the time since its creation the session may be valid
+         * @param timeout       if positive, a timeout duration to enforce against the estimated time of last use
+         * @param client_addr   if set, a client address to enforce for use of the session
          * 
          * @return reconstituted session data or a null object if the session was absent, expired, or inactive
          */
-        virtual DDF read(const char* key, time_t lifetime, time_t timeout) const=0;
+        virtual DDF read(
+            const char* applicationId, const char* key, time_t lifetime=0, time_t timeout=0, const char* client_addr=nullptr
+        ) const=0;
 
         /**
-         * Informs the storage medium that a session is active at the current point in time.
+         * Informs the storage medium that a session was used at the current point in time.
          * 
          * @param key session key/ID
          * 
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index 4edacffc..262dd4f7 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -21,8 +21,10 @@
 #include "internal.h"
 #include "exceptions.h"
 #include "AgentConfig.h"
-#include "session/AbstractSessionCache.h"
+#include "SPRequest.h"
+#include "io/CookieManager.h"
 #include "logging/Category.h"
+#include "session/AbstractSessionCache.h"
 
 #include <chrono>
 #include <boost/property_tree/ptree.hpp>
@@ -45,6 +47,26 @@ namespace shibsp {
     extern SessionCache* SHIBSP_DLLLOCAL StorageServiceSessionCacheFactory(ptree& pt, bool deprecationSupport);
 }
 
+static const char CLEANUP_INTERVAL_PROP_NAME[] = "cleanupInterval";
+static const char INPROC_TIMEOUT_PROP_NAME[] = "inprocTimeout";
+static const char ISSUER_ATTRIBUTE_PROP_NAME[] = "issuerAttribute";
+static const char COOKIE_NAME_PROP_NAME[] = "cookieName";
+static const char COOKIE_SECURE_PROP_NAME[] = "cookieSecure";
+static const char COOKIE_HTTPONLY_PROP_NAME[] = "cookieHttpOnly";
+static const char COOKIE_PATH_PROP_NAME[] = "cookiePath";
+static const char COOKIE_DOMAIN_PROP_NAME[] = "cookieDomain";
+static const char COOKIE_MAXAGE_PROP_NAME[] = "cookieMaxAge";
+static const char COOKIE_SAMESITE_PROP_NAME[] = "cookieSameSite";
+
+static unsigned int CLEANUP_INTERVAL_PROP_DEFAULT = 900;
+static unsigned int INPROC_TIMEOUT_PROP_DEFAULT = 900;
+static const char ISSUER_ATTRIBUTE_PROP_DEFAULT[] = "Shib-Identity-Provider";
+static const char COOKIE_NAME_PROP_DEFAULT[] = "__Host-shibsession";
+static bool COOKIE_SECURE_PROP_DEFAULT = true;
+static bool COOKIE_HTTPONLY_PROP_DEFAULT = true;
+static const char COOKIE_PATH_PROP_DEFAULT[] = "/";
+static int COOKIE_MAXAGE_PROP_DEFAULT = -1;
+
 void SHIBSP_API shibsp::registerSessionCaches()
 {
     AgentConfig::getConfig().SessionCacheManager.registerFactory(FILESYSTEM_SESSION_CACHE, FilesystemSessionCacheFactory);
@@ -67,15 +89,21 @@ SessionCache::~SessionCache()
 {
 }
 
-const char AbstractSessionCache::CLEANUP_INTERVAL_PROP_NAME[] = "cleanupInterval";
-const char AbstractSessionCache::INPROC_TIMEOUT_PROP_NAME[] = "inprocTimeout";
-
-unsigned int AbstractSessionCache::CLEANUP_INTERVAL_PROP_DEFAULT = 900;
-unsigned int AbstractSessionCache::INPROC_TIMEOUT_PROP_DEFAULT = 900;
-
 AbstractSessionCache::AbstractSessionCache(const ptree& pt) : m_log(Category::getInstance(SHIBSP_LOGCAT ".SessionCache"))
 {
     load(pt);
+
+    m_issuerAttribute = getString(ISSUER_ATTRIBUTE_PROP_NAME, ISSUER_ATTRIBUTE_PROP_DEFAULT);
+
+    // Set up cookie manager.
+    m_cookieManager.reset(new CookieManager(getString(COOKIE_NAME_PROP_NAME, COOKIE_NAME_PROP_DEFAULT)));
+    m_cookieManager->setCookieNamePolicy("sessionCookieName", true);
+    m_cookieManager->setSecure(getBool(COOKIE_SECURE_PROP_NAME, COOKIE_SECURE_PROP_DEFAULT));
+    m_cookieManager->setHttpOnly(getBool(COOKIE_HTTPONLY_PROP_NAME, COOKIE_HTTPONLY_PROP_DEFAULT));
+    m_cookieManager->setPath(getString(COOKIE_PATH_PROP_NAME, COOKIE_PATH_PROP_DEFAULT));
+    m_cookieManager->setMaxAge(getInt(COOKIE_MAXAGE_PROP_NAME, COOKIE_MAXAGE_PROP_DEFAULT));
+    m_cookieManager->setDomain(getString(COOKIE_DOMAIN_PROP_NAME));
+    m_cookieManager->setSameSite(getString(COOKIE_SAMESITE_PROP_NAME));
 }
 
 AbstractSessionCache::~AbstractSessionCache()
@@ -105,6 +133,161 @@ bool AbstractSessionCache::start()
     return false;
 }
 
+string AbstractSessionCache::create(SPRequest& request, DDF session)
+{
+    m_log.debug("creating new session");
+
+    // Isolate from parent.
+    session.remove();
+
+    // Add additional fields managed by agent.
+    // attributes and data members should be present from hub.
+    session.addmember("creation").longinteger(time(nullptr));
+    session.addmember("app_id").string(request.getRequestSettings().first->getString("applicationId"));
+    session.addmember("addr").string(request.getRemoteAddr());
+
+    // Write the data to the back-end, obtaining a key.
+    string key;
+    try {
+        m_log.debug("writing new session to persistent store");
+        key = SessionCacheSPI::create(session);
+    }
+    catch (const IOException& ex) {
+        m_log.error("IOException writing new session to persistent store: %s", ex.what());
+        session.destroy();
+        return string();
+    }
+
+    unique_ptr<BasicSession> sessionObject(new BasicSession(*this, session));
+
+    const char* issuer = nullptr;
+    const auto& attr = sessionObject->getAttributes().find(m_issuerAttribute);
+    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());
+
+    // Drop a cookie with the session ID.
+    m_cookieManager->setCookie(request, key.c_str());
+
+    // Lock the cache and insert the new session.
+
+    // Note, the C23 standard includes a typeof operator, but until then...
+#if defined(HAVE_CXX17)
+    lock_guard<shared_mutex> locker(m_lock);
+#elif defined(HAVE_CXX14)
+    lock_guard<shared_timed_mutex> locker(m_lock);
+#else
+    lock_guard<mutex> locker(m_lock);
+#endif
+
+    m_hashtable[key] = std::move_if_noexcept(sessionObject);
+
+    return key;
+}
+
+unique_lock<Session> AbstractSessionCache::find(SPRequest& request, bool checkTimeout, bool ignoreAddress)
+{
+    return unique_lock<Session>();
+}
+
+unique_lock<Session> AbstractSessionCache::find(const char* applicationId, const char* key)
+{
+    return _find(applicationId, key, 0, 0, nullptr);
+}
+
+unique_lock<Session> AbstractSessionCache::_find(
+    const char* applicationId, const char* key, time_t lifetime, time_t timeout, const char* client_addr
+    )
+{
+
+    m_log.debug("searching local cache for session (%s)", key);
+#if defined(HAVE_CXX17)
+    shared_lock<shared_mutex> readlocker(m_lock);
+#elif defined(HAVE_CXX14)
+    shared_lock<shared_timed_mutex> readlocker(m_lock);
+#else
+    unique_lock<mutex> readlocker(m_lock);
+#endif
+    const auto& i = m_hashtable.find(key);
+    if (i != m_hashtable.end()) {
+        // Save off and lock the session.
+        unique_lock<Session> session(*(i->second));
+        readlocker.unlock();
+        m_log.debug("session found locally, validating it for use");
+        if (!dynamic_cast<BasicSession*>(session.mutex())->isValid(applicationId, lifetime, timeout, client_addr)) {
+            session.unlock();
+            m_log.debug("session (%s) was found but was invalid, removing it", key);
+            remove(applicationId, key);
+        }
+        return session;
+    }
+    else {
+        readlocker.unlock();
+    }
+
+    DDF obj;
+    try {
+        // Note this performs the relevant enforcement for us.
+        obj = read(applicationId, key, lifetime, timeout, client_addr);
+    }
+    catch (const exception& ex) {
+        m_log.error("error reading session (%s) from persistent store: %s", key, ex.what());
+        return unique_lock<Session>();
+    }
+
+    if (obj.isnull()) {
+        m_log.info("session (%s) not found in persistent store", key);
+        return unique_lock<Session>();
+    }
+
+    m_log.info("valid session (%s) loaded from persistent store");
+
+    // Wrap the object in a local wraper to guard it before it's saved off.
+    unique_ptr<BasicSession> newSession(new BasicSession(*this, obj));
+
+    // Lock the cache and check for a race condition with another thread...
+
+    // Note, the C23 standard includes a typeof operator, but until then...
+#if defined(HAVE_CXX17)
+    lock_guard<shared_mutex> locker(m_lock);
+#elif defined(HAVE_CXX14)
+    lock_guard<shared_timed_mutex> locker(m_lock);
+#else
+    lock_guard<mutex> locker(m_lock);
+#endif
+
+    if (m_hashtable.count(key)) {
+        // There was an existing entry, but we want to swap the "newest" copy in place of it.
+        // Since we're holding the cache write lock, we know nobody can have a lock on the
+        // new copy yet, but the old copy might be locked by somebody. However, once we acquire
+        // a lock on the old Session, we know nobody else is waiting for that lock because they
+        // would have to be inside the cache critical section to get to it.
+        // This, this sequence transfers ownership out of the table, removes the entry, then
+        // locks, unlocks, and finally deletes the old session object.
+        m_log.debug("session (%s) already inserted by another thread, replacing with our copy", key);
+        unique_ptr<BasicSession> oldSession;
+        oldSession.swap(m_hashtable[key]);
+        m_hashtable.erase(key);
+        lock_guard<BasicSession> oldSessionLock(*oldSession.get());
+    }
+
+    // Finally, get an "empty" smart pointer out of the table to "insert" the new entry,
+    // swap over ownership from our copy, and finally return a locked wrapper around it.
+    unique_ptr<BasicSession>& ref = m_hashtable[key];
+    ref.swap(newSession);
+    return unique_lock<Session>(*ref);
+}
+
+void AbstractSessionCache::remove(SPRequest& request, time_t revocationExp)
+{
+}
+
+void AbstractSessionCache::remove(const char* applicationId, const char* key, time_t revocationExp)
+{
+}
+
 void AbstractSessionCache::dormant(const string& key)
 {
     m_log.debug("deleting local copy of session (%s)", key.c_str());
@@ -209,12 +392,15 @@ void* AbstractSessionCache::cleanup_fn(void* p)
 }
 
 BasicSession::BasicSession(AbstractSessionCache& cache, DDF& obj)
-    : m_obj(obj), m_cache(cache), m_creation(0), m_lastAccess(time(nullptr))
+    : m_obj(obj), m_cache(cache), m_creation(0), m_lastAccess(time(nullptr)), m_lastAccessReported(m_lastAccess)
 {
     m_creation = m_obj["creation"].longinteger();
 
-    // We have to index and vector-up the values of the attributes.
-    // (Among other reasons, the DDF iteration API isn't threadsafe.)
+    // This is safe to directly expose for iteration of the attributes
+    // as long as we maintain the mutex-based lock approach for exclusive
+    // Session access. If we made that a shared lock, this all has to change.
+
+    // We do have to index the attributes.
     DDF attrs = m_obj["attributes"];
     DDF attr = attrs.first();
     while (!attr.isnull()) {
@@ -235,21 +421,12 @@ const char* BasicSession::getID() const
 
 const char* BasicSession::getApplicationID() const
 {
-    return m_obj["applicaton_id"].string();
+    return m_obj["app_id"].string();
 }
 
-const char* BasicSession::getClientAddress(const char* family) const
+const char* BasicSession::getClientAddress() const
 {
-    return m_obj["client_addr"][family].string();
-}
-
-void BasicSession::setClientAddress(const char* client_addr)
-{
-    DDF obj = m_obj["client_addr"];
-    if (!obj.isstruct()) {
-        obj = m_obj.addmember("client_addr").structure();
-    }
-    obj.addmember(getAddressFamily(client_addr)).string(client_addr);
+    return m_obj["addr"].string();
 }
 
 const std::map<std::string,DDF>& BasicSession::getAttributes() const
@@ -257,8 +434,9 @@ const std::map<std::string,DDF>& BasicSession::getAttributes() const
     return m_attributes;
 }
 
-void BasicSession::validate(const char* applicationId, const char* client_addr, time_t* timeout)
+bool BasicSession::isValid(const char* applicationId, time_t lifetime, time_t timeout, const char* client_addr)
 {
+    return false;
 }
 
 time_t BasicSession::getCreation() const
@@ -286,16 +464,6 @@ void BasicSession::unlock()
     m_lock.unlock();
 }
 
-// Allows the cache to bind sessions to multiple client address
-// families based on whatever this function returns.
-const char* BasicSession::getAddressFamily(const char* addr)
-{
-    if (strchr(addr, ':'))
-        return "6";
-    else
-        return "4";
-}
-
 /*
 
 void BasicSession::validate(const char* applicationId, const char* client_addr, time_t* timeout)
diff --git a/shibsp/session/impl/FilesystemSessionCache.cpp b/shibsp/session/impl/FilesystemSessionCache.cpp
index d8efd00a..a4fccf0a 100644
--- a/shibsp/session/impl/FilesystemSessionCache.cpp
+++ b/shibsp/session/impl/FilesystemSessionCache.cpp
@@ -37,20 +37,23 @@ namespace {
 
         // For now this is just a dummy implementation to support further development.
 
-        string active(const SPRequest& request) {return "";}
-        Session* find(SPRequest& request, const char* client_addr=nullptr, time_t* timeout=nullptr) {return nullptr;}
-        void remove(SPRequest& request, time_t revocationExp=0) {}
-        Session* find(const char* bucketID, const char* key) {return nullptr;}
-        void remove(const char* bucketID, const char* key, time_t revocationExp=0) {}
     };
 };
 
 namespace shibsp {
     SessionCache* SHIBSP_DLLLOCAL FilesystemSessionCacheFactory(ptree& pt, bool deprecationSupport) {
-        return new FilesystemSessionCache(pt);
+        //return new FilesystemSessionCache(pt);
     }
 }
 
+SessionCacheSPI::SessionCacheSPI()
+{
+}
+
+SessionCacheSPI::~SessionCacheSPI()
+{
+}
+
 FilesystemSessionCache::FilesystemSessionCache(const ptree& pt) : AbstractSessionCache(pt)
 {
 }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list