From ee410a379a43d33251be98f6f91c2d2b128486ef Mon Sep 17 00:00:00 2001
From: Ian Young <ian@iay.org.uk>
Date: Thu, 4 Apr 2013 13:27:22 +0000
Subject: [PATCH] Major refactoring of the fixup system. Move everything
 related to fixups into the mdx/uk directory; move the major related beans
 definitions into the generate spring definition file, as they are not (and
 should not) be used elsewhere. Split the existing fixups definitions
 (applying and checking for fixups) into two: one set for the EncryptionMethod
 fixup and one set for the IdP KeyDescriptor/@use fixup. Move the application
 of fixups away from the input channels and into the output pipelines. Move
 the generation of the test aggregate into a pipeline (actually, a composite
 stage) of its own, so that it's more similar-looking to the other output
 pipelines. This is a pure refactoring, with no output changes as a result. 
 However, we're now ready to start publishing EncryptionMethod elements in
 selected aggregates if and when desired.

---
 mdx/uk/beans.xml                              |  36 ----
 mdx/uk/check_fixup_encmethod.xsl              |  36 ++++
 .../check_fixup_keyuse.xsl}                   |  37 +---
 mdx/uk/{fixups.xsl => fixup_keyuse.xsl}       |  19 +-
 mdx/uk/generate.xml                           | 170 +++++++++++++++---
 mdx/validation-beans.xml                      |  12 --
 6 files changed, 195 insertions(+), 115 deletions(-)
 create mode 100644 mdx/uk/check_fixup_encmethod.xsl
 rename mdx/{_rules/check_fixups.xsl => uk/check_fixup_keyuse.xsl} (50%)
 rename mdx/uk/{fixups.xsl => fixup_keyuse.xsl} (68%)

diff --git a/mdx/uk/beans.xml b/mdx/uk/beans.xml
index 77a83069..0bfeb525 100644
--- a/mdx/uk/beans.xml
+++ b/mdx/uk/beans.xml
@@ -252,27 +252,6 @@
     </bean>
     
     
-    <!--
-        uk_checkPublishable
-        
-        Check an aggregate metadata document for publishability.  This is applied during
-        all UK publication flows prior to any signature step.  It is not applied to
-        export flows, for which we desire the closest possible correspondence to
-        the registered metadata.
-    -->
-    <bean id="uk_checkPublishable" parent="composite_parent"
-        p:id="uk_checkPublishable">
-        <property name="composedStages">
-            <list>
-                <ref bean="checkSchemas"/>
-                <ref bean="check_aggregate"/>
-                <ref bean="check_filtered"/>
-                <ref bean="check_fixups"/>
-            </list>
-        </property>
-    </bean>
-    
-
     <!--
         uk_trustRootsDocument
         
@@ -340,21 +319,6 @@
     </bean>
     
     
-    <!--
-        uk_performFixups
-        
-        This stage performs any fixup actions required before publication to UK federation members.
-    -->
-    <bean id="uk_performFixups" parent="xslt_parent"
-        p:id="uk_performFixups">
-        <property name="xslResource">
-            <bean parent="file_parent">
-                <constructor-arg value="#{ systemProperties['basedir'] }/mdx/uk/fixups.xsl"/>
-            </bean>
-        </property>
-    </bean>
-    
-    
     <!--
         Populate UKId values from entities.
     -->
diff --git a/mdx/uk/check_fixup_encmethod.xsl b/mdx/uk/check_fixup_encmethod.xsl
new file mode 100644
index 00000000..0e5825cf
--- /dev/null
+++ b/mdx/uk/check_fixup_encmethod.xsl
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+	check_fixup_encmethod.xsl
+
+-->
+<xsl:stylesheet version="1.0"
+	xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+	xmlns:ds="http://www.w3.org/2000/09/xmldsig#"
+	xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
+	xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
+	xmlns:shibmd="urn:mace:shibboleth:metadata:1.0"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xmlns:idpdisc="urn:oasis:names:tc:SAML:profiles:SSO:idp-discovery-protocol"
+	xmlns="urn:oasis:names:tc:SAML:2.0:metadata">
+
+	<!--
+		Common support functions.
+	-->
+	<xsl:import href="../_rules/check_framework.xsl"/>
+
+	
+	<!--
+		Use of EncryptionMethod within KeyDescriptor causes metadata loading problems
+		for OpenSAML-C 2.0.
+		
+		See https://wiki.shibboleth.net/confluence/display/SHIB2/MetadataCorrectness#MetadataCorrectness-Version2.0
+	-->
+	<xsl:template match="md:KeyDescriptor/md:EncryptionMethod">
+		<xsl:call-template name="error">
+			<xsl:with-param name="m">KeyDescriptor contains EncryptionMethod: OpenSAML-C 2.0 problem</xsl:with-param>
+		</xsl:call-template>
+	</xsl:template>
+	
+	
+</xsl:stylesheet>
diff --git a/mdx/_rules/check_fixups.xsl b/mdx/uk/check_fixup_keyuse.xsl
similarity index 50%
rename from mdx/_rules/check_fixups.xsl
rename to mdx/uk/check_fixup_keyuse.xsl
index 53f90ee6..cb91fe5d 100644
--- a/mdx/_rules/check_fixups.xsl
+++ b/mdx/uk/check_fixup_keyuse.xsl
@@ -1,12 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
 
-	check_fixups.xsl
-
-	This checking ruleset verifies that certain fixups have been performed on the
-	metadata before it is published.
-	
-	Author: Ian A. Young <ian@iay.org.uk>
+	check_fixup_keyuse.xsl
 
 -->
 <xsl:stylesheet version="1.0"
@@ -22,17 +17,17 @@
 	<!--
 		Common support functions.
 	-->
-	<xsl:import href="check_framework.xsl"/>
+	<xsl:import href="../_rules/check_framework.xsl"/>
 
 	
 	<!--
-		Checks for an IdP whose KeyDescriptor elements do not include a @use attribute.
-		This causes problems with the Shibboleth 1.3 SP prior to V1.3.1, which
-		interprets this as "no use permitted" rather than "either signing or encryption use
-		permitted".
-		
-		Two checks are required, one for each of the IdP role descriptors.
-	-->
+        Checks for an IdP whose KeyDescriptor elements do not include a @use attribute.
+        This causes problems with the Shibboleth 1.3 SP prior to V1.3.1, which
+        interprets this as "no use permitted" rather than "either signing or encryption use
+        permitted".
+        
+        Two checks are required, one for each of the IdP role descriptors.
+    -->
 	
 	<xsl:template match="md:IDPSSODescriptor/md:KeyDescriptor[not(@use)]">
 		<xsl:call-template name="error">
@@ -45,19 +40,5 @@
 			<xsl:with-param name="m">IdP AA KeyDescriptor lacking @use</xsl:with-param>
 		</xsl:call-template>
 	</xsl:template>
-	
-
-	<!--
-		Use of EncryptionMethod within KeyDescriptor causes metadata loading problems
-		for OpenSAML-C 2.0.
 		
-		See https://wiki.shibboleth.net/confluence/display/SHIB2/MetadataCorrectness#MetadataCorrectness-Version2.0
-	-->
-	<xsl:template match="md:KeyDescriptor/md:EncryptionMethod">
-		<xsl:call-template name="error">
-			<xsl:with-param name="m">KeyDescriptor contains EncryptionMethod: OpenSAML-C 2.0 problem</xsl:with-param>
-		</xsl:call-template>
-	</xsl:template>
-	
-	
 </xsl:stylesheet>
diff --git a/mdx/uk/fixups.xsl b/mdx/uk/fixup_keyuse.xsl
similarity index 68%
rename from mdx/uk/fixups.xsl
rename to mdx/uk/fixup_keyuse.xsl
index 403d9a3e..04afcc7c 100644
--- a/mdx/uk/fixups.xsl
+++ b/mdx/uk/fixup_keyuse.xsl
@@ -1,21 +1,15 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
 
-	fixups.xsl
-	
-	XSL stylesheet to perform any fixups required to an EntityDescriptor prior to
-	publication to the UK federation membership.
+	fixup_keyuse.xsl
 	
 -->
 <xsl:stylesheet version="1.0"
-	xmlns:ds="http://www.w3.org/2000/09/xmldsig#"
 	xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
-	xmlns:ukfedlabel="http://ukfederation.org.uk/2006/11/label"
 	
 	xmlns="urn:oasis:names:tc:SAML:2.0:metadata"
-    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
-	exclude-result-prefixes="xsi xsl">
+	exclude-result-prefixes="xsl">
 
 	<!--Force UTF-8 encoding for the output.-->
 	<xsl:output omit-xml-declaration="no" method="xml" encoding="UTF-8" indent="yes"/>
@@ -34,15 +28,6 @@
 	</xsl:template>
 	
 	
-	<!--
-		Remove any EncryptionMethod elements within KeyDescriptor elements
-		to avoid triggering a problem in OpenSAML-C 2.0.
-		
-		See https://wiki.shibboleth.net/confluence/display/SHIB2/MetadataCorrectness#MetadataCorrectness-Version2.0
-	-->
-	<xsl:template match="md:KeyDescriptor/md:EncryptionMethod"/>
-	
-		
 	<!--
         *********************************************
         ***                                       ***
diff --git a/mdx/uk/generate.xml b/mdx/uk/generate.xml
index f8fc5b29..67296377 100644
--- a/mdx/uk/generate.xml
+++ b/mdx/uk/generate.xml
@@ -28,6 +28,120 @@
     <import resource="../us_incommon/beans.xml"/>
 
 
+    <!--
+        ***********************
+        ***                 ***
+        ***   F I X U P S   ***
+        ***                 ***
+        ***********************
+    -->
+    
+    <!--
+        Published UK federation metadata for consumption by federation
+        members has a couple of restrictions arising from known bugs in
+        early software.  We address these by "fixups" which transform the
+        metadata into a form which doesn't trigger the bug, and verify that
+        each constraints is met before publication.
+        
+        We apply fixups "late" in each publication pipeline, so that
+        the metadata is only transformed if required for a particular
+        output document.  Our export aggregate, for example, does
+        not have any of the fixups applied.
+        
+        In the long term, we'd hope to retire these fixups entirely as the
+        software they to cater for is retired.
+    -->
+    
+    <!--
+        fixup_EncryptionMethod
+        
+        Remove all md:EncryptionMethod elements.
+        
+        This is normally done to avoid triggering a problem in OpenSAML-C 2.0.
+        
+        See https://wiki.shibboleth.net/confluence/display/SHIB2/MetadataCorrectness#MetadataCorrectness-Version20
+    -->
+    <bean id="fixup_EncryptionMethod" parent="stripElement_parent"
+        p:id="fixup_EncryptionMethod"
+        p:elementName="EncryptionMethod"
+        p:elementNamespace="urn:oasis:names:tc:SAML:2.0:metadata"/>
+    
+    <!--
+        fixup_keyuse
+        
+        Patch any @use-less KeyName descriptors in IdP roles
+        for the benefit of Shib SPs pre-1.3.1.
+    -->
+    <bean id="fixup_keyuse" parent="xslt_parent"
+        p:id="fixup_keyuse">
+        <property name="xslResource">
+            <bean parent="file_parent">
+                <constructor-arg value="#{ systemProperties['basedir'] }/mdx/uk/fixup_keyuse.xsl"/>
+            </bean>
+        </property>
+    </bean>
+    
+    <!--
+        performOtherFixups
+        
+        Perform all fixup actions which are required for *every* output document.
+        
+        Fixup actions which are only required in specific output pipelines need to
+        appear there individually.
+    -->
+    <bean id="performOtherFixups" parent="composite_parent"
+        p:id="performOtherFixups">
+        <property name="composedStages">
+            <list>
+                <ref bean="fixup_keyuse"/>
+            </list>
+        </property>
+    </bean>
+    
+    <!--
+        check_fixup_encmethod
+    -->
+    <bean id="check_fixup_encmethod" parent="check_xslt_parent"
+        p:id="check_fixup_encmethod">
+        <property name="xslResource">
+            <bean parent="file_parent">
+                <constructor-arg value="#{ systemProperties['basedir'] }/mdx/uk/check_fixup_encmethod.xsl"/>
+            </bean>
+        </property>
+    </bean>
+    
+    <!--
+        check_fixup_keyuse
+    -->
+    <bean id="check_fixup_keyuse" parent="check_xslt_parent"
+        p:id="check_fixup_keyuse">
+        <property name="xslResource">
+            <bean parent="file_parent">
+                <constructor-arg value="#{ systemProperties['basedir'] }/mdx/uk/check_fixup_keyuse.xsl"/>
+            </bean>
+        </property>
+    </bean>
+    
+    <!--
+        checkPublishable
+        
+        Check an aggregate metadata document for publishability.  This is applied during
+        all UK publication flows prior to any signature step.  It is not applied to
+        export flows, for which we desire the closest possible correspondence to
+        the registered metadata.
+    -->
+    <bean id="checkPublishable" parent="composite_parent"
+        p:id="checkPublishable">
+        <property name="composedStages">
+            <list>
+                <ref bean="checkSchemas"/>
+                <ref bean="check_aggregate"/>
+                <ref bean="check_filtered"/>
+                <ref bean="check_fixup_keyuse"/>
+            </list>
+        </property>
+    </bean>
+    
     <!--
         *******************************************
         ***                                     ***
@@ -41,7 +155,6 @@
         <property name="stages">
             <list>
                 <ref bean="ie_edugate_exportedEntities"/>
-                <ref bean="uk_performFixups"/>
                 <ref bean="uk_fix_mailto"/>
                 <ref bean="uk_hide_idps"/>
             </list>
@@ -53,7 +166,6 @@
         <property name="stages">
             <list>
                 <ref bean="us_incommon_exportedEntities"/>
-                <ref bean="uk_performFixups"/>
                 <ref bean="uk_fix_mailto"/>
                 <ref bean="uk_hide_idps"/>
             </list>
@@ -156,12 +268,15 @@
         <property name="stages">
             <list>
                 <ref bean="uk_assemble"/>
+                <ref bean="fixup_EncryptionMethod"/>
+                <ref bean="performOtherFixups"/>
                 <ref bean="uk_addTrustRoots"/>
                 <ref bean="uk_finaliseProduction"/>
                 <ref bean="uk_normaliseNamespaces"/>
                 
                 <!-- production aggregate MUST pass publishability test -->
-                <ref bean="uk_checkPublishable"/>
+                <ref bean="checkPublishable"/>
+                <ref bean="check_fixup_encmethod"/>
                 <ref bean="errorTerminatingFilter"/>
                 
                 <ref bean="serializeUnsignedProductionAggregate"/>
@@ -196,11 +311,14 @@
         <property name="stages">
             <list>
                 <ref bean="uk_assemble"/>
+                <ref bean="fixup_EncryptionMethod"/>
+                <ref bean="performOtherFixups"/>
                 <ref bean="uk_finaliseProduction"/>
                 <ref bean="uk_normaliseNamespaces"/>
 
                 <!-- WAYF aggregate MUST pass publishability test -->
-                <ref bean="uk_checkPublishable"/>
+                <ref bean="checkPublishable"/>
+                <ref bean="check_fixup_encmethod"/>
                 <ref bean="errorTerminatingFilter"/>
 
                 <ref bean="serializeUnsignedWayfAggregate"/>
@@ -253,13 +371,16 @@
         <property name="stages">
             <list>
                 <ref bean="uk_assemble"/>
+                <ref bean="fixup_EncryptionMethod"/>
+                <ref bean="performOtherFixups"/>
                 <ref bean="removeEmptyExtensions"/>
                 <ref bean="uk_addTrustRoots"/>
                 <ref bean="uk_finaliseFallback"/>
                 <ref bean="uk_normaliseFallback"/>
                 
                 <!-- fallback aggregate MUST pass publishability test -->
-                <ref bean="uk_checkPublishable"/>
+                <ref bean="checkPublishable"/>
+                <ref bean="check_fixup_encmethod"/>
                 <ref bean="errorTerminatingFilter"/>
                 
                 <ref bean="serializeUnsignedFallbackAggregate"/>
@@ -308,6 +429,27 @@
         </property>
     </bean>
     
+    <bean id="uk_testPipeline" parent="composite_parent"
+        p:id="uk_testPipeline">
+        <property name="composedStages">
+            <list>
+                <ref bean="uk_assembleHierarchicalAggregate"/>
+                <ref bean="fixup_EncryptionMethod"/>
+                <ref bean="performOtherFixups"/>
+                <ref bean="uk_addTrustRoots"/>
+                <ref bean="uk_finaliseTest"/>
+                <ref bean="uk_normaliseTest"/>
+                
+                <!-- test aggregate MUST pass publishability test -->
+                <ref bean="checkPublishable"/>
+                <ref bean="check_fixup_encmethod"/>
+                <ref bean="errorTerminatingFilter"/>
+                
+                <ref bean="serializeUnsignedTestAggregate"/>
+            </list>
+        </property>
+    </bean>
+
     <!--
         *******************************************
         ***                                     ***
@@ -445,13 +587,6 @@
                 <ref bean="uk_stripExtensions"/>
                 <ref bean="removeEmptyExtensions"/>
                 
-                <!--
-                    Perform fixups on UK federation entities.  We need to make sure
-                    this is also performed on all inbound metadata too, but that is
-                    done elsewhere.
-                -->
-                <ref bean="uk_performFixups"/>
-                
                 <!--
                     ***************************************************
                     ***                                             ***
@@ -544,16 +679,7 @@
                 -->
                 
                 <!-- pipeline continues to generate test aggregate -->
-                <ref bean="uk_assembleHierarchicalAggregate"/>
-                <ref bean="uk_addTrustRoots"/>
-                <ref bean="uk_finaliseTest"/>
-                <ref bean="uk_normaliseTest"/>
-                
-                <!-- test aggregate MUST pass publishability test -->
-                <ref bean="uk_checkPublishable"/>
-                <ref bean="errorTerminatingFilter"/>
-                
-                <ref bean="serializeUnsignedTestAggregate"/>
+                <ref bean="uk_testPipeline"/>
 
             </list>
         </property>
diff --git a/mdx/validation-beans.xml b/mdx/validation-beans.xml
index 5644fecd..7b56fa4b 100644
--- a/mdx/validation-beans.xml
+++ b/mdx/validation-beans.xml
@@ -254,18 +254,6 @@
         </property>
     </bean>
     
-    <!--
-        check_fixups
-    -->
-    <bean id="check_fixups" parent="check_xslt_parent"
-        p:id="check_fixups">
-        <property name="xslResource">
-            <bean parent="file_parent">
-                <constructor-arg value="#{ systemProperties['rulesdir'] }/check_fixups.xsl"/>
-            </bean>
-        </property>
-    </bean>
-    
     <!--
         check_hoksso
     -->