[cpp-sp] branch main updated: Start Session cleanup to simplify tests.

Scott Cantor cantor.2 at osu.edu
Tue Dec 17 17:10:21 UTC 2024


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=5838568b7808a05bf0936af73988fae41b02b52c

The following commit(s) were added to refs/heads/main by this push:
     new 5838568b Start Session cleanup to simplify tests.
5838568b is described below

commit 5838568b7808a05bf0936af73988fae41b02b52c
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Dec 17 12:10:16 2024 -0500

    Start Session cleanup to simplify tests.
---
 apache/mod_shib_24.cpp                          | 32 ++-------
 shibsp/ServiceProvider.cpp                      | 66 ++++++------------
 shibsp/SessionCache.h                           | 91 +++++--------------------
 shibsp/exceptions.h                             |  1 +
 shibsp/handler/impl/AdminLogoutInitiator.cpp    |  6 +-
 shibsp/handler/impl/AttributeCheckerHandler.cpp |  6 +-
 shibsp/handler/impl/LocalLogoutInitiator.cpp    |  8 +--
 shibsp/handler/impl/SAML2LogoutInitiator.cpp    |  4 +-
 shibsp/handler/impl/SessionHandler.cpp          | 22 +++---
 shibsp/impl/StoredSession.cpp                   | 85 ++++++-----------------
 shibsp/impl/StoredSession.h                     | 60 ++++------------
 shibsp/impl/XMLAccessControl.cpp                | 15 ----
 12 files changed, 99 insertions(+), 297 deletions(-)

diff --git a/apache/mod_shib_24.cpp b/apache/mod_shib_24.cpp
index 51f32cd7..641df6e0 100644
--- a/apache/mod_shib_24.cpp
+++ b/apache/mod_shib_24.cpp
@@ -1195,7 +1195,7 @@ extern "C" authz_status shib_session_check_authz(request_rec* r, const char*, co
 
     try {
         Session* session = sta.first->getSession(false, true, false);
-        Locker slocker(session, false);
+        lock_guard<Session> slocker(*session, adopt_lock);
         if (session) {
             sta.first->log(SPRequest::SPDebug, "htaccess: accepting shib-session/valid-user based on active session");
             return AUTHZ_GRANTED;
@@ -1292,7 +1292,7 @@ extern "C" authz_status shib_acclass_check_authz(request_rec* r, const char* req
 
     try {
         Session* session = sta.first->getSession(false, true, false);
-        Locker slocker(session, false);
+        lock_guard<Session> slocker(*session, adopt_lock);
         if (session && hta.doAuthnContext(*sta.first, session->getAuthnContextClassRef(), require_line) == AccessControl::shib_acl_true)
             return AUTHZ_GRANTED;
         return session ? AUTHZ_DENIED : AUTHZ_DENIED_NO_USER;
@@ -1304,28 +1304,6 @@ extern "C" authz_status shib_acclass_check_authz(request_rec* r, const char* req
     return AUTHZ_GENERAL_ERROR;
 }
 
-extern "C" authz_status shib_acdecl_check_authz(request_rec* r, const char* require_line, const void*)
-{
-    pair<ShibTargetApache*,authz_status> sta = shib_base_check_authz(r);
-    if (!sta.first)
-        return sta.second;
-
-    const htAccessControl& hta = dynamic_cast<const ApacheRequestMapper*>(sta.first->getRequestSettings().first)->getHTAccessControl();
-
-    try {
-        Session* session = sta.first->getSession(false, true, false);
-        Locker slocker(session, false);
-        if (session && hta.doAuthnContext(*sta.first, session->getAuthnContextDeclRef(), require_line) == AccessControl::shib_acl_true)
-            return AUTHZ_GRANTED;
-        return session ? AUTHZ_DENIED : AUTHZ_DENIED_NO_USER;
-    }
-    catch (std::exception& e) {
-        sta.first->log(SPRequest::SPWarn, string("htaccess: unable to obtain session for access control check: ") +  e.what());
-    }
-
-    return AUTHZ_GENERAL_ERROR;
-}
-
 extern "C" authz_status shib_attr_check_authz(request_rec* r, const char* require_line, const void*)
 {
     pair<ShibTargetApache*,authz_status> sta = shib_base_check_authz(r);
@@ -1336,7 +1314,7 @@ extern "C" authz_status shib_attr_check_authz(request_rec* r, const char* requir
 
     try {
         Session* session = sta.first->getSession(false, true, false);
-        Locker slocker(session, false);
+        lock_guard<Session> slocker(*session, adopt_lock);
         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)
@@ -1361,7 +1339,7 @@ extern "C" authz_status shib_plugin_check_authz(request_rec* r, const char* requ
 
     try {
         Session* session = sta.first->getSession(false, true, false);
-        Locker slocker(session, false);
+        lock_guard<Session> slocker(*session, adopt_lock);
         if (session) {
             const char* config = ap_getword_conf(r->pool, &require_line);
             if (config && hta.doAccessControl(*sta.first, session, config) == AccessControl::shib_acl_true)
@@ -1585,7 +1563,6 @@ extern "C" const authz_provider shib_authz_session_provider = { &shib_session_ch
 extern "C" const authz_provider shib_authz_user_provider = { &shib_user_check_authz, nullptr };
 extern "C" const authz_provider shib_authz_ext_user_provider = { &shib_ext_user_check_authz, nullptr };
 extern "C" const authz_provider shib_authz_acclass_provider = { &shib_acclass_check_authz, nullptr };
-extern "C" const authz_provider shib_authz_acdecl_provider = { &shib_acdecl_check_authz, nullptr };
 extern "C" const authz_provider shib_authz_attr_provider = { &shib_attr_check_authz, nullptr };
 extern "C" const authz_provider shib_authz_plugin_provider = { &shib_plugin_check_authz, nullptr };
 
@@ -1618,7 +1595,6 @@ extern "C" void shib_register_hooks (apr_pool_t *p)
     ap_register_auth_provider(p, AUTHZ_PROVIDER_GROUP, "user", AUTHZ_PROVIDER_VERSION, &shib_authz_user_provider, AP_AUTH_INTERNAL_PER_CONF);
     ap_register_auth_provider(p, AUTHZ_PROVIDER_GROUP, "shib-user", AUTHZ_PROVIDER_VERSION, &shib_authz_ext_user_provider, AP_AUTH_INTERNAL_PER_CONF);
     ap_register_auth_provider(p, AUTHZ_PROVIDER_GROUP, "authnContextClassRef", AUTHZ_PROVIDER_VERSION, &shib_authz_acclass_provider, AP_AUTH_INTERNAL_PER_CONF);
-    ap_register_auth_provider(p, AUTHZ_PROVIDER_GROUP, "authnContextDeclRef", AUTHZ_PROVIDER_VERSION, &shib_authz_acdecl_provider, AP_AUTH_INTERNAL_PER_CONF);
     ap_register_auth_provider(p, AUTHZ_PROVIDER_GROUP, "shib-attr", AUTHZ_PROVIDER_VERSION, &shib_authz_attr_provider, AP_AUTH_INTERNAL_PER_CONF);
     ap_register_auth_provider(p, AUTHZ_PROVIDER_GROUP, "shib-plugin", AUTHZ_PROVIDER_VERSION, &shib_authz_plugin_provider, AP_AUTH_INTERNAL_PER_CONF);
 }
diff --git a/shibsp/ServiceProvider.cpp b/shibsp/ServiceProvider.cpp
index b95d8fde..7363012c 100644
--- a/shibsp/ServiceProvider.cpp
+++ b/shibsp/ServiceProvider.cpp
@@ -28,6 +28,7 @@
 #include "SPRequest.h"
 #include "attribute/Attribute.h"
 #include "handler/SessionInitiator.h"
+#include "util/Date.h"
 #include "util/PathResolver.h"
 #include "util/TemplateParameters.h"
 #include "util/URLEncoder.h"
@@ -362,7 +363,7 @@ pair<bool,long> ServiceProvider::doAuthentication(SPRequest& request, bool handl
             throw;
         }
 
-        Locker slocker(session, false); // pop existing lock on exit
+        lock_guard<Session> slocker(*session, adopt_lock); // pop existing lock on exit
         if (session) {
             // Check for logout interception.
             if (requireLogoutWith) {
@@ -439,7 +440,7 @@ pair<bool,long> ServiceProvider::doAuthorization(SPRequest& request) const
 
     const Application* app = nullptr;
     Session* session = nullptr;
-    Locker slocker;
+    unique_lock<Session> slocker;
     string targetURL = request.getRequestURL();
 
     try {
@@ -462,8 +463,10 @@ pair<bool,long> ServiceProvider::doAuthorization(SPRequest& request) const
         if (settings.second) {
             try {
                 session = request.getSession(false, false, false);  // ignore timeout and do not cache
-                if (session)
-                    slocker.assign(session, false); // assign to lock popper
+                if (session) {
+                    unique_lock<Session> slocker2(*session, adopt_lock);
+                    slocker.swap(slocker2); // assign to lock popper
+                }
             }
             catch (const exception& e) {
                 log.warn("unable to obtain session to pass to access control provider: %s", e.what());
@@ -508,7 +511,7 @@ pair<bool,long> ServiceProvider::doExport(SPRequest& request, bool requireSessio
 
     const Application* app = nullptr;
     Session* session = nullptr;
-    Locker slocker;
+    unique_lock<Session> slocker;
     string targetURL = request.getRequestURL();
 
     try {
@@ -517,8 +520,10 @@ pair<bool,long> ServiceProvider::doExport(SPRequest& request, bool requireSessio
 
         try {
             session = request.getSession(false, false, false);  // ignore timeout and do not cache
-            if (session)
-                slocker.assign(session, false); // assign to lock popper
+            if (session) {
+                unique_lock<Session> slocker2(*session, adopt_lock);
+                slocker.swap(slocker2); // assign to lock popper
+            }
         }
         catch (const exception& e) {
             log.warn("unable to obtain session to export to request: %s", e.what());
@@ -548,20 +553,18 @@ pair<bool,long> ServiceProvider::doExport(SPRequest& request, bool requireSessio
             const char* hval = session->getEntityID();
             if (hval)
                 app->setHeader(request, "Shib-Identity-Provider", hval);
-            hval = session->getAuthnInstant();
-            if (hval)
-                app->setHeader(request, "Shib-Authentication-Instant", hval);
+            time_t ts = session->getAuthnInstant();
+            if (ts > 0) {
+                // TODO: Need to see what the output format of this really is.
+                ostringstream os;
+                os << date::format("%FT%TZ", chrono::system_clock::from_time_t(ts));
+                app->setHeader(request, "Shib-Authentication-Instant", os.str().c_str());
+            }
             hval = session->getAuthnContextClassRef();
             if (hval) {
                 app->setHeader(request, "Shib-Authentication-Method", hval);
                 app->setHeader(request, "Shib-AuthnContext-Class", hval);
             }
-            hval = session->getAuthnContextDeclRef();
-            if (hval)
-                app->setHeader(request, "Shib-AuthnContext-Decl", hval);
-            hval = session->getSessionIndex();
-            if (hval)
-                app->setHeader(request, "Shib-Session-Index", hval);
 
             app->setHeader(request, "Shib-Session-Expires", boost::lexical_cast<string>(session->getExpiration()).c_str());
             pair<bool,unsigned int> timeout = sessionProps ? sessionProps->getUnsignedInt("timeout") : pair<bool,unsigned int>(false, 0);
@@ -577,35 +580,6 @@ pair<bool,long> ServiceProvider::doExport(SPRequest& request, bool requireSessio
             app->setHeader(request, "Shib-Cookie-Name", cookieprops.first.c_str());
         }
 
-        // Maybe export the assertion keys.
-        bool exportAssertion = settings.first->getBool("exportAssertion", false);
-        if (exportAssertion) {
-            pair<bool,const char*> exportLocation = sessionProps ? sessionProps->getString("exportLocation") : make_pair(false,nullptr);
-            if (!exportLocation.first)
-                log.warn("can't export assertions without an exportLocation Sessions property");
-            else {
-                string exportName = "Shib-Assertion-00";
-                string baseURL;
-                if (!strncmp(exportLocation.second, "http", 4)) {
-                    baseURL = exportLocation.second;
-                }
-                else {
-                    baseURL = string(request.getHandlerURL(targetURL.c_str())) + exportLocation.second;
-                }
-                baseURL = baseURL + "?key=" + session->getID() + "&ID=";
-                const vector<const char*>& tokens = session->getAssertionIDs();
-                vector<const char*>::size_type count = 0;
-                for (vector<const char*>::const_iterator tokenids = tokens.begin(); tokenids!=tokens.end(); ++tokenids) {
-                    count++;
-                    *(exportName.rbegin()) = '0' + (count%10);
-                    *(++exportName.rbegin()) = '0' + (count/10);
-                    string fullURL = baseURL + AgentConfig::getConfig().getURLEncoder().encode(*tokenids);
-                    app->setHeader(request, exportName.c_str(), fullURL.c_str());
-                }
-                app->setHeader(request, "Shib-Assertion-Count", exportName.c_str() + 15);
-            }
-        }
-
         // Export the attributes.
         exportAttributes(request, session, settings);
 
@@ -694,7 +668,7 @@ pair<bool,long> ServiceProvider::doHandler(SPRequest& request) const
         }
         catch (const exception&) {
         }
-        Locker slocker(session, false); // pop existing lock on exit
+        lock_guard<Session> slocker(*session, adopt_lock); // pop existing lock on exit
         TemplateParameters tp(&e, nullptr, session);
         tp.m_map["requestURL"] = targetURL.substr(0, targetURL.find('?'));
         //stp.m_request = &request;
diff --git a/shibsp/SessionCache.h b/shibsp/SessionCache.h
index 4739b429..b604e748 100644
--- a/shibsp/SessionCache.h
+++ b/shibsp/SessionCache.h
@@ -1,21 +1,15 @@
-/**
- * Licensed to the University Corporation for Advanced Internet
- * Development, Inc. (UCAID) under one or more contributor license
- * agreements. See the NOTICE file distributed with this work for
- * additional information regarding copyright ownership.
- *
- * UCAID licenses this file to you 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
+/*
+ * 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
+ *    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.
+ * 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.
  */
 
 /**
@@ -27,15 +21,13 @@
 #ifndef __shibsp_sessioncache_h__
 #define __shibsp_sessioncache_h__
 
-#include <shibsp/base.h>
+#include <shibsp/util/Lockable.h>
 
 #include <map>
-#include <set>
+#include <memory>
 #include <string>
 #include <vector>
 #include <ctime>
-#include <xercesc/util/XercesDefs.hpp>
-#include <xmltooling/Lockable.h>
 
 namespace shibsp {
 
@@ -52,7 +44,7 @@ namespace shibsp {
      * or at least controlled, and the caller must unlock a Session
      * to dispose of it.</p>
      */
-    class SHIBSP_API Session : public virtual xmltooling::Lockable
+    class SHIBSP_API Session : public virtual BasicLockable
     {
         MAKE_NONCOPYABLE(Session);
     protected:
@@ -60,7 +52,7 @@ namespace shibsp {
         virtual ~Session();
     public:
         /**
-         * Returns the session key.
+         * Returns the session ID.
          *
          * @return unique ID of session
          */
@@ -109,18 +101,11 @@ namespace shibsp {
         virtual const char* getProtocol() const=0;
 
         /**
-         * Returns the UTC timestamp on the authentication event at the IdP.
-         *
-         * @return  the UTC authentication timestamp
-         */
-        virtual const char* getAuthnInstant() const=0;
-
-        /**
-         * Returns the SessionIndex provided with the session.
+         * Returns the timestamp of the authentication event at the IdP.
          *
-         * @return the SessionIndex from the original SSO assertion, if any
+         * @return  the authentication timestamp
          */
-        virtual const char* getSessionIndex() const=0;
+        virtual time_t getAuthnInstant() const=0;
 
         /**
          * Returns a URI containing an AuthnContextClassRef provided with the session.
@@ -131,19 +116,12 @@ namespace shibsp {
          */
         virtual const char* getAuthnContextClassRef() const=0;
 
-        /**
-         * Returns a URI containing an AuthnContextDeclRef provided with the session.
-         *
-         * @return  a URI identifying the authentication context declaration
-         */
-        virtual const char* getAuthnContextDeclRef() const=0;
-
         /**
          * Returns the resolved attributes associated with the session.
          *
          * @return an immutable array of attributes
          */
-        virtual const std::vector<Attribute*>& getAttributes() const=0;
+        virtual const std::vector<std::unique_ptr<Attribute>>& getAttributes() const=0;
 
         /**
          * Returns the resolved attributes associated with the session, indexed by ID.
@@ -151,39 +129,6 @@ namespace shibsp {
          * @return an immutable map of attributes keyed by attribute ID
          */
         virtual const std::multimap<std::string,const Attribute*>& getIndexedAttributes() const=0;
-
-        /**
-         * Returns the identifiers of the assertion(s) cached by the session.
-         *
-         * <p>The SSO assertion is guaranteed to be first in the set.</p>
-         *
-         * @return  an immutable array of AssertionID values
-         */
-        virtual const std::vector<const char*>& getAssertionIDs() const=0;
-
-#ifndef SHIBSP_LITE
-        /**
-         * Adds additional attributes to the session.
-         *
-         * @param attributes    reference to an array of Attributes to cache (will be freed by cache)
-         */
-        virtual void addAttributes(const std::vector<Attribute*>& attributes)=0;
-
-        /**
-         * Returns an assertion cached by the session.
-         *
-         * @param id    identifier of the assertion to retrieve
-         * @return pointer to assertion, or nullptr
-         */
-        virtual const opensaml::Assertion* getAssertion(const char* id) const=0;
-
-        /**
-         * Stores an assertion in the session.
-         *
-         * @param assertion pointer to an assertion to cache (will be freed by cache)
-         */
-        virtual void addAssertion(opensaml::Assertion* assertion)=0;
-#endif
     };
 
     /**
diff --git a/shibsp/exceptions.h b/shibsp/exceptions.h
index 7de77bba..849b42c4 100644
--- a/shibsp/exceptions.h
+++ b/shibsp/exceptions.h
@@ -35,6 +35,7 @@ namespace shibsp {
     DECL_XMLTOOLING_EXCEPTION(AttributeException,SHIBSP_EXCEPTIONAPI(SHIBSP_API),shibsp,xmltooling::XMLToolingException,Exceptions during attribute processing.);
     DECL_XMLTOOLING_EXCEPTION(ConfigurationException,SHIBSP_EXCEPTIONAPI(SHIBSP_API),shibsp,xmltooling::XMLToolingException,Exceptions during configuration.);
     DECL_XMLTOOLING_EXCEPTION(ListenerException,SHIBSP_EXCEPTIONAPI(SHIBSP_API),shibsp,xmltooling::XMLToolingException,Exceptions during inter-process communication.);
+    DECL_XMLTOOLING_EXCEPTION(SessionException,SHIBSP_EXCEPTIONAPI(SHIBSP_API),shibsp,xmltooling::XMLToolingException,Exceptions during session processing.);
 
 };
 
diff --git a/shibsp/handler/impl/AdminLogoutInitiator.cpp b/shibsp/handler/impl/AdminLogoutInitiator.cpp
index d7f4f0f6..60d1abd4 100644
--- a/shibsp/handler/impl/AdminLogoutInitiator.cpp
+++ b/shibsp/handler/impl/AdminLogoutInitiator.cpp
@@ -196,7 +196,7 @@ pair<bool,long> AdminLogoutInitiator::doRequest(const Application& application,
 
     time_t revocationExp = session->getExpiration();
 
-    Locker sessionLocker(session, false);
+    unique_lock<Session> sessionLocker(*session, adopt_lock);
 
     bool doSAML = false;
 
@@ -217,7 +217,7 @@ pair<bool,long> AdminLogoutInitiator::doRequest(const Application& application,
     // Do back channel notification.
     vector<string> sessions(1, session->getID());
     if (!notifyBackChannel(application, httpRequest.getRequestURL(), sessions, true)) {
-        sessionLocker.assign();
+        sessionLocker.unlock();
         session = nullptr;
         application.getServiceProvider().getSessionCache()->remove(application, sessionId, revocationExp);
         
@@ -226,7 +226,7 @@ pair<bool,long> AdminLogoutInitiator::doRequest(const Application& application,
     }
 
     if (!doSAML) {
-        sessionLocker.assign();
+        sessionLocker.unlock();
         session = nullptr;
         application.getServiceProvider().getSessionCache()->remove(application, sessionId, revocationExp);
 
diff --git a/shibsp/handler/impl/AttributeCheckerHandler.cpp b/shibsp/handler/impl/AttributeCheckerHandler.cpp
index f74bdac0..91f1e2fb 100644
--- a/shibsp/handler/impl/AttributeCheckerHandler.cpp
+++ b/shibsp/handler/impl/AttributeCheckerHandler.cpp
@@ -157,7 +157,7 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
         request.log(SPRequest::SPWarn, string("AttributeChecker caught exception accessing session immediately after creation: ") + ex.what());
     }
 
-    Locker sessionLocker(session, false);
+    unique_lock<Session> sessionLocker(*session, adopt_lock);
 
     bool checked = false;
     if (session) {
@@ -203,7 +203,7 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
         XMLToolingConfig::getConfig().getTemplateEngine()->run(infile, str, tp);
         if (m_flushSession && session) {
             time_t revocationExp = session->getExpiration();
-            sessionLocker.assign(); // unlock the session
+            sessionLocker.unlock(); // unlock the session
             flushSession(request, revocationExp);
         }
         return make_pair(true, request.sendError(str));
@@ -211,7 +211,7 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
 
     if (m_flushSession && session) {
         time_t revocationExp = session->getExpiration();
-        sessionLocker.assign(); // unlock the session
+        sessionLocker.unlock(); // unlock the session
         flushSession(request, revocationExp);
     }
     m_log.error("could not process error template (%s)", m_template.c_str());
diff --git a/shibsp/handler/impl/LocalLogoutInitiator.cpp b/shibsp/handler/impl/LocalLogoutInitiator.cpp
index 1d3f08ff..b4d70fb3 100644
--- a/shibsp/handler/impl/LocalLogoutInitiator.cpp
+++ b/shibsp/handler/impl/LocalLogoutInitiator.cpp
@@ -33,10 +33,6 @@
 #include "handler/AbstractHandler.h"
 #include "handler/LogoutInitiator.h"
 
-#ifndef SHIBSP_LITE
-using namespace boost;
-#endif
-
 using namespace shibsp;
 using namespace xmltooling;
 using namespace std;
@@ -175,14 +171,14 @@ pair<bool,long> LocalLogoutInitiator::doRequest(
 {
     if (session) {
         // Guard the session in case of exception.
-        Locker locker(session, false);
+        unique_lock<Session> locker(*session, adopt_lock);
 
         // Do back channel notification.
         bool result;
         vector<string> sessions(1, session->getID());
         result = notifyBackChannel(application, httpRequest.getRequestURL(), sessions, true);
         time_t revocationExp = session->getExpiration();
-        locker.assign();    // unlock the session
+        locker.unlock();    // unlock the session
         application.getServiceProvider().getSessionCache()->remove(application, httpRequest, &httpResponse, revocationExp);
         if (!result)
             return sendLogoutPage(application, httpRequest, httpResponse, "partial");
diff --git a/shibsp/handler/impl/SAML2LogoutInitiator.cpp b/shibsp/handler/impl/SAML2LogoutInitiator.cpp
index 76020197..1e758023 100644
--- a/shibsp/handler/impl/SAML2LogoutInitiator.cpp
+++ b/shibsp/handler/impl/SAML2LogoutInitiator.cpp
@@ -239,13 +239,13 @@ pair<bool,long> SAML2LogoutInitiator::doRequest(
     const Application& application, const HTTPRequest& httpRequest, HTTPResponse& httpResponse, Session* session
     ) const
 {
-    Locker sessionLocker(session, false);
+    unique_lock<Session> sessionLocker(*session, adopt_lock);
 
     // Do back channel notification.
     vector<string> sessions(1, session->getID());
     if (!notifyBackChannel(application, httpRequest.getRequestURL(), sessions, false)) {
         time_t revocationExp = session->getExpiration();
-        sessionLocker.assign();
+        sessionLocker.unlock();
         session = nullptr;
         application.getServiceProvider().getSessionCache()->remove(application, httpRequest, &httpResponse, revocationExp);
         return sendLogoutPage(application, httpRequest, httpResponse, "partial");
diff --git a/shibsp/handler/impl/SessionHandler.cpp b/shibsp/handler/impl/SessionHandler.cpp
index 558353d1..3e6591b0 100644
--- a/shibsp/handler/impl/SessionHandler.cpp
+++ b/shibsp/handler/impl/SessionHandler.cpp
@@ -32,6 +32,7 @@
 #include "SPRequest.h"
 #include "attribute/Attribute.h"
 #include "handler/SecuredHandler.h"
+#include "util/Date.h"
 
 #include <ctime>
 #include <sstream>
@@ -183,19 +184,17 @@ pair<bool,long> SessionHandler::doJSON(SPRequest& request) const
 
         if (session->getAuthnInstant()) {
             s << ", \"authn_instant\": ";
-            json_safe(s, session->getAuthnInstant());
+            time_t ts = session->getAuthnInstant();
+            // TODO: Need to see what the output format of this really is.
+            ostringstream os;
+            os << date::format("%FT%TZ", chrono::system_clock::from_time_t(ts));
+            json_safe(s, os.str().c_str());
         }
 
         if (session->getAuthnContextClassRef()) {
             s << ", \"authncontext_class\": ";
             json_safe(s, session->getAuthnContextClassRef());
         }
-
-        if (session->getAuthnContextDeclRef()) {
-            s << ", \"authncontext_decl\": ";
-            json_safe(s, session->getAuthnContextDeclRef());
-        }
-
     }
 
     /*
@@ -290,9 +289,14 @@ pair<bool,long> SessionHandler::doHTML(SPRequest& request) const
     bool stdvars = request.getRequestSettings().first->getBool("exportStdVars", true);
     if (stdvars) {
         s << "<strong>Identity Provider:</strong> " << (session->getEntityID() ? session->getEntityID() : "(none)") << endl;
-        s << "<strong>Authentication Time:</strong> " << (session->getAuthnInstant() ? session->getAuthnInstant() : "(none)") << endl;
+        time_t ts = session->getAuthnInstant();
+        if (ts > 0) {
+            // TODO: Need to see what the output format of this really is.
+            ostringstream os;
+            os << date::format("%FT%TZ", chrono::system_clock::from_time_t(ts));
+            s << "<strong>Authentication Time:</strong> " << os.str() << endl;
+        }
         s << "<strong>Authentication Context Class:</strong> " << (session->getAuthnContextClassRef() ? session->getAuthnContextClassRef() : "(none)") << endl;
-        s << "<strong>Authentication Context Decl:</strong> " << (session->getAuthnContextDeclRef() ? session->getAuthnContextDeclRef() : "(none)") << endl;
     }
 
     s << endl << "<u>Attributes</u>" << endl;
diff --git a/shibsp/impl/StoredSession.cpp b/shibsp/impl/StoredSession.cpp
index 828f0513..ae9db433 100644
--- a/shibsp/impl/StoredSession.cpp
+++ b/shibsp/impl/StoredSession.cpp
@@ -31,12 +31,7 @@
 #include "impl/StoredSession.h"
 #include "impl/StorageServiceSessionCache.h"
 
-#include <xmltooling/util/Threads.h>
-
-#include <xercesc/util/XMLDateTime.hpp>
-
 using namespace shibsp;
-using namespace xmltooling;
 using namespace boost;
 using namespace std;
 
@@ -66,48 +61,27 @@ StoredSession::StoredSession(SSCache* cache, DDF& obj)
             addrobj.addmember(getAddressFamily(saddr)).string(saddr);
         }
     }
-
-    auto_ptr_XMLCh exp(m_obj["expires"].string());
-    if (exp.get()) {
-        XMLDateTime iso(exp.get());
-        iso.parseDateTime();
-        m_expires = iso.getEpoch();
-    }
-
-#ifndef SHIBSP_LITE
-    const char* nameid = obj["nameid"].string();
-    if (nameid) {
-        // Parse and bind the document into an XMLObject.
-        istringstream instr(nameid);
-        DOMDocument* doc = XMLToolingConfig::getConfig().getParser().parse(instr);
-        XercesJanitor<DOMDocument> janitor(doc);
-        m_nameid.reset(saml2::NameIDBuilder::buildNameID());
-        m_nameid->unmarshall(doc->getDocumentElement(), true);
-        janitor.release();
-    }
-#endif
-    if (cache->inproc)
-        m_lock.reset(Mutex::create());
+    m_expires = m_obj["expires"].longinteger();
 }
 
 StoredSession::~StoredSession()
 {
     m_obj.destroy();
-    for_each(m_attributes.begin(), m_attributes.end(), xmltooling::cleanup<Attribute>());
 }
 
-Lockable* StoredSession::lock()
+void StoredSession::lock()
+{
+    m_lock.lock();
+}
+
+bool StoredSession::try_lock()
 {
-    if (m_lock.get())
-        m_lock->lock();
-    return this;
+    return m_lock.try_lock();
 }
+
 void StoredSession::unlock()
 {
-    if (m_lock.get())
-        m_lock->unlock();
-    else
-        delete this;
+    m_lock.unlock();
 }
 
 const multimap<string, const Attribute*>& StoredSession::getIndexedAttributes() const
@@ -115,42 +89,28 @@ const multimap<string, const Attribute*>& StoredSession::getIndexedAttributes()
     if (m_attributeIndex.empty()) {
         if (m_attributes.empty())
             unmarshallAttributes();
-        for (vector<Attribute*>::const_iterator a = m_attributes.begin(); a != m_attributes.end(); ++a) {
-            const vector<string>& aliases = (*a)->getAliases();
-            for (vector<string>::const_iterator alias = aliases.begin(); alias != aliases.end(); ++alias)
-                m_attributeIndex.insert(multimap<string, const Attribute*>::value_type(*alias, *a));
+        for (const unique_ptr<Attribute>& a : m_attributes) {
+            const vector<string>& aliases = a->getAliases();
+            for (const string& alias : a->getAliases()) {
+                m_attributeIndex.insert(multimap<string, const Attribute*>::value_type(alias, a.get()));
+            }
         }
     }
     return m_attributeIndex;
 }
 
-const vector<const char*>& StoredSession::getAssertionIDs() const
-{
-    if (m_ids.empty()) {
-        DDF ids = m_obj["assertions"];
-        DDF id = ids.first();
-        while (id.isstring()) {
-            m_ids.push_back(id.string());
-            id = ids.next();
-        }
-    }
-    return m_ids;
-}
-
 void StoredSession::unmarshallAttributes() const
 {
-    Attribute* attribute;
     DDF attrs = m_obj["attributes"];
     DDF attr = attrs.first();
     while (!attr.isnull()) {
         try {
-            attribute = Attribute::unmarshall(attr);
-            m_attributes.push_back(attribute);
+            m_attributes.push_back(unique_ptr<Attribute>(Attribute::unmarshall(attr)));
             if (m_cache->m_log.isDebugEnabled())
                 m_cache->m_log.debug("unmarshalled attribute (ID: %s) with %d value%s",
-                    attribute->getId(), attr.first().integer(), attr.first().integer()!=1 ? "s" : "");
+                    m_attributes.back()->getId(), attr.first().integer(), attr.first().integer()!=1 ? "s" : "");
         }
-        catch (AttributeException& ex) {
+        catch (const AttributeException& ex) {
             const char* id = attr.first().name();
             m_cache->m_log.error("error unmarshalling attribute (ID: %s): %s", id ? id : "none", ex.what());
         }
@@ -166,7 +126,7 @@ void StoredSession::validate(const Application& app, const char* client_addr, ti
     if (m_expires > 0) {
         if (now > m_expires) {
             m_cache->m_log.info("session expired (ID: %s)", getID());
-            throw XMLToolingException("Your session has expired, and you must re-authenticate.");
+            throw SessionException("Your session has expired, and you must re-authenticate.");
         }
     }
 
@@ -176,9 +136,8 @@ void StoredSession::validate(const Application& app, const char* client_addr, ti
         if (saddr && *saddr) {
             if (!m_cache->compareAddresses(client_addr, saddr)) {
                 m_cache->m_log.warn("client address mismatch, client (%s), session (%s)", client_addr, saddr);
-                throw XMLToolingException(
-                    "Your IP address ($1) does not match the address recorded at the time the session was established.",
-                    params(1, client_addr)
+                throw SessionException(
+                    string("Your IP address (") + client_addr + ") does not match the address recorded at the time the session was established."
                     );
             }
             client_addr = nullptr;  // clear out parameter as signal that session need not be updated below
@@ -217,8 +176,6 @@ void StoredSession::validate(const Application& app, const char* client_addr, ti
         if (out.isstruct()) {
             // We got an updated record back.
             m_cache->m_log.debug("session updated, reconstituting it");
-            m_ids.clear();
-            for_each(m_attributes.begin(), m_attributes.end(), xmltooling::cleanup<Attribute>());
             m_attributes.clear();
             m_attributeIndex.clear();
             m_obj.destroy();
diff --git a/shibsp/impl/StoredSession.h b/shibsp/impl/StoredSession.h
index 2976e482..44803b39 100644
--- a/shibsp/impl/StoredSession.h
+++ b/shibsp/impl/StoredSession.h
@@ -19,7 +19,7 @@
  */
 
 /**
- * StoredSession.h
+ * impl/StoredSession.h
  *
  * Internal declaration of Session subclass used by StorageService-backed SessionCache.
  */
@@ -31,23 +31,7 @@
 #include "SessionCache.h"
 #include "remoting/ddf.h"
 
-#include <ctime>
-#include <boost/scoped_ptr.hpp>
-#include <boost/shared_ptr.hpp>
-
-namespace xmltooling {
-    class Mutex;
-};
-
-#ifndef SHIBSP_LITE
-namespace opensaml {
-    class Assertion;
-
-    namespace saml2 {
-        class NameID;
-    };
-};
-#endif
+#include <mutex>
 
 namespace shibsp {
 
@@ -60,7 +44,8 @@ namespace shibsp {
 
         virtual ~StoredSession();
 
-        xmltooling::Lockable* lock();
+        void lock();
+        bool try_lock();
         void unlock();
 
         const char* getID() const {
@@ -91,40 +76,21 @@ namespace shibsp {
         const char* getProtocol() const {
             return m_obj["protocol"].string();
         }
-        const char* getAuthnInstant() const {
-            return m_obj["authn_instant"].string();
-        }
-#ifndef SHIBSP_LITE
-        const opensaml::saml2::NameID* getNameID() const {
-            return m_nameid.get();
-        }
-#endif
-        const char* getSessionIndex() const {
-            return m_obj["session_index"].string();
+        time_t getAuthnInstant() const {
+            return m_obj["authn_instant"].longinteger();
         }
         const char* getAuthnContextClassRef() const {
             return m_obj["authncontext_class"].string();
         }
-        const char* getAuthnContextDeclRef() const {
-            return m_obj["authncontext_decl"].string();
-        }
-        const std::vector<Attribute*>& getAttributes() const {
+        const std::vector<std::unique_ptr<Attribute>>& getAttributes() const {
             if (m_attributes.empty())
                 unmarshallAttributes();
             return m_attributes;
         }
         const std::multimap<std::string, const Attribute*>& getIndexedAttributes() const;
 
-        const std::vector<const char*>& getAssertionIDs() const;
-
         void validate(const Application& application, const char* client_addr, time_t* timeout);
 
-#ifndef SHIBSP_LITE
-        void addAttributes(const std::vector<Attribute*>& attributes);
-        const opensaml::Assertion* getAssertion(const char* id) const;
-        void addAssertion(opensaml::Assertion* assertion);
-#endif
-
         time_t getExpiration() const { return m_expires; }
         time_t getLastAccess() const { return m_lastAccess; }
 
@@ -136,17 +102,15 @@ namespace shibsp {
         void unmarshallAttributes() const;
 
         DDF m_obj;
-#ifndef SHIBSP_LITE
-        boost::scoped_ptr<opensaml::saml2::NameID> m_nameid;
-        mutable std::map< std::string,boost::shared_ptr<opensaml::Assertion> > m_tokens;
-#endif
-        mutable std::vector<Attribute*> m_attributes;
+        mutable std::vector<std::unique_ptr<Attribute>> m_attributes;
         mutable std::multimap<std::string,const Attribute*> m_attributeIndex;
-        mutable std::vector<const char*> m_ids;
 
         SSCache* m_cache;
         time_t m_expires,m_lastAccess;
-        boost::scoped_ptr<xmltooling::Mutex> m_lock;
+        // 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;
     };
 
 }
diff --git a/shibsp/impl/XMLAccessControl.cpp b/shibsp/impl/XMLAccessControl.cpp
index a9acabd6..372bf20c 100644
--- a/shibsp/impl/XMLAccessControl.cpp
+++ b/shibsp/impl/XMLAccessControl.cpp
@@ -187,14 +187,6 @@ AccessControl::aclresult_t Rule::authorized(const SPRequest& request, const Sess
         }
         return shib_acl_false;
     }
-    else if (m_alias == "authnContextDeclRef") {
-        const char* ref = session->getAuthnContextDeclRef();
-        if (ref && m_vals.find(ref) != m_vals.end()) {
-            request.log(SPRequest::SPDebug, string("AccessControl plugin expecting authnContextDeclRef (") + ref + "), authz granted");
-            return shib_acl_true;
-        }
-        return shib_acl_false;
-    }
 
     // Find the attribute(s) matching the require rule.
     pair<multimap<string,const Attribute*>::const_iterator, multimap<string,const Attribute*>::const_iterator> attrs =
@@ -282,13 +274,6 @@ AccessControl::aclresult_t RuleRegex::authorized(const SPRequest& request, const
         }
         return shib_acl_false;
     }
-    else if (m_alias == "authnContextDeclRef") {
-        if (session->getAuthnContextDeclRef() && regex_match(session->getAuthnContextDeclRef(), m_re)) {
-            request.log(SPRequest::SPDebug, string("AccessControl plugin expecting authnContextDeclRef (") + m_exp + "), authz granted");
-            return shib_acl_true;
-        }
-        return shib_acl_false;
-    }
 
     // Find the attribute(s) matching the require rule.
     auto attrs = session->getIndexedAttributes().equal_range(m_alias);

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


More information about the commits mailing list