diff --git a/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy index b0bd6f86db..56b3d9ff62 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy @@ -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') diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java index b0c3341db6..4267ef23f5 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java @@ -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); diff --git a/web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java b/web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java index 188225aff0..f0f70e0cfb 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessor.java @@ -41,15 +41,28 @@ abstract class AbstractVariableEvaluationContextPostProcessor @Override public final EvaluationContext postProcess(EvaluationContext context, FilterInvocation invocation) { - HttpServletRequest request = invocation.getHttpRequest(); - Map variables = extractVariables(request); - for (Map.Entry entry : variables.entrySet()) { - context.setVariable(entry.getKey(), entry.getValue()); - } - return context; + final HttpServletRequest request = invocation.getHttpRequest(); + return new DelegatingEvaluationContext(context) { + private Map 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 extractVariables( - HttpServletRequest request); + abstract Map extractVariables(HttpServletRequest request); + + abstract String postProcessVariableName(String variableName); } diff --git a/web/src/main/java/org/springframework/security/web/access/expression/DelegatingEvaluationContext.java b/web/src/main/java/org/springframework/security/web/access/expression/DelegatingEvaluationContext.java new file mode 100644 index 0000000000..efa9868329 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/access/expression/DelegatingEvaluationContext.java @@ -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 getConstructorResolvers() { + return this.delegate.getConstructorResolvers(); + } + + @Override + public List getMethodResolvers() { + return this.delegate.getMethodResolvers(); + } + + @Override + public List 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); + } +} diff --git a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java index 3a9fb67f8e..8f77a946c2 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java @@ -109,10 +109,14 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource } @Override - protected Map extractVariables( - HttpServletRequest request) { + Map extractVariables(HttpServletRequest request) { return this.matcher.extractUriTemplateVariables(request); } + + @Override + String postProcessVariableName(String variableName) { + return this.matcher.postProcessVariableName(variableName); + } } } diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java index cc6b44c21f..598dbc33c9 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java @@ -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(); diff --git a/web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java b/web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java index 3bdb301f71..b7eda64420 100644 --- a/web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java +++ b/web/src/test/java/org/springframework/security/web/access/expression/AbstractVariableEvaluationContextPostProcessorTests.java @@ -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 RESULTS = Collections.singletonMap("a", "b"); + Map results = Collections.singletonMap(KEY, VALUE); @Override protected Map extractVariables(HttpServletRequest request) { - return RESULTS; + return this.results; } + @Override + String postProcessVariableName(String variableName) { + return KEY; + } } } diff --git a/web/src/test/java/org/springframework/security/web/access/expression/DelegatingEvaluationContextTests.java b/web/src/test/java/org/springframework/security/web/access/expression/DelegatingEvaluationContextTests.java new file mode 100644 index 0000000000..008b8e028a --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/access/expression/DelegatingEvaluationContextTests.java @@ -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 expected = new ArrayList(); + when(this.delegate.getConstructorResolvers()).thenReturn(expected); + + assertThat(this.context.getConstructorResolvers()).isEqualTo(expected); + } + + @Test + public void getMethodResolvers() { + List expected = new ArrayList(); + when(this.delegate.getMethodResolvers()).thenReturn(expected); + + assertThat(this.context.getMethodResolvers()).isEqualTo(expected); + } + + @Test + public void getPropertyAccessors() { + List expected = new ArrayList(); + 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); + } +} \ No newline at end of file diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/AntPathRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/AntPathRequestMatcherTests.java index b864549939..52fcdc4dba 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/AntPathRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/util/matcher/AntPathRequestMatcherTests.java @@ -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);