From a5ffda73697282f24031e0bb55935321f48ed9d8 Mon Sep 17 00:00:00 2001 From: Ben Alex Date: Fri, 21 Oct 2005 08:00:15 +0000 Subject: [PATCH] SEC-63: Do not return an absolute URL unless switching from HTTP to HTTPS. --- ...henticationProcessingFilterEntryPoint.java | 36 +++++++++---------- ...cationProcessingFilterEntryPointTests.java | 24 ++++--------- 2 files changed, 23 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/ui/webapp/AuthenticationProcessingFilterEntryPoint.java b/core/src/main/java/org/acegisecurity/ui/webapp/AuthenticationProcessingFilterEntryPoint.java index 239b9698d4..335c47b819 100644 --- a/core/src/main/java/org/acegisecurity/ui/webapp/AuthenticationProcessingFilterEntryPoint.java +++ b/core/src/main/java/org/acegisecurity/ui/webapp/AuthenticationProcessingFilterEntryPoint.java @@ -12,7 +12,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package net.sf.acegisecurity.ui.webapp; import net.sf.acegisecurity.AuthenticationException; @@ -26,6 +25,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.InitializingBean; + import org.springframework.util.Assert; import java.io.IOException; @@ -44,7 +44,7 @@ import javax.servlet.http.HttpServletResponse; * holds the location of the login form, relative to the web app context path, * and is used to commence a redirect to that form. *

- * + * *

* By setting the forceHttps property to true, you may configure the * class to force the protocol used for the login form to be @@ -57,23 +57,17 @@ import javax.servlet.http.HttpServletResponse; * * @author Ben Alex * @author colin sampaleanu + * @author Omri Spector * @version $Id$ */ public class AuthenticationProcessingFilterEntryPoint implements AuthenticationEntryPoint, InitializingBean { - //~ Static fields/initializers ============================================= - private static final Log logger = LogFactory.getLog(AuthenticationProcessingFilterEntryPoint.class); - - //~ Instance fields ======================================================== - private PortMapper portMapper = new PortMapperImpl(); private PortResolver portResolver = new PortResolverImpl(); private String loginFormUrl; private boolean forceHttps = false; - //~ Methods ================================================================ - /** * Set to true to force login form access to be via https. If this value is * ture (the default is false), and the incoming request for the protected @@ -122,7 +116,7 @@ public class AuthenticationProcessingFilterEntryPoint } public void afterPropertiesSet() throws Exception { - Assert.hasLength(loginFormUrl,"loginFormUrl must be specified"); + Assert.hasLength(loginFormUrl, "loginFormUrl must be specified"); Assert.notNull(portMapper, "portMapper must be specified"); Assert.notNull(portResolver, "portResolver must be specified"); } @@ -136,7 +130,11 @@ public class AuthenticationProcessingFilterEntryPoint int serverPort = portResolver.getServerPort(request); String contextPath = req.getContextPath(); - boolean includePort = true; + boolean inHttp = "http".equals(scheme.toLowerCase()); + boolean inHttps = "https".equals(scheme.toLowerCase()); + + boolean includePort = ((inHttp && (serverPort == 80)) || + (inHttps && (serverPort == 443))); if ("http".equals(scheme.toLowerCase()) && (serverPort == 80)) { includePort = false; @@ -146,11 +144,9 @@ public class AuthenticationProcessingFilterEntryPoint includePort = false; } - String redirectUrl = scheme + "://" + serverName - + ((includePort) ? (":" + serverPort) : "") + contextPath - + loginFormUrl; + String redirectUrl = contextPath + loginFormUrl; - if (forceHttps && req.getScheme().equals("http")) { + if (forceHttps && inHttp) { Integer httpPort = new Integer(portResolver.getServerPort(request)); Integer httpsPort = (Integer) portMapper.lookupHttpsPort(httpPort); @@ -161,9 +157,9 @@ public class AuthenticationProcessingFilterEntryPoint includePort = true; } - redirectUrl = "https://" + serverName - + ((includePort) ? (":" + httpsPort) : "") + contextPath - + loginFormUrl; + redirectUrl = "https://" + serverName + + ((includePort) ? (":" + httpsPort) : "") + contextPath + + loginFormUrl; } } @@ -171,7 +167,7 @@ public class AuthenticationProcessingFilterEntryPoint logger.debug("Redirecting to: " + redirectUrl); } - ((HttpServletResponse) response).sendRedirect(((HttpServletResponse) response) - .encodeRedirectURL(redirectUrl)); + ((HttpServletResponse) response).sendRedirect(((HttpServletResponse) response).encodeRedirectURL( + redirectUrl)); } } diff --git a/core/src/test/java/org/acegisecurity/ui/webapp/AuthenticationProcessingFilterEntryPointTests.java b/core/src/test/java/org/acegisecurity/ui/webapp/AuthenticationProcessingFilterEntryPointTests.java index 74a0711de8..2a4174ffff 100644 --- a/core/src/test/java/org/acegisecurity/ui/webapp/AuthenticationProcessingFilterEntryPointTests.java +++ b/core/src/test/java/org/acegisecurity/ui/webapp/AuthenticationProcessingFilterEntryPointTests.java @@ -12,23 +12,19 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package net.sf.acegisecurity.ui.webapp; import junit.framework.TestCase; - - import net.sf.acegisecurity.MockPortResolver; - import net.sf.acegisecurity.util.PortMapperImpl; -import java.util.HashMap; -import java.util.Map; - import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import java.util.HashMap; +import java.util.Map; + /** * Tests {@link AuthenticationProcessingFilterEntryPoint}. @@ -38,8 +34,6 @@ import org.springframework.mock.web.MockHttpServletResponse; * @version $Id$ */ public class AuthenticationProcessingFilterEntryPointTests extends TestCase { - //~ Methods ================================================================ - public final void setUp() throws Exception { super.setUp(); } @@ -178,15 +172,13 @@ public class AuthenticationProcessingFilterEntryPointTests extends TestCase { ep.afterPropertiesSet(); ep.commence(request, response, null); - assertEquals("https://www.example.com/bigWebApp/hello", - response.getRedirectedUrl()); + assertEquals("/bigWebApp/hello", response.getRedirectedUrl()); request.setServerPort(8443); response = new MockHttpServletResponse(); ep.setPortResolver(new MockPortResolver(8080, 8443)); ep.commence(request, response, null); - assertEquals("https://www.example.com:8443/bigWebApp/hello", - response.getRedirectedUrl()); + assertEquals("/bigWebApp/hello", response.getRedirectedUrl()); } public void testNormalOperation() throws Exception { @@ -208,8 +200,7 @@ public class AuthenticationProcessingFilterEntryPointTests extends TestCase { ep.afterPropertiesSet(); ep.commence(request, response, null); - assertEquals("http://www.example.com/bigWebApp/hello", - response.getRedirectedUrl()); + assertEquals("/bigWebApp/hello", response.getRedirectedUrl()); } public void testOperationWhenHttpsRequestsButHttpsPortUnknown() @@ -235,7 +226,6 @@ public class AuthenticationProcessingFilterEntryPointTests extends TestCase { ep.commence(request, response, null); // Response doesn't switch to HTTPS, as we didn't know HTTP port 8888 to HTTP port mapping - assertEquals("http://www.example.com:8888/bigWebApp/hello", - response.getRedirectedUrl()); + assertEquals("/bigWebApp/hello", response.getRedirectedUrl()); } }