From a2bd1bc9af3630be84baae9eb0b16fee4e365263 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Thu, 5 Aug 2010 21:09:34 +0100 Subject: [PATCH] SEC-1498: Allow use of absolute URL fopr login form in LoginUrlAuthenticationEntryPoint. --- .../LoginUrlAuthenticationEntryPoint.java | 36 ++++++---- ...LoginUrlAuthenticationEntryPointTests.java | 67 ++++++++++++------- 2 files changed, 65 insertions(+), 38 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java b/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java index fd00025c7b..52a690062e 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java +++ b/web/src/main/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPoint.java @@ -15,8 +15,6 @@ package org.springframework.security.web.authentication; - - import java.io.IOException; import javax.servlet.RequestDispatcher; @@ -43,17 +41,20 @@ import org.springframework.util.StringUtils; /** * Used by the {@link ExceptionTranslationFilter} to commence a form login - * authentication via the {@link UsernamePasswordAuthenticationFilter}. This object - * holds the location of the login form, relative to the web app context path, - * and is used to commence a redirect to that form. + * authentication via the {@link UsernamePasswordAuthenticationFilter}. *

- * By setting the forceHttps property to true, you may configure the - * class to force the protocol used for the login form to be HTTPS, + * Holds the location of the login form in the {@code loginFormUrl} property, and + * uses that to build a redirect URL to the login page. Alternatively, an absolute URL + * can be set in this property and that will be used exclusively. + *

+ * When using a relative URL, you can set the {@code forceHttps} property to true, + * to force the protocol used for the login form to be {@code HTTPS}, * even if the original intercepted request for a resource used the - * HTTP protocol. When this happens, after a successful login + * {@code HTTP} protocol. When this happens, after a successful login * (via HTTPS), the original resource will still be accessed as HTTP, via the * original request URL. For the forced HTTPS feature to work, the {@link - * PortMapper} is consulted to determine the HTTP:HTTPS pairs. + * PortMapper} is consulted to determine the HTTP:HTTPS pairs. The value of + * {@code forceHttps} will have no effect if an absolute URL is used. * * @author Ben Alex * @author colin sampaleanu @@ -85,6 +86,9 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin public void afterPropertiesSet() throws Exception { Assert.isTrue(StringUtils.hasText(loginFormUrl) && UrlUtils.isValidRedirectUrl(loginFormUrl), "loginFormUrl must be specified and must be a valid redirect URL"); + if (useForward && UrlUtils.isAbsoluteUrl(loginFormUrl)) { + throw new IllegalArgumentException("useForward must be false if using an absolute loginFormURL"); + } Assert.notNull(portMapper, "portMapper must be specified"); Assert.notNull(portResolver, "portResolver must be specified"); } @@ -146,6 +150,11 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin AuthenticationException authException) { String loginForm = determineUrlToUseForThisRequest(request, response, authException); + + if (UrlUtils.isAbsoluteUrl(loginForm)) { + return loginForm; + } + int serverPort = portResolver.getServerPort(request); String scheme = request.getScheme(); @@ -217,8 +226,8 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin /** * The URL where the UsernamePasswordAuthenticationFilter login - * page can be found. Should be relative to the web-app context path, and - * include a leading / + * page can be found. Should either be relative to the web-app context path + * (include a leading {@code /}) or an absolute URL. */ public void setLoginFormUrl(String loginFormUrl) { this.loginFormUrl = loginFormUrl; @@ -245,10 +254,11 @@ public class LoginUrlAuthenticationEntryPoint implements AuthenticationEntryPoin } /** - * Tells if we are to do a forward to the loginFormUrl using the RequestDispatcher, + * Tells if we are to do a forward to the {@code loginFormUrl} using the {@code RequestDispatcher}, * instead of a 302 redirect. * - * @param useForward + * @param useForward true if a forward to the login page should be used. Must be false (the default) if + * {@code loginFormUrl} is set to an absolute value. */ public void setUseForward(boolean useForward) { this.useForward = useForward; diff --git a/web/src/test/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPointTests.java b/web/src/test/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPointTests.java index 1ca323587a..9c64b0071e 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPointTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/LoginUrlAuthenticationEntryPointTests.java @@ -15,18 +15,15 @@ package org.springframework.security.web.authentication; -import junit.framework.TestCase; +import static org.junit.Assert.*; -import org.springframework.security.MockPortResolver; - -import org.springframework.security.web.PortMapperImpl; -import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint; +import java.util.*; +import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; - -import java.util.HashMap; -import java.util.Map; +import org.springframework.security.MockPortResolver; +import org.springframework.security.web.PortMapperImpl; /** @@ -35,45 +32,36 @@ import java.util.Map; * @author Ben Alex * @author colin sampaleanu */ -public class LoginUrlAuthenticationEntryPointTests extends TestCase { +public class LoginUrlAuthenticationEntryPointTests { //~ Methods ======================================================================================================== + @Test(expected=IllegalArgumentException.class) public void testDetectsMissingLoginFormUrl() throws Exception { LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); ep.setPortMapper(new PortMapperImpl()); ep.setPortResolver(new MockPortResolver(80, 443)); - - try { - ep.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + ep.afterPropertiesSet(); } + @Test(expected=IllegalArgumentException.class) public void testDetectsMissingPortMapper() throws Exception { LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); ep.setLoginFormUrl("xxx"); ep.setPortMapper(null); - try { - ep.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + ep.afterPropertiesSet(); } + @Test(expected=IllegalArgumentException.class) public void testDetectsMissingPortResolver() throws Exception { LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); ep.setLoginFormUrl("xxx"); ep.setPortResolver(null); - try { - ep.afterPropertiesSet(); - fail("Should have thrown IllegalArgumentException"); - } catch (IllegalArgumentException expected) { - } + ep.afterPropertiesSet(); } + @Test public void testGettersSetters() { LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); ep.setLoginFormUrl("/hello"); @@ -87,8 +75,12 @@ public class LoginUrlAuthenticationEntryPointTests extends TestCase { assertFalse(ep.isForceHttps()); ep.setForceHttps(true); assertTrue(ep.isForceHttps()); + assertFalse(ep.isUseForward()); + ep.setUseForward(true); + assertTrue(ep.isUseForward()); } + @Test public void testHttpsOperationFromOriginalHttpUrl() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI("/some_path"); @@ -140,6 +132,7 @@ public class LoginUrlAuthenticationEntryPointTests extends TestCase { assertEquals("https://www.example.com:9999/bigWebApp/hello", response.getRedirectedUrl()); } + @Test public void testHttpsOperationFromOriginalHttpsUrl() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI("/some_path"); @@ -168,6 +161,7 @@ public class LoginUrlAuthenticationEntryPointTests extends TestCase { assertEquals("https://www.example.com:8443/bigWebApp/hello", response.getRedirectedUrl()); } + @Test public void testNormalOperation() throws Exception { LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); ep.setLoginFormUrl("/hello"); @@ -189,6 +183,7 @@ public class LoginUrlAuthenticationEntryPointTests extends TestCase { assertEquals("http://www.example.com/bigWebApp/hello", response.getRedirectedUrl()); } + @Test public void testOperationWhenHttpsRequestsButHttpsPortUnknown() throws Exception { LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); ep.setLoginFormUrl("/hello"); @@ -212,6 +207,7 @@ public class LoginUrlAuthenticationEntryPointTests extends TestCase { assertEquals("http://www.example.com:8888/bigWebApp/hello", response.getRedirectedUrl()); } + @Test public void testServerSideRedirectWithoutForceHttpsForwardsToLoginPage() throws Exception { LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); ep.setLoginFormUrl("/hello"); @@ -232,6 +228,7 @@ public class LoginUrlAuthenticationEntryPointTests extends TestCase { assertEquals("/hello", response.getForwardedUrl()); } + @Test public void testServerSideRedirectWithForceHttpsRedirectsCurrentRequest() throws Exception { LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); ep.setLoginFormUrl("/hello"); @@ -253,4 +250,24 @@ public class LoginUrlAuthenticationEntryPointTests extends TestCase { assertEquals("https://www.example.com/bigWebApp/some_path", response.getRedirectedUrl()); } + // SEC-1498 + @Test + public void absoluteLoginFormUrlIsSupported() throws Exception { + LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); + final String loginFormUrl = "http://somesite.com/login"; + ep.setLoginFormUrl(loginFormUrl); + ep.afterPropertiesSet(); + MockHttpServletResponse response = new MockHttpServletResponse(); + ep.commence(new MockHttpServletRequest("GET", "/someUrl"), response, null); + assertEquals(loginFormUrl, response.getRedirectedUrl()); + } + + @Test(expected=IllegalArgumentException.class) + public void absoluteLoginFormUrlCantBeUsedWithForwarding() throws Exception { + LoginUrlAuthenticationEntryPoint ep = new LoginUrlAuthenticationEntryPoint(); + final String loginFormUrl = "http://somesite.com/login"; + ep.setLoginFormUrl(loginFormUrl); + ep.setUseForward(true); + ep.afterPropertiesSet(); + } }