From 93438defffe5c339026469afa09dad60b2928a4f Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Mon, 1 Mar 2010 00:59:46 +0000 Subject: [PATCH] SEC-1407: Use RequestMatcher instances as the FilterInvocationSecurityMetadataSource keys and in the FilterChainMap use by FilterChainProxy. This greatly simplifies the code and opens up possibilities for other matching strategies (e.g. EL). This also means that matching is now completely strict - the order of the matchers is all that matters (not whether an HTTP method is included or not). The first matcher that returns true will be used. --- .../http/DefaultFilterChainValidator.java | 18 +- ...FilterChainMapBeanDefinitionDecorator.java | 20 +- ...nvocationSecurityMetadataSourceParser.java | 27 +- .../config/http/HttpConfigurationBuilder.java | 35 +- .../HttpSecurityBeanDefinitionParser.java | 66 +-- .../security/config/http/MatcherType.java | 66 +++ .../security/config/spring-security-3.1.rnc | 19 +- .../security/config/spring-security-3.1.xsd | 67 ++- .../config/FilterChainProxyConfigTests.java | 23 +- ...HttpSecurityBeanDefinitionParserTests.java | 59 +-- .../security/util/filtertest-valid.xml | 31 +- .../security/web/FilterChainProxy.java | 167 +++---- .../security/web/FilterInvocation.java | 429 +++++++++++++++++- ...efaultWebInvocationPrivilegeEvaluator.java | 417 +---------------- ...ilterInvocationSecurityMetadataSource.java | 22 +- ...ilterInvocationSecurityMetadataSource.java | 173 +------ .../web/util/AntPathRequestMatcher.java | 94 ++++ .../security/web/util/AnyRequestMatcher.java | 17 + .../web/util/RegexRequestMatcher.java | 64 +++ .../intercept => }/FilterInvocationTests.java | 19 +- ...tWebInvocationPrivilegeEvaluatorTests.java | 17 - ...InvocationSecurityMetadataSourceTests.java | 119 ++--- web/template.mf | 1 + 23 files changed, 978 insertions(+), 992 deletions(-) create mode 100644 config/src/main/java/org/springframework/security/config/http/MatcherType.java create mode 100644 web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java create mode 100644 web/src/main/java/org/springframework/security/web/util/AnyRequestMatcher.java create mode 100644 web/src/main/java/org/springframework/security/web/util/RegexRequestMatcher.java rename web/src/test/java/org/springframework/security/web/{access/intercept => }/FilterInvocationTests.java (87%) diff --git a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java index b92f704a44..38450244d8 100644 --- a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java +++ b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java @@ -2,7 +2,6 @@ package org.springframework.security.config.http; import java.util.Collection; import java.util.List; -import java.util.Map; import javax.servlet.Filter; @@ -11,6 +10,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.authentication.AnonymousAuthenticationToken; import org.springframework.security.web.FilterChainProxy; +import org.springframework.security.web.FilterInvocation; import org.springframework.security.web.access.ExceptionTranslationFilter; import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; @@ -22,18 +22,17 @@ import org.springframework.security.web.authentication.www.BasicAuthenticationFi import org.springframework.security.web.context.SecurityContextPersistenceFilter; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.session.SessionManagementFilter; +import org.springframework.security.web.util.AnyRequestMatcher; public class DefaultFilterChainValidator implements FilterChainProxy.FilterChainValidator { private Log logger = LogFactory.getLog(getClass()); public void validate(FilterChainProxy fcp) { - Map> filterChainMap = fcp.getFilterChainMap(); - for(String pattern : fcp.getFilterChainMap().keySet()) { - List filters = filterChainMap.get(pattern); + for(List filters : fcp.getFilterChainMap().values()) { checkFilterStack(filters); } - checkLoginPageIsntProtected(fcp, filterChainMap.get(fcp.getMatcher().getUniversalMatchPattern())); + checkLoginPageIsntProtected(fcp); } private Object getFilter(Class type, List filters) { @@ -78,12 +77,14 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain } /* Checks for the common error of having a login page URL protected by the security interceptor */ - private void checkLoginPageIsntProtected(FilterChainProxy fcp, List defaultFilters) { + private void checkLoginPageIsntProtected(FilterChainProxy fcp) { + List defaultFilters = fcp.getFilterChainMap().get(new AnyRequestMatcher()); ExceptionTranslationFilter etf = (ExceptionTranslationFilter)getFilter(ExceptionTranslationFilter.class, defaultFilters); if (etf.getAuthenticationEntryPoint() instanceof LoginUrlAuthenticationEntryPoint) { String loginPage = ((LoginUrlAuthenticationEntryPoint)etf.getAuthenticationEntryPoint()).getLoginFormUrl(); + FilterInvocation loginRequest = new FilterInvocation(loginPage, "POST"); List filters = fcp.getFilters(loginPage); logger.info("Checking whether login URL '" + loginPage + "' is accessible with your configuration"); @@ -100,7 +101,8 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain FilterSecurityInterceptor fsi = (FilterSecurityInterceptor) getFilter(FilterSecurityInterceptor.class, filters); DefaultFilterInvocationSecurityMetadataSource fids = (DefaultFilterInvocationSecurityMetadataSource) fsi.getSecurityMetadataSource(); - Collection attributes = fids.lookupAttributes(loginPage, "POST"); + + Collection attributes = fids.getAttributes(loginRequest); if (attributes == null) { logger.debug("No access attributes defined for login page URL"); @@ -122,7 +124,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain AnonymousAuthenticationToken token = new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(), anonPF.getUserAttribute().getAuthorities()); try { - fsi.getAccessDecisionManager().decide(token, new Object(), fids.lookupAttributes(loginPage, "POST")); + fsi.getAccessDecisionManager().decide(token, new Object(), attributes); } catch (Exception e) { logger.warn("Anonymous access to the login page doesn't appear to be enabled. This is almost certainly " + "an error. Please check your configuration allows unauthenticated access to the configured " + diff --git a/config/src/main/java/org/springframework/security/config/http/FilterChainMapBeanDefinitionDecorator.java b/config/src/main/java/org/springframework/security/config/http/FilterChainMapBeanDefinitionDecorator.java index 2490ea525f..1b2deadd88 100644 --- a/config/src/main/java/org/springframework/security/config/http/FilterChainMapBeanDefinitionDecorator.java +++ b/config/src/main/java/org/springframework/security/config/http/FilterChainMapBeanDefinitionDecorator.java @@ -1,5 +1,10 @@ package org.springframework.security.config.http; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanDefinitionHolder; import org.springframework.beans.factory.config.RuntimeBeanReference; @@ -8,14 +13,11 @@ import org.springframework.beans.factory.support.ManagedMap; import org.springframework.beans.factory.xml.BeanDefinitionDecorator; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.security.config.Elements; -import org.springframework.security.web.util.RegexUrlPathMatcher; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; import org.w3c.dom.Node; -import java.util.*; - /** * Sets the filter chain Map for a FilterChainProxy bean declaration. * @@ -30,11 +32,7 @@ public class FilterChainMapBeanDefinitionDecorator implements BeanDefinitionDeco Map filterChainMap = new LinkedHashMap(); Element elt = (Element)node; - String pathType = elt.getAttribute(HttpSecurityBeanDefinitionParser.ATT_PATH_TYPE); - - if (HttpSecurityBeanDefinitionParser.OPT_PATH_TYPE_REGEX.equals(pathType)) { - filterChainProxy.getPropertyValues().addPropertyValue("matcher", new RegexUrlPathMatcher()); - } + MatcherType matcherType = MatcherType.fromElement(elt); List filterChainElts = DomUtils.getChildElementsByTagName(elt, Elements.FILTER_CHAIN); @@ -52,8 +50,10 @@ public class FilterChainMapBeanDefinitionDecorator implements BeanDefinitionDeco "'must not be empty", elt); } + BeanDefinition matcher = matcherType.createMatcher(path, null); + if (filters.equals(HttpSecurityBeanDefinitionParser.OPT_FILTERS_NONE)) { - filterChainMap.put(path, Collections.EMPTY_LIST); + filterChainMap.put(matcher, Collections.EMPTY_LIST); } else { String[] filterBeanNames = StringUtils.tokenizeToStringArray(filters, ","); ManagedList filterChain = new ManagedList(filterBeanNames.length); @@ -62,7 +62,7 @@ public class FilterChainMapBeanDefinitionDecorator implements BeanDefinitionDeco filterChain.add(new RuntimeBeanReference(filterBeanNames[i])); } - filterChainMap.put(path, filterChain); + filterChainMap.put(matcher, filterChain); } } diff --git a/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java b/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java index 6505a7701a..c8e9382677 100644 --- a/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java +++ b/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java @@ -17,9 +17,6 @@ import org.springframework.security.web.access.expression.DefaultWebSecurityExpr import org.springframework.security.web.access.expression.ExpressionBasedFilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource; -import org.springframework.security.web.access.intercept.RequestKey; -import org.springframework.security.web.util.AntUrlPathMatcher; -import org.springframework.security.web.util.UrlMatcher; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; @@ -63,11 +60,11 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit } static BeanDefinition createSecurityMetadataSource(List interceptUrls, Element elt, ParserContext pc) { - UrlMatcher matcher = HttpSecurityBeanDefinitionParser.createUrlMatcher(elt); + MatcherType matcherType = MatcherType.fromElement(elt); boolean useExpressions = isUseExpressions(elt); ManagedMap requestToAttributesMap = parseInterceptUrlsForFilterInvocationRequestMap( - interceptUrls, useExpressions, pc); + matcherType, interceptUrls, useExpressions, pc); BeanDefinitionBuilder fidsBuilder; if (useExpressions) { @@ -83,16 +80,14 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit } fidsBuilder = BeanDefinitionBuilder.rootBeanDefinition(ExpressionBasedFilterInvocationSecurityMetadataSource.class); - fidsBuilder.addConstructorArgValue(matcher); fidsBuilder.addConstructorArgValue(requestToAttributesMap); fidsBuilder.addConstructorArgReference(expressionHandlerRef); } else { fidsBuilder = BeanDefinitionBuilder.rootBeanDefinition(DefaultFilterInvocationSecurityMetadataSource.class); - fidsBuilder.addConstructorArgValue(matcher); fidsBuilder.addConstructorArgValue(requestToAttributesMap); } - fidsBuilder.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher); +// fidsBuilder.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher); fidsBuilder.getRawBeanDefinition().setSource(pc.extractSource(elt)); return fidsBuilder.getBeanDefinition(); @@ -102,8 +97,9 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit return "true".equals(elt.getAttribute(ATT_USE_EXPRESSIONS)); } - private static ManagedMap parseInterceptUrlsForFilterInvocationRequestMap(List urlElts, - boolean useExpressions, ParserContext parserContext) { + private static ManagedMap + parseInterceptUrlsForFilterInvocationRequestMap(MatcherType matcherType, + List urlElts, boolean useExpressions, ParserContext parserContext) { ManagedMap filterInvocationDefinitionMap = new ManagedMap(); @@ -124,10 +120,7 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit method = null; } - BeanDefinitionBuilder keyBldr = BeanDefinitionBuilder.rootBeanDefinition(RequestKey.class); - keyBldr.addConstructorArgValue(path); - keyBldr.addConstructorArgValue(method); - + BeanDefinition matcher = matcherType.createMatcher(path, method); BeanDefinitionBuilder attributeBuilder = BeanDefinitionBuilder.rootBeanDefinition(SecurityConfig.class); attributeBuilder.addConstructorArgValue(access); @@ -140,13 +133,11 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit attributeBuilder.setFactoryMethod("createListFromCommaDelimitedString"); } - BeanDefinition key = keyBldr.getBeanDefinition(); - - if (filterInvocationDefinitionMap.containsKey(key)) { + if (filterInvocationDefinitionMap.containsKey(matcher)) { logger.warn("Duplicate URL defined: " + path + ". The original attribute values will be overwritten"); } - filterInvocationDefinitionMap.put(key, attributeBuilder.getBeanDefinition()); + filterInvocationDefinitionMap.put(matcher, attributeBuilder.getBeanDefinition()); } return filterInvocationDefinitionMap; diff --git a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java index 4b506aafa5..461ff91b58 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java @@ -35,7 +35,6 @@ import org.springframework.security.web.access.channel.SecureChannelProcessor; import org.springframework.security.web.access.expression.WebExpressionVoter; import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; -import org.springframework.security.web.access.intercept.RequestKey; import org.springframework.security.web.authentication.SimpleUrlAuthenticationFailureHandler; import org.springframework.security.web.authentication.session.ConcurrentSessionControlStrategy; import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy; @@ -48,8 +47,6 @@ import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; import org.springframework.security.web.session.ConcurrentSessionFilter; import org.springframework.security.web.session.SessionManagementFilter; -import org.springframework.security.web.util.AntUrlPathMatcher; -import org.springframework.security.web.util.UrlMatcher; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; @@ -81,10 +78,9 @@ class HttpConfigurationBuilder { private final Element httpElt; private final ParserContext pc; - private final UrlMatcher matcher; - private final Boolean convertPathsToLowerCase; private final SessionCreationPolicy sessionPolicy; private final List interceptUrls; + private final MatcherType matcherType; // Use ManagedMap to allow placeholder resolution private ManagedMap> filterChainMap; @@ -102,15 +98,12 @@ class HttpConfigurationBuilder { private BeanReference fsi; private BeanReference requestCache; - public HttpConfigurationBuilder(Element element, ParserContext pc, UrlMatcher matcher, + public HttpConfigurationBuilder(Element element, ParserContext pc, MatcherType matcherType, String portMapperName, BeanReference authenticationManager) { this.httpElt = element; this.pc = pc; this.portMapperName = portMapperName; - this.matcher = matcher; - // SEC-501 - should paths stored in request maps be converted to lower case - // true if Ant path and using lower case - convertPathsToLowerCase = (matcher instanceof AntUrlPathMatcher) && matcher.requiresLowerCaseUrl(); + this.matcherType = matcherType; interceptUrls = DomUtils.getChildElementsByTagName(element, Elements.INTERCEPT_URL); String createSession = element.getAttribute(ATT_CREATE_SESSION); @@ -139,10 +132,7 @@ class HttpConfigurationBuilder { pc.getReaderContext().error("path attribute cannot be empty or null", urlElt); } - BeanDefinitionBuilder pathBean = BeanDefinitionBuilder.rootBeanDefinition(HttpConfigurationBuilder.class); - pathBean.setFactoryMethod("createPath"); - pathBean.addConstructorArgValue(path); - pathBean.addConstructorArgValue(convertPathsToLowerCase); + BeanDefinition matcher = matcherType.createMatcher(path, null); String filters = urlElt.getAttribute(ATT_FILTERS); @@ -153,7 +143,7 @@ class HttpConfigurationBuilder { } List noFilters = Collections.emptyList(); - filterChainMap.put(pathBean.getBeanDefinition(), noFilters); + filterChainMap.put(matcher, noFilters); } } } @@ -378,9 +368,8 @@ class HttpConfigurationBuilder { RootBeanDefinition channelFilter = new RootBeanDefinition(ChannelProcessingFilter.class); BeanDefinitionBuilder metadataSourceBldr = BeanDefinitionBuilder.rootBeanDefinition(DefaultFilterInvocationSecurityMetadataSource.class); - metadataSourceBldr.addConstructorArgValue(matcher); metadataSourceBldr.addConstructorArgValue(channelRequestMap); - metadataSourceBldr.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher); +// metadataSourceBldr.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher); channelFilter.getPropertyValues().addPropertyValue("securityMetadataSource", metadataSourceBldr.getBeanDefinition()); RootBeanDefinition channelDecisionManager = new RootBeanDefinition(ChannelDecisionManagerImpl.class); @@ -413,26 +402,22 @@ class HttpConfigurationBuilder { for (Element urlElt : interceptUrls) { String path = urlElt.getAttribute(ATT_PATH_PATTERN); + String method = urlElt.getAttribute(ATT_HTTP_METHOD); if(!StringUtils.hasText(path)) { - pc.getReaderContext().error("path attribute cannot be empty or null", urlElt); - } - - if (convertPathsToLowerCase) { - path = path.toLowerCase(); + pc.getReaderContext().error("pattern attribute cannot be empty or null", urlElt); } String requiredChannel = urlElt.getAttribute(ATT_REQUIRES_CHANNEL); if (StringUtils.hasText(requiredChannel)) { - BeanDefinition requestKey = new RootBeanDefinition(RequestKey.class); - requestKey.getConstructorArgumentValues().addGenericArgumentValue(path); + BeanDefinition matcher = matcherType.createMatcher(path, method); RootBeanDefinition channelAttributes = new RootBeanDefinition(ChannelAttributeFactory.class); channelAttributes.getConstructorArgumentValues().addGenericArgumentValue(requiredChannel); channelAttributes.setFactoryMethodName("createChannelAttributes"); - channelRequestMap.put(requestKey, channelAttributes); + channelRequestMap.put(matcher, channelAttributes); } } diff --git a/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java index 9df15b18d9..0ea95e8de5 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java @@ -26,9 +26,7 @@ import org.springframework.security.config.BeanIds; import org.springframework.security.config.Elements; import org.springframework.security.config.authentication.AuthenticationManagerFactoryBean; import org.springframework.security.web.FilterChainProxy; -import org.springframework.security.web.util.AntUrlPathMatcher; -import org.springframework.security.web.util.RegexUrlPathMatcher; -import org.springframework.security.web.util.UrlMatcher; +import org.springframework.security.web.util.AnyRequestMatcher; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; @@ -44,17 +42,13 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { private static final Log logger = LogFactory.getLog(HttpSecurityBeanDefinitionParser.class); static final String ATT_PATH_PATTERN = "pattern"; - static final String ATT_PATH_TYPE = "path-type"; - static final String OPT_PATH_TYPE_REGEX = "regex"; - private static final String DEF_PATH_TYPE_ANT = "ant"; + static final String ATT_HTTP_METHOD = "method"; static final String ATT_FILTERS = "filters"; static final String OPT_FILTERS_NONE = "none"; static final String ATT_REQUIRES_CHANNEL = "requires-channel"; - private static final String ATT_LOWERCASE_COMPARISONS = "lowercase-comparisons"; - private static final String ATT_REF = "ref"; static final String EXPRESSION_FIMDS_CLASS = "org.springframework.security.web.access.expression.ExpressionBasedFilterInvocationSecurityMetadataSource"; @@ -80,25 +74,25 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { final Object source = pc.extractSource(element); final String portMapperName = createPortMapper(element, pc); - final UrlMatcher matcher = createUrlMatcher(element); + + MatcherType matcherType = MatcherType.fromElement(element); ManagedList authenticationProviders = new ManagedList(); BeanReference authenticationManager = createAuthenticationManager(element, pc, authenticationProviders, null); - HttpConfigurationBuilder httpBldr = new HttpConfigurationBuilder(element, pc, matcher, + HttpConfigurationBuilder httpBldr = new HttpConfigurationBuilder(element, pc, matcherType, portMapperName, authenticationManager); AuthenticationConfigBuilder authBldr = new AuthenticationConfigBuilder(element, pc, httpBldr.getSessionCreationPolicy(), httpBldr.getRequestCache(), authenticationManager, httpBldr.getSessionStrategy()); + authenticationProviders.addAll(authBldr.getProviders()); + List unorderedFilterChain = new ArrayList(); unorderedFilterChain.addAll(httpBldr.getFilters()); unorderedFilterChain.addAll(authBldr.getFilters()); - - authenticationProviders.addAll(authBldr.getProviders()); - unorderedFilterChain.addAll(buildCustomFilterList(element, pc)); Collections.sort(unorderedFilterChain, new OrderComparator()); @@ -111,11 +105,10 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { } ManagedMap> filterChainMap = httpBldr.getFilterChainMap(); - BeanDefinition universalMatch = new RootBeanDefinition(String.class); - universalMatch.getConstructorArgumentValues().addGenericArgumentValue(matcher.getUniversalMatchPattern()); + BeanDefinition universalMatch = new RootBeanDefinition(AnyRequestMatcher.class); filterChainMap.put(universalMatch, filterChain); - registerFilterChainProxy(pc, filterChainMap, matcher, source); + registerFilterChainProxy(pc, filterChainMap, source); pc.popAndRegisterContainingComponent(); return null; @@ -222,57 +215,20 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { return customFilters; } - private void registerFilterChainProxy(ParserContext pc, Map> filterChainMap, UrlMatcher matcher, Object source) { + private void registerFilterChainProxy(ParserContext pc, Map> filterChainMap, Object source) { if (pc.getRegistry().containsBeanDefinition(BeanIds.FILTER_CHAIN_PROXY)) { pc.getReaderContext().error("Duplicate element detected", source); } BeanDefinitionBuilder fcpBldr = BeanDefinitionBuilder.rootBeanDefinition(FilterChainProxy.class); fcpBldr.getRawBeanDefinition().setSource(source); - fcpBldr.addPropertyValue("matcher", matcher); - fcpBldr.addPropertyValue("stripQueryStringFromUrls", Boolean.valueOf(matcher instanceof AntUrlPathMatcher)); +// fcpBldr.addPropertyValue("stripQueryStringFromUrls", Boolean.valueOf(matcher instanceof AntUrlPathMatcher)); fcpBldr.addPropertyValue("filterChainMap", filterChainMap); BeanDefinition fcpBean = fcpBldr.getBeanDefinition(); pc.registerBeanComponent(new BeanComponentDefinition(fcpBean, BeanIds.FILTER_CHAIN_PROXY)); pc.getRegistry().registerAlias(BeanIds.FILTER_CHAIN_PROXY, BeanIds.SPRING_SECURITY_FILTER_CHAIN); } - static UrlMatcher createUrlMatcher(Element element) { - String patternType = element.getAttribute(ATT_PATH_TYPE); - if (!StringUtils.hasText(patternType)) { - patternType = DEF_PATH_TYPE_ANT; - } - - boolean useRegex = patternType.equals(OPT_PATH_TYPE_REGEX); - - UrlMatcher matcher = new AntUrlPathMatcher(); - - if (useRegex) { - matcher = new RegexUrlPathMatcher(); - } - - // Deal with lowercase conversion requests - String lowercaseComparisons = element.getAttribute(ATT_LOWERCASE_COMPARISONS); - if (!StringUtils.hasText(lowercaseComparisons)) { - lowercaseComparisons = null; - } - - // Only change from the defaults if the attribute has been set - if ("true".equals(lowercaseComparisons)) { - if (useRegex) { - ((RegexUrlPathMatcher)matcher).setRequiresLowerCaseUrl(true); - } - // Default for ant is already to force lower case - } else if ("false".equals(lowercaseComparisons)) { - if (!useRegex) { - ((AntUrlPathMatcher)matcher).setRequiresLowerCaseUrl(false); - } - // Default for regex is no change - } - - return matcher; - } - } class OrderDecorator implements Ordered { diff --git a/config/src/main/java/org/springframework/security/config/http/MatcherType.java b/config/src/main/java/org/springframework/security/config/http/MatcherType.java new file mode 100644 index 0000000000..21880b252a --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/http/MatcherType.java @@ -0,0 +1,66 @@ +package org.springframework.security.config.http; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.security.web.util.AntPathRequestMatcher; +import org.springframework.security.web.util.AnyRequestMatcher; +import org.springframework.security.web.util.RegexRequestMatcher; +import org.springframework.security.web.util.RequestMatcher; +import org.springframework.util.StringUtils; +import org.w3c.dom.Element; + +/** + * Defines the {@link RequestMatcher} types supported by the namespace. + * + * @author Luke Taylor + * @since 3.1 + */ +public enum MatcherType { + ant (AntPathRequestMatcher.class), + regex (RegexRequestMatcher.class), + ciRegex (RegexRequestMatcher.class); + + private static final Log logger = LogFactory.getLog(HttpSecurityBeanDefinitionParser.class); + + private static final String ATT_MATCHER_TYPE = "request-matcher"; + private static final String ATT_PATH_TYPE = "path-type"; + + private final Class type; + + MatcherType(Class type) { + this.type = type; + } + + BeanDefinition createMatcher(String path, String method) { + if ("/**".equals(path)) { + return new RootBeanDefinition(AnyRequestMatcher.class); + } + + BeanDefinitionBuilder matcherBldr = BeanDefinitionBuilder.rootBeanDefinition(type); + + matcherBldr.addConstructorArgValue(path); + matcherBldr.addConstructorArgValue(method); + + if (this == ciRegex) { + matcherBldr.addConstructorArgValue(true); + } + + return matcherBldr.getBeanDefinition(); + } + + static MatcherType fromElement(Element elt) { + if (StringUtils.hasText(elt.getAttribute(ATT_MATCHER_TYPE))) { + return valueOf(elt.getAttribute(ATT_MATCHER_TYPE)); + } + + if (StringUtils.hasText(elt.getAttribute(ATT_PATH_TYPE))) { + logger.warn("'" + ATT_PATH_TYPE + "' is deprecated. Please use '" + ATT_MATCHER_TYPE +"' instead."); + return valueOf(elt.getAttribute(ATT_PATH_TYPE)); + } + + return ant; + } +} diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-3.1.rnc b/config/src/main/resources/org/springframework/security/config/spring-security-3.1.rnc index 9f1923cf07..a4fc94d80a 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-3.1.rnc +++ b/config/src/main/resources/org/springframework/security/config/spring-security-3.1.rnc @@ -11,6 +11,9 @@ hash = base64 = ## Whether a string should be base64 encoded attribute base64 {"true" | "false"} +request-matcher = + ## Supersedes the 'path-type' attribute. Defines the strategy use for matching incoming requests. Currently the options are 'ant' (for ant path patterns), 'regex' for regular expressions and 'iciRegex' for case-insensitive regular expressions. + attribute request-matcher {"ant" | "regex" | "ciRegex"} path-type = ## Defines the type of pattern used to specify URL paths (either JDK 1.4-compatible regular expressions, or Apache Ant expressions). Defaults to "ant" if unspecified. attribute path-type {"ant" | "regex"} @@ -264,11 +267,10 @@ http.attlist &= ## A reference to a SecurityContextRepository bean. This can be used to customize how the SecurityContext is stored between requests. attribute security-context-repository-ref {xsd:token}? http.attlist &= - ## The path format used to define the paths in child elements. - path-type? + request-matcher? http.attlist &= - ## Whether test URLs should be converted to lower case prior to comparing with defined path patterns. If unspecified, defaults to "true". - attribute lowercase-comparisons {boolean}? + ## Deprecated. Use request-matcher instead. + path-type? http.attlist &= ## Provides versions of HttpServletRequest security methods such as isUserInRole() and getPrincipal() which are implemented by accessing the Spring SecurityContext. Defaults to "true". attribute servlet-api-provision {boolean}? @@ -392,7 +394,10 @@ filter-chain-map = ## Used to explicitly configure a FilterChainProxy instance with a FilterChainMap element filter-chain-map {filter-chain-map.attlist, filter-chain+} filter-chain-map.attlist &= - path-type + ## Deprecated. Use request-matcher instead. + path-type? +filter-chain-map.attlist &= + request-matcher? filter-chain = ## Used within filter-chain-map to define a specific URL pattern and the list of filters which apply to the URLs matching that pattern. When multiple filter-chain elements are used within a filter-chain-map element, the most specific patterns must be placed at the top of the list, with most general ones at the bottom. @@ -413,8 +418,10 @@ fsmds.attlist &= ## as for http element attribute lowercase-comparisons {boolean}? fsmds.attlist &= - ## as for http element + ## Deprecate. Use request-matcher instead. path-type? +fsmds.attlist &= + request-matcher? filter-invocation-definition-source = ## Deprecated synonym for filter-security-metadata-source diff --git a/config/src/main/resources/org/springframework/security/config/spring-security-3.1.xsd b/config/src/main/resources/org/springframework/security/config/spring-security-3.1.xsd index de2d6e750d..22ca6b1564 100644 --- a/config/src/main/resources/org/springframework/security/config/spring-security-3.1.xsd +++ b/config/src/main/resources/org/springframework/security/config/spring-security-3.1.xsd @@ -31,6 +31,20 @@ + + + + Supersedes the 'path-type' attribute. Defines the strategy use for matching incoming requests. Currently the options are 'ant' (for ant path patterns), 'regex' for regular expressions and 'iciRegex' for case-insensitive regular expressions. + + + + + + + + + + @@ -692,6 +706,18 @@ A reference to a SecurityContextRepository bean. This can be used to customize how the SecurityContext is stored between requests. + + + Superseded the 'path-type' attribute. Defines the strategy use for matching incoming requests. Currently the options are 'ant' (for ant path patterns), 'regex' for regular expressions and 'iciRegex' for case-insensitive regular expressions. + + + + + + + + + Defines the type of pattern used to specify URL paths (either JDK 1.4-compatible regular expressions, or Apache Ant expressions). Defaults to "ant" if unspecified. @@ -703,11 +729,6 @@ - - - Whether test URLs should be converted to lower case prior to comparing with defined path patterns. If unspecified, defaults to "true". - - Provides versions of HttpServletRequest security methods such as isUserInRole() and getPrincipal() which are implemented by accessing the Spring SecurityContext. Defaults to "true". @@ -902,7 +923,29 @@ - + + + Defines the type of pattern used to specify URL paths (either JDK 1.4-compatible regular expressions, or Apache Ant expressions). Defaults to "ant" if unspecified. + + + + + + + + + + + Superseded the 'path-type' attribute. Defines the strategy use for matching incoming requests. Currently the options are 'ant' (for ant path patterns), 'regex' for regular expressions and 'iciRegex' for case-insensitive regular expressions. + + + + + + + + + @@ -948,6 +991,18 @@ + + + Superseded the 'path-type' attribute. Defines the strategy use for matching incoming requests. Currently the options are 'ant' (for ant path patterns), 'regex' for regular expressions and 'iciRegex' for case-insensitive regular expressions. + + + + + + + + + Deprecated synonym for filter-security-metadata-source diff --git a/config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java b/config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java index 6e1e73bb28..8e96d386b1 100644 --- a/config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java @@ -37,6 +37,9 @@ import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.context.SecurityContextPersistenceFilter; import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter; +import org.springframework.security.web.util.AntPathRequestMatcher; +import org.springframework.security.web.util.AnyRequestMatcher; +import org.springframework.security.web.util.RequestMatcher; /** * Tests {@link FilterChainProxy}. @@ -101,27 +104,15 @@ public class FilterChainProxyConfigTests { assertEquals(null, filterChainProxy.getFilters("/nomatch")); } - @Test - public void urlStrippingPropertyIsRespected() throws Exception { - FilterChainProxy filterChainProxy = (FilterChainProxy) appCtx.getBean("newFilterChainProxyNoDefaultPath", FilterChainProxy.class); - - // Should only match if we are stripping the query string - String url = "/blah.bar?x=something"; - assertNotNull(filterChainProxy.getFilters(url)); - assertEquals(2, filterChainProxy.getFilters(url).size()); - filterChainProxy.setStripQueryStringFromUrls(false); - assertNull(filterChainProxy.getFilters(url)); - } - // SEC-1235 @Test public void mixingPatternsAndPlaceholdersDoesntCauseOrderingIssues() throws Exception { FilterChainProxy filterChainProxy = (FilterChainProxy) appCtx.getBean("sec1235FilterChainProxy", FilterChainProxy.class); - String[] paths = filterChainProxy.getFilterChainMap().keySet().toArray(new String[0]); - assertEquals("/login*", paths[0]); - assertEquals("/logout", paths[1]); - assertEquals("/**", paths[2]); + RequestMatcher[] matchers = filterChainProxy.getFilterChainMap().keySet().toArray(new RequestMatcher[0]); + assertEquals("/login*", ((AntPathRequestMatcher)matchers[0]).getPattern()); + assertEquals("/logout", ((AntPathRequestMatcher)matchers[1]).getPattern()); + assertTrue(matchers[2] instanceof AnyRequestMatcher); } private void checkPathAndFilterOrder(FilterChainProxy filterChainProxy) throws Exception { diff --git a/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java index fa526f8dd9..641d04a907 100644 --- a/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParserTests.java @@ -125,9 +125,6 @@ public class HttpSecurityBeanDefinitionParserTests { List filterList = getFilters("/anyurl"); checkAutoConfigFilters(filterList); - - assertEquals(true, FieldUtils.getFieldValue(appContext.getBean(BeanIds.FILTER_CHAIN_PROXY), "stripQueryStringFromUrls")); - assertEquals(true, FieldUtils.getFieldValue(filterList.get(AUTO_CONFIG_FILTERS-1), "securityMetadataSource.stripQueryStringFromUrls")); } @Test(expected=BeanDefinitionParsingException.class) @@ -136,8 +133,6 @@ public class HttpSecurityBeanDefinitionParserTests { } private void checkAutoConfigFilters(List filterList) throws Exception { -// assertEquals("Expected " + AUTO_CONFIG_FILTERS + " filters in chain", AUTO_CONFIG_FILTERS, filterList.size()); - Iterator filters = filterList.iterator(); assertTrue(filters.next() instanceof SecurityContextPersistenceFilter); @@ -184,37 +179,34 @@ public class HttpSecurityBeanDefinitionParserTests { assertTrue(filters.size() == 0); } - @Test public void regexPathsWorkCorrectly() throws Exception { setContext( - " " + + " " + " " + " " + AUTH_PROVIDER_XML); assertEquals(0, getFilters("/imlowercase").size()); - // This will be matched by the default pattern ".*" - List allFilters = getFilters("/ImCaughtByTheUniversalMatchPattern"); + List allFilters = getFilters("/ImCaughtByTheAnyRequestMatcher"); checkAutoConfigFilters(allFilters); - assertEquals(false, FieldUtils.getFieldValue(appContext.getBean(BeanIds.FILTER_CHAIN_PROXY), "stripQueryStringFromUrls")); - assertEquals(false, FieldUtils.getFieldValue(allFilters.get(AUTO_CONFIG_FILTERS-1), "securityMetadataSource.stripQueryStringFromUrls")); } @Test - public void lowerCaseComparisonAttributeIsRespectedByFilterChainProxy() throws Exception { + public void ciRegexPathsWorkCorrectly() throws Exception { setContext( - " " + - " " + + " " + + " " + " " + AUTH_PROVIDER_XML); - assertEquals(0, getFilters("/Secure").size()); - // These will be matched by the default pattern "/**" - checkAutoConfigFilters(getFilters("/secure")); - checkAutoConfigFilters(getFilters("/ImCaughtByTheUniversalMatchPattern")); + assertEquals(0, getFilters("/imMixedCase").size()); + // This will be matched by the default pattern ".*" + List allFilters = getFilters("/Im_Caught_By_The_AnyRequestMatcher"); + assertTrue(allFilters.size() > 0); + checkAutoConfigFilters(allFilters); } @Test public void formLoginWithNoLoginPageAddsDefaultLoginPageFilter() throws Exception { setContext( - "" + + "" + " " + "" + AUTH_PROVIDER_XML); // These will be matched by the default pattern "/**" @@ -315,26 +307,6 @@ public class HttpSecurityBeanDefinitionParserTests { assertSame(appContext.getBean("logoutHandler"), handler); } - @Test - public void lowerCaseComparisonIsRespectedBySecurityFilterInvocationDefinitionSource() throws Exception { - setContext( - " " + - " " + - " " + - " " + AUTH_PROVIDER_XML); - - FilterSecurityInterceptor fis = getFilter(FilterSecurityInterceptor.class); - - FilterInvocationSecurityMetadataSource fids = fis.getSecurityMetadataSource(); - Collection attrDef = fids.getAttributes(createFilterinvocation("/Secure", null)); - assertEquals(2, attrDef.size()); - assertTrue(attrDef.contains(new SecurityConfig("ROLE_A"))); - assertTrue(attrDef.contains(new SecurityConfig("ROLE_B"))); - attrDef = fids.getAttributes(createFilterinvocation("/secure", null)); - assertEquals(1, attrDef.size()); - assertTrue(attrDef.contains(new SecurityConfig("ROLE_C"))); - } - // SEC-1201 @Test public void interceptUrlsAndFormLoginSupportPropertyPlaceholders() throws Exception { @@ -395,9 +367,9 @@ public class HttpSecurityBeanDefinitionParserTests { public void httpMethodMatchIsSupported() throws Exception { setContext( " " + - " " + " " + " " + + " " + " " + AUTH_PROVIDER_XML); FilterSecurityInterceptor fis = getFilter(FilterSecurityInterceptor.class); @@ -692,7 +664,6 @@ public class HttpSecurityBeanDefinitionParserTests { "" + " " + AUTH_PROVIDER_XML); -// AbstractRememberMeServices rememberMeServices = (AbstractRememberMeServices) appContext.getBean(BeanIds.REMEMBER_ME_SERVICES); } @Test @@ -746,7 +717,6 @@ public class HttpSecurityBeanDefinitionParserTests { "" + "" + AUTH_PROVIDER_XML); - //session-authentication-strategy-ref } @Test @@ -768,12 +738,10 @@ public class HttpSecurityBeanDefinitionParserTests { getFilter(ConcurrentSessionFilter.class), "sessionRegistry"); Object sessionRegistryFromFormLoginFilter = FieldUtils.getFieldValue( getFilter(UsernamePasswordAuthenticationFilter.class),"sessionStrategy.sessionRegistry"); -// Object sessionRegistryFromController = FieldUtils.getFieldValue(getConcurrentSessionController(),"sessionRegistry"); Object sessionRegistryFromMgmtFilter = FieldUtils.getFieldValue( getFilter(SessionManagementFilter.class),"sessionStrategy.sessionRegistry"); assertSame(sessionRegistry, sessionRegistryFromConcurrencyFilter); -// assertSame(sessionRegistry, sessionRegistryFromController); assertSame(sessionRegistry, sessionRegistryFromMgmtFilter); // SEC-1143 assertSame(sessionRegistry, sessionRegistryFromFormLoginFilter); @@ -791,8 +759,6 @@ public class HttpSecurityBeanDefinitionParserTests { UsernamePasswordAuthenticationToken auth = new UsernamePasswordAuthenticationToken("bob", "pass"); SecurityContextHolder.getContext().setAuthentication(auth); // Register 2 sessions and then check a third -// req.setSession(new MockHttpSession()); -// auth.setDetails(new WebAuthenticationDetails(req)); MockHttpServletResponse mockResponse = new MockHttpServletResponse(); SaveContextOnUpdateOrErrorResponseWrapper response = new SaveContextOnUpdateOrErrorResponseWrapper(mockResponse, false) { protected void saveContext(SecurityContext context) { @@ -1240,7 +1206,6 @@ public class HttpSecurityBeanDefinitionParserTests { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod(method); request.setRequestURI(null); - request.setServletPath(path); return new FilterInvocation(request, new MockHttpServletResponse(), new MockFilterChain()); diff --git a/config/src/test/resources/org/springframework/security/util/filtertest-valid.xml b/config/src/test/resources/org/springframework/security/util/filtertest-valid.xml index 5d887602d6..899174b22f 100644 --- a/config/src/test/resources/org/springframework/security/util/filtertest-valid.xml +++ b/config/src/test/resources/org/springframework/security/util/filtertest-valid.xml @@ -105,27 +105,44 @@ http://www.springframework.org/schema/security http://www.springframework.org/sc - - - - + + + + + + - + + + + + + - + + + + + + - + + + + + + diff --git a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java index 776fa183b7..7ec9989b67 100644 --- a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java +++ b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java @@ -26,16 +26,15 @@ import java.util.Set; import javax.servlet.Filter; import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource; -import org.springframework.security.web.util.AntUrlPathMatcher; -import org.springframework.security.web.util.UrlMatcher; +import org.springframework.security.web.util.AnyRequestMatcher; +import org.springframework.security.web.util.RequestMatcher; import org.springframework.util.Assert; import org.springframework.web.filter.DelegatingFilterProxy; import org.springframework.web.filter.GenericFilterBean; @@ -45,26 +44,22 @@ import org.springframework.web.filter.GenericFilterBean; * Delegates Filter requests to a list of Spring-managed beans. * As of version 2.0, you shouldn't need to explicitly configure a FilterChainProxy bean in your application * context unless you need very fine control over the filter chain contents. Most cases should be adequately covered - * by the default <security:http /> namespace configuration options. - * - *

The FilterChainProxy is loaded via a standard Spring {@link DelegatingFilterProxy} declaration in - * web.xml. FilterChainProxy will then pass {@link #init(FilterConfig)}, {@link #destroy()} - * and {@link #doFilter(ServletRequest, ServletResponse, FilterChain)} invocations through to each Filter - * defined against FilterChainProxy. - * - *

As of version 2.0, FilterChainProxy is configured using an ordered Map of path patterns to Lists - * of Filter objects. In previous - * versions, a {@link FilterInvocationSecurityMetadataSource} was used. This is now deprecated in favour of namespace-based - * configuration which provides a more robust and simplfied syntax. The Map instance will normally be - * created while parsing the namespace configuration, so doesn't have to be set explicitly. - * Instead the <filter-chain-map> element should be used within the FilterChainProxy bean declaration. + * by the default <security:http /> namespace configuration options. + *

+ * The FilterChainProxy is loaded via a standard Spring {@link DelegatingFilterProxy} declaration in + * web.xml. + *

+ * As of version 3.1, FilterChainProxy is configured using an ordered Map of {@link RequestMatcher} instances + * to Lists of Filters. The Map instance will normally be created while parsing the namespace + * configuration, so doesn't have to be set explicitly. Instead the <filter-chain-map> element should be used + * within the FilterChainProxy bean declaration. * This in turn should have a list of child <filter-chain> elements which each define a URI pattern and the list * of filters (as comma-separated bean names) which should be applied to requests which match the pattern. * An example configuration might look like this: * *

  <bean id="myfilterChainProxy" class="org.springframework.security.util.FilterChainProxy">
-     <security:filter-chain-map pathType="ant">
+     <security:filter-chain-map request-matcher="ant">
          <security:filter-chain pattern="/do/not/filter" filters="none"/>
          <security:filter-chain pattern="/**" filters="filter1,filter2,filter3"/>
      </security:filter-chain-map>
@@ -73,30 +68,24 @@ import org.springframework.web.filter.GenericFilterBean;
  *
  * The names "filter1", "filter2", "filter3" should be the bean names of Filter instances defined in the
  * application context. The order of the names defines the order in which the filters will be applied. As shown above,
- * use of the value "none" for the "filters" can be used to exclude
- * Please consult the security namespace schema file for a full list of available configuration options.
- *
+ * use of the value "none" for the "filters" can be used to exclude a request pattern from the security filter chain
+ * entirely. Please consult the security namespace schema file for a full list of available configuration options.
  * 

- * Each possible URI pattern that FilterChainProxy should service must be entered. - * The first matching URI pattern for a given request will be used to define all of the - * Filters that apply to that request. NB: This means you must put most specific URI patterns at the top - * of the list, and ensure all Filters that should apply for a given URI pattern are entered against the - * respective entry. The FilterChainProxy will not iterate the remainder of the URI patterns to locate - * additional Filters. - * - *

FilterChainProxy respects normal handling of Filters that elect not to call {@link + * Each possible pattern that FilterChainProxy should service must be entered. + * The first match for a given request will be used to define all of the Filters that apply to that + * request. This means you must put most specific matches at the top of the list, and ensure all Filters + * that should apply for a given matcher are entered against the respective entry. + * The FilterChainProxy will not iterate through the remainder of the map entries to locate additional + * Filters. + *

+ * FilterChainProxy respects normal handling of Filters that elect not to call {@link * javax.servlet.Filter#doFilter(javax.servlet.ServletRequest, javax.servlet.ServletResponse, * javax.servlet.FilterChain)}, in that the remainder of the original or FilterChainProxy-declared filter * chain will not be called. - * - *

Note the Filter lifecycle mismatch between the servlet container and IoC + *

+ * Note the Filter lifecycle mismatch between the servlet container and IoC * container. As described in the {@link DelegatingFilterProxy} JavaDocs, we recommend you allow the IoC - * container to manage the lifecycle instead of the servlet container. By default the DelegatingFilterProxy - * will never call this class' {@link #init(FilterConfig)} and {@link #destroy()} methods, which in turns means that - * the corresponding methods on the filter beans managed by this class will never be called. If you do need your filters to be - * initialized and destroyed, please set the targetFilterLifecycle initialization parameter against the - * DelegatingFilterProxy to specify that servlet container lifecycle management should be used. You don't - * need to worry about this in most cases. + * container to manage the lifecycle instead of the servlet container. * * @author Carlos Sanchez * @author Ben Alex @@ -111,20 +100,15 @@ public class FilterChainProxy extends GenericFilterBean { //~ Instance fields ================================================================================================ -// private ApplicationContext applicationContext; - /** Map of the original pattern Strings to filter chains */ - private Map> uncompiledFilterChainMap; - /** Compiled pattern version of the filter chain map */ - private Map> filterChainMap; - private UrlMatcher matcher = new AntUrlPathMatcher(); - private boolean stripQueryStringFromUrls = true; + private Map> filterChainMap; + private FilterChainValidator filterChainValidator = new NullFilterChainValidator(); //~ Methods ======================================================================================================== @Override public void afterPropertiesSet() { - Assert.notNull(uncompiledFilterChainMap, "filterChainMap must be set"); + Assert.notNull(filterChainMap, "filterChainMap must be set"); filterChainValidator.validate(this); } @@ -132,7 +116,7 @@ public class FilterChainProxy extends GenericFilterBean { throws IOException, ServletException { FilterInvocation fi = new FilterInvocation(request, response, chain); - List filters = getFilters(fi.getRequestUrl()); + List filters = getFilters(fi.getRequest()); if (filters == null || filters.size() == 0) { if (logger.isDebugEnabled()) { @@ -149,40 +133,18 @@ public class FilterChainProxy extends GenericFilterBean { virtualFilterChain.doFilter(fi.getRequest(), fi.getResponse()); } + /** * Returns the first filter chain matching the supplied URL. * - * @param url the request URL + * @param request the request to match * @return an ordered array of Filters defining the filter chain */ - public List getFilters(String url) { - if (stripQueryStringFromUrls) { - // String query string - see SEC-953 - int firstQuestionMarkIndex = url.indexOf("?"); + private List getFilters(HttpServletRequest request) { + for (Map.Entry> entry : filterChainMap.entrySet()) { + RequestMatcher matcher = entry.getKey(); - if (firstQuestionMarkIndex != -1) { - url = url.substring(0, firstQuestionMarkIndex); - } - } - - for (Map.Entry> entry : filterChainMap.entrySet()) { - Object path = entry.getKey(); - - if (matcher.requiresLowerCaseUrl()) { - url = url.toLowerCase(); - - if (logger.isDebugEnabled()) { - logger.debug("Converted URL to lowercase, from: '" + url + "'; to: '" + url + "'"); - } - } - - boolean matched = matcher.pathMatchesUrl(path, url); - - if (logger.isDebugEnabled()) { - logger.debug("Candidate is: '" + url + "'; pattern is " + path + "; matched=" + matched); - } - - if (matched) { + if (matcher.matches(request)) { return entry.getValue(); } } @@ -190,6 +152,16 @@ public class FilterChainProxy extends GenericFilterBean { return null; } + /** + * Convenience method, mainly for testing. + * + * @param url the URL + * @return matching filter list + */ + public List getFilters(String url) { + return getFilters(new FilterInvocation(url, null).getRequest()); + } + /** * Obtains all of the uniqueFilter instances registered in the map of * filter chains. @@ -225,15 +197,14 @@ public class FilterChainProxy extends GenericFilterBean { @SuppressWarnings("unchecked") public void setFilterChainMap(Map filterChainMap) { checkContents(filterChainMap); - uncompiledFilterChainMap = new LinkedHashMap>(filterChainMap); + this.filterChainMap = new LinkedHashMap>(filterChainMap); checkPathOrder(); - createCompiledMap(); } @SuppressWarnings("unchecked") private void checkContents(Map filterChainMap) { for (Object key : filterChainMap.keySet()) { - Assert.isInstanceOf(String.class, key, "Path key must be a String but found " + key); + Assert.isInstanceOf(RequestMatcher.class, key, "Path key must be a RequestMatcher but found " + key); Object filters = filterChainMap.get(key); Assert.isInstanceOf(List.class, filters, "Value must be a filter list"); // Check the contents @@ -248,50 +219,25 @@ public class FilterChainProxy extends GenericFilterBean { private void checkPathOrder() { // Check that the universal pattern is listed at the end, if at all - String[] paths = (String[]) uncompiledFilterChainMap.keySet().toArray(new String[0]); - String universalMatch = matcher.getUniversalMatchPattern(); + RequestMatcher[] matchers = filterChainMap.keySet().toArray(new RequestMatcher[0]); - for (int i=0; i < paths.length-1; i++) { - if (paths[i].equals(universalMatch)) { - throw new IllegalArgumentException("A universal match pattern " + universalMatch + " is defined " + + for (int i=0; i < matchers.length-1; i++) { + if (matchers[i] instanceof AnyRequestMatcher) { + throw new IllegalArgumentException("A universal match pattern ('/**') is defined " + " before other patterns in the filter chain, causing them to be ignored. Please check the " + "ordering in your namespace or FilterChainProxy bean configuration"); } } } - private void createCompiledMap() { - filterChainMap = new LinkedHashMap>(uncompiledFilterChainMap.size()); - - for (String path : uncompiledFilterChainMap.keySet()) { - filterChainMap.put(matcher.compile(path), uncompiledFilterChainMap.get(path)); - } - } - /** * Returns a copy of the underlying filter chain map. Modifications to the map contents * will not affect the FilterChainProxy state - to change the map call setFilterChainMap. * * @return the map of path pattern Strings to filter chain lists (with ordering guaranteed). */ - public Map> getFilterChainMap() { - return new LinkedHashMap>(uncompiledFilterChainMap); - } - - public void setMatcher(UrlMatcher matcher) { - this.matcher = matcher; - } - - public UrlMatcher getMatcher() { - return matcher; - } - - /** - * If set to 'true', the query string will be stripped from the request URL before - * attempting to find a matching filter chain. This is the default value. - */ - public void setStripQueryStringFromUrls(boolean stripQueryStringFromUrls) { - this.stripQueryStringFromUrls = stripQueryStringFromUrls; + public Map> getFilterChainMap() { + return new LinkedHashMap>(filterChainMap); } /** @@ -306,9 +252,8 @@ public class FilterChainProxy extends GenericFilterBean { public String toString() { StringBuilder sb = new StringBuilder(); sb.append("FilterChainProxy["); - sb.append(" UrlMatcher = ").append(matcher); - sb.append("; Filter Chains: "); - sb.append(uncompiledFilterChainMap); + sb.append("Filter Chains: "); + sb.append(filterChainMap); sb.append("]"); return sb.toString(); diff --git a/web/src/main/java/org/springframework/security/web/FilterInvocation.java b/web/src/main/java/org/springframework/security/web/FilterInvocation.java index 49cb8ede37..8bda36a9d5 100644 --- a/web/src/main/java/org/springframework/security/web/FilterInvocation.java +++ b/web/src/main/java/org/springframework/security/web/FilterInvocation.java @@ -15,13 +15,28 @@ package org.springframework.security.web; -import org.springframework.security.web.util.UrlUtils; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.UnsupportedEncodingException; +import java.security.Principal; +import java.util.Enumeration; +import java.util.Locale; +import java.util.Map; import javax.servlet.FilterChain; +import javax.servlet.RequestDispatcher; +import javax.servlet.ServletException; +import javax.servlet.ServletInputStream; +import javax.servlet.ServletOutputStream; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; +import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + +import org.springframework.security.web.util.UrlUtils; /** @@ -36,6 +51,15 @@ import javax.servlet.http.HttpServletResponse; * @author colin sampaleanu */ public class FilterInvocation { + //~ Static fields ================================================================================================== + static final FilterChain DUMMY_CHAIN = new FilterChain() { + public void doFilter(ServletRequest req, ServletResponse res) throws IOException, ServletException { + throw new UnsupportedOperationException("Dummy filter chain"); + } + }; + + static final HttpServletResponse DUMMY_RESPONSE = new DummyResponse(); + //~ Instance fields ================================================================================================ private FilterChain chain; @@ -54,6 +78,26 @@ public class FilterInvocation { this.chain = chain; } + public FilterInvocation(String servletPath, String method) { + this(null, servletPath, method); + } + + public FilterInvocation(String contextPath, String servletPath, String method) { + this(contextPath, servletPath, null, null, method); + } + + public FilterInvocation(String contextPath, String servletPath, String pathInfo, String query, String method) { + DummyRequest request = new DummyRequest(); + if (contextPath == null) { + contextPath = "/cp"; + } + request.setContextPath(contextPath); + request.setServletPath(servletPath); + request.setPathInfo(pathInfo); + request.setMethod(method); + this.request = request; + } + //~ Methods ======================================================================================================== public FilterChain getChain() { @@ -101,3 +145,386 @@ public class FilterInvocation { return "FilterInvocation: URL: " + getRequestUrl(); } } + +@SuppressWarnings("unchecked") +class DummyRequest implements HttpServletRequest { + private String requestURI; + private String contextPath = ""; + private String servletPath; + private String pathInfo; + private String queryString; + private String method; + + public void setRequestURI(String requestURI) { + this.requestURI = requestURI; + } + + public void setPathInfo(String pathInfo) { + this.pathInfo = pathInfo; + } + + public String getRequestURI() { + return requestURI; + } + + public void setContextPath(String contextPath) { + this.contextPath = contextPath; + } + + public String getContextPath() { + return contextPath; + } + + public void setServletPath(String servletPath) { + this.servletPath = servletPath; + } + + public String getServletPath() { + return servletPath; + } + + public void setMethod(String method) { + this.method = method; + } + + public String getMethod() { + return method; + } + + public String getPathInfo() { + return pathInfo; + } + + public String getQueryString() { + return queryString; + } + + public void setQueryString(String queryString) { + this.queryString = queryString; + } + + + public String getAuthType() { + throw new UnsupportedOperationException(); + } + + public Cookie[] getCookies() { + throw new UnsupportedOperationException(); + } + + public long getDateHeader(String name) { + throw new UnsupportedOperationException(); + } + + public String getHeader(String name) { + throw new UnsupportedOperationException(); + } + + public Enumeration getHeaderNames() { + throw new UnsupportedOperationException(); + } + + public Enumeration getHeaders(String name) { + throw new UnsupportedOperationException(); + } + + public int getIntHeader(String name) { + throw new UnsupportedOperationException(); + } + + public String getPathTranslated() { + throw new UnsupportedOperationException(); + } + + public String getRemoteUser() { + throw new UnsupportedOperationException(); + } + + public StringBuffer getRequestURL() { + throw new UnsupportedOperationException(); + } + + public String getRequestedSessionId() { + throw new UnsupportedOperationException(); + } + + public HttpSession getSession() { + throw new UnsupportedOperationException(); + } + + public HttpSession getSession(boolean create) { + throw new UnsupportedOperationException(); + } + + public Principal getUserPrincipal() { + throw new UnsupportedOperationException(); + } + + public boolean isRequestedSessionIdFromCookie() { + throw new UnsupportedOperationException(); + } + + public boolean isRequestedSessionIdFromURL() { + throw new UnsupportedOperationException(); + } + + public boolean isRequestedSessionIdFromUrl() { + throw new UnsupportedOperationException(); + } + + public boolean isRequestedSessionIdValid() { + throw new UnsupportedOperationException(); + } + + public boolean isUserInRole(String role) { + throw new UnsupportedOperationException(); + } + + public Object getAttribute(String name) { + throw new UnsupportedOperationException(); + } + + public Enumeration getAttributeNames() { + throw new UnsupportedOperationException(); + } + + public String getCharacterEncoding() { + throw new UnsupportedOperationException(); + } + + public int getContentLength() { + throw new UnsupportedOperationException(); + } + + public String getContentType() { + throw new UnsupportedOperationException(); + } + + public ServletInputStream getInputStream() throws IOException { + throw new UnsupportedOperationException(); + } + + public String getLocalAddr() { + throw new UnsupportedOperationException(); + + } + + public String getLocalName() { + throw new UnsupportedOperationException(); + } + + public int getLocalPort() { + throw new UnsupportedOperationException(); + } + + public Locale getLocale() { + throw new UnsupportedOperationException(); + } + + public Enumeration getLocales() { + throw new UnsupportedOperationException(); + } + + public String getParameter(String name) { + throw new UnsupportedOperationException(); + } + + public Map getParameterMap() { + throw new UnsupportedOperationException(); + } + + public Enumeration getParameterNames() { + throw new UnsupportedOperationException(); + } + + public String[] getParameterValues(String name) { + throw new UnsupportedOperationException(); + } + + public String getProtocol() { + throw new UnsupportedOperationException(); + } + + public BufferedReader getReader() throws IOException { + throw new UnsupportedOperationException(); + } + + public String getRealPath(String path) { + throw new UnsupportedOperationException(); + } + + public String getRemoteAddr() { + throw new UnsupportedOperationException(); + } + + public String getRemoteHost() { + throw new UnsupportedOperationException(); + } + + public int getRemotePort() { + throw new UnsupportedOperationException(); + } + + public RequestDispatcher getRequestDispatcher(String path) { + throw new UnsupportedOperationException(); + } + + public String getScheme() { + throw new UnsupportedOperationException(); + } + + public String getServerName() { + throw new UnsupportedOperationException(); + } + + public int getServerPort() { + throw new UnsupportedOperationException(); + } + + public boolean isSecure() { + throw new UnsupportedOperationException(); + } + + public void removeAttribute(String name) { + throw new UnsupportedOperationException(); + } + + public void setAttribute(String name, Object o) { + throw new UnsupportedOperationException(); + } + + public void setCharacterEncoding(String env) throws UnsupportedEncodingException { + throw new UnsupportedOperationException(); + } +} + +class DummyResponse implements HttpServletResponse { + public void addCookie(Cookie cookie) { + throw new UnsupportedOperationException(); + } + + public void addDateHeader(String name, long date) { + throw new UnsupportedOperationException(); + } + + public void addHeader(String name, String value) { + throw new UnsupportedOperationException(); + } + + public void addIntHeader(String name, int value) { + throw new UnsupportedOperationException(); + } + + public boolean containsHeader(String name) { + throw new UnsupportedOperationException(); + } + + public String encodeRedirectURL(String url) { + throw new UnsupportedOperationException(); + } + + public String encodeRedirectUrl(String url) { + throw new UnsupportedOperationException(); + } + + public String encodeURL(String url) { + throw new UnsupportedOperationException(); + } + + public String encodeUrl(String url) { + throw new UnsupportedOperationException(); + } + + public void sendError(int sc) throws IOException { + throw new UnsupportedOperationException(); + + } + + public void sendError(int sc, String msg) throws IOException { + throw new UnsupportedOperationException(); + } + + public void sendRedirect(String location) throws IOException { + throw new UnsupportedOperationException(); + } + + public void setDateHeader(String name, long date) { + throw new UnsupportedOperationException(); + } + + public void setHeader(String name, String value) { + throw new UnsupportedOperationException(); + } + + public void setIntHeader(String name, int value) { + throw new UnsupportedOperationException(); + } + + public void setStatus(int sc) { + throw new UnsupportedOperationException(); + } + + public void setStatus(int sc, String sm) { + throw new UnsupportedOperationException(); + } + + public void flushBuffer() throws IOException { + throw new UnsupportedOperationException(); + } + + public int getBufferSize() { + throw new UnsupportedOperationException(); + } + + public String getCharacterEncoding() { + throw new UnsupportedOperationException(); + } + + public String getContentType() { + throw new UnsupportedOperationException(); + } + + public Locale getLocale() { + throw new UnsupportedOperationException(); + } + + public ServletOutputStream getOutputStream() throws IOException { + throw new UnsupportedOperationException(); + } + + public PrintWriter getWriter() throws IOException { + throw new UnsupportedOperationException(); + } + + public boolean isCommitted() { + throw new UnsupportedOperationException(); + } + + public void reset() { + throw new UnsupportedOperationException(); + } + + public void resetBuffer() { + throw new UnsupportedOperationException(); + } + + public void setBufferSize(int size) { + throw new UnsupportedOperationException(); + } + + public void setCharacterEncoding(String charset) { + throw new UnsupportedOperationException(); + } + + public void setContentLength(int len) { + throw new UnsupportedOperationException(); + } + + public void setContentType(String type) { + throw new UnsupportedOperationException(); + } + + public void setLocale(Locale loc) { + throw new UnsupportedOperationException(); + } +} diff --git a/web/src/main/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluator.java b/web/src/main/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluator.java index d60d3bdcb4..863fd802c5 100644 --- a/web/src/main/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluator.java +++ b/web/src/main/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluator.java @@ -15,27 +15,7 @@ package org.springframework.security.web.access; -import java.io.BufferedReader; -import java.io.IOException; -import java.io.PrintWriter; -import java.io.UnsupportedEncodingException; -import java.security.Principal; import java.util.Collection; -import java.util.Enumeration; -import java.util.Locale; -import java.util.Map; - -import javax.servlet.FilterChain; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletException; -import javax.servlet.ServletInputStream; -import javax.servlet.ServletOutputStream; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -59,14 +39,6 @@ public class DefaultWebInvocationPrivilegeEvaluator implements WebInvocationPriv protected static final Log logger = LogFactory.getLog(DefaultWebInvocationPrivilegeEvaluator.class); - static final FilterChain DUMMY_CHAIN = new FilterChain() { - public void doFilter(ServletRequest req, ServletResponse res) throws IOException, ServletException { - throw new UnsupportedOperationException("DefaultWebInvocationPrivilegeEvaluator does not support filter chains"); - } - }; - - static final HttpServletResponse DUMMY_RESPONSE = new DummyResponse(); - //~ Instance fields ================================================================================================ private AbstractSecurityInterceptor securityInterceptor; @@ -114,11 +86,7 @@ public class DefaultWebInvocationPrivilegeEvaluator implements WebInvocationPriv public boolean isAllowed(String contextPath, String uri, String method, Authentication authentication) { Assert.notNull(uri, "uri parameter is required"); - if (contextPath == null) { - contextPath = "/ctxpath"; - } - - FilterInvocation fi = createFilterInvocation(contextPath, uri, method); + FilterInvocation fi = new FilterInvocation(contextPath, uri, method); Collection attrs = securityInterceptor.obtainSecurityMetadataSource().getAttributes(fi); if (attrs == null) { @@ -145,389 +113,6 @@ public class DefaultWebInvocationPrivilegeEvaluator implements WebInvocationPriv return true; } - - private FilterInvocation createFilterInvocation(String contextPath, String uri, String method) { - Assert.hasText(uri, "URI required"); - - DummyRequest req = new DummyRequest(); - req.setRequestURI(contextPath + uri); - req.setContextPath(contextPath); - req.setServletPath(null); - req.setMethod(method); - - return new FilterInvocation(req, DUMMY_RESPONSE, DUMMY_CHAIN); - } } -@SuppressWarnings("unchecked") -class DummyRequest implements HttpServletRequest { - private String requestURI; - private String contextPath = ""; - private String servletPath; - private String method; - - public void setRequestURI(String requestURI) { - this.requestURI = requestURI; - } - - public String getRequestURI() { - return requestURI; - } - - public void setContextPath(String contextPath) { - this.contextPath = contextPath; - } - - public String getContextPath() { - return contextPath; - } - - public void setServletPath(String servletPath) { - this.servletPath = servletPath; - } - - public String getServletPath() { - return servletPath; - } - - public void setMethod(String method) { - this.method = method; - } - - public String getMethod() { - return method; - } - - public String getPathInfo() { - return null; - } - - public String getQueryString() { - return null; - } - - public String getAuthType() { - throw new UnsupportedOperationException(); - } - - public Cookie[] getCookies() { - throw new UnsupportedOperationException(); - } - - public long getDateHeader(String name) { - throw new UnsupportedOperationException(); - } - - public String getHeader(String name) { - throw new UnsupportedOperationException(); - } - - public Enumeration getHeaderNames() { - throw new UnsupportedOperationException(); - } - - public Enumeration getHeaders(String name) { - throw new UnsupportedOperationException(); - } - - public int getIntHeader(String name) { - throw new UnsupportedOperationException(); - } - - public String getPathTranslated() { - throw new UnsupportedOperationException(); - } - - public String getRemoteUser() { - throw new UnsupportedOperationException(); - } - - public StringBuffer getRequestURL() { - throw new UnsupportedOperationException(); - } - - public String getRequestedSessionId() { - throw new UnsupportedOperationException(); - } - - public HttpSession getSession() { - throw new UnsupportedOperationException(); - } - - public HttpSession getSession(boolean create) { - throw new UnsupportedOperationException(); - } - - public Principal getUserPrincipal() { - throw new UnsupportedOperationException(); - } - - public boolean isRequestedSessionIdFromCookie() { - throw new UnsupportedOperationException(); - } - - public boolean isRequestedSessionIdFromURL() { - throw new UnsupportedOperationException(); - } - - public boolean isRequestedSessionIdFromUrl() { - throw new UnsupportedOperationException(); - } - - public boolean isRequestedSessionIdValid() { - throw new UnsupportedOperationException(); - } - - public boolean isUserInRole(String role) { - throw new UnsupportedOperationException(); - } - - public Object getAttribute(String name) { - throw new UnsupportedOperationException(); - } - - public Enumeration getAttributeNames() { - throw new UnsupportedOperationException(); - } - - public String getCharacterEncoding() { - throw new UnsupportedOperationException(); - } - - public int getContentLength() { - throw new UnsupportedOperationException(); - } - - public String getContentType() { - throw new UnsupportedOperationException(); - } - - public ServletInputStream getInputStream() throws IOException { - throw new UnsupportedOperationException(); - } - - public String getLocalAddr() { - throw new UnsupportedOperationException(); - - } - - public String getLocalName() { - throw new UnsupportedOperationException(); - } - - public int getLocalPort() { - throw new UnsupportedOperationException(); - } - - public Locale getLocale() { - throw new UnsupportedOperationException(); - } - - public Enumeration getLocales() { - throw new UnsupportedOperationException(); - } - - public String getParameter(String name) { - throw new UnsupportedOperationException(); - } - - public Map getParameterMap() { - throw new UnsupportedOperationException(); - } - - public Enumeration getParameterNames() { - throw new UnsupportedOperationException(); - } - - public String[] getParameterValues(String name) { - throw new UnsupportedOperationException(); - } - - public String getProtocol() { - throw new UnsupportedOperationException(); - } - - public BufferedReader getReader() throws IOException { - throw new UnsupportedOperationException(); - } - - public String getRealPath(String path) { - throw new UnsupportedOperationException(); - } - - public String getRemoteAddr() { - throw new UnsupportedOperationException(); - } - - public String getRemoteHost() { - throw new UnsupportedOperationException(); - } - - public int getRemotePort() { - throw new UnsupportedOperationException(); - } - - public RequestDispatcher getRequestDispatcher(String path) { - throw new UnsupportedOperationException(); - } - - public String getScheme() { - throw new UnsupportedOperationException(); - } - - public String getServerName() { - throw new UnsupportedOperationException(); - } - - public int getServerPort() { - throw new UnsupportedOperationException(); - } - - public boolean isSecure() { - throw new UnsupportedOperationException(); - } - - public void removeAttribute(String name) { - throw new UnsupportedOperationException(); - } - - public void setAttribute(String name, Object o) { - throw new UnsupportedOperationException(); - } - - public void setCharacterEncoding(String env) throws UnsupportedEncodingException { - throw new UnsupportedOperationException(); - } -} - -class DummyResponse implements HttpServletResponse { - public void addCookie(Cookie cookie) { - throw new UnsupportedOperationException(); - } - - public void addDateHeader(String name, long date) { - throw new UnsupportedOperationException(); - } - - public void addHeader(String name, String value) { - throw new UnsupportedOperationException(); - } - - public void addIntHeader(String name, int value) { - throw new UnsupportedOperationException(); - } - - public boolean containsHeader(String name) { - throw new UnsupportedOperationException(); - } - - public String encodeRedirectURL(String url) { - throw new UnsupportedOperationException(); - } - - public String encodeRedirectUrl(String url) { - throw new UnsupportedOperationException(); - } - - public String encodeURL(String url) { - throw new UnsupportedOperationException(); - } - - public String encodeUrl(String url) { - throw new UnsupportedOperationException(); - } - - public void sendError(int sc) throws IOException { - throw new UnsupportedOperationException(); - - } - - public void sendError(int sc, String msg) throws IOException { - throw new UnsupportedOperationException(); - } - - public void sendRedirect(String location) throws IOException { - throw new UnsupportedOperationException(); - } - - public void setDateHeader(String name, long date) { - throw new UnsupportedOperationException(); - } - - public void setHeader(String name, String value) { - throw new UnsupportedOperationException(); - } - - public void setIntHeader(String name, int value) { - throw new UnsupportedOperationException(); - } - - public void setStatus(int sc) { - throw new UnsupportedOperationException(); - } - - public void setStatus(int sc, String sm) { - throw new UnsupportedOperationException(); - } - - public void flushBuffer() throws IOException { - throw new UnsupportedOperationException(); - } - - public int getBufferSize() { - throw new UnsupportedOperationException(); - } - - public String getCharacterEncoding() { - throw new UnsupportedOperationException(); - } - - public String getContentType() { - throw new UnsupportedOperationException(); - } - - public Locale getLocale() { - throw new UnsupportedOperationException(); - } - - public ServletOutputStream getOutputStream() throws IOException { - throw new UnsupportedOperationException(); - } - - public PrintWriter getWriter() throws IOException { - throw new UnsupportedOperationException(); - } - - public boolean isCommitted() { - throw new UnsupportedOperationException(); - } - - public void reset() { - throw new UnsupportedOperationException(); - } - - public void resetBuffer() { - throw new UnsupportedOperationException(); - } - - public void setBufferSize(int size) { - throw new UnsupportedOperationException(); - } - - public void setCharacterEncoding(String charset) { - throw new UnsupportedOperationException(); - } - - public void setContentLength(int len) { - throw new UnsupportedOperationException(); - } - - public void setContentType(String type) { - throw new UnsupportedOperationException(); - } - - public void setLocale(Locale loc) { - throw new UnsupportedOperationException(); - } -} 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 6686c89693..96fcd49b82 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 @@ -11,8 +11,7 @@ import org.springframework.expression.ExpressionParser; import org.springframework.expression.ParseException; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource; -import org.springframework.security.web.access.intercept.RequestKey; -import org.springframework.security.web.util.UrlMatcher; +import org.springframework.security.web.util.RequestMatcher; import org.springframework.util.Assert; /** @@ -24,21 +23,22 @@ import org.springframework.util.Assert; public final class ExpressionBasedFilterInvocationSecurityMetadataSource extends DefaultFilterInvocationSecurityMetadataSource { private final static Log logger = LogFactory.getLog(ExpressionBasedFilterInvocationSecurityMetadataSource.class); - public ExpressionBasedFilterInvocationSecurityMetadataSource(UrlMatcher urlMatcher, - LinkedHashMap> requestMap, WebSecurityExpressionHandler expressionHandler) { - super(urlMatcher, processMap(requestMap, expressionHandler.getExpressionParser())); + public ExpressionBasedFilterInvocationSecurityMetadataSource( + LinkedHashMap> requestMap, + WebSecurityExpressionHandler expressionHandler) { + super(processMap(requestMap, expressionHandler.getExpressionParser())); Assert.notNull(expressionHandler, "A non-null SecurityExpressionHandler is required"); } - private static LinkedHashMap> processMap( - LinkedHashMap> requestMap, ExpressionParser parser) { + private static LinkedHashMap> processMap( + LinkedHashMap> requestMap, ExpressionParser parser) { Assert.notNull(parser, "SecurityExpressionHandler returned a null parser object"); - LinkedHashMap> requestToExpressionAttributesMap = - new LinkedHashMap>(requestMap); + LinkedHashMap> requestToExpressionAttributesMap = + new LinkedHashMap>(requestMap); - for (Map.Entry> entry : requestMap.entrySet()) { - RequestKey request = entry.getKey(); + for (Map.Entry> entry : requestMap.entrySet()) { + RequestMatcher request = entry.getKey(); Assert.isTrue(entry.getValue().size() == 1, "Expected a single expression attribute for " + request); ArrayList attributes = new ArrayList(1); String expression = entry.getValue().toArray(new ConfigAttribute[1])[0].getAttribute(); diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java b/web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java index f6cf0bf3dc..9854cc274b 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java @@ -15,192 +15,75 @@ package org.springframework.security.web.access.intercept; -import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; +import javax.servlet.http.HttpServletRequest; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.web.FilterInvocation; -import org.springframework.security.web.util.UrlMatcher; +import org.springframework.security.web.util.RequestMatcher; /** * Default implementation of FilterInvocationDefinitionSource. *

- * Stores an ordered map of compiled URL paths to ConfigAttribute lists and provides URL matching - * against the items stored in this map using the configured UrlMatcher. + * Stores an ordered map of {@link RequestMatcher}s to ConfigAttribute collections and provides matching + * of {@code FilterInvocation}s against the items stored in the map. *

- * The order of the URL paths in the map is very important. - * The system will identify the first matching path for a given HTTP URL. It will not proceed to evaluate - * later paths if a match has already been found. Accordingly, the most specific matches should be - * registered first, with the most general matches registered last. + * The order of the {@link RequestMatcher}s in the map is very important. The first one which matches the + * request will be used. Later matchers in the map will not be invoked if a match has already been found. + * Accordingly, the most specific matchers should be registered first, with the most general matches registered last. *

- * If URL paths are registered for a particular HTTP method using, then the method-specific matches will take - * precedence over any URLs which are registered without an HTTP method. + * The most common method creating an instance is using the Spring Security namespace. For example, the {@code pattern} + * and {@code access} attributes of the {@code <intercept-url>} elements defined as children of the + * {@code <http>} element are combined to build the instance used by the {@code FilterSecurityInterceptor}. * * @author Ben Alex * @author Luke Taylor */ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvocationSecurityMetadataSource { - private static final Set HTTP_METHODS = new HashSet(Arrays.asList("DELETE", "GET", "HEAD", "OPTIONS", "POST", "PUT", "TRACE")); - protected final Log logger = LogFactory.getLog(getClass()); - //private Map> requestMap = new LinkedHashMap>(); - /** Stores request maps keyed by specific HTTP methods. A null key matches any method */ - private Map>> httpMethodMap = - new HashMap>>(); - - private UrlMatcher urlMatcher; - - private boolean stripQueryStringFromUrls; + private final Map> requestMap; //~ Constructors =================================================================================================== /** - * Builds the internal request map from the supplied map. The key elements should be of type {@link RequestKey}, - * which contains a URL path and an optional HTTP method (may be null). The path stored in the key will depend on + * Sets the internal request map from the supplied map. The key elements should be of type {@link RequestMatcher}, + * which. The path stored in the key will depend on * the type of the supplied UrlMatcher. * - * @param urlMatcher typically an ant or regular expression matcher. * @param requestMap order-preserving map of request definitions to attribute lists */ - public DefaultFilterInvocationSecurityMetadataSource(UrlMatcher urlMatcher, - LinkedHashMap> requestMap) { - this.urlMatcher = urlMatcher; + public DefaultFilterInvocationSecurityMetadataSource( + LinkedHashMap> requestMap) { - for (Map.Entry> entry : requestMap.entrySet()) { - addSecureUrl(entry.getKey().getUrl(), entry.getKey().getMethod(), entry.getValue()); - } + this.requestMap = requestMap; } //~ Methods ======================================================================================================== - /** - * Adds a URL,attribute-list pair to the request map, first allowing the UrlMatcher to - * process the pattern if required, using its compile method. The returned object will be used as the key - * to the request map and will be passed back to the UrlMatcher when iterating through the map to find - * a match for a particular URL. - */ - private void addSecureUrl(String pattern, String method, Collection attrs) { - Map> mapToUse = getRequestMapForHttpMethod(method); - - mapToUse.put(urlMatcher.compile(pattern), attrs); - - if (logger.isDebugEnabled()) { - logger.debug("Added URL pattern: " + pattern + "; attributes: " + attrs + - (method == null ? "" : " for HTTP method '" + method + "'")); - } - } - - /** - * Return the HTTP method specific request map, creating it if it doesn't already exist. - * @param method GET, POST etc - * @return map of URL patterns to ConfigAttributes for this method. - */ - private Map> getRequestMapForHttpMethod(String method) { - if (method != null && !HTTP_METHODS.contains(method)) { - throw new IllegalArgumentException("Unrecognised HTTP method: '" + method + "'"); - } - - Map> methodRequestMap = httpMethodMap.get(method); - - if (methodRequestMap == null) { - methodRequestMap = new LinkedHashMap>(); - httpMethodMap.put(method, methodRequestMap); - } - - return methodRequestMap; - } - public Collection getAllConfigAttributes() { Set allAttributes = new HashSet(); - for (Map.Entry>> entry : httpMethodMap.entrySet()) { - for (Collection attrs : entry.getValue().values()) { - allAttributes.addAll(attrs); - } + for (Map.Entry> entry : requestMap.entrySet()) { + allAttributes.addAll(entry.getValue()); } return allAttributes; } - public Collection getAttributes(Object object) { - if ((object == null) || !this.supports(object.getClass())) { - throw new IllegalArgumentException("Object must be a FilterInvocation"); - } - - String url = ((FilterInvocation) object).getRequestUrl(); - String method = ((FilterInvocation) object).getHttpRequest().getMethod(); - - return lookupAttributes(url, method); - } - - /** - * Performs the actual lookup of the relevant ConfigAttributes for the given FilterInvocation. - *

- * By default, iterates through the stored URL map and calls the - * {@link UrlMatcher#pathMatchesUrl(Object path, String url)} method until a match is found. - * - * @param url the URI to retrieve configuration attributes for - * @param method the HTTP method (GET, POST, DELETE...), or null for any method. - * - * @return the ConfigAttributes that apply to the specified FilterInvocation - * or null if no match is found - */ - public final Collection lookupAttributes(String url, String method) { - if (stripQueryStringFromUrls) { - // Strip anything after a question mark symbol, as per SEC-161. See also SEC-321 - int firstQuestionMarkIndex = url.indexOf("?"); - - if (firstQuestionMarkIndex != -1) { - url = url.substring(0, firstQuestionMarkIndex); - } - } - - if (urlMatcher.requiresLowerCaseUrl()) { - url = url.toLowerCase(); - - if (logger.isDebugEnabled()) { - logger.debug("Converted URL to lowercase, from: '" + url + "'; to: '" + url + "'"); - } - } - - // Obtain the map of request patterns to attributes for this method and lookup the url. - Collection attributes = extractMatchingAttributes(url, httpMethodMap.get(method)); - - // If no attributes found in method-specific map, use the general one stored under the null key - if (attributes == null) { - attributes = extractMatchingAttributes(url, httpMethodMap.get(null)); - } - - return attributes; - } - - private Collection extractMatchingAttributes(String url, Map> map) { - if (map == null) { - return null; - } - - final boolean debug = logger.isDebugEnabled(); - - for (Map.Entry> entry : map.entrySet()) { - Object p = entry.getKey(); - boolean matched = urlMatcher.pathMatchesUrl(entry.getKey(), url); - - if (debug) { - logger.debug("Candidate is: '" + url + "'; pattern is " + p + "; matched=" + matched); - } - - if (matched) { + final HttpServletRequest request = ((FilterInvocation) object).getRequest(); + for (Map.Entry> entry : requestMap.entrySet()) { + if (entry.getKey().matches(request)) { return entry.getValue(); } } @@ -210,16 +93,4 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo public boolean supports(Class clazz) { return FilterInvocation.class.isAssignableFrom(clazz); } - - protected UrlMatcher getUrlMatcher() { - return urlMatcher; - } - - public boolean isConvertUrlToLowercaseBeforeComparison() { - return urlMatcher.requiresLowerCaseUrl(); - } - - public void setStripQueryStringFromUrls(boolean stripQueryStringFromUrls) { - this.stripQueryStringFromUrls = stripQueryStringFromUrls; - } } diff --git a/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java new file mode 100644 index 0000000000..f86ef33472 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/util/AntPathRequestMatcher.java @@ -0,0 +1,94 @@ +package org.springframework.security.web.util; + +import javax.servlet.http.HttpServletRequest; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.http.HttpMethod; +import org.springframework.util.AntPathMatcher; +import org.springframework.util.Assert; +import org.springframework.util.StringUtils; + +/** + * Matcher which compares a pre-defined ant-style pattern against the URL of an + * {@code HttpServletRequest}. Ignores the query string of the URL. + * + * @author Luke Taylor + * @since 3.1 + * + * @see AntPathMatcher + */ +public final class AntPathRequestMatcher implements RequestMatcher { + private final static Log logger = LogFactory.getLog(AntPathRequestMatcher.class); + + private static final AntPathMatcher antMatcher = new AntPathMatcher(); + + private final String pattern; + private final HttpMethod httpMethod; + + /** + * Creates a matcher with the specific pattern which will match all HTTP methods. + * + * @param pattern the ant pattern to use for matching + */ + public AntPathRequestMatcher(String pattern) { + this(pattern, null); + } + + /** + * Creates a matcher with the supplied pattern which will match all HTTP methods. + * + * @param pattern the ant pattern to use for matching + * @param httpMethod the HTTP method. The {@code matches} method will return false if the incoming request doesn't + * have the same method. + */ + public AntPathRequestMatcher(String pattern, String httpMethod) { + Assert.hasText(pattern, "Pattern cannot be null or empty"); + this.pattern = pattern.toLowerCase(); + this.httpMethod = StringUtils.hasText(httpMethod) ? HttpMethod.valueOf(httpMethod) : null; + } + + /** + * Returns true if the configured pattern (and HTTP-Method) match those of the supplied request. + * + * @param request the request to match against. + */ + public boolean matches(HttpServletRequest request) { + if (httpMethod != null && httpMethod != HttpMethod.valueOf(request.getMethod())) { + return false; + } + + String url = request.getServletPath(); + + if (request.getPathInfo() != null) { + url += request.getPathInfo(); + } + + url = url.toLowerCase(); + + if (logger.isDebugEnabled()) { + logger.debug("Checking match of request : '" + url + "'; against '" + pattern + "'"); + } + + // TODO: Optimise, since the pattern is fixed. + return antMatcher.match(pattern, url); + } + + public String getPattern() { + return pattern; + } + + @Override + public String toString() { + StringBuilder sb = new StringBuilder(); + sb.append("Ant [pattern='").append(pattern).append("'"); + + if (httpMethod != null) { + sb.append(", " + httpMethod); + } + + sb.append("]"); + + return sb.toString(); + } +} diff --git a/web/src/main/java/org/springframework/security/web/util/AnyRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/AnyRequestMatcher.java new file mode 100644 index 0000000000..34cd6c22b3 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/util/AnyRequestMatcher.java @@ -0,0 +1,17 @@ +package org.springframework.security.web.util; + +import javax.servlet.http.HttpServletRequest; + +/** + * Matches any supplied request. + * + * @author Luke Taylor + * @since 3.1 + */ +public final class AnyRequestMatcher implements RequestMatcher { + + public boolean matches(HttpServletRequest request) { + return true; + } + +} diff --git a/web/src/main/java/org/springframework/security/web/util/RegexRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/RegexRequestMatcher.java new file mode 100644 index 0000000000..2f64d6d291 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/util/RegexRequestMatcher.java @@ -0,0 +1,64 @@ +package org.springframework.security.web.util; + +import java.util.regex.Pattern; + +import javax.servlet.http.HttpServletRequest; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.http.HttpMethod; +import org.springframework.util.StringUtils; + +/** + * + * @author Luke Taylor + * @since 3.1 + */ +public final class RegexRequestMatcher implements RequestMatcher { + private final static Log logger = LogFactory.getLog(RegexRequestMatcher.class); + + private final Pattern pattern; + private final HttpMethod httpMethod; + + public RegexRequestMatcher(String pattern, String httpMethod) { + this(pattern, httpMethod, false); + } + + public RegexRequestMatcher(String pattern, String httpMethod, boolean caseInsensitive) { + if (caseInsensitive) { + this.pattern = Pattern.compile(pattern, Pattern.CASE_INSENSITIVE); + } else { + this.pattern = Pattern.compile(pattern); + } + this.httpMethod = StringUtils.hasText(httpMethod) ? HttpMethod.valueOf(httpMethod) : null; + } + + public boolean matches(HttpServletRequest request) { + if (httpMethod != null && httpMethod != HttpMethod.valueOf(request.getMethod())) { + return false; + } + + String url = request.getServletPath(); + String pathInfo = request.getPathInfo(); + String query = request.getQueryString(); + + if (pathInfo != null || query != null) { + StringBuilder sb = new StringBuilder(url); + + if (pathInfo != null) { + sb.append(pathInfo); + } + + if (query != null) { + sb.append(query); + } + url = sb.toString(); + } + + if (logger.isDebugEnabled()) { + logger.debug("Checking match of request : '" + url + "'; against '" + pattern + "'"); + } + + return pattern.matcher(url).matches(); + } +} diff --git a/web/src/test/java/org/springframework/security/web/access/intercept/FilterInvocationTests.java b/web/src/test/java/org/springframework/security/web/FilterInvocationTests.java similarity index 87% rename from web/src/test/java/org/springframework/security/web/access/intercept/FilterInvocationTests.java rename to web/src/test/java/org/springframework/security/web/FilterInvocationTests.java index 8fd098ce7d..b62ac9c446 100644 --- a/web/src/test/java/org/springframework/security/web/access/intercept/FilterInvocationTests.java +++ b/web/src/test/java/org/springframework/security/web/FilterInvocationTests.java @@ -13,17 +13,20 @@ * limitations under the License. */ -package org.springframework.security.web.access.intercept; +package org.springframework.security.web; import static org.junit.Assert.assertEquals; import static org.mockito.Mockito.mock; import javax.servlet.FilterChain; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.web.FilterInvocation; +import org.springframework.security.web.util.UrlUtils; /** * Tests {@link FilterInvocation}. @@ -115,4 +118,18 @@ public class FilterInvocationTests { assertEquals("FilterInvocation: URL: /HelloWorld", fi.toString()); assertEquals("http://www.example.com/mycontext/HelloWorld", fi.getFullRequestUrl()); } + + @Test(expected=UnsupportedOperationException.class) + public void dummyChainRejectsInvocation() throws Exception { + FilterInvocation.DUMMY_CHAIN.doFilter(mock(HttpServletRequest.class), mock(HttpServletResponse.class)); + } + + @Test + public void dummyRequestIsSupportedByUrlUtils() throws Exception { + DummyRequest request = new DummyRequest(); + request.setContextPath(""); + request.setRequestURI("/something"); + UrlUtils.buildRequestUrl(request); + } + } diff --git a/web/src/test/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluatorTests.java b/web/src/test/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluatorTests.java index 283154604e..968c9b2b5e 100644 --- a/web/src/test/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluatorTests.java +++ b/web/src/test/java/org/springframework/security/web/access/DefaultWebInvocationPrivilegeEvaluatorTests.java @@ -19,9 +19,6 @@ import static org.junit.Assert.*; import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - import org.junit.Before; import org.junit.Test; import org.springframework.context.ApplicationEventPublisher; @@ -34,7 +31,6 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource; import org.springframework.security.web.access.intercept.FilterSecurityInterceptor; -import org.springframework.security.web.util.UrlUtils; /** @@ -102,17 +98,4 @@ public class DefaultWebInvocationPrivilegeEvaluatorTests { assertFalse(wipe.isAllowed("/foo/index.jsp", token)); } - - @Test(expected=UnsupportedOperationException.class) - public void dummyChainRejectsInvocation() throws Exception { - DefaultWebInvocationPrivilegeEvaluator.DUMMY_CHAIN.doFilter(mock(HttpServletRequest.class), mock(HttpServletResponse.class)); - } - - @Test - public void dummyRequestIsSupportedByUrlUtils() throws Exception { - DummyRequest request = new DummyRequest(); - request.setContextPath(""); - request.setRequestURI("/something"); - UrlUtils.buildRequestUrl(request); - } } diff --git a/web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java b/web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java index 9fce8600df..fe526e8ce4 100644 --- a/web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java +++ b/web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java @@ -29,9 +29,8 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.access.SecurityConfig; import org.springframework.security.web.FilterInvocation; -import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource; -import org.springframework.security.web.access.intercept.RequestKey; -import org.springframework.security.web.util.AntUrlPathMatcher; +import org.springframework.security.web.util.AntPathRequestMatcher; +import org.springframework.security.web.util.RequestMatcher; /** * Tests parts of {@link DefaultFilterInvocationSecurityMetadataSource} not tested by {@link @@ -39,41 +38,25 @@ import org.springframework.security.web.util.AntUrlPathMatcher; * * @author Ben Alex */ -@SuppressWarnings("unchecked") public class DefaultFilterInvocationSecurityMetadataSourceTests { private DefaultFilterInvocationSecurityMetadataSource fids; private Collection def = SecurityConfig.createList("ROLE_ONE"); //~ Methods ======================================================================================================== - private void createFids(String url, String method) { - LinkedHashMap requestMap = new LinkedHashMap(); - requestMap.put(new RequestKey(url, method), def); - fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap); - fids.setStripQueryStringFromUrls(true); - } - - private void createFids(String url, boolean convertToLowerCase) { - LinkedHashMap requestMap = new LinkedHashMap(); - requestMap.put(new RequestKey(url), def); - fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(convertToLowerCase), requestMap); - fids.setStripQueryStringFromUrls(true); - } - - @Test - public void convertUrlToLowercaseIsTrueByDefault() { - LinkedHashMap requestMap = new LinkedHashMap(); - requestMap.put(new RequestKey("/something"), def); - fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap); - assertTrue(fids.isConvertUrlToLowercaseBeforeComparison()); + private void createFids(String pattern, String method) { + LinkedHashMap> requestMap = + new LinkedHashMap>(); + requestMap.put(new AntPathRequestMatcher(pattern, method), def); + 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); - assertEquals(def, fids.lookupAttributes(fi.getRequestUrl(), null)); + assertEquals(def, fids.getAttributes(fi)); } /** @@ -83,29 +66,19 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { public void lookupNotRequiringExactMatchSucceedsIfSecureUrlPathContainsUpperCase() { createFids("/SeCuRE/super/**", null); - FilterInvocation fi = createFilterInvocation("/secure/super/somefile.html", null); + FilterInvocation fi = createFilterInvocation("/secure", "/super/somefile.html", null, null); - Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.getAttributes(fi); assertEquals(def, response); } - @Test - public void lookupRequiringExactMatchFailsIfNotMatching() { - createFids("/secure/super/**", false); - - FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null); - - Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); - assertEquals(null, response); - } - @Test public void lookupRequiringExactMatchIsSuccessful() { - createFids("/SeCurE/super/**", false); + createFids("/SeCurE/super/**", null); - FilterInvocation fi = createFilterInvocation("/SeCurE/super/somefile.html", null); + FilterInvocation fi = createFilterInvocation("/SeCurE/super/somefile.html", null, null, null); - Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.getAttributes(fi); assertEquals(def, response); } @@ -113,9 +86,9 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { public void lookupRequiringExactMatchWithAdditionalSlashesIsSuccessful() { createFids("/someAdminPage.html**", null); - FilterInvocation fi = createFilterInvocation("/someAdminPage.html?a=/test", null); + FilterInvocation fi = createFilterInvocation("/someAdminPage.html", null, "a=/test", null); - Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.getAttributes(fi); assertEquals(def, response); // see SEC-161 (it should truncate after ? sign) } @@ -128,7 +101,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { public void httpMethodLookupSucceeds() { createFids("/somepage**", "GET"); - FilterInvocation fi = createFilterInvocation("/somepage", "GET"); + FilterInvocation fi = createFilterInvocation("/somepage", null, null, "GET"); Collection attrs = fids.getAttributes(fi); assertEquals(def, attrs); } @@ -137,7 +110,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { public void generalMatchIsUsedIfNoMethodSpecificMatchExists() { createFids("/somepage**", null); - FilterInvocation fi = createFilterInvocation("/somepage", "GET"); + FilterInvocation fi = createFilterInvocation("/somepage", null, null, "GET"); Collection attrs = fids.getAttributes(fi); assertEquals(def, attrs); } @@ -146,50 +119,23 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { public void requestWithDifferentHttpMethodDoesntMatch() { createFids("/somepage**", "GET"); - FilterInvocation fi = createFilterInvocation("/somepage", null); + FilterInvocation fi = createFilterInvocation("/somepage", null, null, "POST"); Collection attrs = fids.getAttributes(fi); assertNull(attrs); } - @Test - public void httpMethodSpecificUrlTakesPrecedence() { - LinkedHashMap> requestMap = new LinkedHashMap>(); - // Even though this is added before the Http method-specific def, the latter should match - requestMap.put(new RequestKey("/**"), def); - Collection postOnlyDef = SecurityConfig.createList("ROLE_TWO"); - requestMap.put(new RequestKey("/somepage**", "POST"), postOnlyDef); - fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap); - - Collection attrs = fids.getAttributes(createFilterInvocation("/somepage", "POST")); - assertEquals(postOnlyDef, attrs); - } - // SEC-1236 @Test public void mixingPatternsWithAndWithoutHttpMethodsIsSupported() throws Exception { - LinkedHashMap requestMap = new LinkedHashMap(); + LinkedHashMap> requestMap = + new LinkedHashMap>(); Collection userAttrs = SecurityConfig.createList("A"); - requestMap.put(new RequestKey("/user/**", null), userAttrs); - requestMap.put(new RequestKey("/teller/**", "GET"), SecurityConfig.createList("B")); - fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap); - fids.setStripQueryStringFromUrls(true); - FilterInvocation fi = createFilterInvocation("/user", "GET"); - Collection attrs = fids.getAttributes(fi); - assertEquals(userAttrs, attrs); - } + requestMap.put(new AntPathRequestMatcher("/user/**", null), userAttrs); + requestMap.put(new AntPathRequestMatcher("/teller/**", "GET"), SecurityConfig.createList("B")); + fids = new DefaultFilterInvocationSecurityMetadataSource(requestMap); - @Test - public void methodSpecificMatchTakesPrecdenceRegardlessOfOrdering() throws Exception { - // Unfortunate but unavoidable - LinkedHashMap requestMap = new LinkedHashMap(); - Collection userAttrs = SecurityConfig.createList("A"); - requestMap.put(new RequestKey("/secure/specific.html", null), SecurityConfig.createList("B")); - requestMap.put(new RequestKey("/secure/*.html", "GET"), userAttrs); - fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap); - fids.setStripQueryStringFromUrls(true); - - FilterInvocation fi = createFilterInvocation("/secure/specific.html", "GET"); + FilterInvocation fi = createFilterInvocation("/user", null, null, "GET"); Collection attrs = fids.getAttributes(fi); assertEquals(userAttrs, attrs); } @@ -201,23 +147,24 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests { public void extraQuestionMarkStillMatches() { createFids("/someAdminPage.html*", null); - FilterInvocation fi = createFilterInvocation("/someAdminPage.html?x=2/aa?y=3", null); + FilterInvocation fi = createFilterInvocation("/someAdminPage.html", null, null, null); - Collection response = fids.lookupAttributes(fi.getRequestUrl(), null); + Collection response = fids.getAttributes(fi); assertEquals(def, response); - fi = createFilterInvocation("/someAdminPage.html??", null); + fi = createFilterInvocation("/someAdminPage.html", null, "?", null); - response = fids.lookupAttributes(fi.getRequestUrl(), null); + response = fids.getAttributes(fi); assertEquals(def, response); } - private FilterInvocation createFilterInvocation(String path, String method) { + private FilterInvocation createFilterInvocation(String servletPath, String pathInfo, String queryString, String method) { MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI(null); request.setMethod(method); - - request.setServletPath(path); + request.setServletPath(servletPath); + request.setPathInfo(pathInfo); + request.setQueryString(queryString); return new FilterInvocation(request, new MockHttpServletResponse(), mock(FilterChain.class)); } diff --git a/web/template.mf b/web/template.mf index 0f7d71e5d7..f05c050e25 100644 --- a/web/template.mf +++ b/web/template.mf @@ -27,6 +27,7 @@ Import-Template: org.springframework.dao;version="[${spring.version}, 3.2.0)";resolution:=optional, org.springframework.expression;version="[${spring.version}, 3.2.0)";resolution:=optional, org.springframework.expression.spel.*;version="[${spring.version}, 3.2.0)";resolution:=optional, + org.springframework.http.*;version="[${spring.version}, 3.2.0)", org.springframework.jdbc.*;version="[${spring.version}, 3.2.0)";resolution:=optional, org.springframework.mock.web;version="[${spring.version}, 3.2.0)";resolution:=optional, org.springframework.web.context.*;version="[${spring.version}, 3.2.0)";resolution:=optional,