diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLAssertionWriter.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLAssertionWriter.java index 2578b81706..c1b5b6fa95 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLAssertionWriter.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLAssertionWriter.java @@ -39,7 +39,6 @@ import org.keycloak.saml.common.constants.JBossSAMLConstants; import org.keycloak.saml.common.exceptions.ProcessingException; import org.keycloak.saml.common.util.StaxUtil; import org.keycloak.saml.processing.core.parsers.saml.assertion.SAMLAssertionQNames; -import org.w3c.dom.Element; import javax.xml.datatype.XMLGregorianCalendar; import javax.xml.namespace.QName; @@ -48,7 +47,6 @@ import java.net.URI; import java.util.List; import java.util.Set; -import javax.xml.crypto.dsig.XMLSignature; import static org.keycloak.saml.common.constants.JBossSAMLURIConstants.ASSERTION_NSURI; /** @@ -71,17 +69,8 @@ public class SAMLAssertionWriter extends BaseWriter { * @throws org.keycloak.saml.common.exceptions.ProcessingException */ public void write(AssertionType assertion) throws ProcessingException { - write(assertion, false); - } - - public void write(AssertionType assertion, boolean forceWriteDsigNamespace) throws ProcessingException { - Element sig = assertion.getSignature(); - StaxUtil.writeStartElement(writer, ASSERTION_PREFIX, JBossSAMLConstants.ASSERTION.get(), ASSERTION_NSURI.get()); StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, ASSERTION_NSURI.get()); - if (forceWriteDsigNamespace && sig != null && sig.getPrefix() != null && ! sig.hasAttribute("xmlns:" + sig.getPrefix())) { - StaxUtil.writeNameSpace(writer, sig.getPrefix(), XMLSignature.XMLNS); - } StaxUtil.writeDefaultNameSpace(writer, ASSERTION_NSURI.get()); // Attributes @@ -93,9 +82,6 @@ public class SAMLAssertionWriter extends BaseWriter { if (issuer != null) write(issuer, new QName(ASSERTION_NSURI.get(), JBossSAMLConstants.ISSUER.get(), ASSERTION_PREFIX)); - if (sig != null) - StaxUtil.writeDOMElement(writer, sig); - SubjectType subject = assertion.getSubject(); if (subject != null) { write(subject); diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java index a9e5fba2c1..1bc57f11d2 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/writers/SAMLResponseWriter.java @@ -64,17 +64,8 @@ public class SAMLResponseWriter extends BaseWriter { * @throws org.keycloak.saml.common.exceptions.ProcessingException */ public void write(ResponseType response) throws ProcessingException { - write(response, false); - } - - public void write(ResponseType response, boolean forceWriteDsigNamespace) throws ProcessingException { - Element sig = response.getSignature(); - StaxUtil.writeStartElement(writer, PROTOCOL_PREFIX, JBossSAMLConstants.RESPONSE__PROTOCOL.get(), JBossSAMLURIConstants.PROTOCOL_NSURI.get()); - if (forceWriteDsigNamespace && sig != null && sig.getPrefix() != null && ! sig.hasAttribute("xmlns:" + sig.getPrefix())) { - StaxUtil.writeNameSpace(writer, sig.getPrefix(), XMLSignature.XMLNS); - } StaxUtil.writeNameSpace(writer, PROTOCOL_PREFIX, JBossSAMLURIConstants.PROTOCOL_NSURI.get()); StaxUtil.writeNameSpace(writer, ASSERTION_PREFIX, JBossSAMLURIConstants.ASSERTION_NSURI.get()); @@ -85,9 +76,6 @@ public class SAMLResponseWriter extends BaseWriter { write(issuer, new QName(JBossSAMLURIConstants.ASSERTION_NSURI.get(), JBossSAMLConstants.ISSUER.get(), ASSERTION_PREFIX)); } - if (sig != null) { - StaxUtil.writeDOMElement(writer, sig); - } ExtensionsType extensions = response.getExtensions(); if (extensions != null && extensions.getAny() != null && ! extensions.getAny().isEmpty()) { write(extensions); @@ -101,7 +89,7 @@ public class SAMLResponseWriter extends BaseWriter { for (ResponseType.RTChoiceType choiceType : choiceTypes) { AssertionType assertion = choiceType.getAssertion(); if (assertion != null) { - assertionWriter.write(assertion, forceWriteDsigNamespace); + assertionWriter.write(assertion); } EncryptedAssertionType encryptedAssertion = choiceType.getEncryptedAssertion(); diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLDataMarshaller.java b/services/src/main/java/org/keycloak/broker/saml/SAMLDataMarshaller.java index 532c2841ac..c18104edb8 100644 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLDataMarshaller.java +++ b/services/src/main/java/org/keycloak/broker/saml/SAMLDataMarshaller.java @@ -49,11 +49,11 @@ public class SAMLDataMarshaller extends DefaultDataMarshaller { if (obj instanceof ResponseType) { ResponseType responseType = (ResponseType) obj; SAMLResponseWriter samlWriter = new SAMLResponseWriter(StaxUtil.getXMLStreamWriter(bos)); - samlWriter.write(responseType, true); + samlWriter.write(responseType); } else if (obj instanceof AssertionType) { AssertionType assertion = (AssertionType) obj; SAMLAssertionWriter samlWriter = new SAMLAssertionWriter(StaxUtil.getXMLStreamWriter(bos)); - samlWriter.write(assertion, true); + samlWriter.write(assertion); } else if (obj instanceof AuthnStatementType) { AuthnStatementType authnStatement = (AuthnStatementType) obj; SAMLAssertionWriter samlWriter = new SAMLAssertionWriter(StaxUtil.getXMLStreamWriter(bos)); diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/ModifySamlResponseStepBuilder.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/ModifySamlResponseStepBuilder.java index 5c80786416..9a11484435 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/ModifySamlResponseStepBuilder.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/ModifySamlResponseStepBuilder.java @@ -149,7 +149,11 @@ public class ModifySamlResponseStepBuilder extends SamlDocumentStepBuilder sb = new BaseSAML2BindingBuilder(); + try { + KeyPair keyPair = new KeyPair(SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY_PK, SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY_PK); + sb.signWith("kn", keyPair) + .signatureAlgorithm(RSA_SHA1) + .signAssertions() + .signAssertion(doc); + } catch (ProcessingException ex) { + throw new RuntimeException(ex); + } + + // KeyInfo has lax and can contain custom elements, see https://www.w3.org/TR/xmldsig-core1/#sec-KeyInfo + Element el = findFirstElement(doc, XmlDSigQNames.KEY_INFO); + appendNewElement(el, new QName(NS_VETINARI, "Patrician"), XMLNS_VETINARI); + } + + private static Element findFirstElement(Document doc, HasQName qName) { + NodeList nl = doc.getElementsByTagNameNS(qName.getQName().getNamespaceURI(), qName.getQName().getLocalPart()); + return (nl == null || nl.getLength() == 0) ? null : (Element) nl.item(0); + } + + @Test + public void testAnyNamespacePreservedInContext() throws IOException { + final RealmResource realm = adminClient.realm(REALM_NAME); + + try (IdentityProviderCreator idp = new IdentityProviderCreator(realm, addIdentityProvider("https://saml.idp/"))) { + new SamlClientBuilder() + .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, POST) + .build() + .login().idp(SAML_BROKER_ALIAS).build() + // Virtually perform login at IdP (return artificial SAML response) + .processSamlResponse(REDIRECT) + .transformObject(this::createAuthnResponse) + .transformDocument(BrokerTest::signAndAddCustomNamespaceElementToSignature) + .targetAttributeSamlResponse() + .targetUri(getSamlBrokerUrl(REALM_NAME)) + .targetBinding(POST) + .build() + .assertResponse(org.keycloak.testsuite.util.Matchers.statusCodeIsHC(Status.OK)) + .execute(); + } + } + @Test public void testExpiredAssertion() throws Exception { XMLGregorianCalendar now = XMLTimeUtil.getIssueInstant();