SEC-1063: Moved the justUseSavedRequestOnGet property to ExceptionTranslationFilter. If set, it will not store the SavedRequest for unless the request is a GET.

core/src/main/java/org/springframework/security/ui/ExceptionTranslationFilter.java

@@ -78,6 +78,7 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements
     private PortResolver portResolver = new PortResolverImpl();
     private ThrowableAnalyzer throwableAnalyzer = new DefaultThrowableAnalyzer();
     private boolean createSessionAllowed = true;
+    private boolean justUseSavedRequestOnGet;
 
     //~ Methods ========================================================================================================
 
@@ -169,7 +170,7 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements
 
     /**
      * If <code>true</code>, indicates that <code>ExceptionTranslationFilter</code> is permitted to store the target
-     * URL and exception information in the <code>HttpSession</code> (the default).
+     * URL and exception information in a new <code>HttpSession</code> (the default).
      * In situations where you do not wish to unnecessarily create <code>HttpSession</code>s - because the user agent
      * will know the failed URL, such as with BASIC or Digest authentication - you may wish to set this property to
      * <code>false</code>.
@@ -188,25 +189,25 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements
 
     protected void sendStartAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
             AuthenticationException reason) throws ServletException, IOException {
-        HttpServletRequest httpRequest = (HttpServletRequest) request;
-
-        SavedRequest savedRequest = new SavedRequest(httpRequest, portResolver);
-
-        if (logger.isDebugEnabled()) {
-            logger.debug("Authentication entry point being called; SavedRequest added to Session: " + savedRequest);
-        }
-
-        if (createSessionAllowed) {
-            // Store the HTTP request itself. Used by AbstractProcessingFilter
-            // for redirection after successful authentication (SEC-29)
-            httpRequest.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest);
-        }
-
         // SEC-112: Clear the SecurityContextHolder's Authentication, as the
         // existing Authentication is no longer considered valid
         SecurityContextHolder.getContext().setAuthentication(null);
+        saveRequestIfAllowed(request);
+        logger.debug("Calling Authentication entry point.");
+        authenticationEntryPoint.commence(request, response, reason);
+    }
 
-        authenticationEntryPoint.commence(httpRequest, response, reason);
+    private void saveRequestIfAllowed(HttpServletRequest request) {
+        if (!justUseSavedRequestOnGet || "GET".equals(request.getMethod())) {
+            SavedRequest savedRequest = new SavedRequest(request, portResolver);
+
+            if (createSessionAllowed || request.getSession(false) != null) {
+                // Store the HTTP request itself. Used by AbstractProcessingFilter
+                // for redirection after successful authentication (SEC-29)
+                request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest);
+                logger.debug("SavedRequest added to Session: " + savedRequest);
+            }
+        }
     }
 
     public void setAccessDeniedHandler(AccessDeniedHandler accessDeniedHandler) {
@@ -234,6 +235,14 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements
         this.throwableAnalyzer = throwableAnalyzer;
     }
 
+    /**
+     * If <code>true</code>, will only use <code>SavedRequest</code> to determine the target URL on successful
+     * authentication if the request that caused the authentication request was a GET. Defaults to false.
+     */
+    public void setJustUseSavedRequestOnGet(boolean justUseSavedRequestOnGet) {
+        this.justUseSavedRequestOnGet = justUseSavedRequestOnGet;
+    }
+
     public int getOrder() {
         return FilterChainOrder.EXCEPTION_TRANSLATION_FILTER;
     }

core/src/main/java/org/springframework/security/ui/SavedRequestAwareAuthenticationSuccessHandler.java

@@ -9,14 +9,10 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.springframework.security.Authentication;
 import org.springframework.security.ui.savedrequest.SavedRequest;
 import org.springframework.security.util.RedirectUtils;
-import org.springframework.security.util.UrlUtils;
 import org.springframework.security.wrapper.SavedRequestAwareWrapper;
-import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
 /**
@@ -49,80 +45,30 @@ import org.springframework.util.StringUtils;
  * @version $Id$
  * @since 2.5
  */
-public class SavedRequestAwareAuthenticationSuccessHandler implements AuthenticationSuccessHandler {
-    public static String DEFAULT_TARGET_PARAMETER = "spring-security-redirect";
-
-    protected final Log logger = LogFactory.getLog(this.getClass());
-
-    /* SEC-213 */
-    private String targetUrlParameter = DEFAULT_TARGET_PARAMETER;
-
-    /**
-     * If <code>true</code>, will only use <code>SavedRequest</code> to determine the target URL on successful
-     * authentication if the request that caused the authentication request was a GET.
-     * It will then return null for a POST/PUT request.
-     * Defaults to false.
-     */
-    private boolean justUseSavedRequestOnGet = false;
-
-    private String defaultTargetUrl = "/";
-
-    /**
-     * If <code>true</code>, will always redirect to the value of
-     * {@link #getDefaultTargetUrl} upon successful authentication, irrespective
-     * of the page that caused the authentication request (defaults to
-     * <code>false</code>).
-     */
-    private boolean alwaysUseDefaultTargetUrl = false;
-
-    private boolean useRelativeContext = false;
+public class SavedRequestAwareAuthenticationSuccessHandler extends SimpleUrlAuthenticationSuccessHandler {
 
+    @Override
     public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response,
             Authentication authentication) throws ServletException, IOException {
+        SavedRequest savedRequest = getSavedRequest(request);
+
+        if (savedRequest == null) {
+            super.onAuthenticationSuccess(request, response, authentication);
 
-        if (alwaysUseDefaultTargetUrl) {
-            removeSavedRequest(request);
-            RedirectUtils.sendRedirect(request, response, defaultTargetUrl, useRelativeContext);
             return;
         }
 
-        // Check for the parameter and use that if available
-        String targetUrl = request.getParameter(targetUrlParameter);
-
-        if (StringUtils.hasText(targetUrl)) {
-            try {
-                targetUrl = URLDecoder.decode(targetUrl, "UTF-8");
-            } catch (UnsupportedEncodingException e) {
-                throw new IllegalStateException("UTF-8 not supported. Shouldn't be possible");
-            }
-
-            logger.debug("Found targetUrlParameter in request. Redirecting to: " + targetUrl);
-
+        if (isAlwaysUseDefaultTargetUrl() || StringUtils.hasText(request.getParameter(targetUrlParameter))) {
             removeSavedRequest(request);
-            RedirectUtils.sendRedirect(request, response, targetUrl, useRelativeContext);
+            super.onAuthenticationSuccess(request, response, authentication);
 
             return;
         }
 
-        // Try the SavedRequest URL
-        SavedRequest savedRequest = getSavedRequest(request);
-
-        if (savedRequest != null) {
-            if (!justUseSavedRequestOnGet || savedRequest.getMethod().equals("GET")) {
-                targetUrl = savedRequest.getFullRequestUrl();
-                logger.debug("Redirecting to SavedRequest Url: " + targetUrl);
-            } else {
-                removeSavedRequest(request);
-            }
-        }
-
-        if (targetUrl == null) {
-            targetUrl = defaultTargetUrl;
-            logger.debug("Redirecting to default Url: " + targetUrl);
-        }
-
+        // Use the SavedRequest URL
+        String targetUrl = savedRequest.getFullRequestUrl();
+        logger.debug("Redirecting to SavedRequest Url: " + targetUrl);
         RedirectUtils.sendRedirect(request, response, targetUrl, useRelativeContext);
-
     }
 
     private SavedRequest getSavedRequest(HttpServletRequest request) {
@@ -143,78 +89,4 @@ public class SavedRequestAwareAuthenticationSuccessHandler implements Authentica
             session.removeAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY);
         }
     }
-
-    /**
-     * Supplies the default target Url that will be used if no saved request is found or the
-     * <tt>alwaysUseDefaultTargetUrl</tt> property is set to true. If not set, defaults to <tt>/</tt>.
-     *
-     * @return the defaultTargetUrl property
-     */
-    protected String getDefaultTargetUrl() {
-        return defaultTargetUrl;
-    }
-
-    /**
-     * Supplies the default target Url that will be used if no saved request is found in the session, or the
-     * <tt>alwaysUseDefaultTargetUrl</tt> property is set to true. If not set, defaults to <tt>/</tt>. It
-     * will be treated as relative to the web-app's context path, and should include the leading <code>/</code>.
-     * Alternatively, inclusion of a scheme name (such as "http://" or "https://") as the prefix will denote a
-     * fully-qualified URL and this is also supported.
-     *
-     * @param defaultTargetUrl
-     */
-    public void setDefaultTargetUrl(String defaultTargetUrl) {
-        Assert.isTrue(UrlUtils.isValidRedirectUrl(defaultTargetUrl),
-                "defaultTarget must start with '/' or with 'http(s)'");
-        this.defaultTargetUrl = defaultTargetUrl;
-    }
-
-    /**
-     * If <code>true</code>, will always redirect to the value of <tt>defaultTargetUrl</tt>
-     * (defaults to <code>false</code>).
-     */
-    public void setAlwaysUseDefaultTargetUrl(boolean alwaysUseDefaultTargetUrl) {
-        this.alwaysUseDefaultTargetUrl = alwaysUseDefaultTargetUrl;
-    }
-
-    boolean isAlwaysUseDefaultTargetUrl() {
-        return alwaysUseDefaultTargetUrl;
-    }
-
-    /**
-     * @return <code>true</code> if just GET request will be used
-     * to determine target URLs, <code>false</code> otherwise.
-     */
-    protected boolean isJustUseSavedRequestOnGet() {
-        return justUseSavedRequestOnGet;
-    }
-
-    /**
-     * @param justUseSavedRequestOnGet set to <code>true</code> if
-     * just GET request will be used to determine target URLs,
-     * <code>false</code> otherwise.
-     */
-    public void setJustUseSavedRequestOnGet(boolean justUseSavedRequestOnGet) {
-        this.justUseSavedRequestOnGet = justUseSavedRequestOnGet;
-    }
-
-    /**
-     * Before checking the SavedRequest, the current request will be checked for this parameter
-     * and the value used as the target URL if resent.
-     *
-     *  @param targetUrlParameter the name of the parameter containing the encoded target URL. Defaults
-     *  to "redirect".
-     */
-    public void setTargetUrlParameter(String targetUrlParameter) {
-        Assert.hasText("targetUrlParamete canot be null or empty");
-        this.targetUrlParameter = targetUrlParameter;
-    }
-
-    /**
-     * If <tt>true</tt>, causes any redirection URLs to be calculated minus the protocol
-     * and context path (defaults to <tt>false</tt>).
-     */
-    public void setUseRelativeContext(boolean useRelativeContext) {
-        this.useRelativeContext = useRelativeContext;
-    }
 }

core/src/main/java/org/springframework/security/ui/SimpleUrlAuthenticationSuccessHandler.java

@@ -0,0 +1,117 @@
+package org.springframework.security.ui;
+
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.springframework.security.Authentication;
+import org.springframework.security.util.RedirectUtils;
+import org.springframework.security.util.UrlUtils;
+import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
+
+public class SimpleUrlAuthenticationSuccessHandler implements AuthenticationSuccessHandler {
+
+    public static String DEFAULT_TARGET_PARAMETER = "spring-security-redirect";
+    protected final Log logger = LogFactory.getLog(this.getClass());
+    protected String targetUrlParameter = DEFAULT_TARGET_PARAMETER;
+    protected String defaultTargetUrl = "/";
+    protected boolean alwaysUseDefaultTargetUrl = false;
+    protected boolean useRelativeContext = false;
+
+    public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response,
+            Authentication authentication) throws IOException, ServletException {
+
+        if (isAlwaysUseDefaultTargetUrl()) {
+            RedirectUtils.sendRedirect(request, response, getDefaultTargetUrl(), useRelativeContext);
+            return;
+        }
+
+        // Check for the parameter and use that if available
+        String targetUrl = request.getParameter(targetUrlParameter);
+
+        if (StringUtils.hasText(targetUrl)) {
+            try {
+                targetUrl = URLDecoder.decode(targetUrl, "UTF-8");
+            } catch (UnsupportedEncodingException e) {
+                throw new IllegalStateException("UTF-8 not supported. Shouldn't be possible");
+            }
+
+            logger.debug("Found targetUrlParameter in request. Redirecting to: " + targetUrl);
+
+            RedirectUtils.sendRedirect(request, response, targetUrl, useRelativeContext);
+
+            return;
+        }
+
+        if (targetUrl == null) {
+            targetUrl = getDefaultTargetUrl();
+            logger.debug("Redirecting to default Url: " + targetUrl);
+        }
+
+        RedirectUtils.sendRedirect(request, response, targetUrl, useRelativeContext);
+    }
+
+    /**
+     * Supplies the default target Url that will be used if no saved request is found or the
+     * <tt>alwaysUseDefaultTargetUrl</tt> property is set to true. If not set, defaults to <tt>/</tt>.
+     *
+     * @return the defaultTargetUrl property
+     */
+    protected String getDefaultTargetUrl() {
+        return defaultTargetUrl;
+    }
+
+    /**
+     * Supplies the default target Url that will be used if no saved request is found in the session, or the
+     * <tt>alwaysUseDefaultTargetUrl</tt> property is set to true. If not set, defaults to <tt>/</tt>. It
+     * will be treated as relative to the web-app's context path, and should include the leading <code>/</code>.
+     * Alternatively, inclusion of a scheme name (such as "http://" or "https://") as the prefix will denote a
+     * fully-qualified URL and this is also supported.
+     *
+     * @param defaultTargetUrl
+     */
+    public void setDefaultTargetUrl(String defaultTargetUrl) {
+        Assert.isTrue(UrlUtils.isValidRedirectUrl(defaultTargetUrl),
+                "defaultTarget must start with '/' or with 'http(s)'");
+        this.defaultTargetUrl = defaultTargetUrl;
+    }
+
+    /**
+     * If <code>true</code>, will always redirect to the value of <tt>defaultTargetUrl</tt>
+     * (defaults to <code>false</code>).
+     */
+    public void setAlwaysUseDefaultTargetUrl(boolean alwaysUseDefaultTargetUrl) {
+        this.alwaysUseDefaultTargetUrl = alwaysUseDefaultTargetUrl;
+    }
+
+    protected boolean isAlwaysUseDefaultTargetUrl() {
+        return alwaysUseDefaultTargetUrl;
+    }
+
+    /**
+     * Before checking the SavedRequest, the current request will be checked for this parameter
+     * and the value used as the target URL if resent.
+     *
+     *  @param targetUrlParameter the name of the parameter containing the encoded target URL. Defaults
+     *  to "redirect".
+     */
+    public void setTargetUrlParameter(String targetUrlParameter) {
+        Assert.hasText("targetUrlParamete canot be null or empty");
+        this.targetUrlParameter = targetUrlParameter;
+    }
+
+    /**
+     * If <tt>true</tt>, causes any redirection URLs to be calculated minus the protocol
+     * and context path (defaults to <tt>false</tt>).
+     */
+    public void setUseRelativeContext(boolean useRelativeContext) {
+        this.useRelativeContext = useRelativeContext;
+    }
+}

core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java

@@ -366,33 +366,6 @@ public class AbstractProcessingFilterTests extends TestCase {
         assertNotNull(SecurityContextHolder.getContext().getAuthentication());
     }
 
-    public void testSuccessfulAuthenticationCausesRedirectToDefaultTargetUrlOnPOSTSavedRequest() throws Exception {
-        // Setup our HTTP request with a POST method request
-        MockHttpServletRequest request = createMockRequest();
-        request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makePostSavedRequestForUrl());
-
-       // Setup our filter configuration
-        MockFilterConfig config = new MockFilterConfig(null, null);
-
-        // Setup our expectation that the filter chain will be invoked, as we want to go to the location requested in the session
-        MockFilterChain chain = new MockFilterChain(true);
-        MockHttpServletResponse response = new MockHttpServletResponse();
-
-        // Setup our test object, to grant access
-        MockAbstractProcessingFilter filter = new MockAbstractProcessingFilter(true);
-
-        filter.setFilterProcessesUrl("/j_mock_post");
-        filter.setSuccessHandler(successHandler);
-        successHandler.setDefaultTargetUrl("/foobar");
-        // Configure not to use POST SavedRequest
-        successHandler.setJustUseSavedRequestOnGet(true);
-
-        // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
-        assertEquals("/mycontext/foobar", response.getRedirectedUrl());
-        assertNotNull(SecurityContextHolder.getContext().getAuthentication());
-    }
-
     /**
      * SEC-297 fix.
      */

core/src/test/java/org/springframework/security/ui/ExceptionTranslationFilterTests.java

@@ -196,6 +196,18 @@ public class ExceptionTranslationFilterTests extends TestCase {
         assertEquals("http://www.example.com:8080/mycontext/secure/page.html", getSavedRequestUrl(request));
     }
 
+    public void testSavedRequestIsNotStoredForPostIfJustUseSaveRequestOnGetIsSet() throws Exception {
+        ExceptionTranslationFilter filter = new ExceptionTranslationFilter();
+        filter.setJustUseSavedRequestOnGet(true);
+        filter.setAuthenticationEntryPoint(new MockAuthenticationEntryPoint("/login.jsp"));
+        filter.setPortResolver(new MockPortResolver(8080, 8443));
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockFilterChain chain = new MockFilterChain(false, true, false, false);
+        request.setMethod("POST");
+        filter.doFilter(request, new MockHttpServletResponse(), chain);
+        assertTrue(request.getSession().getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY) == null);
+    }
+
     public void testStartupDetectsMissingAuthenticationEntryPoint() throws Exception {
         ExceptionTranslationFilter filter = new ExceptionTranslationFilter();