[cpp-sp] branch main updated: Some cleanup.

Scott Cantor cantor.2 at osu.edu
Wed May 21 17:25:14 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=2fdaf4987f51926d7326974041aa2c74dc1454ab

The following commit(s) were added to refs/heads/main by this push:
     new 2fdaf498 Some cleanup.
2fdaf498 is described below

commit 2fdaf4987f51926d7326974041aa2c74dc1454ab
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed May 21 13:25:10 2025 -0400

    Some cleanup.
---
 shibsp/RequestMapper.h                       | 10 ++++++
 shibsp/impl/XMLRequestMapper.cpp             | 12 ++++++-
 shibsp/session/AbstractSessionCache.h        |  7 +++--
 shibsp/session/SessionCache.h                |  4 +--
 shibsp/session/SessionCacheSPI.h             | 16 +++++++---
 shibsp/session/impl/AbstractSessionCache.cpp | 47 +++++++++++++++++++++++++---
 6 files changed, 82 insertions(+), 14 deletions(-)

diff --git a/shibsp/RequestMapper.h b/shibsp/RequestMapper.h
index 6ffef204..b23baf99 100644
--- a/shibsp/RequestMapper.h
+++ b/shibsp/RequestMapper.h
@@ -52,6 +52,16 @@ namespace shibsp {
         /** Combination of configuration settings and effective access control. */
         typedef std::pair<const PropertySet*,AccessControl*> Settings;
 
+        static const char APPLICATION_ID_PROP_NAME[];
+        static const char LIFETIME_PROP_NAME[];
+        static const char TIMEOUT_PROP_NAME[];
+        static const char CONSISTENT_ADDRESS_PROP_NAME[];
+
+        static const char APPLICATION_ID_PROP_DEFAULT[];
+        static unsigned int LIFETIME_PROP_DEFAULT;
+        static unsigned int TIMEOUT_PROP_DEFAULT;
+        static bool CONSISTENT_ADDRESS_PROP_DEFAULT;
+
         /**
          * Map request to settings.
          * 
diff --git a/shibsp/impl/XMLRequestMapper.cpp b/shibsp/impl/XMLRequestMapper.cpp
index f05ebc54..c93a7fab 100644
--- a/shibsp/impl/XMLRequestMapper.cpp
+++ b/shibsp/impl/XMLRequestMapper.cpp
@@ -155,6 +155,16 @@ void SHIBSP_API shibsp::registerRequestMappers()
     conf.RequestMapperManager.registerFactory(NATIVE_REQUEST_MAPPER, XMLRequestMapperFactory);
 }
 
+const char RequestMapper::APPLICATION_ID_PROP_NAME[] = "applicationId";
+const char RequestMapper::LIFETIME_PROP_NAME[] = "lifetime";
+const char RequestMapper::TIMEOUT_PROP_NAME[] = "timeout";
+const char RequestMapper::CONSISTENT_ADDRESS_PROP_NAME[] = "consistentAddress";
+
+const char RequestMapper::APPLICATION_ID_PROP_DEFAULT[] = "default";
+unsigned int RequestMapper::LIFETIME_PROP_DEFAULT = 3600 * 8;
+unsigned int RequestMapper::TIMEOUT_PROP_DEFAULT = 3600;
+bool RequestMapper::CONSISTENT_ADDRESS_PROP_DEFAULT = true;
+
 RequestMapper::RequestMapper()
 {
 }
@@ -490,7 +500,7 @@ XMLRequestMapperImpl::XMLRequestMapperImpl(ptree& pt, Category& log)
     // Inject "default" app ID if not explicit.
     const boost::optional<string> appId = pt.get_optional<string>(APPLICATION_ID_PROP_PATH);
     if (!appId) {
-        pt.put(APPLICATION_ID_PROP_PATH, "default");
+        pt.put(APPLICATION_ID_PROP_PATH, RequestMapper::APPLICATION_ID_PROP_DEFAULT);
     }
 
     // Load the property set.
diff --git a/shibsp/session/AbstractSessionCache.h b/shibsp/session/AbstractSessionCache.h
index d0696519..702a5571 100644
--- a/shibsp/session/AbstractSessionCache.h
+++ b/shibsp/session/AbstractSessionCache.h
@@ -57,8 +57,9 @@ namespace shibsp {
         const std::map<std::string,DDF>& getAttributes() const;
         time_t getCreation() const;
         time_t getLastAccess() const;
+        void setLastAccess(time_t ts);
 
-        bool isValid(const char* applicationId, time_t lifetime, time_t timeout, const char* client_addr);
+        bool isValid(const char* applicationId, unsigned int lifetime, unsigned int timeout, const char* client_addr);
 
     private:
         DDF m_obj;
@@ -82,7 +83,7 @@ namespace shibsp {
             bool start();
 
             // SessiomCache API
-            std::string create(SPRequest& request, DDF session);
+            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);
@@ -111,7 +112,7 @@ namespace shibsp {
             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
+                const char* applicationID, const char* key, unsigned int lifetime, unsigned int timeout, const char* client_addr
                 );
 
             Category& m_log;
diff --git a/shibsp/session/SessionCache.h b/shibsp/session/SessionCache.h
index d6c57429..42270fda 100644
--- a/shibsp/session/SessionCache.h
+++ b/shibsp/session/SessionCache.h
@@ -102,7 +102,7 @@ namespace shibsp {
         SessionCache();
     public:
         virtual ~SessionCache();
-
+        
         /**
          * Signals the implementation it may start any background tasks or do any
          * additional once-per-process work.
@@ -123,7 +123,7 @@ namespace shibsp {
          * 
          * @return the newly created session ID
          */
-        virtual std::string create(SPRequest& request, DDF session)=0;
+        virtual std::string create(SPRequest& request, DDF& session)=0;
 
         /**
          * Locates an existing session bound to a request.
diff --git a/shibsp/session/SessionCacheSPI.h b/shibsp/session/SessionCacheSPI.h
index d45ddd71..77a5ac21 100644
--- a/shibsp/session/SessionCacheSPI.h
+++ b/shibsp/session/SessionCacheSPI.h
@@ -51,11 +51,15 @@ namespace shibsp {
          * 
          * @return session key/ID created, this MUST be URL-safe
          */
-        virtual std::string create(DDF sessionData)=0;
+        virtual std::string create(DDF& sessionData)=0;
 
         /**
          * Read a session record from the underlying storage medium and return its data.
          * 
+         * <p>The inputs direct the implementation to perform policy enforcement of various
+         * sorts on the session prior to returning it. An invalid session MUST NOT be returned
+         * to the caller.</p>
+         * 
          * <p>To the extent possible, the implementation SHOULD ensure that the underlying
          * 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
@@ -69,11 +73,15 @@ namespace shibsp {
          * @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
+         * @return reconstituted session data or a null object if the session was absent or invalid
          */
         virtual DDF read(
-            const char* applicationId, const char* key, time_t lifetime=0, time_t timeout=0, const char* client_addr=nullptr
-        ) const=0;
+            const char* applicationId,
+            const char* key,
+            unsigned int lifetime=0,
+            unsigned int timeout=0,
+            const char* client_addr=nullptr
+            ) const=0;
 
         /**
          * Informs the storage medium that a session was used at the current point in time.
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index 262dd4f7..b4653ba4 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -133,7 +133,7 @@ bool AbstractSessionCache::start()
     return false;
 }
 
-string AbstractSessionCache::create(SPRequest& request, DDF session)
+string AbstractSessionCache::create(SPRequest& request, DDF& session)
 {
     m_log.debug("creating new session");
 
@@ -189,16 +189,49 @@ string AbstractSessionCache::create(SPRequest& request, DDF session)
 
 unique_lock<Session> AbstractSessionCache::find(SPRequest& request, bool checkTimeout, bool ignoreAddress)
 {
-    return unique_lock<Session>();
+    // Validation here depends on request content settings plus the input flags. The resulting policy
+    // is passed to the _find method for enforcement.
+
+    const char* key = m_cookieManager->getCookieValue(request);
+    if (!key) {
+        m_log.debug("no session cookie present, no session found");
+        return unique_lock<Session>();
+    }
+
+    const auto& settings = request.getRequestSettings().first;
+
+    const char* applicationId = settings->getString(RequestMapper::APPLICATION_ID_PROP_NAME, RequestMapper::APPLICATION_ID_PROP_NAME);
+    unsigned int lifetime = settings->getUnsignedInt(RequestMapper::LIFETIME_PROP_NAME, RequestMapper::LIFETIME_PROP_DEFAULT);
+    unsigned int timeout = 0;
+    if (checkTimeout) {
+        timeout = settings->getUnsignedInt(RequestMapper::TIMEOUT_PROP_NAME, RequestMapper::TIMEOUT_PROP_DEFAULT);
+    }
+    const char* client_addr = nullptr;
+    if (!ignoreAddress && settings->getBool(RequestMapper::CONSISTENT_ADDRESS_PROP_NAME, RequestMapper::CONSISTENT_ADDRESS_PROP_DEFAULT)) {
+        client_addr = request.getRemoteAddr().c_str();
+    }
+
+    unique_lock<Session> session = _find(applicationId, key, lifetime, timeout, client_addr);
+    if (!session) {
+        // If no session, we need to clear the session cookie to prevent further use.
+        m_log.debug("clearing cookie for session (%s)", key);
+        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;
 }
 
 unique_lock<Session> AbstractSessionCache::find(const char* applicationId, const char* key)
 {
+    // This variant does no request-based validation so it returns the session best effort
+    // using the underlying _find method. It does ensure the applicationId matches if set.
     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
+    const char* applicationId, const char* key, unsigned int lifetime, unsigned int timeout, const char* client_addr
     )
 {
 
@@ -434,7 +467,7 @@ const std::map<std::string,DDF>& BasicSession::getAttributes() const
     return m_attributes;
 }
 
-bool BasicSession::isValid(const char* applicationId, time_t lifetime, time_t timeout, const char* client_addr)
+bool BasicSession::isValid(const char* applicationId, unsigned int lifetime, unsigned int timeout, const char* client_addr)
 {
     return false;
 }
@@ -449,6 +482,12 @@ 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();

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


More information about the commits mailing list