From feadb3582a3c85fb83bf3f172700e51eb2261588 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Thu, 31 Jan 2008 16:05:25 +0000 Subject: [PATCH] SEC-516: TargetUrlResolver path to avoid redirecting to POST requests. --- .../security/ui/AbstractProcessingFilter.java | 37 +++++++-- .../security/ui/TargetUrlResolver.java | 42 ++++++++++ .../security/ui/TargetUrlResolverImpl.java | 76 +++++++++++++++++++ .../ui/AbstractProcessingFilterTests.java | 42 +++++++++- 4 files changed, 189 insertions(+), 8 deletions(-) create mode 100644 core/src/main/java/org/springframework/security/ui/TargetUrlResolver.java create mode 100644 core/src/main/java/org/springframework/security/ui/TargetUrlResolverImpl.java 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 9c511521de..c0cdd924a1 100644 --- a/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java +++ b/core/src/main/java/org/springframework/security/ui/AbstractProcessingFilter.java @@ -153,6 +153,8 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl private RememberMeServices rememberMeServices = new NullRememberMeServices(); + private TargetUrlResolver targetUrlResolver = new TargetUrlResolverImpl(); + /** Where to redirect the browser to if authentication fails */ private String authenticationFailureUrl; @@ -216,7 +218,8 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl Assert.hasLength(defaultTargetUrl, "defaultTargetUrl must be specified"); Assert.hasLength(authenticationFailureUrl, "authenticationFailureUrl must be specified"); Assert.notNull(authenticationManager, "authenticationManager must be specified"); - Assert.notNull(this.rememberMeServices); + Assert.notNull(rememberMeServices, "rememberMeServices cannot be null"); + Assert.notNull(targetUrlResolver, "targetUrlResolver cannot be null"); } /** @@ -266,6 +269,12 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl } 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) { @@ -274,9 +283,9 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl SavedRequest savedRequest = (SavedRequest) session.getAttribute(SPRING_SECURITY_SAVED_REQUEST_KEY); - return savedRequest == null ? null : savedRequest.getFullRequestUrl(); - } - + return savedRequest; + } + protected void onPreAuthentication(HttpServletRequest request, HttpServletResponse response) throws AuthenticationException, IOException { } @@ -416,9 +425,9 @@ 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 : obtainFullSavedRequestUrl(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()); if (targetUrl == null) { targetUrl = getDefaultTargetUrl(); @@ -578,4 +587,18 @@ public abstract class AbstractProcessingFilter extends SpringSecurityFilter impl public void setAllowSessionCreation(boolean allowSessionCreation) { this.allowSessionCreation = allowSessionCreation; } + + /** + * @return the targetUrlResolver + */ + protected TargetUrlResolver getTargetUrlResolver() { + return targetUrlResolver; + } + + /** + * @param targetUrlResolver the targetUrlResolver to set + */ + public void setTargetUrlResolver(TargetUrlResolver targetUrlResolver) { + this.targetUrlResolver = targetUrlResolver; + } } diff --git a/core/src/main/java/org/springframework/security/ui/TargetUrlResolver.java b/core/src/main/java/org/springframework/security/ui/TargetUrlResolver.java new file mode 100644 index 0000000000..1b12547df6 --- /dev/null +++ b/core/src/main/java/org/springframework/security/ui/TargetUrlResolver.java @@ -0,0 +1,42 @@ +/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +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 + * successfull authentication. + * + * @author Martino Piccinato + * @version $Id$ + * @since 2.0 + * + */ +public interface TargetUrlResolver { + + /** + * @param savedRequest The request that initiated the authentication process + * @param currentRequest the current request + * @param auth The authentication token generated after successfull authentication + * @return The URL to be used + */ + public String determineTargetUrl(SavedRequest savedRequest, 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 new file mode 100644 index 0000000000..176ee647cd --- /dev/null +++ b/core/src/main/java/org/springframework/security/ui/TargetUrlResolverImpl.java @@ -0,0 +1,76 @@ +/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.ui; + +import javax.servlet.http.HttpServletRequest; + +import org.springframework.security.Authentication; +import org.springframework.security.ui.savedrequest.SavedRequest; + +/** + * Default implementation for {@link TargetUrlResolver} + * + * @author Martino Piccinato + * @version $Id$ + * @since 2.0 + * + */ +public class TargetUrlResolverImpl implements TargetUrlResolver { + + /** + * 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 return null on POST/PUT request. + * In most cases it's meaningless to redirect to a Url generated by a POST/PUT request. + * Defaults to true. + */ + private boolean justUseSavedRequestOnGet = true; + + /* (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) { + + String targetUrl = null; + + if (savedRequest != null) { + if (!justUseSavedRequestOnGet || savedRequest.getMethod().equals("GET")) { + targetUrl = savedRequest.getFullRequestUrl(); + } + } + + return targetUrl; + } + + /** + * @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; + } + +} 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 8f34b2ed5d..c0ad502228 100644 --- a/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java +++ b/core/src/test/java/org/springframework/security/ui/AbstractProcessingFilterTests.java @@ -31,7 +31,6 @@ import org.springframework.security.util.PortResolverImpl; import org.springframework.mock.web.MockFilterConfig; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; -import org.springframework.mock.web.MockHttpSession; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -93,7 +92,18 @@ public class AbstractProcessingFilterTests extends TestCase { return new SavedRequest(request, new PortResolverImpl()); } + + private SavedRequest 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 SavedRequest(request, new PortResolverImpl()); + } + protected void setUp() throws Exception { super.setUp(); SecurityContextHolder.clearContext(); @@ -425,6 +435,36 @@ public class AbstractProcessingFilterTests extends TestCase { assertEquals(makeSavedRequestForUrl().getFullRequestUrl(), response.getRedirectedUrl()); assertNotNull(SecurityContextHolder.getContext().getAuthentication()); } + + + 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()); + + // 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.setDefaultTargetUrl("/foobar"); + + // Configure target resolver default implementation not to use POST SavedRequest + TargetUrlResolverImpl targetUrlResolver = new TargetUrlResolverImpl(); + targetUrlResolver.setJustUseSavedRequestOnGet(true); + filter.setTargetUrlResolver(targetUrlResolver); + + // Test + executeFilterInContainerSimulator(config, filter, request, response, chain); + assertEquals("/mycontext/foobar", response.getRedirectedUrl()); + assertNotNull(SecurityContextHolder.getContext().getAuthentication()); + } /** * SEC-297 fix.