Browse Source

SEC-60: Make method more friendly towards Hibernate detached object. Please note my comments in the JIRA task, as I believing calling toString() is not an unreasonable expectation.

Ben Alex 20 years ago
parent
commit
c6d5363e5d

+ 45 - 56
core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java

@@ -12,7 +12,6 @@
  * See the License for the specific language governing permissions and
  * See the License for the specific language governing permissions and
  * limitations under the License.
  * limitations under the License.
  */
  */
-
 package net.sf.acegisecurity.intercept;
 package net.sf.acegisecurity.intercept;
 
 
 import net.sf.acegisecurity.AccessDecisionManager;
 import net.sf.acegisecurity.AccessDecisionManager;
@@ -51,15 +50,15 @@ import java.util.Set;
 
 
 /**
 /**
  * Abstract class that implements security interception for secure objects.
  * Abstract class that implements security interception for secure objects.
- * 
+ *
  * <P>
  * <P>
  * The <code>AbstractSecurityInterceptor</code> will ensure the proper startup
  * The <code>AbstractSecurityInterceptor</code> will ensure the proper startup
  * configuration of the security interceptor. It will also implement the
  * configuration of the security interceptor. It will also implement the
  * proper handling of secure object invocations, being:
  * proper handling of secure object invocations, being:
- * 
+ *
  * <ol>
  * <ol>
  * <li>
  * <li>
- * Obtain the {@link Authentication} object from the 
+ * Obtain the {@link Authentication} object from the
  * {@link SecurityContextHolder}.
  * {@link SecurityContextHolder}.
  * </li>
  * </li>
  * <li>
  * <li>
@@ -70,7 +69,7 @@ import java.util.Set;
  * <li>
  * <li>
  * For an invocation that is secured (there is a
  * For an invocation that is secured (there is a
  * <code>ConfigAttributeDefinition</code> for the secure object invocation):
  * <code>ConfigAttributeDefinition</code> for the secure object invocation):
- * 
+ *
  * <ol type="a">
  * <ol type="a">
  * <li>
  * <li>
  * If either the {@link net.sf.acegisecurity.Authentication#isAuthenticated()}
  * If either the {@link net.sf.acegisecurity.Authentication#isAuthenticated()}
@@ -109,12 +108,12 @@ import java.util.Set;
  * caller.
  * caller.
  * </li>
  * </li>
  * </ol>
  * </ol>
- * 
+ *
  * </li>
  * </li>
  * <li>
  * <li>
  * For an invocation that is public (there is no
  * For an invocation that is public (there is no
  * <code>ConfigAttributeDefinition</code> for the secure object invocation):
  * <code>ConfigAttributeDefinition</code> for the secure object invocation):
- * 
+ *
  * <ol type="a">
  * <ol type="a">
  * <li>
  * <li>
  * As described above, the concrete subclass will be returned an
  * As described above, the concrete subclass will be returned an
@@ -125,7 +124,7 @@ import java.util.Set;
  * Object)} is called.
  * Object)} is called.
  * </li>
  * </li>
  * </ol>
  * </ol>
- * 
+ *
  * </li>
  * </li>
  * <li>
  * <li>
  * Control again returns to the concrete subclass, along with the
  * Control again returns to the concrete subclass, along with the
@@ -140,12 +139,7 @@ import java.util.Set;
  */
  */
 public abstract class AbstractSecurityInterceptor implements InitializingBean,
 public abstract class AbstractSecurityInterceptor implements InitializingBean,
     ApplicationContextAware {
     ApplicationContextAware {
-    //~ Static fields/initializers =============================================
-
     protected static final Log logger = LogFactory.getLog(AbstractSecurityInterceptor.class);
     protected static final Log logger = LogFactory.getLog(AbstractSecurityInterceptor.class);
-
-    //~ Instance fields ========================================================
-
     private AccessDecisionManager accessDecisionManager;
     private AccessDecisionManager accessDecisionManager;
     private AfterInvocationManager afterInvocationManager;
     private AfterInvocationManager afterInvocationManager;
     private ApplicationContext context;
     private ApplicationContext context;
@@ -154,8 +148,6 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
     private boolean alwaysReauthenticate = false;
     private boolean alwaysReauthenticate = false;
     private boolean validateConfigAttributes = true;
     private boolean validateConfigAttributes = true;
 
 
-    //~ Methods ================================================================
-
     public void setAfterInvocationManager(
     public void setAfterInvocationManager(
         AfterInvocationManager afterInvocationManager) {
         AfterInvocationManager afterInvocationManager) {
         this.afterInvocationManager = afterInvocationManager;
         this.afterInvocationManager = afterInvocationManager;
@@ -253,27 +245,27 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
 
 
         if (!this.obtainObjectDefinitionSource().supports(getSecureObjectClass())) {
         if (!this.obtainObjectDefinitionSource().supports(getSecureObjectClass())) {
             throw new IllegalArgumentException(
             throw new IllegalArgumentException(
-                "ObjectDefinitionSource does not support secure object class: "
-                + getSecureObjectClass());
+                "ObjectDefinitionSource does not support secure object class: " +
+                getSecureObjectClass());
         }
         }
 
 
         if (!this.runAsManager.supports(getSecureObjectClass())) {
         if (!this.runAsManager.supports(getSecureObjectClass())) {
             throw new IllegalArgumentException(
             throw new IllegalArgumentException(
-                "RunAsManager does not support secure object class: "
-                + getSecureObjectClass());
+                "RunAsManager does not support secure object class: " +
+                getSecureObjectClass());
         }
         }
 
 
         if (!this.accessDecisionManager.supports(getSecureObjectClass())) {
         if (!this.accessDecisionManager.supports(getSecureObjectClass())) {
             throw new IllegalArgumentException(
             throw new IllegalArgumentException(
-                "AccessDecisionManager does not support secure object class: "
-                + getSecureObjectClass());
+                "AccessDecisionManager does not support secure object class: " +
+                getSecureObjectClass());
         }
         }
 
 
-        if ((this.afterInvocationManager != null)
-            && !this.afterInvocationManager.supports(getSecureObjectClass())) {
+        if ((this.afterInvocationManager != null) &&
+                !this.afterInvocationManager.supports(getSecureObjectClass())) {
             throw new IllegalArgumentException(
             throw new IllegalArgumentException(
-                "AfterInvocationManager does not support secure object class: "
-                + getSecureObjectClass());
+                "AfterInvocationManager does not support secure object class: " +
+                getSecureObjectClass());
         }
         }
 
 
         if (this.validateConfigAttributes) {
         if (this.validateConfigAttributes) {
@@ -289,18 +281,16 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
                 Set set = new HashSet();
                 Set set = new HashSet();
 
 
                 while (iter.hasNext()) {
                 while (iter.hasNext()) {
-                    ConfigAttributeDefinition def = (ConfigAttributeDefinition) iter
-                        .next();
+                    ConfigAttributeDefinition def = (ConfigAttributeDefinition) iter.next();
                     Iterator attributes = def.getConfigAttributes();
                     Iterator attributes = def.getConfigAttributes();
 
 
                     while (attributes.hasNext()) {
                     while (attributes.hasNext()) {
-                        ConfigAttribute attr = (ConfigAttribute) attributes
-                            .next();
+                        ConfigAttribute attr = (ConfigAttribute) attributes.next();
 
 
-                        if (!this.runAsManager.supports(attr)
-                            && !this.accessDecisionManager.supports(attr)
-                            && ((this.afterInvocationManager == null)
-                            || !this.afterInvocationManager.supports(attr))) {
+                        if (!this.runAsManager.supports(attr) &&
+                                !this.accessDecisionManager.supports(attr) &&
+                                ((this.afterInvocationManager == null) ||
+                                !this.afterInvocationManager.supports(attr))) {
                             set.add(attr);
                             set.add(attr);
                         }
                         }
                     }
                     }
@@ -312,8 +302,8 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
                     }
                     }
                 } else {
                 } else {
                     throw new IllegalArgumentException(
                     throw new IllegalArgumentException(
-                        "Unsupported configuration attributes: "
-                        + set.toString());
+                        "Unsupported configuration attributes: " +
+                        set.toString());
                 }
                 }
             }
             }
         }
         }
@@ -340,18 +330,16 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
 
 
         if (token.isContextHolderRefreshRequired()) {
         if (token.isContextHolderRefreshRequired()) {
             if (logger.isDebugEnabled()) {
             if (logger.isDebugEnabled()) {
-                logger.debug("Reverting to original Authentication: "
-                    + token.getAuthentication().toString());
+                logger.debug("Reverting to original Authentication: " +
+                    token.getAuthentication().toString());
             }
             }
 
 
-            SecurityContextHolder.getContext().setAuthentication(token
-                .getAuthentication());
+            SecurityContextHolder.getContext().setAuthentication(token.getAuthentication());
         }
         }
 
 
         if (afterInvocationManager != null) {
         if (afterInvocationManager != null) {
-            returnedObject = afterInvocationManager.decide(token
-                    .getAuthentication(), token.getSecureObject(),
-                    token.getAttr(), returnedObject);
+            returnedObject = afterInvocationManager.decide(token.getAuthentication(),
+                    token.getSecureObject(), token.getAttr(), returnedObject);
         }
         }
 
 
         return returnedObject;
         return returnedObject;
@@ -360,17 +348,18 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
     protected InterceptorStatusToken beforeInvocation(Object object) {
     protected InterceptorStatusToken beforeInvocation(Object object) {
         Assert.notNull(object, "Object was null");
         Assert.notNull(object, "Object was null");
         Assert.isTrue(getSecureObjectClass().isAssignableFrom(object.getClass()),
         Assert.isTrue(getSecureObjectClass().isAssignableFrom(object.getClass()),
-            "Security invocation attempted for object " + object
-            + " but AbstractSecurityInterceptor only configured to support secure objects of type: "
-            + getSecureObjectClass());
+            "Security invocation attempted for object " +
+            object.getClass().getName() +
+            " but AbstractSecurityInterceptor only configured to support secure objects of type: " +
+            getSecureObjectClass());
 
 
         ConfigAttributeDefinition attr = this.obtainObjectDefinitionSource()
         ConfigAttributeDefinition attr = this.obtainObjectDefinitionSource()
                                              .getAttributes(object);
                                              .getAttributes(object);
 
 
         if (attr != null) {
         if (attr != null) {
             if (logger.isDebugEnabled()) {
             if (logger.isDebugEnabled()) {
-                logger.debug("Secure object: " + object.toString()
-                    + "; ConfigAttributes: " + attr.toString());
+                logger.debug("Secure object: " + object.toString() +
+                    "; ConfigAttributes: " + attr.toString());
             }
             }
 
 
             // We check for just the property we're interested in (we do
             // We check for just the property we're interested in (we do
@@ -384,8 +373,8 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
             Authentication authenticated;
             Authentication authenticated;
 
 
             if (!SecurityContextHolder.getContext().getAuthentication()
             if (!SecurityContextHolder.getContext().getAuthentication()
-                                      .isAuthenticated()
-                || alwaysReauthenticate) {
+                                          .isAuthenticated() ||
+                    alwaysReauthenticate) {
                 try {
                 try {
                     authenticated = this.authenticationManager.authenticate(SecurityContextHolder.getContext()
                     authenticated = this.authenticationManager.authenticate(SecurityContextHolder.getContext()
                                                                                                  .getAuthentication());
                                                                                                  .getAuthentication());
@@ -402,8 +391,8 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
 
 
                 // We don't authenticated.setAuthentication(true), because each provider should do that
                 // We don't authenticated.setAuthentication(true), because each provider should do that
                 if (logger.isDebugEnabled()) {
                 if (logger.isDebugEnabled()) {
-                    logger.debug("Successfully Authenticated: "
-                        + authenticated.toString());
+                    logger.debug("Successfully Authenticated: " +
+                        authenticated.toString());
                 }
                 }
 
 
                 SecurityContextHolder.getContext().setAuthentication(authenticated);
                 SecurityContextHolder.getContext().setAuthentication(authenticated);
@@ -412,8 +401,8 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
                                                      .getAuthentication();
                                                      .getAuthentication();
 
 
                 if (logger.isDebugEnabled()) {
                 if (logger.isDebugEnabled()) {
-                    logger.debug("Previously Authenticated: "
-                        + authenticated.toString());
+                    logger.debug("Previously Authenticated: " +
+                        authenticated.toString());
                 }
                 }
             }
             }
 
 
@@ -450,8 +439,8 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
                     object); // no further work post-invocation
                     object); // no further work post-invocation
             } else {
             } else {
                 if (logger.isDebugEnabled()) {
                 if (logger.isDebugEnabled()) {
-                    logger.debug("Switching to RunAs Authentication: "
-                        + runAs.toString());
+                    logger.debug("Switching to RunAs Authentication: " +
+                        runAs.toString());
                 }
                 }
 
 
                 SecurityContextHolder.getContext().setAuthentication(runAs);
                 SecurityContextHolder.getContext().setAuthentication(runAs);
@@ -473,7 +462,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean,
     /**
     /**
      * Helper method which generates an exception containing the passed reason,
      * Helper method which generates an exception containing the passed reason,
      * and publishes an event to the application context.
      * and publishes an event to the application context.
-     * 
+     *
      * <P>
      * <P>
      * Always throws an exception.
      * Always throws an exception.
      * </p>
      * </p>