[java-identity-provider] 10/51: IDP-1121 Remove Policy and Attribute rule references

Rod Widdowson rdw at steadingsoftware.com
Wed Feb 6 08:42:41 EST 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=65a89b28bb9294c81f7fcab60c3e96c4918ebe04

commit 65a89b28bb9294c81f7fcab60c3e96c4918ebe04
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Wed Nov 28 15:26:11 2018 +0000

    IDP-1121 Remove Policy and Attribute rule references
    
    https://issues.shibboleth.net/jira/browse/IDP-1121
---
 .../attribute/filter/spring/BaseFilterParser.java  | 13 -----
 .../spring/impl/AttributeFilterPolicyParser.java   | 48 +----------------
 .../filter/spring/impl/AttributeRuleParser.java    | 63 ++++------------------
 .../idp/attribute/filter/spring/deny2.xml          | 10 ++--
 .../idp/attribute/filter/spring/policy4.xml        | 25 ++++-----
 .../idp/attribute/filter/spring/policy5.xml        | 28 +++++-----
 .../src/main/resources/schema/shibboleth-afp.xsd   | 46 +++++-----------
 7 files changed, 53 insertions(+), 180 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 8e935ac..1aa7132 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
@@ -105,19 +105,6 @@ public abstract class BaseFilterParser extends AbstractSingleBeanDefinitionParse
     }
 
     /**
-     * Gets the reference text from an element.
-     * 
-     * @param element the element to look at.
-     * @return the text.
-     * 
-     */
-    @Nullable protected String getReferenceText(@Nonnull final Element element) {
-        final String reference = StringSupport.trimOrNull(element.getAttributeNS(null, "ref"));
-
-        return reference;
-    }
-
-    /**
      * Gets the absolute reference given a possibly relative reference.
      * 
      * @param configElement component configuration element
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 664c122..d8d1a3f 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
@@ -25,9 +25,7 @@ import javax.xml.namespace.QName;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.BeanMetadataElement;
-import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.config.BeanDefinition;
-import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.ManagedList;
 import org.springframework.beans.factory.xml.ParserContext;
@@ -36,8 +34,6 @@ import org.w3c.dom.Element;
 import net.shibboleth.ext.spring.util.SpringSupport;
 import net.shibboleth.idp.attribute.filter.AttributeFilterPolicy;
 import net.shibboleth.idp.attribute.filter.spring.BaseFilterParser;
-import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
-import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 import net.shibboleth.utilities.java.support.xml.ElementSupport;
 
@@ -52,17 +48,9 @@ public class AttributeFilterPolicyParser extends BaseFilterParser {
     public static final QName TYPE_NAME = new QName(BaseFilterParser.NAMESPACE,
             "AttributeFilterPolicyType");
 
-    /** The PolicyRequirementRuleReference QName. */
-    @Deprecated public static final QName POLICY_REQUIREMENT_RULE_REF = new QName(BaseFilterParser.NAMESPACE,
-            "PolicyRequirementRuleReference");
-
     /** The AttributeRule QName. */
     private static final QName ATTRIBUTE_RULE = new QName(BaseFilterParser.NAMESPACE, "AttributeRule");
 
-    /** The AttributeRuleReference QName. */
-    @Deprecated private static final QName ATTRIBUTE_RULE_REF = new QName(BaseFilterParser.NAMESPACE,
-            "AttributeRuleReference");
-
     /** Class logger. */
     private Logger log = LoggerFactory.getLogger(AttributeFilterPolicyParser.class);
 
@@ -73,9 +61,7 @@ public class AttributeFilterPolicyParser extends BaseFilterParser {
     }
 
     /** {@inheritDoc} */
- // Checkstyle: CyclomaticComplexity OFF
-    @Override
-    protected void doParse(@Nonnull final Element config, @Nonnull final ParserContext parserContext,
+    @Override protected void doParse(@Nonnull final Element config, @Nonnull final ParserContext parserContext,
             @Nonnull final BeanDefinitionBuilder builder) {
         super.doParse(config, parserContext, builder);
 
@@ -93,26 +79,6 @@ public class AttributeFilterPolicyParser extends BaseFilterParser {
             final ManagedList<BeanDefinition> requirements =
                     SpringSupport.parseCustomElements(policyRequirements, parserContext);
             builder.addConstructorArgValue(requirements.get(0));
-        } else {
-            final List<Element> policyRequirementsRef =
-                    ElementSupport.getChildElements(config, POLICY_REQUIREMENT_RULE_REF);
-            if (policyRequirementsRef != null && policyRequirementsRef.size() > 0) {
-                    DeprecationSupport.warnOnce(ObjectType.ELEMENT, POLICY_REQUIREMENT_RULE_REF.toString(),
-                        parserContext.getReaderContext().getResource().getDescription(),  null);
-
-                final String referenceText = getReferenceText(policyRequirementsRef.get(0));
-                if (null == referenceText) {
-                    throw new BeanCreationException("Attribute Filter '" + policyId + "' no text or reference for "
-                            + POLICY_REQUIREMENT_RULE_REF);
-                }
-
-                final String reference = getAbsoluteReference(config, "PolicyRequirementRule", referenceText);
-                log.debug("Adding PolicyRequirementRule reference to {}", reference);
-                builder.addConstructorArgValue(new RuntimeBeanReference(reference));
-            } else {
-                throw new BeanCreationException("Attribute Filter '" + policyId
-                        + "' A PolicyRequirementRule or a PolicyRequirementRuleReference should be present");
-            }
         }
 
         // Get the attribute rules, both inline or referenced.
@@ -122,18 +88,6 @@ public class AttributeFilterPolicyParser extends BaseFilterParser {
             attributeRules.addAll(SpringSupport.parseCustomElements(rules, parserContext));
         }
 
-        final List<Element> rulesRef = ElementSupport.getChildElements(config, ATTRIBUTE_RULE_REF);
-        if (rulesRef != null && rulesRef.size() > 0) {
-            DeprecationSupport.warnOnce(ObjectType.ELEMENT, ATTRIBUTE_RULE_REF.toString(),
-                    parserContext.getReaderContext().getResource().getDescription(),  null);
-            
-            for (final Element ruleRef : rulesRef) {
-                final String reference = getAbsoluteReference(config, "AttributeRule", getReferenceText(ruleRef));
-                attributeRules.add(new RuntimeBeanReference(reference));
-            }
-        }
-
         builder.addConstructorArgValue(attributeRules);
     }
-    // Checkstyle: CyclomaticComplexity ON
 }
\ No newline at end of file
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 57d45bf..cec8c6b 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
@@ -23,26 +23,22 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.xml.namespace.QName;
 
-import net.shibboleth.ext.spring.util.SpringSupport;
-import net.shibboleth.idp.attribute.filter.AttributeRule;
-import net.shibboleth.idp.attribute.filter.Matcher;
-import net.shibboleth.idp.attribute.filter.spring.BaseFilterParser;
-import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
-import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
-import net.shibboleth.utilities.java.support.primitive.StringSupport;
-import net.shibboleth.utilities.java.support.xml.AttributeSupport;
-import net.shibboleth.utilities.java.support.xml.ElementSupport;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.config.BeanDefinition;
-import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.ManagedList;
 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.AttributeRule;
+import net.shibboleth.idp.attribute.filter.Matcher;
+import net.shibboleth.idp.attribute.filter.spring.BaseFilterParser;
+import net.shibboleth.utilities.java.support.primitive.StringSupport;
+import net.shibboleth.utilities.java.support.xml.AttributeSupport;
+import net.shibboleth.utilities.java.support.xml.ElementSupport;
+
 /**
  * Spring bean definition parser to configure an {@link AttributeRule}.
  */
@@ -54,14 +50,6 @@ public class AttributeRuleParser extends BaseFilterParser {
     /** Schema type name. */
     public static final QName TYPE_NAME = new QName(BaseFilterParser.NAMESPACE, "AttributeRuleType");
 
-    /** PermitValueRuleReference. */
-    @Deprecated public static final QName PERMIT_VALUE_REF = new QName(BaseFilterParser.NAMESPACE,
-            "PermitValueRuleReference");
-
-    /** DenyValueRuleReference. */
-    @Deprecated public static final QName DENY_VALUE_REF = new QName(BaseFilterParser.NAMESPACE,
-            "DenyValueRuleReference");
-
     /** permitAny Attribute. */
     public static final String PERMIT_ANY_ATTRIBUTE = "permitAny";
 
@@ -88,9 +76,7 @@ public class AttributeRuleParser extends BaseFilterParser {
 
         final List<Element> permitValueRule = ElementSupport.getChildElements(config,
                 BaseFilterParser.PERMIT_VALUE_RULE);
-        final List<Element> permitValueReference = ElementSupport.getChildElements(config, PERMIT_VALUE_REF);
         final List<Element> denyValueRule = ElementSupport.getChildElements(config, BaseFilterParser.DENY_VALUE_RULE);
-        final List<Element> denyValueReference = ElementSupport.getChildElements(config, DENY_VALUE_REF);
 
         if (permitValueRule != null && !permitValueRule.isEmpty()) {
 
@@ -100,21 +86,6 @@ public class AttributeRuleParser extends BaseFilterParser {
             builder.addPropertyValue("matcher", permitValueRules.get(0));
             builder.addPropertyValue("isDenyRule", false);
 
-        } else if (permitValueReference != null && !permitValueReference.isEmpty()) {
-            DeprecationSupport.warnOnce(ObjectType.ELEMENT, PERMIT_VALUE_REF.toString(),
-                    parserContext.getReaderContext().getResource().getDescription(),  null);
-
-            final String referenceText = getReferenceText(permitValueReference.get(0));
-            if (null == referenceText) {
-                throw new BeanCreationException("Attribute Rule '" + id + "' no text or reference for "
-                        + PERMIT_VALUE_REF);
-            }
-
-            final String reference = getAbsoluteReference(config, "PermitValueRule", referenceText);
-            log.debug("Adding PermitValueRule reference to {}", reference);
-            builder.addPropertyValue("matcher", new RuntimeBeanReference(reference));
-            builder.addPropertyValue("isDenyRule", false);
-
         } else if (denyValueRule != null && !denyValueRule.isEmpty()) {
 
             final ManagedList<BeanDefinition> denyValueRules =
@@ -123,27 +94,15 @@ public class AttributeRuleParser extends BaseFilterParser {
             builder.addPropertyValue("matcher", denyValueRules.get(0));
             builder.addPropertyValue("isDenyRule", true);
 
-        } else if (denyValueReference != null && !denyValueReference.isEmpty()) {
-            DeprecationSupport.warnOnce(ObjectType.ELEMENT, DENY_VALUE_REF.toString(),
-                    parserContext.getReaderContext().getResource().getDescription(),  null);
-
-            final String referenceText = getReferenceText(denyValueReference.get(0));
-            if (null == referenceText) {
-                throw new BeanCreationException("Attribute Rule '" + id + "' no text or reference for "
-                        + DENY_VALUE_REF);
-            }
-            final String reference = getAbsoluteReference(config, "DenyValueRule", referenceText);
-            log.debug("Adding DenyValueRule reference to {}", reference);
-            builder.addPropertyValue("matcher", new RuntimeBeanReference(reference));
-            builder.addPropertyValue("isDenyRule", true);
         } else if (config.hasAttributeNS(null, PERMIT_ANY_ATTRIBUTE)
                 && AttributeSupport.getAttributeValueAsBoolean(config.getAttributeNodeNS(null, PERMIT_ANY_ATTRIBUTE))) {
             // Note the documented restriction that permitAny cannot be property replaced.
             builder.addPropertyValue("isDenyRule", false);
             builder.addPropertyValue("matcher", Matcher.MATCHES_ALL);
         } else {
-            log.warn("Attribute rule must have one of PermitValueRule, "
-                    + "DenyValueRule or have attribute permitAny=\"true\"");
+            log.warn("{}: Attribute rule must have PermitValueRule or a DenyValueRule" +
+                    ", or have attribute permitAny=\"true\"",
+                    parserContext.getReaderContext().getResource().getDescription());
         }
     }
     // Checkstyle: CyclomaticComplexity ON
diff --git a/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/deny2.xml b/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/deny2.xml
index e643903..cdcfe92 100644
--- a/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/deny2.xml
+++ b/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/deny2.xml
@@ -3,16 +3,14 @@
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="urn:mace:shibboleth:2.0:afp"
 	xsi:schemaLocation="urn:mace:shibboleth:2.0:afp http://shibboleth.net/schema/idp/shibboleth-afp.xsd">
 
-	<DenyValueRule xsi:type="OR" id="deny">
-		<Rule xsi:type="Value" value="staff" />
-		<Rule xsi:type="Value" value="student" />
-	</DenyValueRule>
-
 	<AttributeFilterPolicy id="InCommonRelease">
 		<PolicyRequirementRule xsi:type="ANY" />
 
 		<AttributeRule attributeID="affiliation">
-			<DenyValueRuleReference ref="deny" />
+			<DenyValueRule xsi:type="OR" id="deny">
+				<Rule xsi:type="Value" value="staff" />
+				<Rule xsi:type="Value" value="student" />
+			</DenyValueRule>
 		</AttributeRule>
 
 	</AttributeFilterPolicy>
diff --git a/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/policy4.xml b/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/policy4.xml
index 920eab9..f0892e5 100644
--- a/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/policy4.xml
+++ b/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/policy4.xml
@@ -4,24 +4,21 @@
     	xmlns="urn:mace:shibboleth:2.0:afp"
         xsi:schemaLocation="urn:mace:shibboleth:2.0:afp http://shibboleth.net/schema/idp/shibboleth-afp.xsd">
                             
-	<AttributeRule attributeID="affiliation" id="wobble">
-		<PermitValueRule 
-			id="Wibble" xsi:type="OR">
-			<Rule xsi:type="Value" value="faculty" id="fac"/>
-			<Rule xsi:type="Value" value="student" id="stud"/>
-			<Rule xsi:type="Value" value="staff" id="stadd"/>
-			<Rule xsi:type="Value" value="alum" id="alum"/>
-			<Rule xsi:type="Value" value="member" id="member"/>
-			<Rule xsi:type="Value" value="affiliate" id="affilient"/>
-			<Rule xsi:type="Value" value="employee" id = "emp"/>
-		</PermitValueRule>
-	</AttributeRule>
-
 	<!-- Release affiliation to anyone in the InCommon group -->
 	<AttributeFilterPolicy id="InCommonRelease">
 		<PolicyRequirementRule xsi:type="ANY" id="pq"/>
 
-		<AttributeRuleReference ref="wobble"/>
+		<AttributeRule attributeID="affiliation" id="wobble">
+			<PermitValueRule xsi:type="OR">
+				<Rule xsi:type="Value" value="faculty" id="fac"/>
+				<Rule xsi:type="Value" value="student" id="stud"/>
+				<Rule xsi:type="Value" value="staff" id="stadd"/>
+				<Rule xsi:type="Value" value="alum" id="alum"/>
+				<Rule xsi:type="Value" value="member" id="member"/>
+				<Rule xsi:type="Value" value="affiliate" id="affilient"/>
+				<Rule xsi:type="Value" value="employee" id = "emp"/>
+			</PermitValueRule>
+		</AttributeRule>
 
 	</AttributeFilterPolicy>
 
diff --git a/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/policy5.xml b/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/policy5.xml
index 58263e6..6821774 100644
--- a/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/policy5.xml
+++ b/idp-attribute-filter-spring/src/test/resources/net/shibboleth/idp/attribute/filter/spring/policy5.xml
@@ -4,27 +4,23 @@
     	xmlns="urn:mace:shibboleth:2.0:afp"
         xsi:schemaLocation="urn:mace:shibboleth:2.0:afp http://shibboleth.net/schema/idp/shibboleth-afp.xsd">
 
-        <PermitValueRule
-            id="Wibble" xsi:type="OR">
-            <Rule xsi:type="Value" value="faculty" id="fac"/>
-            <Rule xsi:type="Value" value="student" id="stud"/>
-            <Rule xsi:type="Value" value="staff" id="stadd"/>
-            <Rule xsi:type="Value" value="alum" id="alum"/>
-            <Rule xsi:type="Value" value="member" id="member"/>
-            <Rule xsi:type="Value" value="affiliate" id="affilient"/>
-            <Rule xsi:type="Value" value="employee" id = "emp"/>
-        </PermitValueRule>
-                            
-	<AttributeRule attributeID="affiliation" id="wobble">
-		<PermitValueRuleReference ref="Wibble"/>
-	</AttributeRule>
-
 	<!-- Release affiliation to anyone in the InCommon group -->
 	<AttributeFilterPolicy id="InCommonRelease">
         <!--  Turn on rule if an attribute call firstName has value "john" -->
 		<PolicyRequirementRule xsi:type="Value" value="john" attributeID="firstName"/>
 
-		<AttributeRuleReference ref="wobble"/>
+		<AttributeRule attributeID="affiliation" id="wobble">
+	        <PermitValueRule xsi:type="OR">
+	            <Rule xsi:type="Value" value="faculty" id="fac"/>
+	            <Rule xsi:type="Value" value="student" id="stud"/>
+	            <Rule xsi:type="Value" value="staff" id="stadd"/>
+	            <Rule xsi:type="Value" value="alum" id="alum"/>
+	            <Rule xsi:type="Value" value="member" id="member"/>
+	            <Rule xsi:type="Value" value="affiliate" id="affilient"/>
+	            <Rule xsi:type="Value" value="employee" id = "emp"/>
+	        </PermitValueRule>
+	                            
+		</AttributeRule>
 
 	</AttributeFilterPolicy>
 
diff --git a/idp-schema/src/main/resources/schema/shibboleth-afp.xsd b/idp-schema/src/main/resources/schema/shibboleth-afp.xsd
index fa63c9d..66d93dd 100644
--- a/idp-schema/src/main/resources/schema/shibboleth-afp.xsd
+++ b/idp-schema/src/main/resources/schema/shibboleth-afp.xsd
@@ -85,38 +85,20 @@
         <complexContent>
             <extension base="afp:IdentityType">
                 <sequence>
-                    <choice>
-                        <element ref="afp:PolicyRequirementRule">
-                            <annotation>
-                                <documentation>
-                                    A requirement that if met signals that this filter policy should be used.
-                                </documentation>
-                            </annotation>
-                        </element>
-                        <element name="PolicyRequirementRuleReference" type="afp:ReferenceType">
-                            <annotation>
-                                <documentation>
-                                    Reference to a PolicyRequirement defined within this policy group or another.
-                                </documentation>
-                            </annotation>
-                        </element>
-                    </choice>
-                    <choice minOccurs="0" maxOccurs="unbounded">
-                        <element ref="afp:AttributeRule">
-                            <annotation>
-                                <documentation>
-                                    A rule that describes how values of an attribute will be filtered.
-                                </documentation>
-                            </annotation>
-                        </element>
-                        <element name="AttributeRuleReference" type="afp:ReferenceType">
-                            <annotation>
-                                <documentation>
-                                    Reference to a AttributeRule defined within this policy group or another.
-                                </documentation>
-                            </annotation>
-                        </element>
-                    </choice>
+	                 <element ref="afp:PolicyRequirementRule">
+	                     <annotation>
+	                         <documentation>
+	                             A requirement that if met signals that this filter policy should be used.
+	                         </documentation>
+	                     </annotation>
+	                 </element>
+	                 <element ref="afp:AttributeRule" minOccurs="0" maxOccurs="unbounded">
+	                     <annotation>
+	                         <documentation>
+	                             A rule that describes how values of an attribute will be filtered.
+	                         </documentation>
+	                     </annotation>
+	                 </element>
                 </sequence>
             </extension>
         </complexContent>

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


More information about the commits mailing list