From 24caad5a67fba9c8da7154551b0a2674a82d2ebb Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 22 Jan 2008 20:25:46 +0000 Subject: [PATCH] Make sure default lower/upper case is respected for regex and ant paths when not set explicitly using the lowercase-comparisons attribute. Added much more comprehensive testing of HttpSecurityBeanDefinitionParser. --- .../HttpSecurityBeanDefinitionParser.java | 31 ++- .../OrderedFilterBeanDefinitionDecorator.java | 4 + ...HttpSecurityBeanDefinitionParserTests.java | 186 ++++++++++++++---- 3 files changed, 178 insertions(+), 43 deletions(-) diff --git a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java index e9f42dd1f8..42fa72022d 100644 --- a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java +++ b/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java @@ -32,6 +32,7 @@ import org.springframework.security.securechannel.RetryWithHttpsEntryPoint; import org.springframework.security.ui.ExceptionTranslationFilter; import org.springframework.security.util.FilterChainProxy; import org.springframework.security.util.RegexUrlPathMatcher; +import org.springframework.security.util.AntUrlPathMatcher; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; @@ -120,25 +121,39 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { FilterInvocationDefinitionMap interceptorFilterInvDefSource = new PathBasedFilterInvocationDefinitionMap(); FilterInvocationDefinitionMap channelFilterInvDefSource = new PathBasedFilterInvocationDefinitionMap(); - if (patternType.equals(OPT_PATH_TYPE_REGEX)) { - filterChainProxy.getPropertyValues().addPropertyValue("matcher", new RegexUrlPathMatcher()); - interceptorFilterInvDefSource = new RegExpBasedFilterInvocationDefinitionMap(); - channelFilterInvDefSource = new RegExpBasedFilterInvocationDefinitionMap(); - } - // Deal with lowercase conversion requests String lowercaseComparisons = element.getAttribute(ATT_LOWERCASE_COMPARISONS); if (!StringUtils.hasText(lowercaseComparisons)) { - lowercaseComparisons = DEF_LOWERCASE_COMPARISONS; + lowercaseComparisons = null; } + + // Only change from the defaults if the attribute has been set if ("true".equals(lowercaseComparisons)) { interceptorFilterInvDefSource.setConvertUrlToLowercaseBeforeComparison(true); channelFilterInvDefSource.setConvertUrlToLowercaseBeforeComparison(true); - } else { + } else if ("false".equals(lowercaseComparisons)) { interceptorFilterInvDefSource.setConvertUrlToLowercaseBeforeComparison(false); channelFilterInvDefSource.setConvertUrlToLowercaseBeforeComparison(false); } + if (patternType.equals(OPT_PATH_TYPE_REGEX)) { + RegexUrlPathMatcher matcher = new RegexUrlPathMatcher(); + + if (lowercaseComparisons != null) { + matcher.setRequiresLowerCaseUrl("true".equals(lowercaseComparisons)); + } + + filterChainProxy.getPropertyValues().addPropertyValue("matcher", matcher); + + interceptorFilterInvDefSource = new RegExpBasedFilterInvocationDefinitionMap(); + channelFilterInvDefSource = new RegExpBasedFilterInvocationDefinitionMap(); + } else if (lowercaseComparisons != null) { + AntUrlPathMatcher matcher = new AntUrlPathMatcher(); + matcher.setRequiresLowerCaseUrl("true".equals(lowercaseComparisons)); + + filterChainProxy.getPropertyValues().addPropertyValue("matcher", matcher); + } + // Add servlet-api integration filter if required String provideServletApi = element.getAttribute(ATT_SERVLET_API_PROVISION); if (!StringUtils.hasText(provideServletApi)) { diff --git a/core/src/main/java/org/springframework/security/config/OrderedFilterBeanDefinitionDecorator.java b/core/src/main/java/org/springframework/security/config/OrderedFilterBeanDefinitionDecorator.java index c9491998df..5b925e518f 100644 --- a/core/src/main/java/org/springframework/security/config/OrderedFilterBeanDefinitionDecorator.java +++ b/core/src/main/java/org/springframework/security/config/OrderedFilterBeanDefinitionDecorator.java @@ -82,5 +82,9 @@ public class OrderedFilterBeanDefinitionDecorator implements BeanDefinitionDecor public final void setOrder(int order) { this.order = new Integer(order); } + + public String getBeanName() { + return beanName; + } } } diff --git a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java index 7c72c07907..b61cc6e2b2 100644 --- a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java +++ b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java @@ -1,8 +1,9 @@ package org.springframework.security.config; -import org.springframework.security.concurrent.ConcurrentSessionFilter; import org.springframework.security.context.HttpSessionContextIntegrationFilter; import org.springframework.security.intercept.web.FilterSecurityInterceptor; +import org.springframework.security.intercept.web.FilterInvocationDefinitionSource; +import org.springframework.security.intercept.web.FilterInvocation; import org.springframework.security.securechannel.ChannelProcessingFilter; import org.springframework.security.ui.ExceptionTranslationFilter; import org.springframework.security.ui.basicauth.BasicProcessingFilter; @@ -12,15 +13,19 @@ import org.springframework.security.ui.webapp.AuthenticationProcessingFilter; import org.springframework.security.ui.webapp.DefaultLoginPageGeneratingFilter; import org.springframework.security.util.FilterChainProxy; import org.springframework.security.util.PortMapperImpl; +import org.springframework.security.util.InMemoryXmlApplicationContext; import org.springframework.security.wrapper.SecurityContextHolderAwareRequestFilter; -import org.springframework.context.support.ClassPathXmlApplicationContext; -import org.springframework.beans.BeansException; +import org.springframework.security.providers.anonymous.AnonymousProcessingFilter; +import org.springframework.security.MockFilterChain; +import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.SecurityConfig; +import org.springframework.context.support.AbstractXmlApplicationContext; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; -import org.junit.AfterClass; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import org.junit.BeforeClass; +import static org.junit.Assert.*; import org.junit.Test; +import org.junit.After; import java.util.Iterator; import java.util.List; @@ -30,47 +35,39 @@ import java.util.List; * @version $Id$ */ public class HttpSecurityBeanDefinitionParserTests { - private static ClassPathXmlApplicationContext appContext; + private AbstractXmlApplicationContext appContext; + private static final String AUTH_PROVIDER_XML = + " " + + " " + + " " + + " " + + " " + + " "; - @BeforeClass - public static void loadContext() { - try { - appContext = new ClassPathXmlApplicationContext("org/springframework/security/config/http-security.xml"); - } catch (BeansException e) { - e.printStackTrace(); - } - } - - @AfterClass - public static void closeAppContext() { + @After + public void closeAppContext() { if (appContext != null) { appContext.close(); + appContext = null; } } @Test - public void filterChainProxyShouldReturnEmptyFilterListForUnprotectedUrl() { - FilterChainProxy filterChainProxy = - (FilterChainProxy) appContext.getBean(BeanIds.FILTER_CHAIN_PROXY); + public void httpAutoConfigSetsUpCorrectFilterList() { + setContext("" + AUTH_PROVIDER_XML); - List filters = filterChainProxy.getFilters("/unprotected"); + FilterChainProxy filterChainProxy = getFilterChainProxy(); - assertTrue(filters.size() == 0); + List filterList = filterChainProxy.getFilters("/anyurl"); + + checkAutoConfigFilters(filterList); } - @Test - public void filterChainProxyShouldReturnCorrectFilterListForProtectedUrl() { - FilterChainProxy filterChainProxy = - (FilterChainProxy) appContext.getBean(BeanIds.FILTER_CHAIN_PROXY); - - List filterList = filterChainProxy.getFilters("/someurl"); - - assertEquals("Expected 12 filters in chain", 12, filterList.size()); + private void checkAutoConfigFilters(List filterList) { + assertEquals("Expected 10 filters in chain", 10, filterList.size()); Iterator filters = filterList.iterator(); - assertTrue(filters.next() instanceof ChannelProcessingFilter); - assertTrue(filters.next() instanceof ConcurrentSessionFilter); assertTrue(filters.next() instanceof HttpSessionContextIntegrationFilter); assertTrue(filters.next() instanceof LogoutFilter); assertTrue(filters.next() instanceof AuthenticationProcessingFilter); @@ -78,17 +75,136 @@ public class HttpSecurityBeanDefinitionParserTests { assertTrue(filters.next() instanceof BasicProcessingFilter); assertTrue(filters.next() instanceof SecurityContextHolderAwareRequestFilter); assertTrue(filters.next() instanceof RememberMeProcessingFilter); + assertTrue(filters.next() instanceof AnonymousProcessingFilter); assertTrue(filters.next() instanceof ExceptionTranslationFilter); assertTrue(filters.next() instanceof FilterSecurityInterceptor); - assertTrue(filters.next() instanceof OrderedFilterBeanDefinitionDecorator.OrderedFilterDecorator); + } + + @Test + public void filterListShouldBeEmptyForUnprotectedUrl() { + setContext( + " " + + " " + + " " + AUTH_PROVIDER_XML); + + FilterChainProxy filterChainProxy = getFilterChainProxy(); + + List filters = filterChainProxy.getFilters("/unprotected"); + + assertTrue(filters.size() == 0); + } + + @Test + public void regexPathsWorkCorrectly() { + setContext( + " " + + " " + + " " + AUTH_PROVIDER_XML); + FilterChainProxy filterChainProxy = getFilterChainProxy(); + assertEquals(0, filterChainProxy.getFilters("/imlowercase").size()); + // This will be matched by the default pattern ".*" + checkAutoConfigFilters(filterChainProxy.getFilters("/ImCaughtByTheUniversalMatchPattern")); + } + + @Test + public void lowerCaseComparisonAttributeIsRespectedByFilterChainProxy() { + setContext( + " " + + " " + + " " + AUTH_PROVIDER_XML); + FilterChainProxy filterChainProxy = getFilterChainProxy(); + assertEquals(0, filterChainProxy.getFilters("/Secure").size()); + // These will be matched by the default pattern "/**" + checkAutoConfigFilters(filterChainProxy.getFilters("/secure")); + checkAutoConfigFilters(filterChainProxy.getFilters("/ImCaughtByTheUniversalMatchPattern")); } + @Test + public void lowerCaseComparisonIsRespectedBySecurityFilterInvocationDefinitionSource() throws Exception { + setContext( + " " + + " " + + " " + + " " + AUTH_PROVIDER_XML); + + FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR); + + FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource(); + ConfigAttributeDefinition attrs = fids.getAttributes(createFilterinvocation("/Secure")); + assertEquals(2, attrs.size()); + assertTrue(attrs.contains(new SecurityConfig("ROLE_A"))); + assertTrue(attrs.contains(new SecurityConfig("ROLE_B"))); + attrs = fids.getAttributes(createFilterinvocation("/secure")); + assertEquals(1, attrs.size()); + assertTrue(attrs.contains(new SecurityConfig("ROLE_C"))); + } + + @Test + public void minimalConfigurationParses() { + setContext("" + AUTH_PROVIDER_XML); + } + + @Test + public void interceptUrlWithRequiresChannelAddsChannelFilterToStack() { + setContext( + " " + + " " + + " " + AUTH_PROVIDER_XML); + FilterChainProxy filterChainProxy = getFilterChainProxy(); + + List filters = filterChainProxy.getFilters("/someurl"); + + assertEquals("Expected 11 filters in chain", 11, filters.size()); + + assertTrue(filters.get(0) instanceof ChannelProcessingFilter); + } + @Test public void portMappingsAreParsedCorrectly() throws Exception { + setContext( + " " + + " " + + " " + + " " + + " " + AUTH_PROVIDER_XML); + PortMapperImpl pm = (PortMapperImpl) appContext.getBean(BeanIds.PORT_MAPPER); assertEquals(1, pm.getTranslatedPortMappings().size()); assertEquals(Integer.valueOf(9080), pm.lookupHttpPort(9443)); assertEquals(Integer.valueOf(9443), pm.lookupHttpsPort(9080)); } + + @Test + public void externalFiltersAreTreatedCorrectly() { + // Decorated user-filter should be added to stack. The other one should be ignored + setContext( + "" + AUTH_PROVIDER_XML + + "" + + " " + + "" + + ""); + List filters = getFilterChainProxy().getFilters("/someurl"); + + assertEquals(11, filters.size()); + assertTrue(filters.get(10) instanceof OrderedFilterBeanDefinitionDecorator.OrderedFilterDecorator); + assertEquals("userFilter", ((OrderedFilterBeanDefinitionDecorator.OrderedFilterDecorator)filters.get(10)).getBeanName()); + } + + private void setContext(String context) { + appContext = new InMemoryXmlApplicationContext(context); + } + + private FilterChainProxy getFilterChainProxy() { + return (FilterChainProxy) appContext.getBean(BeanIds.FILTER_CHAIN_PROXY); + } + + private FilterInvocation createFilterinvocation(String path) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI(null); + + request.setServletPath(path); + + return new FilterInvocation(request, new MockHttpServletResponse(), new MockFilterChain()); + } }