From 0521d100691a15912fddc2c24a5de4eb2e09e8f2 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Wed, 31 Mar 2010 22:37:22 +0100 Subject: [PATCH] SEC-1294: Enable access to beans from ApplicationContext in EL expressions. ExpressionHandlers are now ApplicationContextAware and set the app context on the SecurityExpressionRoot. A custom PropertyAccessor resolves the properties against the root by looking them up in the app context. --- config/config.gradle | 5 ++- ...nvocationSecurityMetadataSourceParser.java | 13 +++++-- .../config/http/HttpConfigurationBuilder.java | 7 +++- ...thodSecurityBeanDefinitionParserTests.java | 15 ++++++++ .../expression/SecurityExpressionRoot.java | 10 +++++ ...ecurityExpressionRootPropertyAccessor.java | 38 +++++++++++++++++++ ...efaultMethodSecurityExpressionHandler.java | 14 ++++++- .../MethodSecurityEvaluationContext.java | 24 +++++++++++- ...xpressionProtectedBusinessServiceImpl.java | 6 +++ .../resources/http-extra-fsi-app-context.xml | 3 +- .../DefaultWebSecurityExpressionHandler.java | 17 +++++++-- ...aultWebSecurityExpressionHandlerTests.java | 32 ++++++++++++++++ 12 files changed, 170 insertions(+), 14 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRootPropertyAccessor.java create mode 100644 web/src/test/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandlerTests.java diff --git a/config/config.gradle b/config/config.gradle index f4c34de019..7539f0b46c 100644 --- a/config/config.gradle +++ b/config/config.gradle @@ -6,6 +6,7 @@ dependencies { compile project(':spring-security-core'), project(':spring-security-web'), "org.aspectj:aspectjweaver:$aspectjVersion", + 'aopalliance:aopalliance:1.0', "org.springframework:spring-aop:$springVersion", "org.springframework:spring-context:$springVersion", "org.springframework:spring-web:$springVersion", @@ -17,10 +18,10 @@ dependencies { project(':spring-security-openid'), files(this.project(':spring-security-core').sourceSets.test.classesDir), 'javax.annotation:jsr250-api:1.0', - 'aopalliance:aopalliance:1.0', "org.springframework.ldap:spring-ldap-core:$springLdapVersion", "org.springframework:spring-jdbc:$springVersion", "org.springframework:spring-tx:$springVersion" - testRuntime "hsqldb:hsqldb:$hsqlVersion" + testRuntime "hsqldb:hsqldb:$hsqlVersion", + "cglib:cglib-nodep:2.2" } diff --git a/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java b/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java index c8e9382677..7e4e48bbd9 100644 --- a/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java +++ b/config/src/main/java/org/springframework/security/config/http/FilterInvocationSecurityMetadataSourceParser.java @@ -74,9 +74,7 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit if (StringUtils.hasText(expressionHandlerRef)) { logger.info("Using bean '" + expressionHandlerRef + "' as web SecurityExpressionHandler implementation"); } else { - BeanDefinition expressionHandler = BeanDefinitionBuilder.rootBeanDefinition(DefaultWebSecurityExpressionHandler.class).getBeanDefinition(); - expressionHandlerRef = pc.getReaderContext().generateBeanName(expressionHandler); - pc.registerBeanComponent(new BeanComponentDefinition(expressionHandler, expressionHandlerRef)); + expressionHandlerRef = registerDefaultExpressionHandler(pc); } fidsBuilder = BeanDefinitionBuilder.rootBeanDefinition(ExpressionBasedFilterInvocationSecurityMetadataSource.class); @@ -87,12 +85,19 @@ public class FilterInvocationSecurityMetadataSourceParser implements BeanDefinit fidsBuilder.addConstructorArgValue(requestToAttributesMap); } -// fidsBuilder.addPropertyValue("stripQueryStringFromUrls", matcher instanceof AntUrlPathMatcher); fidsBuilder.getRawBeanDefinition().setSource(pc.extractSource(elt)); return fidsBuilder.getBeanDefinition(); } + static String registerDefaultExpressionHandler(ParserContext pc) { + BeanDefinition expressionHandler = BeanDefinitionBuilder.rootBeanDefinition(DefaultWebSecurityExpressionHandler.class).getBeanDefinition(); + String expressionHandlerRef = pc.getReaderContext().generateBeanName(expressionHandler); + pc.registerBeanComponent(new BeanComponentDefinition(expressionHandler, expressionHandlerRef)); + + return expressionHandlerRef; + } + static boolean isUseExpressions(Element elt) { return "true".equals(elt.getAttribute(ATT_USE_EXPRESSIONS)); } diff --git a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java index 461ff91b58..1cc6bae9f4 100644 --- a/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/HttpConfigurationBuilder.java @@ -461,7 +461,12 @@ class HttpConfigurationBuilder { ManagedList voters = new ManagedList(2); if (useExpressions) { - voters.add(new RootBeanDefinition(WebExpressionVoter.class)); + BeanDefinitionBuilder expressionVoter = BeanDefinitionBuilder.rootBeanDefinition(WebExpressionVoter.class); + RuntimeBeanReference expressionHandler = new RuntimeBeanReference( + FilterInvocationSecurityMetadataSourceParser.registerDefaultExpressionHandler(pc)); + expressionVoter.addPropertyValue("expressionHandler", expressionHandler); + + voters.add(expressionVoter.getBeanDefinition()); } else { voters.add(new RootBeanDefinition(RoleVoter.class)); voters.add(new RootBeanDefinition(AuthenticatedVoter.class)); diff --git a/config/src/test/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParserTests.java b/config/src/test/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParserTests.java index 834838619c..489eae4223 100644 --- a/config/src/test/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParserTests.java +++ b/config/src/test/java/org/springframework/security/config/method/GlobalMethodSecurityBeanDefinitionParserTests.java @@ -19,6 +19,7 @@ import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.access.SecurityConfig; import org.springframework.security.access.annotation.BusinessService; +import org.springframework.security.access.annotation.ExpressionProtectedBusinessServiceImpl; import org.springframework.security.access.intercept.AfterInvocationProviderManager; import org.springframework.security.access.intercept.RunAsManagerImpl; import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor; @@ -242,6 +243,20 @@ public class GlobalMethodSecurityBeanDefinitionParserTests { target.someAdminMethod(); } + @Test + public void beanNameExpressionPropertyIsSupported() { + setContext( + "" + + "" + + " " + + "" + + "" + + AUTH_PROVIDER_XML); + SecurityContextHolder.getContext().setAuthentication(bob); + ExpressionProtectedBusinessServiceImpl target = (ExpressionProtectedBusinessServiceImpl) appContext.getBean("target"); + target.methodWithBeanNamePropertyAccessExpression("x"); + } + @Test public void preAndPostFilterAnnotationsWorkWithLists() { setContext( diff --git a/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java b/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java index 0d6174a569..51b5203565 100644 --- a/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java +++ b/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java @@ -4,6 +4,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.Set; +import org.springframework.context.ApplicationContext; import org.springframework.security.access.hierarchicalroles.RoleHierarchy; import org.springframework.security.authentication.AuthenticationTrustResolver; import org.springframework.security.core.Authentication; @@ -22,6 +23,7 @@ public abstract class SecurityExpressionRoot { private AuthenticationTrustResolver trustResolver; private RoleHierarchy roleHierarchy; private Set roles; + private ApplicationContext applicationContext; /** Allows "permitAll" expression */ public final boolean permitAll = true; @@ -92,6 +94,14 @@ public abstract class SecurityExpressionRoot { this.roleHierarchy = roleHierarchy; } + ApplicationContext getApplicationContext() { + return applicationContext; + } + + public void setApplicationContext(ApplicationContext applicationContext) { + this.applicationContext = applicationContext; + } + private Set getAuthoritySet() { if (roles == null) { roles = new HashSet(); diff --git a/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRootPropertyAccessor.java b/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRootPropertyAccessor.java new file mode 100644 index 0000000000..0411469f22 --- /dev/null +++ b/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRootPropertyAccessor.java @@ -0,0 +1,38 @@ +package org.springframework.security.access.expression; + +import org.springframework.context.ApplicationContext; +import org.springframework.expression.AccessException; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.PropertyAccessor; +import org.springframework.expression.TypedValue; + +@SuppressWarnings("unchecked") +public final class SecurityExpressionRootPropertyAccessor implements PropertyAccessor { + public Class[] CLASSES = {SecurityExpressionRoot.class}; + + public boolean canRead(EvaluationContext context, Object target, String name) throws AccessException { + ApplicationContext ctx = ((SecurityExpressionRoot)target).getApplicationContext(); + + if (ctx == null) { + return false; + } + + return ctx.containsBean(name); + } + + public TypedValue read(EvaluationContext context, Object target, String name) throws AccessException { + return new TypedValue(((SecurityExpressionRoot)target).getApplicationContext().getBean(name)); + } + + public boolean canWrite(EvaluationContext context, Object target, String name) throws AccessException { + return false; + } + + public void write(EvaluationContext context, Object target, String name, Object newValue) throws AccessException { + } + + public Class[] getSpecificTargetClasses() { + return CLASSES; + } + +} diff --git a/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java b/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java index 99cb2099d1..bbdd86ee71 100644 --- a/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java +++ b/core/src/main/java/org/springframework/security/access/expression/method/DefaultMethodSecurityExpressionHandler.java @@ -9,6 +9,9 @@ import java.util.List; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.BeansException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.core.LocalVariableTableParameterNameDiscoverer; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.expression.EvaluationContext; @@ -18,6 +21,7 @@ import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.security.access.PermissionCacheOptimizer; import org.springframework.security.access.PermissionEvaluator; import org.springframework.security.access.expression.ExpressionUtils; +import org.springframework.security.access.expression.SecurityExpressionRootPropertyAccessor; import org.springframework.security.access.hierarchicalroles.RoleHierarchy; import org.springframework.security.authentication.AuthenticationTrustResolver; import org.springframework.security.authentication.AuthenticationTrustResolverImpl; @@ -31,7 +35,7 @@ import org.springframework.security.core.Authentication; * @author Luke Taylor * @since 3.0 */ -public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExpressionHandler { +public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExpressionHandler, ApplicationContextAware { protected final Log logger = LogFactory.getLog(getClass()); @@ -39,8 +43,10 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp private PermissionEvaluator permissionEvaluator = new DenyAllPermissionEvaluator(); private PermissionCacheOptimizer permissionCacheOptimizer = null; private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl(); + private final SecurityExpressionRootPropertyAccessor sxrpa = new SecurityExpressionRootPropertyAccessor(); private ExpressionParser expressionParser = new SpelExpressionParser(); private RoleHierarchy roleHierarchy; + private ApplicationContext applicationContext; public DefaultMethodSecurityExpressionHandler() { } @@ -55,7 +61,9 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp root.setTrustResolver(trustResolver); root.setPermissionEvaluator(permissionEvaluator); root.setRoleHierarchy(roleHierarchy); + root.setApplicationContext(applicationContext); ctx.setRootObject(root); + ctx.addPropertyAccessor(sxrpa); return ctx; } @@ -170,4 +178,8 @@ public class DefaultMethodSecurityExpressionHandler implements MethodSecurityExp public void setRoleHierarchy(RoleHierarchy roleHierarchy) { this.roleHierarchy = roleHierarchy; } + + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + this.applicationContext = applicationContext; + } } diff --git a/core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java b/core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java index 30cfab4be1..4eb1d18ba7 100644 --- a/core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java +++ b/core/src/main/java/org/springframework/security/access/expression/method/MethodSecurityEvaluationContext.java @@ -6,6 +6,8 @@ import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.aop.support.AopUtils; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.context.ApplicationContext; import org.springframework.core.LocalVariableTableParameterNameDiscoverer; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.expression.spel.support.StandardEvaluationContext; @@ -23,8 +25,9 @@ class MethodSecurityEvaluationContext extends StandardEvaluationContext { private static Log logger = LogFactory.getLog(MethodSecurityEvaluationContext.class); private ParameterNameDiscoverer parameterNameDiscoverer; + private final MethodInvocation mi; + private ApplicationContext appContext; private boolean argumentsAdded; - private MethodInvocation mi; /** * Intended for testing. Don't use in practice as it creates a new parameter resolver @@ -44,6 +47,7 @@ class MethodSecurityEvaluationContext extends StandardEvaluationContext { @Override public Object lookupVariable(String name) { Object variable = super.lookupVariable(name); + if (variable != null) { return variable; } @@ -53,7 +57,23 @@ class MethodSecurityEvaluationContext extends StandardEvaluationContext { argumentsAdded = true; } - return super.lookupVariable(name); + variable = super.lookupVariable(name); + + if (variable != null) { + return variable; + } + + if (appContext != null) { + try { + super.setVariable(name, appContext.getBean(name)); + + return super.lookupVariable(name); + } catch (NoSuchBeanDefinitionException e) { + logger.debug("Bean lookup for variable '" + name + "' failed"); + } + } + + return null; } public void setParameterNameDiscoverer(ParameterNameDiscoverer parameterNameDiscoverer) { diff --git a/core/src/test/java/org/springframework/security/access/annotation/ExpressionProtectedBusinessServiceImpl.java b/core/src/test/java/org/springframework/security/access/annotation/ExpressionProtectedBusinessServiceImpl.java index 2a8f6f91b4..723f9dee97 100644 --- a/core/src/test/java/org/springframework/security/access/annotation/ExpressionProtectedBusinessServiceImpl.java +++ b/core/src/test/java/org/springframework/security/access/annotation/ExpressionProtectedBusinessServiceImpl.java @@ -4,6 +4,7 @@ import java.util.ArrayList; import java.util.List; import org.springframework.security.access.prepost.PostFilter; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.access.prepost.PreFilter; @@ -43,4 +44,9 @@ public class ExpressionProtectedBusinessServiceImpl implements BusinessService { public Object[] methodReturningAnArray(Object[] someArray) { return someArray; } + + @PreAuthorize("#x == 'x' and number.intValue() == 1294 ") + public void methodWithBeanNamePropertyAccessExpression(String x) { + + } } diff --git a/itest/context/src/test/resources/http-extra-fsi-app-context.xml b/itest/context/src/test/resources/http-extra-fsi-app-context.xml index eb2edf40be..b208e66161 100644 --- a/itest/context/src/test/resources/http-extra-fsi-app-context.xml +++ b/itest/context/src/test/resources/http-extra-fsi-app-context.xml @@ -11,7 +11,8 @@ http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security.xsd"> - + + diff --git a/web/src/main/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandler.java b/web/src/main/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandler.java index 11236a96a9..85b08984d5 100644 --- a/web/src/main/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandler.java +++ b/web/src/main/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandler.java @@ -1,10 +1,14 @@ package org.springframework.security.web.access.expression; +import org.springframework.beans.BeansException; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.expression.EvaluationContext; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.security.access.expression.SecurityExpressionRoot; +import org.springframework.security.access.expression.SecurityExpressionRootPropertyAccessor; import org.springframework.security.access.hierarchicalroles.RoleHierarchy; import org.springframework.security.authentication.AuthenticationTrustResolver; import org.springframework.security.authentication.AuthenticationTrustResolverImpl; @@ -18,22 +22,25 @@ import org.springframework.security.web.FilterInvocation; * @author Luke Taylor * @since 3.0 */ -public class DefaultWebSecurityExpressionHandler implements WebSecurityExpressionHandler { +public class DefaultWebSecurityExpressionHandler implements WebSecurityExpressionHandler, ApplicationContextAware { private AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl(); private ExpressionParser expressionParser = new SpelExpressionParser(); + private final SecurityExpressionRootPropertyAccessor sxrpa = new SecurityExpressionRootPropertyAccessor(); private RoleHierarchy roleHierarchy; + private ApplicationContext applicationContext; public ExpressionParser getExpressionParser() { return expressionParser; } public EvaluationContext createEvaluationContext(Authentication authentication, FilterInvocation fi) { - StandardEvaluationContext ctx = new StandardEvaluationContext(); SecurityExpressionRoot root = new WebSecurityExpressionRoot(authentication, fi); root.setTrustResolver(trustResolver); root.setRoleHierarchy(roleHierarchy); - ctx.setRootObject(root); + root.setApplicationContext(applicationContext); + StandardEvaluationContext ctx = new StandardEvaluationContext(root); + ctx.addPropertyAccessor(sxrpa); return ctx; } @@ -41,4 +48,8 @@ public class DefaultWebSecurityExpressionHandler implements WebSecurityExpressio public void setRoleHierarchy(RoleHierarchy roleHierarchy) { this.roleHierarchy = roleHierarchy; } + + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + this.applicationContext = applicationContext; + } } diff --git a/web/src/test/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandlerTests.java b/web/src/test/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandlerTests.java new file mode 100644 index 0000000000..d2dee4b7f0 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/access/expression/DefaultWebSecurityExpressionHandlerTests.java @@ -0,0 +1,32 @@ +package org.springframework.security.web.access.expression; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +import org.junit.Test; +import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.context.support.StaticApplicationContext; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.ExpressionParser; +import org.springframework.security.access.SecurityConfig; +import org.springframework.security.core.Authentication; +import org.springframework.security.web.FilterInvocation; + +public class DefaultWebSecurityExpressionHandlerTests { + + @Test + public void expressionPropertiesAreResolvedAgainsAppContextBeans() throws Exception { + DefaultWebSecurityExpressionHandler handler = new DefaultWebSecurityExpressionHandler(); + StaticApplicationContext appContext = new StaticApplicationContext(); + RootBeanDefinition bean = new RootBeanDefinition(SecurityConfig.class); + bean.getConstructorArgumentValues().addGenericArgumentValue("ROLE_A"); + appContext.registerBeanDefinition("role", bean); + handler.setApplicationContext(appContext); + + EvaluationContext ctx = handler.createEvaluationContext(mock(Authentication.class), mock(FilterInvocation.class)); + ExpressionParser parser = handler.getExpressionParser(); + assertTrue(parser.parseExpression("role.getAttribute() == 'ROLE_A'").getValue(ctx, Boolean.class)); + assertTrue(parser.parseExpression("role.attribute == 'ROLE_A'").getValue(ctx, Boolean.class)); + } + +}