Refactored to pull "public invocation" behaviour (attr==null) into a single guard clause.

core/src/main/java/org/acegisecurity/intercept/AbstractSecurityInterceptor.java

@@ -239,95 +239,95 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
 
         ConfigAttributeDefinition attr = this.obtainObjectDefinitionSource().getAttributes(object);
 
-        if ((attr == null) && rejectPublicInvocations) {
-            throw new IllegalArgumentException("No public invocations are allowed via this AbstractSecurityInterceptor. "
+        if (attr == null) {
+            if(rejectPublicInvocations) {
+                throw new IllegalArgumentException(
+                      "No public invocations are allowed via this AbstractSecurityInterceptor. "
                     + "This indicates a configuration error because the "
                     + "AbstractSecurityInterceptor.rejectPublicInvocations property is set to 'true'");
-        }
+            }
 
-        if (attr != null) {
             if (logger.isDebugEnabled()) {
-                logger.debug("Secure object: " + object.toString() + "; ConfigAttributes: " + attr.toString());
+                logger.debug("Public object - authentication not attempted");
             }
 
-            // We check for just the property we're interested in (we do
-            // not call Context.validate() like the ContextInterceptor)
-            if (SecurityContextHolder.getContext().getAuthentication() == null) {
-                credentialsNotFound(messages.getMessage("AbstractSecurityInterceptor.authenticationNotFound",
-                        "An Authentication object was not found in the SecurityContext"), object, attr);
-            }
+            publishEvent(new PublicInvocationEvent(object));
 
-            // Attempt authentication if not already authenticated, or user always wants reauthentication
-            Authentication authenticated;
+            return null; // no further work post-invocation
+        }
 
-            if (!SecurityContextHolder.getContext().getAuthentication().isAuthenticated() || alwaysReauthenticate) {
-                try {
-                    authenticated = this.authenticationManager.authenticate(SecurityContextHolder.getContext()
-                                                                                                 .getAuthentication());
-                } catch (AuthenticationException authenticationException) {
-                    throw authenticationException;
-                }
 
-                // We don't authenticated.setAuthentication(true), because each provider should do that
-                if (logger.isDebugEnabled()) {
-                    logger.debug("Successfully Authenticated: " + authenticated.toString());
-                }
+        if (logger.isDebugEnabled()) {
+            logger.debug("Secure object: " + object.toString() + "; ConfigAttributes: " + attr.toString());
+        }
 
-                SecurityContextHolder.getContext().setAuthentication(authenticated);
-            } else {
-                authenticated = SecurityContextHolder.getContext().getAuthentication();
+        if (SecurityContextHolder.getContext().getAuthentication() == null) {
+            credentialsNotFound(messages.getMessage("AbstractSecurityInterceptor.authenticationNotFound",
+                    "An Authentication object was not found in the SecurityContext"), object, attr);
+        }
 
-                if (logger.isDebugEnabled()) {
-                    logger.debug("Previously Authenticated: " + authenticated.toString());
-                }
-            }
+        // Attempt authentication if not already authenticated, or user always wants reauthentication
+        Authentication authenticated;
 
-            // Attempt authorization
+        if (!SecurityContextHolder.getContext().getAuthentication().isAuthenticated() || alwaysReauthenticate) {
             try {
-                this.accessDecisionManager.decide(authenticated, object, attr);
-            } catch (AccessDeniedException accessDeniedException) {
-                AuthorizationFailureEvent event = new AuthorizationFailureEvent(object, attr, authenticated,
-                        accessDeniedException);
-                publishEvent(event);
+                authenticated = this.authenticationManager.authenticate(SecurityContextHolder.getContext()
+                                                                                             .getAuthentication());
+            } catch (AuthenticationException authenticationException) {
+                throw authenticationException;
+            }
 
-                throw accessDeniedException;
+            // We don't authenticated.setAuthentication(true), because each provider should do that
+            if (logger.isDebugEnabled()) {
+                logger.debug("Successfully Authenticated: " + authenticated.toString());
             }
 
+            SecurityContextHolder.getContext().setAuthentication(authenticated);
+        } else {
+            authenticated = SecurityContextHolder.getContext().getAuthentication();
+
             if (logger.isDebugEnabled()) {
-                logger.debug("Authorization successful");
+                logger.debug("Previously Authenticated: " + authenticated.toString());
             }
+        }
 
-            AuthorizedEvent event = new AuthorizedEvent(object, attr, authenticated);
+        // Attempt authorization
+        try {
+            this.accessDecisionManager.decide(authenticated, object, attr);
+        } catch (AccessDeniedException accessDeniedException) {
+            AuthorizationFailureEvent event = new AuthorizationFailureEvent(object, attr, authenticated,
+                    accessDeniedException);
             publishEvent(event);
 
-            // Attempt to run as a different user
-            Authentication runAs = this.runAsManager.buildRunAs(authenticated, object, attr);
+            throw accessDeniedException;
+        }
 
-            if (runAs == null) {
-                if (logger.isDebugEnabled()) {
-                    logger.debug("RunAsManager did not change Authentication object");
-                }
+        if (logger.isDebugEnabled()) {
+            logger.debug("Authorization successful");
+        }
 
-                // no further work post-invocation
-                return new InterceptorStatusToken(authenticated, false, attr, object);
-            } else {
-                if (logger.isDebugEnabled()) {
-                    logger.debug("Switching to RunAs Authentication: " + runAs.toString());
-                }
+        AuthorizedEvent event = new AuthorizedEvent(object, attr, authenticated);
+        publishEvent(event);
 
-                SecurityContextHolder.getContext().setAuthentication(runAs);
+        // Attempt to run as a different user
+        Authentication runAs = this.runAsManager.buildRunAs(authenticated, object, attr);
 
-                // revert to token.Authenticated post-invocation
-                return new InterceptorStatusToken(authenticated, true, attr, object);
+        if (runAs == null) {
+            if (logger.isDebugEnabled()) {
+                logger.debug("RunAsManager did not change Authentication object");
             }
+
+            // no further work post-invocation
+            return new InterceptorStatusToken(authenticated, false, attr, object);
         } else {
             if (logger.isDebugEnabled()) {
-                logger.debug("Public object - authentication not attempted");
+                logger.debug("Switching to RunAs Authentication: " + runAs.toString());
             }
 
-            publishEvent(new PublicInvocationEvent(object));
+            SecurityContextHolder.getContext().setAuthentication(runAs);
 
-            return null; // no further work post-invocation
+            // revert to token.Authenticated post-invocation
+            return new InterceptorStatusToken(authenticated, true, attr, object);
         }
     }