From 614d8c03217bd2eab217778da92b426f57199cd0 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Fri, 22 Apr 2011 13:33:56 +0100 Subject: [PATCH] SEC-1723: Use standard SpEL syntax for accessing beans in the app context by name. --- core/core.gradle | 3 +- .../AbstractSecurityExpressionHandler.java | 8 +-- .../ApplicationContextPropertyAccessor.java | 42 --------------- ...xpressionProtectedBusinessServiceImpl.java | 2 +- ...bstractSecurityExpressionHandlerTests.java | 53 +++++++++++++++++++ .../SecurityExpressionRootTests.java | 2 +- gradle/javaprojects.gradle | 1 + ...aultWebSecurityExpressionHandlerTests.java | 4 +- 8 files changed, 65 insertions(+), 50 deletions(-) delete mode 100644 core/src/main/java/org/springframework/security/access/expression/ApplicationContextPropertyAccessor.java create mode 100644 core/src/test/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandlerTests.java diff --git a/core/core.gradle b/core/core.gradle index 826a81f0eb..732b8589b5 100644 --- a/core/core.gradle +++ b/core/core.gradle @@ -17,7 +17,8 @@ dependencies { "org.springframework:spring-test:$springVersion", "org.slf4j:jcl-over-slf4j:$slf4jVersion" - testRuntime "hsqldb:hsqldb:$hsqlVersion" + testRuntime "hsqldb:hsqldb:$hsqlVersion", + "cglib:cglib-nodep:$cglibVersion" } // jdkVersion = System.properties['java.version'] diff --git a/core/src/main/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandler.java b/core/src/main/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandler.java index 4db1d77cab..39bf647d8f 100644 --- a/core/src/main/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandler.java +++ b/core/src/main/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandler.java @@ -2,6 +2,8 @@ package org.springframework.security.access.expression; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; +import org.springframework.context.expression.BeanFactoryResolver; +import org.springframework.expression.BeanResolver; import org.springframework.expression.EvaluationContext; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; @@ -21,7 +23,7 @@ import org.springframework.security.core.Authentication; public abstract class AbstractSecurityExpressionHandler implements SecurityExpressionHandler, ApplicationContextAware { private final AuthenticationTrustResolver trustResolver = new AuthenticationTrustResolverImpl(); private final ExpressionParser expressionParser = new SpelExpressionParser(); - private ApplicationContextPropertyAccessor sxrpa = new ApplicationContextPropertyAccessor(null); + private BeanResolver br; private RoleHierarchy roleHierarchy; public final ExpressionParser getExpressionParser() { @@ -42,7 +44,7 @@ public abstract class AbstractSecurityExpressionHandler implements SecurityEx root.setTrustResolver(trustResolver); root.setRoleHierarchy(roleHierarchy); StandardEvaluationContext ctx = createEvaluationContextInternal(authentication, invocation); - ctx.addPropertyAccessor(sxrpa); + ctx.setBeanResolver(br); ctx.setRootObject(root); return ctx; @@ -76,6 +78,6 @@ public abstract class AbstractSecurityExpressionHandler implements SecurityEx } public void setApplicationContext(ApplicationContext applicationContext) { - sxrpa = new ApplicationContextPropertyAccessor(applicationContext); + br = new BeanFactoryResolver(applicationContext); } } diff --git a/core/src/main/java/org/springframework/security/access/expression/ApplicationContextPropertyAccessor.java b/core/src/main/java/org/springframework/security/access/expression/ApplicationContextPropertyAccessor.java deleted file mode 100644 index bd03364a4a..0000000000 --- a/core/src/main/java/org/springframework/security/access/expression/ApplicationContextPropertyAccessor.java +++ /dev/null @@ -1,42 +0,0 @@ -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; - -/** - * General property accessor which resolves properties as bean names within an {@code ApplicationContext}. - */ -final class ApplicationContextPropertyAccessor implements PropertyAccessor { - private final ApplicationContext ctx; - - ApplicationContextPropertyAccessor(ApplicationContext ctx) { - this.ctx = ctx; - } - - public boolean canRead(EvaluationContext context, Object target, String name) throws AccessException { - if (ctx == null) { - return false; - } - - return ctx.containsBean(name); - } - - public TypedValue read(EvaluationContext context, Object target, String name) throws AccessException { - return new TypedValue(ctx.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 null; - } - -} 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 723f9dee97..d0480a1797 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 @@ -45,7 +45,7 @@ public class ExpressionProtectedBusinessServiceImpl implements BusinessService { return someArray; } - @PreAuthorize("#x == 'x' and number.intValue() == 1294 ") + @PreAuthorize("#x == 'x' and @number.intValue() == 1294 ") public void methodWithBeanNamePropertyAccessExpression(String x) { } diff --git a/core/src/test/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandlerTests.java b/core/src/test/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandlerTests.java new file mode 100644 index 0000000000..42ceadf9b0 --- /dev/null +++ b/core/src/test/java/org/springframework/security/access/expression/AbstractSecurityExpressionHandlerTests.java @@ -0,0 +1,53 @@ +package org.springframework.security.access.expression; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +import org.junit.*; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.expression.Expression; +import org.springframework.security.core.Authentication; + +import java.util.*; + +/** + * @author Luke Taylor + */ +public class AbstractSecurityExpressionHandlerTests { + private AbstractSecurityExpressionHandler handler; + + @Before + public void setUp() throws Exception { + handler = new AbstractSecurityExpressionHandler() { + @Override + protected SecurityExpressionRoot createSecurityExpressionRoot(Authentication authentication, Object o) { + return new SecurityExpressionRoot(authentication) {}; + } + }; + } + + @Test + public void beanNamesAreCorrectlyResolved() throws Exception { + handler.setApplicationContext(new AnnotationConfigApplicationContext(TestConfiguration.class)); + + Expression expression = handler.getExpressionParser().parseExpression("@number10.compareTo(@number20) < 0"); + assertTrue((Boolean) expression.getValue(handler.createEvaluationContext(mock(Authentication.class), new Object()))); + } +} + +@Configuration +class TestConfiguration { + + @Bean + Integer number10() { + return 10; + } + + @Bean + Integer number20() { + return 20; + } +} diff --git a/core/src/test/java/org/springframework/security/access/expression/SecurityExpressionRootTests.java b/core/src/test/java/org/springframework/security/access/expression/SecurityExpressionRootTests.java index a93732cfab..8cfe9feb72 100644 --- a/core/src/test/java/org/springframework/security/access/expression/SecurityExpressionRootTests.java +++ b/core/src/test/java/org/springframework/security/access/expression/SecurityExpressionRootTests.java @@ -20,7 +20,7 @@ import org.springframework.security.core.authority.AuthorityUtils; * @since 3.0 */ public class SecurityExpressionRootTests { - private final Authentication JOE = new TestingAuthenticationToken("joe", "pass", "A", "B"); + final static Authentication JOE = new TestingAuthenticationToken("joe", "pass", "A", "B"); @Test public void denyAllIsFalsePermitAllTrue() throws Exception { diff --git a/gradle/javaprojects.gradle b/gradle/javaprojects.gradle index 1d8b5c48ce..20f399fd25 100644 --- a/gradle/javaprojects.gradle +++ b/gradle/javaprojects.gradle @@ -11,6 +11,7 @@ jettyVersion = '6.1.22' hsqlVersion = '1.8.0.10' slf4jVersion = '1.6.1' logbackVersion = '0.9.24' +cglibVersion = '2.2' bundlorProperties = [ version: version, 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 index d2dee4b7f0..86eefd637b 100644 --- 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 @@ -25,8 +25,8 @@ public class DefaultWebSecurityExpressionHandlerTests { 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)); + assertTrue(parser.parseExpression("@role.getAttribute() == 'ROLE_A'").getValue(ctx, Boolean.class)); + assertTrue(parser.parseExpression("@role.attribute == 'ROLE_A'").getValue(ctx, Boolean.class)); } }