SEC-1262: Added new (replacement) AspectJ interceptor which wraps the JoinPoint in a MethodInvocation adapter to provide compatibility with classes which only support MethodInvocation instances.

Also deprecated the existing AspectJ interceptors. This will also allow future simplification of the AbstractMethodSecurityMetadataSource, as it no longer needs to support JoinPoints.
This commit is contained in:
Luke Taylor 2010-03-09 23:39:07 +00:00
parent 2b8b8819e4
commit 55de2cfcb1
13 changed files with 315 additions and 56 deletions

View File

@ -1,5 +1,6 @@
dependencies {
compile project(':spring-security-core'),
"org.springframework:spring-beans:$springVersion"
"org.springframework:spring-beans:$springVersion",
"org.springframework:spring-context:$springVersion"
}

View File

@ -2,11 +2,12 @@ package org.springframework.security.access.intercept.aspectj.aspect;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.access.prepost.*;
import org.springframework.security.access.intercept.aspectj.AspectJCallback;
import org.springframework.security.access.intercept.aspectj.AspectJSecurityInterceptor;
import org.springframework.security.access.intercept.aspectj.AspectJMethodSecurityInterceptor;
/**
* Concrete AspectJ transaction aspect using Spring Security @Secured annotation
* Concrete AspectJ aspect using Spring Security @Secured annotation
* for JDK 1.5+.
*
* <p>
@ -16,8 +17,8 @@ import org.springframework.security.access.intercept.aspectj.AspectJSecurityInte
* interfaces are <i>not</i> inherited. This will vary from Spring AOP.
*
* @author Mike Wiesner
* @since 1.0
* @version $Id$
* @author Luke Taylor
* @since 3.1
*/
public aspect AnnotationSecurityAspect implements InitializingBean {
@ -34,11 +35,19 @@ public aspect AnnotationSecurityAspect implements InitializingBean {
private pointcut executionOfSecuredMethod() :
execution(* *(..)) && @annotation(Secured);
/**
* Matches the execution of any method with Pre/Post annotations.
*/
private pointcut executionOfPrePostAnnotatedMethod() :
execution(* *(..)) && (@annotation(PreAuthorize) || @annotation(PreFilter)
|| @annotation(PostAuthorize) || @annotation(PostFilter));
private pointcut securedMethodExecution() :
executionOfAnyPublicMethodInAtSecuredType() ||
executionOfSecuredMethod();
executionOfSecuredMethod() ||
executionOfPrePostAnnotatedMethod();
private AspectJSecurityInterceptor securityInterceptor;
private AspectJMethodSecurityInterceptor securityInterceptor;
Object around(): securedMethodExecution() {
if (this.securityInterceptor == null) {
@ -54,13 +63,14 @@ public aspect AnnotationSecurityAspect implements InitializingBean {
return this.securityInterceptor.invoke(thisJoinPoint, callback);
}
public void setSecurityInterceptor(AspectJSecurityInterceptor securityInterceptor) {
public void setSecurityInterceptor(AspectJMethodSecurityInterceptor securityInterceptor) {
this.securityInterceptor = securityInterceptor;
}
public void afterPropertiesSet() throws Exception {
if (this.securityInterceptor == null)
if (this.securityInterceptor == null) {
throw new IllegalArgumentException("securityInterceptor required");
}
}
}

View File

@ -0,0 +1,110 @@
package org.springframework.security.access.intercept.aspectj.aspect;
import java.util.Arrays;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.springframework.security.access.AccessDecisionManager;
import org.springframework.security.access.AccessDecisionVoter;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.access.annotation.Secured;
import org.springframework.security.access.annotation.SecuredAnnotationSecurityMetadataSource;
import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler;
import org.springframework.security.access.expression.method.ExpressionBasedAnnotationAttributeFactory;
import org.springframework.security.access.expression.method.ExpressionBasedPreInvocationAdvice;
import org.springframework.security.access.intercept.aspectj.AspectJMethodSecurityInterceptor;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.access.prepost.PreInvocationAuthorizationAdviceVoter;
import org.springframework.security.access.prepost.PrePostAnnotationSecurityMetadataSource;
import org.springframework.security.access.vote.AffirmativeBased;
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.core.context.SecurityContextHolder;
/**
*
* @author Luke Taylor
* @since 3.0.3
*/
public class AnnotationSecurityAspectTests {
private @Mock AccessDecisionManager adm;
private @Mock AuthenticationManager authman;
private TestingAuthenticationToken anne = new TestingAuthenticationToken("anne", "", "ROLE_A");
// private TestingAuthenticationToken bob = new TestingAuthenticationToken("bob", "", "ROLE_B");
private AspectJMethodSecurityInterceptor interceptor;
private SecuredImpl secured = new SecuredImpl();
private PrePostSecured prePostSecured = new PrePostSecured();
@Before
public final void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
interceptor = new AspectJMethodSecurityInterceptor();
interceptor.setAccessDecisionManager(adm);
interceptor.setAuthenticationManager(authman);
interceptor.setSecurityMetadataSource(new SecuredAnnotationSecurityMetadataSource());
AnnotationSecurityAspect secAspect = AnnotationSecurityAspect.aspectOf();
secAspect.setSecurityInterceptor(interceptor);
}
@After
public void clearContext() {
SecurityContextHolder.clearContext();
}
@Test
public void securedInterfaceMethodAllowsAllAccess() throws Exception {
secured.securedMethod();
}
@Test(expected=AuthenticationCredentialsNotFoundException.class)
public void securedClassMethodDeniesUnauthenticatedAccess() throws Exception {
secured.securedClassMethod();
}
@Test
public void securedClassMethodAllowsAccessToRoleA() throws Exception {
SecurityContextHolder.getContext().setAuthentication(anne);
secured.securedClassMethod();
}
// SEC-1262
@Test(expected=AccessDeniedException.class)
public void denyAllPreAuthorizeDeniesAccess() throws Exception {
SecurityContextHolder.getContext().setAuthentication(anne);
interceptor.setSecurityMetadataSource(new PrePostAnnotationSecurityMetadataSource(
new ExpressionBasedAnnotationAttributeFactory(new DefaultMethodSecurityExpressionHandler())));
AffirmativeBased adm = new AffirmativeBased();
AccessDecisionVoter[] voters = new AccessDecisionVoter[]
{new PreInvocationAuthorizationAdviceVoter(new ExpressionBasedPreInvocationAdvice())};
adm.setDecisionVoters(Arrays.asList(voters));
interceptor.setAccessDecisionManager(adm);
prePostSecured.denyAllMethod();
}
}
interface SecuredInterface {
@Secured("ROLE_X")
void securedMethod();
}
class SecuredImpl implements SecuredInterface {
// Not really secured because AspectJ doesn't inherit annotations from interfaces
public void securedMethod() {
}
@Secured("ROLE_A")
public void securedClassMethod() {
}
}
class PrePostSecured {
@PreAuthorize("denyAll")
public void denyAllMethod() {
}
}

View File

@ -6,8 +6,9 @@ package org.springframework.security.access.intercept.aspectj;
* AspectJ processing to continue.
*
* @author Mike Wiesner
* @deprecated
*/
@Deprecated
public interface AspectJAnnotationCallback {
//~ Methods ========================================================================================================

View File

@ -11,7 +11,9 @@ import org.aspectj.lang.JoinPoint;
* AspectJ interceptor that supports @Aspect notation.
*
* @author Mike Wiesner
* @deprecated Use AspectJMethodSecurityInterceptor instead
*/
@Deprecated
public class AspectJAnnotationSecurityInterceptor extends AbstractSecurityInterceptor {
//~ Instance fields ================================================================================================

View File

@ -0,0 +1,51 @@
package org.springframework.security.access.intercept.aspectj;
import org.aspectj.lang.JoinPoint;
import org.springframework.security.access.intercept.InterceptorStatusToken;
import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor;
/**
* AspectJ {@code JoinPoint} security interceptor which wraps the {@code JoinPoint} in a {@code MethodInvocation}
* adapter to make it compatible with security infrastructure classes which only support {@code MethodInvocation}s.
* <p>
* One of the {@code invoke} methods should be called from the {@code around()} advice in your aspect.
* Alternatively you can use one of the pre-defined aspects from the aspects module.
*
* @author Luke Taylor
* @since 3.0.3
*/
public final class AspectJMethodSecurityInterceptor extends MethodSecurityInterceptor {
/**
* Method that is suitable for user with @Aspect notation.
*
* @param jp The AspectJ joint point being invoked which requires a security decision
* @return The returned value from the method invocation
* @throws Throwable if the invocation throws one
*/
public Object invoke(JoinPoint jp) throws Throwable {
return super.invoke(new MethodInvocationAdapter(jp));
}
/**
* Method that is suitable for user with traditional AspectJ-code aspects.
*
* @param jp The AspectJ joint point being invoked which requires a security decision
* @param advisorProceed the advice-defined anonymous class that implements {@code AspectJCallback} containing
* a simple {@code return proceed();} statement
*
* @return The returned value from the method invocation
*/
public Object invoke(JoinPoint jp, AspectJCallback advisorProceed) {
Object result = null;
InterceptorStatusToken token = super.beforeInvocation(new MethodInvocationAdapter(jp));
try {
result = advisorProceed.proceedWithObject();
} finally {
result = super.afterInvocation(token, result);
}
return result;
}
}

View File

@ -37,7 +37,9 @@ import org.aspectj.lang.JoinPoint;
* Refer to {@link AbstractSecurityInterceptor} for details on the workflow.
*
* @author Ben Alex
* @deprecated Use AspectJMethodSecurityInterceptor instead
*/
@Deprecated
public class AspectJSecurityInterceptor extends AbstractSecurityInterceptor {
//~ Instance fields ================================================================================================

View File

@ -0,0 +1,61 @@
package org.springframework.security.access.intercept.aspectj;
import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Method;
import org.aopalliance.intercept.MethodInvocation;
import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.reflect.CodeSignature;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
/**
* Decorates a JoinPoint to allow it to be used with method-security infrastructure
* classes which support {@code MethodInvocation} instances.
*
* @author Luke Taylor
* @since 3.0.3
*/
public final class MethodInvocationAdapter implements MethodInvocation {
private final ProceedingJoinPoint jp;
private final Method method;
private final Object target;
MethodInvocationAdapter(JoinPoint jp) {
this.jp = (ProceedingJoinPoint)jp;
if (jp.getTarget() != null) {
target = jp.getTarget();
} else {
// SEC-1295: target may be null if an ITD is in use
target = jp.getSignature().getDeclaringType();
}
String targetMethodName = jp.getStaticPart().getSignature().getName();
Class<?>[] types = ((CodeSignature) jp.getStaticPart().getSignature()).getParameterTypes();
Class<?> declaringType = ((CodeSignature) jp.getStaticPart().getSignature()).getDeclaringType();
method = ClassUtils.getMethodIfAvailable(declaringType, targetMethodName, types);
Assert.notNull(method, "Could not obtain target method from JoinPoint: '"+ jp + "'");
}
public Method getMethod() {
return method;
}
public Object[] getArguments() {
return jp.getArgs();
}
public AccessibleObject getStaticPart() {
return method;
}
public Object getThis() {
return target;
}
public Object proceed() throws Throwable {
return jp.proceed();
}
}

View File

@ -49,7 +49,13 @@ public abstract class AbstractMethodSecurityMetadataSource implements MethodSecu
if (object instanceof MethodInvocation) {
MethodInvocation mi = (MethodInvocation) object;
Object target = mi.getThis();
return getAttributes(mi.getMethod(), target == null ? null : target.getClass());
Class<?> targetClass = null;
if (target != null) {
targetClass = target instanceof Class<?> ? (Class<?>)target : target.getClass();
}
return getAttributes(mi.getMethod(), targetClass);
}
if (object instanceof JoinPoint) {

View File

@ -15,26 +15,28 @@
package org.springframework.security.access.intercept.aspectj;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.*;
import java.lang.reflect.Method;
import java.util.List;
import java.util.Collection;
import org.aspectj.lang.JoinPoint;
import org.jmock.Expectations;
import org.jmock.Mockery;
import org.jmock.integration.junit4.JUnit4Mockery;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.springframework.security.MockJoinPoint;
import org.springframework.security.TargetObject;
import org.springframework.security.access.AccessDecisionManager;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.access.SecurityConfig;
import org.springframework.security.access.intercept.aspectj.AspectJCallback;
import org.springframework.security.access.intercept.aspectj.AspectJSecurityInterceptor;
import org.springframework.security.access.method.MethodSecurityMetadataSource;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.TestingAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
@ -42,33 +44,33 @@ import org.springframework.security.core.context.SecurityContextHolder;
* Tests {@link AspectJSecurityInterceptor}.
*
* @author Ben Alex
* @author Luke Taylor
*/
@SuppressWarnings("deprecation")
public class AspectJSecurityInterceptorTests {
private Mockery jmock = new JUnit4Mockery();
private TestingAuthenticationToken token;
private AspectJSecurityInterceptor interceptor;
private AccessDecisionManager adm;
private MethodSecurityMetadataSource mds;
private AuthenticationManager authman;
private AspectJCallback aspectJCallback;
private @Mock AccessDecisionManager adm;
private @Mock MethodSecurityMetadataSource mds;
private @Mock AuthenticationManager authman;
private @Mock AspectJCallback aspectJCallback;
private JoinPoint joinPoint;
//~ Methods ========================================================================================================
@Before
public final void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
SecurityContextHolder.clearContext();
token = new TestingAuthenticationToken("Test", "Password");
interceptor = new AspectJSecurityInterceptor();
adm = jmock.mock(AccessDecisionManager.class);
authman = jmock.mock(AuthenticationManager.class);
mds = jmock.mock(MethodSecurityMetadataSource.class);
interceptor.setAccessDecisionManager(adm);
interceptor.setAuthenticationManager(authman);
interceptor.setSecurityMetadataSource(mds);
Method method = TargetObject.class.getMethod("countLength", new Class[] {String.class});
joinPoint = new MockJoinPoint(new TargetObject(), method);
aspectJCallback = jmock.mock(AspectJCallback.class);
when(mds.getAttributes(any(JoinPoint.class))).thenReturn(SecurityConfig.createList("ROLE_USER"));
when(authman.authenticate(token)).thenReturn(token);
}
@After
@ -77,33 +79,23 @@ public class AspectJSecurityInterceptorTests {
}
@Test
@SuppressWarnings("unchecked")
public void callbackIsInvokedWhenPermissionGranted() throws Exception {
jmock.checking(new Expectations() {{
oneOf(mds).getAttributes(with(any(JoinPoint.class))); will (returnValue(SecurityConfig.createList("ROLE_USER")));
oneOf(authman).authenticate(token); will(returnValue(token));
oneOf(adm).decide(with(token), with(aNonNull(JoinPoint.class)), with(aNonNull(List.class)));
oneOf(aspectJCallback).proceedWithObject();
}});
SecurityContextHolder.getContext().setAuthentication(token);
interceptor.invoke(joinPoint, aspectJCallback);
jmock.assertIsSatisfied();
verify(aspectJCallback).proceedWithObject();
}
@SuppressWarnings("unchecked")
@Test(expected=AccessDeniedException.class)
@Test
public void callbackIsNotInvokedWhenPermissionDenied() throws Exception {
jmock.checking(new Expectations() {{
oneOf(mds).getAttributes(with(any(JoinPoint.class))); will (returnValue(SecurityConfig.createList("ROLE_USER")));
oneOf(authman).authenticate(token); will(returnValue(token));
oneOf(adm).decide(with(token), with(aNonNull(JoinPoint.class)), with(aNonNull(List.class)));
will(throwException(new AccessDeniedException("denied")));
never(aspectJCallback).proceedWithObject();
}});
doThrow(new AccessDeniedException("denied")).when(adm).decide(any(Authentication.class), any(), any(Collection.class));
SecurityContextHolder.getContext().setAuthentication(token);
interceptor.invoke(joinPoint, aspectJCallback);
jmock.assertIsSatisfied();
try {
interceptor.invoke(joinPoint, aspectJCallback);
fail("Expected AccessDeniedException");
} catch (AccessDeniedException expected) {
}
verify(aspectJCallback, never()).proceedWithObject();
}
}

View File

@ -10,15 +10,38 @@ dependencies {
compile "org.aspectj:aspectjrt:$aspectjVersion"
}
task compileJava(dependsOn: JavaPlugin.PROCESS_RESOURCES_TASK_NAME, overwrite: true, description: 'Compiles AspectJ Source') << {
println "Running ajc ..."
ant.taskdef(resource: "org/aspectj/tools/ant/taskdefs/aspectjTaskdefs.properties", classpath: configurations.ajtools.asPath)
ant.iajc(classpath: configurations.compile.asPath, fork: 'true', destDir: sourceSets.main.classesDir.absolutePath, source: sourceCompatibility, target: targetCompatibility,
aspectPath: configurations.aspectpath.asPath, sourceRootCopyFilter: '**/*.java') {
sourceroots {
sourceSets.main.java.srcDirs.each {
pathelement(location: it.absolutePath)
task compileJava(overwrite: true, description: 'Compiles AspectJ Source', type: Ajc) {
dependsOn processResources
sourceSet = sourceSets.main
aspectPath = configurations.aspectpath
}
task compileTestJava(overwrite: true, description: 'Compiles AspectJ Test Source', type: Ajc) {
dependsOn processTestResources, compileJava, jar
sourceSet = sourceSets.test
aspectPath = files(configurations.aspectpath, jar.archivePath)
}
class Ajc extends DefaultTask {
@Input
SourceSet sourceSet
@Input
FileCollection aspectPath
@TaskAction
def compile() {
println "Running ajc ..."
ant.taskdef(resource: "org/aspectj/tools/ant/taskdefs/aspectjTaskdefs.properties", classpath: project.configurations.ajtools.asPath)
ant.iajc(classpath: sourceSet.compileClasspath.asPath, fork: 'true', destDir: sourceSet.classesDir.absolutePath,
source: project.convention.plugins.java.sourceCompatibility,
target: project.convention.plugins.java.targetCompatibility,
aspectPath: aspectPath.asPath, sourceRootCopyFilter: '**/*.java', showWeaveInfo: 'true') {
sourceroots {
sourceSet.java.srcDirs.each {
pathelement(location: it.absolutePath)
}
}
}
}
}
}

View File

@ -22,7 +22,7 @@ dependencies {
}
testCompile 'junit:junit:4.7',
'org.mockito:mockito-core:1.7',
'org.mockito:mockito-core:1.8.3',
'org.jmock:jmock:2.5.1',
'org.jmock:jmock-junit4:2.5.1',
'org.hamcrest:hamcrest-core:1.1',

View File

@ -4,7 +4,7 @@
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd">
<bean id="aspectJSecurityInterceptor"
class="org.springframework.security.access.intercept.aspectj.AspectJSecurityInterceptor">
class="org.springframework.security.access.intercept.aspectj.AspectJMethodSecurityInterceptor">
<property name="authenticationManager" ref="authenticationManager" />
<property name="accessDecisionManager" ref="accessDecisionManager" />
<property name="securityMetadataSource">