[java-identity-provider] 02/03: IDP-1450 Better handling of null properties in Mapped Attribute Definitions
Rod Widdowson
rdw at steadingsoftware.com
Wed Jul 3 08:19:03 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=42c7cbd026d043eb2fcc26703b765f27aebf7898
commit 42c7cbd026d043eb2fcc26703b765f27aebf7898
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Thu Jun 20 15:38:42 2019 +0100
IDP-1450 Better handling of null properties in Mapped Attribute Definitions
https://issues.shibboleth.net/jira/browse/IDP-1450
---
.../ad/mapped/impl/MappedAttributeDefinition.java | 6 +++++-
.../attribute/resolver/ad/mapped/impl/SourceValue.java | 10 ++++++++--
.../resolver/ad/mapped/impl/SourceValueTest.java | 3 ---
.../mapped/impl/MappedAttributeDefinitionParser.java | 2 +-
.../spring/ad/mapped/impl/SourceValueParser.java | 18 ++++++------------
.../ad/mapped/MappedAttributeDefinitionParserTest.java | 9 +++++++++
.../spring/ad/mapped/SourceValueParserTest.java | 16 ++++++++++++++++
.../resolver/spring/ad/mapped/resolver/empty.xml | 14 ++++++++++++++
.../spring/ad/mapped/resolver/sourceValueEmptyCase.xml | 10 ++++++++++
.../ad/mapped/resolver/sourceValueEmptyPartial.xml | 10 ++++++++++
10 files changed, 79 insertions(+), 19 deletions(-)
diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/MappedAttributeDefinition.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/MappedAttributeDefinition.java
index beea056..231d631 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/MappedAttributeDefinition.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/MappedAttributeDefinition.java
@@ -147,9 +147,13 @@ public class MappedAttributeDefinition extends AbstractAttributeDefinition {
*
* @param newPassThru whether the definition passes unmatched values unchanged or suppresses them.
*/
- public void setPassThru(final boolean newPassThru) {
+ public void setPassThru(final @Nullable Boolean newPassThru) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+ if (newPassThru == null) {
+ log.warn("Attribute Definition {}: empty provided as passThru", getLogPrefix());
+ return;
+ }
passThru = newPassThru;
}
diff --git a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/SourceValue.java b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/SourceValue.java
index a4ff4ff..517e11a 100644
--- a/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/SourceValue.java
+++ b/idp-attribute-resolver-impl/src/main/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/SourceValue.java
@@ -22,6 +22,9 @@ import java.util.regex.Pattern;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -37,6 +40,9 @@ import com.google.common.base.MoreObjects;
*/
public class SourceValue extends AbstractInitializableComponent {
+ /** Class logger. */
+ @Nonnull private final Logger log = LoggerFactory.getLogger(SourceValue.class);
+
/**
* Value string. This may contain regular expressions.
*/
@@ -72,14 +78,14 @@ public class SourceValue extends AbstractInitializableComponent {
/**
* Set whether case is sensitive.
*
- * @param theCaseSensitive whether case should be ignored when matching. Null defaults to false;
+ * @param theCaseSensitive whether case should be ignored when matching. Null taken as default;
*/
public void setCaseSensitive( @Nullable final Boolean theCaseSensitive) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
if (null != theCaseSensitive) {
caseSensitive = theCaseSensitive;
} else {
- caseSensitive = true;
+ log.warn("Empty value specified for case sensitive");
}
}
diff --git a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/SourceValueTest.java b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/SourceValueTest.java
index 0abc2e1..f13e56e 100644
--- a/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/SourceValueTest.java
+++ b/idp-attribute-resolver-impl/src/test/java/net/shibboleth/idp/attribute/resolver/ad/mapped/impl/SourceValueTest.java
@@ -67,9 +67,6 @@ public class SourceValueTest {
value.setCaseSensitive(false);
assertFalse(value.isCaseSensitive());
assertTrue(value.isIgnoreCase());
- value.setCaseSensitive(null);
- assertTrue(value.isCaseSensitive());
- assertFalse(value.isIgnoreCase());
}
public static SourceValue newSourceValue(final String value, final boolean ignoreCase, final boolean partialMatch)
diff --git a/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/impl/MappedAttributeDefinitionParser.java b/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/impl/MappedAttributeDefinitionParser.java
index f7215cd..1fd96bd 100644
--- a/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/impl/MappedAttributeDefinitionParser.java
+++ b/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/impl/MappedAttributeDefinitionParser.java
@@ -80,7 +80,7 @@ public class MappedAttributeDefinitionParser extends BaseResolverPluginParser {
log.info("{} Default value and passThru both specified", getLogPrefix(), getDefinitionId());
}
passThru = StringSupport.trimOrNull(defaultValueElement.getAttributeNS(null, "passThru"));
- builder.addPropertyValue("passThru", passThru);
+ builder.addPropertyValue("passThru", SpringSupport.getStringValueAsBoolean(passThru));
}
}
diff --git a/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/impl/SourceValueParser.java b/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/impl/SourceValueParser.java
index d8287d0..13a4b27 100644
--- a/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/impl/SourceValueParser.java
+++ b/idp-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/impl/SourceValueParser.java
@@ -28,6 +28,7 @@ import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser;
import org.springframework.beans.factory.xml.ParserContext;
import org.w3c.dom.Element;
+import net.shibboleth.ext.spring.util.SpringSupport;
import net.shibboleth.idp.attribute.resolver.ad.mapped.impl.SourceValue;
import net.shibboleth.idp.attribute.resolver.spring.impl.AttributeResolverNamespaceHandler;
import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
@@ -60,27 +61,20 @@ public class SourceValueParser extends AbstractSingleBeanDefinitionParser {
final String value = config.getTextContent();
builder.addPropertyValue("value", value);
- String ignoreCase = null;
- if (config.hasAttributeNS(null, "ignoreCase")) {
- ignoreCase = StringSupport.trimOrNull(config.getAttributeNS(null, "ignoreCase"));
- }
String caseSensitive = null;
if (config.hasAttributeNS(null, "caseSensitive")) {
caseSensitive = StringSupport.trimOrNull(config.getAttributeNS(null, "caseSensitive"));
- }
-
- if (caseSensitive != null) {
- builder.addPropertyValue("caseSensitive", caseSensitive);
- if (ignoreCase!=null) {
+ builder.addPropertyValue("caseSensitive", SpringSupport.getStringValueAsBoolean(caseSensitive));
+ if (config.hasAttributeNS(null, "ignoreCase")) {
log.warn("{}: Both \"caseSensitive\" and \"ignoreCase\" specified, only the former will be used",
parserContext.getReaderContext().getResource().getDescription());
}
- } else if (ignoreCase!=null) {
+ } else if (config.hasAttributeNS(null, "ignoreCase")) {
DeprecationSupport.warnOnce(ObjectType.ELEMENT,
"ignoreCase",
parserContext.getReaderContext().getResource().getDescription(),
"caseSensitive");
- builder.addPropertyValue("ignoreCase", ignoreCase);
+ builder.addPropertyValue("ignoreCase", StringSupport.trimOrNull(config.getAttributeNS(null, "ignoreCase")));
}
String partialMatch = null;
@@ -89,7 +83,7 @@ public class SourceValueParser extends AbstractSingleBeanDefinitionParser {
builder.addPropertyValue("partialMatch", partialMatch);
}
- log.debug("SourceValue value: {}, ignoreCase: {}, partialMatch: {}", value, ignoreCase, partialMatch);
+ log.debug("SourceValue value: {}, caseSensitive: {}, partialMatch: {}", value, caseSensitive, partialMatch);
}
diff --git a/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/MappedAttributeDefinitionParserTest.java b/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/MappedAttributeDefinitionParserTest.java
index 11ec9c1..9433fc4 100644
--- a/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/MappedAttributeDefinitionParserTest.java
+++ b/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/MappedAttributeDefinitionParserTest.java
@@ -54,6 +54,15 @@ public class MappedAttributeDefinitionParserTest extends BaseAttributeDefinition
assertEquals(defn.getValueMaps().size(), 2);
assertEquals(defn.getDefaultAttributeValue().getValue(), "foobar");
}
+
+ @Test public void emptyPassThru() {
+ final MappedAttributeDefinition defn = getDefinition("resolver/empty.xml");
+
+ assertFalse(defn.isPassThru());
+ assertEquals(defn.getValueMaps().size(), 2);
+ assertEquals(defn.getDefaultAttributeValue().getValue(), "foobar");
+ }
+
@Test public void noDefault() {
final MappedAttributeDefinition defn = getDefinition("resolver/mappedNoDefault.xml");
diff --git a/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/SourceValueParserTest.java b/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/SourceValueParserTest.java
index e4016b6..6ea5b24 100644
--- a/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/SourceValueParserTest.java
+++ b/idp-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/SourceValueParserTest.java
@@ -85,6 +85,22 @@ public class SourceValueParserTest extends BaseAttributeDefinitionParserTest {
}
}
+ @Test public void emptyCase() {
+ SourceValue value = getSourceValue("resolver/sourceValueEmptyCase.xml");
+
+ assertTrue(value.isCaseSensitive());
+ assertTrue(value.isPartialMatch());
+ assertEquals(value.getValue(), "sourceValueAttributes1");
+ }
+
+ @Test public void emptyPartial() {
+ SourceValue value = getSourceValue("resolver/sourceValueEmptyPartial.xml");
+
+ assertFalse(value.isCaseSensitive());
+ assertFalse(value.isPartialMatch());
+ }
+
+
@SuppressWarnings("deprecation")
@Test public void deprecated() {
SourceValue value = getSourceValue("resolver/sourceValueDeprecated.xml");
diff --git a/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/empty.xml b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/empty.xml
new file mode 100644
index 0000000..26e8b17
--- /dev/null
+++ b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/empty.xml
@@ -0,0 +1,14 @@
+<AttributeDefintion xsi:type="Mapped"
+ xmlns="urn:mace:shibboleth:2.0:resolver" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" id="Mapped"
+ xsi:schemaLocation="urn:mace:shibboleth:2.0:resolver http://shibboleth.net/schema/idp/shibboleth-attribute-resolver.xsd">
+ <InputAttributeDefinition ref="TheOrphan" />
+ <ValueMap>
+ <ReturnValue>return</ReturnValue>
+ <SourceValue>source</SourceValue>
+ </ValueMap>
+ <DefaultValue passThru=" ">foobar</DefaultValue>
+ <ValueMap>
+ <ReturnValue>return1</ReturnValue>
+ <SourceValue>source2</SourceValue>
+ </ValueMap>
+</AttributeDefintion>
diff --git a/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/sourceValueEmptyCase.xml b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/sourceValueEmptyCase.xml
new file mode 100644
index 0000000..742694c
--- /dev/null
+++ b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/sourceValueEmptyCase.xml
@@ -0,0 +1,10 @@
+<AttributeDefinition xmlns="urn:mace:shibboleth:2.0:resolver" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ id="container"
+ xsi:schemaLocation="urn:mace:shibboleth:2.0:resolver http://shibboleth.net/schema/idp/shibboleth-attribute-resolver.xsd"
+ xsi:type="Mapped">
+ <InputAttributeDefinition ref="TheOrphan" />
+ <ValueMap>
+ <ReturnValue>return</ReturnValue>
+ <SourceValue caseSensitive=" " partialMatch="true">sourceValueAttributes1</SourceValue>
+ </ValueMap>
+</AttributeDefinition>
diff --git a/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/sourceValueEmptyPartial.xml b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/sourceValueEmptyPartial.xml
new file mode 100644
index 0000000..87df2e7
--- /dev/null
+++ b/idp-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/ad/mapped/resolver/sourceValueEmptyPartial.xml
@@ -0,0 +1,10 @@
+<AttributeDefinition xmlns="urn:mace:shibboleth:2.0:resolver" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ id="container"
+ xsi:schemaLocation="urn:mace:shibboleth:2.0:resolver http://shibboleth.net/schema/idp/shibboleth-attribute-resolver.xsd"
+ xsi:type="Mapped">
+ <InputAttributeDefinition ref="TheOrphan" />
+ <ValueMap>
+ <ReturnValue>return</ReturnValue>
+ <SourceValue caseSensitive="false" partialMatch=" ">sourceValueAttributes1</SourceValue>
+ </ValueMap>
+</AttributeDefinition>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list