OPEN - issue SEC-757: Add validation of redirect URLs on namespace

http://jira.springframework.org/browse/SEC-757. Added validation method to ConfigUtils and calls to it for url attributes.

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

@@ -55,6 +55,7 @@ public class ConcurrentSessionsBeanDefinitionParser implements BeanDefinitionPar
         String expiryUrl = element.getAttribute(ATT_EXPIRY_URL);
 
         if (StringUtils.hasText(expiryUrl)) {
+        	ConfigUtils.validateHttpRedirect(expiryUrl, parserContext, source);
             filterBuilder.addPropertyValue("expiredUrl", expiryUrl);
         }
 

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

@@ -177,4 +177,15 @@ public abstract class ConfigUtils {
 			this.filters = filters;
 		}
     }
+    
+    /**
+     * Checks the value of an XML attribute which represents a redirect URL.
+     * If not empty or starting with "/" or "http" it will raise an error. 
+     */
+    static void validateHttpRedirect(String url, ParserContext pc, Object source) {
+    	if (!StringUtils.hasText(url) || url.startsWith("/") || url.toLowerCase().startsWith("http")) {
+    		return;
+    	}
+    	pc.getReaderContext().error(url + " is not a valid redirect URL (must start with '/' or http(s))", source);
+    }
 }

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

@@ -46,7 +46,7 @@ public class FormLoginBeanDefinitionParser implements BeanDefinitionParser {
     	this.filterClassName = filterClassName;
     }
 
-    public BeanDefinition parse(Element elt, ParserContext parserContext) {
+    public BeanDefinition parse(Element elt, ParserContext pc) {
         String loginUrl = null;
         String defaultTargetUrl = null;
         String authenticationFailureUrl = null;
@@ -55,26 +55,31 @@ public class FormLoginBeanDefinitionParser implements BeanDefinitionParser {
         Object source = null;
 
         if (elt != null) {
+        	source = pc.extractSource(elt);
             loginUrl = elt.getAttribute(ATT_LOGIN_URL);
+            ConfigUtils.validateHttpRedirect(loginUrl, pc, source);
             defaultTargetUrl = elt.getAttribute(ATT_FORM_LOGIN_TARGET_URL);
+            ConfigUtils.validateHttpRedirect(defaultTargetUrl, pc, source);
             authenticationFailureUrl = elt.getAttribute(ATT_FORM_LOGIN_AUTHENTICATION_FAILURE_URL);
+            ConfigUtils.validateHttpRedirect(authenticationFailureUrl, pc, source);
             alwaysUseDefault = elt.getAttribute(ATT_ALWAYS_USE_DEFAULT_TARGET_URL);
             loginPage = elt.getAttribute(ATT_LOGIN_PAGE);
             
             if (!StringUtils.hasText(loginPage)) {
             	loginPage = null;
             }
-            source = parserContext.extractSource(elt);
+            ConfigUtils.validateHttpRedirect(loginPage, pc, source);
+            
         }
 
-        ConfigUtils.registerProviderManagerIfNecessary(parserContext);
+        ConfigUtils.registerProviderManagerIfNecessary(pc);
         
         filterBean = createFilterBean(loginUrl, defaultTargetUrl, alwaysUseDefault, loginPage, authenticationFailureUrl);
         filterBean.setSource(source);
         filterBean.getPropertyValues().addPropertyValue("authenticationManager",
                 new RuntimeBeanReference(BeanIds.AUTHENTICATION_MANAGER));
         
-        if (parserContext.getRegistry().containsBeanDefinition(BeanIds.REMEMBER_ME_SERVICES)) {
+        if (pc.getRegistry().containsBeanDefinition(BeanIds.REMEMBER_ME_SERVICES)) {
             filterBean.getPropertyValues().addPropertyValue("rememberMeServices", 
                     new RuntimeBeanReference(BeanIds.REMEMBER_ME_SERVICES) );
         }

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

@@ -133,7 +133,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
                 DomUtils.getChildElementByTagName(element, Elements.PORT_MAPPINGS), parserContext);
         registry.registerBeanDefinition(BeanIds.PORT_MAPPER, portMapper);
 
-        registerExceptionTranslationFilter(element.getAttribute(ATT_ACCESS_DENIED_PAGE), parserContext);
+        registerExceptionTranslationFilter(element, parserContext);
 
 
         if (channelRequestMap.size() > 0) {
@@ -249,7 +249,9 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
         return true;
     }
     
-    private void registerExceptionTranslationFilter(String accessDeniedPage, ParserContext pc) {
+    private void registerExceptionTranslationFilter(Element element, ParserContext pc) {
+    	String accessDeniedPage = element.getAttribute(ATT_ACCESS_DENIED_PAGE);
+    	ConfigUtils.validateHttpRedirect(accessDeniedPage, pc, pc.extractSource(element));
         BeanDefinitionBuilder exceptionTranslationFilterBuilder
             = BeanDefinitionBuilder.rootBeanDefinition(ExceptionTranslationFilter.class);
  
@@ -327,8 +329,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
             pc.getRegistry().registerBeanDefinition(BeanIds.SESSION_FIXATION_PROTECTION_FILTER, 
                     sessionFixationFilter.getBeanDefinition());
             ConfigUtils.addHttpFilter(pc, new RuntimeBeanReference(BeanIds.SESSION_FIXATION_PROTECTION_FILTER));
-        }        
+        }
-        
     }
     
     private void parseBasicFormLoginAndOpenID(Element element, ParserContext pc, boolean autoConfig) {
@@ -342,12 +343,12 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
         String realm = element.getAttribute(ATT_REALM);
         if (!StringUtils.hasText(realm)) {
         	realm = DEF_REALM;
-        }        
+        }
         
         Element basicAuthElt = DomUtils.getChildElementByTagName(element, Elements.BASIC_AUTH);
         if (basicAuthElt != null || autoConfig) {
             new BasicAuthenticationBeanDefinitionParser(realm).parse(basicAuthElt, pc);
-        }        
+        }
         
     	Element formLoginElt = DomUtils.getChildElementByTagName(element, Elements.FORM_LOGIN);
         

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

@@ -31,15 +31,18 @@ public class LogoutBeanDefinitionParser implements BeanDefinitionParser {
         String logoutSuccessUrl = null;
         String invalidateSession = null;
 
+        BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(LogoutFilter.class);
+
         if (element != null) {
+        	Object source = parserContext.extractSource(element);
+        	builder.setSource(source);
             logoutUrl = element.getAttribute(ATT_LOGOUT_URL);
+            ConfigUtils.validateHttpRedirect(logoutUrl, parserContext, source);
             logoutSuccessUrl = element.getAttribute(ATT_LOGOUT_SUCCESS_URL);
+            ConfigUtils.validateHttpRedirect(logoutSuccessUrl, parserContext, source);
             invalidateSession = element.getAttribute(ATT_INVALIDATE_SESSION);
         }
 
-        BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(LogoutFilter.class);
-        builder.setSource(parserContext.extractSource(element));
-
         if (!StringUtils.hasText(logoutUrl)) {
         	logoutUrl = DEF_LOGOUT_URL;
         }

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

@@ -70,6 +70,11 @@ public class HttpSecurityBeanDefinitionParserTests {
         }
     }
 
+    @Test
+    public void minimalConfigurationParses() {
+        setContext("<http><http-basic /></http>" + AUTH_PROVIDER_XML);
+    }
+    
     @Test
     public void httpAutoConfigSetsUpCorrectFilterList() throws Exception {
         setContext("<http auto-config='true' />" + AUTH_PROVIDER_XML);
@@ -83,7 +88,7 @@ public class HttpSecurityBeanDefinitionParserTests {
     public void duplicateElementCausesError() throws Exception {
         setContext("<http auto-config='true' /><http auto-config='true' />" + AUTH_PROVIDER_XML);
     }    
-    
+        
     private void checkAutoConfigFilters(List filterList) throws Exception {
         assertEquals("Expected 11 filters in chain", 11, filterList.size());
 
@@ -168,6 +173,40 @@ public class HttpSecurityBeanDefinitionParserTests {
         assertEquals("/default", filter.getDefaultTargetUrl());
         assertEquals(Boolean.TRUE, FieldUtils.getFieldValue(filter, "alwaysUseDefaultTargetUrl"));
     }
+
+    @Test(expected=BeanDefinitionParsingException.class)
+    public void invalidLoginPageIsDetected() throws Exception {
+        setContext(
+                "<http>" +
+                "   <form-login login-page='noLeadingSlash'/>" +
+                "</http>" + AUTH_PROVIDER_XML);
+    }    
+    
+    @Test(expected=BeanDefinitionParsingException.class)
+    public void invalidDefaultTargetUrlIsDetected() throws Exception {
+        setContext(
+                "<http>" +
+                "   <form-login default-target-url='noLeadingSlash'/>" +
+                "</http>" + AUTH_PROVIDER_XML);
+    }    
+
+    @Test(expected=BeanDefinitionParsingException.class)
+    public void invalidLogoutUrlIsDetected() throws Exception {
+        setContext(
+                "<http>" +
+                "   <logout logout-url='noLeadingSlash'/>" +                
+                "   <form-login />" +
+                "</http>" + AUTH_PROVIDER_XML);
+    }    
+    
+    @Test(expected=BeanDefinitionParsingException.class)
+    public void invalidLogoutSuccessUrlIsDetected() throws Exception {
+        setContext(
+                "<http>" +
+                "   <logout logout-success-url='noLeadingSlash'/>" +                
+                "   <form-login />" +
+                "</http>" + AUTH_PROVIDER_XML);
+    }    
     
     @Test
     public void lowerCaseComparisonIsRespectedBySecurityFilterInvocationDefinitionSource() throws Exception {
@@ -206,11 +245,6 @@ public class HttpSecurityBeanDefinitionParserTests {
         assertTrue(attrs.contains(new SecurityConfig("ROLE_B")));
     }
 
-    @Test
-    public void minimalConfigurationParses() {
-        setContext("<http><http-basic /></http>" + AUTH_PROVIDER_XML);
-    }
-
     @Test
     public void oncePerRequestAttributeIsSupported() throws Exception {
         setContext("<http once-per-request='false'><http-basic /></http>" + AUTH_PROVIDER_XML);
@@ -229,6 +263,11 @@ public class HttpSecurityBeanDefinitionParserTests {
         ExceptionTranslationFilter etf = (ExceptionTranslationFilter) filters.get(filters.size() - 2);
         
         assertEquals("/access-denied", FieldUtils.getFieldValue(etf, "accessDeniedHandler.errorPage"));
+    }
+    
+    @Test(expected=BeanDefinitionParsingException.class)
+    public void invalidAccessDeniedUrlIsDetected() throws Exception {
+        setContext("<http auto-config='true' access-denied-page='noLeadingSlash'/>" + AUTH_PROVIDER_XML);
     }    
     
     @Test
@@ -371,7 +410,7 @@ public class HttpSecurityBeanDefinitionParserTests {
         auth.setDetails(new WebAuthenticationDetails(req));
         seshController.checkAuthenticationAllowed(auth);
     }
-    
+
     @Test
     public void customEntryPointIsSupported() throws Exception {
         setContext(