diff --git a/core/src/main/java/org/springframework/security/access/method/AbstractMethodSecurityMetadataSource.java b/core/src/main/java/org/springframework/security/access/method/AbstractMethodSecurityMetadataSource.java index 50259d2ac1..7fa9082b79 100644 --- a/core/src/main/java/org/springframework/security/access/method/AbstractMethodSecurityMetadataSource.java +++ b/core/src/main/java/org/springframework/security/access/method/AbstractMethodSecurityMetadataSource.java @@ -46,8 +46,14 @@ public abstract class AbstractMethodSecurityMetadataSource implements MethodSecu if (target != null) { targetClass = target instanceof Class ? (Class)target : AopProxyUtils.ultimateTargetClass(target); } - - return getAttributes(mi.getMethod(), targetClass); + Collection attrs = getAttributes(mi.getMethod(), targetClass); + if(attrs != null && !attrs.isEmpty()) { + return attrs; + } + if(target != null && !(target instanceof Class)) { + attrs = getAttributes(mi.getMethod(), target.getClass()); + } + return attrs; } throw new IllegalArgumentException("Object must be a non-null MethodInvocation"); diff --git a/core/src/test/java/org/springframework/security/access/annotation/Jsr250MethodDefinitionSourceTests.java b/core/src/test/java/org/springframework/security/access/annotation/Jsr250MethodDefinitionSourceTests.java index 3e0781583d..4058e3dcde 100644 --- a/core/src/test/java/org/springframework/security/access/annotation/Jsr250MethodDefinitionSourceTests.java +++ b/core/src/test/java/org/springframework/security/access/annotation/Jsr250MethodDefinitionSourceTests.java @@ -1,3 +1,18 @@ +/* + * Copyright 2002-2014 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 org.springframework.security.access.annotation; import static org.fest.assertions.Assertions.assertThat; @@ -8,8 +23,7 @@ import java.util.Collection; import javax.annotation.security.PermitAll; import javax.annotation.security.RolesAllowed; -import junit.framework.Assert; - +import org.junit.Assert; import org.junit.Test; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.access.intercept.method.MockMethodInvocation; diff --git a/core/src/test/java/org/springframework/security/access/annotation/SecuredAnnotationSecurityMetadataSourceTests.java b/core/src/test/java/org/springframework/security/access/annotation/SecuredAnnotationSecurityMetadataSourceTests.java index 56b5699bd8..eda1e6755d 100644 --- a/core/src/test/java/org/springframework/security/access/annotation/SecuredAnnotationSecurityMetadataSourceTests.java +++ b/core/src/test/java/org/springframework/security/access/annotation/SecuredAnnotationSecurityMetadataSourceTests.java @@ -14,13 +14,11 @@ */ package org.springframework.security.access.annotation; -import static org.junit.Assert.*; - -import org.junit.*; -import org.springframework.security.access.ConfigAttribute; -import org.springframework.security.access.SecurityConfig; -import org.springframework.security.access.intercept.method.MockMethodInvocation; -import org.springframework.security.core.GrantedAuthority; +import static org.fest.assertions.Assertions.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.lang.annotation.ElementType; import java.lang.annotation.Inherited; @@ -28,7 +26,17 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.Method; -import java.util.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.EnumSet; +import java.util.List; + +import org.junit.Test; +import org.springframework.security.access.ConfigAttribute; +import org.springframework.security.access.SecurityConfig; +import org.springframework.security.access.annotation.sec2150.MethodInvocationFactory; +import org.springframework.security.access.intercept.method.MockMethodInvocation; +import org.springframework.security.core.GrantedAuthority; /** @@ -178,6 +186,14 @@ public class SecuredAnnotationSecurityMetadataSourceTests { assertEquals("CUSTOM", attrs[0].getAttribute()); } + @Test + public void proxyFactoryInterfaceAttributesFound() throws Exception { + MockMethodInvocation mi = MethodInvocationFactory.createSec2150MethodInvocation(); + Collection attributes = mds.getAttributes(mi); + assertThat(attributes.size()).isEqualTo(1); + assertThat(attributes).onProperty("attribute").containsOnly("ROLE_PERSON"); + } + // Inner classes class Department extends Entity { public Department(String name) { @@ -190,6 +206,7 @@ public class SecuredAnnotationSecurityMetadataSourceTests { Department someUserMethod3(Department dept); } + @SuppressWarnings("serial") class DepartmentServiceImpl extends BusinessServiceImpl implements DepartmentService { @Secured({"ROLE_ADMIN"}) public Department someUserMethod3(final Department dept) { diff --git a/core/src/test/java/org/springframework/security/access/annotation/sec2150/CrudRepository.java b/core/src/test/java/org/springframework/security/access/annotation/sec2150/CrudRepository.java new file mode 100644 index 0000000000..11a70310f4 --- /dev/null +++ b/core/src/test/java/org/springframework/security/access/annotation/sec2150/CrudRepository.java @@ -0,0 +1,21 @@ +/* + * 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 org.springframework.security.access.annotation.sec2150; + +public interface CrudRepository { + + Iterable findAll(); +} \ No newline at end of file diff --git a/core/src/test/java/org/springframework/security/access/annotation/sec2150/MethodInvocationFactory.java b/core/src/test/java/org/springframework/security/access/annotation/sec2150/MethodInvocationFactory.java new file mode 100644 index 0000000000..65cca0dcfc --- /dev/null +++ b/core/src/test/java/org/springframework/security/access/annotation/sec2150/MethodInvocationFactory.java @@ -0,0 +1,36 @@ +/* + * Copyright 2002-2014 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 org.springframework.security.access.annotation.sec2150; + +import org.springframework.aop.framework.ProxyFactory; +import org.springframework.security.access.intercept.method.MockMethodInvocation; + +public class MethodInvocationFactory { + + /** + * In order to reproduce the bug for SEC-2150, we must have a proxy object + * that implements TargetSourceAware and implements our annotated interface. + * + * @return + * @throws NoSuchMethodException + */ + public static MockMethodInvocation createSec2150MethodInvocation() throws NoSuchMethodException { + ProxyFactory factory = new ProxyFactory(new Class[] {PersonRepository.class}); + factory.setTargetClass(CrudRepository.class); + PersonRepository repository = (PersonRepository) factory.getProxy(); + return new MockMethodInvocation(repository, PersonRepository.class , "findAll"); + } +} diff --git a/core/src/test/java/org/springframework/security/access/annotation/sec2150/PersonRepository.java b/core/src/test/java/org/springframework/security/access/annotation/sec2150/PersonRepository.java new file mode 100644 index 0000000000..c7d80ec446 --- /dev/null +++ b/core/src/test/java/org/springframework/security/access/annotation/sec2150/PersonRepository.java @@ -0,0 +1,30 @@ +/* + * 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 org.springframework.security.access.annotation.sec2150; + +import org.springframework.security.access.annotation.Secured; +import org.springframework.security.access.prepost.PreAuthorize; + +/** + * Note that JSR-256 states that annotations have no impact when placed on + * interfaces, so SEC-2150 is not impacted by JSR-256 support. + * + * @author Rob Winch + * + */ +@Secured("ROLE_PERSON") +@PreAuthorize("hasRole('ROLE_PERSON')") +public interface PersonRepository extends CrudRepository {} \ No newline at end of file diff --git a/core/src/test/java/org/springframework/security/access/expression/method/PrePostAnnotationSecurityMetadataSourceTests.java b/core/src/test/java/org/springframework/security/access/expression/method/PrePostAnnotationSecurityMetadataSourceTests.java index 4a77ce43ff..f39375b2aa 100644 --- a/core/src/test/java/org/springframework/security/access/expression/method/PrePostAnnotationSecurityMetadataSourceTests.java +++ b/core/src/test/java/org/springframework/security/access/expression/method/PrePostAnnotationSecurityMetadataSourceTests.java @@ -1,23 +1,31 @@ package org.springframework.security.access.expression.method; -import static org.junit.Assert.*; +import static org.fest.assertions.Assertions.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import java.lang.annotation.ElementType; import java.lang.annotation.Inherited; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.Collection; import java.util.List; import org.junit.Before; import org.junit.Test; +import org.springframework.expression.Expression; import org.springframework.security.access.ConfigAttribute; +import org.springframework.security.access.annotation.sec2150.MethodInvocationFactory; import org.springframework.security.access.intercept.method.MockMethodInvocation; import org.springframework.security.access.prepost.PostAuthorize; import org.springframework.security.access.prepost.PostFilter; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.access.prepost.PreFilter; import org.springframework.security.access.prepost.PrePostAnnotationSecurityMetadataSource; +import org.springframework.test.util.ReflectionTestUtils; /** * @@ -148,6 +156,15 @@ public class PrePostAnnotationSecurityMetadataSourceTests { assertEquals(1, attrs.length); } + @Test + public void proxyFactoryInterfaceAttributesFound() throws Exception { + MockMethodInvocation mi = MethodInvocationFactory.createSec2150MethodInvocation(); + Collection attributes = mds.getAttributes(mi); + assertThat(attributes.size()).isEqualTo(1); + Expression expression = (Expression) ReflectionTestUtils.getField(attributes.iterator().next(),"authorizeExpression"); + assertThat(expression.getExpressionString()).isEqualTo("hasRole('ROLE_PERSON')"); + } + //~ Inner Classes ================================================================================================== public static interface ReturnVoid {