Shift policy responsibility to ArtifactResolver, make msg rules more forgiving.

diff --git a/saml/saml1/binding/impl/SAML1ArtifactDecoder.cpp b/saml/saml1/binding/impl/SAML1ArtifactDecoder.cpp
index 23a9505..5435224 100644 (file)
--- a/saml/saml1/binding/impl/SAML1ArtifactDecoder.cpp
+++ b/saml/saml1/binding/impl/SAML1ArtifactDecoder.cpp
@@ -146,7 +146,7 @@ XMLObject* SAML1ArtifactDecoder::decode(
             m_artifactResolver->resolve(artifacts, dynamic_cast<const IDPSSODescriptor&>(*roledesc), policy)
             );
         
             m_artifactResolver->resolve(artifacts, dynamic_cast<const IDPSSODescriptor&>(*roledesc), policy)
             );
         

-        policy.evaluate(*(response.get()), &genericRequest);
+        // The policy should be enforced against the Response by the resolve step.

         
         for_each(artifacts.begin(), artifacts.end(), xmltooling::cleanup<SAMLArtifact>());
         return response.release();
         
         for_each(artifacts.begin(), artifacts.end(), xmltooling::cleanup<SAMLArtifact>());
         return response.release();

diff --git a/saml/saml1/binding/impl/SAML1MessageRule.cpp b/saml/saml1/binding/impl/SAML1MessageRule.cpp
index 3d70d87..b54a12c 100644 (file)
--- a/saml/saml1/binding/impl/SAML1MessageRule.cpp
+++ b/saml/saml1/binding/impl/SAML1MessageRule.cpp
@@ -53,17 +53,18 @@ void SAML1MessageRule::evaluate(const XMLObject& message, const GenericRequest*
     
     const QName& q = message.getElementQName(); 
     policy.setMessageQName(&q);
     
     const QName& q = message.getElementQName(); 
     policy.setMessageQName(&q);

+
+    if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML1P_NS)) {
+        log.debug("not a SAML 1.x protocol message");
+        return;
+    }
+

     
     try {
         const RootObject& samlRoot = dynamic_cast<const RootObject&>(message);
         policy.setMessageID(samlRoot.getID());
         policy.setIssueInstant(samlRoot.getIssueInstantEpoch());
 
     
     try {
         const RootObject& samlRoot = dynamic_cast<const RootObject&>(message);
         policy.setMessageID(samlRoot.getID());
         policy.setIssueInstant(samlRoot.getIssueInstantEpoch());
 

-        if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML1P_NS)) {
-            log.warn("not a SAML 1.x protocol message");
-            throw BindingException("Message was not a recognized SAML 1.x protocol element.");
-        }
-

         log.debug("extracting issuer from message");
 
         // Only samlp:Response is known to carry issuer (via payload) in standard SAML 1.x.
         log.debug("extracting issuer from message");
 
         // Only samlp:Response is known to carry issuer (via payload) in standard SAML 1.x.

diff --git a/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp b/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp
index db3e4b7..b108a5f 100644 (file)
--- a/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp
+++ b/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp
@@ -143,7 +143,7 @@ XMLObject* SAML2ArtifactDecoder::decode(
         m_artifactResolver->resolve(*(artifact2.get()), dynamic_cast<const SSODescriptorType&>(*roledesc), policy)
         );
     
         m_artifactResolver->resolve(*(artifact2.get()), dynamic_cast<const SSODescriptorType&>(*roledesc), policy)
         );
     

-    policy.evaluate(*(response.get()), &genericRequest);
+    // The policy should be enforced against the ArtifactResponse by the resolve step. 

 
     // Extract payload and check that message.
     XMLObject* payload = response->getPayload();
 
     // Extract payload and check that message.
     XMLObject* payload = response->getPayload();

diff --git a/saml/saml2/binding/impl/SAML2MessageRule.cpp b/saml/saml2/binding/impl/SAML2MessageRule.cpp
index fe604c2..8f2a666 100644 (file)
--- a/saml/saml2/binding/impl/SAML2MessageRule.cpp
+++ b/saml/saml2/binding/impl/SAML2MessageRule.cpp
@@ -52,27 +52,26 @@ void SAML2MessageRule::evaluate(const XMLObject& message, const GenericRequest*
     const QName& q = message.getElementQName(); 
     policy.setMessageQName(&q);
     
     const QName& q = message.getElementQName(); 
     policy.setMessageQName(&q);
     

+    if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML20P_NS)) {
+        log.debug("not a SAML 2.0 protocol message");
+        return;
+    }
+

     try {
     try {

-        const opensaml::RootObject& samlRoot = dynamic_cast<const opensaml::RootObject&>(message);
+        const saml2::RootObject& samlRoot = dynamic_cast<const saml2::RootObject&>(message);

         policy.setMessageID(samlRoot.getID());
         policy.setIssueInstant(samlRoot.getIssueInstantEpoch());
 
         policy.setMessageID(samlRoot.getID());
         policy.setIssueInstant(samlRoot.getIssueInstantEpoch());
 

-        if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML20P_NS)) {
-            log.warn("not a SAML 2.0 protocol message");
-            throw BindingException("Message was not a recognized SAML 2.0 protocol element.");
-        }
-

         log.debug("extracting issuer from message");
         log.debug("extracting issuer from message");

-        const saml2::RootObject& saml2Root = dynamic_cast<const saml2::RootObject&>(samlRoot);
-        Issuer* issuer = saml2Root.getIssuer();
+        Issuer* issuer = samlRoot.getIssuer();

         if (issuer && issuer->getName()) {
             auto_ptr<Issuer> copy(issuer->cloneIssuer());
             policy.setIssuer(copy.get());
             copy.release();
         }
         if (issuer && issuer->getName()) {
             auto_ptr<Issuer> copy(issuer->cloneIssuer());
             policy.setIssuer(copy.get());
             copy.release();
         }

-        else {
+        else if (XMLString::equals(q.getLocalPart(), Response::LOCAL_NAME)) {

             // No issuer in the message, so we have to try the Response approach. 
             // No issuer in the message, so we have to try the Response approach. 

-            const vector<Assertion*>& assertions = dynamic_cast<const Response&>(saml2Root).getAssertions();
+            const vector<Assertion*>& assertions = dynamic_cast<const Response&>(samlRoot).getAssertions();

             if (!assertions.empty()) {
                 issuer = assertions.front()->getIssuer();
                 if (issuer && issuer->getName()) {
             if (!assertions.empty()) {
                 issuer = assertions.front()->getIssuer();
                 if (issuer && issuer->getName()) {

diff --git a/samltest/saml1/binding/SAML1ArtifactTest.h b/samltest/saml1/binding/SAML1ArtifactTest.h
index 7ce89e2..05ea3cb 100644 (file)
--- a/samltest/saml1/binding/SAML1ArtifactTest.h
+++ b/samltest/saml1/binding/SAML1ArtifactTest.h
@@ -146,6 +146,7 @@ public:
         vector<Signature*> sigs(1,response->getSignature());\r
         response->marshall((DOMDocument*)NULL,&sigs);\r
         SchemaValidators.validate(response.get());\r
         vector<Signature*> sigs(1,response->getSignature());\r
         response->marshall((DOMDocument*)NULL,&sigs);\r
         SchemaValidators.validate(response.get());\r

+        policy.evaluate(*(response.get()), this);\r

         return response.release();\r
     }\r
 \r
         return response.release();\r
     }\r
 \r

diff --git a/samltest/saml2/binding/SAML2ArtifactTest.h b/samltest/saml2/binding/SAML2ArtifactTest.h
index 593df73..7add2f8 100644 (file)
--- a/samltest/saml2/binding/SAML2ArtifactTest.h
+++ b/samltest/saml2/binding/SAML2ArtifactTest.h
@@ -122,6 +122,7 @@ public:
         sc->setValue(StatusCode::SUCCESS);\r
         response->marshall();\r
         SchemaValidators.validate(response.get());\r
         sc->setValue(StatusCode::SUCCESS);\r
         response->marshall();\r
         SchemaValidators.validate(response.get());\r

+        policy.evaluate(*(response.get()), this);\r

         return response.release();\r
     }\r
 };\r
         return response.release();\r
     }\r
 };\r