Skip to content

Commit

Permalink
Use MvcRequestMatcher by default if Spring MVC is present
Browse files Browse the repository at this point in the history
Closes gh-11899
  • Loading branch information
marcusdacoregio committed Oct 6, 2022
1 parent 353ca76 commit c4d23f2
Show file tree
Hide file tree
Showing 82 changed files with 391 additions and 177 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -108,7 +108,7 @@ private String createAuthorizationManager(Element element, ParserContext parserC
if (expressionHandlerRef == null) {
expressionHandlerRef = registerDefaultExpressionHandler(parserContext);
}
MatcherType matcherType = MatcherType.fromElement(element);
MatcherType matcherType = MatcherType.fromElementOrMvc(element);
ManagedMap<BeanMetadataElement, BeanDefinition> matcherToExpression = new ManagedMap<>();
List<Element> interceptMessages = DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL);
for (Element interceptMessage : interceptMessages) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -39,7 +39,7 @@ public class FilterChainBeanDefinitionParser implements BeanDefinitionParser {

@Override
public BeanDefinition parse(Element elt, ParserContext pc) {
MatcherType matcherType = MatcherType.fromElement(elt);
MatcherType matcherType = MatcherType.fromElementOrMvc(elt);
String path = elt.getAttribute(HttpSecurityBeanDefinitionParser.ATT_PATH_PATTERN);
String requestMatcher = elt.getAttribute(ATT_REQUEST_MATCHER_REF);
String filters = elt.getAttribute(HttpSecurityBeanDefinitionParser.ATT_FILTERS);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -47,7 +47,7 @@ public BeanDefinitionHolder decorate(Node node, BeanDefinitionHolder holder, Par
BeanDefinition filterChainProxy = holder.getBeanDefinition();
ManagedList<BeanMetadataElement> securityFilterChains = new ManagedList<>();
Element elt = (Element) node;
MatcherType matcherType = MatcherType.fromElement(elt);
MatcherType matcherType = MatcherType.fromElementOrMvc(elt);
List<Element> filterChainElts = DomUtils.getChildElementsByTagName(elt, Elements.FILTER_CHAIN);
for (Element chain : filterChainElts) {
String path = chain.getAttribute(HttpSecurityBeanDefinitionParser.ATT_PATH_PATTERN);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public BeanDefinition parse(Element element, ParserContext parserContext) {

static RootBeanDefinition createSecurityMetadataSource(List<Element> interceptUrls, boolean addAllAuth,
Element httpElt, ParserContext pc) {
MatcherType matcherType = MatcherType.fromElement(httpElt);
MatcherType matcherType = MatcherType.fromElementOrMvc(httpElt);
boolean useExpressions = isUseExpressions(httpElt);
ManagedMap<BeanMetadataElement, BeanDefinition> requestToAttributesMap = parseInterceptUrlsForFilterInvocationRequestMap(
matcherType, interceptUrls, useExpressions, addAllAuth, pc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class HttpConfigurationBuilder {
this.pc = pc;
this.portMapper = portMapper;
this.portResolver = portResolver;
this.matcherType = MatcherType.fromElement(element);
this.matcherType = MatcherType.fromElementOrMvc(element);
this.interceptUrls = DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL);
validateInterceptUrls(pc);
String createSession = element.getAttribute(ATT_CREATE_SESSION);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -196,7 +196,7 @@ private BeanReference createSecurityFilterChainBean(Element element, ParserConte

}
else if (StringUtils.hasText(filterChainPattern)) {
filterChainMatcher = MatcherType.fromElement(element).createMatcher(pc, filterChainPattern, null);
filterChainMatcher = MatcherType.fromElementOrMvc(element).createMatcher(pc, filterChainPattern, null);
}
else {
filterChainMatcher = new RootBeanDefinition(AnyRequestMatcher.class);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -22,11 +22,13 @@
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
import org.springframework.beans.factory.support.RootBeanDefinition;
import org.springframework.beans.factory.xml.ParserContext;
import org.springframework.http.HttpMethod;
import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
import org.springframework.security.web.util.matcher.RegexRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;

/**
Expand All @@ -40,12 +42,18 @@ public enum MatcherType {
ant(AntPathRequestMatcher.class), regex(RegexRequestMatcher.class), ciRegex(RegexRequestMatcher.class), mvc(
MvcRequestMatcher.class);

private static final String HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME = "mvcHandlerMappingIntrospector";
private static final String HANDLER_MAPPING_INTROSPECTOR = "org.springframework.web.servlet.handler.HandlerMappingIntrospector";

private static final boolean mvcPresent;

private static final String ATT_MATCHER_TYPE = "request-matcher";

final Class<? extends RequestMatcher> type;

static {
mvcPresent = ClassUtils.isPresent(HANDLER_MAPPING_INTROSPECTOR, MatcherType.class.getClassLoader());
}

MatcherType(Class<? extends RequestMatcher> type) {
this.type = type;
}
Expand All @@ -64,7 +72,7 @@ public BeanDefinition createMatcher(ParserContext pc, String path, String method
}
matcherBldr.addConstructorArgValue(path);
if (this == mvc) {
matcherBldr.addPropertyValue("method", method);
matcherBldr.addPropertyValue("method", (StringUtils.hasText(method) ? HttpMethod.valueOf(method) : null));
matcherBldr.addPropertyValue("servletPath", servletPath);
}
else {
Expand All @@ -84,4 +92,12 @@ static MatcherType fromElement(Element elt) {
return ant;
}

static MatcherType fromElementOrMvc(Element elt) {
String matcherTypeName = elt.getAttribute(ATT_MATCHER_TYPE);
if (!StringUtils.hasText(matcherTypeName) && mvcPresent) {
return MatcherType.mvc;
}
return MatcherType.fromElement(elt);
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -36,8 +36,9 @@
import org.springframework.security.web.context.SecurityContextPersistenceFilter;
import org.springframework.security.web.firewall.DefaultHttpFirewall;
import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.AnyRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.test.util.ReflectionTestUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -113,12 +114,13 @@ public void mixingPatternsAndPlaceholdersDoesntCauseOrderingIssues() {
List<SecurityFilterChain> chains = fcp.getFilterChains();
assertThat(getPattern(chains.get(0))).isEqualTo("/login*");
assertThat(getPattern(chains.get(1))).isEqualTo("/logout");
assertThat(((DefaultSecurityFilterChain) chains.get(2)).getRequestMatcher() instanceof AnyRequestMatcher)
.isTrue();
assertThat(((DefaultSecurityFilterChain) chains.get(2)).getRequestMatcher())
.isInstanceOf(AnyRequestMatcher.class);
}

private String getPattern(SecurityFilterChain chain) {
return ((AntPathRequestMatcher) ((DefaultSecurityFilterChain) chain).getRequestMatcher()).getPattern();
RequestMatcher requestMatcher = ((DefaultSecurityFilterChain) chain).getRequestMatcher();
return (String) ReflectionTestUtils.getField(requestMatcher, "pattern");
}

private void checkPathAndFilterOrder(FilterChainProxy filterChainProxy) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -92,10 +92,12 @@ public void expressionsAreSupported() {
public void interceptUrlsSupportPropertyPlaceholders() {
System.setProperty("secure.url", "/secure");
System.setProperty("secure.role", "ROLE_A");
setContext("<b:bean class='org.springframework.beans.factory.config.PropertyPlaceholderConfigurer'/>"
+ "<filter-security-metadata-source id='fids' use-expressions='false'>"
+ " <intercept-url pattern='${secure.url}' access='${secure.role}'/>"
+ "</filter-security-metadata-source>");
setContext(
"<b:bean class=\"org.springframework.web.servlet.handler.HandlerMappingIntrospector\" name=\"mvcHandlerMappingIntrospector\"/>"
+ "<b:bean class='org.springframework.beans.factory.config.PropertyPlaceholderConfigurer'/>"
+ "<filter-security-metadata-source id='fids' use-expressions='false'>"
+ " <intercept-url pattern='${secure.url}' access='${secure.role}'/>"
+ "</filter-security-metadata-source>");
DefaultFilterInvocationSecurityMetadataSource fids = (DefaultFilterInvocationSecurityMetadataSource) this.appContext
.getBean("fids");
Collection<ConfigAttribute> cad = fids.getAttributes(createFilterInvocation("/secure", "GET"));
Expand All @@ -105,7 +107,8 @@ public void interceptUrlsSupportPropertyPlaceholders() {
@Test
public void parsingWithinFilterSecurityInterceptorIsSuccessful() {
// @formatter:off
setContext("<http auto-config='true' use-expressions='false'/>"
setContext("<b:bean class=\"org.springframework.web.servlet.handler.HandlerMappingIntrospector\" name=\"mvcHandlerMappingIntrospector\"/>" +
"<http auto-config='true' use-expressions='false'/>"
+ "<b:bean id='fsi' class='org.springframework.security.web.access.intercept.FilterSecurityInterceptor' autowire='byType'>"
+ " <b:property name='securityMetadataSource'>"
+ " <filter-security-metadata-source use-expressions='false'>"
Expand All @@ -130,9 +133,8 @@ public void parsingInterceptUrlServletPathFails() {

private FilterInvocation createFilterInvocation(String path, String method) {
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
request.setRequestURI(null);
request.setRequestURI(path);
request.setMethod(method);
request.setServletPath(path);
return new FilterInvocation(request, new MockHttpServletResponse(), new MockFilterChain());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -24,6 +24,7 @@
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.stubbing.Answer;

import org.springframework.beans.factory.BeanCreationException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
import org.springframework.mock.web.MockServletContext;
Expand All @@ -39,6 +40,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
Expand Down Expand Up @@ -361,14 +363,20 @@ public void configureWhenUsingCiRegexMatcherAndServletPathAndAuthorizationManage
}

@Test
public void configureWhenUsingDefaultMatcherAndServletPathThenThrowsException() {
assertThatExceptionOfType(BeanDefinitionParsingException.class)
public void configureWhenUsingDefaultMatcherAndServletPathThenNoException() {
assertThatNoException()
.isThrownBy(() -> this.spring.configLocations(this.xml("DefaultMatcherServletPath")).autowire());
}

@Test
public void configureWhenUsingDefaultMatcherAndServletPathAndAuthorizationManagerThenThrowsException() {
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() -> this.spring
public void configureWhenUsingDefaultMatcherAndNoIntrospectorBeanThenException() {
assertThatExceptionOfType(BeanCreationException.class)
.isThrownBy(() -> this.spring.configLocations(this.xml("DefaultMatcherNoIntrospectorBean")).autowire());
}

@Test
public void configureWhenUsingDefaultMatcherAndServletPathAndAuthorizationManagerThenNoException() {
assertThatNoException().isThrownBy(() -> this.spring
.configLocations(this.xml("DefaultMatcherServletPathAuthorizationManager")).autowire());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
<!--
~ Copyright 2002-2022 the original author or authors.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ https://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

<beans xmlns="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:context="http://www.springframework.org/schema/context"
xmlns:tx="http://www.springframework.org/schema/tx" xmlns:util="http://www.springframework.org/schema/util"
Expand Down Expand Up @@ -27,5 +43,7 @@ http://www.springframework.org/schema/security https://www.springframework.org/s
</security:user-service>
</security:authentication-provider>
</security:authentication-manager>

<import resource="handlermappingintrospector.xml"/>

</beans>
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright 2002-2018 the original author or authors.
~ Copyright 2002-2022 the original author or authors.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ https://www.apache.org/licenses/LICENSE-2.0
~ https://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
Expand All @@ -29,6 +29,7 @@
</http>

<b:import resource="CsrfConfigTests-shared-userservice.xml"/>
<b:import resource="handlermappingintrospector.xml"/>

<b:bean id="firewall" class="org.springframework.security.web.firewall.StrictHttpFirewall"
p:unsafeAllowAnyHttpMethod="true"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright 2002-2018 the original author or authors.
~ Copyright 2002-2022 the original author or authors.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ https://www.apache.org/licenses/LICENSE-2.0
~ https://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
Expand All @@ -33,4 +33,5 @@
<b:bean name="path" class="org.springframework.security.config.http.InterceptUrlConfigTests.PathController"/>

<b:import resource="userservice.xml"/>
<b:import resource="handlermappingintrospector.xml"/>
</b:beans>
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright 2002-2018 the original author or authors.
~ Copyright 2002-2022 the original author or authors.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ https://www.apache.org/licenses/LICENSE-2.0
~ https://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
Expand All @@ -33,4 +33,5 @@
<b:bean name="path" class="org.springframework.security.config.http.InterceptUrlConfigTests.PathController"/>

<b:import resource="userservice.xml"/>
<b:import resource="handlermappingintrospector.xml"/>
</b:beans>
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright 2002-2022 the original author or authors.
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ https://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

<b:beans xmlns:b="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="http://www.springframework.org/schema/security"
xsi:schemaLocation="
http://www.springframework.org/schema/security
https://www.springframework.org/schema/security/spring-security.xsd
http://www.springframework.org/schema/beans
https://www.springframework.org/schema/beans/spring-beans.xsd">

<http>
<intercept-url pattern="/path" access="denyAll" servlet-path="/spring"/>
<http-basic/>
</http>

<b:import resource="userservice.xml"/>
</b:beans>
Loading

0 comments on commit c4d23f2

Please sign in to comment.