[java-identity-provider] branch master updated: IDP-1518 Revise handling on EmptyAttributeValue

Rod Widdowson rdw at steadingsoftware.com
Fri Nov 8 10:50:13 EST 2019


This is an automated email from the git hooks/post-receive script.

rdw pushed a commit to branch master
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=87d8d0dc979fc756ae35e6dc7f99ff8b252508ee

The following commit(s) were added to refs/heads/master by this push:
       new  87d8d0d   IDP-1518 Revise handling on EmptyAttributeValue
87d8d0d is described below

commit 87d8d0dc979fc756ae35e6dc7f99ff8b252508ee
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri Nov 8 15:46:30 2019 +0000

    IDP-1518 Revise handling on EmptyAttributeValue
    
    https://issues.shibboleth.net/jira/browse/IDP-1518
    
    Also broaden tests and add some extra logging for surprise avoidance.
---
 idp-consent-impl/pom.xml                           |  6 ++
 .../logic/impl/AttributeValuesHashFunction.java    | 18 ++++--
 .../impl/AttributeValuesHashFunctionTest.java      | 65 ++++++++++++++++++++--
 .../src/test/resources/logback-test.xml            |  4 +-
 4 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/idp-consent-impl/pom.xml b/idp-consent-impl/pom.xml
index bbf23d6..2876955 100644
--- a/idp-consent-impl/pom.xml
+++ b/idp-consent-impl/pom.xml
@@ -88,6 +88,12 @@
 
         <!-- Test Dependencies -->
         <dependency>
+            <groupId>${opensaml.groupId}</groupId>
+            <artifactId>opensaml-core</artifactId>
+            <type>test-jar</type>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
             <groupId>${project.groupId}</groupId>
             <artifactId>idp-profile-api</artifactId>
             <version>${project.version}</version>
diff --git a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunction.java b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunction.java
index 61e2f07..85a1378 100644
--- a/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunction.java
+++ b/idp-consent-impl/src/main/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunction.java
@@ -36,6 +36,7 @@ import org.opensaml.saml.saml2.core.NameIDType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import net.shibboleth.idp.attribute.ByteAttributeValue;
 import net.shibboleth.idp.attribute.EmptyAttributeValue;
 import net.shibboleth.idp.attribute.IdPAttributeValue;
 import net.shibboleth.idp.attribute.ScopedStringAttributeValue;
@@ -75,6 +76,10 @@ public class AttributeValuesHashFunction implements Function<Collection<IdPAttri
             final ObjectOutputStream objectOutputStream = new ObjectOutputStream(byteArrayOutputStream)) {
 
             for (final IdPAttributeValue value : filteredInput) {
+                if (log.isTraceEnabled()) {
+                    log.trace("Considering value of '{}' with native value {}",
+                            value.getClass(), value.getNativeValue());
+                }
                 if (value instanceof ScopedStringAttributeValue) {
                     objectOutputStream.writeObject(((ScopedStringAttributeValue) value).getValue() + '@'
                             + ((ScopedStringAttributeValue) value).getScope());
@@ -96,15 +101,18 @@ public class AttributeValuesHashFunction implements Function<Collection<IdPAttri
                 } else if (value instanceof EmptyAttributeValue) {
                     // unique signature
                     objectOutputStream.writeObject(Long.valueOf(42));
-                    if (EmptyAttributeValue.NULL.getValue().equals(value.getNativeValue())) {
-                        objectOutputStream.writeObject("NULLVALUE");
-                    } else if (EmptyAttributeValue.ZERO_LENGTH.getValue().equals(value.getNativeValue())) {
-                        objectOutputStream.writeObject("EMPTY VALUE");
-                    } else {
+                    if (!EmptyAttributeValue.NULL.equals(value) &&
+                        !EmptyAttributeValue.ZERO_LENGTH.equals(value)) {
                         log.error("Internal error - impossible null attribute");
                     }
+                    objectOutputStream.writeObject(value.getNativeValue().toString());
+                } else if (value instanceof ByteAttributeValue) {
+                    objectOutputStream.writeObject(((ByteAttributeValue)value).getValue());
                 } else if (value.getNativeValue() != null) {
+                    log.debug("Unknown atribute value '{}' hashed as {}", value.getClass(), value.getNativeValue());
                     objectOutputStream.writeObject(value.getNativeValue());
+                } else {
+                    log.warn("Unknown attribute value '{}' with no value was not hashed", value.getClass());
                 }
             }
 
diff --git a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunctionTest.java b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunctionTest.java
index f01d9a3..edb9a7a 100644
--- a/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunctionTest.java
+++ b/idp-consent-impl/src/test/java/net/shibboleth/idp/consent/logic/impl/AttributeValuesHashFunctionTest.java
@@ -22,15 +22,23 @@ import static org.testng.Assert.assertEquals;
 import java.util.Collections;
 import java.util.List;
 
+import org.opensaml.core.xml.XMLObjectBaseTestCase;
+import org.opensaml.core.xml.XMLObjectBuilder;
+import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
+import org.opensaml.core.xml.schema.XSString;
 import org.testng.Assert;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import net.shibboleth.idp.attribute.ByteAttributeValue;
 import net.shibboleth.idp.attribute.EmptyAttributeValue;
+import net.shibboleth.idp.attribute.IdPAttributeValue;
+import net.shibboleth.idp.attribute.ScopedStringAttributeValue;
+import net.shibboleth.idp.attribute.XMLObjectAttributeValue;
 import net.shibboleth.idp.consent.impl.ConsentTestingSupport;
 
 /** {@link AttributeValuesHashFunction} unit test. */
-public class AttributeValuesHashFunctionTest {
+public class AttributeValuesHashFunctionTest extends XMLObjectBaseTestCase {
 
     private AttributeValuesHashFunction function;
 
@@ -45,13 +53,19 @@ public class AttributeValuesHashFunctionTest {
     @Test public void testEmptyInput() {
         Assert.assertNull(function.apply(Collections.emptyList()));
     }
-    
+
     @Test(enabled = true) public void testNullValue() {
-        assertEquals(function.apply(List.of(EmptyAttributeValue.NULL)), "GO6W6gt/9+cYhDCPAJSdQnhYbNP07CvgEUNsjTHNWjM=");
-        assertEquals(function.apply(List.of(EmptyAttributeValue.ZERO_LENGTH)), "b3sJLxUOXzXC263DWVp0F/l5fxfxJxKuJYGpKCtjhaw=");
+        // NOTE Any change is an ODS drift
+        assertEquals(function.apply(List.of(EmptyAttributeValue.NULL)), "QlRl6kTdT/0tD4h3xpwJn+hoFjZssUN15Bc6DO/CXws=");
+    }
+
+    @Test(enabled = true) public void testEmptyValue() {
+        // NOTE Any change is an ODS drift
+        assertEquals(function.apply(List.of(EmptyAttributeValue.ZERO_LENGTH)), "vwBWiidH5q5XZY3uEPcnJYeDTgqJnpV9WpmgWZS9wR4=");
     }
 
     @Test public void testSingleValue() {
+        // NOTE Any change is an ODS drift
         final String hash = function.apply(ConsentTestingSupport.newAttributeMap().get("attribute1").getValues());
         assertEquals(hash, "yePBj0hcjLihhDtDb//R/ymyw2CHZAUreX/4RupmSXM=");
     }
@@ -61,4 +75,47 @@ public class AttributeValuesHashFunctionTest {
         assertEquals(hash, "xxuA06hGJ1DcJ4JSaWiBXXGfcRr6oxHM5jaURXBBnbA=");
     }
 
+    @Test public void testScoped() {
+        // NOTE Any change is an ODS drift
+        final IdPAttributeValue val = new ScopedStringAttributeValue("Value", "Scope");
+        assertEquals(function.apply(Collections.singletonList(val)), "WFoLzGdi3WmUjhWe3Q6uSyoHVZJXukDWeOUb7CyH5V8=");
+    }
+
+    @Test public void testByte() {
+        // NOTE Any change is an ODS drift
+        final byte[] theBytes = {1,2,3};
+        final IdPAttributeValue val = new ByteAttributeValue(theBytes);
+        assertEquals(function.apply(Collections.singletonList(val)), "saP1UTQcyQPZHOPI6tVhVWMKOmB3BDCTn/l5QFSsyX4=");
+    }
+
+    @Test public void testXML() {
+        // NOTE Any change is an ODS drift
+        final XMLObjectBuilder<XSString> builder =
+                XMLObjectProviderRegistrySupport.getBuilderFactory().<XSString>getBuilderOrThrow(
+                        XSString.TYPE_NAME);
+        final XSString xmlString = builder.buildObject(XSString.TYPE_NAME);
+        xmlString.setValue("value");
+        final IdPAttributeValue val = new XMLObjectAttributeValue(xmlString);
+        assertEquals(function.apply(Collections.singletonList(val)), "c+NqWOijlvFBpla4r1q3F0RkpYZK7phCNe2gKb0r57o=");
+    }
+
+    private IdPAttributeValue testAV(Object type) {
+        return new IdPAttributeValue() {
+
+            public Object getNativeValue() {
+                return type;
+            }
+
+            public String getDisplayValue() {
+                return "Display";
+            }};
+    }
+
+    @Test public void unknownTypeValue() {
+        assertEquals(function.apply(Collections.singletonList(testAV("42"))), "Lt6BAjtq4qQJ6ADEZKf/s5XZxzBh6mShY/UCphriugY=");
+    }
+
+    @Test public void unknownTypeNoValue() {
+        assertEquals(function.apply(Collections.singletonList(testAV(null))), "xPtMT+sJsVtAtjNLzPrBBlfbY/yUsAQ7Ncxxc7Q5k70=");
+    }
 }
diff --git a/idp-consent-impl/src/test/resources/logback-test.xml b/idp-consent-impl/src/test/resources/logback-test.xml
index f3280e5..2e5d5ba 100644
--- a/idp-consent-impl/src/test/resources/logback-test.xml
+++ b/idp-consent-impl/src/test/resources/logback-test.xml
@@ -9,8 +9,10 @@
         </encoder>
     </appender>
 
+    <logger name="net.shibboleth.idp.consent" level="DEBUG" />
+
     <root>
-        <level value="warn" />
+        <level value="WARN" />
         <appender-ref ref="STDOUT" />
     </root>
     

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list