diff --git a/core/src/main/java/org/springframework/security/ConfigAttributeEditor.java b/core/src/main/java/org/springframework/security/ConfigAttributeEditor.java index 1958002188..20272f7c52 100644 --- a/core/src/main/java/org/springframework/security/ConfigAttributeEditor.java +++ b/core/src/main/java/org/springframework/security/ConfigAttributeEditor.java @@ -34,7 +34,7 @@ public class ConfigAttributeEditor extends PropertyEditorSupport { public void setAsText(String s) throws IllegalArgumentException { if (StringUtils.hasText(s)) { - setValue(new ConfigAttributeDefinition(StringUtils.commaDelimitedListToStringArray(s))); + setValue(SecurityConfig.createList(StringUtils.commaDelimitedListToStringArray(s))); } else { setValue(null); } diff --git a/core/src/main/java/org/springframework/security/SecurityConfig.java b/core/src/main/java/org/springframework/security/SecurityConfig.java index 2c4194b92f..af0ada3be2 100644 --- a/core/src/main/java/org/springframework/security/SecurityConfig.java +++ b/core/src/main/java/org/springframework/security/SecurityConfig.java @@ -15,6 +15,9 @@ package org.springframework.security; +import java.util.ArrayList; +import java.util.List; + import org.springframework.util.Assert; /** @@ -31,7 +34,7 @@ public class SecurityConfig implements ConfigAttribute { //~ Constructors =================================================================================================== public SecurityConfig(String config) { - Assert.hasText(config, "You must provide a configuration attribute"); + Assert.hasText(config, "You must provide a configuration attribute"); this.attrib = config; } @@ -58,4 +61,15 @@ public class SecurityConfig implements ConfigAttribute { public String toString() { return this.attrib; } + + public static List createList(String... attributeNames) { + Assert.notNull(attributeNames, "You must supply a list of argument names"); + List attributes = new ArrayList(attributeNames.length); + + for (String attribute : attributeNames) { + attributes.add(new SecurityConfig(attribute)); + } + + return attributes; + } } diff --git a/core/src/main/java/org/springframework/security/annotation/Jsr250MethodDefinitionSource.java b/core/src/main/java/org/springframework/security/annotation/Jsr250MethodDefinitionSource.java index 1d6ceda30a..34eefa5bcc 100644 --- a/core/src/main/java/org/springframework/security/annotation/Jsr250MethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/annotation/Jsr250MethodDefinitionSource.java @@ -48,7 +48,7 @@ public class Jsr250MethodDefinitionSource extends AbstractFallbackMethodDefiniti return processAnnotations(AnnotationUtils.getAnnotations(method)); } - public Collection getConfigAttributeDefinitions() { + public Collection> getConfigAttributeDefinitions() { return null; } diff --git a/core/src/main/java/org/springframework/security/annotation/SecuredMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/annotation/SecuredMethodDefinitionSource.java index 7e785c298e..de8d0e1aff 100644 --- a/core/src/main/java/org/springframework/security/annotation/SecuredMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/annotation/SecuredMethodDefinitionSource.java @@ -43,7 +43,7 @@ public class SecuredMethodDefinitionSource extends AbstractFallbackMethodDefinit return processAnnotation(AnnotationUtils.findAnnotation(method, Secured.class)); } - public Collection getConfigAttributeDefinitions() { + public Collection> getConfigAttributeDefinitions() { return null; } diff --git a/core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java b/core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java index 1e8dcafcdb..774489bb25 100644 --- a/core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java +++ b/core/src/main/java/org/springframework/security/config/FilterChainProxyPostProcessor.java @@ -16,6 +16,7 @@ import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.core.OrderComparator; import org.springframework.core.Ordered; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.config.ConfigUtils.FilterChainList; import org.springframework.security.context.HttpSessionContextIntegrationFilter; @@ -33,66 +34,66 @@ import org.springframework.security.util.FilterChainProxy; import org.springframework.security.wrapper.SecurityContextHolderAwareRequestFilter; /** - * + * * @author Luke Taylor * @version $Id$ * @since 2.0 */ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFactoryAware { private Log logger = LogFactory.getLog(getClass()); - + private ListableBeanFactory beanFactory; - + public Object postProcessBeforeInitialization(Object bean, String beanName) throws BeansException { if(!BeanIds.FILTER_CHAIN_PROXY.equals(beanName)) { return bean; } - + FilterChainProxy filterChainProxy = (FilterChainProxy) bean; FilterChainList filterList = (FilterChainList) beanFactory.getBean(BeanIds.FILTER_LIST); - + List filters = new ArrayList(filterList.getFilters()); Collections.sort(filters, new OrderComparator()); - + logger.info("Checking sorted filter chain: " + filters); - + for(int i=0; i < filters.size(); i++) { Ordered filter = (Ordered)filters.get(i); if (i > 0) { Ordered previous = (Ordered)filters.get(i-1); if (filter.getOrder() == previous.getOrder()) { - throw new SecurityConfigurationException("Filters '" + unwrapFilter(filter) + "' and '" + + throw new SecurityConfigurationException("Filters '" + unwrapFilter(filter) + "' and '" + unwrapFilter(previous) + "' have the same 'order' value. When using custom filters, " + - "please make sure the positions do not conflict with default filters. " + - "Alternatively you can disable the default filters by removing the corresponding " + - "child elements from and avoiding the use of ."); + "please make sure the positions do not conflict with default filters. " + + "Alternatively you can disable the default filters by removing the corresponding " + + "child elements from and avoiding the use of ."); } } } - logger.info("Filter chain..."); + logger.info("Filter chain..."); for (int i=0; i < filters.size(); i++) { // Remove the ordered wrapper from the filter and put it back in the chain at the same position. Filter filter = unwrapFilter(filters.get(i)); - logger.info("[" + i + "] - " + filter); + logger.info("[" + i + "] - " + filter); filters.set(i, filter); } - + checkFilterStack(filters); - + // Note that this returns a copy Map filterMap = filterChainProxy.getFilterChainMap(); filterMap.put(filterChainProxy.getMatcher().getUniversalMatchPattern(), filters); filterChainProxy.setFilterChainMap(filterMap); - + checkLoginPageIsntProtected(filterChainProxy); - + logger.info("FilterChainProxy: " + filterChainProxy); return bean; } - + /** * Checks the filter list for possible errors and logs them */ @@ -105,7 +106,7 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac checkForDuplicates(ExceptionTranslationFilter.class, filters); checkForDuplicates(FilterSecurityInterceptor.class, filters); } - + private void checkForDuplicates(Class clazz, List filters) { for (int i=0; i < filters.size(); i++) { Filter f1 = (Filter)filters.get(i); @@ -122,53 +123,55 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac } } } - + /* Checks for the common error of having a login page URL protected by the security interceptor */ private void checkLoginPageIsntProtected(FilterChainProxy fcp) { ExceptionTranslationFilter etf = (ExceptionTranslationFilter) beanFactory.getBean(BeanIds.EXCEPTION_TRANSLATION_FILTER); - + if (etf.getAuthenticationEntryPoint() instanceof AuthenticationProcessingFilterEntryPoint) { - String loginPage = + String loginPage = ((AuthenticationProcessingFilterEntryPoint)etf.getAuthenticationEntryPoint()).getLoginFormUrl(); List filters = fcp.getFilters(loginPage); logger.info("Checking whether login URL '" + loginPage + "' is accessible with your configuration"); - + if (filters == null || filters.isEmpty()) { logger.debug("Filter chain is empty for the login page"); return; } - + if (loginPage.equals(DefaultLoginPageGeneratingFilter.DEFAULT_LOGIN_PAGE_URL) && beanFactory.containsBean(BeanIds.DEFAULT_LOGIN_PAGE_GENERATING_FILTER)) { logger.debug("Default generated login page is in use"); return; } - - FilterSecurityInterceptor fsi = + + FilterSecurityInterceptor fsi = ((FilterSecurityInterceptor)beanFactory.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR)); - DefaultFilterInvocationDefinitionSource fids = + DefaultFilterInvocationDefinitionSource fids = (DefaultFilterInvocationDefinitionSource) fsi.getObjectDefinitionSource(); - ConfigAttributeDefinition cad = fids.lookupAttributes(loginPage, "POST"); - - if (cad == null) { + List attributes = fids.lookupAttributes(loginPage, "POST"); + + if (attributes == null) { logger.debug("No access attributes defined for login page URL"); if (fsi.isRejectPublicInvocations()) { logger.warn("FilterSecurityInterceptor is configured to reject public invocations." + - " Your login page may not be accessible."); + " Your login page may not be accessible."); } return; } + ConfigAttributeDefinition cad = new ConfigAttributeDefinition(fids.lookupAttributes(loginPage, "POST")); + if (!beanFactory.containsBean(BeanIds.ANONYMOUS_PROCESSING_FILTER)) { logger.warn("The login page is being protected by the filter chain, but you don't appear to have" + - " anonymous authentication enabled. This is almost certainly an error."); + " anonymous authentication enabled. This is almost certainly an error."); return; } - + // Simulate an anonymous access with the supplied attributes. AnonymousProcessingFilter anonPF = (AnonymousProcessingFilter) beanFactory.getBean(BeanIds.ANONYMOUS_PROCESSING_FILTER); - AnonymousAuthenticationToken token = - new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(), + AnonymousAuthenticationToken token = + new AnonymousAuthenticationToken("key", anonPF.getUserAttribute().getPassword(), anonPF.getUserAttribute().getAuthorities()); try { fsi.getAccessDecisionManager().decide(token, new Object(), cad); @@ -179,15 +182,15 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac } } } - - /** - * Returns the delegate filter of a wrapper, or the unchanged filter if it isn't wrapped. + + /** + * Returns the delegate filter of a wrapper, or the unchanged filter if it isn't wrapped. */ private Filter unwrapFilter(Object filter) { if (filter instanceof OrderedFilterBeanDefinitionDecorator.OrderedFilterDecorator) { return ((OrderedFilterBeanDefinitionDecorator.OrderedFilterDecorator)filter).getDelegate(); } - + return (Filter) filter; } @@ -195,7 +198,7 @@ public class FilterChainProxyPostProcessor implements BeanPostProcessor, BeanFac return bean; } - public void setBeanFactory(BeanFactory beanFactory) throws BeansException { - this.beanFactory = (ListableBeanFactory) beanFactory; - } + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + this.beanFactory = (ListableBeanFactory) beanFactory; + } } 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 c5abdc1546..3526379b44 100644 --- a/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java +++ b/core/src/main/java/org/springframework/security/config/HttpSecurityBeanDefinitionParser.java @@ -561,7 +561,7 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser { } editor.setAsText(channelConfigAttribute); - channelRequestMap.put(new RequestKey(path), (ConfigAttributeDefinition) editor.getValue()); + channelRequestMap.put(new RequestKey(path), editor.getValue()); } String filters = urlElt.getAttribute(ATT_FILTERS); diff --git a/core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java b/core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java index 510702ae09..c6acd8fee0 100644 --- a/core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java +++ b/core/src/main/java/org/springframework/security/expression/SecurityExpressionRoot.java @@ -17,7 +17,7 @@ public class SecurityExpressionRoot { this.authentication = a; } - public boolean hasRole(String role) { + public final boolean hasRole(String role) { return hasAnyRole(role); } @@ -33,19 +33,27 @@ public class SecurityExpressionRoot { return false; } - public boolean isAnonymous() { + public final boolean permitAll() { + return true; + } + + public final boolean denyAll() { + return false; + } + + public final boolean isAnonymous() { return trustResolver.isAnonymous(authentication); } - public boolean isRememberMe() { + public final boolean isRememberMe() { return trustResolver.isRememberMe(authentication); } - public String getName() { - return authentication.getName(); + public Authentication getAuthentication() { + return authentication; } - public boolean isFullyAuthenticated() { + public final boolean isFullyAuthenticated() { return !trustResolver.isAnonymous(authentication) && !trustResolver.isRememberMe(authentication); } @@ -64,4 +72,8 @@ public class SecurityExpressionRoot { public Object getReturnObject() { return returnObject; } + + public Object getPrincipal() { + return authentication.getPrincipal(); + } } diff --git a/core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java b/core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java index 7f13cf8894..a77d4e354e 100644 --- a/core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java +++ b/core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java @@ -8,7 +8,14 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** - * Annotation for specifying a method filtering expression which will be evaluated after a method has been invoked. + * Annotation for specifying a method filtering expression which will be evaluated before a method has been invoked. + * The name of the argument to be filtered is specified using the filterTarget attribute. This must be a + * Java Collection implementation which supports the {@link java.util.Collection#remove(Object) remove} method. + * Pre-filtering isn't supported on array types. + *

+ * The annotation value contains the expression which will be evaluated for each element in the collection. If the + * expression evaluates to false, the element will be removed. The reserved name "filterObject" can be used within the + * expression to refer to the current object which is being evaluated. * * @author Luke Taylor * @version $Id$ diff --git a/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java index 871cbc0236..bc9f930ff5 100644 --- a/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java @@ -8,7 +8,6 @@ import java.util.List; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.expression.ParseException; import org.springframework.security.ConfigAttribute; -import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.config.SecurityConfigurationException; import org.springframework.security.expression.annotation.PostAuthorize; import org.springframework.security.expression.annotation.PostFilter; @@ -19,6 +18,10 @@ import org.springframework.security.intercept.method.AbstractFallbackMethodDefin /** * MethodDefinitionSource which extracts metadata from the @PreFilter and @PreAuthorize annotations * placed on a method. The metadata is encapsulated in a {@link AbstractExpressionBasedMethodConfigAttribute} instance. + *

+ * Annotations may be specified on classes or methods, and method-specific annotations will take precedence. + * If you use any annotation and do not specify a pre-authorization condition, then the method will be + * allowed as if a @PreAuthorize("permitAll") were present. * * @see MethodExpressionVoter * @@ -30,38 +33,80 @@ public class ExpressionAnnotationMethodDefinitionSource extends AbstractFallback @Override protected List findAttributes(Method method, Class targetClass) { - ConfigAttribute pre = processPreInvocationAnnotations(AnnotationUtils.findAnnotation(method, PreFilter.class), - AnnotationUtils.findAnnotation(method, PreAuthorize.class)); - ConfigAttribute post = processPostInvocationAnnotations(AnnotationUtils.findAnnotation(method, PostFilter.class), - AnnotationUtils.findAnnotation(method, PostAuthorize.class)); + PreFilter preFilter = AnnotationUtils.findAnnotation(method, PreFilter.class); + PreAuthorize preAuthorize = AnnotationUtils.findAnnotation(method, PreAuthorize.class); + PostFilter postFilter = AnnotationUtils.findAnnotation(method, PostFilter.class); + PostAuthorize postAuthorize = AnnotationUtils.findAnnotation(method, PostAuthorize.class); - if (pre == null && post == null) { + if (preFilter == null && preAuthorize == null && postFilter == null && postAuthorize == null ) { + // There is no method level meta-data so return and allow parent to query at class-level return null; } - List attrs = new ArrayList(2); - if (pre != null) { - attrs.add(pre); + // There is at least one non-null value, so the parent class will not query for class-specific annotations + // and we have to locate them here as appropriate. + + if (preAuthorize == null) { + preAuthorize = (PreAuthorize)targetClass.getAnnotation(PreAuthorize.class); } - if (post != null) { - attrs.add(post); + if (preFilter == null) { + preFilter = (PreFilter)targetClass.getAnnotation(PreFilter.class); } - return attrs; + if (postFilter == null) { + // TODO: Can we check for void methods and throw an exception here? + postFilter = (PostFilter)targetClass.getAnnotation(PostFilter.class); + } + + if (postAuthorize == null) { + postAuthorize = (PostAuthorize)targetClass.getAnnotation(PostAuthorize.class); + } + + return createAttributeList(preFilter, preAuthorize, postFilter, postAuthorize); } @Override protected List findAttributes(Class targetClass) { - ConfigAttribute pre = processPreInvocationAnnotations((PreFilter)targetClass.getAnnotation(PreFilter.class), - (PreAuthorize)targetClass.getAnnotation(PreAuthorize.class)); - ConfigAttribute post = processPostInvocationAnnotations((PostFilter)targetClass.getAnnotation(PostFilter.class), - (PostAuthorize)targetClass.getAnnotation(PostAuthorize.class)); + PreFilter preFilter = (PreFilter)targetClass.getAnnotation(PreFilter.class); + PreAuthorize preAuthorize = (PreAuthorize)targetClass.getAnnotation(PreAuthorize.class); + PostFilter postFilter = (PostFilter)targetClass.getAnnotation(PostFilter.class); + PostAuthorize postAuthorize = (PostAuthorize)targetClass.getAnnotation(PostAuthorize.class); - if (pre == null && post == null) { + if (preFilter == null && preAuthorize == null && postFilter == null && postAuthorize == null ) { + // There is no class level meta-data (and by implication no meta-data at all) return null; } + return createAttributeList(preFilter, preAuthorize, postFilter, postAuthorize); + } + + public Collection> getConfigAttributeDefinitions() { + return null; + } + + private List createAttributeList(PreFilter preFilter, PreAuthorize preAuthorize, + PostFilter postFilter, PostAuthorize postAuthorize) { + ConfigAttribute pre = null; + ConfigAttribute post = null; + + // TODO: Optimization of permitAll + String preAuthorizeExpression = preAuthorize == null ? "permitAll()" : preAuthorize.value(); + String preFilterExpression = preFilter == null ? null : preFilter.value(); + String filterObject = preFilter == null ? null : preFilter.filterTarget(); + String postAuthorizeExpression = postAuthorize == null ? null : postAuthorize.value(); + String postFilterExpression = postFilter == null ? null : postFilter.value(); + + try { + pre = new PreInvocationExpressionConfigAttribute(preFilterExpression, filterObject, preAuthorizeExpression); + if (postFilterExpression != null || postAuthorizeExpression != null) { + post = new PostInvocationExpressionConfigAttribute(postFilterExpression, postAuthorizeExpression); + } + } catch (ParseException e) { + throw new SecurityConfigurationException("Failed to parse expression '" + e.getExpressionString() + "'", e); + } + + List attrs = new ArrayList(2); if (pre != null) { attrs.add(pre); @@ -73,39 +118,4 @@ public class ExpressionAnnotationMethodDefinitionSource extends AbstractFallback return attrs; } - - public Collection getConfigAttributeDefinitions() { - return null; - } - - private ConfigAttribute processPreInvocationAnnotations(PreFilter preFilter, PreAuthorize preAuthz) { - if (preFilter == null && preAuthz == null) { - return null; - } - - String preAuthorizeExpression = preAuthz == null ? null : preAuthz.value(); - String preFilterExpression = preFilter == null ? null : preFilter.value(); - String filterObject = preFilter == null ? null : preFilter.filterTarget(); - - try { - return new PreInvocationExpressionBasedMethodConfigAttribute(preFilterExpression, filterObject, preAuthorizeExpression); - } catch (ParseException e) { - throw new SecurityConfigurationException("Failed to parse expression '" + e.getExpressionString() + "'", e); - } - } - - private ConfigAttribute processPostInvocationAnnotations(PostFilter postFilter, PostAuthorize postAuthz) { - if (postFilter == null && postAuthz == null) { - return null; - } - - String postAuthorizeExpression = postAuthz == null ? null : postAuthz.value(); - String postFilterExpression = postFilter == null ? null : postFilter.value(); - - try { - return new PostInvocationExpressionBasedMethodConfigAttribute(postFilterExpression, postAuthorizeExpression); - } catch (ParseException e) { - throw new SecurityConfigurationException("Failed to parse expression '" + e.getExpressionString() + "'", e); - } - } } diff --git a/core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java b/core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java index 919a978eff..4343042429 100644 --- a/core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java +++ b/core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java @@ -35,7 +35,7 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP public Object decide(Authentication authentication, Object object, ConfigAttributeDefinition config, Object returnedObject) throws AccessDeniedException { - PostInvocationExpressionBasedMethodConfigAttribute mca = findMethodAccessControlExpression(config); + PostInvocationExpressionConfigAttribute mca = findMethodAccessControlExpression(config); if (mca == null) { return returnedObject; @@ -86,11 +86,11 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP } } - private PostInvocationExpressionBasedMethodConfigAttribute findMethodAccessControlExpression(ConfigAttributeDefinition config) { + private PostInvocationExpressionConfigAttribute findMethodAccessControlExpression(ConfigAttributeDefinition config) { // Find the MethodAccessControlExpression attribute for (ConfigAttribute attribute : config.getConfigAttributes()) { - if (attribute instanceof PostInvocationExpressionBasedMethodConfigAttribute) { - return (PostInvocationExpressionBasedMethodConfigAttribute)attribute; + if (attribute instanceof PostInvocationExpressionConfigAttribute) { + return (PostInvocationExpressionConfigAttribute)attribute; } } @@ -98,7 +98,7 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP } public boolean supports(ConfigAttribute attribute) { - return attribute instanceof PostInvocationExpressionBasedMethodConfigAttribute; + return attribute instanceof PostInvocationExpressionConfigAttribute; } public boolean supports(Class clazz) { diff --git a/core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java b/core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java index 19fb8bb2d4..000fc82d94 100644 --- a/core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java +++ b/core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java @@ -44,7 +44,7 @@ public class MethodExpressionVoter implements AccessDecisionVoter { } public int vote(Authentication authentication, Object object, ConfigAttributeDefinition config) { - PreInvocationExpressionBasedMethodConfigAttribute mace = findMethodAccessControlExpression(config); + PreInvocationExpressionConfigAttribute mace = findMethodAccessControlExpression(config); if (mace == null) { // No expression based metadata, so abstain @@ -62,7 +62,7 @@ public class MethodExpressionVoter implements AccessDecisionVoter { if (preFilter != null) { // TODO: Allow null target if only single parameter, or single collection/array? - Object filtered = ExpressionUtils.doFilter(filterTarget, preFilter, ctx); + ExpressionUtils.doFilter(filterTarget, preFilter, ctx); } if (preAuthorize == null) { @@ -88,19 +88,25 @@ public class MethodExpressionVoter implements AccessDecisionVoter { } } - if (filterTargetName != null && filterTarget == null) { - throw new IllegalArgumentException("No filter target argument with name " + filterTargetName + - " found in method: " + method.getName()); + if (filterTargetName != null) { + if (filterTarget == null) { + throw new IllegalArgumentException("No filter target argument with name " + filterTargetName + + " found in method: " + method.getName()); + } + if (filterTarget.getClass().isArray()) { + throw new IllegalArgumentException("Pre-filtering on array types is not supported. Changing '" + + filterTargetName +"' to a collection will solve this problem"); + } } return filterTarget; } - private PreInvocationExpressionBasedMethodConfigAttribute findMethodAccessControlExpression(ConfigAttributeDefinition config) { + private PreInvocationExpressionConfigAttribute findMethodAccessControlExpression(ConfigAttributeDefinition config) { // Find the MethodAccessControlExpression attribute for (ConfigAttribute attribute : config.getConfigAttributes()) { - if (attribute instanceof AbstractExpressionBasedMethodConfigAttribute) { - return (PreInvocationExpressionBasedMethodConfigAttribute)attribute; + if (attribute instanceof PreInvocationExpressionConfigAttribute) { + return (PreInvocationExpressionConfigAttribute)attribute; } } diff --git a/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionBasedMethodConfigAttribute.java b/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionBasedMethodConfigAttribute.java deleted file mode 100644 index 52a94ac7dc..0000000000 --- a/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionBasedMethodConfigAttribute.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.springframework.security.expression.support; - -import org.springframework.expression.ParseException; - -class PostInvocationExpressionBasedMethodConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute { - - PostInvocationExpressionBasedMethodConfigAttribute(String filterExpression, String authorizeExpression) - throws ParseException { - super(filterExpression, authorizeExpression); - } - -} diff --git a/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java b/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java new file mode 100644 index 0000000000..8fd273e21a --- /dev/null +++ b/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java @@ -0,0 +1,12 @@ +package org.springframework.security.expression.support; + +import org.springframework.expression.ParseException; + +class PostInvocationExpressionConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute { + + PostInvocationExpressionConfigAttribute(String filterExpression, String authorizeExpression) + throws ParseException { + super(filterExpression, authorizeExpression); + } + +} diff --git a/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionBasedMethodConfigAttribute.java b/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionBasedMethodConfigAttribute.java deleted file mode 100644 index 1c522e1a96..0000000000 --- a/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionBasedMethodConfigAttribute.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.springframework.security.expression.support; - -import org.springframework.expression.ParseException; - -class PreInvocationExpressionBasedMethodConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute { - private final String filterTarget; - - PreInvocationExpressionBasedMethodConfigAttribute(String filterExpression, String filterTarget, - String authorizeExpression) throws ParseException { - super(filterExpression, authorizeExpression); - - this.filterTarget = filterTarget; - } - - String getFilterTarget() { - return filterTarget; - } -} diff --git a/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java b/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java new file mode 100644 index 0000000000..8ba1140bd9 --- /dev/null +++ b/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java @@ -0,0 +1,23 @@ +package org.springframework.security.expression.support; + +import org.springframework.expression.ParseException; + +class PreInvocationExpressionConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute { + private final String filterTarget; + + PreInvocationExpressionConfigAttribute(String filterExpression, String filterTarget, String authorizeExpression) + throws ParseException { + super(filterExpression, authorizeExpression); + + this.filterTarget = filterTarget; + } + + /** + * The parameter name of the target argument (must be a Collection) to which filtering will be applied. + * + * @return the method parameter name + */ + String getFilterTarget() { + return filterTarget; + } +} diff --git a/core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java b/core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java index 01a026ba5c..be55c657b7 100644 --- a/core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java +++ b/core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java @@ -51,6 +51,7 @@ import org.springframework.util.Assert; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Set; import java.util.Collection; @@ -110,7 +111,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A protected static final Log logger = LogFactory.getLog(AbstractSecurityInterceptor.class); //~ Instance fields ================================================================================================ - + protected MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor(); private ApplicationEventPublisher eventPublisher; private AccessDecisionManager accessDecisionManager; @@ -184,7 +185,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A } if (this.validateConfigAttributes) { - Collection attributeDefs = this.obtainObjectDefinitionSource().getConfigAttributeDefinitions(); + Collection> attributeDefs = this.obtainObjectDefinitionSource().getConfigAttributeDefinitions(); if (attributeDefs == null) { logger.warn("Could not validate configuration attributes as the ObjectDefinitionSource did not return " @@ -192,16 +193,10 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A return; } - Iterator iter = attributeDefs.iterator(); Set unsupportedAttrs = new HashSet(); - while (iter.hasNext()) { - ConfigAttributeDefinition def = (ConfigAttributeDefinition) iter.next(); - Iterator attributes = def.getConfigAttributes().iterator(); - - while (attributes.hasNext()) { - ConfigAttribute attr = (ConfigAttribute) attributes.next(); - + for (List def : attributeDefs) { + for (ConfigAttribute attr : def) { if (!this.runAsManager.supports(attr) && !this.accessDecisionManager.supports(attr) && ((this.afterInvocationManager == null) || !this.afterInvocationManager.supports(attr))) { unsupportedAttrs.add(attr); @@ -227,7 +222,13 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A + getSecureObjectClass()); } - ConfigAttributeDefinition attr = this.obtainObjectDefinitionSource().getAttributes(object); + List attributes = this.obtainObjectDefinitionSource().getAttributes(object); + ConfigAttributeDefinition attr = null; + + // TODO: temporary until refactor security interceptor and AccessManager + if (attributes != null) { + attr = new ConfigAttributeDefinition(attributes); + } if (attr == null) { if (rejectPublicInvocations) { diff --git a/core/src/main/java/org/springframework/security/intercept/ObjectDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/ObjectDefinitionSource.java index fae4f9b7be..f3957aae23 100644 --- a/core/src/main/java/org/springframework/security/intercept/ObjectDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/ObjectDefinitionSource.java @@ -15,9 +15,11 @@ package org.springframework.security.intercept; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import java.util.Collection; +import java.util.List; /** @@ -32,17 +34,17 @@ public interface ObjectDefinitionSource { //~ Methods ======================================================================================================== /** - * Accesses the ConfigAttributeDefinition that applies to a given secure object.

Returns - * null if no ConfigAttribiteDefinition applies.

+ * Accesses the ConfigAttributeDefinition that applies to a given secure object. + *

Returns null if no attributes apply. * * @param object the object being secured * - * @return the ConfigAttributeDefinition that applies to the passed object + * @return the attributes that apply to the passed in secured object * * @throws IllegalArgumentException if the passed object is not of a type supported by the * ObjectDefinitionSource implementation */ - ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException; + List getAttributes(Object object) throws IllegalArgumentException; /** * If available, returns all of the ConfigAttributeDefinitions defined by the implementing class. @@ -52,7 +54,7 @@ public interface ObjectDefinitionSource { * * @return the ConfigAttributeDefinitions or null if unsupported */ - Collection getConfigAttributeDefinitions(); + Collection> getConfigAttributeDefinitions(); /** * Indicates whether the ObjectDefinitionSource implementation is able to provide diff --git a/core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java index 14c9023f09..763c0f0f01 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/AbstractFallbackMethodDefinitionSource.java @@ -1,6 +1,7 @@ package org.springframework.security.intercept.method; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -11,7 +12,6 @@ import org.apache.commons.logging.LogFactory; import org.aspectj.lang.JoinPoint; import org.aspectj.lang.reflect.CodeSignature; import org.springframework.security.ConfigAttribute; -import org.springframework.security.ConfigAttributeDefinition; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; @@ -39,12 +39,12 @@ import org.springframework.util.ObjectUtils; * @since 2.0 */ public abstract class AbstractFallbackMethodDefinitionSource implements MethodDefinitionSource { + protected final Log logger = LogFactory.getLog(getClass()); - private static final Log logger = LogFactory.getLog(AbstractFallbackMethodDefinitionSource.class); - private final static Object NULL_CONFIG_ATTRIBUTE = new Object(); - private final Map attributeCache = new HashMap(); + private final static List NULL_CONFIG_ATTRIBUTE = new ArrayList(0); + private final Map> attributeCache = new HashMap(); - public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException { + public List getAttributes(Object object) throws IllegalArgumentException { Assert.notNull(object, "Object cannot be null"); if (object instanceof MethodInvocation) { @@ -73,11 +73,11 @@ public abstract class AbstractFallbackMethodDefinitionSource implements MethodDe return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz)); } - public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) { + public List getAttributes(Method method, Class targetClass) { // First, see if we have a cached value. - Object cacheKey = new DefaultCacheKey(method, targetClass); - synchronized (this.attributeCache) { - Object cached = this.attributeCache.get(cacheKey); + DefaultCacheKey cacheKey = new DefaultCacheKey(method, targetClass); + synchronized (attributeCache) { + List cached = attributeCache.get(cacheKey); if (cached != null) { // Value will either be canonical value indicating there is no config attribute, // or an actual config attribute. @@ -85,25 +85,22 @@ public abstract class AbstractFallbackMethodDefinitionSource implements MethodDe return null; } else { - return (ConfigAttributeDefinition) cached; + return cached; } } else { // We need to work it out. List attributes = computeAttributes(method, targetClass); - ConfigAttributeDefinition cfgAtt = null; // Put it in the cache. if (attributes == null) { this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE); } else { - cfgAtt = new ConfigAttributeDefinition(attributes); - if (logger.isDebugEnabled()) { - logger.debug("Adding security method [" + cacheKey + "] with attribute [" + cfgAtt + "]"); + logger.debug("Adding security method [" + cacheKey + "] with attributes " + attributes); } - this.attributeCache.put(cacheKey, cfgAtt); + this.attributeCache.put(cacheKey, attributes); } - return cfgAtt; + return attributes; } } } diff --git a/core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java index f5a9f56019..5a1648ba78 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/AbstractMethodDefinitionSource.java @@ -15,7 +15,7 @@ package org.springframework.security.intercept.method; -import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.ConfigAttribute; import org.aopalliance.intercept.MethodInvocation; @@ -28,6 +28,7 @@ import org.aspectj.lang.reflect.CodeSignature; import org.springframework.util.Assert; import java.lang.reflect.Method; +import java.util.List; /** @@ -44,7 +45,7 @@ public abstract class AbstractMethodDefinitionSource implements MethodDefinition //~ Methods ======================================================================================================== - public ConfigAttributeDefinition getAttributes(Object object) + public List getAttributes(Object object) throws IllegalArgumentException { Assert.notNull(object, "Object cannot be null"); @@ -89,7 +90,7 @@ public abstract class AbstractMethodDefinitionSource implements MethodDefinition * * @return the ConfigAttributeDefinition that applies to the specified Method */ - protected abstract ConfigAttributeDefinition lookupAttributes(Method method); + protected abstract List lookupAttributes(Method method); public boolean supports(Class clazz) { return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz)); diff --git a/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java index e0372a5ee1..66bea4e503 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/DelegatingMethodDefinitionSource.java @@ -8,73 +8,74 @@ import java.util.List; import java.util.Set; import org.springframework.beans.factory.InitializingBean; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.util.Assert; /** * Automatically tries a series of method definition sources, relying on the first source of metadata * that provides a non-null response. - * + * * @author Ben Alex * @version $Id$ */ public final class DelegatingMethodDefinitionSource implements MethodDefinitionSource, InitializingBean { - private List methodDefinitionSources; - - public void afterPropertiesSet() throws Exception { - Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required"); - } + private List methodDefinitionSources; - public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) { - Iterator i = methodDefinitionSources.iterator(); - while (i.hasNext()) { - MethodDefinitionSource s = (MethodDefinitionSource) i.next(); - ConfigAttributeDefinition result = s.getAttributes(method, targetClass); - if (result != null) { - return result; - } - } - return null; - } + public void afterPropertiesSet() throws Exception { + Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required"); + } - public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException { - Iterator i = methodDefinitionSources.iterator(); - while (i.hasNext()) { - MethodDefinitionSource s = (MethodDefinitionSource) i.next(); - ConfigAttributeDefinition result = s.getAttributes(object); - if (result != null) { - return result; - } - } - return null; - } + public List getAttributes(Method method, Class targetClass) { + Iterator i = methodDefinitionSources.iterator(); + while (i.hasNext()) { + MethodDefinitionSource s = (MethodDefinitionSource) i.next(); + List result = s.getAttributes(method, targetClass); + if (result != null) { + return result; + } + } + return null; + } - public Collection getConfigAttributeDefinitions() { - Set set = new HashSet(); - Iterator i = methodDefinitionSources.iterator(); - while (i.hasNext()) { - MethodDefinitionSource s = (MethodDefinitionSource) i.next(); - Collection attrs = s.getConfigAttributeDefinitions(); - if (attrs != null) { - set.addAll(attrs); - } - } - return set; - } + public List getAttributes(Object object) throws IllegalArgumentException { + Iterator i = methodDefinitionSources.iterator(); + while (i.hasNext()) { + MethodDefinitionSource s = (MethodDefinitionSource) i.next(); + List result = s.getAttributes(object); + if (result != null) { + return result; + } + } + return null; + } - public boolean supports(Class clazz) { - Iterator i = methodDefinitionSources.iterator(); - while (i.hasNext()) { - MethodDefinitionSource s = (MethodDefinitionSource) i.next(); - if (s.supports(clazz)) { - return true; - } - } - return false; - } + public Collection> getConfigAttributeDefinitions() { + Set set = new HashSet(); + Iterator i = methodDefinitionSources.iterator(); + while (i.hasNext()) { + MethodDefinitionSource s = (MethodDefinitionSource) i.next(); + Collection> attrs = s.getConfigAttributeDefinitions(); + if (attrs != null) { + set.addAll(attrs); + } + } + return set; + } - public void setMethodDefinitionSources(List methodDefinitionSources) { - Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required"); - this.methodDefinitionSources = methodDefinitionSources; - } + public boolean supports(Class clazz) { + Iterator i = methodDefinitionSources.iterator(); + while (i.hasNext()) { + MethodDefinitionSource s = (MethodDefinitionSource) i.next(); + if (s.supports(clazz)) { + return true; + } + } + return false; + } + + public void setMethodDefinitionSources(List methodDefinitionSources) { + Assert.notEmpty(methodDefinitionSources, "A list of MethodDefinitionSources is required"); + this.methodDefinitionSources = methodDefinitionSources; + } } diff --git a/core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java index d9afe74907..17feea7f62 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSource.java @@ -219,14 +219,8 @@ public class MapBasedMethodDefinitionSource extends AbstractFallbackMethodDefini * * @return the attributes explicitly defined against this bean */ - public Collection getConfigAttributeDefinitions() { - List configAttrs = new ArrayList(methodMap.values().size()); - - for(List attrList : methodMap.values()) { - configAttrs.add(new ConfigAttributeDefinition(attrList)); - } - - return configAttrs; + public Collection> getConfigAttributeDefinitions() { + return methodMap.values(); } /** diff --git a/core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSource.java b/core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSource.java index fd97377067..ea928073fd 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSource.java @@ -16,8 +16,9 @@ package org.springframework.security.intercept.method; import java.lang.reflect.Method; +import java.util.List; -import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.ConfigAttribute; import org.springframework.security.intercept.ObjectDefinitionSource; @@ -29,5 +30,5 @@ import org.springframework.security.intercept.ObjectDefinitionSource; * @version $Id$ */ public interface MethodDefinitionSource extends ObjectDefinitionSource { - public ConfigAttributeDefinition getAttributes(Method method, Class targetClass); + public List getAttributes(Method method, Class targetClass); } diff --git a/core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditor.java b/core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditor.java index 15ddc8c94e..203052c891 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditor.java +++ b/core/src/main/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditor.java @@ -36,9 +36,11 @@ import java.util.LinkedHashMap; /** * Property editor to assist with the setup of a {@link MethodDefinitionSource}. - *

The class creates and populates a {@link MapBasedMethodDefinitionSource}.

+ *

+ * The class creates and populates a {@link MapBasedMethodDefinitionSource}. * * @author Ben Alex + * @deprecated use method annotations or the protect-pointcut support from the namespace * @version $Id$ */ public class MethodDefinitionSourceEditor extends PropertyEditorSupport { diff --git a/core/src/main/java/org/springframework/security/intercept/method/MethodInvocationPrivilegeEvaluator.java b/core/src/main/java/org/springframework/security/intercept/method/MethodInvocationPrivilegeEvaluator.java index ce105dd24f..e14b960dbe 100644 --- a/core/src/main/java/org/springframework/security/intercept/method/MethodInvocationPrivilegeEvaluator.java +++ b/core/src/main/java/org/springframework/security/intercept/method/MethodInvocationPrivilegeEvaluator.java @@ -15,8 +15,11 @@ package org.springframework.security.intercept.method; +import java.util.List; + import org.springframework.security.AccessDeniedException; import org.springframework.security.Authentication; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.intercept.AbstractSecurityInterceptor; @@ -60,7 +63,7 @@ public class MethodInvocationPrivilegeEvaluator implements InitializingBean { Assert.notNull(mi, "MethodInvocation required"); Assert.notNull(mi.getMethod(), "MethodInvocation must provide a non-null getMethod()"); - ConfigAttributeDefinition attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(mi); + List attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(mi); if (attrs == null) { if (securityInterceptor.isRejectPublicInvocations()) { @@ -76,7 +79,7 @@ public class MethodInvocationPrivilegeEvaluator implements InitializingBean { } try { - securityInterceptor.getAccessDecisionManager().decide(authentication, mi, attrs); + securityInterceptor.getAccessDecisionManager().decide(authentication, mi, new ConfigAttributeDefinition(attrs)); } catch (AccessDeniedException unauthorized) { if (logger.isDebugEnabled()) { logger.debug(mi.toString() + " denied for " + authentication.toString(), unauthorized); 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 index 9bb386a7ee..083675bf96 100644 --- a/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSource.java @@ -15,12 +15,15 @@ package org.springframework.security.intercept.web; +import org.springframework.security.ConfigAttribute; 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.Collection; +import java.util.List; import java.util.Map; import java.util.LinkedHashMap; import java.util.Iterator; @@ -28,7 +31,6 @@ import java.util.HashMap; import java.util.Set; import java.util.HashSet; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; @@ -47,7 +49,7 @@ import java.util.Collections; *

* 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. + * precedence over any URLs which are registered without an HTTP method. * * @author Ben Alex * @author Luke Taylor @@ -78,30 +80,27 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation DefaultFilterInvocationDefinitionSource(UrlMatcher urlMatcher) { this.urlMatcher = urlMatcher; } - + /** * 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 + * which contains a URL path and an optional HTTP method (may be null). 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 . */ - public DefaultFilterInvocationDefinitionSource(UrlMatcher urlMatcher, LinkedHashMap requestMap) { + public DefaultFilterInvocationDefinitionSource(UrlMatcher urlMatcher, + LinkedHashMap> requestMap) { this.urlMatcher = urlMatcher; - Iterator iterator = requestMap.entrySet().iterator(); - - while (iterator.hasNext()) { - Map.Entry entry = (Map.Entry) iterator.next(); - RequestKey reqKey = (RequestKey) entry.getKey(); - addSecureUrl(reqKey.getUrl(), reqKey.getMethod(), (ConfigAttributeDefinition) entry.getValue()); + for (Map.Entry> entry : requestMap.entrySet()) { + addSecureUrl(entry.getKey().getUrl(), entry.getKey().getMethod(), entry.getValue()); } } //~ Methods ======================================================================================================== - void addSecureUrl(String pattern, ConfigAttributeDefinition attr) { + void addSecureUrl(String pattern, List attr) { addSecureUrl(pattern, null, attr); } @@ -111,7 +110,7 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation * 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. */ - void addSecureUrl(String pattern, String method, ConfigAttributeDefinition attr) { + void addSecureUrl(String pattern, String method, List attr) { Map mapToUse = getRequestMapForHttpMethod(method); mapToUse.put(urlMatcher.compile(pattern), attr); @@ -145,11 +144,11 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation return methodRequestmap; } - public Collection getConfigAttributeDefinitions() { + public Collection> getConfigAttributeDefinitions() { return Collections.unmodifiableCollection(getRequestMap().values()); } - public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException { + public List getAttributes(Object object) throws IllegalArgumentException { if ((object == null) || !this.supports(object.getClass())) { throw new IllegalArgumentException("Object must be a FilterInvocation"); } @@ -160,7 +159,7 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation return lookupAttributes(url, method); } - protected ConfigAttributeDefinition lookupAttributes(String url) { + protected List lookupAttributes(String url) { return lookupAttributes(url, null); } @@ -179,14 +178,14 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation * @return the ConfigAttributeDefinition that applies to the specified FilterInvocation * or null if no match is foud */ - public ConfigAttributeDefinition lookupAttributes(String url, String method) { + public List 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()) { @@ -197,7 +196,7 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation } } - ConfigAttributeDefinition attributes = null; + List attributes = null; Map methodSpecificMap = (Map) httpMethodMap.get(method); @@ -212,20 +211,19 @@ public class DefaultFilterInvocationDefinitionSource implements FilterInvocation return attributes; } - private ConfigAttributeDefinition lookupUrlInMap(Map requestMap, String url) { - Iterator entries = requestMap.entrySet().iterator(); + private List lookupUrlInMap(Map> requestMap, + String url) { - while (entries.hasNext()) { - Map.Entry entry = (Map.Entry) entries.next(); + for (Map.Entry> entry : requestMap.entrySet()) { Object p = entry.getKey(); - boolean matched = urlMatcher.pathMatchesUrl(p, url); + boolean matched = urlMatcher.pathMatchesUrl(entry.getKey(), url); if (logger.isDebugEnabled()) { logger.debug("Candidate is: '" + url + "'; pattern is " + p + "; matched=" + matched); } if (matched) { - return (ConfigAttributeDefinition) entry.getValue(); + return entry.getValue(); } } 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 65b68af3e7..0709695432 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 @@ -3,7 +3,6 @@ package org.springframework.security.intercept.web; import org.springframework.context.ApplicationContext; import org.springframework.util.Assert; import org.springframework.security.ConfigAttribute; -import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.util.FilterChainProxy; import org.springframework.security.util.UrlMatcher; @@ -38,13 +37,11 @@ public class FIDSToFilterChainMapConverter { while (paths.hasNext()) { Object entry = paths.next(); String path = entry instanceof Pattern ? ((Pattern)entry).pattern() : (String)entry; - ConfigAttributeDefinition configAttributeDefinition = (ConfigAttributeDefinition) requestMap.get(entry); + List configAttributeDefinition = (List) requestMap.get(entry); List filters = new ArrayList(); - Iterator attributes = configAttributeDefinition.getConfigAttributes().iterator(); - while (attributes.hasNext()) { - ConfigAttribute attr = (ConfigAttribute) attributes.next(); + for(ConfigAttribute attr : configAttributeDefinition) { String filterName = attr.getAttribute(); Assert.notNull(filterName, "Configuration attribute: '" + attr + "' returned null to the getAttribute() " + 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 c5bf09cba0..5c19c7f837 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 @@ -25,7 +25,7 @@ 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.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.SecurityConfig; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -167,7 +167,7 @@ public class FilterInvocationDefinitionSourceEditor extends PropertyEditorSuppor String[] tokens = StringUtils.commaDelimitedListToStringArray(value); - urlMap.put(new RequestKey(name), new ConfigAttributeDefinition(tokens)); + urlMap.put(new RequestKey(name), SecurityConfig.createList(tokens)); } DefaultFilterInvocationDefinitionSource fids = diff --git a/core/src/main/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluator.java b/core/src/main/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluator.java index 794197b07e..89776d793f 100644 --- a/core/src/main/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluator.java +++ b/core/src/main/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluator.java @@ -15,8 +15,11 @@ package org.springframework.security.intercept.web; +import java.util.List; + import org.springframework.security.AccessDeniedException; import org.springframework.security.Authentication; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.intercept.AbstractSecurityInterceptor; @@ -53,7 +56,7 @@ public class WebInvocationPrivilegeEvaluator implements InitializingBean { public boolean isAllowed(FilterInvocation fi, Authentication authentication) { Assert.notNull(fi, "FilterInvocation required"); - ConfigAttributeDefinition attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(fi); + List attrs = securityInterceptor.obtainObjectDefinitionSource().getAttributes(fi); if (attrs == null) { if (securityInterceptor.isRejectPublicInvocations()) { @@ -69,7 +72,7 @@ public class WebInvocationPrivilegeEvaluator implements InitializingBean { } try { - securityInterceptor.getAccessDecisionManager().decide(authentication, fi, attrs); + securityInterceptor.getAccessDecisionManager().decide(authentication, fi, new ConfigAttributeDefinition(attrs)); } catch (AccessDeniedException unauthorized) { if (logger.isDebugEnabled()) { logger.debug(fi.toString() + " denied for " + authentication.toString(), unauthorized); diff --git a/core/src/main/java/org/springframework/security/securechannel/ChannelProcessingFilter.java b/core/src/main/java/org/springframework/security/securechannel/ChannelProcessingFilter.java index fa95416b67..cdc117839a 100644 --- a/core/src/main/java/org/springframework/security/securechannel/ChannelProcessingFilter.java +++ b/core/src/main/java/org/springframework/security/securechannel/ChannelProcessingFilter.java @@ -31,6 +31,7 @@ import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Set; import java.util.Collection; @@ -59,7 +60,7 @@ public class ChannelProcessingFilter extends SpringSecurityFilter implements Ini Assert.notNull(filterInvocationDefinitionSource, "filterInvocationDefinitionSource must be specified"); Assert.notNull(channelDecisionManager, "channelDecisionManager must be specified"); - Collection attrDefs = this.filterInvocationDefinitionSource.getConfigAttributeDefinitions(); + Collection> attrDefs = this.filterInvocationDefinitionSource.getConfigAttributeDefinitions(); if (attrDefs == null) { if (logger.isWarnEnabled()) { @@ -70,16 +71,10 @@ public class ChannelProcessingFilter extends SpringSecurityFilter implements Ini return; } - Iterator iter = attrDefs.iterator(); Set set = new HashSet(); - while (iter.hasNext()) { - ConfigAttributeDefinition def = (ConfigAttributeDefinition) iter.next(); - Iterator attributes = def.getConfigAttributes().iterator(); - - while (attributes.hasNext()) { - ConfigAttribute attr = (ConfigAttribute) attributes.next(); - + for (List def : attrDefs) { + for (ConfigAttribute attr : def) { if (!this.channelDecisionManager.supports(attr)) { set.add(attr); } @@ -99,14 +94,14 @@ public class ChannelProcessingFilter extends SpringSecurityFilter implements Ini throws IOException, ServletException { FilterInvocation fi = new FilterInvocation(request, response, chain); - ConfigAttributeDefinition attr = this.filterInvocationDefinitionSource.getAttributes(fi); + List attr = this.filterInvocationDefinitionSource.getAttributes(fi); if (attr != null) { if (logger.isDebugEnabled()) { - logger.debug("Request: " + fi.toString() + "; ConfigAttributes: " + attr.toString()); + logger.debug("Request: " + fi.toString() + "; ConfigAttributes: " + attr); } - channelDecisionManager.decide(fi, attr); + channelDecisionManager.decide(fi, new ConfigAttributeDefinition(attr)); if (fi.getResponse().isCommitted()) { return; diff --git a/core/src/test/java/org/springframework/security/ConfigAttributeEditorTests.java b/core/src/test/java/org/springframework/security/ConfigAttributeEditorTests.java deleted file mode 100644 index 66440c65e8..0000000000 --- a/core/src/test/java/org/springframework/security/ConfigAttributeEditorTests.java +++ /dev/null @@ -1,138 +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; - -import junit.framework.TestCase; - -import java.util.ArrayList; -import java.util.Iterator; - - -/** - * Tests {@link ConfigAttributeEditor} and associated {@link ConfigAttributeDefinition}. - * - * @author Ben Alex - * @version $Id$ - */ -public class ConfigAttributeEditorTests extends TestCase { - private static final String[] ATTRIBUTES = new String[] {"A", "B"}; - - //~ Constructors =================================================================================================== - - public ConfigAttributeEditorTests() { - super(); - } - - public ConfigAttributeEditorTests(String arg0) { - super(arg0); - } - - //~ Methods ======================================================================================================== - - public void testCorrectOperation() { - ConfigAttributeEditor editor = new ConfigAttributeEditor(); - editor.setAsText("HELLO,DOCTOR,NAME,YESTERDAY,TOMORROW"); - - ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue(); - Iterator iter = result.getConfigAttributes().iterator(); - int position = 0; - - while (iter.hasNext()) { - position++; - iter.next(); - } - - assertEquals(5, position); - - assertEquals(5, result.getConfigAttributes().size()); - - assertTrue(result.contains(new SecurityConfig("HELLO"))); - assertTrue(result.contains(new SecurityConfig("TOMORROW"))); - assertFalse(result.contains(new SecurityConfig("FOOBAR"))); - } - - public void testEmptyStringReturnsNull() { - ConfigAttributeEditor editor = new ConfigAttributeEditor(); - editor.setAsText(""); - - ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue(); - assertTrue(result == null); - } - - public void testEqualsHandlingWhenDifferentObjectTypes() { - ConfigAttributeDefinition def1 = new ConfigAttributeDefinition(ATTRIBUTES); - - assertTrue(!def1.equals("A_STRING")); - } - - public void testEqualsHandlingWhenExactlyEqual() { - ConfigAttributeDefinition def1 = new ConfigAttributeDefinition(ATTRIBUTES); - ConfigAttributeDefinition def2 = new ConfigAttributeDefinition(ATTRIBUTES); - - assertEquals(def1, def2); - } - - public void testEqualsHandlingWhenOrderingNotEqual() { - ConfigAttributeDefinition def1 = new ConfigAttributeDefinition(ATTRIBUTES); - ConfigAttributeDefinition def2 = new ConfigAttributeDefinition(new String[] {"B", "A"}); - - assertFalse(def1.equals(def2)); - } - - public void testEqualsHandlingWhenTestObjectHasNoAttributes() { - ConfigAttributeDefinition def1 = new ConfigAttributeDefinition(ATTRIBUTES); - ConfigAttributeDefinition def2 = new ConfigAttributeDefinition(new String[] {}); - - assertFalse(def1.equals(def2)); - assertFalse(def2.equals(def1)); - } - - public void testNullReturnsNull() { - ConfigAttributeEditor editor = new ConfigAttributeEditor(); - editor.setAsText(null); - - ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue(); - assertTrue(result == null); - } - - public void testStripsTrailingAndLeadingSpaces() { - ConfigAttributeEditor editor = new ConfigAttributeEditor(); - editor.setAsText(" HELLO, DOCTOR,NAME, YESTERDAY ,TOMORROW "); - - ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue(); - Iterator iter = result.getConfigAttributes().iterator(); - - ArrayList list = new ArrayList(); - - while (iter.hasNext()) { - list.add(iter.next()); - } - - assertEquals("HELLO", ((ConfigAttribute) list.get(0)).getAttribute()); - assertEquals("DOCTOR", ((ConfigAttribute) list.get(1)).getAttribute()); - assertEquals("NAME", ((ConfigAttribute) list.get(2)).getAttribute()); - assertEquals("YESTERDAY", ((ConfigAttribute) list.get(3)).getAttribute()); - assertEquals("TOMORROW", ((ConfigAttribute) list.get(4)).getAttribute()); - } - - public void testToString() { - ConfigAttributeEditor editor = new ConfigAttributeEditor(); - editor.setAsText("KOALA,KANGAROO,EMU,WOMBAT"); - - ConfigAttributeDefinition result = (ConfigAttributeDefinition) editor.getValue(); - assertEquals("[KOALA, KANGAROO, EMU, WOMBAT]", result.toString()); - } -} diff --git a/core/src/test/java/org/springframework/security/annotation/ExpressionProtectedBusinessServiceImpl.java b/core/src/test/java/org/springframework/security/annotation/ExpressionProtectedBusinessServiceImpl.java index 56ca1001f9..d5ad568e65 100644 --- a/core/src/test/java/org/springframework/security/annotation/ExpressionProtectedBusinessServiceImpl.java +++ b/core/src/test/java/org/springframework/security/annotation/ExpressionProtectedBusinessServiceImpl.java @@ -28,7 +28,7 @@ public class ExpressionProtectedBusinessServiceImpl implements BusinessService { public void someUserMethod2() { } - @PreFilter(filterTarget="someList", value="filterObject == name or filterObject == 'sam'") + @PreFilter(filterTarget="someList", value="filterObject == authentication.name or filterObject == 'sam'") @PostFilter("filterObject == 'bob'") public List methodReturningAList(List someList) { return someList; @@ -38,7 +38,6 @@ public class ExpressionProtectedBusinessServiceImpl implements BusinessService { return new ArrayList(); } - @PreFilter(filterTarget="someArray", value="filterObject == name or filterObject == 'sam'") @PostFilter("filterObject == 'bob'") public Object[] methodReturningAnArray(Object[] someArray) { return someArray; diff --git a/core/src/test/java/org/springframework/security/annotation/MethodDefinitionSourceEditorTigerTests.java b/core/src/test/java/org/springframework/security/annotation/MethodDefinitionSourceEditorTigerTests.java index 55d1ffe47e..c367588b7d 100644 --- a/core/src/test/java/org/springframework/security/annotation/MethodDefinitionSourceEditorTigerTests.java +++ b/core/src/test/java/org/springframework/security/annotation/MethodDefinitionSourceEditorTigerTests.java @@ -15,18 +15,22 @@ package org.springframework.security.annotation; -import java.lang.reflect.AccessibleObject; -import java.lang.reflect.Method; +import static org.junit.Assert.*; + +import java.util.List; import junit.framework.TestCase; -import org.aopalliance.intercept.MethodInvocation; -import org.springframework.security.ConfigAttributeDefinition; +import org.junit.Before; +import org.junit.Test; +import org.springframework.security.ConfigAttribute; +import org.springframework.security.SecurityConfig; import org.springframework.security.annotation.test.Entity; import org.springframework.security.annotation.test.PersonServiceImpl; import org.springframework.security.annotation.test.Service; import org.springframework.security.intercept.method.MapBasedMethodDefinitionSource; import org.springframework.security.intercept.method.MethodDefinitionSourceEditor; +import org.springframework.security.intercept.method.MockMethodInvocation; /** @@ -35,9 +39,17 @@ import org.springframework.security.intercept.method.MethodDefinitionSourceEdito * @author Ben Alex * @version $Id$ */ -public class MethodDefinitionSourceEditorTigerTests extends TestCase { - //~ Methods ======================================================================================================== +public class MethodDefinitionSourceEditorTigerTests { + private MockMethodInvocation makeUpper; + private MockMethodInvocation makeLower; + @Before + public void createMethodInvocations() throws Exception { + makeUpper = new MockMethodInvocation(new PersonServiceImpl(), Service.class,"makeUpperCase", Entity.class); + makeLower = new MockMethodInvocation(new PersonServiceImpl(), Service.class,"makeLowerCase", Entity.class); + } + + @Test public void testConcreteClassInvocations() throws Exception { MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor(); editor.setAsText( @@ -48,64 +60,29 @@ public class MethodDefinitionSourceEditorTigerTests extends TestCase { MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue(); assertEquals(3, map.getMethodMapSize()); - ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(Service.class, - "makeLowerCase", new Class[]{Entity.class}, new PersonServiceImpl())); - ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_FROM_INTERFACE"); + List returnedMakeLower = map.getAttributes(makeLower); + List expectedMakeLower = SecurityConfig.createList("ROLE_FROM_INTERFACE"); assertEquals(expectedMakeLower, returnedMakeLower); - ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(Service.class, - "makeUpperCase", new Class[]{Entity.class}, new PersonServiceImpl())); - ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_IMPLEMENTATION"}); + List returnedMakeUpper = map.getAttributes(makeUpper); + List expectedMakeUpper = SecurityConfig.createList(new String[]{"ROLE_FROM_IMPLEMENTATION"}); assertEquals(expectedMakeUpper, returnedMakeUpper); } + @Test public void testBridgeMethodResolution() throws Exception { MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor(); editor.setAsText( "org.springframework.security.annotation.test.PersonService.makeUpper*=ROLE_FROM_INTERFACE\r\n" + "org.springframework.security.annotation.test.ServiceImpl.makeUpper*=ROLE_FROM_ABSTRACT\r\n" + - "org.springframework.security.annotation.test.PersonServiceImpl.makeUpper*=ROLE_FROM_PSI"); + "org.springframework.security.annotation.test.PersonServiceImpl.makeUpper*=ROLE_FROM_PSI"); MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue(); assertEquals(3, map.getMethodMapSize()); - ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(Service.class, - "makeUpperCase", new Class[]{Entity.class}, new PersonServiceImpl())); - ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_PSI"}); + List returnedMakeUpper = map.getAttributes(makeUpper); + List expectedMakeUpper = SecurityConfig.createList("ROLE_FROM_PSI"); assertEquals(expectedMakeUpper, returnedMakeUpper); } - //~ Inner Classes ================================================================================================== - - private class MockMethodInvocation implements MethodInvocation { - private Method method; - private Object targetObject; - - public MockMethodInvocation(Class clazz, String methodName, Class[] parameterTypes, Object targetObject) - throws NoSuchMethodException { - this.method = clazz.getMethod(methodName, parameterTypes); - this.targetObject = targetObject; - } - - public Object[] getArguments() { - return null; - } - - public Method getMethod() { - return method; - } - - public AccessibleObject getStaticPart() { - return null; - } - - public Object getThis() { - return targetObject; - } - - public Object proceed() throws Throwable { - return null; - } - } - } diff --git a/core/src/test/java/org/springframework/security/config/FilterInvocationDefinitionSourceParserTests.java b/core/src/test/java/org/springframework/security/config/FilterInvocationDefinitionSourceParserTests.java index d1ef57d295..6950a401b3 100644 --- a/core/src/test/java/org/springframework/security/config/FilterInvocationDefinitionSourceParserTests.java +++ b/core/src/test/java/org/springframework/security/config/FilterInvocationDefinitionSourceParserTests.java @@ -2,12 +2,15 @@ package org.springframework.security.config; import static org.junit.Assert.*; +import java.util.List; + import org.junit.After; import org.junit.Test; import org.springframework.context.support.AbstractXmlApplicationContext; import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.SecurityConfig; import org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource; @@ -41,7 +44,7 @@ public class FilterInvocationDefinitionSourceParserTests { " " + ""); DefaultFilterInvocationDefinitionSource fids = (DefaultFilterInvocationDefinitionSource) appContext.getBean("fids"); - ConfigAttributeDefinition cad = fids.getAttributes(createFilterInvocation("/anything", "GET")); + List cad = fids.getAttributes(createFilterInvocation("/anything", "GET")); assertTrue(cad.contains(new SecurityConfig("ROLE_A"))); } diff --git a/core/src/test/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParserTests.java b/core/src/test/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParserTests.java index c55ec7c8e7..41a4f4da65 100644 --- a/core/src/test/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParserTests.java +++ b/core/src/test/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParserTests.java @@ -214,7 +214,7 @@ public class GlobalMethodSecurityBeanDefinitionParserTests { } @Test - public void preAndPostFilterAnnotationsWorkWithArrays() { + public void prePostFilterAnnotationWorksWithArrays() { setContext( "" + "" + 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 cf08193a25..9b1580cfa0 100644 --- a/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java +++ b/core/src/test/java/org/springframework/security/config/HttpSecurityBeanDefinitionParserTests.java @@ -16,6 +16,7 @@ import org.springframework.context.support.AbstractXmlApplicationContext; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockHttpSession; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.MockAuthenticationEntryPoint; import org.springframework.security.MockFilterChain; @@ -221,12 +222,12 @@ public class HttpSecurityBeanDefinitionParserTests { FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR); FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource(); - ConfigAttributeDefinition attrDef = fids.getAttributes(createFilterinvocation("/Secure", null)); - assertEquals(2, attrDef.getConfigAttributes().size()); + List 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.getConfigAttributes().size()); + assertEquals(1, attrDef.size()); assertTrue(attrDef.contains(new SecurityConfig("ROLE_C"))); } @@ -241,8 +242,8 @@ public class HttpSecurityBeanDefinitionParserTests { FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR); FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource(); - ConfigAttributeDefinition attrs = fids.getAttributes(createFilterinvocation("/secure", "POST")); - assertEquals(2, attrs.getConfigAttributes().size()); + List attrs = fids.getAttributes(createFilterinvocation("/secure", "POST")); + assertEquals(2, attrs.size()); assertTrue(attrs.contains(new SecurityConfig("ROLE_A"))); assertTrue(attrs.contains(new SecurityConfig("ROLE_B"))); } @@ -656,8 +657,8 @@ public class HttpSecurityBeanDefinitionParserTests { FilterSecurityInterceptor fis = (FilterSecurityInterceptor) appContext.getBean(BeanIds.FILTER_SECURITY_INTERCEPTOR); FilterInvocationDefinitionSource fids = fis.getObjectDefinitionSource(); - ConfigAttributeDefinition attrDef = fids.getAttributes(createFilterinvocation("/someurl", null)); - assertEquals(1, attrDef.getConfigAttributes().size()); + List attrDef = fids.getAttributes(createFilterinvocation("/someurl", null)); + assertEquals(1, attrDef.size()); assertTrue(attrDef.contains(new SecurityConfig("ROLE_B"))); } diff --git a/core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java b/core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java index 7d0c15947d..9eae7362a2 100644 --- a/core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java +++ b/core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java @@ -12,8 +12,6 @@ import org.junit.Before; import org.junit.Test; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.annotation.ExpressionProtectedBusinessServiceImpl; -import org.springframework.security.expression.support.AbstractExpressionBasedMethodConfigAttribute; -import org.springframework.security.expression.support.MethodExpressionVoter; import org.springframework.security.providers.TestingAuthenticationToken; import org.springframework.security.util.SimpleMethodInvocation; import org.springframework.security.vote.AccessDecisionVoter; @@ -22,7 +20,10 @@ public class MethodExpressionVoterTests { private TestingAuthenticationToken joe = new TestingAuthenticationToken("joe", "joespass", "blah"); private MethodInvocation miStringArgs; private MethodInvocation miListArg; + private MethodInvocation miArrayArg; private List listArg; + private Object[] arrayArg; + private MethodExpressionVoter am = new MethodExpressionVoter(); @Before public void setUp() throws Exception { @@ -30,42 +31,52 @@ public class MethodExpressionVoterTests { String.class, String.class); miStringArgs = new SimpleMethodInvocation(new Object(), m, new String[] {"joe", "arg2Value"}); m = ExpressionProtectedBusinessServiceImpl.class.getMethod("methodReturningAList", List.class); - listArg = new ArrayList(Arrays.asList("joe", "bob")); + listArg = new ArrayList(Arrays.asList("joe", "bob", "sam")); miListArg = new SimpleMethodInvocation(new Object(), m, new Object[] {listArg}); + m = ExpressionProtectedBusinessServiceImpl.class.getMethod("methodReturningAnArray", Object[].class); + arrayArg = new Object[] {"joe", "bob", "sam"}; + miArrayArg = new SimpleMethodInvocation(new Object(), m, new Object[] {arrayArg}); } @Test public void hasRoleExpressionAllowsUserWithRole() throws Exception { - MethodExpressionVoter am = new MethodExpressionVoter(); - ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionBasedMethodConfigAttribute(null, null, "hasRole('blah')")); - + ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute(null, null, "hasRole('blah')")); assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, miStringArgs, cad)); } @Test public void hasRoleExpressionDeniesUserWithoutRole() throws Exception { - MethodExpressionVoter am = new MethodExpressionVoter(); - ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionBasedMethodConfigAttribute(null, null, "hasRole('joedoesnt')")); - + ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute(null, null, "hasRole('joedoesnt')")); assertEquals(AccessDecisionVoter.ACCESS_DENIED, am.vote(joe, miStringArgs, cad)); } @Test public void matchingArgAgainstAuthenticationNameIsSuccessful() throws Exception { - MethodExpressionVoter am = new MethodExpressionVoter(); - ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionBasedMethodConfigAttribute(null, null, "(#userName == name) and (name == 'joe')")); - + // userName is an argument name of this method + ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute(null, null, "(#userName == principal) and (principal == 'joe')")); assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, miStringArgs, cad)); } @Test public void accessIsGrantedIfNoPreAuthorizeAttributeIsUsed() throws Exception { - MethodExpressionVoter am = new MethodExpressionVoter(); - ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionBasedMethodConfigAttribute("(name == 'jim')", "someList", null)); - + ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute("(filterObject == 'jim')", "someList", null)); assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, miListArg, cad)); // All objects should have been removed, because the expression is always false assertEquals(0, listArg.size()); } + @Test(expected=IllegalArgumentException.class) + public void arraysCannotBePrefiltered() throws Exception { + ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute("(filterObject == 'jim')", "someArray", null)); + am.vote(joe, miArrayArg, cad); + } + + @Test + public void listPreFilteringIsSuccessful() throws Exception { + ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new PreInvocationExpressionConfigAttribute("(filterObject == 'joe' or filterObject == 'sam')", "someList", null)); + am.vote(joe, miListArg, cad); + assertEquals("joe and sam should still be in the list", 2, listArg.size()); + assertEquals("joe", listArg.get(0)); + assertEquals("sam", listArg.get(1)); + } } diff --git a/core/src/test/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSourceTests.java b/core/src/test/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSourceTests.java index 53d32acc1b..7225e3d016 100644 --- a/core/src/test/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSourceTests.java +++ b/core/src/test/java/org/springframework/security/intercept/method/MapBasedMethodDefinitionSourceTests.java @@ -35,7 +35,7 @@ public class MapBasedMethodDefinitionSourceTests { public void wildcardedMatchIsOverwrittenByMoreSpecificMatch() { mds.addSecureMethod(MockService.class, "some*", ROLE_A); mds.addSecureMethod(MockService.class, "someMethod*", ROLE_B); - assertEquals(ROLE_B, mds.getAttributes(someMethodInteger, MockService.class).getConfigAttributes()); + assertEquals(ROLE_B, mds.getAttributes(someMethodInteger, MockService.class)); } @Test @@ -43,8 +43,8 @@ public class MapBasedMethodDefinitionSourceTests { mds.addSecureMethod(MockService.class, someMethodInteger, ROLE_A); mds.addSecureMethod(MockService.class, someMethodString, ROLE_B); - assertEquals(ROLE_A, mds.getAttributes(someMethodInteger, MockService.class).getConfigAttributes()); - assertEquals(ROLE_B, mds.getAttributes(someMethodString, MockService.class).getConfigAttributes()); + assertEquals(ROLE_A, mds.getAttributes(someMethodInteger, MockService.class)); + assertEquals(ROLE_B, mds.getAttributes(someMethodString, MockService.class)); } private class MockService { diff --git a/core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditorTests.java b/core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditorTests.java index c29e125d9e..a3fe917955 100644 --- a/core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditorTests.java +++ b/core/src/test/java/org/springframework/security/intercept/method/MethodDefinitionSourceEditorTests.java @@ -17,10 +17,12 @@ package org.springframework.security.intercept.method; import junit.framework.TestCase; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.ITargetObject; import org.springframework.security.MockJoinPoint; import org.springframework.security.OtherTargetObject; +import org.springframework.security.SecurityConfig; import org.springframework.security.TargetObject; import org.aopalliance.intercept.MethodInvocation; @@ -29,6 +31,7 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Method; import java.util.Iterator; +import java.util.List; /** @@ -63,10 +66,9 @@ public class MethodDefinitionSourceEditorTests extends TestCase { Method method = clazz.getMethod("countLength", new Class[] {String.class}); MockJoinPoint joinPoint = new MockJoinPoint(new TargetObject(), method); - ConfigAttributeDefinition returnedCountLength = map.getAttributes(joinPoint); + List returnedCountLength = map.getAttributes(joinPoint); - ConfigAttributeDefinition expectedCountLength = - new ConfigAttributeDefinition(new String[] {"ROLE_ONE", "ROLE_TWO", "RUN_AS_ENTRY"}); + List expectedCountLength = SecurityConfig.createList("ROLE_ONE", "ROLE_TWO", "RUN_AS_ENTRY"); assertEquals(expectedCountLength, returnedCountLength); } @@ -116,20 +118,20 @@ public class MethodDefinitionSourceEditorTests extends TestCase { MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue(); assertEquals(6, map.getMethodMapSize()); - ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new OtherTargetObject())); - ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_FROM_INTERFACE"); + List returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new OtherTargetObject())); + List expectedMakeLower = SecurityConfig.createList("ROLE_FROM_INTERFACE"); assertEquals(expectedMakeLower, returnedMakeLower); - ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeUpperCase", new Class[] {String.class}, new OtherTargetObject())); - ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_IMPLEMENTATION"}); + List returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeUpperCase", new Class[] {String.class}, new OtherTargetObject())); + List expectedMakeUpper = SecurityConfig.createList("ROLE_FROM_IMPLEMENTATION"); assertEquals(expectedMakeUpper, returnedMakeUpper); - ConfigAttributeDefinition returnedComputeHashCode = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "computeHashCode", new Class[] {String.class}, new OtherTargetObject())); - ConfigAttributeDefinition expectedComputeHashCode = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_OTO"}); + List returnedComputeHashCode = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "computeHashCode", new Class[] {String.class}, new OtherTargetObject())); + List expectedComputeHashCode = SecurityConfig.createList("ROLE_FROM_OTO"); assertEquals(expectedComputeHashCode, returnedComputeHashCode); returnedComputeHashCode = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "computeHashCode", new Class[] {String.class}, new TargetObject())); - expectedComputeHashCode = new ConfigAttributeDefinition(new String[]{"ROLE_FROM_TO"}); + expectedComputeHashCode = SecurityConfig.createList("ROLE_FROM_TO"); assertEquals(expectedComputeHashCode, returnedComputeHashCode); } @@ -175,19 +177,19 @@ public class MethodDefinitionSourceEditorTests extends TestCase { MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue(); assertEquals(14, map.getMethodMapSize()); - ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class, + List returnedMakeLower = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new TargetObject())); - ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition("ROLE_LOWER"); + List expectedMakeLower = SecurityConfig.createList("ROLE_LOWER"); assertEquals(expectedMakeLower, returnedMakeLower); - ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class, + List returnedMakeUpper = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "makeUpperCase", new Class[] {String.class}, new TargetObject())); - ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition("ROLE_UPPER"); + List expectedMakeUpper = SecurityConfig.createList("ROLE_UPPER"); assertEquals(expectedMakeUpper, returnedMakeUpper); - ConfigAttributeDefinition returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class, + List returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "countLength", new Class[] {String.class}, new TargetObject())); - ConfigAttributeDefinition expectedCountLength = new ConfigAttributeDefinition("ROLE_GENERAL"); + List expectedCountLength = SecurityConfig.createList("ROLE_GENERAL"); assertEquals(expectedCountLength, returnedCountLength); } @@ -197,7 +199,7 @@ public class MethodDefinitionSourceEditorTests extends TestCase { MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue(); - ConfigAttributeDefinition configAttributeDefinition = map.getAttributes(new MockMethodInvocation( + List configAttributeDefinition = map.getAttributes(new MockMethodInvocation( ITargetObject.class, "makeLowerCase", new Class[] {String.class}, new TargetObject())); assertNull(configAttributeDefinition); } @@ -216,11 +218,9 @@ public class MethodDefinitionSourceEditorTests extends TestCase { MapBasedMethodDefinitionSource map = (MapBasedMethodDefinitionSource) editor.getValue(); - ConfigAttributeDefinition returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class, + List returnedCountLength = map.getAttributes(new MockMethodInvocation(ITargetObject.class, "countLength", new Class[] {String.class}, new TargetObject())); - ConfigAttributeDefinition expectedCountLength = new ConfigAttributeDefinition( - new String[] {"ROLE_ONE", "ROLE_TWO", "RUN_AS_ENTRY"}); - assertEquals(expectedCountLength, returnedCountLength); + assertEquals(SecurityConfig.createList("ROLE_ONE", "ROLE_TWO", "RUN_AS_ENTRY"), returnedCountLength); } //~ Inner Classes ================================================================================================== diff --git a/core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java b/core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java index f90c943d73..1c0ac7542b 100644 --- a/core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java +++ b/core/src/test/java/org/springframework/security/intercept/method/MockMethodDefinitionSource.java @@ -17,13 +17,15 @@ package org.springframework.security.intercept.method; import org.aopalliance.intercept.MethodInvocation; import org.aspectj.lang.JoinPoint; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.SecurityConfig; import java.lang.reflect.Method; +import java.util.Collection; import java.util.List; import java.util.Vector; -import java.util.Collection; /** @@ -43,38 +45,38 @@ public class MockMethodDefinitionSource implements MethodDefinitionSource { returnACollection = returnACollectionWhenRequested; list = new Vector(); - ConfigAttributeDefinition def1 = new ConfigAttributeDefinition("MOCK_LOWER"); + List def1 = SecurityConfig.createList("MOCK_LOWER"); list.add(def1); if (includeInvalidAttributes) { - ConfigAttributeDefinition def2 = new ConfigAttributeDefinition(new String[] {"MOCK_LOWER","INVALID_ATTRIBUTE"}); + List def2 = SecurityConfig.createList("MOCK_LOWER","INVALID_ATTRIBUTE"); list.add(def2); } - ConfigAttributeDefinition def3 = new ConfigAttributeDefinition(new String[] {"MOCK_UPPER", "RUN_AS_"}); + List def3 = SecurityConfig.createList("MOCK_UPPER", "RUN_AS_"); list.add(def3); if (includeInvalidAttributes) { - ConfigAttributeDefinition def4 = new ConfigAttributeDefinition(new String[] {"MOCK_SOMETHING", "ANOTHER_INVALID"}); + List def4 = SecurityConfig.createList("MOCK_SOMETHING", "ANOTHER_INVALID"); list.add(def4); } } //~ Methods ======================================================================================================== - public Collection getConfigAttributeDefinitions() { + public Collection> getConfigAttributeDefinitions() { if (returnACollection) { return list; } else { return null; - } + } } - public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException { + public List getAttributes(Object object) throws IllegalArgumentException { throw new UnsupportedOperationException("mock method not implemented"); } - public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) { + public List getAttributes(Method method, Class targetClass) { throw new UnsupportedOperationException("mock method not implemented"); } diff --git a/core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodSecurityInterceptorTests.java b/core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodSecurityInterceptorTests.java index 1f07e6e4ae..e5c3c35501 100644 --- a/core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodSecurityInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/intercept/method/aopalliance/MethodSecurityInterceptorTests.java @@ -49,6 +49,7 @@ import org.springframework.context.support.ClassPathXmlApplicationContext; import java.lang.reflect.Method; import java.util.Collection; +import java.util.List; /** @@ -359,8 +360,7 @@ public class MethodSecurityInterceptorTests extends TestCase { } } - public void testValidationFailsIfInvalidAttributePresented() - throws Exception { + public void testValidationFailsIfInvalidAttributePresented() throws Exception { MethodSecurityInterceptor si = new MethodSecurityInterceptor(); si.setAccessDecisionManager(new MockAccessDecisionManager()); si.setAuthenticationManager(new MockAuthenticationManager()); @@ -447,11 +447,11 @@ public class MethodSecurityInterceptorTests extends TestCase { } private class MockObjectDefinitionSourceWhichOnlySupportsStrings implements MethodDefinitionSource { - public Collection getConfigAttributeDefinitions() { + public Collection> getConfigAttributeDefinitions() { return null; } - public ConfigAttributeDefinition getAttributes(Method method, Class targetClass) { + public List getAttributes(Method method, Class targetClass) { throw new UnsupportedOperationException("mock method not implemented"); } @@ -463,7 +463,7 @@ public class MethodSecurityInterceptorTests extends TestCase { } } - public ConfigAttributeDefinition getAttributes(Object object) { + public List getAttributes(Object object) { throw new UnsupportedOperationException("mock method not implemented"); } } diff --git a/core/src/test/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSourceTests.java b/core/src/test/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSourceTests.java index f78304a836..6c322b2c61 100644 --- a/core/src/test/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSourceTests.java +++ b/core/src/test/java/org/springframework/security/intercept/web/DefaultFilterInvocationDefinitionSourceTests.java @@ -15,17 +15,22 @@ package org.springframework.security.intercept.web; -import org.springframework.security.ConfigAttributeDefinition; -import org.springframework.security.MockFilterChain; -import org.springframework.security.util.AntUrlPathMatcher; -import org.springframework.security.util.InMemoryXmlApplicationContext; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; - -import org.junit.Test; -import org.junit.Before; -import static org.junit.Assert.*; +import org.springframework.security.ConfigAttribute; +import org.springframework.security.MockFilterChain; +import org.springframework.security.SecurityConfig; +import org.springframework.security.util.AntUrlPathMatcher; +import org.springframework.security.util.InMemoryXmlApplicationContext; /** * Tests parts of {@link DefaultFilterInvocationDefinitionSource} not tested by {@link @@ -35,7 +40,8 @@ import static org.junit.Assert.*; * @version $Id$ */ public class DefaultFilterInvocationDefinitionSourceTests { - DefaultFilterInvocationDefinitionSource map; + private DefaultFilterInvocationDefinitionSource map; + private List def = SecurityConfig.createList("ROLE_ONE"); //~ Methods ======================================================================================================== @Before @@ -51,13 +57,11 @@ public class DefaultFilterInvocationDefinitionSourceTests { @Test public void lookupNotRequiringExactMatchSuccessIfNotMatching() { - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); map.addSecureUrl("/secure/super/**", def); FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null); - ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); - assertEquals(def, response); + assertEquals(def, map.lookupAttributes(fi.getRequestUrl())); } /** @@ -65,12 +69,11 @@ public class DefaultFilterInvocationDefinitionSourceTests { */ @Test public void lookupNotRequiringExactMatchSucceedsIfSecureUrlPathContainsUpperCase() { - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); map.addSecureUrl("/SeCuRE/super/**", def); FilterInvocation fi = createFilterInvocation("/secure/super/somefile.html", null); - ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); + List response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); } @@ -78,75 +81,69 @@ public class DefaultFilterInvocationDefinitionSourceTests { @Test public void lookupRequiringExactMatchFailsIfNotMatching() { map = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher(false)); - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); map.addSecureUrl("/secure/super/**", def); FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null); - ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); + List response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(null, response); } @Test public void lookupRequiringExactMatchIsSuccessful() { map = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher(false)); - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); map.addSecureUrl("/SeCurE/super/**", def); FilterInvocation fi = createFilterInvocation("/SeCurE/super/somefile.html", null); - ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); + List response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); } @Test public void lookupRequiringExactMatchWithAdditionalSlashesIsSuccessful() { - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); map.addSecureUrl("/someAdminPage.html**", def); FilterInvocation fi = createFilterInvocation("/someAdminPage.html?a=/test", null); - ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); + List response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); // see SEC-161 (it should truncate after ? sign) } @Test(expected = IllegalArgumentException.class) public void unknownHttpMethodIsRejected() { - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); map.addSecureUrl("/someAdminPage.html**", "UNKNOWN", def); } @Test public void httpMethodLookupSucceeds() { - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); map.addSecureUrl("/somepage**", "GET", def); FilterInvocation fi = createFilterInvocation("/somepage", "GET"); - ConfigAttributeDefinition attrs = map.getAttributes(fi); + List attrs = map.getAttributes(fi); assertEquals(def, attrs); } @Test public void requestWithDifferentHttpMethodDoesntMatch() { - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); map.addSecureUrl("/somepage**", "GET", def); FilterInvocation fi = createFilterInvocation("/somepage", null); - ConfigAttributeDefinition attrs = map.getAttributes(fi); + List attrs = map.getAttributes(fi); assertNull(attrs); } @Test public void httpMethodSpecificUrlTakesPrecedence() { // Even though this is added before the method-specific def, the latter should match - ConfigAttributeDefinition allMethodDef = new ConfigAttributeDefinition("ROLE_ONE"); - map.addSecureUrl("/**", null, allMethodDef); + List allMethodDef = def; + map.addSecureUrl("/**", null, def); - ConfigAttributeDefinition postOnlyDef = new ConfigAttributeDefinition("ROLE_TWO"); + List postOnlyDef = SecurityConfig.createList("ROLE_TWO"); map.addSecureUrl("/somepage**", "POST", postOnlyDef); FilterInvocation fi = createFilterInvocation("/somepage", "POST"); - ConfigAttributeDefinition attrs = map.getAttributes(fi); + List attrs = map.getAttributes(fi); assertEquals(postOnlyDef, attrs); } @@ -154,13 +151,12 @@ public class DefaultFilterInvocationDefinitionSourceTests { * Check fixes for SEC-321 */ @Test - public void extraQuestionMarkStillMatches() { - ConfigAttributeDefinition def = new ConfigAttributeDefinition("ROLE_ONE"); + public void extraQuestionMarkStillMatches() { map.addSecureUrl("/someAdminPage.html*", def); FilterInvocation fi = createFilterInvocation("/someAdminPage.html?x=2/aa?y=3", null); - ConfigAttributeDefinition response = map.lookupAttributes(fi.getRequestUrl()); + List response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); fi = createFilterInvocation("/someAdminPage.html??", null); @@ -168,13 +164,13 @@ public class DefaultFilterInvocationDefinitionSourceTests { response = map.lookupAttributes(fi.getRequestUrl()); assertEquals(def, response); } - + @Test public void xmlMapConfigurationIsSuccessful() { InMemoryXmlApplicationContext context = new InMemoryXmlApplicationContext( "" + " " + - " " + + " " + " " + " " + " " + @@ -193,11 +189,11 @@ public class DefaultFilterInvocationDefinitionSourceTests { " " + "" ); - + DefaultFilterInvocationDefinitionSource fids = (DefaultFilterInvocationDefinitionSource) context.getBean("fids"); - ConfigAttributeDefinition cad = fids.lookupAttributes("/anything", "GET"); + List cad = fids.lookupAttributes("/anything", "GET"); assertNotNull(cad); - assertEquals(1, cad.getConfigAttributes().size()); + assertEquals(1, cad.size()); context.close(); } 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 0dcdd6bfb0..05013edca5 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 @@ -17,8 +17,9 @@ package org.springframework.security.intercept.web; import junit.framework.TestCase; -import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.ConfigAttribute; import org.springframework.security.MockFilterChain; +import org.springframework.security.SecurityConfig; import org.springframework.security.util.RegexUrlPathMatcher; import org.springframework.security.util.AntUrlPathMatcher; @@ -26,6 +27,7 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import java.util.Iterator; +import java.util.List; import java.util.regex.PatternSyntaxException; @@ -164,7 +166,7 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/totally/different/path/index.html"); - ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest, + List returned = map.getAttributes(new FilterInvocation(httpRequest, new MockHttpServletResponse(), new MockFilterChain())); assertEquals(null, returned); @@ -197,13 +199,10 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); - ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest, + List returned = map.getAttributes(new FilterInvocation(httpRequest, new MockHttpServletResponse(), new MockFilterChain())); - ConfigAttributeDefinition expected = new ConfigAttributeDefinition( - new String[] {"ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"}); - - assertEquals(expected, returned); + assertEquals(SecurityConfig.createList("ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"), returned); } public void testOrderOfEntriesIsPreservedOrderB() { @@ -216,11 +215,10 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); - ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest, + List returned = map.getAttributes(new FilterInvocation(httpRequest, new MockHttpServletResponse(), new MockFilterChain())); - ConfigAttributeDefinition expected = new ConfigAttributeDefinition(new String[] {"ROLE_SUPERVISOR", "ROLE_TELLER"}); - assertEquals(expected, returned); + assertEquals(SecurityConfig.createList("ROLE_SUPERVISOR", "ROLE_TELLER"), returned); } public void testSingleUrlParsingWithRegularExpressions() throws Exception { @@ -232,9 +230,9 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); - ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest, + List returned = map.getAttributes(new FilterInvocation(httpRequest, new MockHttpServletResponse(), new MockFilterChain())); - ConfigAttributeDefinition expected = new ConfigAttributeDefinition(new String[] {"ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"}); + List expected = SecurityConfig.createList("ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"); assertEquals(expected, returned); } @@ -248,11 +246,10 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); - ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest, + List returned = map.getAttributes(new FilterInvocation(httpRequest, new MockHttpServletResponse(), new MockFilterChain())); - ConfigAttributeDefinition expected = new ConfigAttributeDefinition(new String[] {"ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"}); - assertEquals(expected, returned); + assertEquals(SecurityConfig.createList("ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"), returned); } public void testWhitespaceAndCommentsAndLinesWithoutEqualsSignsAreIgnored() { @@ -292,9 +289,9 @@ public class FilterInvocationDefinitionSourceEditorTests extends TestCase { MockHttpServletRequest httpRequest = new MockHttpServletRequest(null, null); httpRequest.setServletPath("/secure/super/very_secret.html"); - ConfigAttributeDefinition returned = map.getAttributes(new FilterInvocation(httpRequest, + List returned = map.getAttributes(new FilterInvocation(httpRequest, new MockHttpServletResponse(), new MockFilterChain())); - ConfigAttributeDefinition expected = new ConfigAttributeDefinition(new String[] {"ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"}); + List expected = SecurityConfig.createList("ROLE_WE_DONT_HAVE", "ANOTHER_ROLE"); assertEquals(expected, returned); } diff --git a/core/src/test/java/org/springframework/security/intercept/web/FilterSecurityInterceptorTests.java b/core/src/test/java/org/springframework/security/intercept/web/FilterSecurityInterceptorTests.java index 4e64c455da..aed4c6c1d3 100644 --- a/core/src/test/java/org/springframework/security/intercept/web/FilterSecurityInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/intercept/web/FilterSecurityInterceptorTests.java @@ -29,6 +29,7 @@ import org.springframework.security.MockAuthenticationManager; import org.springframework.security.MockRunAsManager; import org.springframework.security.RunAsManager; import org.springframework.security.MockApplicationEventPublisher; +import org.springframework.security.SecurityConfig; import org.springframework.security.util.AntUrlPathMatcher; import org.springframework.security.util.RegexUrlPathMatcher; import org.springframework.security.context.SecurityContextHolder; @@ -40,6 +41,7 @@ import java.io.IOException; import java.util.Collection; import java.util.LinkedHashMap; +import java.util.List; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -145,8 +147,7 @@ public class FilterSecurityInterceptorTests extends TestCase { interceptor.setApplicationEventPublisher(new MockApplicationEventPublisher(true)); // Setup a mock config attribute definition - ConfigAttributeDefinition def = new ConfigAttributeDefinition("MOCK_OK"); - MockFilterInvocationDefinitionMap mockSource = new MockFilterInvocationDefinitionMap("/secure/page.html", def); + MockFilterInvocationDefinitionMap mockSource = new MockFilterInvocationDefinitionMap("/secure/page.html", "MOCK_OK"); interceptor.setObjectDefinitionSource(mockSource); // Setup our expectation that the filter chain will be invoked, as access is granted @@ -187,7 +188,6 @@ public class FilterSecurityInterceptorTests extends TestCase { * We just test invocation works in a success event. There is no need to test access denied events as the * abstract parent enforces that logic, which is extensively tested separately. * - * @throws Throwable DOCUMENT ME! */ public void testSuccessfulInvocation() throws Throwable { // Setup the FilterSecurityInterceptor @@ -198,8 +198,7 @@ public class FilterSecurityInterceptorTests extends TestCase { interceptor.setApplicationEventPublisher(new MockApplicationEventPublisher(true)); // Setup a mock config attribute definition - ConfigAttributeDefinition def = new ConfigAttributeDefinition("MOCK_OK"); - MockFilterInvocationDefinitionMap mockSource = new MockFilterInvocationDefinitionMap("/secure/page.html", def); + MockFilterInvocationDefinitionMap mockSource = new MockFilterInvocationDefinitionMap("/secure/page.html", "MOCK_OK"); interceptor.setObjectDefinitionSource(mockSource); // Setup our expectation that the filter chain will be invoked, as access is granted @@ -224,7 +223,7 @@ public class FilterSecurityInterceptorTests extends TestCase { LinkedHashMap reqMap = new LinkedHashMap(); reqMap.put(new RequestKey("/secure/**", null), new ConfigAttributeDefinition(new String[] {"ROLE_USER"})); DefaultFilterInvocationDefinitionSource fids - = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher()); + = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher()); FilterSecurityInterceptor filter = new FilterSecurityInterceptor(); filter.setObjectDefinitionSource(fids); @@ -260,15 +259,15 @@ public class FilterSecurityInterceptorTests extends TestCase { } private class MockFilterInvocationDefinitionMap implements FilterInvocationDefinitionSource { - private ConfigAttributeDefinition toReturn; + private List toReturn; private String servletPath; - public MockFilterInvocationDefinitionMap(String servletPath, ConfigAttributeDefinition toReturn) { + public MockFilterInvocationDefinitionMap(String servletPath, String... toReturn) { this.servletPath = servletPath; - this.toReturn = toReturn; + this.toReturn = SecurityConfig.createList(toReturn); } - public ConfigAttributeDefinition getAttributes(Object object) + public List getAttributes(Object object) throws IllegalArgumentException { FilterInvocation fi = (FilterInvocation) object; @@ -279,7 +278,7 @@ public class FilterSecurityInterceptorTests extends TestCase { } } - public Collection getConfigAttributeDefinitions() { + public Collection> getConfigAttributeDefinitions() { return null; } 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 2ee4c9b810..ff532e4b0d 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 @@ -15,12 +15,13 @@ package org.springframework.security.intercept.web; +import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.util.AntUrlPathMatcher; +import java.util.Collection; import java.util.List; import java.util.Vector; -import java.util.Collection; /** @@ -61,7 +62,7 @@ public class MockFilterInvocationDefinitionSource extends DefaultFilterInvocatio //~ Methods ======================================================================================================== - public Collection getConfigAttributeDefinitions() { + public Collection> getConfigAttributeDefinitions() { if (returnAnIterator) { return list; } else { @@ -69,7 +70,7 @@ public class MockFilterInvocationDefinitionSource extends DefaultFilterInvocatio } } - public ConfigAttributeDefinition lookupAttributes(String url, String method) { + public List lookupAttributes(String url, String method) { throw new UnsupportedOperationException("mock method not implemented"); } } diff --git a/core/src/test/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluatorTests.java b/core/src/test/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluatorTests.java index 376638b2f7..0c0bbc2d5f 100644 --- a/core/src/test/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluatorTests.java +++ b/core/src/test/java/org/springframework/security/intercept/web/WebInvocationPrivilegeEvaluatorTests.java @@ -47,10 +47,6 @@ public class WebInvocationPrivilegeEvaluatorTests extends TestCase { //~ Methods ======================================================================================================== - public static void main(String[] args) { - junit.textui.TestRunner.run(WebInvocationPrivilegeEvaluatorTests.class); - } - private FilterSecurityInterceptor makeFilterSecurityInterceptor() { ApplicationContext context = new ClassPathXmlApplicationContext( "org/springframework/security/intercept/web/applicationContext.xml"); diff --git a/core/src/test/java/org/springframework/security/securechannel/ChannelProcessingFilterTests.java b/core/src/test/java/org/springframework/security/securechannel/ChannelProcessingFilterTests.java index f9ed4a7ee1..3b5e2f728d 100644 --- a/core/src/test/java/org/springframework/security/securechannel/ChannelProcessingFilterTests.java +++ b/core/src/test/java/org/springframework/security/securechannel/ChannelProcessingFilterTests.java @@ -19,6 +19,7 @@ import junit.framework.TestCase; import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.SecurityConfig; import org.springframework.security.intercept.web.FilterInvocation; import org.springframework.security.intercept.web.FilterInvocationDefinitionSource; @@ -28,9 +29,9 @@ import org.springframework.mock.web.MockHttpServletResponse; import java.io.IOException; +import java.util.Collection; import java.util.List; import java.util.Vector; -import java.util.Collection; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -51,8 +52,7 @@ public class ChannelProcessingFilterTests extends TestCase { throws Exception { ChannelProcessingFilter filter = new ChannelProcessingFilter(); - ConfigAttributeDefinition attr = new ConfigAttributeDefinition("MOCK"); - MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true); + MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "MOCK"); filter.setFilterInvocationDefinitionSource(fids); try { @@ -80,9 +80,7 @@ public class ChannelProcessingFilterTests extends TestCase { ChannelProcessingFilter filter = new ChannelProcessingFilter(); filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "SUPPORTS_MOCK_ONLY")); - ConfigAttributeDefinition attr = new ConfigAttributeDefinition("SUPPORTS_MOCK_ONLY"); - - MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true); + MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "SUPPORTS_MOCK_ONLY"); filter.setFilterInvocationDefinitionSource(fids); @@ -94,8 +92,7 @@ public class ChannelProcessingFilterTests extends TestCase { ChannelProcessingFilter filter = new ChannelProcessingFilter(); filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "SUPPORTS_MOCK_ONLY")); - ConfigAttributeDefinition attr = new ConfigAttributeDefinition(new String[] {"SUPPORTS_MOCK_ONLY", "INVALID_ATTRIBUTE"}); - MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true); + MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "SUPPORTS_MOCK_ONLY", "INVALID_ATTRIBUTE"); filter.setFilterInvocationDefinitionSource(fids); @@ -111,9 +108,7 @@ public class ChannelProcessingFilterTests extends TestCase { ChannelProcessingFilter filter = new ChannelProcessingFilter(); filter.setChannelDecisionManager(new MockChannelDecisionManager(true, "SOME_ATTRIBUTE")); - ConfigAttributeDefinition attr = new ConfigAttributeDefinition("SOME_ATTRIBUTE"); - - MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true); + MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "SOME_ATTRIBUTE"); filter.setFilterInvocationDefinitionSource(fids); @@ -132,9 +127,7 @@ public class ChannelProcessingFilterTests extends TestCase { ChannelProcessingFilter filter = new ChannelProcessingFilter(); filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "SOME_ATTRIBUTE")); - ConfigAttributeDefinition attr = new ConfigAttributeDefinition("SOME_ATTRIBUTE"); - - MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true); + MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "SOME_ATTRIBUTE"); filter.setFilterInvocationDefinitionSource(fids); @@ -154,9 +147,7 @@ public class ChannelProcessingFilterTests extends TestCase { ChannelProcessingFilter filter = new ChannelProcessingFilter(); filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "NOT_USED")); - ConfigAttributeDefinition attr = new ConfigAttributeDefinition("NOT_USED"); - - MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, true); + MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "NOT_USED"); filter.setFilterInvocationDefinitionSource(fids); @@ -196,9 +187,7 @@ public class ChannelProcessingFilterTests extends TestCase { filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "MOCK")); assertTrue(filter.getChannelDecisionManager() != null); - ConfigAttributeDefinition attr = new ConfigAttributeDefinition("MOCK"); - - MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", attr, false); + MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", false, "MOCK"); filter.setFilterInvocationDefinitionSource(fids); assertTrue(filter.getFilterInvocationDefinitionSource() != null); @@ -257,18 +246,17 @@ public class ChannelProcessingFilterTests extends TestCase { } private class MockFilterInvocationDefinitionMap implements FilterInvocationDefinitionSource { - private ConfigAttributeDefinition toReturn; + private List toReturn; private String servletPath; private boolean provideIterator; - public MockFilterInvocationDefinitionMap(String servletPath, ConfigAttributeDefinition toReturn, - boolean provideIterator) { + public MockFilterInvocationDefinitionMap(String servletPath, boolean provideIterator, String... toReturn) { this.servletPath = servletPath; - this.toReturn = toReturn; + this.toReturn = SecurityConfig.createList(toReturn); this.provideIterator = provideIterator; } - public ConfigAttributeDefinition getAttributes(Object object) + public List getAttributes(Object object) throws IllegalArgumentException { FilterInvocation fi = (FilterInvocation) object; @@ -279,7 +267,7 @@ public class ChannelProcessingFilterTests extends TestCase { } } - public Collection getConfigAttributeDefinitions() { + public Collection> getConfigAttributeDefinitions() { if (!provideIterator) { return null; } diff --git a/core/src/test/java/org/springframework/security/util/FilterChainProxyTests.java b/core/src/test/java/org/springframework/security/util/FilterChainProxyTests.java index 3d9842df13..4304bf8f79 100644 --- a/core/src/test/java/org/springframework/security/util/FilterChainProxyTests.java +++ b/core/src/test/java/org/springframework/security/util/FilterChainProxyTests.java @@ -18,6 +18,7 @@ package org.springframework.security.util; import org.springframework.security.ConfigAttribute; import org.springframework.security.ConfigAttributeDefinition; import org.springframework.security.MockFilterConfig; +import org.springframework.security.SecurityConfig; import org.springframework.security.context.HttpSessionContextIntegrationFilter; import org.springframework.security.intercept.web.MockFilterInvocationDefinitionSource; import org.springframework.security.intercept.web.DefaultFilterInvocationDefinitionSource; @@ -76,10 +77,8 @@ public class FilterChainProxyTests { FilterChainProxy filterChainProxy = new FilterChainProxy(); filterChainProxy.setApplicationContext(new StaticApplicationContext()); - ConfigAttributeDefinition cad = new ConfigAttributeDefinition(new MockConfigAttribute()); - LinkedHashMap map = new LinkedHashMap(); - map.put(new RequestKey("/**"), cad); + map.put(new RequestKey("/**"), SecurityConfig.createList(null)); DefaultFilterInvocationDefinitionSource fids = new DefaultFilterInvocationDefinitionSource(new AntUrlPathMatcher(), map); @@ -226,12 +225,4 @@ public class FilterChainProxyTests { assertTrue(filter.isWasDoFiltered()); assertTrue(filter.isWasDestroyed()); } - - //~ Inner Classes ================================================================================================== - - private class MockConfigAttribute implements ConfigAttribute { - public String getAttribute() { - return null; - } - } }