diff --git a/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java b/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java index e0884555e8..5d67feb6a2 100644 --- a/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java +++ b/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java @@ -68,7 +68,7 @@ import javax.servlet.http.HttpSession; * *

* To configure this filter to redirect to specific pages as the result of @@ -132,8 +132,6 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl ApplicationEventPublisherAware, MessageSourceAware { //~ Static fields/initializers ===================================================================================== - public static final String SPRING_SECURITY_SAVED_REQUEST_KEY = "SPRING_SECURITY_SAVED_REQUEST_KEY"; - public static final String SPRING_SECURITY_LAST_EXCEPTION_KEY = "SPRING_SECURITY_LAST_EXCEPTION"; //~ Instance fields ================================================================================================ @@ -160,8 +158,8 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl private String authenticationFailureUrl; /** - * Where to redirect the browser to if authentication is successful but - * SPRING_SECURITY_SAVED_REQUEST_KEY is null + * Where to redirect the browser to if authentication is successful and no saved request is stored + * in the session. */ private String defaultTargetUrl; @@ -278,24 +276,6 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl chain.doFilter(request, response); } - public static String obtainFullSavedRequestUrl(HttpServletRequest request) { - SavedRequest savedRequest = getSavedRequest(request); - - return savedRequest == null ? null : savedRequest.getFullRequestUrl(); - } - - private static SavedRequest getSavedRequest(HttpServletRequest request) { - HttpSession session = request.getSession(false); - - if (session == null) { - return null; - } - - SavedRequest savedRequest = (SavedRequest) session.getAttribute(SPRING_SECURITY_SAVED_REQUEST_KEY); - - return savedRequest; - } - protected void onPreAuthentication(HttpServletRequest request, HttpServletResponse response) throws AuthenticationException, IOException { } @@ -389,7 +369,7 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl protected String determineTargetUrl(HttpServletRequest request) { // Don't attempt to obtain the url from the saved request if alwaysUsedefaultTargetUrl is set String targetUrl = alwaysUseDefaultTargetUrl ? null : - targetUrlResolver.determineTargetUrl(getSavedRequest(request), request, SecurityContextHolder.getContext().getAuthentication()); + targetUrlResolver.determineTargetUrl(request, SecurityContextHolder.getContext().getAuthentication()); if (targetUrl == null) { targetUrl = getDefaultTargetUrl(); 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 90f26b30c9..1651fb53a3 100644 --- a/core/src/main/java/org/springframework/security/ui/ExceptionTranslationFilter.java +++ b/core/src/main/java/org/springframework/security/ui/ExceptionTranslationFilter.java @@ -199,7 +199,7 @@ public class ExceptionTranslationFilter extends SpringSecurityFilter implements if (createSessionAllowed) { // Store the HTTP request itself. Used by AbstractProcessingFilter // for redirection after successful authentication (SEC-29) - httpRequest.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest); + httpRequest.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest); } // SEC-112: Clear the SecurityContextHolder's Authentication, as the diff --git a/core/src/main/java/org/springframework/security/ui/TargetUrlResolver.java b/core/src/main/java/org/springframework/security/ui/TargetUrlResolver.java index 7cd95d0461..30e9bfad29 100644 --- a/core/src/main/java/org/springframework/security/ui/TargetUrlResolver.java +++ b/core/src/main/java/org/springframework/security/ui/TargetUrlResolver.java @@ -18,7 +18,6 @@ package org.springframework.security.ui; import javax.servlet.http.HttpServletRequest; import org.springframework.security.Authentication; -import org.springframework.security.ui.savedrequest.SavedRequest; /** * Used by {@link AbstractProcessingFilter} to determine target URL in case of @@ -32,11 +31,10 @@ import org.springframework.security.ui.savedrequest.SavedRequest; public interface TargetUrlResolver { /** - * @param savedRequest The request that initiated the authentication process * @param currentRequest the current request * @param auth The authentication token generated after successful authentication * @return The URL to be used */ - public String determineTargetUrl(SavedRequest savedRequest, HttpServletRequest currentRequest, Authentication auth); + public String determineTargetUrl(HttpServletRequest currentRequest, Authentication auth); } diff --git a/core/src/main/java/org/springframework/security/ui/TargetUrlResolverImpl.java b/core/src/main/java/org/springframework/security/ui/TargetUrlResolverImpl.java index 136b9a16fd..5ddc995563 100644 --- a/core/src/main/java/org/springframework/security/ui/TargetUrlResolverImpl.java +++ b/core/src/main/java/org/springframework/security/ui/TargetUrlResolverImpl.java @@ -19,6 +19,7 @@ import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; import org.springframework.security.Authentication; import org.springframework.security.ui.savedrequest.SavedRequest; @@ -29,9 +30,9 @@ import org.springframework.util.StringUtils; /** * Default implementation for {@link TargetUrlResolver} *

- * Returns a target URL based from the contents of the configured targetUrlParameter if present on - * the current request. Failing that, the SavedRequest in the session will be used. - * + * Returns a target URL based from the contents of the configured targetUrlParameter if present on + * the current request. Failing that, the SavedRequest in the session will be used. + * * @author Martino Piccinato * @author Luke Taylor * @version $Id$ @@ -40,26 +41,22 @@ import org.springframework.util.StringUtils; */ public class TargetUrlResolverImpl implements TargetUrlResolver { public static String DEFAULT_TARGET_PARAMETER = "spring-security-redirect"; - + /* SEC-213 */ private String targetUrlParameter = DEFAULT_TARGET_PARAMETER; - - /** - * If true, will only use SavedRequest to determine the target URL on successful + + /** + * 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 boolean justUseSavedRequestOnGet = false; - /* (non-Javadoc) - * @see org.acegisecurity.ui.TargetUrlResolver#determineTargetUrl(org.acegisecurity.ui.savedrequest.SavedRequest, javax.servlet.http.HttpServletRequest, org.acegisecurity.Authentication) - */ - public String determineTargetUrl(SavedRequest savedRequest, HttpServletRequest currentRequest, - Authentication auth) { + public String determineTargetUrl(HttpServletRequest currentRequest, Authentication auth) { String targetUrl = currentRequest.getParameter(targetUrlParameter); - + if (StringUtils.hasText(targetUrl)) { try { return URLDecoder.decode(targetUrl, "UTF-8"); @@ -68,6 +65,8 @@ public class TargetUrlResolverImpl implements TargetUrlResolver { } } + SavedRequest savedRequest = getSavedRequest(currentRequest); + if (savedRequest != null) { if (!justUseSavedRequestOnGet || savedRequest.getMethod().equals("GET")) { targetUrl = savedRequest.getFullRequestUrl(); @@ -75,35 +74,47 @@ public class TargetUrlResolverImpl implements TargetUrlResolver { } return targetUrl; - } + } - /** - * @return true if just GET request will be used - * to determine target URLs, false otherwise. - */ - protected boolean isJustUseSavedRequestOnGet() { - return justUseSavedRequestOnGet; - } + private static SavedRequest getSavedRequest(HttpServletRequest request) { + HttpSession session = request.getSession(false); - /** - * @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; - } + if (session == null) { + return null; + } - - /** - * 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"); + SavedRequest savedRequest = (SavedRequest) session.getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY); + + return savedRequest; + } + + /** + * @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; } } diff --git a/core/src/main/java/org/springframework/security/ui/savedrequest/SavedRequest.java b/core/src/main/java/org/springframework/security/ui/savedrequest/SavedRequest.java index 3c2f5a8491..5f001cc92b 100644 --- a/core/src/main/java/org/springframework/security/ui/savedrequest/SavedRequest.java +++ b/core/src/main/java/org/springframework/security/ui/savedrequest/SavedRequest.java @@ -69,6 +69,8 @@ public class SavedRequest implements java.io.Serializable { private String servletPath; private int serverPort; + public static final String SPRING_SECURITY_SAVED_REQUEST_KEY = "SPRING_SECURITY_SAVED_REQUEST_KEY"; + //~ Constructors =================================================================================================== @SuppressWarnings("unchecked") diff --git a/core/src/main/java/org/springframework/security/wrapper/SavedRequestAwareWrapper.java b/core/src/main/java/org/springframework/security/wrapper/SavedRequestAwareWrapper.java index fce84e952e..2255477290 100644 --- a/core/src/main/java/org/springframework/security/wrapper/SavedRequestAwareWrapper.java +++ b/core/src/main/java/org/springframework/security/wrapper/SavedRequestAwareWrapper.java @@ -34,7 +34,6 @@ import javax.servlet.http.HttpSession; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.security.ui.AbstractProcessingFilter; import org.springframework.security.ui.savedrequest.Enumerator; import org.springframework.security.ui.savedrequest.FastHttpDateFormat; import org.springframework.security.ui.savedrequest.SavedRequest; @@ -91,7 +90,7 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW return; } - SavedRequest saved = (SavedRequest) session.getAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY); + SavedRequest saved = (SavedRequest) session.getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY); if ((saved != null) && saved.doesRequestMatch(request, portResolver)) { if (logger.isDebugEnabled()) { @@ -99,7 +98,7 @@ public class SavedRequestAwareWrapper extends SecurityContextHolderAwareRequestW } savedRequest = saved; - session.removeAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY); + session.removeAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY); formats[0] = new SimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US); formats[1] = new SimpleDateFormat("EEEEEE, dd-MMM-yy HH:mm:ss zzz", Locale.US); 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 cd77adc74d..ebc85bc878 100644 --- a/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java @@ -352,7 +352,7 @@ public class AbstractProcessingFilterTests extends TestCase { throws Exception { // Setup our HTTP request MockHttpServletRequest request = createMockRequest(); - request.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl()); + request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl()); // Setup our filter configuration MockFilterConfig config = new MockFilterConfig(null, null); @@ -378,7 +378,7 @@ public class AbstractProcessingFilterTests extends TestCase { public void testSuccessfulAuthenticationCausesRedirectToSessionSpecifiedUrl() throws Exception { // Setup our HTTP request MockHttpServletRequest request = createMockRequest(); - request.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl()); + request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makeSavedRequestForUrl()); // Setup our filter configuration MockFilterConfig config = new MockFilterConfig(null, null); @@ -400,7 +400,7 @@ public class AbstractProcessingFilterTests extends TestCase { public void testSuccessfulAuthenticationCausesRedirectToDefaultTargetUrlOnPOSTSavedRequest() throws Exception { // Setup our HTTP request with a POST method request MockHttpServletRequest request = createMockRequest(); - request.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, makePostSavedRequestForUrl()); + request.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, makePostSavedRequestForUrl()); // Setup our filter configuration MockFilterConfig config = new MockFilterConfig(null, null); 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 fa46b08021..a6188716c8 100644 --- a/core/src/test/java/org/springframework/security/ui/ExceptionTranslationFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/ExceptionTranslationFilterTests.java @@ -27,6 +27,7 @@ import org.springframework.security.MockPortResolver; import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.providers.anonymous.AnonymousAuthenticationToken; +import org.springframework.security.ui.savedrequest.SavedRequest; import org.springframework.security.util.AuthorityUtils; import org.springframework.mock.web.MockHttpServletRequest; @@ -38,6 +39,8 @@ import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; /** * Tests {@link ExceptionTranslationFilter}. @@ -54,6 +57,18 @@ public class ExceptionTranslationFilterTests extends TestCase { SecurityContextHolder.clearContext(); } + private static String getSavedRequestUrl(HttpServletRequest request) { + HttpSession session = request.getSession(false); + + if (session == null) { + return null; + } + + SavedRequest savedRequest = (SavedRequest) session.getAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY); + + return savedRequest.getFullRequestUrl(); + } + public void testAccessDeniedWhenAnonymous() throws Exception { // Setup our HTTP request MockHttpServletRequest request = new MockHttpServletRequest(); @@ -79,8 +94,7 @@ public class ExceptionTranslationFilterTests extends TestCase { MockHttpServletResponse response = new MockHttpServletResponse(); filter.doFilter(request, response, chain); assertEquals("/mycontext/login.jsp", response.getRedirectedUrl()); - assertEquals("http://www.example.com/mycontext/secure/page.html", AbstractProcessingFilter - .obtainFullSavedRequestUrl(request)); + assertEquals("http://www.example.com/mycontext/secure/page.html", getSavedRequestUrl(request)); } public void testAccessDeniedWhenNonAnonymous() throws Exception { @@ -148,8 +162,7 @@ public class ExceptionTranslationFilterTests extends TestCase { MockHttpServletResponse response = new MockHttpServletResponse(); filter.doFilter(request, response, chain); assertEquals("/mycontext/login.jsp", response.getRedirectedUrl()); - assertEquals("http://www.example.com/mycontext/secure/page.html", AbstractProcessingFilter - .obtainFullSavedRequestUrl(request)); + assertEquals("http://www.example.com/mycontext/secure/page.html", getSavedRequestUrl(request)); } public void testRedirectedToLoginFormAndSessionShowsOriginalTargetWithExoticPortWhenAuthenticationException() @@ -180,8 +193,7 @@ public class ExceptionTranslationFilterTests extends TestCase { MockHttpServletResponse response = new MockHttpServletResponse(); filter.doFilter(request, response, chain); assertEquals("/mycontext/login.jsp", response.getRedirectedUrl()); - assertEquals("http://www.example.com:8080/mycontext/secure/page.html", AbstractProcessingFilter - .obtainFullSavedRequestUrl(request)); + assertEquals("http://www.example.com:8080/mycontext/secure/page.html", getSavedRequestUrl(request)); } public void testStartupDetectsMissingAuthenticationEntryPoint() throws Exception { diff --git a/core/src/test/java/org/springframework/security/wrapper/SavedRequestAwareWrapperTests.java b/core/src/test/java/org/springframework/security/wrapper/SavedRequestAwareWrapperTests.java index 8b0a1ac5e7..63384fe762 100644 --- a/core/src/test/java/org/springframework/security/wrapper/SavedRequestAwareWrapperTests.java +++ b/core/src/test/java/org/springframework/security/wrapper/SavedRequestAwareWrapperTests.java @@ -11,7 +11,6 @@ import javax.servlet.http.Cookie; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; -import org.springframework.security.ui.AbstractProcessingFilter; import org.springframework.security.ui.savedrequest.FastHttpDateFormat; import org.springframework.security.ui.savedrequest.SavedRequest; import org.springframework.security.util.PortResolverImpl; @@ -21,7 +20,7 @@ public class SavedRequestAwareWrapperTests { private SavedRequestAwareWrapper createWrapper(MockHttpServletRequest requestToSave, MockHttpServletRequest requestToWrap) { if (requestToSave != null) { SavedRequest savedRequest = new SavedRequest(requestToSave, new PortResolverImpl()); - requestToWrap.getSession().setAttribute(AbstractProcessingFilter.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest); + requestToWrap.getSession().setAttribute(SavedRequest.SPRING_SECURITY_SAVED_REQUEST_KEY, savedRequest); } return new SavedRequestAwareWrapper(requestToWrap, new PortResolverImpl(),"ROLE_"); } diff --git a/core/src/test/resources/log4j.properties b/core/src/test/resources/log4j.properties index d7d40296e6..a6fd809a78 100644 --- a/core/src/test/resources/log4j.properties +++ b/core/src/test/resources/log4j.properties @@ -6,9 +6,9 @@ log4j.rootLogger=INFO, stdout log4j.appender.stdout=org.apache.log4j.ConsoleAppender log4j.appender.stdout.layout=org.apache.log4j.PatternLayout -log4j.appender.stdout.layout.ConversionPattern=%d %p %c{1} - %m%n +log4j.appender.stdout.layout.ConversionPattern=%p %c{1} - %m%n -log4j.logger.org.springframework.security=DEBUG,stdout +log4j.logger.org.springframework.security=DEBUG log4j.logger.org.springframework.ldap=DEBUG -log4j.logger.org.apache.directory=ERROR,stdout +log4j.logger.org.apache.directory=ERROR