diff --git a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java index 4d44fa564e..866b81e0f7 100644 --- a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java +++ b/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java @@ -18,11 +18,8 @@ import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.ConfigAttributeEditor; import org.springframework.security.wrapper.SecurityContextHolderAwareRequestFilter; import org.springframework.security.context.HttpSessionContextIntegrationFilter; -import org.springframework.security.intercept.web.AbstractFilterInvocationDefinitionSource; -import org.springframework.security.intercept.web.FilterInvocationDefinitionMap; +import org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource; import org.springframework.security.intercept.web.FilterSecurityInterceptor; -import org.springframework.security.intercept.web.PathBasedFilterInvocationDefinitionMap; -import org.springframework.security.intercept.web.RegExpBasedFilterInvocationDefinitionMap; import org.springframework.security.securechannel.ChannelDecisionManagerImpl; import org.springframework.security.securechannel.ChannelProcessingFilter; import org.springframework.security.securechannel.InsecureChannelProcessor; @@ -33,6 +30,7 @@ import org.springframework.security.ui.ExceptionTranslationFilter; import org.springframework.security.util.FilterChainProxy; import org.springframework.security.util.RegexUrlPathMatcher; import org.springframework.security.util.AntUrlPathMatcher; +import org.springframework.security.util.UrlMatcher; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; @@ -65,6 +63,8 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { static final String OPT_REQUIRES_HTTPS = "https"; static final String OPT_ANY_CHANNEL = "any"; + static final String ATT_HTTP_METHOD = "method"; + static final String ATT_CREATE_SESSION = "create-session"; static final String DEF_CREATE_SESSION_IF_REQUIRED = "ifRequired"; static final String OPT_CREATE_SESSION_ALWAYS = "always"; @@ -118,8 +118,13 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { patternType = DEF_PATH_TYPE_ANT; } - FilterInvocationDefinitionMap interceptorFilterInvDefSource = new PathBasedFilterInvocationDefinitionMap(); - FilterInvocationDefinitionMap channelFilterInvDefSource = new PathBasedFilterInvocationDefinitionMap(); + 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); @@ -127,32 +132,26 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { lowercaseComparisons = null; } + // Only change from the defaults if the attribute has been set if ("true".equals(lowercaseComparisons)) { - interceptorFilterInvDefSource.setConvertUrlToLowercaseBeforeComparison(true); - channelFilterInvDefSource.setConvertUrlToLowercaseBeforeComparison(true); + if (useRegex) { + ((RegexUrlPathMatcher)matcher).setRequiresLowerCaseUrl(true); + } + // Default for ant is already to force lower case } else if ("false".equals(lowercaseComparisons)) { - interceptorFilterInvDefSource.setConvertUrlToLowercaseBeforeComparison(false); - channelFilterInvDefSource.setConvertUrlToLowercaseBeforeComparison(false); - } - - if (patternType.equals(OPT_PATH_TYPE_REGEX)) { - RegexUrlPathMatcher matcher = new RegexUrlPathMatcher(); - - if (lowercaseComparisons != null) { - matcher.setRequiresLowerCaseUrl("true".equals(lowercaseComparisons)); + if (!useRegex) { + ((AntUrlPathMatcher)matcher).setRequiresLowerCaseUrl(false); } + // Default for regex is no change + } - filterChainProxy.getPropertyValues().addPropertyValue("matcher", matcher); - - interceptorFilterInvDefSource = new RegExpBasedFilterInvocationDefinitionMap(); - channelFilterInvDefSource = new RegExpBasedFilterInvocationDefinitionMap(); - } else if (lowercaseComparisons != null) { - AntUrlPathMatcher matcher = new AntUrlPathMatcher(); - matcher.setRequiresLowerCaseUrl("true".equals(lowercaseComparisons)); + DefaultFilterInvocationDefinitionSource interceptorFilterInvDefSource = + new DefaultFilterInvocationDefinitionSource(matcher); + DefaultFilterInvocationDefinitionSource channelFilterInvDefSource = + new DefaultFilterInvocationDefinitionSource(matcher); - filterChainProxy.getPropertyValues().addPropertyValue("matcher", matcher); - } + filterChainProxy.getPropertyValues().addPropertyValue("matcher", matcher); // Add servlet-api integration filter if required String provideServletApi = element.getAttribute(ATT_SERVLET_API_PROVISION); @@ -181,11 +180,16 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { filterSecurityInterceptorBuilder.addPropertyValue("authenticationManager", ConfigUtils.registerProviderManagerIfNecessary(parserContext)); + // SEC-501 - should paths stored in request maps be converted to lower case + // true if Ant path and using lower case + boolean convertPathsToLowerCase = (matcher instanceof AntUrlPathMatcher) && matcher.requiresLowerCaseUrl(); + parseInterceptUrls(DomUtils.getChildElementsByTagName(element, "intercept-url"), - filterChainMap, interceptorFilterInvDefSource, channelFilterInvDefSource, parserContext); + filterChainMap, interceptorFilterInvDefSource, channelFilterInvDefSource, + convertPathsToLowerCase, parserContext); // Check if we need to register the channel processing beans - if (((AbstractFilterInvocationDefinitionSource)channelFilterInvDefSource).getMapSize() > 0) { + if (((DefaultFilterInvocationDefinitionSource)channelFilterInvDefSource).getMapSize() > 0) { // At least one channel requirement has been specified RootBeanDefinition channelFilter = new RootBeanDefinition(ChannelProcessingFilter.class); channelFilter.getPropertyValues().addPropertyValue("channelDecisionManager", @@ -268,8 +272,9 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { * FilterInvocationDefinitionSource used in FilterSecurityInterceptor. */ private void parseInterceptUrls(List urlElts, Map filterChainMap, - FilterInvocationDefinitionMap interceptorFilterInvDefSource, - FilterInvocationDefinitionMap channelFilterInvDefSource, ParserContext parserContext) { + DefaultFilterInvocationDefinitionSource interceptorFilterInvDefSource, + DefaultFilterInvocationDefinitionSource channelFilterInvDefSource, + boolean useLowerCasePaths, ParserContext parserContext) { Iterator urlEltsIterator = urlElts.iterator(); @@ -279,6 +284,14 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { Element urlElt = (Element) urlEltsIterator.next(); String path = urlElt.getAttribute(ATT_PATH_PATTERN); + if (useLowerCasePaths) { + path = path.toLowerCase(); + } + + String method = urlElt.getAttribute(ATT_HTTP_METHOD); + if (!StringUtils.hasText(method)) { + method = null; + } Assert.hasText(path, "path attribute cannot be empty or null"); @@ -287,7 +300,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { // Convert the comma-separated list of access attributes to a ConfigAttributeDefinition if (StringUtils.hasText(access)) { editor.setAsText(access); - interceptorFilterInvDefSource.addSecureUrl(path, (ConfigAttributeDefinition) editor.getValue()); + interceptorFilterInvDefSource.addSecureUrl(path, method, (ConfigAttributeDefinition) editor.getValue()); } String requiredChannel = urlElt.getAttribute(ATT_REQUIRES_CHANNEL); diff --git a/core/src/main/java/org/springframework/security/intercept/web/AbstractFilterInvocationDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/web/AbstractFilterInvocationDefinitionSource.java deleted file mode 100644 index ffd5872725..0000000000 --- a/core/src/main/java/org/springframework/security/intercept/web/AbstractFilterInvocationDefinitionSource.java +++ /dev/null @@ -1,152 +0,0 @@ -/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.security.intercept.web; - -import org.springframework.security.ConfigAttributeDefinition; -import org.springframework.security.util.UrlMatcher; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - -import java.util.Map; -import java.util.LinkedHashMap; -import java.util.Iterator; - - -/** - * Abstract implementation of FilterInvocationDefinitionSource. - *

- * Stores an ordered map of compiled URL paths to ConfigAttributeDefinitions and provides URL matching - * against the items stored in this map using the confgured UrlMatcher. - *

- * The order of registering the regular expressions using the {@link #addSecureUrl(String, - * ConfigAttributeDefinition)} is very important. The system will identify the first matching regular - * expression for a given HTTP URL. It will not proceed to evaluate later regular expressions if a match has already - * been found. Accordingly, the most specific regular expressions should be registered first, with the most general - * regular expressions registered last. - * - * @author Ben Alex - * @author Luke Taylor - * @version $Id$ - */ -public abstract class AbstractFilterInvocationDefinitionSource implements FilterInvocationDefinitionSource { - - protected final Log logger = LogFactory.getLog(getClass()); - - private Map requestMap = new LinkedHashMap(); - - private UrlMatcher urlMatcher; - - protected AbstractFilterInvocationDefinitionSource(UrlMatcher urlMatcher) { - this.urlMatcher = urlMatcher; - } - - //~ Methods ======================================================================================================== - - /** - * Adds a URL-ConfigAttributeDefinition 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. - */ - public void addSecureUrl(String pattern, ConfigAttributeDefinition attr) { - requestMap.put(urlMatcher.compile(pattern), attr); - - if (logger.isDebugEnabled()) { - logger.debug("Added URL pattern: " + pattern + "; attributes: " + attr); - } - } - - public Iterator getConfigAttributeDefinitions() { - return getRequestMap().values().iterator(); - } - - public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException { - if ((object == null) || !this.supports(object.getClass())) { - throw new IllegalArgumentException("Object must be a FilterInvocation"); - } - - String url = ((FilterInvocation) object).getRequestUrl(); - - return lookupAttributes(url); - } - - /** - * Performs the actual lookup of the relevant ConfigAttributeDefinition for the specified - * 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. - *

- * Subclasses can override if required to perform any modifications to the URL. - *

- * Public visiblity so that tablibs or other view helper classes can access the - * ConfigAttributeDefinition applying to a given URI pattern without needing to construct a mock - * FilterInvocation and retrieving the attibutes via the {@link #getAttributes(Object)} method. - * - * @param url the URI to retrieve configuration attributes for - * - * @return the ConfigAttributeDefinition that applies to the specified FilterInvocation - * or null if no match is foud - */ - public ConfigAttributeDefinition lookupAttributes(String url) { - if (urlMatcher.requiresLowerCaseUrl()) { - url = url.toLowerCase(); - - if (logger.isDebugEnabled()) { - logger.debug("Converted URL to lowercase, from: '" + url + "'; to: '" + url + "'"); - } - } - - Iterator entries = requestMap.entrySet().iterator(); - - while (entries.hasNext()) { - Map.Entry entry = (Map.Entry) entries.next(); - Object p = entry.getKey(); - boolean matched = urlMatcher.pathMatchesUrl(p, url); - - if (logger.isDebugEnabled()) { - logger.debug("Candidate is: '" + url + "'; pattern is " + p + "; matched=" + matched); - } - - if (matched) { - return (ConfigAttributeDefinition) entry.getValue(); - } - } - - return null; - } - - public boolean supports(Class clazz) { - return FilterInvocation.class.isAssignableFrom(clazz); - } - - public int getMapSize() { - return this.requestMap.size(); - } - - Map getRequestMap() { - return requestMap; - } - - protected UrlMatcher getUrlMatcher() { - return urlMatcher; - } - - public boolean isConvertUrlToLowercaseBeforeComparison() { - return urlMatcher.requiresLowerCaseUrl(); - } -} diff --git a/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java new file mode 100644 index 0000000000..e1d77b3115 --- /dev/null +++ b/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java @@ -0,0 +1,263 @@ +/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.intercept.web; + +import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.SecurityConfig; +import org.springframework.security.util.UrlMatcher; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import java.util.Map; +import java.util.LinkedHashMap; +import java.util.Iterator; +import java.util.HashMap; +import java.util.Set; +import java.util.HashSet; +import java.util.Arrays; +import java.util.List; + + +/** + * Default implementation of FilterInvocationDefinitionSource. + *

+ * Stores an ordered map of compiled URL paths to ConfigAttributeDefinitions and provides URL matching + * against the items stored in this map using the configured UrlMatcher. + *

+ * The order of registering the regular expressions using the + * {@link #addSecureUrl(String, ConfigAttributeDefinition)} is very important. + * The system will identify the first matching regular + * expression for a given HTTP URL. It will not proceed to evaluate later regular expressions if a match has already + * been found. Accordingly, the most specific regular expressions should be registered first, with the most general + * regular expressions registered last. + *

+ * If URLs are registered for a particular HTTP method using + * {@link #addSecureUrl(String, String, ConfigAttributeDefinition)}, then the method-specific matches will take + * precedence over any URLs which are registered without an HTTP method. + * + * @author Ben Alex + * @author Luke Taylor + * @version $Id$ + */ +public class DefaultFilterInvocationDefinitionSource implements FilterInvocationDefinitionSource { + + private static final Set HTTP_METHODS = new HashSet(Arrays.asList(new String[]{ "GET", "PUT", "DELETE", "POST" })); + + protected final Log logger = LogFactory.getLog(getClass()); + + /** + * Non method-specific map of URL patterns to ConfigAttributeDefinitions + * TODO: Store in the httpMethod map with null key. + */ + private Map requestMap = new LinkedHashMap(); + /** Stores request maps keyed by specific HTTP methods */ + private Map httpMethodMap = new HashMap(); + + private UrlMatcher urlMatcher; + + private boolean stripQueryStringFromUrls; + + /** + * Creates a FilterInvocationDefinitionSource with the supplied URL matching strategy. + * @param urlMatcher + */ + public DefaultFilterInvocationDefinitionSource(UrlMatcher urlMatcher) { + this.urlMatcher = urlMatcher; + } + + //~ Methods ======================================================================================================== + + public void addSecureUrl(String pattern, ConfigAttributeDefinition attr) { + addSecureUrl(pattern, null, attr); + } + + /** + * Adds a URL-ConfigAttributeDefinition 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. + */ + public void addSecureUrl(String pattern, String method, ConfigAttributeDefinition attr) { + Map mapToUse = getRequestMapForHttpMethod(method); + + mapToUse.put(urlMatcher.compile(pattern), attr); + + if (logger.isDebugEnabled()) { + logger.debug("Added URL pattern: " + pattern + "; attributes: " + attr + + (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 ConfigAttributeDefinitions for this method. + */ + private Map getRequestMapForHttpMethod(String method) { + if (method == null) { + return requestMap; + } + if (!HTTP_METHODS.contains(method)) { + throw new IllegalArgumentException("Unrecognised HTTP method: '" + method + "'"); + } + + Map methodRequestmap = (Map) httpMethodMap.get(method); + + if (methodRequestmap == null) { + methodRequestmap = new LinkedHashMap(); + httpMethodMap.put(method, methodRequestmap); + } + + return methodRequestmap; + } + + public Iterator getConfigAttributeDefinitions() { + return getRequestMap().values().iterator(); + } + + public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException { + 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); + } + + protected ConfigAttributeDefinition lookupAttributes(String url) { + return lookupAttributes(url, null); + } + + /** + * Performs the actual lookup of the relevant ConfigAttributeDefinition for the specified + * 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. + *

+ * Subclasses can override if required to perform any modifications to the URL. + * + * @param url the URI to retrieve configuration attributes for + * @param method the HTTP method (GET, POST, DELETE...). + * + * @return the ConfigAttributeDefinition that applies to the specified FilterInvocation + * or null if no match is foud + */ + protected ConfigAttributeDefinition 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 + "'"); + } + } + + ConfigAttributeDefinition attributes = null; + + Map methodSpecificMap = (Map) httpMethodMap.get(method); + + if (methodSpecificMap != null) { + attributes = lookupUrlInMap(methodSpecificMap, url); + } + + if (attributes == null) { + attributes = lookupUrlInMap(requestMap, url); + } + + return attributes; + } + + private ConfigAttributeDefinition lookupUrlInMap(Map requestMap, String url) { + Iterator entries = requestMap.entrySet().iterator(); + + while (entries.hasNext()) { + Map.Entry entry = (Map.Entry) entries.next(); + Object p = entry.getKey(); + boolean matched = urlMatcher.pathMatchesUrl(p, url); + + if (logger.isDebugEnabled()) { + logger.debug("Candidate is: '" + url + "'; pattern is " + p + "; matched=" + matched); + } + + if (matched) { + return (ConfigAttributeDefinition) entry.getValue(); + } + } + + return null; + } + + /** + * Allows or easier configuration using {@link FilterInvocationDefinitionSourceMapping}. + * + * @param mappings + * {@link java.util.List} of + * {@link FilterInvocationDefinitionSourceMapping} objects. + */ + void setMappings(List mappings) { + Iterator it = mappings.iterator(); + + while (it.hasNext()) { + FilterInvocationDefinitionSourceMapping mapping = (FilterInvocationDefinitionSourceMapping) it.next(); + ConfigAttributeDefinition configDefinition = new ConfigAttributeDefinition(); + + Iterator configAttributesIt = mapping.getConfigAttributes().iterator(); + while (configAttributesIt.hasNext()) { + String s = (String) configAttributesIt.next(); + configDefinition.addConfigAttribute(new SecurityConfig(s)); + } + + addSecureUrl(mapping.getUrl(), configDefinition); + } + } + + + public boolean supports(Class clazz) { + return FilterInvocation.class.isAssignableFrom(clazz); + } + + public int getMapSize() { + return this.requestMap.size(); + } + + Map getRequestMap() { + return requestMap; + } + + protected UrlMatcher getUrlMatcher() { + return urlMatcher; + } + + public boolean isConvertUrlToLowercaseBeforeComparison() { + return urlMatcher.requiresLowerCaseUrl(); + } + + protected void setStripQueryStringFromUrls(boolean stripQueryStringFromUrls) { + this.stripQueryStringFromUrls = stripQueryStringFromUrls; + } +} diff --git a/core/src/main/java/org/springframework/security/intercept/web/FIDSToFilterChainMapConverter.java b/core/src/main/java/org/springframework/security/intercept/web/FIDSToFilterChainMapConverter.java index 5951c35196..1e8ca652ca 100644 --- a/core/src/main/java/org/springframework/security/intercept/web/FIDSToFilterChainMapConverter.java +++ b/core/src/main/java/org/springframework/security/intercept/web/FIDSToFilterChainMapConverter.java @@ -29,13 +29,11 @@ public class FIDSToFilterChainMapConverter { // TODO: Check if this is necessary. Retained from refactoring of FilterChainProxy Assert.notNull(source.getConfigAttributeDefinitions(), "FilterChainProxy requires the " + "FilterInvocationDefinitionSource to return a non-null response to getConfigAttributeDefinitions()"); - Assert.isTrue( - source instanceof PathBasedFilterInvocationDefinitionMap || - source instanceof RegExpBasedFilterInvocationDefinitionMap, + Assert.isTrue(source instanceof DefaultFilterInvocationDefinitionSource, "Can't handle FilterInvocationDefinitionSource type " + source.getClass()); - AbstractFilterInvocationDefinitionSource fids = (AbstractFilterInvocationDefinitionSource)source; + DefaultFilterInvocationDefinitionSource fids = (DefaultFilterInvocationDefinitionSource)source; Map requestMap = fids.getRequestMap(); Iterator paths = requestMap.keySet().iterator(); diff --git a/core/src/main/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditor.java b/core/src/main/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditor.java index 5e2c16ee70..2d0965f973 100644 --- a/core/src/main/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditor.java +++ b/core/src/main/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditor.java @@ -23,20 +23,31 @@ import java.util.ArrayList; import java.util.List; import org.springframework.security.util.StringSplitUtils; +import org.springframework.security.util.RegexUrlPathMatcher; +import org.springframework.security.util.UrlMatcher; +import org.springframework.security.util.AntUrlPathMatcher; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.util.StringUtils; /** - * Property editor to assist with the setup of a {@link FilterInvocationDefinitionSource}.

The class creates and - * populates a {@link RegExpBasedFilterInvocationDefinitionMap} or {@link PathBasedFilterInvocationDefinitionMap} - * (depending on the type of patterns presented).

- *

By default the class treats presented patterns as regular expressions. If the keyword + * Property editor to assist with the setup of a {@link FilterInvocationDefinitionSource}. + *

+ * Note that from version 2.0, the use of property-editor based configuration is deprecated in favour of namespace + * configuration options. + *

+ * The class creates and populates a + * {@link org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource} + * using either an Ant or Regular Expression URL matching strategy depending on the type of patterns presented. + *

+ * By default the class treats presented patterns as regular expressions. If the keyword * PATTERN_TYPE_APACHE_ANT is present (case sensitive), patterns will be treated as Apache Ant paths - * rather than regular expressions.

+ * rather than regular expressions. * * @author Ben Alex + * @deprecated Use namespace configuration instead. May be removed in future versions. * @version $Id$ */ public class FilterInvocationDefinitionSourceEditor extends PropertyEditorSupport { @@ -50,38 +61,50 @@ public class FilterInvocationDefinitionSourceEditor extends PropertyEditorSuppor //~ Methods ======================================================================================================== public void setAsText(String s) throws IllegalArgumentException { - FilterInvocationDefinitionDecorator source = new FilterInvocationDefinitionDecorator(); + //FilterInvocationDefinitionDecorator source = new FilterInvocationDefinitionDecorator(); if ((s == null) || "".equals(s)) { // Leave target object empty - setValue(new RegExpBasedFilterInvocationDefinitionMap()); + setValue(new DefaultFilterInvocationDefinitionSource(new RegexUrlPathMatcher())); return; } - // Check if we need to override the default definition map - if (s.lastIndexOf(DIRECTIVE_PATTERN_TYPE_APACHE_ANT) != -1) { - source.setDecorated(new PathBasedFilterInvocationDefinitionMap()); + boolean useAnt = s.lastIndexOf(DIRECTIVE_PATTERN_TYPE_APACHE_ANT) != -1; + boolean converUrlToLowerCase = s.lastIndexOf(DIRECTIVE_CONVERT_URL_TO_LOWERCASE_BEFORE_COMPARISON) != -1; - if (logger.isDebugEnabled()) { + if (logger.isDebugEnabled()) { + if (useAnt) { logger.debug(("Detected " + DIRECTIVE_PATTERN_TYPE_APACHE_ANT + " directive; using Apache Ant style path expressions")); } - } else { - source.setDecorated(new RegExpBasedFilterInvocationDefinitionMap()); - } - if (s.lastIndexOf(DIRECTIVE_CONVERT_URL_TO_LOWERCASE_BEFORE_COMPARISON) != -1) { - if (logger.isDebugEnabled()) { - logger.debug("Detected " + DIRECTIVE_CONVERT_URL_TO_LOWERCASE_BEFORE_COMPARISON - + " directive; Instructing mapper to convert URLs to lowercase before comparison"); + if (converUrlToLowerCase) { + if (logger.isDebugEnabled()) { + logger.debug("Detected " + DIRECTIVE_CONVERT_URL_TO_LOWERCASE_BEFORE_COMPARISON + + " directive; Instructing mapper to convert URLs to lowercase before comparison"); + } } + } + + UrlMatcher matcher; + + if (useAnt) { + matcher = new AntUrlPathMatcher(); + ((AntUrlPathMatcher)matcher).setRequiresLowerCaseUrl(converUrlToLowerCase); - source.setConvertUrlToLowercaseBeforeComparison(true); } else { - source.setConvertUrlToLowercaseBeforeComparison(false); + matcher = new RegexUrlPathMatcher(); + ((RegexUrlPathMatcher)matcher).setRequiresLowerCaseUrl(converUrlToLowerCase); } + DefaultFilterInvocationDefinitionSource fids = new DefaultFilterInvocationDefinitionSource(matcher); + + if (useAnt) { + fids.setStripQueryStringFromUrls(true); + } + + BufferedReader br = new BufferedReader(new StringReader(s)); int counter = 0; String line; @@ -147,8 +170,7 @@ public class FilterInvocationDefinitionSourceEditor extends PropertyEditorSuppor } // Attempt to detect malformed lines (as per SEC-204) - if (source.isConvertUrlToLowercaseBeforeComparison() - && source.getDecorated() instanceof PathBasedFilterInvocationDefinitionMap) { + if (converUrlToLowerCase && useAnt) { // Should all be lowercase; check each character // We only do this for Ant (regexp have control chars) for (int i = 0; i < name.length(); i++) { @@ -174,9 +196,8 @@ public class FilterInvocationDefinitionSourceEditor extends PropertyEditorSuppor mappings.add(mapping); } - source.setMappings(mappings); - + fids.setMappings(mappings); - setValue(source.getDecorated()); + setValue(fids); } } diff --git a/core/src/main/java/org/springframework/security/intercept/web/PathBasedFilterInvocationDefinitionMap.java b/core/src/main/java/org/springframework/security/intercept/web/PathBasedFilterInvocationDefinitionMap.java index 238404d907..874ffc7f42 100644 --- a/core/src/main/java/org/springframework/security/intercept/web/PathBasedFilterInvocationDefinitionMap.java +++ b/core/src/main/java/org/springframework/security/intercept/web/PathBasedFilterInvocationDefinitionMap.java @@ -18,11 +18,8 @@ package org.springframework.security.intercept.web; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.util.AntUrlPathMatcher; -import java.util.Iterator; - - /** - * Extends AbstractFilterInvocationDefinitionSource, configuring it with a {@link AntUrlPathMatcher} to match URLs + * Extends DefaultFilterInvocationDefinitionSource, configuring it with a {@link AntUrlPathMatcher} to match URLs * using Apache Ant path-based patterns. *

* Apache Ant path expressions are used to match a HTTP request URL against a ConfigAttributeDefinition. @@ -39,42 +36,29 @@ import java.util.Iterator; * * @author Ben Alex * @author Luke taylor + * @deprecated DefaultFilterInvocationDefinitionSource should now be used with an AntUrlPathMatcher instead. * @version $Id$ */ -public class PathBasedFilterInvocationDefinitionMap extends AbstractFilterInvocationDefinitionSource +public class PathBasedFilterInvocationDefinitionMap extends DefaultFilterInvocationDefinitionSource implements FilterInvocationDefinition { //~ Constructors =================================================================================================== public PathBasedFilterInvocationDefinitionMap() { super(new AntUrlPathMatcher()); + setStripQueryStringFromUrls(true); } //~ Methods ======================================================================================================== - public void addSecureUrl(String antPath, ConfigAttributeDefinition attr) { + public void addSecureUrl(String antPath, String method, ConfigAttributeDefinition attr) { // SEC-501: If using lower case comparison, we should convert the paths to lower case // as any upper case characters included by mistake will prevent the URL from ever being matched. if (getUrlMatcher().requiresLowerCaseUrl()) { antPath = antPath.toLowerCase(); } - super.addSecureUrl(antPath, attr); - } - - public Iterator getConfigAttributeDefinitions() { - return getRequestMap().values().iterator(); - } - - public ConfigAttributeDefinition lookupAttributes(String url) { - // 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); - } - - return super.lookupAttributes(url); + super.addSecureUrl(antPath, method, attr); } public void setConvertUrlToLowercaseBeforeComparison(boolean bool) { diff --git a/core/src/main/java/org/springframework/security/intercept/web/RegExpBasedFilterInvocationDefinitionMap.java b/core/src/main/java/org/springframework/security/intercept/web/RegExpBasedFilterInvocationDefinitionMap.java index 731f191fd8..0887e7f9fc 100644 --- a/core/src/main/java/org/springframework/security/intercept/web/RegExpBasedFilterInvocationDefinitionMap.java +++ b/core/src/main/java/org/springframework/security/intercept/web/RegExpBasedFilterInvocationDefinitionMap.java @@ -16,18 +16,18 @@ package org.springframework.security.intercept.web; import org.springframework.security.util.RegexUrlPathMatcher; -import org.springframework.security.util.AntUrlPathMatcher; /** - * Configures an {@link AbstractFilterInvocationDefinitionSource} with a regular expression URL matching strategy + * Configures an {@link DefaultFilterInvocationDefinitionSource} with a regular expression URL matching strategy * {@link RegexUrlPathMatcher}. * * @author Ben Alex * @author Luke Taylor + * @deprecated * @version $Id$ */ -public class RegExpBasedFilterInvocationDefinitionMap extends AbstractFilterInvocationDefinitionSource +public class RegExpBasedFilterInvocationDefinitionMap extends DefaultFilterInvocationDefinitionSource implements FilterInvocationDefinition { //~ Constructors =================================================================================================== diff --git a/core/src/main/java/org/springframework/security/util/FilterInvocationUtils.java b/core/src/main/java/org/springframework/security/util/FilterInvocationUtils.java index 77c2fdf98f..5c4a287b9b 100644 --- a/core/src/main/java/org/springframework/security/util/FilterInvocationUtils.java +++ b/core/src/main/java/org/springframework/security/util/FilterInvocationUtils.java @@ -48,7 +48,7 @@ public final class FilterInvocationUtils { /** * Creates a FilterInvocation for the specified contextPath and Uri. - * Note the normal subclasses of AbstractFilterInvocationDefinitionSource disregard the + * Note the normal subclasses of DefaultFilterInvocationDefinitionSource disregard the * contextPath when evaluating which secure object metadata applies to a given * FilterInvocation, so generally the contextPath is unimportant unless you are using a * custom FilterInvocationDefinitionSource. diff --git a/core/src/main/resources/org/springframework/security/config/spring-security-2.0.rnc b/core/src/main/resources/org/springframework/security/config/spring-security-2.0.rnc index a8cfe14b31..ca20abcae7 100644 --- a/core/src/main/resources/org/springframework/security/config/spring-security-2.0.rnc +++ b/core/src/main/resources/org/springframework/security/config/spring-security-2.0.rnc @@ -132,6 +132,10 @@ intercept-url.attlist &= intercept-url.attlist &= ## The access configuration attributes that apply for the configured path. attribute access {xsd:string}? +intercept-url.attlist &= + ## The HTTP Method for which the access configuration attributes should apply. If not specified, the attributes will apply to any method. + attribute method {"GET" | "PUT" | "POST" | "DELETE"}? + intercept-url.attlist &= ## The filter list for the path. Currently can be set to "none" to remove a path from having any filters applied. The full filter stack (consisting of all defined filters, will be applied to any other paths). attribute filters {"none"}? diff --git a/core/src/main/resources/org/springframework/security/config/spring-security-2.0.xsd b/core/src/main/resources/org/springframework/security/config/spring-security-2.0.xsd index 85b62961ba..c3682123d3 100644 --- a/core/src/main/resources/org/springframework/security/config/spring-security-2.0.xsd +++ b/core/src/main/resources/org/springframework/security/config/spring-security-2.0.xsd @@ -355,6 +355,19 @@ The access configuration attributes that apply for the configured path. + + + The HTTP Method for which the access configuration attributes should apply. If not specified, the attributes will apply to any method. + + + + + + + + + + The filter list for the path. Currently can be set to "none" to remove a path from having any filters applied. The full filter stack (consisting of all defined filters, will be applied to any other paths). diff --git a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java index 6316a8c395..de3e8a98cf 100644 --- a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java +++ b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java @@ -132,15 +132,32 @@ public class HttpSecurityBeanDefinitionParserTests { FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR); FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource(); - ConfigAttributeDefinition attrs = fids.getAttributes(createFilterinvocation("/Secure")); + ConfigAttributeDefinition attrs = fids.getAttributes(createFilterinvocation("/Secure", null)); assertEquals(2, attrs.size()); assertTrue(attrs.contains(new SecurityConfig("ROLE_A"))); assertTrue(attrs.contains(new SecurityConfig("ROLE_B"))); - attrs = fids.getAttributes(createFilterinvocation("/secure")); + attrs = fids.getAttributes(createFilterinvocation("/secure", null)); assertEquals(1, attrs.size()); assertTrue(attrs.contains(new SecurityConfig("ROLE_C"))); } + @Test + public void httpMethodMatchIsSupported() throws Exception { + setContext( + " " + + " " + + " " + + " " + + " " + AUTH_PROVIDER_XML); + + FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR); + FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource(); + ConfigAttributeDefinition attrs = fids.getAttributes(createFilterinvocation("/secure", "POST")); + assertEquals(2, attrs.size()); + assertTrue(attrs.contains(new SecurityConfig("ROLE_A"))); + assertTrue(attrs.contains(new SecurityConfig("ROLE_B"))); + } + @Test public void minimalConfigurationParses() { setContext("" + AUTH_PROVIDER_XML); @@ -213,8 +230,9 @@ public class HttpSecurityBeanDefinitionParserTests { return (FilterChainProxy) appContext.getBean(BeanIds.FILTER_CHAIN_PROXY); } - private FilterInvocation createFilterinvocation(String path) { + private FilterInvocation createFilterinvocation(String path, String method) { MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod(method); request.setRequestURI(null); request.setServletPath(path); diff --git a/core/src/test/java/org/springframework/security/intercept/web/AbstractFilterInvocationDefinitionSourceTests.java b/core/src/test/java/org/springframework/security/intercept/web/AbstractFilterInvocationDefinitionSourceTests.java index 9e8843b308..b0f0ba4e91 100644 --- a/core/src/test/java/org/springframework/security/intercept/web/AbstractFilterInvocationDefinitionSourceTests.java +++ b/core/src/test/java/org/springframework/security/intercept/web/AbstractFilterInvocationDefinitionSourceTests.java @@ -29,7 +29,7 @@ import javax.servlet.ServletResponse; /** - * Tests {@link AbstractFilterInvocationDefinitionSource}. + * Tests {@link DefaultFilterInvocationDefinitionSource}. * * @author Ben Alex * @version $Id$ diff --git a/core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorTests.java b/core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorTests.java index b7680c3786..6392633d57 100644 --- a/core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorTests.java +++ b/core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorTests.java @@ -20,6 +20,7 @@ import junit.framework.TestCase; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.MockFilterChain; import org.springframework.security.SecurityConfig; +import org.springframework.security.util.RegexUrlPathMatcher; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -51,7 +52,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE\r\n\\A/secure/.*\\Z=ROLE_SUPERVISOR,ROLE_TELLER"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); assertFalse(map.isConvertUrlToLowercaseBeforeComparison()); } @@ -72,7 +73,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { editor.setAsText( "CONVERT_URL_TO_LOWERCASE_BEFORE_COMPARISON\r\n\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE\r\n\\A/secure/.*\\Z=ROLE_SUPERVISOR,ROLE_TELLER"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); assertTrue(map.isConvertUrlToLowercaseBeforeComparison()); } @@ -80,8 +81,8 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE\r\n\\A/secure/.*\\Z=ROLE_SUPERVISOR,ROLE_TELLER"); - FilterInvocationDefinitionMap map = (FilterInvocationDefinitionMap) editor.getValue(); - assertTrue(map instanceof RegExpBasedFilterInvocationDefinitionMap); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); + assertTrue(map.getUrlMatcher() instanceof RegexUrlPathMatcher); } public void testDetectsDuplicateDirectivesOnSameLineSituation1() { @@ -124,7 +125,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText(""); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); assertEquals(0, map.getMapSize()); } @@ -142,7 +143,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE\r\n\\A/secure/.*\\Z=ROLE_SUPERVISOR,ROLE_TELLER"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); Iterator iter = map.getConfigAttributeDefinitions(); int counter = 0; @@ -158,7 +159,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE,ANOTHER_ROLE"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/totally/different/path/index.html"); @@ -173,7 +174,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE\r\n\\A/secure/.*\\Z=ROLE_SUPERVISOR,ROLE_TELLER"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); assertEquals(2, map.getMapSize()); } @@ -181,7 +182,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText(null); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); assertEquals(0, map.getMapSize()); } @@ -190,7 +191,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { editor.setAsText( "\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE,ANOTHER_ROLE\r\n\\A/secure/.*\\Z=ROLE_SUPERVISOR,ROLE_TELLER"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); // Test ensures we match the first entry, not the second MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); @@ -211,7 +212,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { editor.setAsText( "\\A/secure/.*\\Z=ROLE_SUPERVISOR,ROLE_TELLER\r\n\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE,ANOTHER_ROLE"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); @@ -230,7 +231,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("\\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE,ANOTHER_ROLE"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); @@ -249,7 +250,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("PATTERN_TYPE_APACHE_ANT\r\n/secure/super/**=ROLE_WE_DONT_HAVE,ANOTHER_ROLE"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); @@ -269,7 +270,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { editor.setAsText( " \\A/secure/super.*\\Z=ROLE_WE_DONT_HAVE,ANOTHER_ROLE \r\n \r\n \r\n // comment line \r\n \\A/testing.*\\Z=ROLE_TEST \r\n"); - RegExpBasedFilterInvocationDefinitionMap map = (RegExpBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); assertEquals(2, map.getMapSize()); } } diff --git a/core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorWithPathsTests.java b/core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorWithPathsTests.java index 9b13836bc8..31ffba1365 100644 --- a/core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorWithPathsTests.java +++ b/core/src/test/java/org/springframework/security/intercept/web/FilterInvocationDefinitionSourceEditorWithPathsTests.java @@ -20,6 +20,7 @@ import junit.framework.TestCase; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.MockFilterChain; import org.springframework.security.SecurityConfig; +import org.springframework.security.util.AntUrlPathMatcher; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; @@ -52,8 +53,8 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa editor.setAsText( "PATTERN_TYPE_APACHE_ANT\r\n/secure/super/*=ROLE_WE_DONT_HAVE\r\n/secure/*=ROLE_SUPERVISOR,ROLE_TELLER"); - FilterInvocationDefinitionMap map = (FilterInvocationDefinitionMap) editor.getValue(); - assertTrue(map instanceof PathBasedFilterInvocationDefinitionMap); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); + assertTrue(map.getUrlMatcher() instanceof AntUrlPathMatcher); } public void testConvertUrlToLowercaseDefaultSettingUnchangedByEditor() { @@ -61,8 +62,8 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa editor.setAsText( "PATTERN_TYPE_APACHE_ANT\r\n/secure/super/*=ROLE_WE_DONT_HAVE\r\n/secure/*=ROLE_SUPERVISOR,ROLE_TELLER"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); - assertFalse(map.isConvertUrlToLowercaseBeforeComparison()); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); + assertFalse(map.getUrlMatcher().requiresLowerCaseUrl()); } public void testConvertUrlToLowercaseSettingApplied() { @@ -70,8 +71,8 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa editor.setAsText( "CONVERT_URL_TO_LOWERCASE_BEFORE_COMPARISON\r\nPATTERN_TYPE_APACHE_ANT\r\n/secure/super/*=ROLE_WE_DONT_HAVE\r\n/secure/*=ROLE_SUPERVISOR,ROLE_TELLER"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); - assertTrue(map.isConvertUrlToLowercaseBeforeComparison()); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); + assertTrue(map.getUrlMatcher().requiresLowerCaseUrl()); } public void testInvalidNameValueFailsToParse() { @@ -89,7 +90,7 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa editor.setAsText( "PATTERN_TYPE_APACHE_ANT\r\n/secure/super/*=ROLE_WE_DONT_HAVE\r\n/secure/*=ROLE_SUPERVISOR,ROLE_TELLER"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); Iterator iter = map.getConfigAttributeDefinitions(); int counter = 0; @@ -105,7 +106,7 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("PATTERN_TYPE_APACHE_ANT\r\n/secure/super/*=ROLE_WE_DONT_HAVE"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/totally/different/path/index.html"); @@ -121,7 +122,7 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa editor.setAsText( "PATTERN_TYPE_APACHE_ANT\r\n/secure/super/*=ROLE_WE_DONT_HAVE\r\n/secure/*=ROLE_SUPERVISOR,ROLE_TELLER"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); assertEquals(2, map.getMapSize()); } @@ -130,7 +131,7 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa editor.setAsText( "PATTERN_TYPE_APACHE_ANT\r\n/secure/super/**=ROLE_WE_DONT_HAVE,ANOTHER_ROLE\r\n/secure/**=ROLE_SUPERVISOR,ROLE_TELLER"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); // Test ensures we match the first entry, not the second MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); @@ -151,7 +152,7 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa editor.setAsText( "PATTERN_TYPE_APACHE_ANT\r\n/secure/**=ROLE_SUPERVISOR,ROLE_TELLER\r\n/secure/super/**=ROLE_WE_DONT_HAVE"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); @@ -170,7 +171,7 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa FilterInvocationDefinitionSourceEditor editor = new FilterInvocationDefinitionSourceEditor(); editor.setAsText("PATTERN_TYPE_APACHE_ANT\r\n/secure/super/*=ROLE_WE_DONT_HAVE,ANOTHER_ROLE"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); @@ -190,7 +191,7 @@ public class FilterInvocationDefinitionSourceEditorWithPathsTests extends TestCa editor.setAsText( " PATTERN_TYPE_APACHE_ANT\r\n /secure/super/*=ROLE_WE_DONT_HAVE\r\n /secure/*=ROLE_SUPERVISOR,ROLE_TELLER \r\n \r\n \r\n // comment line \r\n \r\n"); - PathBasedFilterInvocationDefinitionMap map = (PathBasedFilterInvocationDefinitionMap) editor.getValue(); + DefaultFilterInvocationDefinitionSource map = (DefaultFilterInvocationDefinitionSource) editor.getValue(); assertEquals(2, map.getMapSize()); } } diff --git a/core/src/test/java/org/springframework/security/intercept/web/MockFilterInvocationDefinitionSource.java b/core/src/test/java/org/springframework/security/intercept/web/MockFilterInvocationDefinitionSource.java index fe2fc82443..22c25b6b8b 100644 --- a/core/src/test/java/org/springframework/security/intercept/web/MockFilterInvocationDefinitionSource.java +++ b/core/src/test/java/org/springframework/security/intercept/web/MockFilterInvocationDefinitionSource.java @@ -30,7 +30,7 @@ import java.util.Vector; * @author Ben Alex * @version $Id$ */ -public class MockFilterInvocationDefinitionSource extends AbstractFilterInvocationDefinitionSource { +public class MockFilterInvocationDefinitionSource extends DefaultFilterInvocationDefinitionSource { //~ Instance fields ================================================================================================ private List list; @@ -77,7 +77,7 @@ public class MockFilterInvocationDefinitionSource extends AbstractFilterInvocati } } - public ConfigAttributeDefinition lookupAttributes(String url) { + public ConfigAttributeDefinition lookupAttributes(String url, String method) { throw new UnsupportedOperationException("mock method not implemented"); } } diff --git a/core/src/test/java/org/springframework/security/intercept/web/PathBasedFilterDefinitionMapTests.java b/core/src/test/java/org/springframework/security/intercept/web/PathBasedFilterInvocationDefinitionMapTests.java similarity index 55% rename from core/src/test/java/org/springframework/security/intercept/web/PathBasedFilterDefinitionMapTests.java rename to core/src/test/java/org/springframework/security/intercept/web/PathBasedFilterInvocationDefinitionMapTests.java index 9fb3ce6d90..78f1635515 100644 --- a/core/src/test/java/org/springframework/security/intercept/web/PathBasedFilterDefinitionMapTests.java +++ b/core/src/test/java/org/springframework/security/intercept/web/PathBasedFilterInvocationDefinitionMapTests.java @@ -15,8 +15,6 @@ package org.springframework.security.intercept.web; -import junit.framework.TestCase; - import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.MockFilterChain; import org.springframework.security.SecurityConfig; @@ -24,6 +22,9 @@ import org.springframework.security.SecurityConfig; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.junit.Test; +import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; /** * Tests parts of {@link PathBasedFilterInvocationDefinitionMap} not tested by {@link @@ -32,30 +33,25 @@ import org.springframework.mock.web.MockHttpServletResponse; * @author Ben Alex * @version $Id$ */ -public class PathBasedFilterDefinitionMapTests extends TestCase { - //~ Constructors =================================================================================================== - - public PathBasedFilterDefinitionMapTests() { - } - - public PathBasedFilterDefinitionMapTests(String arg0) { - super(arg0); - } +public class PathBasedFilterInvocationDefinitionMapTests { //~ Methods ======================================================================================================== - public void testConvertUrlToLowercaseIsTrueByDefault() { + @Test + public void convertUrlToLowercaseIsTrueByDefault() { PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); assertTrue(map.isConvertUrlToLowercaseBeforeComparison()); } - public void testConvertUrlToLowercaseSetterRespected() { + @Test + public void convertUrlToLowercaseSetterRespected() { PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); map.setConvertUrlToLowercaseBeforeComparison(false); assertFalse(map.isConvertUrlToLowercaseBeforeComparison()); } - public void testLookupNotRequiringExactMatchSuccessIfNotMatching() { + @Test + public void lookupNotRequiringExactMatchSuccessIfNotMatching() { PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); map.setConvertUrlToLowercaseBeforeComparison(true); @@ -63,90 +59,146 @@ public class PathBasedFilterDefinitionMapTests extends TestCase { def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); map.addSecureUrl("/secure/super/**", def); - FilterInvocation fi = createFilterinvocation("/SeCuRE/super/somefile.html"); + FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null); ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); } /** - * SEC-501. Not that as of 2.0, lower case comparisons are the default for this class. + * SEC-501. Note that as of 2.0, lower case comparisons are the default for this class. */ - public void testLookupNotRequiringExactMatchSucceedsIfSecureUrlPathContainsUpperCase() { + @Test + public void lookupNotRequiringExactMatchSucceedsIfSecureUrlPathContainsUpperCase() { PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); ConfigAttributeDefinition def = new ConfigAttributeDefinition(); def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); map.addSecureUrl("/SeCuRE/super/**", def); - FilterInvocation fi = createFilterinvocation("/secure/super/somefile.html"); + FilterInvocation fi = createFilterInvocation("/secure/super/somefile.html", null); ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); } - public void testLookupRequiringExactMatchFailsIfNotMatching() { + @Test + public void lookupRequiringExactMatchFailsIfNotMatching() { PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); map.setConvertUrlToLowercaseBeforeComparison(false); ConfigAttributeDefinition def = new ConfigAttributeDefinition(); def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); map.addSecureUrl("/secure/super/**", def); - FilterInvocation fi = createFilterinvocation("/SeCuRE/super/somefile.html"); + FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null); ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(null, response); } - public void testLookupRequiringExactMatchIsSuccessful() { + @Test + public void lookupRequiringExactMatchIsSuccessful() { PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); map.setConvertUrlToLowercaseBeforeComparison(false); ConfigAttributeDefinition def = new ConfigAttributeDefinition(); def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); map.addSecureUrl("/SeCurE/super/**", def); - FilterInvocation fi = createFilterinvocation("/SeCurE/super/somefile.html"); + FilterInvocation fi = createFilterInvocation("/SeCurE/super/somefile.html", null); ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); } - public void testLookupRequiringExactMatchWithAdditionalSlashesIsSuccessful() { + @Test + public void lookupRequiringExactMatchWithAdditionalSlashesIsSuccessful() { PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); ConfigAttributeDefinition def = new ConfigAttributeDefinition(); def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); map.addSecureUrl("/someAdminPage.html**", def); - FilterInvocation fi = createFilterinvocation("/someAdminPage.html?a=/test"); + FilterInvocation fi = createFilterInvocation("/someAdminPage.html?a=/test", null); ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); // see SEC-161 (it should truncate after ? sign) } + @Test(expected = IllegalArgumentException.class) + public void unknownHttpMethodIsRejected() { + PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); + ConfigAttributeDefinition def = new ConfigAttributeDefinition(); + def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); + map.addSecureUrl("/someAdminPage.html**", "UNKNOWN", def); + } + + @Test + public void httpMethodLookupSucceeds() { + PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); + ConfigAttributeDefinition def = new ConfigAttributeDefinition(); + def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); + map.addSecureUrl("/somepage**", "GET", def); + + FilterInvocation fi = createFilterInvocation("/somepage", "GET"); + ConfigAttributeDefinition attrs = map.getAttributes(fi); + assertEquals(def, attrs); + } + + @Test + public void requestWithDifferentHttpMethodDoesntMatch() { + PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); + ConfigAttributeDefinition def = new ConfigAttributeDefinition(); + def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); + map.addSecureUrl("/somepage**", "GET", def); + + FilterInvocation fi = createFilterInvocation("/somepage", null); + ConfigAttributeDefinition attrs = map.getAttributes(fi); + assertNull(attrs); + } + + @Test + public void httpMethodSpecificUrlTakesPrecedence() { + PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); + + // Even though this is added before the method-specific def, the latter should match + ConfigAttributeDefinition allMethodDef = new ConfigAttributeDefinition(); + allMethodDef.addConfigAttribute(new SecurityConfig("ROLE_ONE")); + map.addSecureUrl("/**", null, allMethodDef); + + ConfigAttributeDefinition postOnlyDef = new ConfigAttributeDefinition(); + postOnlyDef.addConfigAttribute(new SecurityConfig("ROLE_TWO")); + map.addSecureUrl("/somepage**", "POST", postOnlyDef); + + FilterInvocation fi = createFilterInvocation("/somepage", "POST"); + ConfigAttributeDefinition attrs = map.getAttributes(fi); + assertEquals(postOnlyDef, attrs); + } + /** * Check fixes for SEC-321 */ - public void testExtraQuestionMarkStillMatches() { + @Test + public void extraQuestionMarkStillMatches() { PathBasedFilterInvocationDefinitionMap map = new PathBasedFilterInvocationDefinitionMap(); ConfigAttributeDefinition def = new ConfigAttributeDefinition(); def.addConfigAttribute(new SecurityConfig("ROLE_ONE")); map.addSecureUrl("/someAdminPage.html*", def); - FilterInvocation fi = createFilterinvocation("/someAdminPage.html?x=2/aa?y=3"); + FilterInvocation fi = createFilterInvocation("/someAdminPage.html?x=2/aa?y=3", null); ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); - fi = createFilterinvocation("/someAdminPage.html??"); + fi = createFilterInvocation("/someAdminPage.html??", null); response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); } - private FilterInvocation createFilterinvocation(String path) { + private FilterInvocation createFilterInvocation(String path, String method) { MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI(null); + request.setMethod(method); request.setServletPath(path);