From cc721e4343b8c51bfce2835bf38b48916e18212f Mon Sep 17 00:00:00 2001 From: Torben Knerr Date: Wed, 2 Feb 2011 17:04:27 +0100 Subject: [PATCH] SEC-1665: optionally allowing for private methods to be intercepted with AspectJMethodSecurityInterceptor --- .../aspectj/AspectJMethodSecurityInterceptor.java | 14 ++++- .../intercept/aspectj/MethodInvocationAdapter.java | 37 +++++++++++++- .../org/springframework/security/TargetObject.java | 6 ++ .../AspectJMethodSecurityInterceptorTests.java | 52 +++++++++++++++++++- 4 files changed, 103 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptor.java b/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptor.java index 3ea0231..a7fbb8b 100644 --- a/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptor.java +++ b/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptor.java @@ -16,7 +16,10 @@ import org.springframework.security.access.intercept.aopalliance.MethodSecurityI */ public final class AspectJMethodSecurityInterceptor extends MethodSecurityInterceptor { - /** + // SEC-1665: optionally allow for private methods to be intercepted + private boolean interceptPublicMethodsOnly = true; + + /** * Method that is suitable for user with @Aspect notation. * * @param jp The AspectJ joint point being invoked which requires a security decision @@ -24,7 +27,7 @@ public final class AspectJMethodSecurityInterceptor extends MethodSecurityInterc * @throws Throwable if the invocation throws one */ public Object invoke(JoinPoint jp) throws Throwable { - return super.invoke(new MethodInvocationAdapter(jp)); + return super.invoke(new MethodInvocationAdapter(jp, interceptPublicMethodsOnly)); } /** @@ -37,10 +40,15 @@ public final class AspectJMethodSecurityInterceptor extends MethodSecurityInterc * @return The returned value from the method invocation */ public Object invoke(JoinPoint jp, AspectJCallback advisorProceed) { - InterceptorStatusToken token = super.beforeInvocation(new MethodInvocationAdapter(jp)); + InterceptorStatusToken token = super.beforeInvocation(new MethodInvocationAdapter(jp, interceptPublicMethodsOnly)); Object result = advisorProceed.proceedWithObject(); return super.afterInvocation(token, result); } + + + public void setInterceptPublicMethodsOnly(boolean interceptPublicMethodsOnly) { + this.interceptPublicMethodsOnly= interceptPublicMethodsOnly; + } } diff --git a/core/src/main/java/org/springframework/security/access/intercept/aspectj/MethodInvocationAdapter.java b/core/src/main/java/org/springframework/security/access/intercept/aspectj/MethodInvocationAdapter.java index a664c7c..14667c6 100644 --- a/core/src/main/java/org/springframework/security/access/intercept/aspectj/MethodInvocationAdapter.java +++ b/core/src/main/java/org/springframework/security/access/intercept/aspectj/MethodInvocationAdapter.java @@ -22,7 +22,7 @@ public final class MethodInvocationAdapter implements MethodInvocation { private final Method method; private final Object target; - MethodInvocationAdapter(JoinPoint jp) { + MethodInvocationAdapter(JoinPoint jp, boolean publicOnly) { this.jp = (ProceedingJoinPoint)jp; if (jp.getTarget() != null) { target = jp.getTarget(); @@ -34,10 +34,20 @@ public final class MethodInvocationAdapter implements MethodInvocation { Class[] types = ((CodeSignature) jp.getStaticPart().getSignature()).getParameterTypes(); Class declaringType = ((CodeSignature) jp.getStaticPart().getSignature()).getDeclaringType(); - method = ClassUtils.getMethodIfAvailable(declaringType, targetMethodName, types); + // SEC-1665: allow for interception of private methods + if (publicOnly) { + method = ClassUtils.getMethodIfAvailable(declaringType, targetMethodName, types); + } else { + method = getDeclaredMethodIfAvailable(declaringType, targetMethodName, types); + } Assert.notNull(method, "Could not obtain target method from JoinPoint: '"+ jp + "'"); } + + MethodInvocationAdapter(JoinPoint jp) { + this(jp, true); + } + public Method getMethod() { return method; @@ -58,4 +68,27 @@ public final class MethodInvocationAdapter implements MethodInvocation { public Object proceed() throws Throwable { return jp.proceed(); } + + // XXX: should rather be added to org.springframework.util.ClassUtils + /** + * same as {@link org.springframework.util.ClassUtils#getMethodIfAvailable(Class, String, Class...)} + * but considers non-public methods as well. + * + * @param clazz the clazz to analyze + * @param methodName the name of the method + * @param paramTypes the parameter types of the method + * @return the method, or null if not found + * @see java.lang.Class#getMethod + * @see org.springframework.util.ClassUtils#getMethodIfAvailable(Class, String, Class...) + */ + public static Method getDeclaredMethodIfAvailable(Class clazz, String methodName, Class... paramTypes) { + Assert.notNull(clazz, "Class must not be null"); + Assert.notNull(methodName, "Method name must not be null"); + try { + return clazz.getDeclaredMethod(methodName, paramTypes); + } + catch (NoSuchMethodException ex) { + return null; + } + } } diff --git a/core/src/test/java/org/springframework/security/TargetObject.java b/core/src/test/java/org/springframework/security/TargetObject.java index f754bb3..68a4db2 100644 --- a/core/src/test/java/org/springframework/security/TargetObject.java +++ b/core/src/test/java/org/springframework/security/TargetObject.java @@ -35,6 +35,12 @@ public class TargetObject implements ITargetObject { return input.length(); } + // SEC-1665: require a private method to be intercepted for testing purposes + @SuppressWarnings("unused") + private int getFortyTwo() { + return 42; + } + /** * Returns the lowercase string, followed by security environment information. * diff --git a/core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptorTests.java b/core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptorTests.java index c69fab8..ad2281a 100644 --- a/core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptorTests.java @@ -60,6 +60,7 @@ public class AspectJMethodSecurityInterceptorTests { private @Mock AuthenticationManager authman; private @Mock AspectJCallback aspectJCallback; private ProceedingJoinPoint joinPoint; + //~ Methods ======================================================================================================== @@ -150,5 +151,54 @@ public class AspectJMethodSecurityInterceptorTests { verifyZeroInteractions(aim); } - + + @Test + public void privateMethodsAreNotInterceptedByDefault() throws Throwable { + ProceedingJoinPoint privateJoinPoint = createPrivateProceedingJoinPointMock(); + SecurityContextHolder.getContext().setAuthentication(token); + + // both invoke methods should fail + try { + interceptor.invoke(privateJoinPoint, aspectJCallback); + fail("Expected exception"); + } catch (IllegalArgumentException expected) { + } + try { + interceptor.invoke(privateJoinPoint); + fail("Expected exception"); + } catch (IllegalArgumentException expected) { + } + } + + @Test + public void privateMethodsAreInterceptedIfConfigured() throws Throwable { + ProceedingJoinPoint privateJoinPoint = createPrivateProceedingJoinPointMock(); + SecurityContextHolder.getContext().setAuthentication(token); + + // explicitly allow to intercept private methods + interceptor.setInterceptPublicMethodsOnly(false); + + // no exception should be thrown + interceptor.invoke(privateJoinPoint, aspectJCallback); + verify(aspectJCallback).proceedWithObject(); + interceptor.invoke(privateJoinPoint); + } + + + // mock joinpoint for private method TargetObject.getFortyTwo() + private ProceedingJoinPoint createPrivateProceedingJoinPointMock() { + ProceedingJoinPoint privateJoinPoint = mock(ProceedingJoinPoint.class); + Signature sig = mock(Signature.class); + when(sig.getDeclaringType()).thenReturn(TargetObject.class); + JoinPoint.StaticPart staticPart = mock(JoinPoint.StaticPart.class); + when(privateJoinPoint.getSignature()).thenReturn(sig); + when(privateJoinPoint.getStaticPart()).thenReturn(staticPart); + CodeSignature codeSig = mock(CodeSignature.class); + when(codeSig.getName()).thenReturn("getFortyTwo"); + when(codeSig.getDeclaringType()).thenReturn(TargetObject.class); + when(codeSig.getParameterTypes()).thenReturn(new Class[] {}); + when(staticPart.getSignature()).thenReturn(codeSig); + + return privateJoinPoint; + } } -- 1.7.3.1.msysgit.0