Add min macro for Solaris.
[shibboleth/sp.git] / adfs / adfs.cpp
index 3c0fdff..b900bdc 100644 (file)
 # include <saml/saml2/core/Assertions.h>
 # include <saml/saml2/metadata/Metadata.h>
 # include <saml/saml2/metadata/EndpointManager.h>
+# include <saml/saml2/profile/AssertionValidator.h>
 # include <xmltooling/impl/AnyElement.h>
 # include <xmltooling/validation/ValidatorSuite.h>
 using namespace opensaml::saml2md;
+# ifndef min
+#  define min(a,b)            (((a) < (b)) ? (a) : (b))
+# endif
 #endif
 using namespace shibsp;
 using namespace opensaml;
@@ -128,7 +132,7 @@ namespace {
         }
 
         void receive(DDF& in, ostream& out);
-        pair<bool,long> run(SPRequest& request, const char* entityID=NULL, bool isHandler=true) const;
+        pair<bool,long> run(SPRequest& request, string& entityID, bool isHandler=true) const;
 
     private:
         pair<bool,long> doRequest(
@@ -136,6 +140,7 @@ namespace {
             HTTPResponse& httpResponse,
             const char* entityID,
             const char* acsLocation,
+            const char* authnContextClassRef,
             string& relayState
             ) const;
         string m_appId;
@@ -162,9 +167,10 @@ namespace {
         auto_ptr_XMLCh m_protocol;
 
     private:
-        string implementProtocol(
+        void implementProtocol(
             const Application& application,
             const HTTPRequest& httpRequest,
+            HTTPResponse& httpResponse,
             SecurityPolicy& policy,
             const PropertySet* settings,
             const XMLObject& xmlObject
@@ -208,9 +214,7 @@ namespace {
 #endif
 
     private:
-        pair<bool,long> doRequest(
-            const Application& application, const char* requestURL, const char* entityID, HTTPResponse& httpResponse
-            ) const;
+        pair<bool,long> doRequest(const Application& application, const char* entityID, HTTPResponse& httpResponse) const;
 
         string m_appId;
         auto_ptr_XMLCh m_binding;
@@ -308,15 +312,16 @@ extern "C" void ADFS_EXPORTS xmltooling_extension_term()
     */
 }
 
-pair<bool,long> ADFSSessionInitiator::run(SPRequest& request, const char* entityID, bool isHandler) const
+pair<bool,long> ADFSSessionInitiator::run(SPRequest& request, string& entityID, bool isHandler) const
 {
     // We have to know the IdP to function.
-    if (!entityID || !*entityID)
-        return make_pair(false,0);
+    if (entityID.empty())
+        return make_pair(false,0L);
 
     string target;
     const Handler* ACS=NULL;
     const char* option;
+    pair<bool,const char*> acClass;
     const Application& app=request.getApplication();
 
     if (isHandler) {
@@ -326,38 +331,46 @@ pair<bool,long> ADFSSessionInitiator::run(SPRequest& request, const char* entity
 
         // Since we're passing the ACS by value, we need to compute the return URL,
         // so we'll need the target resource for real.
-        recoverRelayState(request.getApplication(), request, target, false);
+        recoverRelayState(request.getApplication(), request, request, target, false);
+
+        if (acClass.second = request.getParameter("authnContextClassRef"))
+            acClass.first = true;
+        else
+            acClass = getString("authnContextClassRef");
     }
     else {
         // We're running as a "virtual handler" from within the filter.
         // The target resource is the current one and everything else is defaulted.
         target=request.getRequestURL();
+
+        const PropertySet* settings = request.getRequestSettings().first;
+        acClass = settings->getString("authnContextClassRef");
+        if (!acClass.first)
+            acClass = getString("authnContextClassRef");
     }
 
     // Since we're not passing by index, we need to fully compute the return URL.
-    if (!ACS) {
-        // Get all the ADFS endpoints.
-        const vector<const Handler*>& handlers = app.getAssertionConsumerServicesByBinding(m_binding.get());
-
-        // Index comes from request, or default set in the handler, or we just pick the first endpoint.
-        pair<bool,unsigned int> index = make_pair(false,0);
-        if (isHandler) {
-            option = request.getParameter("acsIndex");
-            if (option)
-                index = make_pair(true, atoi(option));
-        }
-        if (!index.first)
-            index = getUnsignedInt("defaultACSIndex");
-        if (index.first) {
-            for (vector<const Handler*>::const_iterator h = handlers.begin(); !ACS && h!=handlers.end(); ++h) {
-                if (index.second == (*h)->getUnsignedInt("index").second)
-                    ACS = *h;
-            }
-        }
-        else if (!handlers.empty()) {
-            ACS = handlers.front();
+    // Get all the ADFS endpoints.
+    const vector<const Handler*>& handlers = app.getAssertionConsumerServicesByBinding(m_binding.get());
+
+    // Index comes from request, or default set in the handler, or we just pick the first endpoint.
+    pair<bool,unsigned int> index(false,0);
+    if (isHandler) {
+        option = request.getParameter("acsIndex");
+        if (option)
+            index = pair<bool,unsigned int>(true, atoi(option));
+    }
+    if (!index.first)
+        index = getUnsignedInt("defaultACSIndex");
+    if (index.first) {
+        for (vector<const Handler*>::const_iterator h = handlers.begin(); !ACS && h!=handlers.end(); ++h) {
+            if (index.second == (*h)->getUnsignedInt("index").second)
+                ACS = *h;
         }
     }
+    else if (!handlers.empty()) {
+        ACS = handlers.front();
+    }
     if (!ACS)
         throw ConfigurationException("Unable to locate ADFS response endpoint.");
 
@@ -375,19 +388,21 @@ pair<bool,long> ADFSSessionInitiator::run(SPRequest& request, const char* entity
             target = option;
     }
 
-    m_log.debug("attempting to initiate session using ADFS with provider (%s)", entityID);
+    m_log.debug("attempting to initiate session using ADFS with provider (%s)", entityID.c_str());
 
     if (SPConfig::getConfig().isEnabled(SPConfig::OutOfProcess))
-        return doRequest(app, request, entityID, ACSloc.c_str(), target);
+        return doRequest(app, request, entityID.c_str(), ACSloc.c_str(), (acClass.first ? acClass.second : NULL), target);
 
     // Remote the call.
     DDF out,in = DDF(m_address.c_str()).structure();
     DDFJanitor jin(in), jout(out);
     in.addmember("application_id").string(app.getId());
-    in.addmember("entity_id").string(entityID);
+    in.addmember("entity_id").string(entityID.c_str());
     in.addmember("acsLocation").string(ACSloc.c_str());
     if (!target.empty())
         in.addmember("RelayState").string(target.c_str());
+    if (acClass.first)
+        in.addmember("authnContextClassRef").string(acClass.second);
 
     // Remote the processing.
     out = request.getServiceProvider().getListenerService()->send(in);
@@ -421,7 +436,7 @@ void ADFSSessionInitiator::receive(DDF& in, ostream& out)
     // Since we're remoted, the result should either be a throw, which we pass on,
     // a false/0 return, which we just return as an empty structure, or a response/redirect,
     // which we capture in the facade and send back.
-    doRequest(*app, *http.get(), entityID, acsLocation, relayState);
+    doRequest(*app, *http.get(), entityID, acsLocation, in["authnContextClassRef"].string(), relayState);
     out << ret;
 }
 
@@ -430,6 +445,7 @@ pair<bool,long> ADFSSessionInitiator::doRequest(
     HTTPResponse& httpResponse,
     const char* entityID,
     const char* acsLocation,
+    const char* authnContextClassRef,
     string& relayState
     ) const
 {
@@ -440,19 +456,23 @@ pair<bool,long> ADFSSessionInitiator::doRequest(
     MetadataProvider::Criteria mc(entityID, &IDPSSODescriptor::ELEMENT_QNAME, m_binding.get());
     pair<const EntityDescriptor*,const RoleDescriptor*> entity=m->getEntityDescriptor(mc);
     if (!entity.first) {
-        m_log.error("unable to locate metadata for provider (%s)", entityID);
+        m_log.warn("unable to locate metadata for provider (%s)", entityID);
         throw MetadataException("Unable to locate metadata for identity provider ($entityID)", namedparams(1, "entityID", entityID));
     }
     else if (!entity.second) {
-        m_log.error("unable to locate ADFS-aware identity provider role for provider (%s)", entityID);
-        return make_pair(false,0);
+        m_log.warn("unable to locate ADFS-aware identity provider role for provider (%s)", entityID);
+        if (getParent())
+            return make_pair(false,0L);
+        throw MetadataException("Unable to locate ADFS-aware identity provider role for provider ($entityID)", namedparams(1, "entityID", entityID));
     }
     const EndpointType* ep = EndpointManager<SingleSignOnService>(
         dynamic_cast<const IDPSSODescriptor*>(entity.second)->getSingleSignOnServices()
         ).getByBinding(m_binding.get());
     if (!ep) {
-        m_log.error("unable to locate compatible SSO service for provider (%s)", entityID);
-        return make_pair(false,0);
+        m_log.warn("unable to locate compatible SSO service for provider (%s)", entityID);
+        if (getParent())
+            return make_pair(false,0L);
+        throw MetadataException("Unable to locate compatible SSO service for provider ($entityID)", namedparams(1, "entityID", entityID));
     }
 
     preserveRelayState(app, httpResponse, relayState);
@@ -473,12 +493,14 @@ pair<bool,long> ADFSSessionInitiator::doRequest(
 
     string req=string(dest.get()) + (strchr(dest.get(),'?') ? '&' : '?') + "wa=wsignin1.0&wreply=" + urlenc->encode(acsLocation) +
         "&wct=" + urlenc->encode(timebuf) + "&wtrealm=" + urlenc->encode(app.getString("entityID").second);
+    if (authnContextClassRef)
+        req += "&wauth=" + urlenc->encode(authnContextClassRef);
     if (!relayState.empty())
         req += "&wctx=" + urlenc->encode(relayState.c_str());
 
     return make_pair(true, httpResponse.sendRedirect(req.c_str()));
 #else
-    return make_pair(false,0);
+    return make_pair(false,0L);
 #endif
 }
 
@@ -530,9 +552,10 @@ XMLObject* ADFSDecoder::decode(string& relayState, const GenericRequest& generic
     return xmlObject.release();
 }
 
-string ADFSConsumer::implementProtocol(
+void ADFSConsumer::implementProtocol(
     const Application& application,
     const HTTPRequest& httpRequest,
+    HTTPResponse& httpResponse,
     SecurityPolicy& policy,
     const PropertySet* settings,
     const XMLObject& xmlObject
@@ -549,9 +572,9 @@ string ADFSConsumer::implementProtocol(
     response = dynamic_cast<const ElementProxy*>(response->getUnknownXMLObjects().front());
     if (!response || !response->hasChildren())
         throw FatalProfileException("Token wrapper element did not contain a security token.");
-    const saml1::Assertion* token = dynamic_cast<const saml1::Assertion*>(response->getUnknownXMLObjects().front());
+    const Assertion* token = dynamic_cast<const Assertion*>(response->getUnknownXMLObjects().front());
     if (!token || !token->getSignature())
-        throw FatalProfileException("Incoming message did not contain a signed SAML 1.1 assertion.");
+        throw FatalProfileException("Incoming message did not contain a signed SAML assertion.");
 
     // Extract message and issuer details from assertion.
     extractMessageDetails(*token, m_protocol.get(), policy);
@@ -564,59 +587,121 @@ string ADFSConsumer::implementProtocol(
     if (!policy.isAuthenticated())
         throw SecurityPolicyException("Unable to establish security of incoming assertion.");
 
-    // Now do profile and core semantic validation to ensure we can use it for SSO.
-    // Profile validator.
     time_t now = time(NULL);
-    saml1::AssertionValidator ssoValidator(application.getAudiences(), now);
-    ssoValidator.validateAssertion(*token);
-    if (!token->getConditions() || !token->getConditions()->getNotBefore() || !token->getConditions()->getNotOnOrAfter())
-        throw FatalProfileException("Assertion did not contain time conditions.");
-    else if (token->getAuthenticationStatements().empty())
-        throw FatalProfileException("Assertion did not contain an authentication statement.");
-
-    // With ADFS, we only have one token, but we need to put it in a vector.
-    vector<const Assertion*> tokens(1,token);
-    const saml1::AuthenticationStatement* ssoStatement=token->getAuthenticationStatements().front();
-
-    // Address checking.
-    saml1::SubjectLocality* locality = ssoStatement->getSubjectLocality();
-    if (locality && locality->getIPAddress()) {
-        auto_ptr_char ip(locality->getIPAddress());
-        checkAddress(application, httpRequest, ip.get());
-    }
+    
+    const PropertySet* sessionProps = application.getPropertySet("Sessions");
+    const EntityDescriptor* entity = policy.getIssuerMetadata() ? dynamic_cast<const EntityDescriptor*>(policy.getIssuerMetadata()->getParent()) : NULL;
 
-    m_log.debug("ADFS profile processing completed successfully");
+    saml1::NameIdentifier* saml1name=NULL;
+    saml2::NameID* saml2name=NULL;
+    const XMLCh* authMethod=NULL;
+    const XMLCh* authInstant=NULL;
+    time_t sessionExp = 0;
+    
+    const saml1::Assertion* saml1token = dynamic_cast<const saml1::Assertion*>(token);
+    if (saml1token) {
+        // Now do profile and core semantic validation to ensure we can use it for SSO.
+        saml1::AssertionValidator ssoValidator(application.getRelyingParty(entity)->getXMLString("entityID").second, application.getAudiences(), now);
+        ssoValidator.validateAssertion(*saml1token);
+        if (!saml1token->getConditions() || !saml1token->getConditions()->getNotBefore() || !saml1token->getConditions()->getNotOnOrAfter())
+            throw FatalProfileException("Assertion did not contain time conditions.");
+        else if (saml1token->getAuthenticationStatements().empty())
+            throw FatalProfileException("Assertion did not contain an authentication statement.");
+        
+        // authnskew allows rejection of SSO if AuthnInstant is too old.
+        pair<bool,unsigned int> authnskew = sessionProps ? sessionProps->getUnsignedInt("maxTimeSinceAuthn") : pair<bool,unsigned int>(false,0);
+
+        const saml1::AuthenticationStatement* ssoStatement=saml1token->getAuthenticationStatements().front();
+        if (authnskew.first && authnskew.second &&
+                ssoStatement->getAuthenticationInstant() && (now - ssoStatement->getAuthenticationInstantEpoch() > authnskew.second))
+            throw FatalProfileException("The gap between now and the time you logged into your identity provider exceeds the limit.");
+
+        // Address checking.
+        saml1::SubjectLocality* locality = ssoStatement->getSubjectLocality();
+        if (locality && locality->getIPAddress()) {
+            auto_ptr_char ip(locality->getIPAddress());
+            checkAddress(application, httpRequest, ip.get());
+        }
 
-    saml1::NameIdentifier* n = ssoStatement->getSubject()->getNameIdentifier();
+        saml1name = ssoStatement->getSubject()->getNameIdentifier();
+        authMethod = ssoStatement->getAuthenticationMethod();
+        if (ssoStatement->getAuthenticationInstant())
+            authInstant = ssoStatement->getAuthenticationInstant()->getRawData();
 
-    // Now we have to extract the authentication details for attribute and session setup.
+        // Session expiration.
+        pair<bool,unsigned int> lifetime = sessionProps ? sessionProps->getUnsignedInt("lifetime") : pair<bool,unsigned int>(true,28800);
+        if (!lifetime.first || lifetime.second == 0)
+            lifetime.second = 28800;
+        sessionExp = now + lifetime.second;
+    }
+    else {
+        const saml2::Assertion* saml2token = dynamic_cast<const saml2::Assertion*>(token);
+        if (!saml2token)
+            throw FatalProfileException("Incoming message did not contain a recognized type of SAML assertion.");
+
+        // Now do profile and core semantic validation to ensure we can use it for SSO.
+        saml2::AssertionValidator ssoValidator(application.getRelyingParty(entity)->getXMLString("entityID").second, application.getAudiences(), now);
+        ssoValidator.validateAssertion(*saml2token);
+        if (!saml2token->getConditions() || !saml2token->getConditions()->getNotBefore() || !saml2token->getConditions()->getNotOnOrAfter())
+            throw FatalProfileException("Assertion did not contain time conditions.");
+        else if (saml2token->getAuthnStatements().empty())
+            throw FatalProfileException("Assertion did not contain an authentication statement.");
+        
+        // authnskew allows rejection of SSO if AuthnInstant is too old.
+        pair<bool,unsigned int> authnskew = sessionProps ? sessionProps->getUnsignedInt("maxTimeSinceAuthn") : pair<bool,unsigned int>(false,0);
+
+        const saml2::AuthnStatement* ssoStatement=saml2token->getAuthnStatements().front();
+        if (authnskew.first && authnskew.second &&
+                ssoStatement->getAuthnInstant() && (now - ssoStatement->getAuthnInstantEpoch() > authnskew.second))
+            throw FatalProfileException("The gap between now and the time you logged into your identity provider exceeds the limit.");
+
+        // Address checking.
+        saml2::SubjectLocality* locality = ssoStatement->getSubjectLocality();
+        if (locality && locality->getAddress()) {
+            auto_ptr_char ip(locality->getAddress());
+            checkAddress(application, httpRequest, ip.get());
+        }
 
-    // Session expiration for ADFS is purely SP-driven, and the method is mapped to a ctx class.
-    const PropertySet* sessionProps = application.getPropertySet("Sessions");
-    pair<bool,unsigned int> lifetime = sessionProps ? sessionProps->getUnsignedInt("lifetime") : pair<bool,unsigned int>(true,28800);
-    if (!lifetime.first || lifetime.second == 0)
-        lifetime.second = 28800;
+        saml2name = saml2token->getSubject() ? saml2token->getSubject()->getNameID() : NULL;
+        if (ssoStatement->getAuthnContext() && ssoStatement->getAuthnContext()->getAuthnContextClassRef())
+            authMethod = ssoStatement->getAuthnContext()->getAuthnContextClassRef()->getReference();
+        if (ssoStatement->getAuthnInstant())
+            authInstant = ssoStatement->getAuthnInstant()->getRawData();
+
+        // Session expiration for SAML 2.0 is jointly IdP- and SP-driven.
+        sessionExp = ssoStatement->getSessionNotOnOrAfter() ? ssoStatement->getSessionNotOnOrAfterEpoch() : 0;
+        pair<bool,unsigned int> lifetime = sessionProps ? sessionProps->getUnsignedInt("lifetime") : pair<bool,unsigned int>(true,28800);
+        if (!lifetime.first || lifetime.second == 0)
+            lifetime.second = 28800;
+        if (sessionExp == 0)
+            sessionExp = now + lifetime.second;     // IdP says nothing, calulate based on SP.
+        else
+            sessionExp = min(sessionExp, now + lifetime.second);    // Use the lowest.
+    }
+    
+    m_log.debug("ADFS profile processing completed successfully");
 
     // We've successfully "accepted" the SSO token.
     // To complete processing, we need to extract and resolve attributes and then create the session.
 
-    // Normalize the SAML 1.x NameIdentifier...
-    auto_ptr<saml2::NameID> nameid(n ? saml2::NameIDBuilder::buildNameID() : NULL);
-    if (n) {
-        nameid->setName(n->getName());
-        nameid->setFormat(n->getFormat());
-        nameid->setNameQualifier(n->getNameQualifier());
+    // Normalize a SAML 1.x NameIdentifier...
+    auto_ptr<saml2::NameID> nameid(saml1name ? saml2::NameIDBuilder::buildNameID() : NULL);
+    if (saml1name) {
+        nameid->setName(saml1name->getName());
+        nameid->setFormat(saml1name->getFormat());
+        nameid->setNameQualifier(saml1name->getNameQualifier());
     }
 
     // The context will handle deleting attributes and new tokens.
-        auto_ptr<ResolutionContext> ctx(
+    vector<const Assertion*> tokens(1,token);
+    auto_ptr<ResolutionContext> ctx(
         resolveAttributes(
             application,
             policy.getIssuerMetadata(),
             m_protocol.get(),
-            n,
-            nameid.get(),
-            ssoStatement->getAuthenticationMethod(),
+            saml1name,
+            (saml1name ? nameid.get() : saml2name),
+            authMethod,
             NULL,
             &tokens
             )
@@ -627,16 +712,17 @@ string ADFSConsumer::implementProtocol(
         tokens.insert(tokens.end(), ctx->getResolvedAssertions().begin(), ctx->getResolvedAssertions().end());
     }
 
-    return application.getServiceProvider().getSessionCache()->insert(
-        now + lifetime.second,
+    application.getServiceProvider().getSessionCache()->insert(
         application,
-        httpRequest.getRemoteAddr().c_str(),
-        policy.getIssuerMetadata() ? dynamic_cast<const EntityDescriptor*>(policy.getIssuerMetadata()->getParent()) : NULL,
+        httpRequest,
+        httpResponse,
+        sessionExp,
+        entity,
         m_protocol.get(),
-        nameid.get(),
-        ssoStatement->getAuthenticationInstant() ? ssoStatement->getAuthenticationInstant()->getRawData() : NULL,
+        (saml1name ? nameid.get() : saml2name),
+        authInstant,
         NULL,
-        ssoStatement->getAuthenticationMethod(),
+        authMethod,
         NULL,
         &tokens,
         ctx.get() ? &ctx->getResolvedAttributes() : NULL
@@ -657,17 +743,17 @@ pair<bool,long> ADFSLogoutInitiator::run(SPRequest& request, bool isHandler) con
     try {
         session = request.getSession(false, true, false);  // don't cache it and ignore all checks
         if (!session)
-            return make_pair(false,0);
+            return make_pair(false,0L);
 
         // We only handle ADFS sessions.
         if (!XMLString::equals(session->getProtocol(), WSFED_NS) || !session->getEntityID()) {
             session->unlock();
-            return make_pair(false,0);
+            return make_pair(false,0L);
         }
     }
     catch (exception& ex) {
         m_log.error("error accessing current session: %s", ex.what());
-        return make_pair(false,0);
+        return make_pair(false,0L);
     }
 
     string entityID(session->getEntityID());
@@ -675,7 +761,7 @@ pair<bool,long> ADFSLogoutInitiator::run(SPRequest& request, bool isHandler) con
 
     if (SPConfig::getConfig().isEnabled(SPConfig::OutOfProcess)) {
         // When out of process, we run natively.
-        return doRequest(request.getApplication(), request.getRequestURL(), entityID.c_str(), request);
+        return doRequest(request.getApplication(), entityID.c_str(), request);
     }
     else {
         // When not out of process, we remote the request.
@@ -683,7 +769,6 @@ pair<bool,long> ADFSLogoutInitiator::run(SPRequest& request, bool isHandler) con
         DDF out,in(m_address.c_str());
         DDFJanitor jin(in), jout(out);
         in.addmember("application_id").string(request.getApplication().getId());
-        in.addmember("url").string(request.getRequestURL());
         in.addmember("entity_id").string(entityID.c_str());
         out=request.getServiceProvider().getListenerService()->send(in);
         return unwrap(request, out);
@@ -710,7 +795,7 @@ void ADFSLogoutInitiator::receive(DDF& in, ostream& out)
     // Since we're remoted, the result should either be a throw, which we pass on,
     // a false/0 return, which we just return as an empty structure, or a response/redirect,
     // which we capture in the facade and send back.
-    doRequest(*app, in["url"].string(), in["entity_id"].string(), *resp.get());
+    doRequest(*app, in["entity_id"].string(), *resp.get());
 
     out << ret;
 #else
@@ -718,9 +803,7 @@ void ADFSLogoutInitiator::receive(DDF& in, ostream& out)
 #endif
 }
 
-pair<bool,long> ADFSLogoutInitiator::doRequest(
-    const Application& application, const char* requestURL, const char* entityID, HTTPResponse& response
-    ) const
+pair<bool,long> ADFSLogoutInitiator::doRequest(const Application& application, const char* entityID, HTTPResponse& response) const
 {
 #ifndef SHIBSP_LITE
     try {
@@ -756,7 +839,7 @@ pair<bool,long> ADFSLogoutInitiator::doRequest(
         m_log.error("error issuing ADFS logout request: %s", ex.what());
     }
 
-    return make_pair(false,0);
+    return make_pair(false,0L);
 #else
     throw ConfigurationException("Cannot perform logout using lite version of shibsp library.");
 #endif
@@ -787,10 +870,6 @@ pair<bool,long> ADFSLogout::run(SPRequest& request, bool isHandler) const
     param = request.getParameter("wreply");
     const Application& app = request.getApplication();
 
-    // Get the session_id.
-    pair<string,const char*> shib_cookie = app.getCookieNameProps("_shibsession_");
-    const char* session_id = request.getCookie(shib_cookie.first.c_str());
-
     if (!returning) {
         // Pass control to the first front channel notification point, if any.
         map<string,string> parammap;
@@ -802,19 +881,19 @@ pair<bool,long> ADFSLogout::run(SPRequest& request, bool isHandler) const
     }
 
     // Best effort on back channel and to remove the user agent's session.
-    if (session_id) {
+    string session_id = app.getServiceProvider().getSessionCache()->active(app, request);
+    if (!session_id.empty()) {
         vector<string> sessions(1,session_id);
         notifyBackChannel(app, request.getRequestURL(), sessions, false);
         try {
-            app.getServiceProvider().getSessionCache()->remove(session_id, app);
+            app.getServiceProvider().getSessionCache()->remove(app, request, &request);
         }
         catch (exception& ex) {
-            m_log.error("error removing session (%s): %s", session_id, ex.what());
+            m_log.error("error removing session (%s): %s", session_id.c_str(), ex.what());
         }
-        request.setCookie(shib_cookie.first.c_str(), shib_cookie.second);
     }
 
     if (param)
         return make_pair(true, request.sendRedirect(param));
-    return sendLogoutPage(app, request, false, "Logout complete.");
+    return sendLogoutPage(app, request, request, false, "Logout complete.");
 }