diff --git a/core/src/main/java/org/springframework/security/config/ConcurrentSessionsBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/ConcurrentSessionsBeanDefinitionParser.java index 28a11ec12c..ce6183813c 100644 --- a/core/src/main/java/org/springframework/security/config/ConcurrentSessionsBeanDefinitionParser.java +++ b/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); } diff --git a/core/src/main/java/org/springframework/security/config/ConfigUtils.java b/core/src/main/java/org/springframework/security/config/ConfigUtils.java index 4ed59e7880..6566c95f4b 100644 --- a/core/src/main/java/org/springframework/security/config/ConfigUtils.java +++ b/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); + } } diff --git a/core/src/main/java/org/springframework/security/config/FormLoginBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/FormLoginBeanDefinitionParser.java index 90363bd5bb..e04d15b055 100644 --- a/core/src/main/java/org/springframework/security/config/FormLoginBeanDefinitionParser.java +++ b/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) ); } diff --git a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java index bc707a49c8..89d738ed23 100644 --- a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java +++ b/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); diff --git a/core/src/main/java/org/springframework/security/config/LogoutBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/LogoutBeanDefinitionParser.java index 19b1ab990f..72eb614d77 100644 --- a/core/src/main/java/org/springframework/security/config/LogoutBeanDefinitionParser.java +++ b/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; } diff --git a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java index e05608fb76..5b8180d602 100644 --- a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java +++ b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java @@ -70,6 +70,11 @@ public class HttpSecurityBeanDefinitionParserTests { } } + @Test + public void minimalConfigurationParses() { + setContext("" + AUTH_PROVIDER_XML); + } + @Test public void httpAutoConfigSetsUpCorrectFilterList() throws Exception { setContext("" + AUTH_PROVIDER_XML); @@ -83,7 +88,7 @@ public class HttpSecurityBeanDefinitionParserTests { public void duplicateElementCausesError() throws Exception { setContext("" + 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( + "" + + " " + + "" + AUTH_PROVIDER_XML); + } + + @Test(expected=BeanDefinitionParsingException.class) + public void invalidDefaultTargetUrlIsDetected() throws Exception { + setContext( + "" + + " " + + "" + AUTH_PROVIDER_XML); + } + + @Test(expected=BeanDefinitionParsingException.class) + public void invalidLogoutUrlIsDetected() throws Exception { + setContext( + "" + + " " + + " " + + "" + AUTH_PROVIDER_XML); + } + + @Test(expected=BeanDefinitionParsingException.class) + public void invalidLogoutSuccessUrlIsDetected() throws Exception { + setContext( + "" + + " " + + " " + + "" + 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("" + AUTH_PROVIDER_XML); - } - @Test public void oncePerRequestAttributeIsSupported() throws Exception { setContext("" + 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("" + 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(