瀏覽代碼

SEC-1592: Updated CasAuthenticationFilter so that it does not continue FilterChain when handling proxy requests.

The fix moves CommonUtils.readAndRespondToProxyReceptorRequest into CasAuthenticationFilter.attemptAuthentication. This makes sense since
The CAS server is authenticating that the proxy url is valid (i.e. it exists and the SSL handshake succeeds). It also allows the FilterChain
to not be processed by returning a null Authentication.
Rob Winch 14 年之前
父節點
當前提交
ece824fca2

+ 19 - 13
cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java

@@ -54,6 +54,7 @@ import org.springframework.security.web.authentication.AbstractAuthenticationPro
  * By default this filter processes the URL <tt>/j_spring_cas_security_check</tt>.
  *
  * @author Ben Alex
+ * @author Rob Winch
  */
 public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFilter {
     //~ Static fields/initializers =====================================================================================
@@ -89,7 +90,13 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
     //~ Methods ========================================================================================================
 
     public Authentication attemptAuthentication(final HttpServletRequest request, final HttpServletResponse response)
-            throws AuthenticationException {
+            throws AuthenticationException, IOException {
+        // if the request is a proxy request process it and return null to indicate the request has been processed
+        if(isProxyRequest(request)) {
+            CommonUtils.readAndRespondToProxyReceptorRequest(request, response, this.proxyGrantingTicketStorage);
+            return null;
+        }
+
         final String username = CAS_STATEFUL_IDENTIFIER;
         String password = request.getParameter(this.artifactParameter);
 
@@ -108,18 +115,7 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
      * Overridden to provide proxying capabilities.
      */
     protected boolean requiresAuthentication(final HttpServletRequest request, final HttpServletResponse response) {
-        final String requestUri = request.getRequestURI();
-
-        if (CommonUtils.isEmpty(this.proxyReceptorUrl) || !requestUri.endsWith(this.proxyReceptorUrl) || this.proxyGrantingTicketStorage == null) {
-            return super.requiresAuthentication(request, response);
-        }
-
-        try {
-            CommonUtils.readAndRespondToProxyReceptorRequest(request, response, this.proxyGrantingTicketStorage);
-            return false;
-        } catch (final IOException e) {
-            return super.requiresAuthentication(request, response);
-        }
+        return isProxyRequest(request) || super.requiresAuthentication(request, response);
     }
 
     public final void setProxyReceptorUrl(final String proxyReceptorUrl) {
@@ -134,4 +130,14 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil
     public final void setServiceProperties(final ServiceProperties serviceProperties) {
         this.artifactParameter = serviceProperties.getArtifactParameter();
     }
+
+    /**
+     * Indicates if the request is eligible to be processed as a proxy request.
+     * @param request
+     * @return
+     */
+    private boolean isProxyRequest(final HttpServletRequest request) {
+        final String requestUri = request.getRequestURI();
+        return this.proxyGrantingTicketStorage != null && !CommonUtils.isEmpty(this.proxyReceptorUrl) && requestUri.endsWith(this.proxyReceptorUrl);
+    }
 }

+ 59 - 0
cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java

@@ -16,7 +16,11 @@
 package org.springframework.security.cas.web;
 
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
+import javax.servlet.FilterChain;
+
+import org.jasig.cas.client.proxy.ProxyGrantingTicketStorage;
 import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
@@ -30,6 +34,7 @@ import org.springframework.security.core.AuthenticationException;
  * Tests {@link CasAuthenticationFilter}.
  *
  * @author Ben Alex
+ * @author Rob Winch
  */
 public class CasAuthenticationFilterTests {
     //~ Methods ========================================================================================================
@@ -67,4 +72,58 @@ public class CasAuthenticationFilterTests {
 
         filter.attemptAuthentication(new MockHttpServletRequest(), new MockHttpServletResponse());
     }
+
+    @Test
+    public void testRequiresAuthenticationFilterProcessUrl() {
+        CasAuthenticationFilter filter = new CasAuthenticationFilter();
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+
+        request.setRequestURI(filter.getFilterProcessesUrl());
+        assertTrue(filter.requiresAuthentication(request, response));
+    }
+
+    @Test
+    public void testRequiresAuthenticationProxyRequest() {
+        CasAuthenticationFilter filter = new CasAuthenticationFilter();
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+
+        request.setRequestURI("/pgtCallback");
+        assertFalse(filter.requiresAuthentication(request, response));
+        filter.setProxyReceptorUrl(request.getRequestURI());
+        assertFalse(filter.requiresAuthentication(request, response));
+        filter.setProxyGrantingTicketStorage(mock(ProxyGrantingTicketStorage.class));
+        assertTrue(filter.requiresAuthentication(request, response));
+        request.setRequestURI("/other");
+        assertFalse(filter.requiresAuthentication(request, response));
+    }
+
+    @Test
+    public void testAuthenticateProxyUrl() throws Exception {
+        CasAuthenticationFilter filter = new CasAuthenticationFilter();
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+
+        request.setRequestURI("/pgtCallback");
+        filter.setProxyGrantingTicketStorage(mock(ProxyGrantingTicketStorage.class));
+        filter.setProxyReceptorUrl(request.getRequestURI());
+        assertNull(filter.attemptAuthentication(request, response));
+    }
+
+    // SEC-1592
+    @Test
+    public void testChainNotInvokedForProxy() throws Exception {
+        CasAuthenticationFilter filter = new CasAuthenticationFilter();
+        MockHttpServletRequest request = new MockHttpServletRequest();
+        MockHttpServletResponse response = new MockHttpServletResponse();
+        FilterChain chain = mock(FilterChain.class);
+
+        request.setRequestURI("/pgtCallback");
+        filter.setProxyGrantingTicketStorage(mock(ProxyGrantingTicketStorage.class));
+        filter.setProxyReceptorUrl(request.getRequestURI());
+
+        filter.doFilter(request,response,chain);
+        verifyZeroInteractions(chain);
+    }
 }