From 992cf44b36a3ebd85e752f847e739f4eb95f319d Mon Sep 17 00:00:00 2001 From: Ben Alex Date: Mon, 18 Oct 2004 06:38:44 +0000 Subject: [PATCH] Refactor MethodDefinitionMap to use Method, not MethodInvocation. Refactor AbstractSecurityInterceptor to not force use of Throwable. Move AOP Alliance based MethodSecurityInterceptor to separate package. --- .../AbstractSecurityInterceptor.java | 103 ++++++++---------- .../intercept/InterceptorStatusToken.java | 53 +++++++++ .../SecurityInterceptorCallback.java | 50 --------- .../AbstractMethodDefinitionSource.java | 65 ++++++++--- .../method/MethodDefinitionAttributes.java | 11 +- .../intercept/method/MethodDefinitionMap.java | 26 ++--- .../method/MethodDefinitionSource.java | 3 +- .../MethodDefinitionSourceAdvisor.java | 6 +- .../MethodSecurityInterceptor.java | 29 +++-- .../intercept/method/aopalliance/package.html | 6 + .../intercept/method/package.html | 4 +- .../web/FilterSecurityInterceptor.java | 20 ++-- .../MethodDefinitionAttributesTests.java | 2 +- .../method/MockMethodDefinitionSource.java | 4 +- .../MethodDefinitionSourceAdvisorTests.java | 4 +- .../MethodSecurityInterceptorTests.java | 40 ++----- samples/attributes/src/applicationContext.xml | 2 +- .../contacts/etc/ca/applicationContext.xml | 4 +- .../contacts/etc/cas/applicationContext.xml | 4 +- .../etc/filter/applicationContext.xml | 4 +- 20 files changed, 229 insertions(+), 211 deletions(-) create mode 100644 core/src/main/java/org/acegisecurity/intercept/InterceptorStatusToken.java delete mode 100644 core/src/main/java/org/acegisecurity/intercept/SecurityInterceptorCallback.java rename core/src/main/java/org/acegisecurity/intercept/method/{ => aopalliance}/MethodDefinitionSourceAdvisor.java (96%) rename core/src/main/java/org/acegisecurity/intercept/method/{ => aopalliance}/MethodSecurityInterceptor.java (77%) create mode 100644 core/src/main/java/org/acegisecurity/intercept/method/aopalliance/package.html rename core/src/test/java/org/acegisecurity/intercept/method/{ => aopalliance}/MethodDefinitionSourceAdvisorTests.java (94%) rename core/src/test/java/org/acegisecurity/intercept/method/{ => aopalliance}/MethodSecurityInterceptorTests.java (93%) diff --git a/core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java b/core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java index be739c46d9..cedee9c272 100644 --- a/core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java +++ b/core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java @@ -76,8 +76,16 @@ import java.util.Set; * Perform any run-as replacement via the configured {@link RunAsManager}. * *
  • - * Perform a callback to the {@link SecurityInterceptorCallback}, which will - * actually proceed with executing the object. + * Pass control back to the concrete subclass, which will actually proceed with + * executing the object. A {@link InterceptorStatusToken} is returned so that + * after the subclass has finished proceeding with execution of the object, + * its finally clause can ensure the AbstractSecurityInterceptor + * is re-called and tidies up correctly. + *
    • + *
  • + * The concrete subclass will re-call the + * AbstractSecurityInterceptor via the {@link + * #afterInvocation(InterceptorStatusToken)} method. *
    • *
  • * If the RunAsManager replaced the Authentication @@ -98,17 +106,20 @@ import java.util.Set; * object to false. *
    • *
  • - * Perform a callback to the {@link SecurityInterceptorCallback}, which will - * actually proceed with the invocation. + * As described above, the concrete subclass will be returned an + * InterceptorStatusToken which is subsequently re-presented to + * the AbstractSecurityInterceptor after the secure object has + * been executed. The AbstractSecurityInterceptor will take no + * further action when its {@link #afterInvocation(InterceptorStatusToken)} is + * called. *
    • * * * *
  • - * Return the result from the SecurityInterceptorCallback to the - * method that called {@link AbstractSecurityInterceptor#interceptor(Object, - * SecurityInterceptorCallback)}. This is almost always a concrete subclass of - * the AbstractSecurityInterceptor. + * Control again returns to the concrete subclass, which will return to the + * caller any result or exception that occurred when it proceeded with the + * execution of the secure object. *
    • * *

    @@ -226,37 +237,24 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean { } } - /** - * Does the work of authenticating and authorizing the request. - * - *

    - * Throws {@link net.sf.acegisecurity.AcegiSecurityException} and its - * subclasses. - *

    - * - * @param object details of a secure object invocation - * @param callback the object that will complete the target secure object - * invocation - * - * @return The value that was returned by the - * SecurityInterceptorCallback - * - * @throws Throwable if any error occurs during the - * SecurityInterceptorCallback - * @throws IllegalArgumentException if a required argument was missing or - * invalid - * @throws AuthenticationCredentialsNotFoundException if the - * ContextHolder is not populated with a valid - * SecureContext - */ - public Object interceptor(Object object, - SecurityInterceptorCallback callback) throws Throwable { - if (object == null) { - throw new IllegalArgumentException("Object was null"); + protected void afterInvocation(InterceptorStatusToken token) { + if (token == null) { + return; } - if (callback == null) { - throw new IllegalArgumentException("Callback was null"); + if (logger.isDebugEnabled()) { + logger.debug("Reverting to original Authentication: " + + token.getAuthenticated().toString()); + } + + SecureContext secureContext = (SecureContext) ContextHolder.getContext(); + secureContext.setAuthentication(token.getAuthenticated()); + ContextHolder.setContext(secureContext); + } + + protected InterceptorStatusToken beforeInvocation(Object object) { + if (object == null) { + throw new IllegalArgumentException("Object was null"); } if (!this.obtainObjectDefinitionSource().supports(object.getClass())) { @@ -294,7 +292,11 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean { Authentication authenticated = this.authenticationManager .authenticate(context.getAuthentication()); authenticated.setAuthenticated(true); - logger.debug("Authenticated: " + authenticated.toString()); + + if (logger.isDebugEnabled()) { + logger.debug("Authenticated: " + authenticated.toString()); + } + context.setAuthentication(authenticated); ContextHolder.setContext((Context) context); @@ -315,31 +317,20 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean { "RunAsManager did not change Authentication object"); } - return callback.proceedWithObject(object); + return null; // no further work post-invocation } else { if (logger.isDebugEnabled()) { logger.debug("Switching to RunAs Authentication: " + runAs.toString()); } - SecureContext origSecureContext = null; + context.setAuthentication(runAs); + ContextHolder.setContext((Context) context); - try { - origSecureContext = (SecureContext) ContextHolder - .getContext(); - context.setAuthentication(runAs); - ContextHolder.setContext((Context) context); + InterceptorStatusToken token = new InterceptorStatusToken(); + token.setAuthenticated(authenticated); - return callback.proceedWithObject(object); - } finally { - if (logger.isDebugEnabled()) { - logger.debug("Reverting to original Authentication: " - + authenticated.toString()); - } - - origSecureContext.setAuthentication(authenticated); - ContextHolder.setContext(origSecureContext); - } + return token; // revert to token.Authenticated post-invocation } } else { if (logger.isDebugEnabled()) { @@ -365,7 +356,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean { } } - return callback.proceedWithObject(object); + return null; // no further work post-invocation } } } diff --git a/core/src/main/java/org/acegisecurity/intercept/InterceptorStatusToken.java b/core/src/main/java/org/acegisecurity/intercept/InterceptorStatusToken.java new file mode 100644 index 0000000000..0d73384dde --- /dev/null +++ b/core/src/main/java/org/acegisecurity/intercept/InterceptorStatusToken.java @@ -0,0 +1,53 @@ +/* Copyright 2004 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 net.sf.acegisecurity.intercept; + +import net.sf.acegisecurity.Authentication; + + +/** + * A return object received by {@link AbstractSecurityInterceptor} subclasses. + * + *

    + * This class reflects the status of the security interception, so that the + * final call to AbstractSecurityInterceptor can tidy up + * correctly. + *

    + * + *

    + * Whilst this class currently only wraps a single object, it has been modelled + * as a class so that future changes to the operation of + * AbstractSecurityInterceptor are abstracted from subclasses. + *

    + * + * @author Ben Alex + * @version $Id$ + */ +public class InterceptorStatusToken { + //~ Instance fields ======================================================== + + private Authentication authenticated; + + //~ Methods ================================================================ + + public void setAuthenticated(Authentication authenticated) { + this.authenticated = authenticated; + } + + public Authentication getAuthenticated() { + return authenticated; + } +} diff --git a/core/src/main/java/org/acegisecurity/intercept/SecurityInterceptorCallback.java b/core/src/main/java/org/acegisecurity/intercept/SecurityInterceptorCallback.java deleted file mode 100644 index 2e1295e3c4..0000000000 --- a/core/src/main/java/org/acegisecurity/intercept/SecurityInterceptorCallback.java +++ /dev/null @@ -1,50 +0,0 @@ -/* Copyright 2004 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 net.sf.acegisecurity.intercept; - -/** - * Allows the {@link AbstractSecurityInterceptor} to continue the secure object - * invocation at the appropriate time. - * - *

    - * Concrete AbstractSecurityInterceptor subclasses are required to - * provide a SecurityInterceptorCallback. This is called by the - * AbstractSecurityInterceptor at the exact time the secure - * object should have its processing continued. The exact way processing is - * continued is specific to the type of secure object. For example, it may - * involve proceeding with a method invocation, servicing a request, or - * continuing a filter chain. - *

    - * - *

    - * The result from processing the secure object should be returned to the - * AbstractSecurityInterceptor, which in turn will ultimately - * return it to the calling class. - *

    - * - * @author Ben Alex - * @version $Id$ - */ -public interface SecurityInterceptorCallback { - //~ Methods ================================================================ - - /** - * Continues to process the secured object. - * - * @return the result (if any) from calling the secured object - */ - public Object proceedWithObject(Object object) throws Throwable; -} diff --git a/core/src/main/java/org/acegisecurity/intercept/method/AbstractMethodDefinitionSource.java b/core/src/main/java/org/acegisecurity/intercept/method/AbstractMethodDefinitionSource.java index 8d2cf0840a..be9716ebac 100644 --- a/core/src/main/java/org/acegisecurity/intercept/method/AbstractMethodDefinitionSource.java +++ b/core/src/main/java/org/acegisecurity/intercept/method/AbstractMethodDefinitionSource.java @@ -22,6 +22,11 @@ import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.aspectj.lang.JoinPoint; +import org.aspectj.lang.reflect.CodeSignature; + +import java.lang.reflect.Method; + /** * Abstract implementation of MethodDefinitionSource. @@ -39,26 +44,55 @@ public abstract class AbstractMethodDefinitionSource public ConfigAttributeDefinition getAttributes(Object object) throws IllegalArgumentException { - if ((object == null) || !this.supports(object.getClass())) { - throw new IllegalArgumentException( - "Object must be a MethodInvocation"); + if (object == null) { + throw new IllegalArgumentException("Object cannot be null"); } - return this.lookupAttributes((MethodInvocation) object); + if (object instanceof MethodInvocation) { + return this.lookupAttributes(((MethodInvocation) object).getMethod()); + } + + if (object instanceof JoinPoint) { + JoinPoint jp = (JoinPoint) object; + Class targetClazz = jp.getTarget().getClass(); + String targetMethodName = jp.getStaticPart().getSignature().getName(); + Class[] types = ((CodeSignature) jp.getStaticPart().getSignature()) + .getParameterTypes(); + + if (logger.isDebugEnabled()) { + logger.debug("Target Class: " + targetClazz); + logger.debug("Target Method Name: " + targetMethodName); + + for (int i = 0; i < types.length; i++) { + if (logger.isDebugEnabled()) { + logger.debug("Target Method Arg #" + i + ": " + + types[i]); + } + } + } + + try { + return this.lookupAttributes(targetClazz.getMethod( + targetMethodName, types)); + } catch (NoSuchMethodException nsme) { + throw new IllegalArgumentException( + "Could not obtain target method from JoinPoint: " + jp); + } + } + + throw new IllegalArgumentException( + "Object must be a MethodInvocation or JoinPoint"); } public boolean supports(Class clazz) { - if (MethodInvocation.class.isAssignableFrom(clazz)) { - return true; - } else { - return false; - } + return (MethodInvocation.class.isAssignableFrom(clazz) + || JoinPoint.class.isAssignableFrom(clazz)); } /** * Performs the actual lookup of the relevant * ConfigAttributeDefinition for the specified - * MethodInvocation. + * Method which is subject of the method invocation. * *

    * Provided so subclasses need only to provide one basic method to properly @@ -67,15 +101,14 @@ public abstract class AbstractMethodDefinitionSource * *

    * Returns null if there are no matching attributes for the - * method invocation. + * method. *

    * - * @param mi the method being invoked for which configuration attributes - * should be looked up + * @param method the method being invoked for which configuration + * attributes should be looked up * * @return the ConfigAttributeDefinition that applies to the - * specified MethodInvocation + * specified Method */ - protected abstract ConfigAttributeDefinition lookupAttributes( - MethodInvocation mi); + protected abstract ConfigAttributeDefinition lookupAttributes(Method method); } diff --git a/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionAttributes.java b/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionAttributes.java index 94d37d9155..5f8089d99c 100644 --- a/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionAttributes.java +++ b/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionAttributes.java @@ -18,8 +18,6 @@ package net.sf.acegisecurity.intercept.method; import net.sf.acegisecurity.ConfigAttribute; import net.sf.acegisecurity.ConfigAttributeDefinition; -import org.aopalliance.intercept.MethodInvocation; - import org.springframework.metadata.Attributes; import java.lang.reflect.Method; @@ -85,11 +83,10 @@ public class MethodDefinitionAttributes extends AbstractMethodDefinitionSource { return null; } - protected ConfigAttributeDefinition lookupAttributes( - MethodInvocation invocation) { + protected ConfigAttributeDefinition lookupAttributes(Method method) { ConfigAttributeDefinition definition = new ConfigAttributeDefinition(); - Class interceptedClass = invocation.getMethod().getDeclaringClass(); + Class interceptedClass = method.getDeclaringClass(); // add the class level attributes for the implementing class addClassAttributes(definition, interceptedClass); @@ -98,10 +95,10 @@ public class MethodDefinitionAttributes extends AbstractMethodDefinitionSource { addClassAttributes(definition, interceptedClass.getInterfaces()); // add the method level attributes for the implemented method - addMethodAttributes(definition, invocation.getMethod()); + addMethodAttributes(definition, method); // add the method level attributes for the implemented intreface methods - addInterfaceMethodAttributes(definition, invocation.getMethod()); + addInterfaceMethodAttributes(definition, method); if (definition.size() == 0) { return null; diff --git a/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java b/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java index 51e66a1df9..492d350d44 100644 --- a/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java +++ b/core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java @@ -38,9 +38,9 @@ import java.util.Map; * *

    * For consistency with {@link MethodDefinitionAttributes} as well as support - * for {@link MethodDefinitionSourceAdvisor}, this implementation will return - * a ConfigAttributeDefinition containing all configuration - * attributes defined against: + * for MethodDefinitionSourceAdvisor, this implementation will + * return a ConfigAttributeDefinition containing all + * configuration attributes defined against: * *