From c3181d9db084275c31ae4064414374277ec34e18 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Mon, 15 Dec 2008 02:48:32 +0000 Subject: [PATCH] SEC-1063: Moved the justUseSavedRequestOnGet property to ExceptionTranslationFilter. If set, it will not store the SavedRequest for unless the request is a GET. --- .../ui/ExceptionTranslationFilter.java | 41 +++-- ...uestAwareAuthenticationSuccessHandler.java | 156 ++---------------- ...SimpleUrlAuthenticationSuccessHandler.java | 117 +++++++++++++ .../ui/AbstractProcessingFilterTests.java | 27 --- .../ui/ExceptionTranslationFilterTests.java | 12 ++ 5 files changed, 168 insertions(+), 185 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/ui/SimpleUrlAuthenticationSuccessHandler.java diff --git a/core/src/main/java/org/springframework/security/ui/ExceptionTranslationFilter.java b/core/src/main/java/org/springframework/security/ui/ExceptionTranslationFilter.java index 1651fb53a3..dc60aa1d7f 100644 --- a/core/src/main/java/org/springframework/security/ui/ExceptionTranslationFilter.java +++ b/core/src/main/java/org/springframework/security/ui/ExceptionTranslationFilter.java @@ -78,6 +78,7 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements private PortResolver portResolver = new PortResolverImpl(); private ThrowableAnalyzer throwableAnalyzer = new DefaultThrowableAnalyzer(); private boolean createSessionAllowed = true; + private boolean justUseSavedRequestOnGet; //~ Methods ======================================================================================================== @@ -169,7 +170,7 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements /** * If true, indicates that ExceptionTranslationFilter is permitted to store the target - * URL and exception information in the HttpSession (the default). + * URL and exception information in a new HttpSession (the default). * In situations where you do not wish to unnecessarily create HttpSessions - because the user agent * will know the failed URL, such as with BASIC or Digest authentication - you may wish to set this property to * false. @@ -188,25 +189,25 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements protected void sendStartAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain, AuthenticationException reason) throws ServletException, IOException { - HttpServletRequest httpRequest = (HttpServletRequest) request; - - SavedRequest savedRequest = new SavedRequest(httpRequest, portResolver); - - if (logger.isDebugEnabled()) { - logger.debug("Authentication entry point being called; SavedRequest added to Session: " + savedRequest); - } - - if (createSessionAllowed) { - // Store the HTTP request itself. Used by AbstractProcessingFilter - // for redirection after successful authentication (SEC-29) - httpRequest.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest); - } - // SEC-112: Clear the SecurityContextHolder's Authentication, as the // existing Authentication is no longer considered valid SecurityContextHolder.getContext().setAuthentication(null); + saveRequestIfAllowed(request); + logger.debug("Calling Authentication entry point."); + authenticationEntryPoint.commence(request, response, reason); + } - authenticationEntryPoint.commence(httpRequest, response, reason); + private void saveRequestIfAllowed(HttpServletRequest request) { + if (!justUseSavedRequestOnGet || "GET".equals(request.getMethod())) { + SavedRequest savedRequest = new SavedRequest(request, portResolver); + + if (createSessionAllowed || request.getSession(false) != null) { + // Store the HTTP request itself. Used by AbstractProcessingFilter + // for redirection after successful authentication (SEC-29) + request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest); + logger.debug("SavedRequest added to Session: " + savedRequest); + } + } } public void setAccessDeniedHandler(AccessDeniedHandler accessDeniedHandler) { @@ -234,6 +235,14 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements this.throwableAnalyzer = throwableAnalyzer; } + /** + * If true, will only use SavedRequest to determine the target URL on successful + * authentication if the request that caused the authentication request was a GET. Defaults to false. + */ + public void setJustUseSavedRequestOnGet(boolean justUseSavedRequestOnGet) { + this.justUseSavedRequestOnGet = justUseSavedRequestOnGet; + } + public int getOrder() { return FilterChainOrder.EXCEPTION_TRANSLATION_FILTER; } diff --git a/core/src/main/java/org/springframework/security/ui/SavedRequestAwareAuthenticationSuccessHandler.java b/core/src/main/java/org/springframework/security/ui/SavedRequestAwareAuthenticationSuccessHandler.java index 384361b3ef..419b170cc5 100644 --- a/core/src/main/java/org/springframework/security/ui/SavedRequestAwareAuthenticationSuccessHandler.java +++ b/core/src/main/java/org/springframework/security/ui/SavedRequestAwareAuthenticationSuccessHandler.java @@ -9,14 +9,10 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.springframework.security.Authentication; import org.springframework.security.ui.savedrequest.SavedRequest; import org.springframework.security.util.RedirectUtils; -import org.springframework.security.util.UrlUtils; import org.springframework.security.wrapper.SavedRequestAwareWrapper; -import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** @@ -49,80 +45,30 @@ import org.springframework.util.StringUtils; * @version $Id$ * @since 2.5 */ -public class SavedRequestAwareAuthenticationSuccessHandler implements AuthenticationSuccessHandler { - public static String DEFAULT_TARGET_PARAMETER = "spring-security-redirect"; - - protected final Log logger = LogFactory.getLog(this.getClass()); - - /* SEC-213 */ - private String targetUrlParameter = DEFAULT_TARGET_PARAMETER; - - /** - * If true, will only use SavedRequest to determine the target URL on successful - * authentication if the request that caused the authentication request was a GET. - * It will then return null for a POST/PUT request. - * Defaults to false. - */ - private boolean justUseSavedRequestOnGet = false; - - private String defaultTargetUrl = "/"; - - /** - * If true, will always redirect to the value of - * {@link #getDefaultTargetUrl} upon successful authentication, irrespective - * of the page that caused the authentication request (defaults to - * false). - */ - private boolean alwaysUseDefaultTargetUrl = false; - - private boolean useRelativeContext = false; +public class SavedRequestAwareAuthenticationSuccessHandler extends SimpleUrlAuthenticationSuccessHandler { + @Override public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws ServletException, IOException { - - if (alwaysUseDefaultTargetUrl) { - removeSavedRequest(request); - RedirectUtils.sendRedirect(request, response, defaultTargetUrl, useRelativeContext); - return; - } - - // Check for the parameter and use that if available - String targetUrl = request.getParameter(targetUrlParameter); - - if (StringUtils.hasText(targetUrl)) { - try { - targetUrl = URLDecoder.decode(targetUrl, "UTF-8"); - } catch (UnsupportedEncodingException e) { - throw new IllegalStateException("UTF-8 not supported. Shouldn't be possible"); - } - - logger.debug("Found targetUrlParameter in request. Redirecting to: " + targetUrl); - - removeSavedRequest(request); - RedirectUtils.sendRedirect(request, response, targetUrl, useRelativeContext); - - return; - } - - // Try the SavedRequest URL SavedRequest savedRequest = getSavedRequest(request); - if (savedRequest != null) { - if (!justUseSavedRequestOnGet || savedRequest.getMethod().equals("GET")) { - targetUrl = savedRequest.getFullRequestUrl(); - logger.debug("Redirecting to SavedRequest Url: " + targetUrl); - } else { - removeSavedRequest(request); - } + if (savedRequest == null) { + super.onAuthenticationSuccess(request, response, authentication); + + return; } - if (targetUrl == null) { - targetUrl = defaultTargetUrl; - logger.debug("Redirecting to default Url: " + targetUrl); + if (isAlwaysUseDefaultTargetUrl() || StringUtils.hasText(request.getParameter(targetUrlParameter))) { + removeSavedRequest(request); + super.onAuthenticationSuccess(request, response, authentication); + + return; } + // Use the SavedRequest URL + String targetUrl = savedRequest.getFullRequestUrl(); + logger.debug("Redirecting to SavedRequest Url: " + targetUrl); RedirectUtils.sendRedirect(request, response, targetUrl, useRelativeContext); - } private SavedRequest getSavedRequest(HttpServletRequest request) { @@ -143,78 +89,4 @@ public class SavedRequestAwareAuthenticationSuccessHandler implements Authentica session.removeAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY); } } - - /** - * Supplies the default target Url that will be used if no saved request is found or the - * alwaysUseDefaultTargetUrl property is set to true. If not set, defaults to /. - * - * @return the defaultTargetUrl property - */ - protected String getDefaultTargetUrl() { - return defaultTargetUrl; - } - - /** - * Supplies the default target Url that will be used if no saved request is found in the session, or the - * alwaysUseDefaultTargetUrl property is set to true. If not set, defaults to /. It - * will be treated as relative to the web-app's context path, and should include the leading /. - * Alternatively, inclusion of a scheme name (such as "http://" or "https://") as the prefix will denote a - * fully-qualified URL and this is also supported. - * - * @param defaultTargetUrl - */ - public void setDefaultTargetUrl(String defaultTargetUrl) { - Assert.isTrue(UrlUtils.isValidRedirectUrl(defaultTargetUrl), - "defaultTarget must start with '/' or with 'http(s)'"); - this.defaultTargetUrl = defaultTargetUrl; - } - - /** - * If true, will always redirect to the value of defaultTargetUrl - * (defaults to false). - */ - public void setAlwaysUseDefaultTargetUrl(boolean alwaysUseDefaultTargetUrl) { - this.alwaysUseDefaultTargetUrl = alwaysUseDefaultTargetUrl; - } - - boolean isAlwaysUseDefaultTargetUrl() { - return alwaysUseDefaultTargetUrl; - } - - /** - * @return true if just GET request will be used - * to determine target URLs, false otherwise. - */ - protected boolean isJustUseSavedRequestOnGet() { - return justUseSavedRequestOnGet; - } - - /** - * @param justUseSavedRequestOnGet set to true if - * just GET request will be used to determine target URLs, - * false otherwise. - */ - public void setJustUseSavedRequestOnGet(boolean justUseSavedRequestOnGet) { - this.justUseSavedRequestOnGet = justUseSavedRequestOnGet; - } - - /** - * Before checking the SavedRequest, the current request will be checked for this parameter - * and the value used as the target URL if resent. - * - * @param targetUrlParameter the name of the parameter containing the encoded target URL. Defaults - * to "redirect". - */ - public void setTargetUrlParameter(String targetUrlParameter) { - Assert.hasText("targetUrlParamete canot be null or empty"); - this.targetUrlParameter = targetUrlParameter; - } - - /** - * If true, causes any redirection URLs to be calculated minus the protocol - * and context path (defaults to false). - */ - public void setUseRelativeContext(boolean useRelativeContext) { - this.useRelativeContext = useRelativeContext; - } } diff --git a/core/src/main/java/org/springframework/security/ui/SimpleUrlAuthenticationSuccessHandler.java b/core/src/main/java/org/springframework/security/ui/SimpleUrlAuthenticationSuccessHandler.java new file mode 100644 index 0000000000..bf26d46a8f --- /dev/null +++ b/core/src/main/java/org/springframework/security/ui/SimpleUrlAuthenticationSuccessHandler.java @@ -0,0 +1,117 @@ +package org.springframework.security.ui; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.security.Authentication; +import org.springframework.security.util.RedirectUtils; +import org.springframework.security.util.UrlUtils; +import org.springframework.util.Assert; +import org.springframework.util.StringUtils; + +public class SimpleUrlAuthenticationSuccessHandler implements AuthenticationSuccessHandler { + + public static String DEFAULT_TARGET_PARAMETER = "spring-security-redirect"; + protected final Log logger = LogFactory.getLog(this.getClass()); + protected String targetUrlParameter = DEFAULT_TARGET_PARAMETER; + protected String defaultTargetUrl = "/"; + protected boolean alwaysUseDefaultTargetUrl = false; + protected boolean useRelativeContext = false; + + public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, + Authentication authentication) throws IOException, ServletException { + + if (isAlwaysUseDefaultTargetUrl()) { + RedirectUtils.sendRedirect(request, response, getDefaultTargetUrl(), useRelativeContext); + return; + } + + // Check for the parameter and use that if available + String targetUrl = request.getParameter(targetUrlParameter); + + if (StringUtils.hasText(targetUrl)) { + try { + targetUrl = URLDecoder.decode(targetUrl, "UTF-8"); + } catch (UnsupportedEncodingException e) { + throw new IllegalStateException("UTF-8 not supported. Shouldn't be possible"); + } + + logger.debug("Found targetUrlParameter in request. Redirecting to: " + targetUrl); + + RedirectUtils.sendRedirect(request, response, targetUrl, useRelativeContext); + + return; + } + + if (targetUrl == null) { + targetUrl = getDefaultTargetUrl(); + logger.debug("Redirecting to default Url: " + targetUrl); + } + + RedirectUtils.sendRedirect(request, response, targetUrl, useRelativeContext); + } + + /** + * Supplies the default target Url that will be used if no saved request is found or the + * alwaysUseDefaultTargetUrl property is set to true. If not set, defaults to /. + * + * @return the defaultTargetUrl property + */ + protected String getDefaultTargetUrl() { + return defaultTargetUrl; + } + + /** + * Supplies the default target Url that will be used if no saved request is found in the session, or the + * alwaysUseDefaultTargetUrl property is set to true. If not set, defaults to /. It + * will be treated as relative to the web-app's context path, and should include the leading /. + * Alternatively, inclusion of a scheme name (such as "http://" or "https://") as the prefix will denote a + * fully-qualified URL and this is also supported. + * + * @param defaultTargetUrl + */ + public void setDefaultTargetUrl(String defaultTargetUrl) { + Assert.isTrue(UrlUtils.isValidRedirectUrl(defaultTargetUrl), + "defaultTarget must start with '/' or with 'http(s)'"); + this.defaultTargetUrl = defaultTargetUrl; + } + + /** + * If true, will always redirect to the value of defaultTargetUrl + * (defaults to false). + */ + public void setAlwaysUseDefaultTargetUrl(boolean alwaysUseDefaultTargetUrl) { + this.alwaysUseDefaultTargetUrl = alwaysUseDefaultTargetUrl; + } + + protected boolean isAlwaysUseDefaultTargetUrl() { + return alwaysUseDefaultTargetUrl; + } + + /** + * Before checking the SavedRequest, the current request will be checked for this parameter + * and the value used as the target URL if resent. + * + * @param targetUrlParameter the name of the parameter containing the encoded target URL. Defaults + * to "redirect". + */ + public void setTargetUrlParameter(String targetUrlParameter) { + Assert.hasText("targetUrlParamete canot be null or empty"); + this.targetUrlParameter = targetUrlParameter; + } + + /** + * If true, causes any redirection URLs to be calculated minus the protocol + * and context path (defaults to false). + */ + public void setUseRelativeContext(boolean useRelativeContext) { + this.useRelativeContext = useRelativeContext; + } +} diff --git a/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java b/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java index 0742a5dc6f..ba197205ab 100644 --- a/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java @@ -366,33 +366,6 @@ public class AbstractProcessingFilterTests extends TestCase { assertNotNull(SecurityContextHolder.getContext().getAuthentication()); } - public void testSuccessfulAuthenticationCausesRedirectToDefaultTargetUrlOnPOSTSavedRequest() throws Exception { - // Setup our HTTP request with a POST method request - MockHttpServletRequest request = createMockRequest(); - request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makePostSavedRequestForUrl()); - - // Setup our filter configuration - MockFilterConfig config = new MockFilterConfig(null, null); - - // Setup our expectation that the filter chain will be invoked, as we want to go to the location requested in the session - MockFilterChain chain = new MockFilterChain(true); - MockHttpServletResponse response = new MockHttpServletResponse(); - - // Setup our test object, to grant access - MockAbstractProcessingFilter filter = new MockAbstractProcessingFilter(true); - - filter.setFilterProcessesUrl("/j_mock_post"); - filter.setSuccessHandler(successHandler); - successHandler.setDefaultTargetUrl("/foobar"); - // Configure not to use POST SavedRequest - successHandler.setJustUseSavedRequestOnGet(true); - - // Test - executeFilterInContainerSimulator(config, filter, request, response, chain); - assertEquals("/mycontext/foobar", response.getRedirectedUrl()); - assertNotNull(SecurityContextHolder.getContext().getAuthentication()); - } - /** * SEC-297 fix. */ diff --git a/core/src/test/java/org/springframework/security/ui/ExceptionTranslationFilterTests.java b/core/src/test/java/org/springframework/security/ui/ExceptionTranslationFilterTests.java index a6188716c8..9d93d9e277 100644 --- a/core/src/test/java/org/springframework/security/ui/ExceptionTranslationFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/ExceptionTranslationFilterTests.java @@ -196,6 +196,18 @@ public class ExceptionTranslationFilterTests extends TestCase { assertEquals("http://www.example.com:8080/mycontext/secure/page.html", getSavedRequestUrl(request)); } + public void testSavedRequestIsNotStoredForPostIfJustUseSaveRequestOnGetIsSet() throws Exception { + ExceptionTranslationFilter filter = new ExceptionTranslationFilter(); + filter.setJustUseSavedRequestOnGet(true); + filter.setAuthenticationEntryPoint(new MockAuthenticationEntryPoint("/login.jsp")); + filter.setPortResolver(new MockPortResolver(8080, 8443)); + MockHttpServletRequest request = new MockHttpServletRequest(); + MockFilterChain chain = new MockFilterChain(false, true, false, false); + request.setMethod("POST"); + filter.doFilter(request, new MockHttpServletResponse(), chain); + assertTrue(request.getSession().getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY) == null); + } + public void testStartupDetectsMissingAuthenticationEntryPoint() throws Exception { ExceptionTranslationFilter filter = new ExceptionTranslationFilter();