Skip to content

Commit

Permalink
KEYCLOAK-19866 Fix user-defined- and xml-fragment-parsing/Add XPathAt…
Browse files Browse the repository at this point in the history 
…tributeMapper
  • Loading branch information
@KnauerSecunet @hmlnarik
KnauerSecunet authored and hmlnarik committed Aug 3, 2022
1 parent 0d3ca43 commit 21f7006
Show file tree
Hide file tree
Showing 9 changed files with 732 additions and 58 deletions.
74 changes: 57 additions & 17 deletions ...va/org/keycloak/saml/processing/core/parsers/saml/assertion/SAMLAttributeValueParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@
*/
package org.keycloak.saml.processing.core.parsers.saml.assertion;

import java.util.Deque;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.Map;
import javax.xml.stream.XMLEventFactory;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.events.Namespace;
import org.keycloak.saml.common.PicketLinkLogger;
import org.keycloak.saml.common.PicketLinkLoggerFactory;
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
Expand Down Expand Up @@ -46,6 +54,8 @@ public class SAMLAttributeValueParser implements StaxParser {
private static final QName NIL = new QName(JBossSAMLURIConstants.XSI_NSURI.get(), "nil", JBossSAMLURIConstants.XSI_PREFIX.get());
private static final QName XSI_TYPE = new QName(JBossSAMLURIConstants.XSI_NSURI.get(), "type", JBossSAMLURIConstants.XSI_PREFIX.get());

private static final ThreadLocal<XMLEventFactory> XML_EVENT_FACTORY = ThreadLocal.withInitial(XMLEventFactory::newInstance);

public static SAMLAttributeValueParser getInstance() {
return INSTANCE;
}
Expand Down Expand Up @@ -88,15 +98,15 @@ public Object parse(XMLEventReader xmlEventReader) throws ParsingException {
}

// when no type attribute assigned -> assume anyType
return parseAnyTypeAsString(xmlEventReader);
return parseAsString(xmlEventReader);
}

// RK Added an additional type check for base64Binary type as calheers is passing this type
String typeValue = StaxParserUtil.getAttributeValue(type);
if (typeValue.contains(":string")) {
return StaxParserUtil.getElementText(xmlEventReader);
} else if (typeValue.contains(":anyType")) {
return parseAnyTypeAsString(xmlEventReader);
return parseAsString(xmlEventReader);
} else if(typeValue.contains(":base64Binary")){
return StaxParserUtil.getElementText(xmlEventReader);
} else if(typeValue.contains(":date")){
Expand All @@ -105,33 +115,29 @@ public Object parse(XMLEventReader xmlEventReader) throws ParsingException {
return StaxParserUtil.getElementText(xmlEventReader);
}

// KEYCLOAK-18417: Simply ignore unknown types
logger.debug("Skipping attribute value of unsupported type " + typeValue);
StaxParserUtil.bypassElementBlock(xmlEventReader);
return null;
return parseAsString(xmlEventReader);
}

public static String parseAnyTypeAsString(XMLEventReader xmlEventReader) throws ParsingException {
private static String parseAsString(XMLEventReader xmlEventReader) throws ParsingException {
try {
XMLEvent event = xmlEventReader.peek();
if (event.isStartElement()) {
event = xmlEventReader.nextTag();
if (xmlEventReader.peek().isStartElement()) {
StringWriter sw = new StringWriter();
XMLEventWriter writer = XMLOutputFactory.newInstance().createXMLEventWriter(sw);
//QName tagName = event.asStartElement().getName();
int tagLevel = 1;
do {
Deque<Map<String, String>> definedNamespaces = new LinkedList<>();
int tagLevel = 0;
while (xmlEventReader.hasNext() && (tagLevel > 0 || !xmlEventReader.peek().isEndElement())) {
XMLEvent event = (XMLEvent) xmlEventReader.next();
writer.add(event);
event = (XMLEvent) xmlEventReader.next();
if (event.isStartElement()) {
definedNamespaces.push(addNamespaceWhenMissing(definedNamespaces, writer, event.asStartElement()));
tagLevel++;
}
if (event.isEndElement()) {
definedNamespaces.pop();
tagLevel--;
}
} while (xmlEventReader.hasNext() && tagLevel > 0);
writer.add(event);
writer.flush();
}
writer.close();
return sw.toString();
} else {
return StaxParserUtil.getElementText(xmlEventReader);
Expand All @@ -141,4 +147,38 @@ public static String parseAnyTypeAsString(XMLEventReader xmlEventReader) throws
}
}

private static Map<String, String> addNamespaceWhenMissing(Deque<Map<String, String>> definedNamespaces, XMLEventWriter writer,
StartElement startElement) throws XMLStreamException {

final Map<String, String> necessaryNamespaces = new HashMap<>();
// Namespace in tag
if (startElement.getName().getPrefix() != null && !startElement.getName().getPrefix().isEmpty()) {
necessaryNamespaces.put(startElement.getName().getPrefix(), startElement.getName().getNamespaceURI());
}
// Namespaces in attributes
final Iterator<Attribute> attributes = startElement.getAttributes();
while (attributes.hasNext()) {
final Attribute attribute = attributes.next();
if (attribute.getName().getPrefix() != null && !attribute.getName().getPrefix().isEmpty()) {
necessaryNamespaces.put(attribute.getName().getPrefix(), attribute.getName().getNamespaceURI());
}
}

// Already contained in stack
necessaryNamespaces.entrySet().removeIf(nn -> definedNamespaces.stream().anyMatch(dn -> dn.containsKey(nn.getKey())));
// Contained in current element
Iterator<Namespace> namespaces = startElement.getNamespaces();
while (namespaces.hasNext() && !necessaryNamespaces.isEmpty()) {
necessaryNamespaces.remove(namespaces.next().getPrefix());
}

// Add all remaining necessaryNamespaces
if (!necessaryNamespaces.isEmpty()) {
XMLEventFactory xmlEventFactory = XML_EVENT_FACTORY.get();
for (Map.Entry<String, String> entry : necessaryNamespaces.entrySet()) {
writer.add(xmlEventFactory.createNamespace(entry.getKey(), entry.getValue()));
}
}
return necessaryNamespaces;
}
}
130 changes: 94 additions & 36 deletions ...est/java/org/keycloak/saml/processing/core/parsers/saml/SAMLAttributeValueParserTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,55 +1,113 @@
package org.keycloak.saml.processing.core.parsers.saml;

import javax.xml.stream.events.XMLEvent;
import javax.xml.stream.XMLEventReader;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import org.junit.Assert;
import org.junit.Test;
import org.keycloak.saml.common.parsers.AbstractParser;
import org.keycloak.saml.processing.core.parsers.saml.assertion.SAMLAttributeValueParser;

import javax.xml.stream.XMLEventReader;
public class SAMLAttributeValueParserTest {

import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
private static final String XML_DOC = "Some Text";

public class SAMLAttributeValueParserTest {
private static final String XML_DOC_NESTED_ELEMENTS =
"<%1$sStreet%2$s>Zillestraße</%1$sStreet><%1$sHouseNumber%2$s>17</%1$sHouseNumber>"
+ "<%1$sZipCode%2$s>10585</%1$sZipCode><%1$sCity%2$s>Berlin</%1$sCity><%1$sCountry%2$s>DE</%1$sCountry>";

private static final String XML_DOC =
"<saml2:Attribute xmlns:saml2=\"urn:oasis:names:tc:SAML:2.0:assertion\"\n>"
+ " <saml2:AttributeValue xmlns:myCustomType=\"http://www.whatever.de/schema/myCustomType/saml/extensions\"\n"
+ " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:type=\"myCustomType:Something\">\n"
+ " Some Text\n"
+ " </saml2:AttributeValue>\n"
+ "</saml2:Attribute>";

private static final String XML_DOC_WITH_NESTED_ELEMENTS =
"<saml2:Attribute xmlns:saml2=\"urn:oasis:names:tc:SAML:2.0:assertion\"\n>"
+ " <saml2:AttributeValue xmlns:myCustomType=\"http://www.whatever.de/schema/myCustomType/saml/extensions\"\n"
+ " xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:type=\"myCustomType:AddressType\">\n"
+ " <myCustomType:Street>Zillestraße</myCustomType:Street>\n"
+ " <myCustomType:HouseNumber>17</myCustomType:HouseNumber>\n"
+ " <myCustomType:ZipCode>10585</myCustomType:ZipCode>\n"
+ " <myCustomType:City>Berlin</myCustomType:City>\n"
+ " <myCustomType:Country>DE</myCustomType:Country>\n"
+ " </saml2:AttributeValue>\n"
+ "</saml2:Attribute>";

@Test
public void parsesAttributeValueElementWithCustomTypes_ReturnsNull() throws Exception {
InputStream input = new ByteArrayInputStream(XML_DOC.getBytes(StandardCharsets.UTF_8));
XMLEventReader xmlEventReader = AbstractParser.createEventReader(input);
xmlEventReader.nextEvent();
final Object attributeValue = SAMLAttributeValueParser.getInstance().parse(xmlEventReader);
private static final String XML_DOC_SINGLE_ELEMENT =
"<%1$sAddressType%2$s>" + String.format(XML_DOC_NESTED_ELEMENTS, "%1$s", "") + "</%1$sAddressType>";

private static final String XML_DOC_NESTED_WITHOUT_PREFIX_AND_NAMESPACE = String.format(XML_DOC_NESTED_ELEMENTS, "", "");

Assert.assertNull(attributeValue);
@Test
public void parsesAttributeValueUserType() throws Exception {
Object actualAttributeValue = parseAttributeValue("xsi:type=\"myCustomType:Something\"", "\n " + XML_DOC + "\n ");
Assert.assertEquals(XML_DOC, actualAttributeValue);
}

@Test
public void parsesAttributeValueElementWithSubElements_ReturnsNull() throws Exception {
InputStream input = new ByteArrayInputStream(XML_DOC_WITH_NESTED_ELEMENTS.getBytes(StandardCharsets.UTF_8));
public void parsesAttributeValueUserTypeWithNamespace() throws Exception {
Object actualAttributeValue = parseAttributeValue(
"xmlns:myCustomType=\"http://my.custom.de/schema/saml/extensions\" xsi:type=\"myCustomType:Something\"",
"\n " + XML_DOC + "\n ");
Assert.assertEquals(XML_DOC, actualAttributeValue);
}

@Test
public void parseAttributeValueAnyType() throws Exception {
Object actualAttributeValue = parseAttributeValue("xsi:type=\"xs:anyType\"", XML_DOC_NESTED_WITHOUT_PREFIX_AND_NAMESPACE);
Assert.assertEquals(XML_DOC_NESTED_WITHOUT_PREFIX_AND_NAMESPACE, actualAttributeValue);
}

@Test
public void parsesAttributeValueUserTypeWithSingleElements() throws Exception {
final String xmlDoc = String.format(XML_DOC_SINGLE_ELEMENT, "", "");
Object actualAttributeValue = parseAttributeValue("xsi:type=\"AddressType\"", xmlDoc);
Assert.assertEquals(xmlDoc, actualAttributeValue);
}

@Test
public void parsesAttributeValueUserTypeWithSingleElementsAndNamespace() throws Exception {
Object actualAttributeValue = parseAttributeValue(
"xmlns:myCustomType=\"http://my.custom.de/schema/saml/extensions\" xsi:type=\"myCustomType:AddressType\"",
String.format(XML_DOC_SINGLE_ELEMENT, "myCustomType:", ""));
Assert.assertEquals(String.format(XML_DOC_SINGLE_ELEMENT, "myCustomType:",
" xmlns:myCustomType=\"http://my.custom.de/schema/saml/extensions\""),
actualAttributeValue);
}

@Test
public void parsesAttributeValueUserTypeWithNestedElements() throws Exception {
Object actualAttributeValue = parseAttributeValue("xsi:type=\"AddressType\"", XML_DOC_NESTED_WITHOUT_PREFIX_AND_NAMESPACE);
Assert.assertEquals(XML_DOC_NESTED_WITHOUT_PREFIX_AND_NAMESPACE, actualAttributeValue);
}

@Test
public void parsesAttributeValueUserTypeWithNestedElementsAndNamespace() throws Exception {
Object actualAttributeValue = parseAttributeValue(
"xmlns:myCustomType=\"http://my.custom.de/schema/saml/extensions\" xsi:type=\"myCustomType:AddressType\"",
String.format(XML_DOC_NESTED_ELEMENTS, "myCustomType:", ""));
Assert.assertEquals(String.format(XML_DOC_NESTED_ELEMENTS, "myCustomType:",
" xmlns:myCustomType=\"http://my.custom.de/schema/saml/extensions\""),
actualAttributeValue);
}

@Test
public void parsesAttributeValueUserTypeWithAttributeAndInnerNamespace() throws Exception {
String xmlDocPayload = "<%1$sAddress myCustomType3:restriction=\"one-way\"%3$s><%1$sStreet>Zillestraße</%1$sStreet><%2$sHouseNumber%4$s>17"
+ "</%2$sHouseNumber><myCustomType4:ZipCode xmlns:myCustomType4=\"http://my.custom4.de/schema/saml/extensions\">10585"
+ "</myCustomType4:ZipCode><City xmlns=\"http://my.custom4.de/schema/saml/extensions\">Berlin</City></%1$sAddress>";
String namespace1 = "xmlns:myCustomType1=\"http://my.custom1.de/schema/saml/extensions\"";
String namespace2 = "xmlns:myCustomType2=\"http://my.custom2.de/schema/saml/extensions\"";
String namespace3 = "xmlns:myCustomType3=\"http://my.custom3.de/schema/saml/extensions\"";
String namespace4 = "xmlns:myCustomType4=\"http://my.custom4.de/schema/saml/extensions\"";
Object actualAttributeValue = parseAttributeValue(
namespace1 + " " + namespace2 + " " + namespace3 + " " + namespace4 + " xsi:type=\"myCustomType1:AddressType\"",
String.format(xmlDocPayload, "myCustomType1:", "myCustomType2:", "", ""));
Assert.assertEquals(String.format(xmlDocPayload, "myCustomType1:", "myCustomType2:", " " + namespace3 + " " + namespace1, " " + namespace2),
actualAttributeValue);
}

private Object parseAttributeValue(String namespaceAndType, String payload) throws Exception {
InputStream input = new ByteArrayInputStream(asAttribute(namespaceAndType, payload).getBytes(StandardCharsets.UTF_8));
XMLEventReader xmlEventReader = AbstractParser.createEventReader(input);
xmlEventReader.nextEvent();
final Object attributeValue = SAMLAttributeValueParser.getInstance().parse(xmlEventReader);
Object attributeValue = SAMLAttributeValueParser.getInstance().parse(xmlEventReader);

XMLEvent nextXmlEvent = xmlEventReader.nextEvent();
Assert.assertTrue(nextXmlEvent.isEndElement());
final String nextName = nextXmlEvent.asEndElement().getName().getLocalPart();
Assert.assertTrue("Attribute".equals(nextName) || "AttributeValue".equals(nextName)); // both are valid

return attributeValue;
}

Assert.assertNull(attributeValue);
private String asAttribute(String namespaceAndType, String payload) {
return "<saml2:Attribute xmlns:saml2=\"urn:oasis:names:tc:SAML:2.0:assertion\"><saml2:AttributeValue xmlns:xsi=\"http://www.w3"
+ ".org/2001/XMLSchema-instance\" " + namespaceAndType + ">" + payload + "</saml2:AttributeValue></saml2:Attribute>";
}
}
17 changes: 13 additions & 4 deletions saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ public void testAuthnRequestInvalidNamespace() throws Exception {
@Test
public void testInvalidEndElement() throws Exception {
thrown.expect(ParsingException.class);
// see KEYCLOAK-7444
// see KEYCLOAK-7444
thrown.expectMessage(containsString("NameIDFormat"));

assertParsed("saml20-entity-descriptor-idp-invalid-end-element.xml", EntityDescriptorType.class);
Expand Down Expand Up @@ -1003,7 +1003,7 @@ public void testSaml20AssertionExample() throws Exception {
AssertionType assertion = assertParsed("saml20-assertion-example.xml", AssertionType.class);

AttributeStatementType attributeStatementType = assertion.getAttributeStatements().iterator().next();
assertThat(attributeStatementType.getAttributes(), hasSize(9));
assertThat(attributeStatementType.getAttributes(), hasSize(12));

for (AttributeStatementType.ASTChoiceType choiceType: attributeStatementType.getAttributes()) {
AttributeType attr = choiceType.getAttribute();
Expand All @@ -1012,7 +1012,7 @@ public void testSaml20AssertionExample() throws Exception {
// test selected attributes
switch (attrName) {
case "portal_id":
assertEquals(value, "060D00000000SHZ");
assertEquals("060D00000000SHZ", value);
break;
case "organization_id":
assertThat(value, instanceOf(String.class));
Expand All @@ -1028,6 +1028,9 @@ public void testSaml20AssertionExample() throws Exception {
case "anytype_no_xml_test":
assertThat(value, is((Object) "value_no_xml"));
break;
case "anytype_xml_fragment":
assertThat(value, is((Object) "<elem1>Foo</elem1><elem2>Bar</elem2>"));
break;
case "logouturl":
assertThat(value, is((Object) "http://www.salesforce.com/security/del_auth/SsoLogoutPage.html"));
break;
Expand All @@ -1037,6 +1040,12 @@ public void testSaml20AssertionExample() throws Exception {
case "status":
assertThat(value, is((Object) "<status><code><status>XYZ</status></code></status>"));
break;
case "userDefined":
assertThat(value, is((Object) "<A><B>Foo</B><C>Bar</C></A>"));
break;
case "userDefinedFragmentWithNamespace":
assertThat(value, is((Object) "<myPrefix:B xmlns:myPrefix=\"urn:myNamespace\">Foo</myPrefix:B><myPrefix:C xmlns:myPrefix=\"urn:myNamespace\">Bar</myPrefix:C>"));
break;
default:
break;
}
Expand Down Expand Up @@ -1130,7 +1139,7 @@ public void testSaml20AssertionsAdviceTag() throws Exception {
assertThat(ac.getSequence(), notNullValue());

assertThat(ac.getSequence().getClassRef().getValue(), is(JBossSAMLURIConstants.AC_UNSPECIFIED.getUri()));

assertThat(ac.getSequence(), notNullValue());
assertThat(ac.getSequence().getAuthnContextDecl(), notNullValue());
assertThat(ac.getSequence().getAuthnContextDecl().getValue(), instanceOf(Element.class));
Expand Down
14 changes: 14 additions & 0 deletions ...est/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-example.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@
<saml:AttributeValue>value_no_xml</saml:AttributeValue>
</saml:Attribute>

<saml:Attribute Name="anytype_xml_fragment">
<saml:AttributeValue xsi:type="xs:anyType">
<elem1>Foo</elem1>
<elem2>Bar</elem2>
</saml:AttributeValue>
</saml:Attribute>

<saml:Attribute Name="ssostartpage"
NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified">

Expand All @@ -138,6 +145,13 @@
<saml:AttributeValue xsi:nil="true" xsi:type="xs:anyType"/>
</saml:Attribute>

<saml:Attribute Name="userDefined">
<saml:AttributeValue xsi:type="MyType"><A><B>Foo</B><C>Bar</C></A></saml:AttributeValue>
</saml:Attribute>

<saml:Attribute Name="userDefinedFragmentWithNamespace">
<saml:AttributeValue xmlns:myPrefix="urn:myNamespace" xsi:type="myPrefix:MyType"><myPrefix:B>Foo</myPrefix:B><myPrefix:C>Bar</myPrefix:C></saml:AttributeValue>
</saml:Attribute>

</saml:AttributeStatement>
</saml:Assertion>
Loading

0 comments on commit 21f7006

Please sign in to comment.