SEC-632: Make order attribute in user-filter optional for cases when the filter implements Ordered directly.

core/src/main/java/org/springframework/security/config/OrderedFilterBeanDefinitionDecorator.java

@@ -6,6 +6,7 @@ import org.springframework.beans.factory.config.BeanDefinitionHolder;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.core.Ordered;
+import org.springframework.util.StringUtils;
 import org.springframework.util.Assert;
 
 import org.w3c.dom.Node;
@@ -20,55 +21,66 @@ import javax.servlet.FilterChain;
 import java.io.IOException;
 
 /**
- * Replaces a Spring bean of type "Filter" with a wrapper class which implement the <tt>Ordered</tt>
- * interface. This allows user to add their own filter to the security chain.
+ * Replaces a Spring bean of type "Filter" with a wrapper class which implements the <tt>Ordered</tt>
+ * interface. This allows user to add their own filter to the security chain. If the user's filter
+ * already implements Ordered
  *
  * @author Luke Taylor
  * @version $Id$
  */
 public class OrderedFilterBeanDefinitionDecorator implements BeanDefinitionDecorator {
+
     public static final String ATT_ORDER = "order";
 
     public BeanDefinitionHolder decorate(Node node, BeanDefinitionHolder holder, ParserContext parserContext) {
         Element elt = (Element)node;
         String order = elt.getAttribute(ATT_ORDER);
         BeanDefinition filter = holder.getBeanDefinition();
-
-        Assert.hasText(order, "order attribute is required");
-
         BeanDefinition wrapper = new RootBeanDefinition(OrderedFilterDecorator.class);
-        wrapper.getConstructorArgumentValues().addGenericArgumentValue(filter);
-        wrapper.getPropertyValues().addPropertyValue("order", order);
+        wrapper.getConstructorArgumentValues().addIndexedArgumentValue(0, holder.getBeanName());
+        wrapper.getConstructorArgumentValues().addIndexedArgumentValue(1, filter);
+
+        if (StringUtils.hasText(order)) {
+            wrapper.getPropertyValues().addPropertyValue("order", order);
+        }
 
         return new BeanDefinitionHolder(wrapper, holder.getBeanName());
     }
-}
 
-class OrderedFilterDecorator implements Filter, Ordered {
-    private int order = LOWEST_PRECEDENCE;
-    private Filter delegate;
+    static class OrderedFilterDecorator implements Filter, Ordered {
+        private Integer order = null;
+        private Filter delegate;
+        private String beanName;
 
-    OrderedFilterDecorator(Filter delegate) {
-        this.delegate = delegate;
-    }
+        OrderedFilterDecorator(String beanName, Filter delegate) {
+            this.delegate = delegate;
+            this.beanName = beanName;
+        }
 
-    public final void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
-        delegate.doFilter(request, response, chain);
-    }
+        public final void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
+            delegate.doFilter(request, response, chain);
+        }
 
-    public final void init(FilterConfig filterConfig) throws ServletException {
-        delegate.init(filterConfig);
-    }
+        public final void init(FilterConfig filterConfig) throws ServletException {
+            delegate.init(filterConfig);
+        }
 
-    public final void destroy() {
-        delegate.destroy();
-    }
+        public final void destroy() {
+            delegate.destroy();
+        }
 
-    public final int getOrder() {
-        return order;
-    }
+        public final int getOrder() {
+            if(order == null) {
+                Assert.isInstanceOf(Ordered.class, "Filter '"+ beanName +"' must implement the 'Ordered' interface " +
+                        " or you must specify an order in <user-filter>");
+
+                return ((Ordered)delegate).getOrder();
+            }
+            return order.intValue();
+        }
 
-    public final void setOrder(int order) {
-        this.order = order;
+        public final void setOrder(int order) {
+            this.order = new Integer(order);
+        }
     }
 }

core/src/main/resources/org/springframework/security/config/spring-security-2.0.rnc

@@ -265,5 +265,5 @@ user-filter =
     element user-filter {user-filter.attlist}
 user-filter.attlist &=
     ## The order value for the chain (user to position the filter relative to the standard filters)   
-    attribute order {xsd:integer}
+    attribute order {xsd:integer}?
     

core/src/main/resources/org/springframework/security/config/spring-security-2.0.xsd

@@ -638,7 +638,7 @@
     </xs:complexType>
   </xs:element>
   <xs:attributeGroup name="user-filter.attlist">
-    <xs:attribute name="order" use="required" type="xs:integer">
+    <xs:attribute name="order" type="xs:integer">
       <xs:annotation>
         <xs:documentation>The order value for the chain (user to position the filter relative to the standard filters)   </xs:documentation>
       </xs:annotation>

core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java

@@ -12,7 +12,6 @@ import org.springframework.security.ui.webapp.AuthenticationProcessingFilter;
 import org.springframework.security.ui.webapp.DefaultLoginPageGeneratingFilter;
 import org.springframework.security.util.FilterChainProxy;
 import org.springframework.security.util.PortMapperImpl;
-import org.springframework.security.util.MockFilter;
 import org.springframework.security.wrapper.SecurityContextHolderAwareRequestFilter;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.beans.BeansException;
@@ -81,7 +80,7 @@ public class HttpSecurityBeanDefinitionParserTests {
         assertTrue(filters.next() instanceof RememberMeProcessingFilter);
         assertTrue(filters.next() instanceof ExceptionTranslationFilter);
         assertTrue(filters.next() instanceof FilterSecurityInterceptor);
-        assertTrue(filters.next() instanceof OrderedFilterDecorator);
+        assertTrue(filters.next() instanceof OrderedFilterBeanDefinitionDecorator.OrderedFilterDecorator);
 
     }