[java-identity-provider] 01/04: IdP-1206 AttributeResolver: Optional filtering of null values after resolution

Rod Widdowson rdw at steadingsoftware.com
Tue Apr 17 11:14:33 EDT 2018


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=5ebb8bc7c779d3512b21aeaefef64ed39c6a8af9

commit 5ebb8bc7c779d3512b21aeaefef64ed39c6a8af9
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Tue Apr 17 10:15:02 2018 +0100

    IdP-1206 AttributeResolver: Optional filtering of null values after resolution
    
    https://issues.shibboleth.net/jira/browse/IDP-1206
---
 .../resolver/impl/AttributeResolverImpl.java       | 73 ++++++++++++++++------
 .../resolver/impl/AttributeResolverImplTest.java   | 43 +++++++++++--
 2 files changed, 92 insertions(+), 24 deletions(-)

diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImpl.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImpl.java
index 6f54370..7583b4e 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImpl.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImpl.java
@@ -29,7 +29,18 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
 
+import org.opensaml.messaging.context.BaseContext;
+import org.opensaml.messaging.context.navigate.ParentContextLookup;
+import org.opensaml.profile.context.MetricContext;
+import org.opensaml.profile.context.ProfileRequestContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableMap;
+
 import net.shibboleth.ext.spring.service.AbstractServiceableComponent;
+import net.shibboleth.idp.attribute.EmptyAttributeValue;
 import net.shibboleth.idp.attribute.IdPAttribute;
 import net.shibboleth.idp.attribute.IdPAttributeValue;
 import net.shibboleth.idp.attribute.resolver.AttributeDefinition;
@@ -57,16 +68,6 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
-import org.opensaml.messaging.context.BaseContext;
-import org.opensaml.messaging.context.navigate.ParentContextLookup;
-import org.opensaml.profile.context.MetricContext;
-import org.opensaml.profile.context.ProfileRequestContext;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.google.common.base.Function;
-import com.google.common.collect.ImmutableMap;
-
 /**
  * A component that resolves the attributes for a particular subject.
  * 
@@ -97,6 +98,9 @@ public class AttributeResolverImpl extends AbstractServiceableComponent<Attribut
     /** The Principal mapper. */
     @Nullable private LegacyPrincipalDecoder principalConnector;
     
+    /** Whether to strip null attribute values. */
+    private boolean stripNulls;
+    
     /** Strategy to get the {@link ProfileRequestContext}. */
     @Nonnull private Function<AttributeResolutionContext,ProfileRequestContext> profileContextStrategy;
 
@@ -174,6 +178,22 @@ public class AttributeResolverImpl extends AbstractServiceableComponent<Attribut
         return dataConnectors;
     }
     
+    /**
+     * Do we strip nulls from attribute values.
+     * @return Returns whether to strip nulls from attribute values
+     */
+    public boolean isStripNulls() {
+        return stripNulls;
+    }
+
+    /** 
+     * Sets whether to strip nulls from attribute values.
+     * @param doStripNulls what to set 
+     */
+    public void setStripNulls(final Boolean doStripNulls) {
+        stripNulls = doStripNulls;
+    }
+
     /** Set the Decoder.
      * @param principalResolver code to resolve the principal
      */
@@ -445,6 +465,7 @@ public class AttributeResolverImpl extends AbstractServiceableComponent<Attribut
      * 
      * @param resolutionContext current resolution context
      */
+ // Checkstyle: CyclomaticComplexity OFF
     protected void finalizeResolvedAttributes(@Nonnull final AttributeResolutionContext resolutionContext) {
         Constraint.isNotNull(resolutionContext, "Attribute resolution context cannot be null");
         final AttributeResolverWorkContext workContext =
@@ -468,25 +489,38 @@ public class AttributeResolverImpl extends AbstractServiceableComponent<Attribut
                 continue;
             }
 
-            // Remove value-less attributes.
-            if (resolvedAttribute.getValues().size() == 0) {
-                log.debug("{} Removing result of attribute definition '{}', contains no values", logPrefix,
-                        definition.getId());
-                continue;
-            }
-
             // Remove duplicate attribute values.
-            log.debug("{} De-duping attribute definition {} result", logPrefix, definition.getId());
+            log.debug("{} De-duping (and null filtering) attribute definition {} result",
+                    logPrefix, definition.getId());
             final Iterator<IdPAttributeValue<?>> valueIter = resolvedAttribute.getValues().iterator();
             final Set<IdPAttributeValue<?>> monitor = new HashSet<>(resolvedAttribute.getValues().size());
             while (valueIter.hasNext()) {
-                final IdPAttributeValue<?> value = valueIter.next();
+                 final IdPAttributeValue<?> value = valueIter.next();
+                
+                if (isStripNulls()) {
+                    if (null == value) {
+                        log.debug("{} Stripping null value", logPrefix);
+                        continue;
+                    } else if (value instanceof EmptyAttributeValue) {
+                        log.debug("{} Stripping {} value", logPrefix, value.getValue());
+                        continue;
+                    } 
+                    // ByteAttributeValue, StringAttributeValue and XMLObjectValue are Constrained to not be empty
+                }
+                
                 if (!monitor.add(value)) {
                     log.debug("{} Removing duplicate value {} of attribute '{}' from resolution result", logPrefix,
                             value, resolvedAttribute.getId());
                 }
             }
 
+            // Remove value-less attributes.
+            if (monitor.isEmpty()) {
+                log.debug("{} Removing result of attribute definition '{}', contains no values", logPrefix,
+                        definition.getId());
+                continue;
+            }
+
             resolvedAttribute.setValues(monitor);
             log.debug("{} Attribute '{}' has {} values after post-processing", logPrefix, resolvedAttribute.getId(),
                     monitor.size());
@@ -496,6 +530,7 @@ public class AttributeResolverImpl extends AbstractServiceableComponent<Attribut
 
         resolutionContext.setResolvedIdPAttributes(resolvedAttributes);
     }
+ // Checkstyle: CyclomaticComplexity ON
 
     /** {@inheritDoc} */
     @Override protected void doInitialize() throws ComponentInitializationException {
diff --git a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImplTest.java b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImplTest.java
index f09f274..79119b7 100644
--- a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImplTest.java
+++ b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/impl/AttributeResolverImplTest.java
@@ -28,6 +28,13 @@ import java.util.Map;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import net.shibboleth.idp.attribute.EmptyAttributeValue;
+import net.shibboleth.idp.attribute.EmptyAttributeValue.EmptyType;
 import net.shibboleth.idp.attribute.IdPAttribute;
 import net.shibboleth.idp.attribute.IdPAttributeValue;
 import net.shibboleth.idp.attribute.StringAttributeValue;
@@ -51,11 +58,6 @@ import net.shibboleth.utilities.java.support.collection.LazySet;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.DestroyedComponentException;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.testng.Assert;
-import org.testng.annotations.Test;
-
 /** Test case for {@link AttributeResolverImpl}. */
 public class AttributeResolverImplTest {
     private final Logger log = LoggerFactory.getLogger(AttributeResolverImplTest.class);
@@ -641,6 +643,37 @@ public class AttributeResolverImplTest {
         Assert.assertTrue(context.getResolvedIdPAttributes().containsKey("ad1"));
         Assert.assertEquals(context.getResolvedIdPAttributes().get("ad1").getValues().size(), 1);
     }
+    
+    /** Test that after resolution that the values for a resolved attribute are deduped. */
+    @Test public void resolveNullValues() throws Exception {
+        final IdPAttribute attribute = new IdPAttribute("ad1");
+        attribute.setValues(Arrays.asList(new EmptyAttributeValue(EmptyType.NULL_VALUE), new EmptyAttributeValue(EmptyType.ZERO_LENGTH_VALUE), null));
+
+        final MockAttributeDefinition definition = new MockAttributeDefinition("ad1", attribute);
+
+        final LazySet<AttributeDefinition> definitions = new LazySet<>();
+        definitions.add(definition);
+        definition.initialize();
+
+        AttributeResolverImpl resolver = newAttributeResolverImpl("foo", definitions, null, null);
+        resolver.initialize();
+
+        AttributeResolutionContext context = new AttributeResolutionContext();
+        resolver.resolveAttributes(context);
+
+        Assert.assertTrue(context.getResolvedIdPAttributes().containsKey("ad1"));
+        Assert.assertEquals(context.getResolvedIdPAttributes().get("ad1").getValues().size(),2);
+        
+        resolver = newAttributeResolverImpl("foo", definitions, null, null);
+        resolver.setStripNulls(true);
+        resolver.initialize();
+
+        context = new AttributeResolutionContext();
+        resolver.resolveAttributes(context);
+
+        Assert.assertTrue(context.getResolvedIdPAttributes().isEmpty());
+    }
+
 
     /**
      * Test that after resolution attribute definitions whose resultant attribute contains no value don't have their

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


More information about the commits mailing list