diff --git a/build.gradle b/build.gradle index abb4f44417..493b60b89a 100644 --- a/build.gradle +++ b/build.gradle @@ -62,7 +62,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 { diff --git a/config/config.gradle b/config/config.gradle index 497333c508..e8ecb0f285 100644 --- a/config/config.gradle +++ b/config/config.gradle @@ -28,6 +28,7 @@ dependencies { testCompile project(':spring-security-cas'), project(':spring-security-core').sourceSets.test.output, + project(':spring-security-aspects'), 'javax.annotation:jsr250-api:1.0', "org.springframework.ldap:spring-ldap-core:$springLdapVersion", "org.springframework:spring-expression:$springVersion", diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityAspectJAutoProxyRegistrar.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityAspectJAutoProxyRegistrar.java index 2504fcb625..811c452e6f 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityAspectJAutoProxyRegistrar.java +++ b/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 - .registerAspectJAnnotationAutoProxyCreatorIfNecessary(registry); + BeanDefinition interceptor = registry.getBeanDefinition("methodSecurityInterceptor"); - Map annotationAttributes = importingClassMetadata - .getAnnotationAttributes(EnableGlobalMethodSecurity.class - .getName()); - AnnotationAttributes enableAJAutoProxy = AnnotationAttributes - .fromMap(annotationAttributes); + BeanDefinitionBuilder aspect = + BeanDefinitionBuilder.rootBeanDefinition("org.springframework.security.access.intercept.aspectj.aspect.AnnotationSecurityAspect"); + aspect.setFactoryMethod("aspectOf"); + aspect.setRole(BeanDefinition.ROLE_INFRASTRUCTURE); + aspect.addPropertyValue("securityInterceptor", interceptor); - if (enableAJAutoProxy.getBoolean("proxyTargetClass")) { - AopConfigUtils.forceAutoProxyCreatorToUseClassProxying(registry); - } + registry.registerBeanDefinition("annotationSecurityAspect$0", aspect.getBeanDefinition()); } } \ No newline at end of file diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java index 31a2faee3e..e36108013f 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java +++ b/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.Configuration; -import org.springframework.context.annotation.ImportAware; -import org.springframework.context.annotation.Role; +import org.springframework.context.annotation.*; 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) diff --git a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy index 534ad6075e..e289ab5819 100644 --- a/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy @@ -15,11 +15,14 @@ */ package org.springframework.security.config.annotation.method.configuration +import org.springframework.security.access.intercept.aspectj.AspectJMethodSecurityInterceptor + import static org.fest.assertions.Assertions.assertThat import static org.junit.Assert.fail import java.lang.reflect.Method +import org.springframework.security.access.intercept.aspectj.aspect.AnnotationSecurityAspect import org.springframework.aop.aspectj.annotation.AnnotationAwareAspectJAutoProxyCreator import org.springframework.beans.factory.BeanCreationException import org.springframework.context.ConfigurableApplicationContext @@ -188,8 +191,8 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec { when: context = new AnnotationConfigApplicationContext(AspectJModeConfig) then: - AnnotationAwareAspectJAutoProxyCreator autoProxyCreator = context.getBean(AnnotationAwareAspectJAutoProxyCreator) - autoProxyCreator.proxyTargetClass == true + context.getBean(AnnotationSecurityAspect) + context.getBean(AspectJMethodSecurityInterceptor) } @Configuration @@ -201,8 +204,8 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec { when: context = new AnnotationConfigApplicationContext(BaseMethodConfig,AspectJModeExtendsGMSCConfig) then: - AnnotationAwareAspectJAutoProxyCreator autoProxyCreator = context.getBean(AnnotationAwareAspectJAutoProxyCreator) - autoProxyCreator.proxyTargetClass == false + context.getBean(AnnotationSecurityAspect) + context.getBean(AspectJMethodSecurityInterceptor) } @Configuration diff --git a/samples/aspectj-jc/build.gradle b/samples/aspectj-jc/build.gradle new file mode 100644 index 0000000000..2c8a490f90 --- /dev/null +++ b/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') +} diff --git a/samples/aspectj-jc/pom.xml b/samples/aspectj-jc/pom.xml new file mode 100644 index 0000000000..ac9430d1d0 --- /dev/null +++ b/samples/aspectj-jc/pom.xml @@ -0,0 +1,133 @@ + + + 4.0.0 + org.springframework.security + spring-security-samples-aspectj-jc + 4.0.0.CI-SNAPSHOT + spring-security-samples-aspectj-jc + spring-security-samples-aspectj-jc + http://spring.io/spring-security + + spring.io + http://spring.io/ + + + + The Apache Software License, Version 2.0 + http://www.apache.org/licenses/LICENSE-2.0.txt + repo + + + + + rwinch + Rob Winch + rwinch@gopivotal.com + + + + scm:git:git://github.com/spring-projects/spring-security + scm:git:git://github.com/spring-projects/spring-security + https://github.com/spring-projects/spring-security + + + + + maven-compiler-plugin + + 1.7 + 1.7 + + + + + + + spring-snasphot + https://repo.spring.io/snapshot + + + + + org.springframework.security + spring-security-config + 4.0.0.CI-SNAPSHOT + compile + + + org.springframework.security + spring-security-core + 4.0.0.CI-SNAPSHOT + compile + + + org.springframework + spring-core + 4.1.0.BUILD-SNAPSHOT + compile + + + commons-logging + commons-logging + + + + + commons-logging + commons-logging + 1.1.1 + compile + true + + + org.aspectj + aspectjrt + 1.6.10 + compile + true + + + org.springframework.security + spring-security-aspects + 4.0.0.CI-SNAPSHOT + runtime + + + ch.qos.logback + logback-classic + 0.9.29 + test + + + junit + junit + 4.11 + test + + + org.easytesting + fest-assert + 1.4 + test + + + org.mockito + mockito-core + 1.9.5 + test + + + org.slf4j + jcl-over-slf4j + 1.7.5 + test + + + org.springframework + spring-test + 4.1.0.BUILD-SNAPSHOT + test + + + diff --git a/samples/aspectj-jc/src/main/java/sample/aspectj/AspectjSecurityConfig.java b/samples/aspectj-jc/src/main/java/sample/aspectj/AspectjSecurityConfig.java new file mode 100644 index 0000000000..cccff1d85c --- /dev/null +++ b/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(); + } +} diff --git a/samples/aspectj-jc/src/main/java/sample/aspectj/SecuredService.java b/samples/aspectj-jc/src/main/java/sample/aspectj/SecuredService.java new file mode 100644 index 0000000000..b6e9e7f075 --- /dev/null +++ b/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 + } + +} diff --git a/samples/aspectj-jc/src/main/java/sample/aspectj/Service.java b/samples/aspectj-jc/src/main/java/sample/aspectj/Service.java new file mode 100644 index 0000000000..8e450a6045 --- /dev/null +++ b/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 + } + +} diff --git a/samples/aspectj-jc/src/test/java/sample/aspectj/AspectJInterceptorTests.java b/samples/aspectj-jc/src/test/java/sample/aspectj/AspectJInterceptorTests.java new file mode 100644 index 0000000000..a8f04f937c --- /dev/null +++ b/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(); + } +} diff --git a/samples/aspectj-jc/src/test/resources/logback-test.xml b/samples/aspectj-jc/src/test/resources/logback-test.xml new file mode 100644 index 0000000000..f293b342dc --- /dev/null +++ b/samples/aspectj-jc/src/test/resources/logback-test.xml @@ -0,0 +1,14 @@ + + + + %d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n + + + + + + + + + + diff --git a/settings.gradle b/settings.gradle index e4ba8a0b67..5e5d09b4c5 100644 --- a/settings.gradle +++ b/settings.gradle @@ -17,6 +17,7 @@ def String[] samples = [ 'contacts-xml', 'openid-xml', 'aspectj-xml', + 'aspectj-jc', 'gae-xml', 'dms-xml', 'preauth-xml',