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.

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 {
                 "<global-method-security pre-post-annotations='enabled'/>" +
                 "<b:bean id='target' class='org.springframework.security.access.annotation.ExpressionProtectedBusinessServiceImpl'/>" +
                 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 {
                 "<global-method-security pre-post-annotations='enabled'/>" +
                 "<b:bean id='target' class='org.springframework.security.access.annotation.ExpressionProtectedBusinessServiceImpl'/>" +
                 AUTH_PROVIDER_XML);
-        SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken("bob","bobspassword"));
+        SecurityContextHolder.getContext().setAuthentication(bob);
         target = (BusinessService) appContext.getBean("target");
         List<String> arg = new ArrayList<String>();
         arg.add("joe");
@@ -264,7 +267,7 @@ public class GlobalMethodSecurityBeanDefinitionParserTests {
                 "<global-method-security pre-post-annotations='enabled'/>" +
                 "<b:bean id='target' class='org.springframework.security.access.annotation.ExpressionProtectedBusinessServiceImpl'/>" +
                 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(
+                "<b:bean id='target' class='" + ConcreteFoo.class.getName()  + "'/>" +
+                "<global-method-security pre-post-annotations='enabled'/>" + 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<SecurityConfig> {
+        @PreAuthorize("#action.attribute == 'A'")
         public void foo(SecurityConfig action) {
         }
     }

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]);
         }