From eda60b72b1b3b285d9f30237079a7b8e10a678b3 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Sat, 27 Mar 2010 17:22:03 +0000 Subject: [PATCH] SEC-1448: Fixed failure to resolve generic method argument names in MethodSecurityEvaluationContext. Changed to use AopUtils.getMostSpecificMethod() when obtaining the method on which the parameter resolution should be performed. Also added better error handling and log warning when parameter names cannot be resolved. The exception will then be a SpEL one, rather than a NPE. --- ...thodSecurityBeanDefinitionParserTests.java | 23 ++++++++++++++++--- .../MethodSecurityEvaluationContext.java | 19 +++++++++++++-- 2 files changed, 37 insertions(+), 5 deletions(-) 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 5b4c39c0d7..834838619c 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 @@ -24,6 +24,7 @@ import org.springframework.security.access.intercept.RunAsManagerImpl; import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor; import org.springframework.security.access.intercept.aopalliance.MethodSecurityMetadataSourceAdvisor; import org.springframework.security.access.prepost.PostInvocationAdviceProvider; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.access.prepost.PreInvocationAuthorizationAdviceVoter; import org.springframework.security.access.vote.AffirmativeBased; import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException; @@ -43,6 +44,8 @@ import org.springframework.security.util.FieldUtils; * @author Luke Taylor */ public class GlobalMethodSecurityBeanDefinitionParserTests { + private final UsernamePasswordAuthenticationToken bob = new UsernamePasswordAuthenticationToken("bob","bobspassword"); + private AbstractXmlApplicationContext appContext; private BusinessService target; @@ -234,7 +237,7 @@ public class GlobalMethodSecurityBeanDefinitionParserTests { "" + "" + AUTH_PROVIDER_XML); - SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken("bob","bobspassword")); + SecurityContextHolder.getContext().setAuthentication(bob); target = (BusinessService) appContext.getBean("target"); target.someAdminMethod(); } @@ -245,7 +248,7 @@ public class GlobalMethodSecurityBeanDefinitionParserTests { "" + "" + AUTH_PROVIDER_XML); - SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken("bob","bobspassword")); + SecurityContextHolder.getContext().setAuthentication(bob); target = (BusinessService) appContext.getBean("target"); List arg = new ArrayList(); arg.add("joe"); @@ -264,7 +267,7 @@ public class GlobalMethodSecurityBeanDefinitionParserTests { "" + "" + AUTH_PROVIDER_XML); - SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken("bob","bobspassword")); + SecurityContextHolder.getContext().setAuthentication(bob); target = (BusinessService) appContext.getBean("target"); Object[] arg = new String[] {"joe", "bob", "sam"}; Object[] result = target.methodReturningAnArray(arg); @@ -300,6 +303,19 @@ public class GlobalMethodSecurityBeanDefinitionParserTests { foo.foo(new SecurityConfig("A")); } + // SEC-1448 + @Test + @SuppressWarnings("unchecked") + public void genericsMethodArgumentNamesAreResolved() throws Exception { + setContext( + "" + + "" + AUTH_PROVIDER_XML + ); + SecurityContextHolder.getContext().setAuthentication(bob); + Foo foo = (Foo) appContext.getBean("target"); + foo.foo(new SecurityConfig("A")); + } + @Test public void runAsManagerIsSetCorrectly() throws Exception { StaticApplicationContext parent = new StaticApplicationContext(); @@ -328,6 +344,7 @@ public class GlobalMethodSecurityBeanDefinitionParserTests { } public static class ConcreteFoo implements Foo { + @PreAuthorize("#action.attribute == 'A'") public void foo(SecurityConfig action) { } } 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 f5fce6b14a..30cfab4be1 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 @@ -3,11 +3,13 @@ package org.springframework.security.access.expression.method; import java.lang.reflect.Method; 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.core.LocalVariableTableParameterNameDiscoverer; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.security.core.Authentication; -import org.springframework.util.ClassUtils; /** * Internal security-specific EvaluationContext implementation which lazily adds the @@ -18,6 +20,8 @@ import org.springframework.util.ClassUtils; * @since 3.0 */ class MethodSecurityEvaluationContext extends StandardEvaluationContext { + private static Log logger = LogFactory.getLog(MethodSecurityEvaluationContext.class); + private ParameterNameDiscoverer parameterNameDiscoverer; private boolean argumentsAdded; private MethodInvocation mi; @@ -58,10 +62,21 @@ class MethodSecurityEvaluationContext extends StandardEvaluationContext { private void addArgumentsAsVariables() { Object[] args = mi.getArguments(); + + if (args.length == 0) { + return; + } + Object targetObject = mi.getThis(); - Method method = ClassUtils.getMostSpecificMethod(mi.getMethod(), targetObject.getClass()); + Method method = AopUtils.getMostSpecificMethod(mi.getMethod(), targetObject.getClass()); String[] paramNames = parameterNameDiscoverer.getParameterNames(method); + if (paramNames == null) { + logger.warn("Unable to resolve method parameter names for method: " + method + + ". Debug symbol information is required if you are using parameter names in expressions."); + return; + } + for(int i=0; i < args.length; i++) { super.setVariable(paramNames[i], args[i]); }