[java-identity-provider] branch main updated: IDP-1666 All IdPAttributeValue implementations should return meaningful getDisplayValue

Rod Widdowson rdw at steadingsoftware.com
Fri Sep 4 10:13:43 UTC 2020


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

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

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

The following commit(s) were added to refs/heads/main by this push:
       new  0c169fa45 IDP-1666 All IdPAttributeValue implementations should return meaningful getDisplayValue
0c169fa45 is described below

commit 0c169fa45729a887a8a3a86abb9fa763b845fb00
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Fri Sep 4 11:12:38 2020 +0100

    IDP-1666 All IdPAttributeValue implementations should return meaningful getDisplayValue
    
    https://issues.shibboleth.net/jira/browse/IDP-1666
    
            Byte... return the same as toHex
            XmlObject.. returns the same as the consert hashing function uses
---
 idp-attribute-api/pom.xml                          | 11 +++++
 .../idp/attribute/ByteAttributeValue.java          | 10 ++---
 .../idp/attribute/XMLObjectAttributeValue.java     | 26 ++++++++---
 .../shibboleth/idp/attribute/AttributeTest.java    | 50 +++++++++++++++++++++-
 4 files changed, 85 insertions(+), 12 deletions(-)

diff --git a/idp-attribute-api/pom.xml b/idp-attribute-api/pom.xml
index c4640b67b..98fe3458d 100644
--- a/idp-attribute-api/pom.xml
+++ b/idp-attribute-api/pom.xml
@@ -74,6 +74,17 @@
             <type>test-jar</type>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>${opensaml.groupId}</groupId>
+            <artifactId>opensaml-core</artifactId>
+            <type>test-jar</type>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>${opensaml.groupId}</groupId>
+            <artifactId>opensaml-saml-impl</artifactId>
+            <scope>test</scope>
+        </dependency>
 
     </dependencies>
 
diff --git a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/ByteAttributeValue.java b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/ByteAttributeValue.java
index 0dab15214..42bceef99 100644
--- a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/ByteAttributeValue.java
+++ b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/ByteAttributeValue.java
@@ -22,16 +22,16 @@ import java.util.Arrays;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.apache.commons.codec.binary.Hex;
+
+import com.google.common.base.MoreObjects;
+
 import net.shibboleth.utilities.java.support.annotation.ParameterName;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.codec.Base64Support;
 import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
-import org.apache.commons.codec.binary.Hex;
-
-import com.google.common.base.MoreObjects;
-
 /** A <code>byte[]</code> value for an {@link IdPAttribute}. */
 public final class ByteAttributeValue implements IdPAttributeValue {
 
@@ -62,7 +62,7 @@ public final class ByteAttributeValue implements IdPAttributeValue {
 
     /** {@inheritDoc} */
     @Override @Nonnull @NotEmpty public String getDisplayValue() {
-        return "(binary data)";
+        return toHex();
     }
 
     /**
diff --git a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/XMLObjectAttributeValue.java b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/XMLObjectAttributeValue.java
index 52ff3040b..adef48e89 100644
--- a/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/XMLObjectAttributeValue.java
+++ b/idp-attribute-api/src/main/java/net/shibboleth/idp/attribute/XMLObjectAttributeValue.java
@@ -20,18 +20,26 @@ package net.shibboleth.idp.attribute;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
-import net.shibboleth.utilities.java.support.annotation.ParameterName;
-import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.logic.Constraint;
-
 import org.opensaml.core.xml.XMLObject;
+import org.opensaml.core.xml.io.MarshallingException;
+import org.opensaml.core.xml.util.XMLObjectSupport;
 import org.opensaml.saml.saml2.core.NameIDType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.base.MoreObjects;
 
+import net.shibboleth.utilities.java.support.annotation.ParameterName;
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.xml.SerializeSupport;
+
 /** A {@link XMLObjectAttributeValue} value for an {@link net.shibboleth.idp.attribute.IdPAttribute}. */
 public final class XMLObjectAttributeValue implements IdPAttributeValue {
 
+    /** Log. */
+    private static final Logger LOG = LoggerFactory.getLogger(XMLObjectAttributeValue.class);
+
     /** Value of the attribute. */
     private final XMLObject value;
 
@@ -61,9 +69,15 @@ public final class XMLObjectAttributeValue implements IdPAttributeValue {
     @Override
     @Nonnull @NotEmpty public String getDisplayValue() {
         if (value instanceof NameIDType) {
-            return ((NameIDType) value).getValue();
+            final NameIDType valAsNameId = (NameIDType) value;
+            return valAsNameId.getValue();
+        }
+        try {
+            return SerializeSupport.nodeToString(XMLObjectSupport.marshall(value));
+        } catch (final MarshallingException e) {
+            LOG.error("Error while marshalling XMLObject value", e);
+            return null;
         }
-        return "(XML data)";
     }
 
     /** {@inheritDoc} */
diff --git a/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeTest.java b/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeTest.java
index 753729eed..46071d5f1 100644
--- a/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeTest.java
+++ b/idp-attribute-api/src/test/java/net/shibboleth/idp/attribute/AttributeTest.java
@@ -17,6 +17,7 @@
 
 package net.shibboleth.idp.attribute;
 
+import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
@@ -24,18 +25,23 @@ import static org.testng.Assert.fail;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
+import org.opensaml.core.OpenSAMLInitBaseTestCase;
+import org.opensaml.saml.saml2.metadata.EntityDescriptor;
+import org.opensaml.saml.saml2.metadata.impl.EntityDescriptorBuilder;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import net.shibboleth.idp.attribute.EmptyAttributeValue.EmptyType;
 import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
 
 /** Unit test for {@link IdPAttribute} class. */
 @SuppressWarnings("javadoc")
-public class AttributeTest {
+public class AttributeTest extends OpenSAMLInitBaseTestCase{
 
     /** Tests that the attribute has its expected state after instantiation. */
     @Test public void instantiation() {
@@ -346,4 +352,46 @@ public class AttributeTest {
         }
         new IdPAttribute("Check%for{deprecation");
     }
+
+    @Test public void displayValues() {
+        final IdPAttributeValue stringVal = StringAttributeValue.valueOf("Stringval");
+        final IdPAttributeValue scopedVal = ScopedStringAttributeValue.valueOf("value", "scope");
+        final byte array[] = {1,2,3,4};
+        final IdPAttributeValue binary = ByteAttributeValue.valueOf(array);
+        final IdPAttributeValue nullEmpty = new EmptyAttributeValue(EmptyType.NULL_VALUE);
+        final IdPAttributeValue zeroEmpty = new EmptyAttributeValue(EmptyType.ZERO_LENGTH_VALUE);
+        final EntityDescriptor entity = (new EntityDescriptorBuilder()).buildObject();
+        entity.setEntityID("https://example.org");
+        final IdPAttributeValue xmlval = new XMLObjectAttributeValue(entity);
+
+        final List<IdPAttributeValue> inList = List.of(stringVal, scopedVal, binary, nullEmpty, zeroEmpty, xmlval, stringVal);
+        final HashSet<IdPAttributeValue> inHash = new HashSet<>(inList);
+        final ArrayList<IdPAttributeValue> sorted = new ArrayList<>(inList);
+        sorted.sort(null);
+        final ArrayList<IdPAttributeValue> sortedDedup = new ArrayList<>(inHash);
+        sortedDedup.sort(null);
+        assertEquals(inList.size()-1, inHash.size());
+        assertEquals(inList.size(), sorted.size());
+        assertEquals(sortedDedup.size(), inHash.size());
+
+        for (IdPAttributeValue v:inList) {
+            assertTrue(inHash.contains(v));
+        }
+        for (IdPAttributeValue v:sorted) {
+            assertTrue(inHash.contains(v));
+        }
+        for (IdPAttributeValue v:sortedDedup) {
+            assertTrue(inHash.contains(v));
+        }
+        int offset = 0;
+        for (int i = 0; i < sortedDedup.size(); i++) {
+            final String s = sorted.get(i+offset).getDisplayValue();
+            final String ds = sortedDedup.get(i).getDisplayValue();
+            if (!ds.equals(s)) {
+                assertEquals(s, sorted.get(i-1).getDisplayValue());
+                assertEquals(ds, sorted.get(i+1).getDisplayValue());
+                offset = 1;
+            }
+        }
+    }
 }

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


More information about the commits mailing list