Default AntPathRequestMatcher to be case sensitive

Issue gh-3831
This commit is contained in:
Rob Winch 2016-04-20 10:41:02 -05:00
parent 6fa1588de9
commit 7fe0a135ec
6 changed files with 81 additions and 76 deletions

View File

@ -136,17 +136,17 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests {
}
def debugFilterHandlesMissingAndEmptyFilterChains() {
when:
xml.debug()
xml.http(pattern: '/unprotected', security: 'none')
createAppContext()
then:
Filter debugFilter = appContext.getBean(BeanIds.SPRING_SECURITY_FILTER_CHAIN);
MockHttpServletRequest request = new MockHttpServletRequest()
request.setServletPath("/unprotected");
debugFilter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
request.setServletPath("/nomatch");
debugFilter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
when:
xml.debug()
xml.http(pattern: '/unprotected', security: 'none')
createAppContext()
then:
Filter debugFilter = appContext.getBean(BeanIds.SPRING_SECURITY_FILTER_CHAIN);
MockHttpServletRequest request = new MockHttpServletRequest()
request.setServletPath("/unprotected");
debugFilter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
request.setServletPath("/nomatch");
debugFilter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
}
def regexPathsWorkCorrectly() {
@ -254,39 +254,39 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests {
attrs.contains(new SecurityConfig("ROLE_B"))
}
def httpMethodMatchIsSupportedForRequiresChannel() {
httpAutoConfig {
'intercept-url'(pattern: '/anyurl')
'intercept-url'(pattern: '/anyurl', 'method':'GET',access: 'ROLE_ADMIN', 'requires-channel': 'https')
}
createAppContext()
def httpMethodMatchIsSupportedForRequiresChannel() {
httpAutoConfig {
'intercept-url'(pattern: '/anyurl')
'intercept-url'(pattern: '/anyurl', 'method':'GET',access: 'ROLE_ADMIN', 'requires-channel': 'https')
}
createAppContext()
def fids = getFilter(ChannelProcessingFilter).getSecurityMetadataSource();
def attrs = fids.getAttributes(createFilterinvocation("/anyurl", "GET"));
def attrsPost = fids.getAttributes(createFilterinvocation("/anyurl", "POST"));
def fids = getFilter(ChannelProcessingFilter).getSecurityMetadataSource();
def attrs = fids.getAttributes(createFilterinvocation("/anyurl", "GET"));
def attrsPost = fids.getAttributes(createFilterinvocation("/anyurl", "POST"));
expect:
attrs.size() == 1
attrs.contains(new SecurityConfig("REQUIRES_SECURE_CHANNEL"))
attrsPost == null
}
expect:
attrs.size() == 1
attrs.contains(new SecurityConfig("REQUIRES_SECURE_CHANNEL"))
attrsPost == null
}
def httpMethodMatchIsSupportedForRequiresChannelAny() {
httpAutoConfig {
'intercept-url'(pattern: '/**')
'intercept-url'(pattern: '/**', 'method':'GET',access: 'ROLE_ADMIN', 'requires-channel': 'https')
}
createAppContext()
def httpMethodMatchIsSupportedForRequiresChannelAny() {
httpAutoConfig {
'intercept-url'(pattern: '/**')
'intercept-url'(pattern: '/**', 'method':'GET',access: 'ROLE_ADMIN', 'requires-channel': 'https')
}
createAppContext()
def fids = getFilter(ChannelProcessingFilter).getSecurityMetadataSource();
def attrs = fids.getAttributes(createFilterinvocation("/anyurl", "GET"));
def attrsPost = fids.getAttributes(createFilterinvocation("/anyurl", "POST"));
def fids = getFilter(ChannelProcessingFilter).getSecurityMetadataSource();
def attrs = fids.getAttributes(createFilterinvocation("/anyurl", "GET"));
def attrsPost = fids.getAttributes(createFilterinvocation("/anyurl", "POST"));
expect:
attrs.size() == 1
attrs.contains(new SecurityConfig("REQUIRES_SECURE_CHANNEL"))
attrsPost == null
}
expect:
attrs.size() == 1
attrs.contains(new SecurityConfig("REQUIRES_SECURE_CHANNEL"))
attrsPost == null
}
def oncePerRequestAttributeIsSupported() {
xml.http('once-per-request': 'false') {
@ -498,7 +498,7 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests {
createAppContext()
def fis = getFilter(FilterSecurityInterceptor)
def fids = fis.securityMetadataSource
Collection attrs = fids.getAttributes(createFilterinvocation("/someurl", null));
Collection attrs = fids.getAttributes(createFilterinvocation("/someUrl", null));
expect:
attrs.size() == 1
@ -716,7 +716,7 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests {
def httpFirewallInjectionIsSupported() {
xml.'http-firewall'(ref: 'fw')
xml.http() {
'form-login'()
'form-login'()
}
bean('fw', DefaultHttpFirewall)
createAppContext()

View File

@ -39,7 +39,7 @@ class PlaceHolderAndELConfigTests extends AbstractHttpConfigTests {
// SEC-1201
def interceptUrlsAndFormLoginSupportPropertyPlaceholders() {
System.setProperty("secure.Url", "/Secure");
System.setProperty("secure.Url", "/secure");
System.setProperty("secure.role", "ROLE_A");
System.setProperty("login.page", "/loginPage");
System.setProperty("default.target", "/defaultTarget");
@ -60,7 +60,7 @@ class PlaceHolderAndELConfigTests extends AbstractHttpConfigTests {
// SEC-1309
def interceptUrlsAndFormLoginSupportEL() {
System.setProperty("secure.url", "/Secure");
System.setProperty("secure.url", "/secure");
System.setProperty("secure.role", "ROLE_A");
System.setProperty("login.page", "/loginPage");
System.setProperty("default.target", "/defaultTarget");

View File

@ -40,6 +40,7 @@ import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextImpl;
import org.springframework.security.web.FilterChainProxy;
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
import static org.assertj.core.api.Assertions.assertThat;
@ -151,7 +152,7 @@ public class AuthorizeRequestsTests {
// @formatter:off
http
.authorizeRequests()
.antMatchers("/user/{user}").access("#user == 'user'")
.requestMatchers(new AntPathRequestMatcher("/user/{user}", null, false)).access("#user == 'user'")
.anyRequest().denyAll();
// @formatter:on
}
@ -192,7 +193,7 @@ public class AuthorizeRequestsTests {
// @formatter:off
http
.authorizeRequests()
.antMatchers("/user/{userName}").access("#userName == 'user'")
.requestMatchers(new AntPathRequestMatcher("/user/{userName}", null, false)).access("#userName == 'user'")
.anyRequest().denyAll();
// @formatter:on
}

View File

@ -80,7 +80,7 @@ public final class AntPathRequestMatcher implements RequestMatcher {
* the incoming request doesn't have the same method.
*/
public AntPathRequestMatcher(String pattern, String httpMethod) {
this(pattern, httpMethod, false);
this(pattern, httpMethod, true);
}
/**

View File

@ -16,15 +16,13 @@
package org.springframework.security.web.access.intercept;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.mock;
import java.util.Collection;
import java.util.LinkedHashMap;
import javax.servlet.FilterChain;
import org.junit.Test;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.access.ConfigAttribute;
@ -33,6 +31,9 @@ import org.springframework.security.web.FilterInvocation;
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
import org.springframework.security.web.util.matcher.RequestMatcher;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
/**
* Tests {@link DefaultFilterInvocationSecurityMetadataSource}.
*
@ -46,18 +47,18 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
// ========================================================================================================
private void createFids(String pattern, String method) {
LinkedHashMap<RequestMatcher, Collection<ConfigAttribute>> requestMap = new LinkedHashMap<RequestMatcher, Collection<ConfigAttribute>>();
requestMap.put(new AntPathRequestMatcher(pattern, method), def);
fids = new DefaultFilterInvocationSecurityMetadataSource(requestMap);
requestMap.put(new AntPathRequestMatcher(pattern, method), this.def);
this.fids = new DefaultFilterInvocationSecurityMetadataSource(requestMap);
}
@Test
public void lookupNotRequiringExactMatchSucceedsIfNotMatching() {
createFids("/secure/super/**", null);
FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null,
FilterInvocation fi = createFilterInvocation("/secure/super/somefile.html", null,
null, null);
assertThat(fids.getAttributes(fi)).isEqualTo(def);
assertThat(this.fids.getAttributes(fi)).isEqualTo(this.def);
}
/**
@ -66,13 +67,13 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
*/
@Test
public void lookupNotRequiringExactMatchSucceedsIfSecureUrlPathContainsUpperCase() {
createFids("/SeCuRE/super/**", null);
createFids("/secure/super/**", null);
FilterInvocation fi = createFilterInvocation("/secure", "/super/somefile.html",
null, null);
Collection<ConfigAttribute> response = fids.getAttributes(fi);
assertThat(response).isEqualTo(def);
Collection<ConfigAttribute> response = this.fids.getAttributes(fi);
assertThat(response).isEqualTo(this.def);
}
@Test
@ -82,8 +83,8 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
FilterInvocation fi = createFilterInvocation("/SeCurE/super/somefile.html", null,
null, null);
Collection<ConfigAttribute> response = fids.getAttributes(fi);
assertThat(response).isEqualTo(def);
Collection<ConfigAttribute> response = this.fids.getAttributes(fi);
assertThat(response).isEqualTo(this.def);
}
@Test
@ -93,8 +94,9 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
FilterInvocation fi = createFilterInvocation("/someAdminPage.html", null,
"a=/test", null);
Collection<ConfigAttribute> response = fids.getAttributes(fi);
assertThat(response); // see SEC-161 (it should truncate after ? sign).isEqualTo(def)
Collection<ConfigAttribute> response = this.fids.getAttributes(fi);
assertThat(response); // see SEC-161 (it should truncate after ?
// sign).isEqualTo(def)
}
@Test(expected = IllegalArgumentException.class)
@ -107,8 +109,8 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
createFids("/somepage**", "GET");
FilterInvocation fi = createFilterInvocation("/somepage", null, null, "GET");
Collection<ConfigAttribute> attrs = fids.getAttributes(fi);
assertThat(attrs).isEqualTo(def);
Collection<ConfigAttribute> attrs = this.fids.getAttributes(fi);
assertThat(attrs).isEqualTo(this.def);
}
@Test
@ -116,8 +118,8 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
createFids("/somepage**", null);
FilterInvocation fi = createFilterInvocation("/somepage", null, null, "GET");
Collection<ConfigAttribute> attrs = fids.getAttributes(fi);
assertThat(attrs).isEqualTo(def);
Collection<ConfigAttribute> attrs = this.fids.getAttributes(fi);
assertThat(attrs).isEqualTo(this.def);
}
@Test
@ -125,7 +127,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
createFids("/somepage**", "GET");
FilterInvocation fi = createFilterInvocation("/somepage", null, null, "POST");
Collection<ConfigAttribute> attrs = fids.getAttributes(fi);
Collection<ConfigAttribute> attrs = this.fids.getAttributes(fi);
assertThat(attrs).isNull();
}
@ -138,10 +140,10 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
requestMap.put(new AntPathRequestMatcher("/user/**", null), userAttrs);
requestMap.put(new AntPathRequestMatcher("/teller/**", "GET"),
SecurityConfig.createList("B"));
fids = new DefaultFilterInvocationSecurityMetadataSource(requestMap);
this.fids = new DefaultFilterInvocationSecurityMetadataSource(requestMap);
FilterInvocation fi = createFilterInvocation("/user", null, null, "GET");
Collection<ConfigAttribute> attrs = fids.getAttributes(fi);
Collection<ConfigAttribute> attrs = this.fids.getAttributes(fi);
assertThat(attrs).isEqualTo(userAttrs);
}
@ -155,13 +157,13 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
FilterInvocation fi = createFilterInvocation("/someAdminPage.html", null, null,
null);
Collection<ConfigAttribute> response = fids.getAttributes(fi);
assertThat(response).isEqualTo(def);
Collection<ConfigAttribute> response = this.fids.getAttributes(fi);
assertThat(response).isEqualTo(this.def);
fi = createFilterInvocation("/someAdminPage.html", null, "?", null);
response = fids.getAttributes(fi);
assertThat(response).isEqualTo(def);
response = this.fids.getAttributes(fi);
assertThat(response).isEqualTo(this.def);
}
private FilterInvocation createFilterInvocation(String servletPath, String pathInfo,

View File

@ -52,7 +52,8 @@ public class AntPathRequestMatcherTests {
@Test
public void trailingWildcardMatchesCorrectly() {
AntPathRequestMatcher matcher = new AntPathRequestMatcher("/blah/blAh/**");
AntPathRequestMatcher matcher = new AntPathRequestMatcher("/blah/blAh/**", null,
false);
assertThat(matcher.matches(createRequest("/BLAH/blah"))).isTrue();
assertThat(matcher.matches(createRequest("/blah/bleh"))).isFalse();
assertThat(matcher.matches(createRequest("/blah/blah/"))).isTrue();
@ -64,11 +65,11 @@ public class AntPathRequestMatcherTests {
request.setPathInfo("blah/bleh");
assertThat(matcher.matches(request)).isTrue();
matcher = new AntPathRequestMatcher("/bl?h/blAh/**");
matcher = new AntPathRequestMatcher("/bl?h/blAh/**", null, false);
assertThat(matcher.matches(createRequest("/BLAH/Blah/aaa/"))).isTrue();
assertThat(matcher.matches(createRequest("/bleh/Blah"))).isTrue();
matcher = new AntPathRequestMatcher("/blAh/**/blah/**");
matcher = new AntPathRequestMatcher("/blAh/**/blah/**", null, false);
assertThat(matcher.matches(createRequest("/blah/blah"))).isTrue();
assertThat(matcher.matches(createRequest("/blah/bleh"))).isFalse();
assertThat(matcher.matches(createRequest("/blah/aaa/blah/bbb"))).isTrue();
@ -76,7 +77,8 @@ public class AntPathRequestMatcherTests {
@Test
public void trailingWildcardWithVariableMatchesCorrectly() {
AntPathRequestMatcher matcher = new AntPathRequestMatcher("/{id}/blAh/**");
AntPathRequestMatcher matcher = new AntPathRequestMatcher("/{id}/blAh/**", null,
false);
assertThat(matcher.matches(createRequest("/1234/blah"))).isTrue();
assertThat(matcher.matches(createRequest("/4567/bleh"))).isFalse();
assertThat(matcher.matches(createRequest("/paskos/blah/"))).isTrue();
@ -210,7 +212,7 @@ public class AntPathRequestMatcherTests {
@Test
public void postProcessVariableNameCaseInsensitive() {
AntPathRequestMatcher matcher = new AntPathRequestMatcher("/**");
AntPathRequestMatcher matcher = new AntPathRequestMatcher("/**", null, false);
String variableName = "userName";
assertThat(matcher.postProcessVariableName(variableName))
.isEqualTo(variableName.toLowerCase());