From 01d4a70f0b95b2bac5a0d66e6fe87f4640433b90 Mon Sep 17 00:00:00 2001 From: Scott Cantor Date: Tue, 12 Dec 2006 17:58:17 +0000 Subject: [PATCH] Shift policy responsibility to ArtifactResolver, make msg rules more forgiving. --- saml/saml1/binding/impl/SAML1ArtifactDecoder.cpp | 2 +- saml/saml1/binding/impl/SAML1MessageRule.cpp | 11 ++++++----- saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp | 2 +- saml/saml2/binding/impl/SAML2MessageRule.cpp | 19 +++++++++---------- samltest/saml1/binding/SAML1ArtifactTest.h | 1 + samltest/saml2/binding/SAML2ArtifactTest.h | 1 + 6 files changed, 19 insertions(+), 17 deletions(-) diff --git a/saml/saml1/binding/impl/SAML1ArtifactDecoder.cpp b/saml/saml1/binding/impl/SAML1ArtifactDecoder.cpp index 23a9505..5435224 100644 --- 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(*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()); return response.release(); diff --git a/saml/saml1/binding/impl/SAML1MessageRule.cpp b/saml/saml1/binding/impl/SAML1MessageRule.cpp index 3d70d87..b54a12c 100644 --- 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); + + if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML1P_NS)) { + log.debug("not a SAML 1.x protocol message"); + return; + } + try { const RootObject& samlRoot = dynamic_cast(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. diff --git a/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp b/saml/saml2/binding/impl/SAML2ArtifactDecoder.cpp index db3e4b7..b108a5f 100644 --- 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(*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(); diff --git a/saml/saml2/binding/impl/SAML2MessageRule.cpp b/saml/saml2/binding/impl/SAML2MessageRule.cpp index fe604c2..8f2a666 100644 --- 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); + if (!XMLString::equals(q.getNamespaceURI(), samlconstants::SAML20P_NS)) { + log.debug("not a SAML 2.0 protocol message"); + return; + } + try { - const opensaml::RootObject& samlRoot = dynamic_cast(message); + const saml2::RootObject& samlRoot = dynamic_cast(message); 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"); - const saml2::RootObject& saml2Root = dynamic_cast(samlRoot); - Issuer* issuer = saml2Root.getIssuer(); + Issuer* issuer = samlRoot.getIssuer(); if (issuer && issuer->getName()) { auto_ptr 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. - const vector& assertions = dynamic_cast(saml2Root).getAssertions(); + const vector& assertions = dynamic_cast(samlRoot).getAssertions(); 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 --- a/samltest/saml1/binding/SAML1ArtifactTest.h +++ b/samltest/saml1/binding/SAML1ArtifactTest.h @@ -146,6 +146,7 @@ public: vector sigs(1,response->getSignature()); response->marshall((DOMDocument*)NULL,&sigs); SchemaValidators.validate(response.get()); + policy.evaluate(*(response.get()), this); return response.release(); } diff --git a/samltest/saml2/binding/SAML2ArtifactTest.h b/samltest/saml2/binding/SAML2ArtifactTest.h index 593df73..7add2f8 100644 --- a/samltest/saml2/binding/SAML2ArtifactTest.h +++ b/samltest/saml2/binding/SAML2ArtifactTest.h @@ -122,6 +122,7 @@ public: sc->setValue(StatusCode::SUCCESS); response->marshall(); SchemaValidators.validate(response.get()); + policy.evaluate(*(response.get()), this); return response.release(); } }; -- 2.1.4