SEC-773: global-method-security fails with JPA

http://jira.springframework.org/browse/SEC-773. Added extra constructor to MethodDefinitionSourceAdvisor to allow for lazy initialization of the advice (MethodSecurityInterceptor), and in turn the AuthenticationManager and ay referenced UserDetailsService implementations.

core-tiger/pom.xml

@@ -15,6 +15,12 @@
             <artifactId>spring-security-core</artifactId>
             <version>${project.version}</version>
         </dependency>
+        <dependency>
+            <groupId>org.springframework.security</groupId>
+            <artifactId>spring-security-core</artifactId>
+            <version>${project.version}</version>
+            <classifier>tests</classifier>
+        </dependency>        
         <dependency>
             <groupId>org.aspectj</groupId>
             <artifactId>aspectjrt</artifactId>

core-tiger/src/test/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParserTests.java

@@ -1,8 +1,11 @@
 package org.springframework.security.config;
 
+import static org.junit.Assert.assertEquals;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.springframework.context.support.AbstractXmlApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.security.AccessDeniedException;
 import org.springframework.security.AuthenticationCredentialsNotFoundException;
@@ -11,17 +14,17 @@ import org.springframework.security.GrantedAuthorityImpl;
 import org.springframework.security.annotation.BusinessService;
 import org.springframework.security.context.SecurityContextHolder;
 import org.springframework.security.providers.UsernamePasswordAuthenticationToken;
+import org.springframework.security.util.InMemoryXmlApplicationContext;
 
 /**
  * @author Ben Alex
  * @version $Id$
  */
 public class GlobalMethodSecurityBeanDefinitionParserTests {
-    private ClassPathXmlApplicationContext appContext;
+    private AbstractXmlApplicationContext appContext;
 
     private BusinessService target;
 
-    @Before
     public void loadContext() {
         appContext = new ClassPathXmlApplicationContext("org/springframework/security/config/global-method-security.xml");
         target = (BusinessService) appContext.getBean("target");
@@ -37,11 +40,13 @@ public class GlobalMethodSecurityBeanDefinitionParserTests {
 
     @Test(expected=AuthenticationCredentialsNotFoundException.class)
     public void targetShouldPreventProtectedMethodInvocationWithNoContext() {
+    	loadContext();
         target.someUserMethod1();
     }
 
     @Test
     public void targetShouldAllowProtectedMethodInvocationWithCorrectRole() {
+    	loadContext();
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
                 new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_USER")});
         SecurityContextHolder.getContext().setAuthentication(token);
@@ -51,10 +56,30 @@ public class GlobalMethodSecurityBeanDefinitionParserTests {
 
     @Test(expected=AccessDeniedException.class)
     public void targetShouldPreventProtectedMethodInvocationWithIncorrectRole() {
+    	loadContext();
         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password",
                 new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_SOMEOTHERROLE")});
         SecurityContextHolder.getContext().setAuthentication(token);
 
         target.someAdminMethod();
     }
+    
+    @Test
+    public void doesntInterfereWithBeanPostProcessing() {
+        setContext(
+                "<b:bean id='myUserService' class='org.springframework.security.config.PostProcessedMockUserDetailsService'/>" +
+                "<global-method-security />" +
+              //  "<http auto-config='true'/>" +
+                "<authentication-provider user-service-ref='myUserService'/>" +
+                "<b:bean id='beanPostProcessor' class='org.springframework.security.config.MockUserServiceBeanPostProcessor'/>"
+        );
+
+        PostProcessedMockUserDetailsService service = (PostProcessedMockUserDetailsService)appContext.getBean("myUserService");
+
+        assertEquals("Hello from the post processor!", service.getPostProcessorWasHere());
+    }
+    
+    private void setContext(String context) {
+        appContext = new InMemoryXmlApplicationContext(context);
+    }
 }

core/src/main/java/org/springframework/security/config/GlobalMethodSecurityBeanDefinitionParser.java

@@ -138,7 +138,10 @@ class GlobalMethodSecurityBeanDefinitionParser implements BeanDefinitionParser {
         RootBeanDefinition advisor = new RootBeanDefinition(MethodDefinitionSourceAdvisor.class);
         advisor.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
         advisor.setSource(parserContext.extractSource(element));
-        advisor.getConstructorArgumentValues().addGenericArgumentValue(interceptor);
+        advisor.getConstructorArgumentValues().addGenericArgumentValue(BeanIds.METHOD_SECURITY_INTERCEPTOR);
+        advisor.getConstructorArgumentValues().addGenericArgumentValue(new RuntimeBeanReference(BeanIds.DELEGATING_METHOD_DEFINITION_SOURCE));
+
+        //advisor.getConstructorArgumentValues().addGenericArgumentValue(interceptor);
         parserContext.getRegistry().registerBeanDefinition(BeanIds.METHOD_DEFINITION_SOURCE_ADVISOR, advisor);
 
         AopNamespaceUtils.registerAutoProxyCreatorIfNecessary(parserContext, element);

core/src/main/java/org/springframework/security/intercept/method/aopalliance/MethodDefinitionSourceAdvisor.java

@@ -23,13 +23,18 @@ import org.aopalliance.intercept.MethodInvocation;
 import org.springframework.aop.Pointcut;
 import org.springframework.aop.support.AbstractPointcutAdvisor;
 import org.springframework.aop.support.StaticMethodMatcherPointcut;
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.BeanFactory;
+import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.security.intercept.method.MethodDefinitionSource;
 import org.springframework.util.Assert;
 
 /**
  * Advisor driven by a {@link MethodDefinitionSource}, used to exclude a {@link MethodSecurityInterceptor} from
- * public (ie non-secure) methods.<p>Because the AOP framework caches advice calculations, this is normally faster
+ * public (ie non-secure) methods.
- * than just letting the <code>MethodSecurityInterceptor</code> run and find out itself that it has no work to do.
+ * <p>
+ * Because the AOP framework caches advice calculations, this is normally faster than just letting the 
+ * <code>MethodSecurityInterceptor</code> run and find out itself that it has no work to do.
  * <p>
  * This class also allows the use of Spring's
  * {@link org.springframework.aop.framework.autoproxy.DefaultAdvisorAutoProxyCreator}, which makes
@@ -42,22 +47,47 @@ import org.springframework.util.Assert;
  * @author Ben Alex
  * @version $Id$
  */
-public class MethodDefinitionSourceAdvisor extends AbstractPointcutAdvisor {
+public class MethodDefinitionSourceAdvisor extends AbstractPointcutAdvisor implements BeanFactoryAware {
     //~ Instance fields ================================================================================================
 
     private MethodDefinitionSource attributeSource;
     private MethodSecurityInterceptor interceptor;
-    private Pointcut pointcut;
+    private Pointcut pointcut = new MethodDefinitionSourcePointcut();
+    private BeanFactory beanFactory;
+    private String adviceBeanName;
+    private final Object adviceMonitor = new Object();
 
     //~ Constructors ===================================================================================================
 
+    /**
+     * @deprecated use the decoupled approach instead
+     */
     public MethodDefinitionSourceAdvisor(MethodSecurityInterceptor advice) {
+    	Assert.notNull(advice.getObjectDefinitionSource(), "Cannot construct a MethodDefinitionSourceAdvisor using a " +
+    			"MethodSecurityInterceptor that has no ObjectDefinitionSource configured");
+
     	this.interceptor = advice;
+        this.attributeSource = advice.getObjectDefinitionSource();
+    }
     
-    	Assert.notNull(advice.getObjectDefinitionSource(), "Cannot construct a MethodDefinitionSourceAdvisor using a MethodSecurityInterceptor that has no ObjectDefinitionSource configured");
+    /**
+     * Alternative constructor for situations where we want the advisor decoupled from the advice. Instead the advice
+     * bean name should be set. This prevents eager instantiation of the interceptor 
+     * (and hence the AuthenticationManager). See SEC-773, for example.
+     * <p>
+     * This is essentially the approach taken by subclasses of {@link AbstractBeanFactoryPointcutAdvisor}, which this
+     * class should extend in future. The original hierarchy and constructor have been retained for backwards 
+     * compatibilty. 
+     * 
+     * @param adviceBeanName name of the MethodSecurityInterceptor bean
+     * @param attributeSource the attribute source (should be the same as the one used on the interceptor)
+     */
+    public MethodDefinitionSourceAdvisor(String adviceBeanName, MethodDefinitionSource attributeSource) {
+    	Assert.notNull(adviceBeanName, "The adviceBeanName cannot be null");
+    	Assert.notNull(attributeSource, "The attributeSource cannot be null");
     	
-        this.attributeSource = advice.getObjectDefinitionSource();
+		this.adviceBeanName = adviceBeanName;
-        this.pointcut = new MethodDefinitionSourcePointcut();
+		this.attributeSource = attributeSource;
     }
 
     //~ Methods ========================================================================================================
@@ -67,7 +97,20 @@ public class MethodDefinitionSourceAdvisor extends AbstractPointcutAdvisor {
 	}
 
 	public Advice getAdvice() {
-		return interceptor;
+		synchronized (this.adviceMonitor) {
+			if (interceptor == null) {
+				Assert.notNull(adviceBeanName, "'adviceBeanName' must be set for use with bean factory lookup.");
+				Assert.state(beanFactory != null, "BeanFactory must be set to resolve 'adviceBeanName'");
+				interceptor = (MethodSecurityInterceptor) 
+						beanFactory.getBean(this.adviceBeanName, MethodSecurityInterceptor.class);
+				attributeSource = interceptor.getObjectDefinitionSource();
+			}
+			return interceptor;
+		}
+	}
+	
+	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
+		this.beanFactory = beanFactory;
 	}
 
     //~ Inner Classes ==================================================================================================