Ver código fonte

Add attribute count to SamlAttribute toString (#131173)

Sometimes SAML IdPs send what _should_ be a list of values as a single
comma-separated string.

That is, we expect something using SAML's multi-valued attribute
feature:

    <saml:Attribute NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
       Name="http://idp.example.org/attributes/groups" FriendlyName="groups">
       <saml:AttributeValue>engineering</saml:AttributeValue>
       <saml:AttributeValue>elasticsearch-admins</saml:AttributeValue>
       <saml:AttributeValue>employees</saml:AttributeValue>
    </saml:Attribute>

but we get

    <saml:Attribute NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
       Name="http://idp.example.org/attributes/groups" FriendlyName="groups">
       <saml:AttributeValue>engineering,elasticsearch-admins,employees</saml:AttributeValue>
    </saml:Attribute>

In order to help detect these cases, this commit changes the
`toString()` on `SamlAttribute` to include the length (e.g. `(len=1)`)
at the end

Relates: #84379, #102769
Tim Vernum 3 meses atrás
pai
commit
dc48b4b28b

+ 5 - 0
docs/changelog/131173.yaml

@@ -0,0 +1,5 @@
+pr: 131173
+summary: Add attribute count to `SamlAttribute` `toString`
+area: Authentication
+type: enhancement
+issues: []

+ 5 - 2
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAttributes.java

@@ -92,11 +92,14 @@ public class SamlAttributes {
 
         @Override
         public String toString() {
+            StringBuilder str = new StringBuilder();
             if (Strings.isNullOrEmpty(friendlyName)) {
-                return name + '=' + values;
+                str.append(name);
             } else {
-                return friendlyName + '(' + name + ")=" + values;
+                str.append(friendlyName).append('(').append(name).append(')');
             }
+            str.append("=").append(values).append("(len=").append(values.size()).append(')');
+            return str.toString();
         }
     }
 

+ 52 - 0
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAttributesTests.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.security.authc.saml;
+
+import org.hamcrest.Matchers;
+import org.opensaml.saml.saml2.core.NameID;
+
+import java.util.List;
+
+public class SamlAttributesTests extends SamlTestCase {
+
+    public void testToString() {
+        final String nameFormat = randomFrom(NameID.TRANSIENT, NameID.PERSISTENT, NameID.EMAIL);
+        final String nameId = randomIdentifier();
+        final String session = randomAlphaOfLength(16);
+        final SamlAttributes attributes = new SamlAttributes(
+            new SamlNameId(nameFormat, nameId, null, null, null),
+            session,
+            List.of(
+                new SamlAttributes.SamlAttribute("urn:oid:0.9.2342.19200300.100.1.1", null, List.of("peter.ng")),
+                new SamlAttributes.SamlAttribute("urn:oid:2.5.4.3", "name", List.of("Peter Ng")),
+                new SamlAttributes.SamlAttribute(
+                    "urn:oid:1.3.6.1.4.1.5923.1.5.1.1",
+                    "groups",
+                    List.of("employees", "engineering", "managers")
+                )
+            )
+        );
+        assertThat(
+            attributes.toString(),
+            Matchers.equalTo(
+                "SamlAttributes("
+                    + ("NameId(" + nameFormat + ")=" + nameId)
+                    + ")["
+                    + session
+                    + "]{["
+                    + "urn:oid:0.9.2342.19200300.100.1.1=[peter.ng](len=1)"
+                    + ", "
+                    + "name(urn:oid:2.5.4.3)=[Peter Ng](len=1)"
+                    + ", "
+                    + "groups(urn:oid:1.3.6.1.4.1.5923.1.5.1.1)=[employees, engineering, managers](len=3)"
+                    + "]}"
+            )
+        );
+    }
+
+}