[java-identity-provider] 04/06: IDP-1376 Warn on all duplicate filter names
Rod Widdowson
rdw at steadingsoftware.com
Sun Jan 31 14:04:40 UTC 2021
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=0694d33e690179f5f160693397d1374221114062
commit 0694d33e690179f5f160693397d1374221114062
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Sat Jan 30 15:10:01 2021 +0000
IDP-1376 Warn on all duplicate filter names
https://issues.shibboleth.net/jira/browse/IDP-1376
(within the same context).
We leverage the somewhat weird qualified name attribute which
was heretofor redundant to get all the names in the context
and then warn - this regardless of whether it matters or not.
a
---
.../attribute/filter/spring/BaseFilterParser.java | 88 ++++++++++++++++++++--
.../filter/spring/basic/impl/AndMatcherParser.java | 5 +-
.../filter/spring/basic/impl/NotMatcherParser.java | 2 +-
.../filter/spring/basic/impl/OrMatcherParser.java | 5 +-
.../spring/basic/impl/ScriptedMatcherParser.java | 2 +-
.../impl/AttributeFilterPolicyGroupParser.java | 19 +++--
.../spring/impl/AttributeFilterPolicyParser.java | 4 +-
.../filter/spring/impl/AttributeRuleParser.java | 2 +-
.../matcher/BaseAttributeValueMatcherParser.java | 2 +-
.../spring/policyrule/BasePolicyRuleParser.java | 2 +-
.../idp/attribute/filter/spring/deny1.xml | 4 +-
11 files changed, 103 insertions(+), 32 deletions(-)
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/BaseFilterParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/BaseFilterParser.java
index e4aa2f234..d8531c349 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/BaseFilterParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/BaseFilterParser.java
@@ -17,11 +17,17 @@
package net.shibboleth.idp.attribute.filter.spring;
+import java.util.Collection;
+import java.util.HashSet;
+
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.xml.namespace.QName;
+import net.shibboleth.ext.spring.util.SpringSupport;
+import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
import net.shibboleth.utilities.java.support.security.IdentifierGenerationStrategy;
import net.shibboleth.utilities.java.support.security.impl.RandomIdentifierGenerationStrategy;
@@ -32,6 +38,7 @@ import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.beans.factory.support.AbstractBeanDefinition;
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.support.ManagedList;
import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser;
import org.springframework.beans.factory.xml.ParserContext;
import org.w3c.dom.Element;
@@ -65,13 +72,15 @@ public abstract class BaseFilterParser extends AbstractSingleBeanDefinitionParse
/** DenyValueRule. */
public static final QName DENY_VALUE_RULE = new QName(NAMESPACE, "DenyValueRule");
+
+ /** The attribute name for the qualified id.*/
+ public static final String QUALIFIED_ID = "qualifiedId";
/** Generator of unique IDs. */
private static IdentifierGenerationStrategy idGen = new RandomIdentifierGenerationStrategy();
/** Class logger. */
- private final Logger log = LoggerFactory.getLogger(BaseFilterParser.class);
-
+ private static final Logger LOG = LoggerFactory.getLogger(BaseFilterParser.class);
/**
* Generates an ID for a filter engine component. If the given localId is null a random one will be generated.
@@ -156,21 +165,21 @@ public abstract class BaseFilterParser extends AbstractSingleBeanDefinitionParse
final String generatedId = getQualifiedId(element, element.getLocalName(), suppliedId);
if (suppliedId == null) {
- log.trace("Element '{}' did not contain an 'id' attribute. Generated id '{}' will be used",
+ LOG.trace("Element '{}' did not contain an 'id' attribute. Generated id '{}' will be used",
element.getLocalName(), generatedId);
} else {
- log.debug("Element '{}' 'id' attribute '{}' is mapped to '{}'", element.getLocalName(), suppliedId,
+ LOG.debug("Element '{}' 'id' attribute '{}' is mapped to '{}'", element.getLocalName(), suppliedId,
generatedId);
}
- builder.getBeanDefinition().setAttribute("qualifiedId", generatedId);
+ builder.getBeanDefinition().setAttribute(BaseFilterParser.QUALIFIED_ID, generatedId);
}
/** {@inheritDoc} */
@Override @Nonnull @NotEmpty protected String resolveId(@Nonnull final Element configElement,
@Nonnull final AbstractBeanDefinition beanDefinition, @Nonnull final ParserContext parserContext) {
- return beanDefinition.getAttribute("qualifiedId").toString();
+ return beanDefinition.getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
}
/**
@@ -196,7 +205,72 @@ public abstract class BaseFilterParser extends AbstractSingleBeanDefinitionParse
}
elem = ElementSupport.getElementAncestor(elem);
} while (elem != null);
- log.warn("Element '{}' : could not find schema defined parent");
+ LOG.warn("Element '{}' : could not find schema defined parent");
return false;
}
+
+ /**
+ * Parse list of elements into bean definitions which are inserted into the parent context.
+ * This is like {{@link SpringSupport#parseCustomElements(Collection, ParserContext)} but with
+ * qualifiedName warnings (and none of the parent bena stuff - which we do not need)
+ *
+ * @param elements list of elements to parse
+ * @param parserContext current parsing context
+ *
+ */
+ @Nullable public static void parseCustomElements(
+ @Nullable @NonnullElements final Collection<Element> elements, @Nonnull final ParserContext parserContext) {
+ if (elements == null) {
+ return;
+ }
+
+ final HashSet<String> beanNames = new HashSet<>(elements.size());
+ for (final Element e : elements) {
+ if (e != null) {
+ final BeanDefinition def = parserContext.getDelegate().parseCustomElement(e, null);
+ final Object name = def.getAttribute(QUALIFIED_ID);
+ if (name != null && !beanNames.add(name.toString())) {
+ LOG.warn("Duplicate filter element id '{}' found", name);
+ }
+ }
+ }
+ }
+
+ /**
+ * Parse list of elements into bean definitions.
+ * This is like {{@link SpringSupport#parseCustomElements(Collection, ParserContext, BeanDefinitionBuilder)}
+ * but with qualifiedName warnings.
+ *
+ * @param elements list of elements to parse
+ * @param parserContext current parsing context
+ * @param parentBuilder the builder we are going to insert into
+ *
+ * @return list of bean definitions
+ */
+ @Nullable public static ManagedList<BeanDefinition> parseCustomElements(
+ @Nullable @NonnullElements final Collection<Element> elements,
+ @Nonnull final ParserContext parserContext,
+ @Nonnull final BeanDefinitionBuilder parentBuilder) {
+
+ if (elements == null) {
+ return null;
+ }
+ Constraint.isNotNull(parentBuilder, "parentBuilder must not be null");
+
+ final ManagedList<BeanDefinition> definitions = new ManagedList<>(elements.size());
+ final HashSet<String> beanNames = new HashSet<>(elements.size());
+ for (final Element e : elements) {
+ if (e != null) {
+ final BeanDefinition def = SpringSupport.parseCustomElement(e, parserContext, parentBuilder, false);
+ definitions.add(def);
+ final Object name = def.getAttribute(QUALIFIED_ID);
+ if (name != null && !beanNames.add(name.toString())) {
+ LOG.warn("Duplicate filter element name {} found", name);
+ }
+ }
+ }
+
+ return definitions;
+ }
+
}
\ No newline at end of file
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/AndMatcherParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/AndMatcherParser.java
index 9e04aac9c..eb1287cbb 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/AndMatcherParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/AndMatcherParser.java
@@ -26,7 +26,6 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder;
import org.springframework.beans.factory.xml.ParserContext;
import org.w3c.dom.Element;
-import net.shibboleth.ext.spring.util.SpringSupport;
import net.shibboleth.idp.attribute.filter.matcher.logic.impl.AndMatcher;
import net.shibboleth.idp.attribute.filter.policyrule.logic.impl.AndPolicyRule;
import net.shibboleth.idp.attribute.filter.spring.BaseFilterParser;
@@ -58,7 +57,7 @@ public class AndMatcherParser extends BaseFilterParser {
@Nonnull final BeanDefinitionBuilder builder) {
super.doParse(configElement, parserContext, builder);
- final String myId = builder.getBeanDefinition().getAttribute("qualifiedId").toString();
+ final String myId = builder.getBeanDefinition().getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
builder.addPropertyValue("id", myId);
@@ -66,7 +65,7 @@ public class AndMatcherParser extends BaseFilterParser {
ElementSupport.getChildElementsByTagNameNS(configElement, BaseFilterParser.NAMESPACE, "Rule");
builder.addPropertyValue("subsidiaries",
- SpringSupport.parseCustomElements(ruleElements, parserContext, builder));
+ BaseFilterParser.parseCustomElements(ruleElements, parserContext, builder));
}
}
\ No newline at end of file
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/NotMatcherParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/NotMatcherParser.java
index d9ff2f4e4..69442b268 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/NotMatcherParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/NotMatcherParser.java
@@ -53,7 +53,7 @@ public class NotMatcherParser extends BaseFilterParser {
@Nonnull final BeanDefinitionBuilder builder) {
super.doParse(configElement, parserContext, builder);
- final String myId = builder.getBeanDefinition().getAttribute("qualifiedId").toString();
+ final String myId = builder.getBeanDefinition().getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
builder.addPropertyValue("id", myId);
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/OrMatcherParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/OrMatcherParser.java
index 65f128cd7..9c06f36c0 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/OrMatcherParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/OrMatcherParser.java
@@ -26,7 +26,6 @@ import org.springframework.beans.factory.support.BeanDefinitionBuilder;
import org.springframework.beans.factory.xml.ParserContext;
import org.w3c.dom.Element;
-import net.shibboleth.ext.spring.util.SpringSupport;
import net.shibboleth.idp.attribute.filter.matcher.logic.impl.OrMatcher;
import net.shibboleth.idp.attribute.filter.policyrule.logic.impl.OrPolicyRule;
import net.shibboleth.idp.attribute.filter.spring.BaseFilterParser;
@@ -58,7 +57,7 @@ public class OrMatcherParser extends BaseFilterParser {
@Nonnull final BeanDefinitionBuilder builder) {
super.doParse(configElement, parserContext, builder);
- final String myId = builder.getBeanDefinition().getAttribute("qualifiedId").toString();
+ final String myId = builder.getBeanDefinition().getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
builder.addPropertyValue("id", myId);
@@ -66,7 +65,7 @@ public class OrMatcherParser extends BaseFilterParser {
ElementSupport.getChildElementsByTagNameNS(configElement, BaseFilterParser.NAMESPACE, "Rule");
builder.addPropertyValue("subsidiaries",
- SpringSupport.parseCustomElements(ruleElements, parserContext, builder));
+ BaseFilterParser.parseCustomElements(ruleElements, parserContext, builder));
}
}
\ No newline at end of file
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/ScriptedMatcherParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/ScriptedMatcherParser.java
index 128697dc4..50625d130 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/ScriptedMatcherParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/basic/impl/ScriptedMatcherParser.java
@@ -69,7 +69,7 @@ public class ScriptedMatcherParser extends BaseFilterParser {
@Nonnull final BeanDefinitionBuilder builder) {
super.doParse(config, parserContext, builder);
- final String myId = builder.getBeanDefinition().getAttribute("qualifiedId").toString();
+ final String myId = builder.getBeanDefinition().getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
final String logPrefix = new StringBuilder("Scipted Filter '").append(myId).append("' :").toString();
final BeanDefinitionBuilder scriptBuilder =
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterPolicyGroupParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterPolicyGroupParser.java
index 91a8beb1b..16ebef6f2 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterPolicyGroupParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterPolicyGroupParser.java
@@ -22,11 +22,6 @@ import java.util.Map;
import javax.xml.namespace.QName;
-import net.shibboleth.ext.spring.util.SpringSupport;
-import net.shibboleth.idp.attribute.filter.spring.BaseFilterParser;
-import net.shibboleth.utilities.java.support.primitive.StringSupport;
-import net.shibboleth.utilities.java.support.xml.ElementSupport;
-
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.config.BeanDefinition;
@@ -34,6 +29,10 @@ import org.springframework.beans.factory.xml.BeanDefinitionParser;
import org.springframework.beans.factory.xml.ParserContext;
import org.w3c.dom.Element;
+import net.shibboleth.idp.attribute.filter.spring.BaseFilterParser;
+import net.shibboleth.utilities.java.support.primitive.StringSupport;
+import net.shibboleth.utilities.java.support.xml.ElementSupport;
+
/**
* Bean definition parser for <afp:AttributeFilterPolicyGroup>, top top level of the filter "stack".
*
@@ -75,23 +74,23 @@ public class AttributeFilterPolicyGroupParser implements BeanDefinitionParser {
//
children = childrenMap.get(new QName(BaseFilterParser.NAMESPACE, "PolicyRequirementRule"));
- SpringSupport.parseCustomElements(children, context);
+ BaseFilterParser.parseCustomElements(children, context);
children = childrenMap.get(new QName(BaseFilterParser.NAMESPACE, "AttributeRule"));
- SpringSupport.parseCustomElements(children, context);
+ BaseFilterParser.parseCustomElements(children, context);
children = childrenMap.get(new QName(BaseFilterParser.NAMESPACE, "PermitValueRule"));
- SpringSupport.parseCustomElements(children, context);
+ BaseFilterParser.parseCustomElements(children, context);
children = childrenMap.get(new QName(BaseFilterParser.NAMESPACE, "DenyValueRule"));
- SpringSupport.parseCustomElements(children, context);
+ BaseFilterParser.parseCustomElements(children, context);
//
// The actual policies
//
children = childrenMap.get(new QName(BaseFilterParser.NAMESPACE, "AttributeFilterPolicy"));
- SpringSupport.parseCustomElements(children, context);
+ BaseFilterParser.parseCustomElements(children, context);
return null;
}
}
\ No newline at end of file
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterPolicyParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterPolicyParser.java
index c52810338..87d27ceac 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterPolicyParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeFilterPolicyParser.java
@@ -66,7 +66,7 @@ public class AttributeFilterPolicyParser extends BaseFilterParser {
String policyId = StringSupport.trimOrNull(config.getAttributeNS(null, "id"));
if (null == policyId) {
- policyId = builder.getBeanDefinition().getAttribute("qualifiedId").toString();
+ policyId = builder.getBeanDefinition().getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
}
log.debug("Parsing configuration for attribute filter policy: {}", policyId);
builder.addConstructorArgValue(policyId);
@@ -76,7 +76,7 @@ public class AttributeFilterPolicyParser extends BaseFilterParser {
BaseFilterParser.POLICY_REQUIREMENT_RULE);
if (policyRequirements != null && policyRequirements.size() > 0) {
final ManagedList<BeanDefinition> requirements =
- SpringSupport.parseCustomElements(policyRequirements, parserContext, builder);
+ BaseFilterParser.parseCustomElements(policyRequirements, parserContext, builder);
builder.addConstructorArgValue(requirements.get(0));
}
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeRuleParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeRuleParser.java
index 03a026781..858ba8f30 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeRuleParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/impl/AttributeRuleParser.java
@@ -65,7 +65,7 @@ public class AttributeRuleParser extends BaseFilterParser {
@Nonnull final BeanDefinitionBuilder builder) {
super.doParse(config, parserContext, builder);
- final String id = builder.getBeanDefinition().getAttribute("qualifiedId").toString();
+ final String id = builder.getBeanDefinition().getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
builder.addPropertyValue("id", id);
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/matcher/BaseAttributeValueMatcherParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/matcher/BaseAttributeValueMatcherParser.java
index 8f8ab9f85..e9abce429 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/matcher/BaseAttributeValueMatcherParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/matcher/BaseAttributeValueMatcherParser.java
@@ -93,7 +93,7 @@ public abstract class BaseAttributeValueMatcherParser extends BaseFilterParser {
@Nonnull final BeanDefinitionBuilder builder) {
super.doParse(element, parserContext, builder);
- final String myId = builder.getBeanDefinition().getAttribute("qualifiedId").toString();
+ final String myId = builder.getBeanDefinition().getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
builder.addPropertyValue("id", myId);
diff --git a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/policyrule/BasePolicyRuleParser.java b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/policyrule/BasePolicyRuleParser.java
index 0b6d22810..8a96fc9f1 100644
--- a/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/policyrule/BasePolicyRuleParser.java
+++ b/idp-attribute-filter-spring/src/main/java/net/shibboleth/idp/attribute/filter/spring/policyrule/BasePolicyRuleParser.java
@@ -76,7 +76,7 @@ public abstract class BasePolicyRuleParser extends BaseFilterParser {
@Nonnull final BeanDefinitionBuilder builder) {
super.doParse(element, parserContext, builder);
- final String myId = builder.getBeanDefinition().getAttribute("qualifiedId").toString();
+ final String myId = builder.getBeanDefinition().getAttribute(BaseFilterParser.QUALIFIED_ID).toString();
builder.addPropertyValue("id", myId);
diff --git a/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/deny1.xml b/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/deny1.xml
index 5f3d864ad..d3abd3950 100644
--- a/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/deny1.xml
+++ b/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/deny1.xml
@@ -5,7 +5,7 @@
<AttributeFilterPolicy id="InCommonRelease">
<PolicyRequirementRule xsi:type="ANY" />
- <AttributeRule attributeID="affiliation">
+ <AttributeRule attributeID="affiliation" id="deny">
<DenyValueRule xsi:type="OR">
<Rule xsi:type="Value" value="staff" />
<Rule xsi:type="Value" value="student" />
@@ -15,7 +15,7 @@
<AttributeFilterPolicy id="InCommonRelease2">
<PolicyRequirementRule xsi:type="ANY" />
- <AttributeRule attributeID="affiliation">
+ <AttributeRule attributeID="affiliation" id="OK">
<PermitValueRule xsi:type="ANY" />
</AttributeRule>
</AttributeFilterPolicy>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list