[java-identity-provider] 01/03: IDP-1450 Revert permissive handling of empty boolean attributes.

Rod Widdowson rdw at steadingsoftware.com
Mon Jul 22 10:36:05 EDT 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=69b40fef030dc58b2bee82d1ecda2dab215a432c

commit 69b40fef030dc58b2bee82d1ecda2dab215a432c
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Mon Jul 22 11:41:13 2019 +0100

    IDP-1450 Revert permissive handling of empty boolean attributes.
    
    https://issues.shibboleth.net/jira/browse/IDP-1450
    
    Attribute Filter Chnages
---
 .../filter/matcher/impl/AbstractStringMatcher.java | 21 ---------------
 .../saml/impl/AttributeInMetadataMatcher.java      | 30 ----------------------
 .../impl/AbstractEntityAttributePolicyRule.java    | 18 -------------
 .../AttributeRequesterInEntityGroupPolicyRule.java | 19 --------------
 .../matcher/impl/AbstractStringMatcherParser.java  |  4 +--
 .../impl/AbstractEntityAttributeRuleParser.java    |  4 +--
 .../AbstractRegistrationAuthorityRuleParser.java   |  6 ++---
 .../AttributeRequesterInEntityGroupRuleParser.java |  4 +--
 .../impl/MappedAttributeInMetadataRuleParser.java  |  6 ++---
 .../matcher/AttributeValueMatcherParserTest.java   |  3 ++-
 ...teIssuerEntityAttributeExactRuleParserTest.java |  3 ++-
 ...ributeRequesterInEntityGroupRuleParserTest.java |  3 ++-
 .../MappedAttributeInMetadataRuleParserTest.java   |  3 ++-
 13 files changed, 19 insertions(+), 105 deletions(-)

diff --git a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/AbstractStringMatcher.java b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/AbstractStringMatcher.java
index f312d67..fe129a1 100644
--- a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/AbstractStringMatcher.java
+++ b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/impl/AbstractStringMatcher.java
@@ -19,9 +19,6 @@ package net.shibboleth.idp.attribute.filter.matcher.impl;
 
 import javax.annotation.Nullable;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import net.shibboleth.idp.attribute.filter.Matcher;
 import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
 import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
@@ -31,9 +28,6 @@ import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.Object
  */
 public abstract class AbstractStringMatcher extends AbstractMatcher implements Matcher {
 
-    /** Class logger. */
-    private final Logger log = LoggerFactory.getLogger(AbstractStringMatcher.class);
-
     /** String to match for a positive evaluation. */
     private String matchString;
 
@@ -100,21 +94,6 @@ public abstract class AbstractStringMatcher extends AbstractMatcher implements M
     }
 
     /**
-     * Sets whether the match evaluation is case sensitive (Boolean Object parameter).
-     * 
-     * @param isCaseSensitive whether the match evaluation is case sensitive
-     * @since 4.0.0
-     */
-    public void setCaseSensitiveBoolean(final @Nullable Boolean isCaseSensitive) {
-        if (null == isCaseSensitive) {
-            log.warn("{} empty property passed to isCaseSensitive", getLogPrefix());
-            return;
-        }
-        caseSensitive = isCaseSensitive;
-    }
-
-    
-    /**
      * Matches the given value against the provided match string. 
      * 
      * @param value the value to evaluate
diff --git a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/saml/impl/AttributeInMetadataMatcher.java b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/saml/impl/AttributeInMetadataMatcher.java
index 551a73c..63285a6 100644
--- a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/saml/impl/AttributeInMetadataMatcher.java
+++ b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/matcher/saml/impl/AttributeInMetadataMatcher.java
@@ -97,21 +97,6 @@ public class AttributeInMetadataMatcher extends AbstractIdentifiableInitializabl
     }
 
     /**
-     * Sets whether optionally requested attributes should be matched (Boolean Object parameter).
-     * 
-     * @param flag whether optionally requested attributes should be matched
-     * @since 4.0.0
-     */
-    public void setOnlyIfRequiredBoolean(final @Nullable Boolean flag) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        if (null == flag) {
-            log.warn("{} empty property passed to onlyIfRequired", getLogPrefix());
-            return;
-        }
-        onlyIfRequired = flag;
-    }
-
-    /**
      * Sets whether optionally requested attributes should be matched.
      * 
      * @param flag whether optionally requested attributes should be matched
@@ -131,21 +116,6 @@ public class AttributeInMetadataMatcher extends AbstractIdentifiableInitializabl
     }
 
     /**
-     * Sets whether to match if the metadata contains no AttributeConsumingService (Boolean object parameter).
-     * 
-     * @param flag whether to match if the metadata contains no AttributeConsumingService
-     * @since 4.0.0
-     */
-    public void setMatchIfMetadataSilentBoolean(final @Nullable Boolean flag) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        if (null == flag) {
-            log.warn("{} empty property passed to matchIfMetadataSilent", getLogPrefix());
-            return;
-        }
-        matchIfMetadataSilent = flag;
-    }
-
-    /**
      * Sets whether to match if the metadata contains no AttributeConsumingService.
      * 
      * @param flag whether to match if the metadata contains no AttributeConsumingService
diff --git a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/saml/impl/AbstractEntityAttributePolicyRule.java b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/saml/impl/AbstractEntityAttributePolicyRule.java
index d13223f..defed0f 100644
--- a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/saml/impl/AbstractEntityAttributePolicyRule.java
+++ b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/saml/impl/AbstractEntityAttributePolicyRule.java
@@ -136,24 +136,6 @@ public abstract class AbstractEntityAttributePolicyRule extends AbstractPolicyRu
     }
 
     /**
-     * Sets whether to ignore unmapped/decoded EntityAttribute extensions as an optimization
-     * (Boolean Object parameter)
-     * 
-     * <p>Defaults to false. Only applies if {@link #nameFormat} property is set.</p>
-     * 
-     * @param flag flag to set
-     * @since 4.0.0
-     */
-    public void setIgnoreUnmappedEntityAttributesBoolean(final @Nullable Boolean flag) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        if (null == flag) {
-            log.warn("{} empty property passed to ignoreUnmappedEntityAttribute", getLogPrefix());
-            return;
-        }
-        ignoreUnmappedEntityAttributes = flag;
-    }
-
-    /**
      * Sets whether to ignore unmapped/decoded EntityAttribute extensions as an optimization.
      * 
      * <p>Defaults to false. Only applies if {@link #nameFormat} property is set.</p>
diff --git a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/saml/impl/AttributeRequesterInEntityGroupPolicyRule.java b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/saml/impl/AttributeRequesterInEntityGroupPolicyRule.java
index 2dd7cd7..5f9d935 100644
--- a/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/saml/impl/AttributeRequesterInEntityGroupPolicyRule.java
+++ b/idp-attribute-filter-impl/src/main/java/net/shibboleth/idp/attribute/filter/policyrule/saml/impl/AttributeRequesterInEntityGroupPolicyRule.java
@@ -75,25 +75,6 @@ public class AttributeRequesterInEntityGroupPolicyRule extends AbstractPolicyRul
     
     /**
      * Set whether to check a supplied {@link org.opensaml.saml.metadata.resolver.MetadataResolver}
-     * for membership in an AffiliationDescriptor as a form of group policy (Boolean parameter).
-     * 
-     * <p>Defaults to false.</p>
-     * 
-     * @param flag flag to set
-     * @since 4.0.0
-     */
-    public void setCheckAffiliationsBoolean(final @Nullable Boolean flag) {
-        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
-        if (null == flag) {
-            log.warn("{} empty property passed to checkAffiliations", getLogPrefix());
-            return;
-        }
-
-        checkAffiliations = flag;
-    }
-    
-    /**
-     * Set whether to check a supplied {@link org.opensaml.saml.metadata.resolver.MetadataResolver}
      * for membership in an AffiliationDescriptor
      * as a form of group policy.
      * 
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/matcher/impl/AbstractStringMatcherParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/matcher/impl/AbstractStringMatcherParser.java
index 5965299..10a7cc0 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/matcher/impl/AbstractStringMatcherParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/matcher/impl/AbstractStringMatcherParser.java
@@ -53,8 +53,8 @@ public abstract class AbstractStringMatcherParser extends BaseAttributeValueMatc
                         parserContext.getReaderContext().getResource().getDescription());
             }
 
-            builder.addPropertyValue("caseSensitiveBoolean", SpringSupport.getStringValueAsBoolean(
-                    StringSupport.trimOrNull(element.getAttributeNS(null, "caseSensitive"))));
+            builder.addPropertyValue("caseSensitive", SpringSupport.getStringValueAsBoolean(
+                    element.getAttributeNS(null, "caseSensitive")));
         
         } else if (element.hasAttributeNS(null, "ignoreCase")) {
             DeprecationSupport.warnOnce(ObjectType.ELEMENT,
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AbstractEntityAttributeRuleParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AbstractEntityAttributeRuleParser.java
index 9ac2c9d..0f77f0a 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AbstractEntityAttributeRuleParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AbstractEntityAttributeRuleParser.java
@@ -45,8 +45,8 @@ public abstract class AbstractEntityAttributeRuleParser extends BasePolicyRulePa
         }
         
         if (element.hasAttributeNS(null, "ignoreUnmappedEntityAttributes")) {
-            builder.addPropertyValue("ignoreUnmappedEntityAttributesBoolean", SpringSupport.getStringValueAsBoolean(
-                    StringSupport.trimOrNull(element.getAttributeNS(null, "ignoreUnmappedEntityAttributes"))));
+            builder.addPropertyValue("ignoreUnmappedEntityAttributes", SpringSupport.getStringValueAsBoolean(
+                    element.getAttributeNS(null, "ignoreUnmappedEntityAttributes")));
         }
     }
 }
\ No newline at end of file
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AbstractRegistrationAuthorityRuleParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AbstractRegistrationAuthorityRuleParser.java
index 747317c..f58cd10 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AbstractRegistrationAuthorityRuleParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AbstractRegistrationAuthorityRuleParser.java
@@ -29,7 +29,6 @@ import org.w3c.dom.Element;
 
 import net.shibboleth.ext.spring.util.SpringSupport;
 import net.shibboleth.idp.attribute.filter.spring.policyrule.BasePolicyRuleParser;
-import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 /** Spring bean definition parser that creates RegistrationAuthorityPolicyRule beans. */
 public abstract class AbstractRegistrationAuthorityRuleParser extends BasePolicyRuleParser {
@@ -48,10 +47,9 @@ public abstract class AbstractRegistrationAuthorityRuleParser extends BasePolicy
             @Nonnull final BeanDefinitionBuilder builder) {
 
         if (element.hasAttributeNS(null, MATCH_IF_METADATA_SILENT_ATTR_NAME)) {
-            final String matchIfSilent =
-                    StringSupport.trimOrNull(element.getAttributeNS(null, MATCH_IF_METADATA_SILENT_ATTR_NAME));
+            final String matchIfSilent =element.getAttributeNS(null, MATCH_IF_METADATA_SILENT_ATTR_NAME);
             log.debug("Registration Authority Filter: Match if Metadata silent = {}", matchIfSilent);
-            builder.addPropertyValue("matchIfMetadataSilent", matchIfSilent);
+            builder.addPropertyValue("matchIfMetadataSilent", SpringSupport.getStringValueAsBoolean(matchIfSilent));
         }
 
         final Attr attr = element.getAttributeNodeNS(null, REGISTRARS_ATTR_NAME);
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AttributeRequesterInEntityGroupRuleParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AttributeRequesterInEntityGroupRuleParser.java
index a004881..0fd2d8c 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AttributeRequesterInEntityGroupRuleParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/AttributeRequesterInEntityGroupRuleParser.java
@@ -49,8 +49,8 @@ public class AttributeRequesterInEntityGroupRuleParser extends BasePolicyRulePar
 
         builder.addPropertyValue("entityGroup", StringSupport.trimOrNull(element.getAttributeNS(null, "groupID")));
         if (element.hasAttributeNS(null, "checkAffiliations")) {
-            builder.addPropertyValue("checkAffiliationsBoolean", SpringSupport.getStringValueAsBoolean(
-                            StringSupport.trimOrNull(element.getAttributeNS(null, "checkAffiliations"))));
+            builder.addPropertyValue("checkAffiliations", SpringSupport.getStringValueAsBoolean(
+                            element.getAttributeNS(null, "checkAffiliations")));
         }
     }
 }
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/MappedAttributeInMetadataRuleParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/MappedAttributeInMetadataRuleParser.java
index 8be782e..7709149 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/MappedAttributeInMetadataRuleParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/saml/impl/MappedAttributeInMetadataRuleParser.java
@@ -58,13 +58,13 @@ public class MappedAttributeInMetadataRuleParser extends BaseAttributeValueMatch
         super.doParse(config, builder);
 
         if (config.hasAttributeNS(null, "onlyIfRequired")) {
-            builder.addPropertyValue("onlyIfRequiredBoolean", SpringSupport.getStringValueAsBoolean(
+            builder.addPropertyValue("onlyIfRequired", SpringSupport.getStringValueAsBoolean(
                     StringSupport.trimOrNull(config.getAttributeNS(null, "onlyIfRequired"))));
         }
 
         if (config.hasAttributeNS(null, "matchIfMetadataSilent")) {
-            builder.addPropertyValue("matchIfMetadataSilentBoolean", SpringSupport.getStringValueAsBoolean(
-                    StringSupport.trimOrNull(config.getAttributeNS(null, "matchIfMetadataSilent"))));
+            builder.addPropertyValue("matchIfMetadataSilent", SpringSupport.getStringValueAsBoolean(
+                    config.getAttributeNS(null, "matchIfMetadataSilent")));
         }
         
         if (config.hasAttributeNS(null, "objectStrategyRef")) {
diff --git a/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/matcher/AttributeValueMatcherParserTest.java b/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/matcher/AttributeValueMatcherParserTest.java
index 6558ba0..2ba5d44 100644
--- a/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/matcher/AttributeValueMatcherParserTest.java
+++ b/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/matcher/AttributeValueMatcherParserTest.java
@@ -26,6 +26,7 @@ import java.util.Map;
 import java.util.Set;
 
 import org.springframework.beans.FatalBeanException;
+import org.springframework.beans.factory.BeanCreationException;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
@@ -175,7 +176,7 @@ public class AttributeValueMatcherParserTest extends BaseAttributeFilterParserTe
         propertyCaseSensitive("false", false);
     }
 
-    @Test public void propertyEmptyCaseSensitive() throws ComponentInitializationException {
+    @Test(expectedExceptions = {BeanCreationException.class}) public void propertyEmptyCaseSensitive() throws ComponentInitializationException {
         propertyCaseSensitive("", false);
     }
 
diff --git a/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/AttributeIssuerEntityAttributeExactRuleParserTest.java b/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/AttributeIssuerEntityAttributeExactRuleParserTest.java
index c26673e..47845d7 100644
--- a/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/AttributeIssuerEntityAttributeExactRuleParserTest.java
+++ b/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/AttributeIssuerEntityAttributeExactRuleParserTest.java
@@ -19,6 +19,7 @@ package net.shibboleth.idp.attribute.filter.spring.saml;
 
 import static org.testng.Assert.*;
 
+import org.springframework.beans.factory.BeanCreationException;
 import org.testng.annotations.Test;
 
 import net.shibboleth.idp.attribute.filter.policyrule.saml.impl.AttributeIssuerEntityAttributeExactPolicyRule;
@@ -39,7 +40,7 @@ public class AttributeIssuerEntityAttributeExactRuleParserTest extends BaseAttri
         assertTrue(rule.getIgnoreUnmappedEntityAttributes());
     }
     
-    @Test public void empty() throws ComponentInitializationException {
+    @Test(expectedExceptions = {BeanCreationException.class}) public void empty() throws ComponentInitializationException {
         AttributeIssuerEntityAttributeExactPolicyRule rule =
                 (AttributeIssuerEntityAttributeExactPolicyRule) getPolicyRule("issuerEA3.xml");
 
diff --git a/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/AttributeRequesterInEntityGroupRuleParserTest.java b/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/AttributeRequesterInEntityGroupRuleParserTest.java
index 568805c..752a866 100644
--- a/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/AttributeRequesterInEntityGroupRuleParserTest.java
+++ b/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/AttributeRequesterInEntityGroupRuleParserTest.java
@@ -19,6 +19,7 @@ package net.shibboleth.idp.attribute.filter.spring.saml;
 
 import static org.testng.Assert.assertEquals;
 
+import org.springframework.beans.factory.BeanCreationException;
 import org.testng.annotations.Test;
 
 import net.shibboleth.idp.attribute.filter.policyrule.saml.impl.AttributeRequesterInEntityGroupPolicyRule;
@@ -38,7 +39,7 @@ public class AttributeRequesterInEntityGroupRuleParserTest extends  BaseAttribut
         assertEquals(rule.isCheckAffiliations(), result);
     }
 
-    @Test public void basic() throws ComponentInitializationException {
+    @Test(expectedExceptions = {BeanCreationException.class}) public void basic() throws ComponentInitializationException {
         testRule("", false);
     }
 
diff --git a/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/MappedAttributeInMetadataRuleParserTest.java b/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/MappedAttributeInMetadataRuleParserTest.java
index 0b77a51..b2c5fe5 100644
--- a/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/MappedAttributeInMetadataRuleParserTest.java
+++ b/idp-attribute-filter-spring/src/test/java/net/shibboleth/idp/attribute/filter/spring/saml/MappedAttributeInMetadataRuleParserTest.java
@@ -20,6 +20,7 @@ package net.shibboleth.idp.attribute.filter.spring.saml;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
 
+import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.context.support.GenericApplicationContext;
 import org.testng.annotations.Test;
 
@@ -67,7 +68,7 @@ public class MappedAttributeInMetadataRuleParserTest extends  BaseAttributeFilte
          test("false", false);
      }
 
-     @Test public void testEmpty() throws ComponentInitializationException {
+     @Test(expectedExceptions = {BeanCreationException.class}) public void testEmpty() throws ComponentInitializationException {
          test("", false, true);
      }
 }

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


More information about the commits mailing list