From b8830299452ad1da0ab1ffd7e49d131c02b13cf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Launay?= Date: Wed, 13 Oct 2010 15:16:03 +0200 Subject: [PATCH] SEC-1592: CAS proxy receptor requests must not pass through the filter chain Currently proxy receptor requests are handled by CasAuthenticationFilter and then passed through the filter chain which may produce unsupported status code for the cas server and then considered the webapp as an invalid end-point. Fix by stopping proxy receptor request processing once it has been handled like it is done in Cas20ProxyReceivingTicketValidationFilter from cas-client. --- .../security/cas/web/CasAuthenticationFilter.java | 42 +++++---- .../cas/web/CasAuthenticationFilterTests.java | 97 +++++++++++++++++++- 2 files changed, 117 insertions(+), 22 deletions(-) diff --git a/cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java b/cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java index 314a1fd..4d84dfd 100644 --- a/cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java +++ b/cas/src/main/java/org/springframework/security/cas/web/CasAuthenticationFilter.java @@ -17,6 +17,10 @@ package org.springframework.security.cas.web; import java.io.IOException; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -88,6 +92,26 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil //~ Methods ======================================================================================================== + /** + * Overridden to provide proxying capabilities. + */ + public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) + throws IOException, ServletException { + HttpServletRequest request = (HttpServletRequest) req; + HttpServletResponse response = (HttpServletResponse) res; + final String requestUri = request.getRequestURI(); + + if (CommonUtils.isEmpty(this.proxyReceptorUrl) || !requestUri.endsWith(this.proxyReceptorUrl) || this.proxyGrantingTicketStorage == null) { + super.doFilter(req, res, chain); + } else { + try { + CommonUtils.readAndRespondToProxyReceptorRequest(request, response, this.proxyGrantingTicketStorage); + } catch (final IOException e) { + super.doFilter(req, res, chain); + } + } + } + public Authentication attemptAuthentication(final HttpServletRequest request, final HttpServletResponse response) throws AuthenticationException { final String username = CAS_STATEFUL_IDENTIFIER; @@ -104,24 +128,6 @@ public class CasAuthenticationFilter extends AbstractAuthenticationProcessingFil return this.getAuthenticationManager().authenticate(authRequest); } - /** - * 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); - } - } - public final void setProxyReceptorUrl(final String proxyReceptorUrl) { this.proxyReceptorUrl = proxyReceptorUrl; } diff --git a/cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java b/cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java index efb8325..1412a33 100644 --- a/cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java +++ b/cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java @@ -16,17 +16,25 @@ package org.springframework.security.cas.web; import static org.junit.Assert.*; -import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.*; + +import java.util.ArrayList; +import java.util.List; + +import javax.servlet.http.HttpServletResponse; import org.jasig.cas.client.proxy.ProxyGrantingTicketStorage; import org.junit.Test; +import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.BadCredentialsException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.cas.ServiceProperties; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; +import org.springframework.security.web.authentication.RememberMeServices; /** @@ -50,18 +58,99 @@ public class CasAuthenticationFilterTests { public void testNormalOperation() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/j_spring_cas_security_check"); request.addParameter("ticket", "ST-0-ER94xMJmn6pha35CQRoZ"); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + RememberMeServices mockRememberMeServices = mock(RememberMeServices.class); + final List authenticationsHolder = new ArrayList(); CasAuthenticationFilter filter = new CasAuthenticationFilter(); filter.setAuthenticationManager(new AuthenticationManager() { public Authentication authenticate(Authentication a) { + authenticationsHolder.add(a); return a; } }); + filter.setRememberMeServices(mockRememberMeServices); + filter.afterPropertiesSet(); + + filter.doFilter(request, response, chain); + + // Check that authentication has been performed + assertEquals(1, authenticationsHolder.size()); + + // Test if the resulting authentication is valid + Authentication authentication = authenticationsHolder.get(0); + assertTrue(authentication instanceof UsernamePasswordAuthenticationToken); + UsernamePasswordAuthenticationToken upat = (UsernamePasswordAuthenticationToken) authentication; + assertEquals(CasAuthenticationFilter.CAS_STATEFUL_IDENTIFIER, upat.getPrincipal()); + assertEquals("ST-0-ER94xMJmn6pha35CQRoZ", upat.getCredentials()); + + // Check that mockRememberMeServices must have been called with a success + verify(mockRememberMeServices).loginSuccess(request, response, authentication); + verifyNoMoreInteractions(mockRememberMeServices); + + // Check that the request/response pair was not passed through the chain + assertNull(chain.getRequest()); + assertNull(chain.getResponse()); + + // A successful authentication means a redirect + assertEquals("/", response.getRedirectedUrl()); + } + + @Test + public void testProxyReceptor() throws Exception { + CasAuthenticationFilter filter = new CasAuthenticationFilter(); + ProxyGrantingTicketStorage proxyGrantingTicketStorage = mock(ProxyGrantingTicketStorage.class); + filter.setProxyGrantingTicketStorage(proxyGrantingTicketStorage); + filter.setProxyReceptorUrl("/proxyReceptor"); + filter.setAuthenticationManager(new AuthenticationManager() { + public Authentication authenticate(Authentication a) { + return a; + } + }); + filter.afterPropertiesSet(); + + // 1. Test the proxy receptor ping (done by cas-server in + // HttpBasedServiceCredentialsAuthenticationHandler to check if the + // remote service is a valid end-point) + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/proxyReceptor"); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + // Check that the request/response pair was not passed through the chain + assertNull(chain.getRequest()); + assertNull(chain.getResponse()); + + // Check that proxyGrantingTicketStorage was not used + verifyNoMoreInteractions(proxyGrantingTicketStorage); + + // A 200 is a valid end-point + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + assertEquals("", response.getContentAsString()); + + // 2. Test the proxy receptor with the proper parameters (PGT/PGTIOU mapping) + request = new MockHttpServletRequest("GET", "/proxyReceptor"); + request.addParameter("pgtId", "PGT-123"); + request.addParameter("pgtIou", "PGTIOU-456"); + response = new MockHttpServletResponse(); + chain = new MockFilterChain(); + + filter.doFilter(request, response, chain); + + // Check that the request/response pair was not passed through the chain + assertNull(chain.getRequest()); + assertNull(chain.getResponse()); - assertTrue(filter.requiresAuthentication(request, new MockHttpServletResponse())); + // Check that the mapping has been stored + verify(proxyGrantingTicketStorage).save("PGTIOU-456", "PGT-123"); + verifyNoMoreInteractions(proxyGrantingTicketStorage); - Authentication result = filter.attemptAuthentication(request, new MockHttpServletResponse()); - assertTrue(result != null); + // Check that the response has been committed + assertEquals(HttpServletResponse.SC_OK, response.getStatus()); + assertEquals("" + + "", response.getContentAsString()); } @Test(expected=AuthenticationException.class) -- 1.7.0.4