Support Camel case URI variables (#3814)

Perviously there were issues with case insenstive patterns and URI
variables that contained upper case characters. For example, the pattern
"/user/{userId}" could not resolve the variable #userId Instead it was
forced to lowercase and #userid was used.

Now if the pattern is case insensitive then so is the variable. This means
that #userId will work as will #userid.

Fixes gh-3786
This commit is contained in:
Rob Winch 2016-04-18 16:54:48 -05:00 committed by Joe Grandja
parent b01437281d
commit fb5776cb5c
9 changed files with 394 additions and 21 deletions

View File

@ -137,6 +137,39 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests {
response.status == HttpServletResponse.SC_FORBIDDEN
}
def "gh-3786 intercept-url supports cammel case path variables"() {
setup:
MockHttpServletRequest request = new MockHttpServletRequest(method:'GET')
MockHttpServletResponse response = new MockHttpServletResponse()
MockFilterChain chain = new MockFilterChain()
xml.http('use-expressions':true) {
'http-basic'()
'intercept-url'(pattern: '/user/{userName}/**', access: "#userName == authentication.name")
'intercept-url'(pattern: '/**', access: "denyAll")
}
createAppContext()
login(request, 'user', 'password')
when: 'user can access'
request.servletPath = '/user/user/abc'
springSecurityFilterChain.doFilter(request,response,chain)
then: 'The response is OK'
response.status == HttpServletResponse.SC_OK
when: 'user cannot access otheruser'
request = new MockHttpServletRequest(method:'GET', servletPath : '/user/otheruser/abc')
login(request, 'user', 'password')
chain.reset()
springSecurityFilterChain.doFilter(request,response,chain)
then: 'The response is OK'
response.status == HttpServletResponse.SC_FORBIDDEN
when: 'user can access case insensitive URL'
request = new MockHttpServletRequest(method:'GET', servletPath : '/USER/user/abc')
login(request, 'user', 'password')
chain.reset()
springSecurityFilterChain.doFilter(request,response,chain)
then: 'The response is OK'
response.status == HttpServletResponse.SC_FORBIDDEN
}
def "SEC-2256: intercept-url supports path variable type conversion"() {
setup:
MockHttpServletRequest request = new MockHttpServletRequest(method:'GET')

View File

@ -157,6 +157,47 @@ public class AuthorizeRequestsTests {
}
}
// gh-3786
@Test
public void antMatchersPathVariablesCaseInsensitiveCamelCaseVariables() throws Exception {
loadConfig(AntMatchersPathVariablesCamelCaseVariables.class);
this.request.setServletPath("/USER/user");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
this.setup();
this.request.setServletPath("/USER/deny");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
}
@EnableWebSecurity
@Configuration
static class AntMatchersPathVariablesCamelCaseVariables extends WebSecurityConfigurerAdapter {
@Override
protected void configure(HttpSecurity http) throws Exception {
// @formatter:off
http
.authorizeRequests()
.antMatchers("/user/{userName}").access("#userName == 'user'")
.anyRequest().denyAll();
// @formatter:on
}
@Override
protected void configure(AuthenticationManagerBuilder auth) throws Exception {
// @formatter:off
auth
.inMemoryAuthentication();
// @formatter:on
}
}
public void loadConfig(Class<?>... configs) {
this.context = new AnnotationConfigWebApplicationContext();
this.context.register(configs);

View File

@ -41,15 +41,28 @@ abstract class AbstractVariableEvaluationContextPostProcessor
@Override
public final EvaluationContext postProcess(EvaluationContext context,
FilterInvocation invocation) {
HttpServletRequest request = invocation.getHttpRequest();
Map<String, String> variables = extractVariables(request);
for (Map.Entry<String, String> entry : variables.entrySet()) {
context.setVariable(entry.getKey(), entry.getValue());
}
return context;
final HttpServletRequest request = invocation.getHttpRequest();
return new DelegatingEvaluationContext(context) {
private Map<String, String> variables;
@Override
public Object lookupVariable(String name) {
Object result = super.lookupVariable(name);
if (result != null) {
return result;
}
if (this.variables == null) {
this.variables = extractVariables(request);
}
name = postProcessVariableName(name);
return this.variables.get(name);
}
};
}
protected abstract Map<String, String> extractVariables(
HttpServletRequest request);
abstract Map<String, String> extractVariables(HttpServletRequest request);
abstract String postProcessVariableName(String variableName);
}

View File

@ -0,0 +1,100 @@
/*
* Copyright 2012-2016 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
*
* http://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.
*/
package org.springframework.security.web.access.expression;
import java.util.List;
import org.springframework.expression.BeanResolver;
import org.springframework.expression.ConstructorResolver;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.MethodResolver;
import org.springframework.expression.OperatorOverloader;
import org.springframework.expression.PropertyAccessor;
import org.springframework.expression.TypeComparator;
import org.springframework.expression.TypeConverter;
import org.springframework.expression.TypeLocator;
import org.springframework.expression.TypedValue;
/**
* An instance of {@link EvaluationContext} that delegates to another implementation.
*
* @author Rob Winch
* @since 4.1
*/
class DelegatingEvaluationContext implements EvaluationContext {
private final EvaluationContext delegate;
public DelegatingEvaluationContext(EvaluationContext delegate) {
super();
this.delegate = delegate;
}
@Override
public TypedValue getRootObject() {
return this.delegate.getRootObject();
}
@Override
public List<ConstructorResolver> getConstructorResolvers() {
return this.delegate.getConstructorResolvers();
}
@Override
public List<MethodResolver> getMethodResolvers() {
return this.delegate.getMethodResolvers();
}
@Override
public List<PropertyAccessor> getPropertyAccessors() {
return this.delegate.getPropertyAccessors();
}
@Override
public TypeLocator getTypeLocator() {
return this.delegate.getTypeLocator();
}
@Override
public TypeConverter getTypeConverter() {
return this.delegate.getTypeConverter();
}
@Override
public TypeComparator getTypeComparator() {
return this.delegate.getTypeComparator();
}
@Override
public OperatorOverloader getOperatorOverloader() {
return this.delegate.getOperatorOverloader();
}
@Override
public BeanResolver getBeanResolver() {
return this.delegate.getBeanResolver();
}
@Override
public void setVariable(String name, Object value) {
this.delegate.setVariable(name, value);
}
@Override
public Object lookupVariable(String name) {
return this.delegate.lookupVariable(name);
}
}

View File

@ -109,10 +109,14 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource
}
@Override
protected Map<String, String> extractVariables(
HttpServletRequest request) {
Map<String, String> extractVariables(HttpServletRequest request) {
return this.matcher.extractUriTemplateVariables(request);
}
@Override
String postProcessVariableName(String variableName) {
return this.matcher.postProcessVariableName(variableName);
}
}
}

View File

@ -172,6 +172,10 @@ public final class AntPathRequestMatcher implements RequestMatcher {
return this.matcher.extractUriTemplateVariables(url);
}
public String postProcessVariableName(String variableName) {
return this.caseSensitive ? variableName : variableName.toLowerCase();
}
private String getRequestPath(HttpServletRequest request) {
String url = request.getServletPath();

View File

@ -23,6 +23,7 @@ import javax.servlet.http.HttpServletRequest;
import org.junit.Before;
import org.junit.Test;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.mock.web.MockFilterChain;
import org.springframework.mock.web.MockHttpServletRequest;
@ -36,18 +37,19 @@ import static org.assertj.core.api.Assertions.assertThat;
*
*/
public class AbstractVariableEvaluationContextPostProcessorTests {
AbstractVariableEvaluationContextPostProcessor processor;
static final String KEY = "a";
static final String VALUE = "b";
VariableEvaluationContextPostProcessor processor;
FilterInvocation invocation;
MockHttpServletRequest request;
MockHttpServletResponse response;
StandardEvaluationContext context;
EvaluationContext context;
@Before
public void setup() {
this.processor = new VariableEvaluationContextPostProcessor();
this.request = new MockHttpServletRequest();
this.request.setServletPath("/");
this.response = new MockHttpServletResponse();
@ -57,23 +59,40 @@ public class AbstractVariableEvaluationContextPostProcessorTests {
}
@Test
public void postProcess() {
this.processor.postProcess(this.context, this.invocation);
public void extractVariables() {
this.context = this.processor.postProcess(this.context, this.invocation);
for (String key : VariableEvaluationContextPostProcessor.RESULTS.keySet()) {
assertThat(this.context.lookupVariable(key))
.isEqualTo(VariableEvaluationContextPostProcessor.RESULTS.get(key));
}
assertThat(this.context.lookupVariable(KEY)).isEqualTo(VALUE);
}
@Test
public void postProcessVariableName() {
this.context = this.processor.postProcess(this.context, this.invocation);
assertThat(this.context.lookupVariable("nothing")).isEqualTo(VALUE);
}
@Test
public void extractVariablesOnlyUsedOnce() {
this.context = this.processor.postProcess(this.context, this.invocation);
assertThat(this.context.lookupVariable(KEY)).isEqualTo(VALUE);
this.processor.results = Collections.emptyMap();
assertThat(this.context.lookupVariable(KEY)).isEqualTo(VALUE);
}
static class VariableEvaluationContextPostProcessor
extends AbstractVariableEvaluationContextPostProcessor {
static final Map<String, String> RESULTS = Collections.singletonMap("a", "b");
Map<String, String> results = Collections.singletonMap(KEY, VALUE);
@Override
protected Map<String, String> extractVariables(HttpServletRequest request) {
return RESULTS;
return this.results;
}
@Override
String postProcessVariableName(String variableName) {
return KEY;
}
}
}

View File

@ -0,0 +1,144 @@
/*
* Copyright 2012-2016 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
*
* http://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.
*/
package org.springframework.security.web.access.expression;
import java.util.ArrayList;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import org.springframework.expression.BeanResolver;
import org.springframework.expression.ConstructorResolver;
import org.springframework.expression.MethodResolver;
import org.springframework.expression.OperatorOverloader;
import org.springframework.expression.PropertyAccessor;
import org.springframework.expression.TypeComparator;
import org.springframework.expression.TypeConverter;
import org.springframework.expression.TypeLocator;
import org.springframework.expression.TypedValue;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
/**
* @author Rob Winch
*/
@RunWith(MockitoJUnitRunner.class)
public class DelegatingEvaluationContextTests {
@Mock
DelegatingEvaluationContext delegate;
@InjectMocks
DelegatingEvaluationContext context;
@Test
public void getRootObject() {
TypedValue expected = mock(TypedValue.class);
when(this.delegate.getRootObject()).thenReturn(expected);
assertThat(this.context.getRootObject()).isEqualTo(expected);
}
@Test
public void getConstructorResolvers() {
List<ConstructorResolver> expected = new ArrayList<ConstructorResolver>();
when(this.delegate.getConstructorResolvers()).thenReturn(expected);
assertThat(this.context.getConstructorResolvers()).isEqualTo(expected);
}
@Test
public void getMethodResolvers() {
List<MethodResolver> expected = new ArrayList<MethodResolver>();
when(this.delegate.getMethodResolvers()).thenReturn(expected);
assertThat(this.context.getMethodResolvers()).isEqualTo(expected);
}
@Test
public void getPropertyAccessors() {
List<PropertyAccessor> expected = new ArrayList<PropertyAccessor>();
when(this.delegate.getPropertyAccessors()).thenReturn(expected);
assertThat(this.context.getPropertyAccessors()).isEqualTo(expected);
}
@Test
public void getTypeLocator() {
TypeLocator expected = mock(TypeLocator.class);
when(this.delegate.getTypeLocator()).thenReturn(expected);
assertThat(this.context.getTypeLocator()).isEqualTo(expected);
}
@Test
public void getTypeConverter() {
TypeConverter expected = mock(TypeConverter.class);
when(this.delegate.getTypeConverter()).thenReturn(expected);
assertThat(this.context.getTypeConverter()).isEqualTo(expected);
}
@Test
public void getTypeComparator() {
TypeComparator expected = mock(TypeComparator.class);
when(this.delegate.getTypeComparator()).thenReturn(expected);
assertThat(this.context.getTypeComparator()).isEqualTo(expected);
}
@Test
public void getOperatorOverloader() {
OperatorOverloader expected = mock(OperatorOverloader.class);
when(this.delegate.getOperatorOverloader()).thenReturn(expected);
assertThat(this.context.getOperatorOverloader()).isEqualTo(expected);
}
@Test
public void getBeanResolver() {
BeanResolver expected = mock(BeanResolver.class);
when(this.delegate.getBeanResolver()).thenReturn(expected);
assertThat(this.context.getBeanResolver()).isEqualTo(expected);
}
@Test
public void setVariable() {
String name = "name";
String value = "value";
this.context.setVariable(name, value);
verify(this.delegate).setVariable(name, value);
}
@Test
public void lookupVariable() {
String name = "name";
String expected = "expected";
when(this.delegate.lookupVariable(name)).thenReturn(expected);
assertThat(this.context.lookupVariable(name)).isEqualTo(expected);
}
}

View File

@ -197,6 +197,21 @@ public class AntPathRequestMatcherTests {
assertThat(matcher.matches(request)).isFalse();
}
@Test
public void postProcessVariableNameCaseInsensitive() {
AntPathRequestMatcher matcher = new AntPathRequestMatcher("/**");
String variableName = "userName";
assertThat(matcher.postProcessVariableName(variableName))
.isEqualTo(variableName.toLowerCase());
}
@Test
public void postProcessVariableNameCaseSensitive() {
AntPathRequestMatcher matcher = new AntPathRequestMatcher("/**", null, true);
String variableName = "userName";
assertThat(matcher.postProcessVariableName(variableName)).isEqualTo(variableName);
}
private HttpServletRequest createRequestWithNullMethod(String path) {
when(this.request.getQueryString()).thenReturn("doesntMatter");
when(this.request.getServletPath()).thenReturn(path);