From 48e2d5ad62f03d5494e5c3b2b32934e7903826b4 Mon Sep 17 00:00:00 2001
From: Luke Taylor <luke.taylor@springsource.com>
Date: Fri, 15 Feb 2008 17:05:58 +0000
Subject: [PATCH] Refactored AbstractSecurityInterceptor, extracting method
 authenticateIfRequired();

---
 .../AbstractSecurityInterceptor.java          | 61 +++++++++++--------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java b/core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java
index f3f7e8c218..4aa6b66dbb 100644
--- a/core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java
+++ b/core/src/main/java/org/springframework/security/intercept/AbstractSecurityInterceptor.java
@@ -129,7 +129,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
      * completed.
      *
      * @param token as returned by the {@link #beforeInvocation(Object)}} method
-     * @param returnedObject any object returned from the secure object invocation (may be<tt>null</tt>)
+     * @param returnedObject any object returned from the secure object invocation (may be <tt>null</tt>)
      * @return the object the secure object invocation should ultimately return to its caller (may be <tt>null</tt>)
      */
     protected Object afterInvocation(InterceptorStatusToken token, Object returnedObject) {
@@ -188,7 +188,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
 
             if (attributeDefs == null) {
                 logger.warn("Could not validate configuration attributes as the ObjectDefinitionSource did not return "
-                        + "a ConfigAttributeDefinition Iterator");
+                        + "a ConfigAttributeDefinition collection");
                 return;
             }
 
@@ -247,7 +247,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
         }
 
         if (logger.isDebugEnabled()) {
-            logger.debug("Secure object: " + object.toString() + "; ConfigAttributes: " + attr.toString());
+            logger.debug("Secure object: " + object + "; ConfigAttributes: " + attr);
         }
 
         if (SecurityContextHolder.getContext().getAuthentication() == null) {
@@ -255,28 +255,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
                     "An Authentication object was not found in the SecurityContext"), object, attr);
         }
 
-        // Attempt authentication if not already authenticated, or user always
-        // wants reauthentication
-        Authentication authenticated;
-
-        if (!SecurityContextHolder.getContext().getAuthentication().isAuthenticated() || alwaysReauthenticate) {
-            authenticated =
-                    this.authenticationManager.authenticate(SecurityContextHolder.getContext().getAuthentication());
-
-            // 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("Previously Authenticated: " + authenticated.toString());
-            }
-        }
+        Authentication authenticated = authenticateIfRequired();
 
         // Attempt authorization
         try {
@@ -309,7 +288,7 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
             return new InterceptorStatusToken(authenticated, false, attr, object);
         } else {
             if (logger.isDebugEnabled()) {
-                logger.debug("Switching to RunAs Authentication: " + runAs.toString());
+                logger.debug("Switching to RunAs Authentication: " + runAs);
             }
 
             SecurityContextHolder.getContext().setAuthentication(runAs);
@@ -319,6 +298,36 @@ public abstract class AbstractSecurityInterceptor implements InitializingBean, A
         }
     }
 
+    /**
+     * Checks the current authentication token and passes it to the AuthenticationManager if
+     * {@link org.springframework.security.Authentication#isAuthenticated()} returns false or the property
+     * <tt>alwaysReauthenticate</tt> has been set to true.
+     *
+     * @return an authenticated <tt>Authentication</tt> object.
+     */
+    private Authentication authenticateIfRequired() {
+        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+
+        if (authentication.isAuthenticated() && !alwaysReauthenticate) {
+            if (logger.isDebugEnabled()) {
+                logger.debug("Previously Authenticated: " + authentication);
+            }
+
+            return authentication;
+        }
+
+        authentication = authenticationManager.authenticate(authentication);
+
+        // We don't authenticated.setAuthentication(true), because each provider should do that
+        if (logger.isDebugEnabled()) {
+            logger.debug("Successfully Authenticated: " + authentication);
+        }
+
+        SecurityContextHolder.getContext().setAuthentication(authentication);
+
+        return authentication;
+    }
+
     /**
      * Helper method which generates an exception containing the passed reason,
      * and publishes an event to the application context.