X-Git-Url: http://www.project-moonshot.org/gitweb/?a=blobdiff_plain;f=shibsp%2FServiceProvider.cpp;h=3ac5236055fb77aad116eaf417142f93b525331c;hb=HEAD;hp=97d5a5ef46f35bb6ee87d1bf63465f9aee8e6448;hpb=65672c36aaa838db4a2c4cacf05d5d66f005832f;p=shibboleth%2Fcpp-sp.git

diff --git a/shibsp/ServiceProvider.cpp b/shibsp/ServiceProvider.cpp
index 97d5a5e..3ac5236 100644
--- a/shibsp/ServiceProvider.cpp
+++ b/shibsp/ServiceProvider.cpp
@@ -193,7 +193,7 @@ Remoted* ServiceProvider::regListener(const char* address, Remoted* listener)
     if (i != m_listenerMap.end())
         ret = i->second;
     m_listenerMap[address] = listener;
-    Category::getInstance(SHIBSP_LOGCAT".ServiceProvider").info("registered remoted message endpoint (%s)",address);
+    Category::getInstance(SHIBSP_LOGCAT ".ServiceProvider").info("registered remoted message endpoint (%s)",address);
     return ret;
 }
 
@@ -205,7 +205,7 @@ bool ServiceProvider::unregListener(const char* address, Remoted* current, Remot
             m_listenerMap[address] = restore;
         else
             m_listenerMap.erase(address);
-        Category::getInstance(SHIBSP_LOGCAT".ServiceProvider").info("unregistered remoted message endpoint (%s)",address);
+        Category::getInstance(SHIBSP_LOGCAT ".ServiceProvider").info("unregistered remoted message endpoint (%s)",address);
         return true;
     }
     return false;
@@ -222,7 +222,7 @@ pair<bool,long> ServiceProvider::doAuthentication(SPRequest& request, bool handl
 #ifdef _DEBUG
     xmltooling::NDC ndc("doAuthentication");
 #endif
-    Category& log = Category::getInstance(SHIBSP_LOGCAT".ServiceProvider");
+    Category& log = Category::getInstance(SHIBSP_LOGCAT ".ServiceProvider");
 
     const Application* app = nullptr;
     string targetURL = request.getRequestURL();
@@ -287,7 +287,7 @@ pair<bool,long> ServiceProvider::doAuthentication(SPRequest& request, bool handl
 
         Session* session = nullptr;
         try {
-            session = request.getSession();
+            session = request.getSession(true, false, false);   // don't cache it
         }
         catch (exception& e) {
             log.warn("error during session lookup: %s", e.what());
@@ -296,6 +296,7 @@ pair<bool,long> ServiceProvider::doAuthentication(SPRequest& request, bool handl
                 throw;
         }
 
+        Locker slocker(session, false); // pop existing lock on exit
         if (session) {
             // Check for logout interception.
             if (requireLogoutWith.first) {
@@ -304,7 +305,9 @@ pair<bool,long> ServiceProvider::doAuthentication(SPRequest& request, bool handl
                 if (!qstr || !strstr(qstr, "shiblogoutdone=1")) {
                     // First leg of circuit, so we redirect to the logout endpoint specified with this URL as a return location.
                     string selfurl = request.getRequestURL();
-                    if (!qstr)
+                    if (qstr)
+                        selfurl += '&';
+                    else
                         selfurl += '?';
                     selfurl += "shiblogoutdone=1";
                     string loc = requireLogoutWith.second;
@@ -339,7 +342,12 @@ pair<bool,long> ServiceProvider::doAuthentication(SPRequest& request, bool handl
                     throw ConfigurationException("No default session initiator found, check configuration.");
             }
 
-            return initiator->run(request, false);
+            // Dispatch to SessionInitiator. This MUST handle the request, or we want to fail here.
+            // Used to fall through into doExport, but this is a cleaner exit path.
+            pair<bool,long> ret = initiator->run(request, false);
+            if (ret.first)
+                return ret;
+            throw ConfigurationException("Session initiator did not handle request for a new session, check configuration.");
         }
 
         request.setAuthType(authType.second);
@@ -362,10 +370,11 @@ pair<bool,long> ServiceProvider::doAuthorization(SPRequest& request) const
 #ifdef _DEBUG
     xmltooling::NDC ndc("doAuthorization");
 #endif
-    Category& log = Category::getInstance(SHIBSP_LOGCAT".ServiceProvider");
+    Category& log = Category::getInstance(SHIBSP_LOGCAT ".ServiceProvider");
 
     const Application* app = nullptr;
-    const Session* session = nullptr;
+    Session* session = nullptr;
+    Locker slocker;
     string targetURL = request.getRequestURL();
 
     try {
@@ -387,7 +396,9 @@ pair<bool,long> ServiceProvider::doAuthorization(SPRequest& request) const
         // Do we have an access control plugin?
         if (settings.second) {
             try {
-                session = request.getSession(false);
+                session = request.getSession(false, false, false);  // ignore timeout and do not cache
+                if (session)
+                    slocker.assign(session, false); // assign to lock popper
             }
             catch (exception& e) {
                 log.warn("unable to obtain session to pass to access control provider: %s", e.what());
@@ -429,10 +440,11 @@ pair<bool,long> ServiceProvider::doExport(SPRequest& request, bool requireSessio
 #ifdef _DEBUG
     xmltooling::NDC ndc("doExport");
 #endif
-    Category& log = Category::getInstance(SHIBSP_LOGCAT".ServiceProvider");
+    Category& log = Category::getInstance(SHIBSP_LOGCAT ".ServiceProvider");
 
     const Application* app = nullptr;
-    const Session* session = nullptr;
+    Session* session = nullptr;
+    Locker slocker;
     string targetURL = request.getRequestURL();
 
     try {
@@ -440,7 +452,9 @@ pair<bool,long> ServiceProvider::doExport(SPRequest& request, bool requireSessio
         app = &(request.getApplication());
 
         try {
-            session = request.getSession(false);
+            session = request.getSession(false, false, false);  // ignore timeout and do not cache
+            if (session)
+                slocker.assign(session, false); // assign to lock popper
         }
         catch (exception& e) {
             log.warn("unable to obtain session to export to request: %s", e.what());
@@ -591,7 +605,7 @@ pair<bool,long> ServiceProvider::doHandler(SPRequest& request) const
 #ifdef _DEBUG
     xmltooling::NDC ndc("doHandler");
 #endif
-    Category& log = Category::getInstance(SHIBSP_LOGCAT".ServiceProvider");
+    Category& log = Category::getInstance(SHIBSP_LOGCAT ".ServiceProvider");
 
     const Application* app = nullptr;
     string targetURL = request.getRequestURL();
@@ -660,12 +674,13 @@ pair<bool,long> ServiceProvider::doHandler(SPRequest& request) const
     }
     catch (exception& e) {
         request.log(SPRequest::SPError, e.what());
-        const Session* session = nullptr;
+        Session* session = nullptr;
         try {
-            session = request.getSession(false, true);
+            session = request.getSession(false, true, false);   // do not cache
         }
         catch (exception&) {
         }
+        Locker slocker(session, false); // pop existing lock on exit
         TemplateParameters tp(&e, nullptr, session);
         tp.m_map["requestURL"] = targetURL.substr(0, targetURL.find('?'));
         tp.m_request = &request;