From d478cdfda49f6d72b6aee1b8567ecf3419628620 Mon Sep 17 00:00:00 2001 From: pskopek Date: Mon, 25 Sep 2017 13:59:05 +0200 Subject: [PATCH] [KEYCLOAK-4374] Support SAML 2.0 AttributeValue of AnyType and nil --- .../core/parsers/util/SAMLParserUtil.java | 57 +++++++- .../core/parsers/saml/SAMLParserTest.java | 135 ++++++++++++++++-- ...ml20-assertion-anytype-attribute-value.xml | 19 +++ .../parsers/saml/saml20-assertion-example.xml | 132 +++++++++++++++++ .../saml/saml20-assertion-nil-wrong-1.xml | 11 ++ .../saml/saml20-assertion-nil-wrong-2.xml | 11 ++ 6 files changed, 348 insertions(+), 17 deletions(-) create mode 100644 saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-anytype-attribute-value.xml create mode 100644 saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-example.xml create mode 100644 saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-nil-wrong-1.xml create mode 100644 saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-nil-wrong-2.xml diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/util/SAMLParserUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/util/SAMLParserUtil.java index a58cdd3b22..8b39ff0062 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/util/SAMLParserUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/util/SAMLParserUtil.java @@ -49,10 +49,13 @@ import org.w3c.dom.Element; import javax.xml.datatype.XMLGregorianCalendar; import javax.xml.namespace.QName; import javax.xml.stream.XMLEventReader; +import javax.xml.stream.XMLEventWriter; +import javax.xml.stream.XMLOutputFactory; import javax.xml.stream.events.Attribute; import javax.xml.stream.events.EndElement; import javax.xml.stream.events.StartElement; import javax.xml.stream.events.XMLEvent; +import java.io.StringWriter; import java.net.URI; import java.util.ArrayList; import java.util.List; @@ -294,7 +297,22 @@ public class SAMLParserUtil { StartElement startElement = StaxParserUtil.getNextStartElement(xmlEventReader); StaxParserUtil.validate(startElement, JBossSAMLConstants.ATTRIBUTE_VALUE.get()); - Attribute type = startElement.getAttributeByName(new QName(JBossSAMLURIConstants.XSI_NSURI.get(), "type", "xsi")); + Attribute type = startElement.getAttributeByName(new QName(JBossSAMLURIConstants.XSI_NSURI.get(), JBossSAMLConstants.TYPE.get(), JBossSAMLURIConstants.XSI_PREFIX.get())); + Attribute nil = startElement.getAttributeByName(new QName(JBossSAMLURIConstants.XSI_NSURI.get(), "nil", JBossSAMLURIConstants.XSI_PREFIX.get())); + if (nil != null) { + String nilValue = StaxParserUtil.getAttributeValue(nil); + if (nilValue != null + && (nilValue.equalsIgnoreCase("true") || nilValue.equals("1"))) { + String elementText = StaxParserUtil.getElementText(xmlEventReader); + if (elementText == null || elementText.isEmpty()) { + return null; + } else { + throw logger.nullValueError("nil attribute is not in SAML20 format"); + } + } else { + throw logger.parserRequiredAttribute(JBossSAMLURIConstants.XSI_PREFIX.get() + ":nil"); + } + } if (type == null) { if (StaxParserUtil.hasTextAhead(xmlEventReader)) { return StaxParserUtil.getElementText(xmlEventReader); @@ -316,25 +334,54 @@ public class SAMLParserUtil { return ""; } - throw logger.unsupportedType(StaxParserUtil.getStartElementName(startElement)); + // when no type attribute assigned -> assume anyType + return parseAnyTypeAsString(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")) { - // TODO: for now assume that it is a text value that can be parsed and set as the attribute value - return StaxParserUtil.getElementText(xmlEventReader); + return parseAnyTypeAsString(xmlEventReader); } else if(typeValue.contains(":base64Binary")){ return StaxParserUtil.getElementText(xmlEventReader); } else if(typeValue.contains(":boolean")){ return StaxParserUtil.getElementText(xmlEventReader); } - throw logger.parserUnknownXSI(typeValue); } + public static String parseAnyTypeAsString(XMLEventReader xmlEventReader) throws ParsingException { + try { + XMLEvent event = xmlEventReader.peek(); + if (event.isStartElement()) { + event = xmlEventReader.nextTag(); + StringWriter sw = new StringWriter(); + XMLEventWriter writer = XMLOutputFactory.newInstance().createXMLEventWriter(sw); + //QName tagName = event.asStartElement().getName(); + int tagLevel = 1; + do { + writer.add(event); + event = (XMLEvent) xmlEventReader.next(); + if (event.isStartElement()) { + tagLevel++; + } + if (event.isEndElement()) { + tagLevel--; + } + } while (xmlEventReader.hasNext() && tagLevel > 0); + writer.add(event); + writer.flush(); + return sw.toString(); + } else { + return StaxParserUtil.getElementText(xmlEventReader); + } + } catch (Exception e) { + throw logger.parserError(e); + } + } + /** * Parse the AuthnStatement inside the assertion * diff --git a/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java b/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java index 2f2157c098..68baf0ddb4 100644 --- a/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java +++ b/saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLParserTest.java @@ -16,25 +16,34 @@ */ package org.keycloak.saml.processing.core.parsers.saml; -import org.keycloak.common.util.Base64; -import org.keycloak.common.util.DerUtils; -import org.keycloak.dom.saml.v2.assertion.AssertionType; -import org.keycloak.dom.saml.v2.assertion.NameIDType; -import org.keycloak.dom.saml.v2.metadata.EntityDescriptorType; -import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import java.io.IOException; import java.io.InputStream; import java.security.PrivateKey; +import org.junit.Before; import org.junit.Test; - -import static org.junit.Assert.*; -import static org.hamcrest.CoreMatchers.*; - +import org.keycloak.common.util.Base64; +import org.keycloak.common.util.DerUtils; +import org.keycloak.dom.saml.v2.assertion.AssertionType; +import org.keycloak.dom.saml.v2.assertion.AttributeStatementType; +import org.keycloak.dom.saml.v2.assertion.AttributeType; +import org.keycloak.dom.saml.v2.assertion.NameIDType; +import org.keycloak.dom.saml.v2.metadata.EntityDescriptorType; import org.keycloak.dom.saml.v2.protocol.LogoutRequestType; import org.keycloak.dom.saml.v2.protocol.ResponseType; - -import org.junit.Before; +import org.keycloak.saml.common.exceptions.ParsingException; +import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil; import org.w3c.dom.Element; /** @@ -216,4 +225,106 @@ public class SAMLParserTest { assertThat(parsedObject, instanceOf(ResponseType.class)); } } + + @Test + public void testSaml20AssertionsAnyTypeAttributeValue() throws Exception { + + String[] xmlSamples = { + "saml20-assertion-anytype-attribute-value.xml", + "saml20-assertion-example.xml" + }; + + for (String fileName: xmlSamples) { + try (InputStream st = SAMLParserTest.class.getResourceAsStream(fileName)) { + Object parsedObject = parser.parse(st); + assertThat("Problem detected in " + fileName + " sample.", parsedObject, instanceOf(AssertionType.class)); + checkCheckParsedResult(fileName, (AssertionType)parsedObject); + } catch (Exception e) { + throw new Exception("Problem detected in " + fileName + " sample.", e); + } + } + } + + private void checkCheckParsedResult(String fileName, AssertionType assertion) throws Exception { + AttributeStatementType attributeStatementType = assertion.getAttributeStatements().iterator().next(); + if ("saml20-assertion-anytype-attribute-value.xml".equals(fileName)) { + assertTrue("There has to be 3 attributes", attributeStatementType.getAttributes().size() == 3); + for (AttributeStatementType.ASTChoiceType choiceType: attributeStatementType.getAttributes()) { + AttributeType attr = choiceType.getAttribute(); + String attrName = attr.getName(); + String attrValueStatement = "unexpected value of attribute " + attrName + " of " + fileName; + String attrTypeStatement = "unexpected type of attribute " + attrName + " of " + fileName; + // test selected attributes + if (attrName.equals("attr:type:string")) { + assertEquals(attrValueStatement, attr.getAttributeValue().get(0), "CITIZEN"); + } else if (attrName.equals("attr:notype:string")) { + assertThat(attrTypeStatement, attr.getAttributeValue().get(0), instanceOf(String.class)); + String value = (String)attr.getAttributeValue().get(0); + assertEquals(attrValueStatement, value, "CITIZEN"); + } else if (attrName.equals("attr:notype:element")) { + assertThat(attrTypeStatement, attr.getAttributeValue().get(0), instanceOf(String.class)); + String value = (String)attr.getAttributeValue().get(0); + assertThat(attrValueStatement, value, containsString("hospitaal x")); + value = (String)attr.getAttributeValue().get(1); + assertThat(attrValueStatement, value, containsString("hopital x")); + } + } + } else if ("saml20-assertion-example.xml".equals(fileName)) { + assertThat("There has to be 9 attributes", attributeStatementType.getAttributes().size(), is(9)); + for (AttributeStatementType.ASTChoiceType choiceType: attributeStatementType.getAttributes()) { + AttributeType attr = choiceType.getAttribute(); + String attrName = attr.getName(); + String attrValueStatement = "unexpected value of attribute " + attrName + " of " + fileName; + String attrTypeStatement = "unexpected type of attribute " + attrName + " of " + fileName; + // test selected attributes + if (attrName.equals("portal_id")) { + assertEquals(attrValueStatement, attr.getAttributeValue().get(0), "060D00000000SHZ"); + } else if (attrName.equals("organization_id")) { + assertThat(attrTypeStatement, attr.getAttributeValue().get(0), instanceOf(String.class)); + String value = (String)attr.getAttributeValue().get(0); + assertThat(attrValueStatement, value, containsString("00DD0000000F7L5")); + } else if (attrName.equals("has_sub_organization")) { + assertThat(attrTypeStatement, attr.getAttributeValue().get(0), instanceOf(String.class)); + String value = (String)attr.getAttributeValue().get(0); + assertThat(attrValueStatement, value, containsString("true")); + } else if (attrName.equals("anytype_test")) { + assertThat(attrTypeStatement, attr.getAttributeValue().get(0), instanceOf(String.class)); + String value = (String)attr.getAttributeValue().get(0); + assertThat(attrValueStatement, value, containsString("val2")); + } else if (attrName.equals("anytype_no_xml_test")) { + assertThat(attrTypeStatement, attr.getAttributeValue().get(0), instanceOf(String.class)); + String value = (String)attr.getAttributeValue().get(0); + assertEquals(attrValueStatement, value, "value_no_xml"); + } else if (attrName.equals("logouturl")) { + assertThat(attrTypeStatement, attr.getAttributeValue().get(0), instanceOf(String.class)); + String value = (String)attr.getAttributeValue().get(0); + assertEquals(attrValueStatement, value, "http://www.salesforce.com/security/del_auth/SsoLogoutPage.html"); + } else if (attrName.equals("nil_value_attribute")) { + assertNull(attrValueStatement, attr.getAttributeValue().get(0)); + } else if (attrName.equals("status")) { + assertThat(attrTypeStatement, attr.getAttributeValue().get(0), instanceOf(String.class)); + String value = (String)attr.getAttributeValue().get(0); + assertThat(attrValueStatement, value, containsString("XYZ")); + } + } + } else { + throw new RuntimeException("test error: wrong file name to check"); + } + } + + @Test(expected = ParsingException.class) + public void testSaml20AssertionsNil1() throws IOException, ParsingException { + try (InputStream st = SAMLParserTest.class.getResourceAsStream("saml20-assertion-nil-wrong-1.xml")) { + parser.parse(st); + } + } + + @Test(expected = ParsingException.class) + public void testSaml20AssertionsNil2() throws IOException, ParsingException { + try (InputStream st = SAMLParserTest.class.getResourceAsStream("saml20-assertion-nil-wrong-2.xml")) { + parser.parse(st); + } + } + + } diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-anytype-attribute-value.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-anytype-attribute-value.xml new file mode 100644 index 0000000000..f4b78861e0 --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-anytype-attribute-value.xml @@ -0,0 +1,19 @@ + + issuer + + + CITIZEN + + + CITIZEN + + + + hospitaal x + + + hopital x + + + + diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-example.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-example.xml new file mode 100644 index 0000000000..591932e191 --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-example.xml @@ -0,0 +1,132 @@ + + + https://www.salesforce.com + + + + + + + + + + + + + + + vzR9Hfp8d16576tEDeq/zhpmLoo= + + + + + AzID5hhJeJlG2llUDvZswNUrlrPtR7S37QYH2W+Un1n8c6kTC + Xr/lihEKPcA2PZt86eBntFBVDWTRlh/W3yUgGOqQBJMFOVbhK + M/CbLHbBUVT5TcxIqvsNvIFdjIGNkf1W0SBqRKZOJ6tzxCcLo + 9dXqAyAUkqDpX5+AyltwrdCPNmncUM4dtRPjI05CL1rRaGeyX + 3kkqOL8p0vjm0fazU5tCAJLbYuYgU1LivPSahWNcpvRSlCI4e + Pn2oiVDyrcc4et12inPMTc2lGIWWWWJyHOPSiXRSkEAIwQVjf + Qm5cpli44Pv8FCrdGWpEE0yXsPBvDkM9jIzwCYGG2fKaLBag== + + + + + MIIEATCCAumgAwIBAgIBBTANBgkqhkiG9w0BAQ0FADCBgzELM + [Certificate truncated for readability...] + + + + + + + + saml01@salesforce.com + + + + + + + + + + + https://saml.salesforce.com + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified + + + + + + + + 060D00000000SHZ + + + + + + + 00DD0000000F7L5 + + + + + + + + + XYZ + + + + + + + true + + + + + + val2 + + + + + + value_no_xml + + + + + + http://www.salesforce.com/security/saml/saml20-gen.jsp + + + + + + + http://www.salesforce.com/security/del_auth/SsoLogoutPage.html + + + + + + + + + + diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-nil-wrong-1.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-nil-wrong-1.xml new file mode 100644 index 0000000000..1ce8a3c6a5 --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-nil-wrong-1.xml @@ -0,0 +1,11 @@ + + issuer + + + CITIZEN + + + + + + diff --git a/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-nil-wrong-2.xml b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-nil-wrong-2.xml new file mode 100644 index 0000000000..e914d3a8f2 --- /dev/null +++ b/saml-core/src/test/resources/org/keycloak/saml/processing/core/parsers/saml/saml20-assertion-nil-wrong-2.xml @@ -0,0 +1,11 @@ + + issuer + + + CITIZEN + + + + + +