[cpp-sp] branch main updated: Continue fleshing out session cache base class.

Scott Cantor cantor.2 at osu.edu
Mon Jun 2 20:29:05 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=3ff483cecd358486c67697b16a76b562ad3dac9b

The following commit(s) were added to refs/heads/main by this push:
     new 3ff483ce Continue fleshing out session cache base class.
3ff483ce is described below

commit 3ff483cecd358486c67697b16a76b562ad3dac9b
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Jun 2 16:27:20 2025 -0400

    Continue fleshing out session cache base class.
---
 Projects/vc22/shibsp.vcxproj                   |  1 +
 Projects/vc22/shibsp.vcxproj.filters           |  3 +
 shibsp/Makefile.am                             |  1 +
 shibsp/session/AbstractSessionCache.h          |  5 +-
 shibsp/session/SessionCache.h                  |  3 +
 shibsp/session/SessionCacheSPI.h               |  8 ++-
 shibsp/session/impl/AbstractSessionCache.cpp   | 99 ++++++++++++++++++++++----
 shibsp/session/impl/FilesystemSessionCache.cpp | 40 ++++++++---
 shibsp/session/impl/MemorySessionCache.cpp     | 92 ++++++++++++++++++++++++
 9 files changed, 226 insertions(+), 26 deletions(-)

diff --git a/Projects/vc22/shibsp.vcxproj b/Projects/vc22/shibsp.vcxproj
index 1d935686..2bcab0c8 100644
--- a/Projects/vc22/shibsp.vcxproj
+++ b/Projects/vc22/shibsp.vcxproj
@@ -184,6 +184,7 @@
     <ClCompile Include="..\..\shibsp\remoting\impl\WinHTTPRemotingService.cpp" />
     <ClCompile Include="..\..\shibsp\session\impl\AbstractSessionCache.cpp" />
     <ClCompile Include="..\..\shibsp\session\impl\FilesystemSessionCache.cpp" />
+    <ClCompile Include="..\..\shibsp\session\impl\MemorySessionCache.cpp" />
     <ClCompile Include="..\..\shibsp\util\BoostPropertySet.cpp" />
     <ClCompile Include="..\..\shibsp\util\CGIParser.cpp" />
     <ClCompile Include="..\..\shibsp\util\IPRange.cpp" />
diff --git a/Projects/vc22/shibsp.vcxproj.filters b/Projects/vc22/shibsp.vcxproj.filters
index a3cd1ae9..cb83b5d9 100644
--- a/Projects/vc22/shibsp.vcxproj.filters
+++ b/Projects/vc22/shibsp.vcxproj.filters
@@ -374,6 +374,9 @@
     <ClCompile Include="..\..\shibsp\session\impl\FilesystemSessionCache.cpp">
       <Filter>Source Files\Session</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\shibsp\session\impl\MemorySessionCache.cpp">
+      <Filter>Source Files\Session</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\shibsp\remoting\impl\CurlHTTPRemotingService.cpp">
       <Filter>Source Files\Remoting</Filter>
     </ClCompile>
diff --git a/shibsp/Makefile.am b/shibsp/Makefile.am
index c94599bb..15fb425d 100644
--- a/shibsp/Makefile.am
+++ b/shibsp/Makefile.am
@@ -138,6 +138,7 @@ libshibsp_la_SOURCES = \
 	remoting/impl/CurlHTTPRemotingService.cpp \
 	session/impl/AbstractSessionCache.cpp \
 	session/impl/FilesystemSessionCache.cpp \
+	session/impl/MemorySessionCache.cpp \
 	util/BoostPropertySet.cpp \
 	util/CGIParser.cpp \
 	util/IPRange.cpp \
diff --git a/shibsp/session/AbstractSessionCache.h b/shibsp/session/AbstractSessionCache.h
index 55358bd2..d7895bf3 100644
--- a/shibsp/session/AbstractSessionCache.h
+++ b/shibsp/session/AbstractSessionCache.h
@@ -62,8 +62,8 @@ namespace shibsp {
         const std::map<std::string,DDF>& getAttributes() const;
         time_t getCreation() const;
         time_t getLastAccess() const;
-        void setLastAccess(time_t ts);
 
+        // Perform validation of a local session based on policy and checks for revocation.
         bool isValid(const char* applicationId, unsigned int lifetime, unsigned int timeout, const char* client_addr);
 
     private:
@@ -134,6 +134,9 @@ namespace shibsp {
             std::thread m_cleanup_thread;
             std::string m_issuerAttribute;
             bool m_shutdown;
+            unsigned int m_storageAccessInterval;
+            
+            friend class BasicSession;
         };
 
 #if defined (_MSC_VER)
diff --git a/shibsp/session/SessionCache.h b/shibsp/session/SessionCache.h
index c7cd1494..fc51b242 100644
--- a/shibsp/session/SessionCache.h
+++ b/shibsp/session/SessionCache.h
@@ -171,6 +171,9 @@ namespace shibsp {
     /** SessionCache implementation backed by a hub-hosted StorageService. */
     #define STORAGESERVICE_SESSION_CACHE    "storage"
 
+    /** SessionCache implementation backed by single process memory, generally only for testing. */
+    #define MEMORY_SESSION_CACHE    "memory"
+
     /**
      * Registers SessionCache classes into the runtime.
      */
diff --git a/shibsp/session/SessionCacheSPI.h b/shibsp/session/SessionCacheSPI.h
index 07f33905..d9a7484c 100644
--- a/shibsp/session/SessionCacheSPI.h
+++ b/shibsp/session/SessionCacheSPI.h
@@ -86,11 +86,15 @@ namespace shibsp {
         /**
          * Informs the storage medium that a session was used at the current point in time.
          * 
+         * <p>This method should return false to indicate that a session has been revoked, removed,
+         * or is no longer valid.</p>
+         * 
          * @param key session key/ID
+         * @param timeout timeout to enforce if non-zero
          * 
-         * @return true iff the storage medium believes the information has been updated
+         * @return true iff the session remains valid/available
          */
-        virtual bool cache_touch(const char* key) const=0;
+        virtual bool cache_touch(const char* key, unsigned int timeout=0) const=0;
 
         /**
          * Delete a session record from the underlying storage medium.
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index f318a770..5cd667c7 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -25,8 +25,8 @@
 #include "io/CookieManager.h"
 #include "logging/Category.h"
 #include "session/AbstractSessionCache.h"
+#include "util/Date.h"
 
-#include <chrono>
 #include <boost/property_tree/ptree.hpp>
 
 #ifndef WIN32
@@ -45,9 +45,11 @@ using namespace std;
 namespace shibsp {
     extern SessionCache* SHIBSP_DLLLOCAL FilesystemSessionCacheFactory(ptree& pt, bool deprecationSupport);
     extern SessionCache* SHIBSP_DLLLOCAL StorageServiceSessionCacheFactory(ptree& pt, bool deprecationSupport);
+    extern SessionCache* SHIBSP_DLLLOCAL MemorySessionCacheFactory(ptree& pt, bool deprecationSupport);
 }
 
 static const char CLEANUP_INTERVAL_PROP_NAME[] = "cleanupInterval";
+static const char STORAGE_ACCESS_INTERVAL_PROP_NAME[] = "storageAccessInterval";
 static const char INPROC_TIMEOUT_PROP_NAME[] = "inprocTimeout";
 static const char ISSUER_ATTRIBUTE_PROP_NAME[] = "issuerAttribute";
 static const char COOKIE_NAME_PROP_NAME[] = "cookieName";
@@ -59,6 +61,7 @@ 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 STORAGE_ACCESS_INTERVAL_PROP_DEFAULT = 600;
 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";
@@ -70,6 +73,7 @@ static int COOKIE_MAXAGE_PROP_DEFAULT = -1;
 void SHIBSP_API shibsp::registerSessionCaches()
 {
     AgentConfig::getConfig().SessionCacheManager.registerFactory(FILESYSTEM_SESSION_CACHE, FilesystemSessionCacheFactory);
+    AgentConfig::getConfig().SessionCacheManager.registerFactory(MEMORY_SESSION_CACHE, MemorySessionCacheFactory);
     //AgentConfig::getConfig().SessionCacheManager.registerFactory(STORAGESERVICE_SESSION_CACHE, StorageServiceSessionCacheFactory);
 }
 
@@ -89,11 +93,20 @@ SessionCache::~SessionCache()
 {
 }
 
+SessionCacheSPI::SessionCacheSPI()
+{
+}
+
+SessionCacheSPI::~SessionCacheSPI()
+{
+}
+
 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);
+    m_storageAccessInterval = getUnsignedInt(STORAGE_ACCESS_INTERVAL_PROP_NAME, STORAGE_ACCESS_INTERVAL_PROP_DEFAULT);
 
     // Set up cookie manager.
     m_cookieManager.reset(new CookieManager(getString(COOKIE_NAME_PROP_NAME, COOKIE_NAME_PROP_DEFAULT)));
@@ -218,8 +231,6 @@ unique_lock<Session> AbstractSessionCache::find(SPRequest& request, bool checkTi
         m_cookieManager->unsetCookie(request);
     }
 
-    // Update last access (if this was just loaded from storage, this is a no-op.
-    dynamic_cast<BasicSession*>(session.mutex())->setLastAccess(time(nullptr));
     return session;
 }
 
@@ -247,16 +258,29 @@ unique_lock<Session> AbstractSessionCache::_find(
         // 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)) {
+
+        m_log.debug("session (%s) found locally, validating for use", key);
+
+        // If timeout is being enforced, check if the session is "stale" from the local cache's perspective.
+        if (timeout && session.mutex()->getLastAccess() + timeout < time(nullptr)) {
+            // Unlock the local copy and drop into the reload logic.
             session.unlock();
-            m_log.debug("session (%s) was found but was invalid, removing it", key);
+            m_log.debug("session (%s) found locally but stale, attempting reload from persistent store", key);
+        }
+        else if (!dynamic_cast<BasicSession*>(session.mutex())->isValid(applicationId, lifetime, timeout, client_addr)) {
+            // Locally invalid on its face, so remove and return nothing.
+            session.unlock();
+            m_log.debug("session (%s) invalid, removing it", key);
             remove(key);
+            return session;
+        } else {
+            // Just return the local copy.
+            return session;
         }
-        return session;
     }
     else {
         readlocker.unlock();
+        m_log.debug("session (%s) not found locally, loading from persistent store", key);
     }
 
     DDF obj;
@@ -298,7 +322,7 @@ unique_lock<Session> AbstractSessionCache::_find(
         // would have to be inside the cache critical section to get to it.
         // Thus, 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);
+        m_log.debug("replacing session (%s) with fresh copy", key);
         unique_ptr<BasicSession> oldSession;
         oldSession.swap(m_hashtable[key]);
         m_hashtable.erase(key);
@@ -477,7 +501,58 @@ const std::map<std::string,DDF>& BasicSession::getAttributes() const
 
 bool BasicSession::isValid(const char* applicationId, unsigned int lifetime, unsigned int timeout, const char* client_addr)
 {
-    return false;
+    // Check client address.
+    // TODO: Implement the fuzzy address matching.
+    if (client_addr && strcmp(client_addr, getClientAddress())) {
+        m_cache.log().warn("session (%s) invalid, bound to address (%s), accessed from (%s)", getID(), getClientAddress(), client_addr);
+        return false;
+    }
+
+    time_t now = time(nullptr);
+
+    // Enforce session lifetime.
+    if (lifetime) {
+        if (getCreation() + lifetime < now) {
+            if (m_cache.log().isWarnEnabled()) {
+                string created(date::format("%FT%TZ", chrono::system_clock::from_time_t(getCreation())));
+                string expired(date::format("%FT%TZ", chrono::system_clock::from_time_t(getCreation() + lifetime)));
+                m_cache.log().warn("session (%s) has expired, created (%s), expired (%s)", getID(), created.c_str(), expired.c_str());
+            }
+            return false;
+        }
+    }
+
+    if (!timeout || m_lastAccess + timeout > now) {
+
+        // Being locally valid, we want to update the activity timestamp remotely and in the persistent store.
+        // This check also notices a session having been revoked, so implements the concept of the cache being
+        // "eventually consistent" across agent processes.
+
+        if (m_lastAccess - m_lastAccessReported > m_cache.m_storageAccessInterval) {
+            // It's been X seconds since we last wrote through to storage...
+            if (!m_cache.cache_touch(getID(), timeout)) {
+                m_cache.log().warn("session (%) missing or invalid in persistent store, invalidating locally", getID());
+                return false;
+            }
+            // Update reporting timestamp.
+            m_lastAccessReported = now;
+        }
+    }
+    else {
+        // The session is locally invalid due to inactivity, but this isn't "truth" because other agent processes may
+        // actively be using it.
+        if (!m_cache.cache_touch(getID(), timeout)) {
+            m_cache.log().warn("session (%s) timed out due to inactivity", getID());
+            return false;
+        }
+        // Update reporting timestamp.
+        m_lastAccessReported = now;
+    }
+
+    // Update last access time locally.
+    m_lastAccess = now;
+
+    return true;
 }
 
 time_t BasicSession::getCreation() const
@@ -490,12 +565,6 @@ time_t BasicSession::getLastAccess() const
     return m_lastAccess;
 }
 
-void BasicSession::setLastAccess(time_t ts)
-{
-    m_lastAccess = ts;
-    // TODO: interval-driven touch method call
-}
-
 void BasicSession::lock()
 {
     m_lock.lock();
diff --git a/shibsp/session/impl/FilesystemSessionCache.cpp b/shibsp/session/impl/FilesystemSessionCache.cpp
index 259653e2..edd7183e 100644
--- a/shibsp/session/impl/FilesystemSessionCache.cpp
+++ b/shibsp/session/impl/FilesystemSessionCache.cpp
@@ -35,30 +35,54 @@ namespace {
         FilesystemSessionCache(const ptree& pt);
         ~FilesystemSessionCache();
 
-        // For now this is just a dummy implementation to support further development.
-
+        string cache_create(DDF& sessionData);
+        DDF cache_read(
+            const char* applicationId,
+            const char* key,
+            unsigned int lifetime=0,
+            unsigned int timeout=0,
+            const char* client_addr=nullptr
+            ) const;
+        bool cache_touch(const char* key, unsigned int timeout=0) const;
+        void cache_remove(const char* key);
     };
 };
 
 namespace shibsp {
     SessionCache* SHIBSP_DLLLOCAL FilesystemSessionCacheFactory(ptree& pt, bool deprecationSupport) {
-        //return new FilesystemSessionCache(pt);
-        return nullptr;
+        return new FilesystemSessionCache(pt);
     }
 }
 
-SessionCacheSPI::SessionCacheSPI()
+FilesystemSessionCache::FilesystemSessionCache(const ptree& pt) : AbstractSessionCache(pt)
 {
 }
 
-SessionCacheSPI::~SessionCacheSPI()
+FilesystemSessionCache::~FilesystemSessionCache()
 {
 }
 
-FilesystemSessionCache::FilesystemSessionCache(const ptree& pt) : AbstractSessionCache(pt)
+string FilesystemSessionCache::cache_create(DDF& sessionData)
 {
+    return string();
 }
 
-FilesystemSessionCache::~FilesystemSessionCache()
+DDF FilesystemSessionCache::cache_read(
+    const char* applicationId,
+    const char* key,
+    unsigned int lifetime,
+    unsigned int timeout,
+    const char* client_addr
+    ) const
+{
+    return DDF();
+}
+
+bool FilesystemSessionCache::cache_touch(const char* key, unsigned int timeout) const
+{
+    return false;
+}
+
+void FilesystemSessionCache::cache_remove(const char* key)
 {
 }
diff --git a/shibsp/session/impl/MemorySessionCache.cpp b/shibsp/session/impl/MemorySessionCache.cpp
new file mode 100644
index 00000000..2d33605f
--- /dev/null
+++ b/shibsp/session/impl/MemorySessionCache.cpp
@@ -0,0 +1,92 @@
+/**
+ * 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/MemorySessionCache.cpp
+ *
+ * SessionCache implementation using non-shared memory.
+ * 
+ * <p>This is a more or less degenerate implementation of the SessionCacheSPI interface
+ * for testing and perhaps very constrained use cases in which only a single agent process
+ * can exist.</p>
+ */
+
+#include "internal.h"
+#include "exceptions.h"
+#include "session/AbstractSessionCache.h"
+#include "logging/Category.h"
+
+#include <boost/property_tree/ptree.hpp>
+
+using namespace shibsp;
+using namespace boost::property_tree;
+using namespace std;
+
+namespace {
+    class MemorySessionCache : public virtual AbstractSessionCache {
+    public:
+        MemorySessionCache(const ptree& pt);
+        ~MemorySessionCache();
+
+        string cache_create(DDF& sessionData);
+        DDF cache_read(
+            const char* applicationId,
+            const char* key,
+            unsigned int lifetime=0,
+            unsigned int timeout=0,
+            const char* client_addr=nullptr
+            ) const;
+        bool cache_touch(const char* key, unsigned int timeout=0) const;
+        void cache_remove(const char* key);
+    };
+};
+
+namespace shibsp {
+    SessionCache* SHIBSP_DLLLOCAL MemorySessionCacheFactory(ptree& pt, bool deprecationSupport) {
+        return new MemorySessionCache(pt);
+    }
+}
+
+MemorySessionCache::MemorySessionCache(const ptree& pt) : AbstractSessionCache(pt)
+{
+}
+
+MemorySessionCache::~MemorySessionCache()
+{
+}
+
+string MemorySessionCache::cache_create(DDF& sessionData)
+{
+    return string();
+}
+
+DDF MemorySessionCache::cache_read(
+    const char* applicationId,
+    const char* key,
+    unsigned int lifetime,
+    unsigned int timeout,
+    const char* client_addr
+    ) const
+{
+    return DDF();
+}
+
+bool MemorySessionCache::cache_touch(const char* key, unsigned int timeout) const
+{
+    return false;
+}
+
+void MemorySessionCache::cache_remove(const char* key)
+{
+}

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


More information about the commits mailing list