SEC-1057: Refactored TargetUrlResolver to remove SavedRequest from determineTargetUrl method.

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

@@ -68,7 +68,7 @@ import javax.servlet.http.HttpSession;
  * <ul>
  * <li><code>defaultTargetUrl</code> indicates the URL that should be used
  * for redirection if the <code>HttpSession</code> attribute named
- * {@link #SPRING_SECURITY_SAVED_REQUEST_KEY} does not indicate the target URL once
+ * {@link SavedRequest#SPRING_SECURITY_SAVED_REQUEST_KEY} does not indicate the target URL once
  * authentication is completed successfully. eg: <code>/</code>. The
  * <code>defaultTargetUrl</code> will be treated as relative to the web-app's
  * context path, and should include the leading <code>/</code>.
@@ -84,7 +84,7 @@ import javax.servlet.http.HttpSession;
  * <li><code>alwaysUseDefaultTargetUrl</code> causes successful
  * authentication to always redirect to the <code>defaultTargetUrl</code>,
  * even if the <code>HttpSession</code> attribute named {@link
- * # SPRING_SECURITY_SAVED_REQUEST_KEY} defines the intended target URL.</li>
+ * SavedRequest# SPRING_SECURITY_SAVED_REQUEST_KEY} defines the intended target URL.</li>
  * </ul>
  * <p>
  * To configure this filter to redirect to specific pages as the result of
@@ -132,8 +132,6 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl
         ApplicationEventPublisherAware, MessageSourceAware {
     //~ Static fields/initializers =====================================================================================
 
-    public static final String SPRING_SECURITY_SAVED_REQUEST_KEY = "SPRING_SECURITY_SAVED_REQUEST_KEY";
-
     public static final String SPRING_SECURITY_LAST_EXCEPTION_KEY = "SPRING_SECURITY_LAST_EXCEPTION";
 
     //~ Instance fields ================================================================================================
@@ -160,8 +158,8 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl
     private String authenticationFailureUrl;
 
     /**
-     * Where to redirect the browser to if authentication is successful but
-     * SPRING_SECURITY_SAVED_REQUEST_KEY is <code>null</code>
+     * Where to redirect the browser to if authentication is successful and no saved request is stored
+     * in the session.
      */
     private String defaultTargetUrl;
 
@@ -278,24 +276,6 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl
         chain.doFilter(request, response);
     }
 
-    public static String obtainFullSavedRequestUrl(HttpServletRequest request) {
-        SavedRequest savedRequest = getSavedRequest(request);
-
-        return savedRequest == null ? null : savedRequest.getFullRequestUrl();
-    }
-
-    private static SavedRequest getSavedRequest(HttpServletRequest request) {
-        HttpSession session = request.getSession(false);
-
-        if (session == null) {
-            return null;
-        }
-
-        SavedRequest savedRequest = (SavedRequest) session.getAttribute(SPRING_SECURITY_SAVED_REQUEST_KEY);
-
-        return savedRequest;
-     }
-
     protected void onPreAuthentication(HttpServletRequest request, HttpServletResponse response)
             throws AuthenticationException, IOException {
     }
@@ -389,7 +369,7 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl
     protected String determineTargetUrl(HttpServletRequest request) {
         // Don't attempt to obtain the url from the saved request if alwaysUsedefaultTargetUrl is set
         String targetUrl = alwaysUseDefaultTargetUrl ? null :
-            targetUrlResolver.determineTargetUrl(getSavedRequest(request), request, SecurityContextHolder.getContext().getAuthentication());
+            targetUrlResolver.determineTargetUrl(request, SecurityContextHolder.getContext().getAuthentication());
 
         if (targetUrl == null) {
             targetUrl = getDefaultTargetUrl();

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

@@ -199,7 +199,7 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements
         if (createSessionAllowed) {
             // Store the HTTP request itself. Used by AbstractProcessingFilter
             // for redirection after successful authentication (SEC-29)
-            httpRequest.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest);
+            httpRequest.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest);
         }
 
         // SEC-112: Clear the SecurityContextHolder's Authentication, as the

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

@@ -18,7 +18,6 @@ package org.springframework.security.ui;
 import javax.servlet.http.HttpServletRequest;
 
 import org.springframework.security.Authentication;
-import org.springframework.security.ui.savedrequest.SavedRequest;
 
 /**
  * Used by {@link AbstractProcessingFilter} to determine target URL in case of
@@ -32,11 +31,10 @@ import org.springframework.security.ui.savedrequest.SavedRequest;
 public interface TargetUrlResolver {
 	
 	/**
-	 * @param savedRequest The request that initiated the authentication process
 	 * @param currentRequest the current request
 	 * @param auth The authentication token generated after successful authentication
 	 * @return The URL to be used 
 	 */
-	public String determineTargetUrl(SavedRequest savedRequest, HttpServletRequest currentRequest, Authentication auth);
+	public String determineTargetUrl(HttpServletRequest currentRequest, Authentication auth);
 
 }

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

@@ -19,6 +19,7 @@ import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
 
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpSession;
 
 import org.springframework.security.Authentication;
 import org.springframework.security.ui.savedrequest.SavedRequest;
@@ -29,9 +30,9 @@ import org.springframework.util.StringUtils;
 /**
  * Default implementation for {@link TargetUrlResolver}
  * <p>
- * Returns a target URL based from the contents of the configured <tt>targetUrlParameter</tt> if present on 
- * the current request. Failing that, the SavedRequest in the session will be used. 
- * 
+ * Returns a target URL based from the contents of the configured <tt>targetUrlParameter</tt> if present on
+ * the current request. Failing that, the SavedRequest in the session will be used.
+ *
  * @author Martino Piccinato
  * @author Luke Taylor
  * @version $Id$
@@ -40,26 +41,22 @@ import org.springframework.util.StringUtils;
  */
 public class TargetUrlResolverImpl implements TargetUrlResolver {
     public static String DEFAULT_TARGET_PARAMETER = "spring-security-redirect";
-    
+
     /* 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
+
+    /**
+     * 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 boolean justUseSavedRequestOnGet = false;
 
-    /* (non-Javadoc)
-	 * @see org.acegisecurity.ui.TargetUrlResolver#determineTargetUrl(org.acegisecurity.ui.savedrequest.SavedRequest, javax.servlet.http.HttpServletRequest, org.acegisecurity.Authentication)
-	 */
-	public String determineTargetUrl(SavedRequest savedRequest, HttpServletRequest currentRequest,
-            Authentication auth) {
+    public String determineTargetUrl(HttpServletRequest currentRequest, Authentication auth) {
 
         String targetUrl = currentRequest.getParameter(targetUrlParameter);
-        
+
         if (StringUtils.hasText(targetUrl)) {
             try {
                 return URLDecoder.decode(targetUrl, "UTF-8");
@@ -68,6 +65,8 @@ public class TargetUrlResolverImpl implements TargetUrlResolver {
             }
         }
 
+        SavedRequest savedRequest = getSavedRequest(currentRequest);
+
         if (savedRequest != null) {
             if (!justUseSavedRequestOnGet || savedRequest.getMethod().equals("GET")) {
                 targetUrl = savedRequest.getFullRequestUrl();
@@ -75,35 +74,47 @@ public class TargetUrlResolverImpl implements TargetUrlResolver {
         }
 
         return targetUrl;
-	}
-
-	/**
-	 * @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");
+    }
+
+    private static SavedRequest getSavedRequest(HttpServletRequest request) {
+        HttpSession session = request.getSession(false);
+
+        if (session == null) {
+            return null;
+        }
+
+        SavedRequest savedRequest = (SavedRequest) session.getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY);
+
+        return savedRequest;
+    }
+
+    /**
+     * @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;
     }
 }

core/src/main/java/org/springframework/security/ui/savedrequest/SavedRequest.java

@@ -69,6 +69,8 @@ public class SavedRequest implements java.io.Serializable {
     private String servletPath;
     private int serverPort;
 
+    public static final String SPRING_SECURITY_SAVED_REQUEST_KEY = "SPRING_SECURITY_SAVED_REQUEST_KEY";
+
     //~ Constructors ===================================================================================================
 
     @SuppressWarnings("unchecked")

core/src/main/java/org/springframework/security/wrapper/SavedRequestAwareWrapper.java

@@ -34,7 +34,6 @@ import javax.servlet.http.HttpSession;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.springframework.security.ui.AbstractProcessingFilter;
 import org.springframework.security.ui.savedrequest.Enumerator;
 import org.springframework.security.ui.savedrequest.FastHttpDateFormat;
 import org.springframework.security.ui.savedrequest.SavedRequest;
@@ -91,7 +90,7 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW
             return;
         }
 
-        SavedRequest saved = (SavedRequest) session.getAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY);
+        SavedRequest saved = (SavedRequest) session.getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY);
 
         if ((saved != null) && saved.doesRequestMatch(request, portResolver)) {
             if (logger.isDebugEnabled()) {
@@ -99,7 +98,7 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW
             }
 
             savedRequest = saved;
-            session.removeAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY);
+            session.removeAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY);
 
             formats[0] = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US);
             formats[1] = new SimpleDateFormat("EEEEEE, dd-MMM-yy HH:mm:ss zzz", Locale.US);

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

@@ -352,7 +352,7 @@ public class AbstractProcessingFilterTests extends TestCase {
             throws Exception {
         // Setup our HTTP request
         MockHttpServletRequest request = createMockRequest();
-        request.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl());
+        request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl());
 
         // Setup our filter configuration
         MockFilterConfig config = new MockFilterConfig(null, null);
@@ -378,7 +378,7 @@ public class AbstractProcessingFilterTests extends TestCase {
     public void testSuccessfulAuthenticationCausesRedirectToSessionSpecifiedUrl() throws Exception {
         // Setup our HTTP request
         MockHttpServletRequest request = createMockRequest();
-        request.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl());
+        request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl());
 
         // Setup our filter configuration
         MockFilterConfig config = new MockFilterConfig(null, null);
@@ -400,7 +400,7 @@ public class AbstractProcessingFilterTests extends TestCase {
     public void testSuccessfulAuthenticationCausesRedirectToDefaultTargetUrlOnPOSTSavedRequest() throws Exception {
         // Setup our HTTP request with a POST method request
         MockHttpServletRequest request = createMockRequest();
-        request.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, makePostSavedRequestForUrl());
+        request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makePostSavedRequestForUrl());
 
        // Setup our filter configuration
         MockFilterConfig config = new MockFilterConfig(null, null);

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

@@ -27,6 +27,7 @@ import org.springframework.security.MockPortResolver;
 import org.springframework.security.context.SecurityContextHolder;
 
 import org.springframework.security.providers.anonymous.AnonymousAuthenticationToken;
+import org.springframework.security.ui.savedrequest.SavedRequest;
 import org.springframework.security.util.AuthorityUtils;
 
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -38,6 +39,8 @@ import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpSession;
 
 /**
  * Tests {@link ExceptionTranslationFilter}.
@@ -54,6 +57,18 @@ public class ExceptionTranslationFilterTests extends TestCase {
         SecurityContextHolder.clearContext();
     }
 
+    private static String getSavedRequestUrl(HttpServletRequest request) {
+        HttpSession session = request.getSession(false);
+
+        if (session == null) {
+            return null;
+        }
+
+        SavedRequest savedRequest = (SavedRequest) session.getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY);
+
+        return savedRequest.getFullRequestUrl();
+    }
+
     public void testAccessDeniedWhenAnonymous() throws Exception {
         // Setup our HTTP request
         MockHttpServletRequest request = new MockHttpServletRequest();
@@ -79,8 +94,7 @@ public class ExceptionTranslationFilterTests extends TestCase {
         MockHttpServletResponse response = new MockHttpServletResponse();
         filter.doFilter(request, response, chain);
         assertEquals("/mycontext/login.jsp", response.getRedirectedUrl());
-        assertEquals("http://www.example.com/mycontext/secure/page.html", AbstractProcessingFilter
-                .obtainFullSavedRequestUrl(request));
+        assertEquals("http://www.example.com/mycontext/secure/page.html", getSavedRequestUrl(request));
     }
 
     public void testAccessDeniedWhenNonAnonymous() throws Exception {
@@ -148,8 +162,7 @@ public class ExceptionTranslationFilterTests extends TestCase {
         MockHttpServletResponse response = new MockHttpServletResponse();
         filter.doFilter(request, response, chain);
         assertEquals("/mycontext/login.jsp", response.getRedirectedUrl());
-        assertEquals("http://www.example.com/mycontext/secure/page.html", AbstractProcessingFilter
-                .obtainFullSavedRequestUrl(request));
+        assertEquals("http://www.example.com/mycontext/secure/page.html", getSavedRequestUrl(request));
     }
 
     public void testRedirectedToLoginFormAndSessionShowsOriginalTargetWithExoticPortWhenAuthenticationException()
@@ -180,8 +193,7 @@ public class ExceptionTranslationFilterTests extends TestCase {
         MockHttpServletResponse response = new MockHttpServletResponse();
         filter.doFilter(request, response, chain);
         assertEquals("/mycontext/login.jsp", response.getRedirectedUrl());
-        assertEquals("http://www.example.com:8080/mycontext/secure/page.html", AbstractProcessingFilter
-                .obtainFullSavedRequestUrl(request));
+        assertEquals("http://www.example.com:8080/mycontext/secure/page.html", getSavedRequestUrl(request));
     }
 
     public void testStartupDetectsMissingAuthenticationEntryPoint() throws Exception {

core/src/test/java/org/springframework/security/wrapper/SavedRequestAwareWrapperTests.java

@@ -11,7 +11,6 @@ import javax.servlet.http.Cookie;
 
 import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
-import org.springframework.security.ui.AbstractProcessingFilter;
 import org.springframework.security.ui.savedrequest.FastHttpDateFormat;
 import org.springframework.security.ui.savedrequest.SavedRequest;
 import org.springframework.security.util.PortResolverImpl;
@@ -21,7 +20,7 @@ public class SavedRequestAwareWrapperTests {
     private SavedRequestAwareWrapper createWrapper(MockHttpServletRequest requestToSave, MockHttpServletRequest requestToWrap) {
         if (requestToSave != null) {
             SavedRequest savedRequest = new SavedRequest(requestToSave, new PortResolverImpl());
-            requestToWrap.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest);
+            requestToWrap.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest);
         }
         return new SavedRequestAwareWrapper(requestToWrap, new PortResolverImpl(),"ROLE_");
     }

core/src/test/resources/log4j.properties

@@ -6,9 +6,9 @@ log4j.rootLogger=INFO, stdout
 
 log4j.appender.stdout=org.apache.log4j.ConsoleAppender
 log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
-log4j.appender.stdout.layout.ConversionPattern=%d %p %c{1} - %m%n
+log4j.appender.stdout.layout.ConversionPattern=%p %c{1} - %m%n
 
-log4j.logger.org.springframework.security=DEBUG,stdout
+log4j.logger.org.springframework.security=DEBUG
 log4j.logger.org.springframework.ldap=DEBUG
 
-log4j.logger.org.apache.directory=ERROR,stdout
+log4j.logger.org.apache.directory=ERROR