diff --git a/config/src/main/java/org/springframework/security/config/http/AuthorizationFilterParser.java b/config/src/main/java/org/springframework/security/config/http/AuthorizationFilterParser.java index 548649675f..de676a0791 100644 --- a/config/src/main/java/org/springframework/security/config/http/AuthorizationFilterParser.java +++ b/config/src/main/java/org/springframework/security/config/http/AuthorizationFilterParser.java @@ -124,6 +124,10 @@ class AuthorizationFilterParser implements BeanDefinitionParser { List interceptMessages = DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL); for (Element interceptMessage : interceptMessages) { String accessExpression = interceptMessage.getAttribute(ATT_ACCESS); + if (!StringUtils.hasText(accessExpression)) { + parserContext.getReaderContext().error("access attribute cannot be empty or null", interceptMessage); + continue; + } BeanDefinitionBuilder authorizationManager = BeanDefinitionBuilder .rootBeanDefinition(WebExpressionAuthorizationManager.class); authorizationManager.addPropertyReference("expressionHandler", expressionHandlerRef); diff --git a/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java b/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java index 8e174f18bd..4716b7f535 100644 --- a/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java +++ b/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java @@ -142,10 +142,11 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit ManagedMap filterInvocationDefinitionMap = new ManagedMap<>(); for (Element urlElt : urlElts) { String access = urlElt.getAttribute(ATT_ACCESS); + String path = urlElt.getAttribute(ATT_PATTERN); if (!StringUtils.hasText(access)) { + parserContext.getReaderContext().error("access attribute cannot be empty or null", urlElt); continue; } - String path = urlElt.getAttribute(ATT_PATTERN); String matcherRef = urlElt.getAttribute(HttpSecurityBeanDefinitionParser.ATT_REQUEST_MATCHER_REF); boolean hasMatcherRef = StringUtils.hasText(matcherRef); if (!hasMatcherRef && !StringUtils.hasText(path)) { diff --git a/config/src/test/java/org/springframework/security/config/http/InterceptUrlConfigTests.java b/config/src/test/java/org/springframework/security/config/http/InterceptUrlConfigTests.java index fab1e932c0..38ac6cf434 100644 --- a/config/src/test/java/org/springframework/security/config/http/InterceptUrlConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/http/InterceptUrlConfigTests.java @@ -337,6 +337,54 @@ public class InterceptUrlConfigTests { assertThat(this.spring.getContext().getBean(AuthorizationManager.class)).isNotNull(); } + /** + * gh-18503 + */ + @Test + public void configWhenInterceptUrlMissingAccessThenException() { + assertThatExceptionOfType(BeanDefinitionParsingException.class) + .isThrownBy(() -> this.spring.configLocations(this.xml("MissingAccess")).autowire()) + .withMessageContaining("access attribute cannot be empty or null"); + } + + /** + * gh-18503 + */ + @Test + public void configWhenInterceptUrlEmptyAccessThenException() { + assertThatExceptionOfType(BeanDefinitionParsingException.class) + .isThrownBy(() -> this.spring.configLocations(this.xml("EmptyAccess")).autowire()) + .withMessageContaining("access attribute cannot be empty or null"); + } + + /** + * gh-18503 + */ + @Test + public void configWhenInterceptUrlValidAccessThenLoads() { + assertThatNoException().isThrownBy(() -> this.spring.configLocations(this.xml("ValidAccess")).autowire()); + } + + /** + * gh-18503 + */ + @Test + public void configWhenUseAuthorizationManagerFalseAndMissingAccessThenException() { + assertThatExceptionOfType(BeanDefinitionParsingException.class) + .isThrownBy(() -> this.spring.configLocations(this.xml("MissingAccessLegacy")).autowire()) + .withMessageContaining("access attribute cannot be empty or null"); + } + + /** + * gh-18503 + */ + @Test + public void configWhenUseAuthorizationManagerFalseAndEmptyAccessThenException() { + assertThatExceptionOfType(BeanDefinitionParsingException.class) + .isThrownBy(() -> this.spring.configLocations(this.xml("EmptyAccessLegacy")).autowire()) + .withMessageContaining("access attribute cannot be empty or null"); + } + private static RequestPostProcessor adminCredentials() { return httpBasic("admin", "password"); } diff --git a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-EmptyAccess.xml b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-EmptyAccess.xml new file mode 100644 index 0000000000..26197d5bd4 --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-EmptyAccess.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + diff --git a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-EmptyAccessLegacy.xml b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-EmptyAccessLegacy.xml new file mode 100644 index 0000000000..897a1852e4 --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-EmptyAccessLegacy.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + diff --git a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MissingAccess.xml b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MissingAccess.xml new file mode 100644 index 0000000000..d09f24205b --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MissingAccess.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + diff --git a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MissingAccessLegacy.xml b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MissingAccessLegacy.xml new file mode 100644 index 0000000000..ad9f6ef9c9 --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MissingAccessLegacy.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + diff --git a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-ValidAccess.xml b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-ValidAccess.xml new file mode 100644 index 0000000000..00a33b7f0f --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-ValidAccess.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + +