SEC-63: Do not return an absolute URL unless switching from HTTP to HTTPS.

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;
@@ -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
+        String redirectUrl = contextPath + loginFormUrl;
-            + ((includePort) ? (":" + serverPort) : "") + 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
+                redirectUrl = "https://" + serverName +
-                    + ((includePort) ? (":" + httpsPort) : "") + contextPath
+                    ((includePort) ? (":" + httpsPort) : "") + contextPath +
-                    + loginFormUrl;
+                    loginFormUrl;
             }
         }
 
@@ -171,7 +167,7 @@ public class AuthenticationProcessingFilterEntryPoint
             logger.debug("Redirecting to: " + redirectUrl);
         }
 
-        ((HttpServletResponse) response).sendRedirect(((HttpServletResponse) response)
+        ((HttpServletResponse) response).sendRedirect(((HttpServletResponse) response).encodeRedirectURL(
-            .encodeRedirectURL(redirectUrl));
+                redirectUrl));
     }
 }

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",
+        assertEquals("/bigWebApp/hello", response.getRedirectedUrl());
-            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",
+        assertEquals("/bigWebApp/hello", response.getRedirectedUrl());
-            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",
+        assertEquals("/bigWebApp/hello", response.getRedirectedUrl());
-            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",
+        assertEquals("/bigWebApp/hello", response.getRedirectedUrl());
-            response.getRedirectedUrl());
     }
 }