diff --git a/web/src/main/java/org/springframework/security/web/WebAttributes.java b/web/src/main/java/org/springframework/security/web/WebAttributes.java index 3072e7ba57..aff2008f5e 100644 --- a/web/src/main/java/org/springframework/security/web/WebAttributes.java +++ b/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"; } diff --git a/web/src/main/java/org/springframework/security/web/access/AccessDeniedHandlerImpl.java b/web/src/main/java/org/springframework/security/web/access/AccessDeniedHandlerImpl.java index b77c250592..7395a68a0c 100644 --- a/web/src/main/java/org/springframework/security/web/access/AccessDeniedHandlerImpl.java +++ b/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 ================================================================================================ diff --git a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java b/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java index 9beb1ce0b6..4095d4940f 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/session/SessionFixationProtectionStrategy.java +++ b/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}. *

* 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 migrateSessionAttributes to - * false). + * session-fixation protection attacks), and copies their session attributes across to the new session + * (can be disabled by setting {@code migrateSessionAttributes} to {@code false}). *

* 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()}. *

- * If concurrent session control is in use, then a SessionRegistry must be injected. + * If concurrent session control is in use, then a {@code SessionRegistry} must be injected. *

- *

Issues with HttpSessionBindingListener

+ *

Issues with {@code HttpSessionBindingListener}

*

* 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 not be discarded. */ - private List retainedAttributes = Arrays.asList(WebAttributes.SAVED_REQUEST); + private List retainedAttributes = null; /** - * If set to true, a session will always be created, even if one didn't exist at the start of the request. - * Defaults to false. + * 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. *

* 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 migrateSessionAttributes 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. *

- * If there is no session, no action is taken unless the alwaysCreateSession 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 attributesToMigrate = createMigratedAttributeMap(session); + Map 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 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. + *

+ * 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 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 attributes, HttpSession newSession) { + if (attributes != null) { + for (Map.Entry entry : attributes.entrySet()) { + newSession.setAttribute(entry.getKey(), entry.getValue()); + } + } + } + @SuppressWarnings("unchecked") private HashMap createMigratedAttributeMap(HttpSession session) { HashMap attributesToMigrate = null; - if (migrateSessionAttributes) { + if (migrateSessionAttributes || retainedAttributes == null) { attributesToMigrate = new HashMap(); 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. + *

+ * 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 retainedAttributes) { + logger.warn("Retained attributes is deprecated. Override the extractAttributes() method instead."); + Assert.notNull(retainedAttributes); this.retainedAttributes = retainedAttributes; } diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java b/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java index df4c922527..6e4878e671 100644 --- a/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java +++ b/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"; diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/HttpSessionRequestCache.java b/web/src/main/java/org/springframework/security/web/savedrequest/HttpSessionRequestCache.java index e8f1e244ad..1a2b15d2af 100644 --- a/web/src/main/java/org/springframework/security/web/savedrequest/HttpSessionRequestCache.java +++ b/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); } } diff --git a/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java b/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java index db86dd86c7..13501d9112 100644 --- a/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java +++ b/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 diff --git a/web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java b/web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java index 5591470b37..693a24d1cb 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilterTests.java +++ b/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()); - } + filter.doFilter(request, response, chain); - /** - * SEC-297 fix. - */ - @Test - public void testFullDefaultTargetUrlDoesNotHaveContextPathPrepended() throws Exception { - MockHttpServletRequest request = createMockRequest(); - MockFilterConfig config = new MockFilterConfig(null, null); + verify(failureHandler).onAuthenticationFailure(any(HttpServletRequest.class), any(HttpServletResponse.class), + any(AuthenticationException.class)); - MockFilterChain chain = new MockFilterChain(true); - MockHttpServletResponse response = new MockHttpServletResponse(); - - // Setup our test object, to grant access - MockAuthenticationFilter filter = new MockAuthenticationFilter(true); - successHandler.setDefaultTargetUrl("https://monkeymachine.co.uk/"); - successHandler.setAlwaysUseDefaultTargetUrl(true); - filter.setAuthenticationSuccessHandler(successHandler); - - 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"); } diff --git a/web/src/test/java/org/springframework/security/web/authentication/ExceptionMappingAuthenticationFailureHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/ExceptionMappingAuthenticationFailureHandlerTests.java new file mode 100644 index 0000000000..14419ebb98 --- /dev/null +++ b/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 { +} diff --git a/web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationFailureHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationFailureHandlerTests.java index 95ab3c211a..d46ddc19b2 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationFailureHandlerTests.java +++ b/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"); diff --git a/web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationSuccessHandlerTests.java b/web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationSuccessHandlerTests.java index c9b6bfbae2..b7e1cfa572 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/SimpleUrlAuthenticationSuccessHandlerTests.java +++ b/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()); + } + } diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/HttpSessionRequestCacheTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/HttpSessionRequestCacheTests.java index 1ab94861aa..5572d2ba44 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/HttpSessionRequestCacheTests.java +++ b/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"); diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java index 89502cdefc..f7faa7dd43 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/RequestCacheAwareFilterTests.java +++ b/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)); } } diff --git a/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java b/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java index 59e7bb1136..63ccc73176 100644 --- a/web/src/test/java/org/springframework/security/web/session/DefaultSessionAuthenticationStrategyTests.java +++ b/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