diff --git a/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJAnnotationCallback.java b/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJAnnotationCallback.java deleted file mode 100644 index 0fd833b7e5..0000000000 --- a/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJAnnotationCallback.java +++ /dev/null @@ -1,16 +0,0 @@ -package org.springframework.security.access.intercept.aspectj; - - -/** - * Called by the {@link AspectJAnnotationSecurityInterceptor} when it wishes for the - * AspectJ processing to continue. - * - * @author Mike Wiesner - * @deprecated - */ -@Deprecated -public interface AspectJAnnotationCallback { - //~ Methods ======================================================================================================== - - Object proceedWithObject() throws Throwable; -} diff --git a/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJAnnotationSecurityInterceptor.java b/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJAnnotationSecurityInterceptor.java deleted file mode 100644 index ee29425010..0000000000 --- a/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJAnnotationSecurityInterceptor.java +++ /dev/null @@ -1,62 +0,0 @@ -package org.springframework.security.access.intercept.aspectj; - -import org.springframework.security.access.SecurityMetadataSource; -import org.springframework.security.access.intercept.AbstractSecurityInterceptor; -import org.springframework.security.access.intercept.InterceptorStatusToken; -import org.springframework.security.access.method.MethodSecurityMetadataSource; - -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 ================================================================================================ - - private MethodSecurityMetadataSource securityMetadataSource; - - //~ Methods ======================================================================================================== - - public MethodSecurityMetadataSource getSecurityMetadataSource() { - return this.securityMetadataSource; - } - - public Class getSecureObjectClass() { - return JoinPoint.class; - } - - /** - * This method should be used to enforce security on a JoinPoint. - * - * @param jp The AspectJ joint point being invoked which requires a security decision - * @param advisorProceed the advice-defined anonymous class that implements AspectJCallback containing - * a simple return proceed(); statement - * - * @return The returned value from the method invocation - */ - public Object invoke(JoinPoint jp, AspectJAnnotationCallback advisorProceed) throws Throwable { - Object result = null; - InterceptorStatusToken token = super.beforeInvocation(jp); - - try { - result = advisorProceed.proceedWithObject(); - } finally { - result = super.afterInvocation(token, result); - } - - return result; - } - - public SecurityMetadataSource obtainSecurityMetadataSource() { - return this.securityMetadataSource; - } - - public void setSecurityMetadataSource(MethodSecurityMetadataSource newSource) { - this.securityMetadataSource = newSource; - } - -} diff --git a/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJSecurityInterceptor.java b/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJSecurityInterceptor.java deleted file mode 100644 index 76837f8d1d..0000000000 --- a/core/src/main/java/org/springframework/security/access/intercept/aspectj/AspectJSecurityInterceptor.java +++ /dev/null @@ -1,83 +0,0 @@ -/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * 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.intercept.aspectj; - -import org.springframework.security.access.SecurityMetadataSource; -import org.springframework.security.access.intercept.AbstractSecurityInterceptor; -import org.springframework.security.access.intercept.InterceptorStatusToken; -import org.springframework.security.access.method.MethodSecurityMetadataSource; - -import org.aspectj.lang.JoinPoint; - - -/** - * Provides security interception of AspectJ method invocations. - *

- * The SecurityMetadataSource required by this security interceptor is of type - * {@link MethodSecurityMetadataSource}. This is shared with the AOP Alliance based security interceptor - * (MethodSecurityInterceptor), since both work with Java Methods. - *

- * The secure object type is org.aspectj.lang.JoinPoint, which is passed from the relevant - * around() advice. The around() advice also passes an anonymous implementation of {@link - * AspectJCallback} which contains the call for AspectJ to continue processing: return proceed();. - *

- * Refer to {@link AbstractSecurityInterceptor} for details on the workflow. - * - * @author Ben Alex - * @deprecated Use AspectJMethodSecurityInterceptor instead - */ -@Deprecated -public class AspectJSecurityInterceptor extends AbstractSecurityInterceptor { - //~ Instance fields ================================================================================================ - - private MethodSecurityMetadataSource securityMetadataSource; - - //~ Methods ======================================================================================================== - - public Class getSecureObjectClass() { - return JoinPoint.class; - } - - /** - * This method should be used to enforce security on a JoinPoint. - * - * @param jp The AspectJ joint point being invoked which requires a security decision - * @param advisorProceed the advice-defined anonymous class that implements AspectJCallback containing - * a simple 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(jp); - - try { - result = advisorProceed.proceedWithObject(); - } finally { - result = super.afterInvocation(token, result); - } - - return result; - } - - public SecurityMetadataSource obtainSecurityMetadataSource() { - return this.securityMetadataSource; - } - - public void setSecurityMetadataSource(MethodSecurityMetadataSource newSource) { - this.securityMetadataSource = newSource; - } -} 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 2146838b9a..3c6a42bb4a 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 @@ -20,13 +20,12 @@ import java.util.Collection; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.aspectj.lang.JoinPoint; import org.springframework.security.access.ConfigAttribute; /** * Abstract implementation of MethodSecurityMetadataSource which resolves the secured object type to - * either a MethodInvocation or a JoinPoint. + * a MethodInvocation. * * @author Ben Alex * @author Luke Taylor @@ -54,6 +53,6 @@ public abstract class AbstractMethodSecurityMetadataSource implements MethodSecu } public final boolean supports(Class clazz) { - return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz)); + return (MethodInvocation.class.isAssignableFrom(clazz)); } } diff --git a/core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java b/core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java index f7e734a170..0c1efead41 100644 --- a/core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java +++ b/core/src/main/java/org/springframework/security/access/vote/AbstractAclVoter.java @@ -14,14 +14,9 @@ */ package org.springframework.security.access.vote; +import org.aopalliance.intercept.MethodInvocation; import org.springframework.security.access.AccessDecisionVoter; import org.springframework.security.access.AuthorizationServiceException; - -import org.aopalliance.intercept.MethodInvocation; - -import org.aspectj.lang.JoinPoint; -import org.aspectj.lang.reflect.CodeSignature; - import org.springframework.util.Assert; @@ -42,15 +37,9 @@ public abstract class AbstractAclVoter implements AccessDecisionVoter { Object[] args; Class[] params; - if (secureObject instanceof MethodInvocation) { - MethodInvocation invocation = (MethodInvocation) secureObject; - params = invocation.getMethod().getParameterTypes(); - args = invocation.getArguments(); - } else { - JoinPoint jp = (JoinPoint) secureObject; - params = ((CodeSignature) jp.getStaticPart().getSignature()).getParameterTypes(); - args = jp.getArgs(); - } + MethodInvocation invocation = (MethodInvocation) secureObject; + params = invocation.getMethod().getParameterTypes(); + args = invocation.getArguments(); for (int i = 0; i < params.length; i++) { if (processDomainObjectClass.isAssignableFrom(params[i])) { @@ -80,6 +69,6 @@ public abstract class AbstractAclVoter implements AccessDecisionVoter { * @return true if the secure object is MethodInvocation, false otherwise */ public boolean supports(Class clazz) { - return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz)); + return (MethodInvocation.class.isAssignableFrom(clazz)); } } diff --git a/core/src/test/java/org/springframework/security/MockJoinPoint.java b/core/src/test/java/org/springframework/security/MockJoinPoint.java deleted file mode 100644 index 964a409a97..0000000000 --- a/core/src/test/java/org/springframework/security/MockJoinPoint.java +++ /dev/null @@ -1,168 +0,0 @@ -/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * 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; - -import org.aspectj.lang.JoinPoint; -import org.aspectj.lang.Signature; -import org.aspectj.lang.reflect.CodeSignature; -import org.aspectj.lang.reflect.SourceLocation; - -import java.lang.reflect.Method; - - -/** - * A mock AspectJ JoinPoint. - * - * @author Ben Alex - */ -@SuppressWarnings("unchecked") -public class MockJoinPoint implements JoinPoint { - //~ Instance fields ================================================================================================ - - private Method beingInvoked; - private Object object; - private Class declaringType; - private Object[] args; - - //~ Constructors =================================================================================================== - - public MockJoinPoint(Object object, Method beingInvoked, Object... args) { - this.object = object; - this.beingInvoked = beingInvoked; - this.declaringType = object.getClass(); - this.args = args; - } - - //~ Methods ======================================================================================================== - - public Object[] getArgs() { - return args; - } - - public String getKind() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public Signature getSignature() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public SourceLocation getSourceLocation() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public StaticPart getStaticPart() { - return new MockStaticPart(beingInvoked, declaringType); - } - - public Object getTarget() { - return object; - } - - public Object getThis() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public String toLongString() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public String toShortString() { - throw new UnsupportedOperationException("mock not implemented"); - } - - //~ Inner Classes ================================================================================================== - - private class MockCodeSignature implements CodeSignature { - private Method beingInvoked; - private Class declaringType; - - public MockCodeSignature(Method beingInvoked, Class declaringType) { - this.beingInvoked = beingInvoked; - this.declaringType = declaringType; - } - - public Class getDeclaringType() { - return this.declaringType; - } - - public String getDeclaringTypeName() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public Class[] getExceptionTypes() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public int getModifiers() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public String getName() { - return beingInvoked.getName(); - } - - public String[] getParameterNames() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public Class[] getParameterTypes() { - return beingInvoked.getParameterTypes(); - } - - public String toLongString() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public String toShortString() { - throw new UnsupportedOperationException("mock not implemented"); - } - } - - private class MockStaticPart implements StaticPart { - private Method beingInvoked; - private Class declaringType; - - public MockStaticPart(Method beingInvoked, Class declaringType) { - this.beingInvoked = beingInvoked; - this.declaringType = declaringType; - } - - public String getKind() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public Signature getSignature() { - return new MockCodeSignature(beingInvoked, declaringType); - } - - public SourceLocation getSourceLocation() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public String toLongString() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public String toShortString() { - throw new UnsupportedOperationException("mock not implemented"); - } - - public int getId() { - throw new UnsupportedOperationException("mock not implemented"); - } - } -} diff --git a/core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJSecurityInterceptorTests.java b/core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptorTests.java similarity index 73% rename from core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJSecurityInterceptorTests.java rename to core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptorTests.java index fb160cbace..cbbd71c4f4 100644 --- a/core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJSecurityInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/access/intercept/aspectj/AspectJMethodSecurityInterceptorTests.java @@ -23,6 +23,9 @@ import java.lang.reflect.Method; import java.util.Collection; import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.ProceedingJoinPoint; +import org.aspectj.lang.Signature; +import org.aspectj.lang.reflect.CodeSignature; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -41,20 +44,20 @@ import org.springframework.security.core.context.SecurityContextHolder; /** - * Tests {@link AspectJSecurityInterceptor}. + * Tests {@link AspectJMethodSecurityInterceptor}. * * @author Ben Alex * @author Luke Taylor */ @SuppressWarnings("deprecation") -public class AspectJSecurityInterceptorTests { +public class AspectJMethodSecurityInterceptorTests { private TestingAuthenticationToken token; - private AspectJSecurityInterceptor interceptor; + private AspectJMethodSecurityInterceptor interceptor; private @Mock AccessDecisionManager adm; private @Mock MethodSecurityMetadataSource mds; private @Mock AuthenticationManager authman; private @Mock AspectJCallback aspectJCallback; - private JoinPoint joinPoint; + private ProceedingJoinPoint joinPoint; //~ Methods ======================================================================================================== @@ -63,12 +66,23 @@ public class AspectJSecurityInterceptorTests { MockitoAnnotations.initMocks(this); SecurityContextHolder.clearContext(); token = new TestingAuthenticationToken("Test", "Password"); - interceptor = new AspectJSecurityInterceptor(); + interceptor = new AspectJMethodSecurityInterceptor(); 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); + // Set up joinpoint information for the countLength method on TargetObject + joinPoint = mock(ProceedingJoinPoint.class); //new MockJoinPoint(new TargetObject(), method); + Signature sig = mock(Signature.class); + when(sig.getDeclaringType()).thenReturn(TargetObject.class); + JoinPoint.StaticPart staticPart = mock(JoinPoint.StaticPart.class); + when(joinPoint.getSignature()).thenReturn(sig); + when(joinPoint.getStaticPart()).thenReturn(staticPart); + CodeSignature codeSig = mock(CodeSignature.class); + when(codeSig.getName()).thenReturn("countLength"); + when(codeSig.getDeclaringType()).thenReturn(TargetObject.class); + when(codeSig.getParameterTypes()).thenReturn(new Class[] {String.class}); + when(staticPart.getSignature()).thenReturn(codeSig); when(mds.getAttributes(any(JoinPoint.class))).thenReturn(SecurityConfig.createList("ROLE_USER")); when(authman.authenticate(token)).thenReturn(token); } @@ -79,10 +93,13 @@ public class AspectJSecurityInterceptorTests { } @Test - public void callbackIsInvokedWhenPermissionGranted() throws Exception { + public void callbackIsInvokedWhenPermissionGranted() throws Throwable { SecurityContextHolder.getContext().setAuthentication(token); interceptor.invoke(joinPoint, aspectJCallback); verify(aspectJCallback).proceedWithObject(); + + // Just try the other method too + interceptor.invoke(joinPoint); } @SuppressWarnings("unchecked") diff --git a/core/src/test/java/org/springframework/security/access/intercept/method/MockMethodSecurityMetadataSource.java b/core/src/test/java/org/springframework/security/access/intercept/method/MockMethodSecurityMetadataSource.java deleted file mode 100644 index c33eee59e0..0000000000 --- a/core/src/test/java/org/springframework/security/access/intercept/method/MockMethodSecurityMetadataSource.java +++ /dev/null @@ -1,78 +0,0 @@ -/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited - * - * 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.intercept.method; - -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; - -import org.aopalliance.intercept.MethodInvocation; -import org.aspectj.lang.JoinPoint; -import org.springframework.security.access.ConfigAttribute; -import org.springframework.security.access.SecurityConfig; -import org.springframework.security.access.method.MethodSecurityMetadataSource; - - -/** - * @author Ben Alex - */ -public class MockMethodSecurityMetadataSource implements MethodSecurityMetadataSource { - //~ Instance fields ================================================================================================ - - private List list; - private boolean returnACollection; - - //~ Constructors =================================================================================================== - - public MockMethodSecurityMetadataSource(boolean includeInvalidAttributes, boolean returnACollectionWhenRequested) { - returnACollection = returnACollectionWhenRequested; - list = new ArrayList(); - - if (includeInvalidAttributes) { - list.addAll(SecurityConfig.createList("MOCK_LOWER","INVALID_ATTRIBUTE")); - } - - list.addAll(SecurityConfig.createList("MOCK_LOWER", "MOCK_UPPER", "RUN_AS_")); - - if (includeInvalidAttributes) { - list.addAll(SecurityConfig.createList("MOCK_SOMETHING", "ANOTHER_INVALID")); - } - } - - //~ Methods ======================================================================================================== - - public Collection getAllConfigAttributes() { - if (returnACollection) { - return list; - } else { - return null; - } - } - - public Collection getAttributes(Object object) throws IllegalArgumentException { - throw new UnsupportedOperationException("mock method not implemented"); - } - - public Collection getAttributes(Method method, Class targetClass) { - throw new UnsupportedOperationException("mock method not implemented"); - } - - public boolean supports(Class clazz) { - return (MethodInvocation.class.isAssignableFrom(clazz) || JoinPoint.class.isAssignableFrom(clazz)); - } - -} diff --git a/core/src/test/java/org/springframework/security/access/vote/AbstractAclVoterTests.java b/core/src/test/java/org/springframework/security/access/vote/AbstractAclVoterTests.java index b321f59312..fa24e3dd45 100644 --- a/core/src/test/java/org/springframework/security/access/vote/AbstractAclVoterTests.java +++ b/core/src/test/java/org/springframework/security/access/vote/AbstractAclVoterTests.java @@ -2,16 +2,11 @@ package org.springframework.security.access.vote; import static org.junit.Assert.*; -import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.Collection; +import java.util.*; import org.aopalliance.intercept.MethodInvocation; -import org.aspectj.lang.JoinPoint; import org.junit.Test; -import org.springframework.security.MockJoinPoint; import org.springframework.security.access.ConfigAttribute; -import org.springframework.security.access.vote.AbstractAclVoter; import org.springframework.security.core.Authentication; import org.springframework.security.util.MethodInvocationUtils; @@ -30,9 +25,8 @@ public class AbstractAclVoterTests { }; @Test - public void supportsMethodInvocationsAndJoinPoints() throws Exception { + public void supportsMethodInvocations() throws Exception { assertTrue(voter.supports(MethodInvocation.class)); - assertTrue(voter.supports(JoinPoint.class)); assertFalse(voter.supports(String.class)); } @@ -43,14 +37,6 @@ public class AbstractAclVoterTests { assertEquals("The Argument", voter.getDomainObjectInstance(mi)); } - @Test - public void expectedDomainObjectArgumentIsReturnedFromJoinPoint() throws Exception { - voter.setProcessDomainObjectClass(String.class); - Method method = TestClass.class.getMethod("methodTakingAString", new Class[] {String.class}); - MockJoinPoint joinPoint = new MockJoinPoint(new TestClass(), method, "The Argument"); - assertEquals("The Argument", voter.getDomainObjectInstance(joinPoint)); - } - @Test public void correctArgumentIsSelectedFromMultipleArgs() throws Exception { voter.setProcessDomainObjectClass(String.class); diff --git a/web/src/test/java/org/springframework/security/web/context/SecurityContextPersistenceFilterTests.java b/web/src/test/java/org/springframework/security/web/context/SecurityContextPersistenceFilterTests.java index 956797d3f3..ded25f36d6 100644 --- a/web/src/test/java/org/springframework/security/web/context/SecurityContextPersistenceFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/context/SecurityContextPersistenceFilterTests.java @@ -108,4 +108,17 @@ public class SecurityContextPersistenceFilterTests { filter.doFilter(request, response, chain); assertNotNull(request.getSession(false)); } + + @Test + public void nullSecurityContextRepoDoesntSaveContextOrCreateSession() throws Exception { + final FilterChain chain = mock(FilterChain.class); + final MockHttpServletRequest request = new MockHttpServletRequest(); + final MockHttpServletResponse response = new MockHttpServletResponse(); + SecurityContextPersistenceFilter filter = new SecurityContextPersistenceFilter(); + SecurityContextRepository repo = new NullSecurityContextRepository(); + filter.setSecurityContextRepository(repo); + filter.doFilter(request, response, chain); + assertFalse(repo.containsContext(request)); + assertNull(request.getSession(false)); + } }