SEC-2595: @EnableGlobalMethodSecurity AspectJ fixes

build.gradle

@@ -59,7 +59,7 @@ ext.javaProjects = subprojects.findAll { project -> project.name != 'docs' && pr
 ext.sampleProjects = subprojects.findAll { project -> project.name.startsWith('spring-security-samples') }
 ext.itestProjects = subprojects.findAll { project -> project.name.startsWith('itest') }
 ext.coreModuleProjects = javaProjects - sampleProjects - itestProjects
-ext.aspectjProjects = [project(':spring-security-aspects'), project(':spring-security-samples-aspectj-xml')]
+ext.aspectjProjects = [project(':spring-security-aspects'), project(':spring-security-samples-aspectj-xml'), project(':spring-security-samples-aspectj-jc')]
 
 configure(allprojects - javaProjects) {
     task afterEclipseImport {

config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityAspectJAutoProxyRegistrar.java

@@ -18,6 +18,8 @@ package org.springframework.security.config.annotation.method.configuration;
 import java.util.Map;
 
 import org.springframework.aop.config.AopConfigUtils;
+import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.context.annotation.ImportBeanDefinitionRegistrar;
 import org.springframework.core.annotation.AnnotationAttributes;
@@ -50,18 +52,15 @@ class GlobalMethodSecurityAspectJAutoProxyRegistrar implements
             AnnotationMetadata importingClassMetadata,
             BeanDefinitionRegistry registry) {
 
-        AopConfigUtils
+        BeanDefinition interceptor = registry.getBeanDefinition("methodSecurityInterceptor");
-                .registerAspectJAnnotationAutoProxyCreatorIfNecessary(registry);
 
-        Map<String, Object> annotationAttributes = importingClassMetadata
+        BeanDefinitionBuilder aspect =
-                .getAnnotationAttributes(EnableGlobalMethodSecurity.class
+                BeanDefinitionBuilder.rootBeanDefinition("org.springframework.security.access.intercept.aspectj.aspect.AnnotationSecurityAspect");
-                        .getName());
+        aspect.setFactoryMethod("aspectOf");
-        AnnotationAttributes enableAJAutoProxy = AnnotationAttributes
+        aspect.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
-                .fromMap(annotationAttributes);
+        aspect.addPropertyValue("securityInterceptor", interceptor);
 
-        if (enableAJAutoProxy.getBoolean("proxyTargetClass")) {
+        registry.registerBeanDefinition("annotationSecurityAspect$0", aspect.getBeanDefinition());
-            AopConfigUtils.forceAutoProxyCreatorToUseClassProxying(registry);
-        }
     }
 
 }

config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java

@@ -24,10 +24,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.config.BeanDefinition;
-import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.*;
-import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.ImportAware;
-import org.springframework.context.annotation.Role;
 import org.springframework.core.annotation.AnnotationAttributes;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.core.type.AnnotationMetadata;
@@ -48,6 +45,7 @@ import org.springframework.security.access.intercept.AfterInvocationProviderMana
 import org.springframework.security.access.intercept.RunAsManager;
 import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor;
 import org.springframework.security.access.intercept.aopalliance.MethodSecurityMetadataSourceAdvisor;
+import org.springframework.security.access.intercept.aspectj.AspectJMethodSecurityInterceptor;
 import org.springframework.security.access.method.DelegatingMethodSecurityMetadataSource;
 import org.springframework.security.access.method.MethodSecurityMetadataSource;
 import org.springframework.security.access.prepost.PostInvocationAdviceProvider;
@@ -111,7 +109,7 @@ public class GlobalMethodSecurityConfiguration implements ImportAware {
      */
     @Bean
     public MethodInterceptor methodSecurityInterceptor() throws Exception {
-        MethodSecurityInterceptor methodSecurityInterceptor = new MethodSecurityInterceptor();
+        MethodSecurityInterceptor methodSecurityInterceptor = isAspectJ() ? new AspectJMethodSecurityInterceptor() : new MethodSecurityInterceptor();
         methodSecurityInterceptor
                 .setAccessDecisionManager(accessDecisionManager());
         methodSecurityInterceptor
@@ -379,6 +377,10 @@ public class GlobalMethodSecurityConfiguration implements ImportAware {
         return (Integer) enableMethodSecurity().get("order");
     }
 
+    private boolean isAspectJ() {
+        return enableMethodSecurity().getEnum("mode") == AdviceMode.ASPECTJ;
+    }
+
     private AnnotationAttributes enableMethodSecurity() {
         if (enableMethodSecurity == null) {
             // if it is null look at this instance (i.e. a subclass was used)

samples/aspectj-jc/build.gradle

@@ -0,0 +1,9 @@
+
+dependencies {
+    compile project(':spring-security-core'),
+            project(':spring-security-config')
+
+    aspectpath project(':spring-security-aspects')
+
+    runtime project(':spring-security-aspects')
+}

samples/aspectj-jc/src/main/java/sample/aspectj/AspectjSecurityConfig.java

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2002-2013 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package sample.aspectj;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.AdviceMode;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
+import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
+
+/**
+ * @author Rob Winch
+ */
+@Configuration
+@EnableGlobalMethodSecurity(mode = AdviceMode.ASPECTJ,securedEnabled = true)
+public class AspectjSecurityConfig {
+	@Bean
+	public Service service() {
+		return new Service();
+	}
+
+	@Bean
+	public SecuredService securedService() {
+		return new SecuredService();
+	}
+
+	@Autowired
+	public void configureGlobal(AuthenticationManagerBuilder auth) throws Exception {
+		auth.inMemoryAuthentication();
+	}
+}

samples/aspectj-jc/src/main/java/sample/aspectj/SecuredService.java

@@ -0,0 +1,18 @@
+package sample.aspectj;
+
+import org.springframework.security.access.annotation.Secured;
+
+/**
+ * Service which is secured on the class level
+ *
+ * @author Mike Wiesner
+ * @since 3.0
+ */
+@Secured("ROLE_USER")
+public class SecuredService {
+
+    public void secureMethod() {
+        // nothing
+    }
+
+}

samples/aspectj-jc/src/main/java/sample/aspectj/Service.java

@@ -0,0 +1,22 @@
+package sample.aspectj;
+
+import org.springframework.security.access.annotation.Secured;
+
+/**
+ * Service which is secured on method level
+ *
+ * @author Mike Wiesner
+ * @since 1.0
+ */
+public class Service {
+
+    @Secured("ROLE_USER")
+    public void secureMethod() {
+        // nothing
+    }
+
+    public void publicMethod() {
+        // nothing
+    }
+
+}

samples/aspectj-jc/src/test/java/sample/aspectj/AspectJInterceptorTests.java

@@ -0,0 +1,93 @@
+package sample.aspectj;
+
+import org.junit.After;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.AdviceMode;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.security.access.AccessDeniedException;
+import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
+import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.authority.AuthorityUtils;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+
+import java.lang.reflect.Proxy;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+@RunWith(SpringJUnit4ClassRunner.class)
+@ContextConfiguration(classes=AspectjSecurityConfig.class)
+public class AspectJInterceptorTests {
+    private Authentication admin = new UsernamePasswordAuthenticationToken("test", "xxx", AuthorityUtils.createAuthorityList("ROLE_ADMIN"));
+    private Authentication user = new UsernamePasswordAuthenticationToken("test", "xxx", AuthorityUtils.createAuthorityList("ROLE_USER"));
+
+    @Autowired
+    private Service service;
+
+    @Autowired
+    private SecuredService securedService;
+
+    @Test
+    public void publicMethod() throws Exception {
+        service.publicMethod();
+    }
+
+    @Test(expected = AuthenticationCredentialsNotFoundException.class)
+    public void securedMethodNotAuthenticated() throws Exception {
+        service.secureMethod();
+    }
+
+    @Test(expected = AccessDeniedException.class)
+    public void securedMethodWrongRole() throws Exception {
+        SecurityContextHolder.getContext().setAuthentication(admin);
+        service.secureMethod();
+    }
+
+    @Test
+    public void securedMethodEverythingOk() throws Exception {
+        SecurityContextHolder.getContext().setAuthentication(user);
+        service.secureMethod();
+    }
+
+    @Test(expected = AuthenticationCredentialsNotFoundException.class)
+    public void securedClassNotAuthenticated() throws Exception {
+        securedService.secureMethod();
+    }
+
+    @Test(expected = AccessDeniedException.class)
+    public void securedClassWrongRole() throws Exception {
+        SecurityContextHolder.getContext().setAuthentication(admin);
+        securedService.secureMethod();
+    }
+
+    @Test(expected = AccessDeniedException.class)
+    public void securedClassWrongRoleOnNewedInstance() throws Exception {
+        SecurityContextHolder.getContext().setAuthentication(admin);
+        new SecuredService().secureMethod();
+    }
+
+    @Test
+    public void securedClassEverythingOk() throws Exception {
+        SecurityContextHolder.getContext().setAuthentication(user);
+        securedService.secureMethod();
+        new SecuredService().secureMethod();
+    }
+
+    // SEC-2595
+    @Test
+    public void notProxy() {
+        assertThat(Proxy.isProxyClass(securedService.getClass())).isFalse();
+    }
+
+    @After
+    public void tearDown() {
+        SecurityContextHolder.clearContext();
+    }
+}

samples/aspectj-jc/src/test/resources/logback-test.xml

@@ -0,0 +1,14 @@
+<configuration>
+  <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+    <encoder>
+      <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+    </encoder>
+  </appender>
+
+  <logger name="org.springframework.security" level="${sec.log.level}:-WARN"/>
+
+  <root level="${root.level}:-WARN">
+    <appender-ref ref="STDOUT" />
+  </root>
+
+</configuration>

settings.gradle

@@ -18,6 +18,7 @@ def String[] samples = [
     'contacts-xml',
     'openid-xml',
     'aspectj-xml',
+    'aspectj-jc',
     'gae-xml',
     'dms-xml',
     'preauth-xml',