SEC-1430: internalize session key for SavedRequest. This should be accessed using the RequestCache interface if required. Additional refactoring of related tests which were still in AbstractAuthenticationProcessingFilterTests for historical reasons, but should be in their respective success/failure handler test classes.

web/src/main/java/org/springframework/security/web/WebAttributes.java

@@ -10,5 +10,4 @@ public final class WebAttributes {
     public static final String ACCESS_DENIED_403 = "SPRING_SECURITY_403_EXCEPTION";
     public static final String AUTHENTICATION_EXCEPTION = "SPRING_SECURITY_LAST_EXCEPTION";
     public static final String LAST_USERNAME = "SPRING_SECURITY_LAST_USERNAME";
-    public static final String SAVED_REQUEST = "SPRING_SECURITY_SAVED_REQUEST_KEY";
 }

web/src/main/java/org/springframework/security/web/access/AccessDeniedHandlerImpl.java

@@ -42,11 +42,7 @@ import org.springframework.security.web.WebAttributes;
  */
 public class AccessDeniedHandlerImpl implements AccessDeniedHandler {
     //~ Static fields/initializers =====================================================================================
-    /**
-     * @deprecated Use the value in {@link WebAttributes} directly.
-     */
-    @Deprecated
-    public static final String SPRING_SECURITY_ACCESS_DENIED_EXCEPTION_KEY = WebAttributes.ACCESS_DENIED_403;
+
     protected static final Log logger = LogFactory.getLog(AccessDeniedHandlerImpl.class);
 
     //~ Instance fields ================================================================================================

web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java

@@ -14,21 +14,21 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.web.WebAttributes;
+import org.springframework.util.Assert;
 
 /**
  * The default implementation of {@link SessionAuthenticationStrategy}.
  * <p>
  * Creates a new session for the newly authenticated user if they already have a session (as a defence against
- * session-fixation protection attacks), and copies their
- * session attributes across to the new session (can be disabled by setting <tt>migrateSessionAttributes</tt> to
- * <tt>false</tt>).
+ * session-fixation protection attacks), and copies their session attributes across to the new session
+ * (can be disabled by setting {@code migrateSessionAttributes} to {@code false}).
  * <p>
  * This approach will only be effective if your servlet container always assigns a new session Id when a session is
  * invalidated and a new session created by calling {@link HttpServletRequest#getSession()}.
  * <p>
- * If concurrent session control is in use, then a <tt>SessionRegistry</tt> must be injected.
+ * If concurrent session control is in use, then a {@code SessionRegistry} must be injected.
  * <p>
- * <h3>Issues with <tt>HttpSessionBindingListener</tt></h3>
+ * <h3>Issues with {@code HttpSessionBindingListener}</h3>
  * <p>
  * The migration of existing attributes to the newly-created session may cause problems if any of the objects
  * implement the {@code HttpSessionBindingListener} interface in a way which makes assumptions about the life-cycle of
@@ -56,11 +56,11 @@ public class SessionFixationProtectionStrategy implements SessionAuthenticationS
      * In the case where the attributes will not be migrated, this field allows a list of named attributes
      * which should <em>not</em> be discarded.
      */
-    private List<String> retainedAttributes = Arrays.asList(WebAttributes.SAVED_REQUEST);
+    private List<String> retainedAttributes = null;
 
     /**
-     * If set to <tt>true</tt>, a session will always be created, even if one didn't exist at the start of the request.
-     * Defaults to <tt>false</tt>.
+     * If set to {@code true}, a session will always be created, even if one didn't exist at the start of the request.
+     * Defaults to {@code false}.
      */
     private boolean alwaysCreateSession;
 
@@ -68,12 +68,12 @@ public class SessionFixationProtectionStrategy implements SessionAuthenticationS
      * Called when a user is newly authenticated.
      * <p>
      * If a session already exists, and matches the session Id from the client, a new session will be created, and the
-     * session attributes copied to it (if <tt>migrateSessionAttributes</tt> is set).
+     * session attributes copied to it (if {@code migrateSessionAttributes} is set).
      * The sessionRegistry will be updated with the new session information. If the client's requested session Id is
      * invalid, nothing will be done, since there is no need to change the session Id if it doesn't match the current
      * session.
      * <p>
-     * If there is no session, no action is taken unless the <tt>alwaysCreateSession</tt> property is set, in which
+     * If there is no session, no action is taken unless the {@code alwaysCreateSession} property is set, in which
      * case a session will be created if one doesn't already exist.
      */
     public void onAuthentication(Authentication authentication, HttpServletRequest request, HttpServletResponse response) {
@@ -97,7 +97,7 @@ public class SessionFixationProtectionStrategy implements SessionAuthenticationS
                         "and" : "without") +  " migrating attributes.");
             }
 
-            HashMap<String, Object> attributesToMigrate = createMigratedAttributeMap(session);
+            Map<String, Object> attributesToMigrate = extractAttributes(session);
 
             session.invalidate();
             session = request.getSession(true); // we now have a new session
@@ -111,12 +111,9 @@ public class SessionFixationProtectionStrategy implements SessionAuthenticationS
                         " not be adequately protected against session-fixation attacks");
             }
 
-            // Copy attributes to new session
-            if (attributesToMigrate != null) {
-                for (Map.Entry<String, Object> entry : attributesToMigrate.entrySet()) {
-                    session.setAttribute(entry.getKey(), entry.getValue());
-                }
-            }
+            transferAttributes(attributesToMigrate, session);
+
+            onSessionChange(originalSessionId, session, authentication);
         }
     }
 
@@ -131,17 +128,47 @@ public class SessionFixationProtectionStrategy implements SessionAuthenticationS
     protected void onSessionChange(String originalSessionId, HttpSession newSession, Authentication auth) {
     }
 
+    /**
+     * Called to extract the existing attributes from the session, prior to invalidating it. If
+     * {@code migrateAttributes} is set to {@code false}, only Spring Security attributes will be retained.
+     * All application attributes will be discarded.
+     * <p>
+     * You can override this method to control exactly what is transferred to the new session.
+     *
+     * @param session the session from which the attributes should be extracted
+     * @return the map of session attributes which should be transferred to the new session
+     */
+    protected Map<String, Object> extractAttributes(HttpSession session) {
+        return createMigratedAttributeMap(session);
+    }
+
+    /**
+     * @param attributes the attributes which were extracted from the original session by {@code extractAttributes}
+     * @param newSession the newly created session
+     */
+    private void transferAttributes(Map<String, Object> attributes, HttpSession newSession) {
+        if (attributes != null) {
+            for (Map.Entry<String, Object> entry : attributes.entrySet()) {
+                newSession.setAttribute(entry.getKey(), entry.getValue());
+            }
+        }
+    }
+
     @SuppressWarnings("unchecked")
     private HashMap<String, Object> createMigratedAttributeMap(HttpSession session) {
         HashMap<String, Object> attributesToMigrate = null;
 
-        if (migrateSessionAttributes) {
+        if (migrateSessionAttributes || retainedAttributes == null) {
             attributesToMigrate = new HashMap<String, Object>();
 
             Enumeration enumer = session.getAttributeNames();
 
             while (enumer.hasMoreElements()) {
                 String key = (String) enumer.nextElement();
+                if (!migrateSessionAttributes && !key.startsWith("SPRING_SECURITY_")) {
+                    // Only retain Spring Security attributes
+                    continue;
+                }
                 attributesToMigrate.put(key, session.getAttribute(key));
             }
         } else {
@@ -160,11 +187,27 @@ public class SessionFixationProtectionStrategy implements SessionAuthenticationS
         return attributesToMigrate;
     }
 
+    /**
+     * Defines whether attributes should be migrated to a new session or not. Has no effect if you
+     * override the {@code extractAttributes} method.
+     * <p>
+     * Attributes used by Spring Security (to store cached requests, for example) will still be retained by default,
+     * even if you set this value to {@code false}.
+     *
+     * @param migrateSessionAttributes whether the attributes from the session should be transferred to the new,
+     *         authenticated session.
+     */
     public void setMigrateSessionAttributes(boolean migrateSessionAttributes) {
         this.migrateSessionAttributes = migrateSessionAttributes;
     }
 
+    /**
+     * @deprecated Override the {@code extractAttributes} method instead
+     */
+    @Deprecated
     public void setRetainedAttributes(List<String> retainedAttributes) {
+        logger.warn("Retained attributes is deprecated. Override the extractAttributes() method instead.");
+        Assert.notNull(retainedAttributes);
         this.retainedAttributes = retainedAttributes;
     }
 

web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java

@@ -52,11 +52,6 @@ public class DefaultSavedRequest implements SavedRequest {
     //~ Static fields/initializers =====================================================================================
 
     protected static final Log logger = LogFactory.getLog(DefaultSavedRequest.class);
-    /**
-     * @deprecated Use the value in {@link WebAttributes} directly.
-     */
-    @Deprecated
-    public static final String SPRING_SECURITY_SAVED_REQUEST_KEY = WebAttributes.SAVED_REQUEST;
 
     private static final String HEADER_IF_NONE_MATCH = "If-None-Match";
 

web/src/main/java/org/springframework/security/web/savedrequest/HttpSessionRequestCache.java

@@ -8,7 +8,6 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.security.web.PortResolver;
 import org.springframework.security.web.PortResolverImpl;
-import org.springframework.security.web.WebAttributes;
 import org.springframework.security.web.util.AnyRequestMatcher;
 import org.springframework.security.web.util.RequestMatcher;
 
@@ -21,6 +20,7 @@ import org.springframework.security.web.util.RequestMatcher;
  * @since 3.0
  */
 public class HttpSessionRequestCache implements RequestCache {
+    static final String SAVED_REQUEST = "SPRING_SECURITY_SAVED_REQUEST";
     protected final Log logger = LogFactory.getLog(this.getClass());
 
     private PortResolver portResolver = new PortResolverImpl();
@@ -37,7 +37,7 @@ public class HttpSessionRequestCache implements RequestCache {
             if (createSessionAllowed || request.getSession(false) != null) {
                 // Store the HTTP request itself. Used by AbstractAuthenticationProcessingFilter
                 // for redirection after successful authentication (SEC-29)
-                request.getSession().setAttribute(WebAttributes.SAVED_REQUEST, savedRequest);
+                request.getSession().setAttribute(SAVED_REQUEST, savedRequest);
                 logger.debug("DefaultSavedRequest added to Session: " + savedRequest);
             }
         } else {
@@ -49,7 +49,7 @@ public class HttpSessionRequestCache implements RequestCache {
         HttpSession session = currentRequest.getSession(false);
 
         if (session != null) {
-            return (DefaultSavedRequest) session.getAttribute(WebAttributes.SAVED_REQUEST);
+            return (DefaultSavedRequest) session.getAttribute(SAVED_REQUEST);
         }
 
         return null;
@@ -60,7 +60,7 @@ public class HttpSessionRequestCache implements RequestCache {
 
         if (session != null) {
             logger.debug("Removing DefaultSavedRequest from session if present");
-            session.removeAttribute(WebAttributes.SAVED_REQUEST);
+            session.removeAttribute(SAVED_REQUEST);
         }
     }
 

web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java

@@ -44,6 +44,7 @@ import org.springframework.security.web.AuthenticationEntryPoint;
 import org.springframework.security.web.WebAttributes;
 import org.springframework.security.web.savedrequest.HttpSessionRequestCache;
 import org.springframework.security.web.savedrequest.DefaultSavedRequest;
+import org.springframework.security.web.savedrequest.SavedRequest;
 import org.springframework.security.web.util.ThrowableAnalyzer;
 
 /**
@@ -66,9 +67,10 @@ public class ExceptionTranslationFilterTests {
             return null;
         }
 
-        DefaultSavedRequest savedRequest = (DefaultSavedRequest) session.getAttribute(WebAttributes.SAVED_REQUEST);
+        HttpSessionRequestCache rc = new HttpSessionRequestCache();
+        SavedRequest sr = rc.getRequest(request, new MockHttpServletResponse());
 
-        return savedRequest.getRedirectUrl();
+        return sr.getRedirectUrl();
     }
 
     @Test

web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java

@@ -16,20 +16,7 @@
 package org.springframework.security.web.authentication;
 
 import static org.junit.Assert.*;
-import static org.mockito.Mockito.mock;
-
-import java.io.IOException;
-import java.util.Properties;
-
-import javax.servlet.Filter;
-import javax.servlet.FilterChain;
-import javax.servlet.FilterConfig;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import javax.servlet.http.HttpSession;
+import static org.mockito.Mockito.*;
 
 import org.junit.After;
 import org.junit.Before;
@@ -37,7 +24,6 @@ import org.junit.Test;
 import org.springframework.mock.web.MockFilterConfig;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
-import org.springframework.security.authentication.AccountExpiredException;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
@@ -45,10 +31,17 @@ import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.security.web.PortResolverImpl;
 import org.springframework.security.web.authentication.rememberme.TokenBasedRememberMeServices;
 import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy;
-import org.springframework.security.web.savedrequest.DefaultSavedRequest;
+
+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.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
 
 
 /**
@@ -63,7 +56,7 @@ public class AbstractAuthenticationProcessingFilterTests {
     SimpleUrlAuthenticationFailureHandler failureHandler;
     //~ Methods ========================================================================================================
 
-    private MockHttpServletRequest createMockRequest() {
+    private MockHttpServletRequest createMockAuthenticationRequest() {
         MockHttpServletRequest request = new MockHttpServletRequest();
 
         request.setServletPath("/j_mock_post");
@@ -75,34 +68,6 @@ public class AbstractAuthenticationProcessingFilterTests {
         return request;
     }
 
-    private void executeFilterInContainerSimulator(FilterConfig filterConfig, Filter filter, ServletRequest request,
-            ServletResponse response, FilterChain filterChain) throws ServletException, IOException {
-        filter.init(filterConfig);
-        filter.doFilter(request, response, filterChain);
-        filter.destroy();
-    }
-
-    private DefaultSavedRequest makeSavedRequestForUrl() {
-        MockHttpServletRequest request = createMockRequest();
-        request.setMethod("GET");
-        request.setServletPath("/some_protected_file.html");
-        request.setScheme("http");
-        request.setServerName("www.example.com");
-        request.setRequestURI("/mycontext/some_protected_file.html");
-
-        return new DefaultSavedRequest(request, new PortResolverImpl());
-    }
-
-//    private DefaultSavedRequest makePostSavedRequestForUrl() {
-//        MockHttpServletRequest request = createMockRequest();
-//        request.setServletPath("/some_protected_file.html");
-//        request.setScheme("http");
-//        request.setServerName("www.example.com");
-//        request.setRequestURI("/mycontext/post/some_protected_file.html");
-//        request.setMethod("POST");
-//
-//        return new DefaultSavedRequest(request, new PortResolverImpl());
-//    }
 
     @Before
     public void setUp() throws Exception {
@@ -120,7 +85,7 @@ public class AbstractAuthenticationProcessingFilterTests {
 
     @Test
     public void testDefaultProcessesFilterUrlMatchesWithPathParameter() {
-        MockHttpServletRequest request = createMockRequest();
+        MockHttpServletRequest request = createMockAuthenticationRequest();
         MockHttpServletResponse response = new MockHttpServletResponse();
         MockAuthenticationFilter filter = new MockAuthenticationFilter();
         filter.setFilterProcessesUrl("/j_spring_security_check");
@@ -129,48 +94,10 @@ public class AbstractAuthenticationProcessingFilterTests {
         assertTrue(filter.requiresAuthentication(request, response));
     }
 
-    @Test
-    public void testFailedAuthenticationRedirectsAppropriately() throws Exception {
-        // Setup our HTTP request
-        MockHttpServletRequest request = createMockRequest();
-
-        // Setup our filter configuration
-        MockFilterConfig config = new MockFilterConfig(null, null);
-
-        // Setup our expectation that the filter chain will not be invoked, as we redirect to authenticationFailureUrl
-        MockFilterChain chain = new MockFilterChain(false);
-        MockHttpServletResponse response = new MockHttpServletResponse();
-
-        // Setup our test object, to deny access
-        MockAuthenticationFilter filter = new MockAuthenticationFilter(false);
-        filter.setAuthenticationFailureHandler(failureHandler);
-
-        // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
-
-        assertEquals("/mycontext/failed.jsp", response.getRedirectedUrl());
-        assertNull(SecurityContextHolder.getContext().getAuthentication());
-
-        //Prepare again, this time using the exception mapping
-        filter = new MockAuthenticationFilter(new AccountExpiredException("You're account is expired"));
-        ExceptionMappingAuthenticationFailureHandler failureHandler = new ExceptionMappingAuthenticationFailureHandler();
-        filter.setAuthenticationFailureHandler(failureHandler);
-        Properties exceptionMappings = new Properties();
-        exceptionMappings.setProperty(AccountExpiredException.class.getName(), "/accountExpired.jsp");
-        failureHandler.setExceptionMappings(exceptionMappings);
-        response = new MockHttpServletResponse();
-
-        // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
-
-        assertEquals("/mycontext/accountExpired.jsp", response.getRedirectedUrl());
-        assertNull(SecurityContextHolder.getContext().getAuthentication());
-    }
-
     @Test
     public void testFilterProcessesUrlVariationsRespected() throws Exception {
         // Setup our HTTP request
-        MockHttpServletRequest request = createMockRequest();
+        MockHttpServletRequest request = createMockAuthenticationRequest();
         request.setServletPath("/j_OTHER_LOCATION");
         request.setRequestURI("/mycontext/j_OTHER_LOCATION");
 
@@ -187,7 +114,7 @@ public class AbstractAuthenticationProcessingFilterTests {
         filter.setAuthenticationSuccessHandler(successHandler);
 
         // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
+        filter.doFilter(request, response, chain);
         assertEquals("/mycontext/logged_in.jsp", response.getRedirectedUrl());
         assertNotNull(SecurityContextHolder.getContext().getAuthentication());
         assertEquals("test", SecurityContextHolder.getContext().getAuthentication().getPrincipal().toString());
@@ -210,7 +137,7 @@ public class AbstractAuthenticationProcessingFilterTests {
     @Test
     public void testIgnoresAnyServletPathOtherThanFilterProcessesUrl() throws Exception {
         // Setup our HTTP request
-        MockHttpServletRequest request = createMockRequest();
+        MockHttpServletRequest request = createMockAuthenticationRequest();
         request.setServletPath("/some.file.html");
         request.setRequestURI("/mycontext/some.file.html");
 
@@ -225,13 +152,13 @@ public class AbstractAuthenticationProcessingFilterTests {
         MockAuthenticationFilter filter = new MockAuthenticationFilter(false);
 
         // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
+        filter.doFilter(request, response, chain);
     }
 
     @Test
     public void testNormalOperationWithDefaultFilterProcessesUrl() throws Exception {
         // Setup our HTTP request
-        MockHttpServletRequest request = createMockRequest();
+        MockHttpServletRequest request = createMockAuthenticationRequest();
         HttpSession sessionPreAuth = request.getSession();
 
         // Setup our filter configuration
@@ -252,7 +179,7 @@ public class AbstractAuthenticationProcessingFilterTests {
         filter.afterPropertiesSet();
 
         // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
+        filter.doFilter(request, response, chain);
         assertEquals("/mycontext/logged_in.jsp", response.getRedirectedUrl());
         assertNotNull(SecurityContextHolder.getContext().getAuthentication());
         assertEquals("test", SecurityContextHolder.getContext().getAuthentication().getPrincipal().toString());
@@ -295,7 +222,7 @@ public class AbstractAuthenticationProcessingFilterTests {
     @Test
     public void testSuccessLoginThenFailureLoginResultsInSessionLosingToken() throws Exception {
         // Setup our HTTP request
-        MockHttpServletRequest request = createMockRequest();
+        MockHttpServletRequest request = createMockAuthenticationRequest();
 
         // Setup our filter configuration
         MockFilterConfig config = new MockFilterConfig(null, null);
@@ -310,7 +237,7 @@ public class AbstractAuthenticationProcessingFilterTests {
         filter.setAuthenticationSuccessHandler(successHandler);
 
         // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
+        filter.doFilter(request, response, chain);
         assertEquals("/mycontext/logged_in.jsp", response.getRedirectedUrl());
         assertNotNull(SecurityContextHolder.getContext().getAuthentication());
         assertEquals("test", SecurityContextHolder.getContext().getAuthentication().getPrincipal().toString());
@@ -327,16 +254,14 @@ public class AbstractAuthenticationProcessingFilterTests {
         filter.setAuthenticationFailureHandler(failureHandler);
 
         // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
+        filter.doFilter(request, response, chain);
         assertNull(SecurityContextHolder.getContext().getAuthentication());
     }
 
     @Test
-    public void testSuccessfulAuthenticationButWithAlwaysUseDefaultTargetUrlCausesRedirectToDefaultTargetUrl()
-            throws Exception {
+    public void testSuccessfulAuthenticationInvokesSuccessHandlerAndSetsContext() throws Exception {
         // Setup our HTTP request
-        MockHttpServletRequest request = createMockRequest();
-        request.getSession().setAttribute(DefaultSavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl());
+        MockHttpServletRequest request = createMockAuthenticationRequest();
 
         // Setup our filter configuration
         MockFilterConfig config = new MockFilterConfig(null, null);
@@ -348,59 +273,42 @@ public class AbstractAuthenticationProcessingFilterTests {
         // Setup our test object, to grant access
         MockAuthenticationFilter filter = new MockAuthenticationFilter(true);
         filter.setFilterProcessesUrl("/j_mock_post");
-        successHandler.setDefaultTargetUrl("/foobar");
-        successHandler.setAlwaysUseDefaultTargetUrl(true);
+        AuthenticationSuccessHandler successHandler = mock(AuthenticationSuccessHandler.class);
         filter.setAuthenticationSuccessHandler(successHandler);
 
         // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
-        assertEquals("/mycontext/foobar", response.getRedirectedUrl());
+        filter.doFilter(request, response, chain);
+
+        verify(successHandler).onAuthenticationSuccess(any(HttpServletRequest.class), any(HttpServletResponse.class),
+                any(Authentication.class));
+
         assertNotNull(SecurityContextHolder.getContext().getAuthentication());
     }
 
     @Test
-    public void testSuccessfulAuthenticationCausesRedirectToSessionSpecifiedUrl() throws Exception {
+    public void testFailedAuthenticationInvokesFailureHandler() throws Exception {
         // Setup our HTTP request
-        MockHttpServletRequest request = createMockRequest();
-        request.getSession().setAttribute(DefaultSavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl());
+        MockHttpServletRequest request = createMockAuthenticationRequest();
 
         // 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);
+        // Setup our expectation that the filter chain will not be invoked, as we redirect to authenticationFailureUrl
+        MockFilterChain chain = new MockFilterChain(false);
         MockHttpServletResponse response = new MockHttpServletResponse();
 
-        // Setup our test object, to grant access
-        MockAuthenticationFilter filter = new MockAuthenticationFilter(true);
-        filter.setFilterProcessesUrl("/j_mock_post");
+        // Setup our test object, to deny access
+        MockAuthenticationFilter filter = new MockAuthenticationFilter(false);
+        AuthenticationFailureHandler failureHandler = mock(AuthenticationFailureHandler.class);
+        filter.setAuthenticationFailureHandler(failureHandler);
 
         // Test
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
-        assertEquals(makeSavedRequestForUrl().getRedirectUrl(), response.getRedirectedUrl());
-        assertNotNull(SecurityContextHolder.getContext().getAuthentication());
-    }
-
-    /**
-     * SEC-297 fix.
-     */
-    @Test
-    public void testFullDefaultTargetUrlDoesNotHaveContextPathPrepended() throws Exception {
-        MockHttpServletRequest request = createMockRequest();
-        MockFilterConfig config = new MockFilterConfig(null, null);
-
-        MockFilterChain chain = new MockFilterChain(true);
-        MockHttpServletResponse response = new MockHttpServletResponse();
+        filter.doFilter(request, response, chain);
 
-        // Setup our test object, to grant access
-        MockAuthenticationFilter filter = new MockAuthenticationFilter(true);
-        successHandler.setDefaultTargetUrl("https://monkeymachine.co.uk/");
-        successHandler.setAlwaysUseDefaultTargetUrl(true);
-        filter.setAuthenticationSuccessHandler(successHandler);
+        verify(failureHandler).onAuthenticationFailure(any(HttpServletRequest.class), any(HttpServletResponse.class),
+                any(AuthenticationException.class));
 
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
-        assertEquals("https://monkeymachine.co.uk/", response.getRedirectedUrl());
-        assertNotNull(SecurityContextHolder.getContext().getAuthentication());
+        assertNull(SecurityContextHolder.getContext().getAuthentication());
     }
 
     /**
@@ -408,7 +316,7 @@ public class AbstractAuthenticationProcessingFilterTests {
      */
     @Test
     public void testNoSessionIsCreatedIfAllowSessionCreationIsFalse() throws Exception {
-        MockHttpServletRequest request = createMockRequest();
+        MockHttpServletRequest request = createMockAuthenticationRequest();
 
         MockFilterConfig config = new MockFilterConfig(null, null);
         MockFilterChain chain = new MockFilterChain(true);
@@ -418,10 +326,8 @@ public class AbstractAuthenticationProcessingFilterTests {
         MockAuthenticationFilter filter = new MockAuthenticationFilter(false);
         failureHandler.setAllowSessionCreation(false);
         filter.setAuthenticationFailureHandler(failureHandler);
-        successHandler.setDefaultTargetUrl("http://monkeymachine.co.uk/");
-        filter.setAuthenticationSuccessHandler(successHandler);
 
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
+        filter.doFilter(request, response, chain);
 
         assertNull(request.getSession(false));
     }
@@ -431,7 +337,7 @@ public class AbstractAuthenticationProcessingFilterTests {
      */
     @Test
     public void testLoginErrorWithNoFailureUrlSendsUnauthorizedStatus() throws Exception {
-        MockHttpServletRequest request = createMockRequest();
+        MockHttpServletRequest request = createMockAuthenticationRequest();
 
         MockFilterConfig config = new MockFilterConfig(null, null);
         MockFilterChain chain = new MockFilterChain(true);
@@ -441,57 +347,11 @@ public class AbstractAuthenticationProcessingFilterTests {
         successHandler.setDefaultTargetUrl("http://monkeymachine.co.uk/");
         filter.setAuthenticationSuccessHandler(successHandler);
 
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
+        filter.doFilter(request, response, chain);
 
         assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus());
     }
 
-    /**
-     * SEC-462
-     */
-    @Test
-    public void testServerSideRedirectForwardsToFailureUrl() throws Exception {
-        MockHttpServletRequest request = createMockRequest();
-
-        MockFilterConfig config = new MockFilterConfig(null, null);
-        MockFilterChain chain = new MockFilterChain(true);
-        MockHttpServletResponse response = new MockHttpServletResponse();
-
-        MockAuthenticationFilter filter = new MockAuthenticationFilter(false);
-        successHandler.setDefaultTargetUrl("http://monkeymachine.co.uk/");
-        filter.setAuthenticationSuccessHandler(successHandler);
-        filter.setAuthenticationFailureHandler(failureHandler);
-        failureHandler.setUseForward(true);
-        failureHandler.setDefaultFailureUrl("/error");
-
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
-
-        assertEquals("/error", response.getForwardedUrl());
-    }
-
-    /**
-     * SEC-213
-     */
-    @Test
-    public void testTargetUrlParameterIsUsedIfPresent() throws Exception {
-        MockHttpServletRequest request = createMockRequest();
-        request.setParameter("targetUrl", "/target");
-
-        MockFilterConfig config = new MockFilterConfig(null, null);
-        MockFilterChain chain = new MockFilterChain(true);
-        MockHttpServletResponse response = new MockHttpServletResponse();
-
-        MockAuthenticationFilter filter = new MockAuthenticationFilter(true);
-        filter.setAuthenticationSuccessHandler(successHandler);
-        successHandler.setDefaultTargetUrl("http://monkeymachine.co.uk/");
-        successHandler.setTargetUrlParameter("targetUrl");
-        filter.setAuthenticationFailureHandler(failureHandler);
-
-        executeFilterInContainerSimulator(config, filter, request, response, chain);
-
-        assertEquals("/mycontext/target", response.getRedirectedUrl());
-    }
-
 
     //~ Inner Classes ==================================================================================================
 
@@ -506,13 +366,6 @@ public class AbstractAuthenticationProcessingFilterTests {
             this.exceptionToThrow = new BadCredentialsException("Mock requested to do so");
         }
 
-        public MockAuthenticationFilter(AuthenticationException exceptionToThrow) {
-            this();
-            setRememberMeServices(new NullRememberMeServices());
-            this.grantAccess = false;
-            this.exceptionToThrow = exceptionToThrow;
-        }
-
         private MockAuthenticationFilter() {
             super("/j_mock_post");
         }

web/src/test/java/org/springframework/security/web/authentication/ExceptionMappingAuthenticationFailureHandlerTests.java

@@ -0,0 +1,11 @@
+package org.springframework.security.web.authentication;
+
+/**
+ * Created by IntelliJ IDEA.
+ * User: luke
+ * Date: Aug 8, 2010
+ * Time: 5:13:18 PM
+ * To change this template use File | Settings | File Templates.
+ */
+public class ExceptionMappingAuthenticationFailureHandlerTests {
+}

web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationFailureHandlerTests.java

@@ -55,6 +55,7 @@ public class SimpleUrlAuthenticationFailureHandlerTests {
         assertNull(request.getSession(false));
     }
 
+    // SEC-462
     @Test
     public void responseIsForwardedIfUseForwardIsTrue() throws Exception {
         SimpleUrlAuthenticationFailureHandler afh = new SimpleUrlAuthenticationFailureHandler("/target");

web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationSuccessHandlerTests.java

@@ -1,6 +1,7 @@
 package org.springframework.security.web.authentication;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.*;
 
 import org.junit.Test;
@@ -13,6 +14,17 @@ import org.springframework.security.core.Authentication;
  * @author Luke Taylor
  */
 public class SimpleUrlAuthenticationSuccessHandlerTests {
+    @Test
+    public void defaultTargetUrlIsUsedIfNoOtherInformationSet() throws Exception {
+        SimpleUrlAuthenticationSuccessHandler ash = new SimpleUrlAuthenticationSuccessHandler();
+
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+
+        ash.onAuthenticationSuccess(request, response, mock(Authentication.class));
+
+        assertEquals("/", response.getRedirectedUrl());
+    }
 
     // SEC-1428
     @Test
@@ -26,4 +38,36 @@ public class SimpleUrlAuthenticationSuccessHandlerTests {
         assertNull(response.getRedirectedUrl());
     }
 
+    /**
+     * SEC-213
+     */
+    @Test
+    public void targetUrlParameterIsUsedIfPresent() throws Exception {
+        SimpleUrlAuthenticationSuccessHandler ash = new SimpleUrlAuthenticationSuccessHandler("/defaultTarget");
+        ash.setTargetUrlParameter("targetUrl");
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+
+        request.setParameter("targetUrl", "/target");
+
+        ash.onAuthenticationSuccess(request, response, mock(Authentication.class));
+
+        assertEquals("/target", response.getRedirectedUrl());
+    }
+
+    /**
+     * SEC-297 fix.
+     */
+    @Test
+    public void absoluteDefaultTargetUrlDoesNotHaveContextPathPrepended() throws Exception {
+        SimpleUrlAuthenticationSuccessHandler ash = new SimpleUrlAuthenticationSuccessHandler();
+        ash.setDefaultTargetUrl("https://monkeymachine.co.uk/");
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+
+        ash.onAuthenticationSuccess(request, response, mock(Authentication.class));
+
+        assertEquals("https://monkeymachine.co.uk/", response.getRedirectedUrl());
+    }
+
 }

web/src/test/java/org/springframework/security/web/savedrequest/HttpSessionRequestCacheTests.java

@@ -7,7 +7,6 @@ import javax.servlet.http.HttpServletRequest;
 import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
-import org.springframework.security.web.WebAttributes;
 import org.springframework.security.web.util.RequestMatcher;
 
 /**
@@ -24,7 +23,7 @@ public class HttpSessionRequestCacheTests {
         MockHttpServletRequest request = new MockHttpServletRequest("GET", "/destination");
         MockHttpServletResponse response = new MockHttpServletResponse();
         cache.saveRequest(request, response);
-        assertNotNull(request.getSession().getAttribute(WebAttributes.SAVED_REQUEST));
+        assertNotNull(request.getSession().getAttribute(HttpSessionRequestCache.SAVED_REQUEST));
         assertNotNull(cache.getRequest(request, response));
 
         MockHttpServletRequest newRequest = new MockHttpServletRequest("POST", "/destination");

web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java

@@ -6,7 +6,6 @@ import org.junit.Test;
 import org.springframework.mock.web.MockFilterChain;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
-import org.springframework.security.web.WebAttributes;
 
 public class RequestCacheAwareFilterTests {
 
@@ -18,9 +17,9 @@ public class RequestCacheAwareFilterTests {
         MockHttpServletRequest request = new MockHttpServletRequest("POST", "/destination");
         MockHttpServletResponse response = new MockHttpServletResponse();
         cache.saveRequest(request, response);
-        assertNotNull(request.getSession().getAttribute(WebAttributes.SAVED_REQUEST));
+        assertNotNull(request.getSession().getAttribute(HttpSessionRequestCache.SAVED_REQUEST));
 
         filter.doFilter(request, response, new MockFilterChain());
-        assertNull(request.getSession().getAttribute(WebAttributes.SAVED_REQUEST));
+        assertNull(request.getSession().getAttribute(HttpSessionRequestCache.SAVED_REQUEST));
     }
 }

web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java

@@ -10,8 +10,8 @@ import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.security.core.Authentication;
-import org.springframework.security.web.WebAttributes;
 import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy;
+import org.springframework.security.web.savedrequest.HttpSessionRequestCache;
 
 /**
  *
@@ -48,12 +48,12 @@ public class DefaultSessionAuthenticationStrategyTests {
         HttpServletRequest request = new MockHttpServletRequest();
         HttpSession session = request.getSession();
         session.setAttribute("blah", "blah");
-        session.setAttribute(WebAttributes.SAVED_REQUEST, "DefaultSavedRequest");
+        session.setAttribute("SPRING_SECURITY_SAVED_REQUEST_KEY", "DefaultSavedRequest");
 
         strategy.onAuthentication(mock(Authentication.class), request, new MockHttpServletResponse());
 
         assertNull(request.getSession().getAttribute("blah"));
-        assertNotNull(request.getSession().getAttribute(WebAttributes.SAVED_REQUEST));
+        assertNotNull(request.getSession().getAttribute("SPRING_SECURITY_SAVED_REQUEST_KEY"));
     }
 
     @Test