KEYCLOAK-12732 Improve SAMLAttribute parsing of unknown attributes

saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/assertion/SAMLAttributeParser.java

@@ -19,8 +19,17 @@
 import org.keycloak.dom.saml.v2.assertion.AttributeType;
 import org.keycloak.saml.common.exceptions.ParsingException;
 import org.keycloak.saml.common.util.StaxParserUtil;
+
+import javax.xml.namespace.QName;
 import javax.xml.stream.XMLEventReader;
+import javax.xml.stream.events.Attribute;
 import javax.xml.stream.events.StartElement;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
 
 /**
  * Parse the <conditions> in the saml assertion
@@ -31,6 +40,12 @@ public class SAMLAttributeParser extends AbstractStaxSamlAssertionParser<Attribu
 
     private static final SAMLAttributeParser INSTANCE = new SAMLAttributeParser();
 
+    private static final Set<QName> DEFAULT_KNOWN_ATTRIBUTE_NAMES = new HashSet<>(Arrays.asList(
+            SAMLAssertionQNames.ATTR_NAME.getQName(),
+            SAMLAssertionQNames.ATTR_FRIENDLY_NAME.getQName(),
+            SAMLAssertionQNames.ATTR_NAME_FORMAT.getQName()
+    ));
+
     private SAMLAttributeParser() {
         super(SAMLAssertionQNames.ATTRIBUTE);
     }
@@ -47,14 +62,39 @@ protected AttributeType instantiateElement(XMLEventReader xmlEventReader, StartE
         attribute.setFriendlyName(StaxParserUtil.getAttributeValue(element, SAMLAssertionQNames.ATTR_FRIENDLY_NAME));
         attribute.setNameFormat(StaxParserUtil.getAttributeValue(element, SAMLAssertionQNames.ATTR_NAME_FORMAT));
 
-        final String x500Encoding = StaxParserUtil.getAttributeValue(element, SAMLAssertionQNames.ATTR_X500_ENCODING);
-        if (x500Encoding != null) {
-            attribute.getOtherAttributes().put(SAMLAssertionQNames.ATTR_X500_ENCODING.getQName(), x500Encoding);
-        }
+        // add non standard elements like SAMLAssertionQNames.ATTR_X500_ENCODING to other attributes
+        attribute.getOtherAttributes().putAll(collectUnknownAttributesFrom(element));
 
         return attribute;
     }
 
+    /**
+     * Returns a {@link Map} with the found non-standard attribute values for the given {@link StartElement}.
+     * An attribute is considered as non-standard, if it is not contained in DEFAULT_KNOWN_LOCAL_ATTRIBUTE_NAMES.
+     *
+     * @return Map
+     */
+    private static Map<QName, String> collectUnknownAttributesFrom(StartElement element) {
+
+        Map<QName, String> otherAttributes = new HashMap<>();
+
+        Iterator<?> attributes = element.getAttributes();
+        while (attributes.hasNext()) {
+            Attribute currentAttribute = (Attribute) attributes.next();
+            QName attributeQName = currentAttribute.getName();
+            if (attributeQName == null || DEFAULT_KNOWN_ATTRIBUTE_NAMES.contains(attributeQName)) {
+                continue;
+            }
+            String attributeValue = currentAttribute.getValue();
+            otherAttributes.put(attributeQName, attributeValue);
+            if (LOGGER.isTraceEnabled()) {
+                LOGGER.trace(String.format("Adding attribute %s with value %s", attributeQName, attributeValue));
+            }
+        }
+
+        return otherAttributes;
+    }
+
     @Override
     protected void processSubElement(XMLEventReader xmlEventReader, AttributeType target, SAMLAssertionQNames element, StartElement elementDetail) throws ParsingException {
         switch (element) {

saml-core/src/test/java/org/keycloak/saml/processing/core/parsers/saml/SAMLAttributeParserTest.java

@@ -0,0 +1,89 @@
+package org.keycloak.saml.processing.core.parsers.saml;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.keycloak.dom.saml.v2.assertion.AttributeType;
+import org.keycloak.saml.common.parsers.AbstractParser;
+import org.keycloak.saml.processing.core.parsers.saml.assertion.SAMLAttributeParser;
+
+import javax.xml.namespace.QName;
+import javax.xml.stream.XMLEventReader;
+import java.io.ByteArrayInputStream;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+
+public class SAMLAttributeParserTest {
+
+    private static String XML_DOC_TEMPLATE = "<samlp:AttributeQuery\n" +
+            "    xmlns:saml=\"urn:oasis:names:tc:SAML:2.0:assertion\"\n" +
+            "    xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\">\n" +
+            "${ATTRIBUTE_ELEMENT}\n" +
+            "</samlp:AttributeQuery>";
+
+    @Test
+    public void parsesAttributeElementWithKnownAttributesCorrectly() throws Exception {
+
+        String nameFormatValue = "urn:oasis:names:tc:SAML:2.0:attrname-format:uri";
+        String nameValue = "urn:oid:2.5.4.42";
+        String friendlyNameValue = "givenName";
+
+        AttributeType attributeType = parseAttributeElement("<saml:Attribute NameFormat=\"" + nameFormatValue + "\" Name=\"" + nameValue + "\" FriendlyName=\"" + friendlyNameValue + "\"/>");
+
+        Assert.assertEquals(nameFormatValue, attributeType.getNameFormat());
+        Assert.assertEquals(nameValue, attributeType.getName());
+        Assert.assertEquals(friendlyNameValue, attributeType.getFriendlyName());
+        Assert.assertTrue("Other attributes should be empty", attributeType.getOtherAttributes().isEmpty());
+    }
+
+    @Test
+    public void parsesAttributeElementWithKnownAndX509_ENCODINGAttributesCorrectly() throws Exception {
+
+        String nameFormatValue = "urn:oasis:names:tc:SAML:2.0:attrname-format:uri";
+        String nameValue = "urn:oid:2.5.4.42";
+        String friendlyNameValue = "givenName";
+        String encodingValue = "LDAP";
+
+        String x500Namespace = "urn:oasis:names:tc:SAML:2.0:profiles:attribute:X500";
+        AttributeType attributeType = parseAttributeElement(String.format("<saml:Attribute xmlns:x500=\"%s\" " + //
+                        "NameFormat=\"%s\" Name=\"%s\" FriendlyName=\"%s\" x500:Encoding=\"%s\"/>", x500Namespace, //
+                nameFormatValue, nameValue, friendlyNameValue, encodingValue));
+
+        Assert.assertEquals(nameFormatValue, attributeType.getNameFormat());
+        Assert.assertEquals(nameValue, attributeType.getName());
+        Assert.assertEquals(friendlyNameValue, attributeType.getFriendlyName());
+        Assert.assertTrue("Other attributes should not be empty", !attributeType.getOtherAttributes().isEmpty());
+        Assert.assertEquals(encodingValue, attributeType.getOtherAttributes().get(new QName(x500Namespace, "Encoding")));
+    }
+
+    @Test
+    public void parsesAttributeElementWithKnownAndOtherAttributesCorrectly() throws Exception {
+
+        String nameFormatValue = "urn:oasis:names:tc:SAML:2.0:attrname-format:uri";
+        String nameValue = "urn:oid:2.5.4.42";
+        String friendlyNameValue = "givenName";
+
+        String someNs = "https://www.thenamespace.ns/path";
+        String someValue1 = "v1";
+        String someValue2 = "v2";
+
+        AttributeType attributeType = parseAttributeElement(String.format("<saml:Attribute xmlns:somens=\"%s\" " + //
+                        "NameFormat=\"%s\" Name=\"%s\" FriendlyName=\"%s\" somens:Value1=\"%s\" somens:Value2=\"%s\"/>", someNs, //
+                nameFormatValue, nameValue, friendlyNameValue, someValue1, someValue2));
+
+        Assert.assertEquals(nameFormatValue, attributeType.getNameFormat());
+        Assert.assertEquals(nameValue, attributeType.getName());
+        Assert.assertEquals(friendlyNameValue, attributeType.getFriendlyName());
+        Assert.assertTrue("Other attributes should not be empty", !attributeType.getOtherAttributes().isEmpty());
+        Assert.assertEquals(someValue1, attributeType.getOtherAttributes().get(new QName(someNs, "Value1")));
+        Assert.assertEquals(someValue2, attributeType.getOtherAttributes().get(new QName(someNs, "Value2")));
+    }
+
+    protected AttributeType parseAttributeElement(String attributeXml) throws Exception {
+
+        String xmlDoc = XML_DOC_TEMPLATE.replace("${ATTRIBUTE_ELEMENT}", attributeXml);
+        InputStream input = new ByteArrayInputStream(xmlDoc.getBytes(StandardCharsets.UTF_8));
+        XMLEventReader xmlEventReader = AbstractParser.createEventReader(input);
+        xmlEventReader.nextEvent();
+        return SAMLAttributeParser.getInstance().parse(xmlEventReader);
+    }
+}