svn commit: r1237609 - /sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java

[fm...@apache.org]

Author: fmeschbe
Date: Mon Jan 30 11:40:56 2012
New Revision: 1237609

URL: http://svn.apache.org/viewvc?rev=1237609&view=rev
Log:
SLING-2390 Ensure sling.auth.redirect request parameter is respected if impersonation state changes

Modified:
    sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java

Modified: sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java?rev=1237609&r1=1237608&r2=1237609&view=diff
==============================================================================
--- sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java (original)
+++ sling/trunk/bundles/auth/core/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java Mon Jan 30 11:40:56 2012
@@ -742,60 +742,40 @@ public class SlingAuthenticator implemen
         try {
             handleImpersonation(request, authInfo);
             ResourceResolver resolver = resourceResolverFactory.getResourceResolver(authInfo);
-
-            setSudoCookie(request, response, authInfo);
+            final boolean impersChanged = setSudoCookie(request, response, authInfo);
 
             if (sendLoginEvent != null) {
                 postLoginEvent(authInfo);
             }
 
-            // handle success feedback
-            if (feedbackHandler != null) {
-
-                // provide the resource resolver to the feedback handler
-                request.setAttribute(REQUEST_ATTRIBUTE_RESOLVER, resolver);
-
-                // call the feedback handler, terminating the request if
-                // so desired by the handler
-                if (feedbackHandler.authenticationSucceeded(request, response,
-                    authInfo)) {
-
-                    // request will now be terminated, so close the resolver
-                    // to release resources
-                    resolver.close();
+            // provide the resource resolver to the feedback handler
+            request.setAttribute(REQUEST_ATTRIBUTE_RESOLVER, resolver);
 
-                    return false;
-                }
-
-            } else {
-
-                // if there is no feedback handler: check whether the client
-                // asked for redirect after authentication and/or impersonation
-                if (DefaultAuthenticationFeedbackHandler.handleRedirect(
-                    request, response)) {
+            boolean processRequest = true;
 
-                    // request will now be terminated, so close the resolver
-                    // to release resources
-                    resolver.close();
+            // custom feedback handler with option to redirect
+            if (feedbackHandler != null) {
+                processRequest = !feedbackHandler.authenticationSucceeded(request, response, authInfo);
+            }
 
-                    return false;
+            if (processRequest) {
+                if (AuthUtil.isValidateRequest(request)) {
+                    AuthUtil.sendValid(response);
+                    processRequest = false;
+                } else if (impersChanged || feedbackHandler == null) {
+                    processRequest = !DefaultAuthenticationFeedbackHandler.handleRedirect(request, response);
                 }
-
             }
 
-            // client requested validation, which succeeds, thus send
-            // success response and close the resolver
-            if (AuthUtil.isValidateRequest(request)) {
-                AuthUtil.sendValid(response);
+            if (processRequest) {
+                // process: set required attributes
+                setAttributes(resolver, authInfo.getAuthType(), request);
+            } else {
+                // terminate: cleanup
                 resolver.close();
-                return false;
             }
 
-            // no redirect desired, so continue processing by first setting
-            // the request attributes and then returning true
-            setAttributes(resolver, authInfo.getAuthType(), request);
-
-            return true;
+            return processRequest;
 
         } catch (LoginException re) {
 
@@ -1250,13 +1230,24 @@ public class SlingAuthenticator implemen
         return currentSudo;
     }
 
-    private void setSudoCookie(HttpServletRequest req,
+    /**
+     * Sets the impersonation cookie on the response if impersonation actually
+     * changed and returns whether the cookie has been set (or cleared) or not.
+     *
+     * @param req Providing the current sudo cookie value
+     * @param res For setting the sudo cookie
+     * @param authInfo Providing information about desired impersonation
+     * @return <code>true</code> if the cookie has been set or cleared or
+     *         <code>false</code> if the cookie is not modified.
+     */
+    private boolean setSudoCookie(HttpServletRequest req,
             HttpServletResponse res, AuthenticationInfo authInfo) {
         String sudo = (String) authInfo.get(ResourceResolverFactory.USER_IMPERSONATION);
         String currentSudo = getSudoCookieValue(req);
 
         // set the (new) impersonation
-        if (sudo != currentSudo) {
+        final boolean setCookie = sudo != currentSudo;
+        if (setCookie) {
             if (sudo == null) {
                 // Parameter set to "-" to clear impersonation, which was
                 // active due to cookie setting
@@ -1273,6 +1264,8 @@ public class SlingAuthenticator implemen
                         sudo);
             }
         }
+
+        return setCookie;
     }
 
     /**