From 2cb99f079152ac05cee5c90457c7feb3bb2de55e Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 24 Jul 2014 16:56:13 -0500 Subject: [PATCH] SEC-2688: CAS Proxy Ticket Authentication uses Service for host & port --- .../security/cas/ServiceProperties.java | 4 +- .../DefaultServiceAuthenticationDetails.java | 23 +++++- .../ServiceAuthenticationDetailsSource.java | 50 ++++++++++++- .../cas/web/ServicePropertiesTests.java | 7 +- ...aultServiceAuthenticationDetailsTests.java | 75 ++++++++++++++++--- ...tserviceauthenticationdetails-explicit.xml | 23 ++++++ ...serviceauthenticationdetails-passivity.xml | 15 ++++ cas/template.mf | 2 +- docs/manual/src/asciidoc/index.adoc | 4 +- 9 files changed, 179 insertions(+), 24 deletions(-) create mode 100644 cas/src/test/resources/org/springframework/security/cas/web/authentication/defaultserviceauthenticationdetails-explicit.xml create mode 100644 cas/src/test/resources/org/springframework/security/cas/web/authentication/defaultserviceauthenticationdetails-passivity.xml diff --git a/cas/src/main/java/org/springframework/security/cas/ServiceProperties.java b/cas/src/main/java/org/springframework/security/cas/ServiceProperties.java index d49e98ccc4..55d913f417 100644 --- a/cas/src/main/java/org/springframework/security/cas/ServiceProperties.java +++ b/cas/src/main/java/org/springframework/security/cas/ServiceProperties.java @@ -49,9 +49,7 @@ public class ServiceProperties implements InitializingBean { //~ Methods ======================================================================================================== public void afterPropertiesSet() throws Exception { - if(!authenticateAllArtifacts) { - Assert.hasLength(this.service, "service must be specified unless authenticateAllArtifacts is true."); - } + Assert.hasLength(this.service, "service cannot be empty."); Assert.hasLength(this.artifactParameter, "artifactParameter cannot be empty."); Assert.hasLength(this.serviceParameter, "serviceParameter cannot be empty."); } diff --git a/cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java b/cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java index f404510eaa..41b4201c22 100644 --- a/cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java +++ b/cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java @@ -15,6 +15,8 @@ */ package org.springframework.security.cas.web.authentication; +import java.net.MalformedURLException; +import java.net.URL; import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; @@ -50,11 +52,13 @@ final class DefaultServiceAuthenticationDetails extends WebAuthenticationDetails * string from containing the artifact name and value. This can * be created using {@link #createArtifactPattern(String)}. */ - DefaultServiceAuthenticationDetails(HttpServletRequest request, Pattern artifactPattern) { + DefaultServiceAuthenticationDetails(String casService, HttpServletRequest request, Pattern artifactPattern) throws MalformedURLException { super(request); + URL casServiceUrl = new URL(casService); + int port = getServicePort(casServiceUrl); final String query = getQueryString(request,artifactPattern); - this.serviceUrl = UrlUtils.buildFullRequestUrl(request.getScheme(), - request.getServerName(), request.getServerPort(), + this.serviceUrl = UrlUtils.buildFullRequestUrl(casServiceUrl.getProtocol(), + casServiceUrl.getHost(), port, request.getRequestURI(), query); } @@ -128,4 +132,17 @@ final class DefaultServiceAuthenticationDetails extends WebAuthenticationDetails Assert.hasLength(artifactParameterName); return Pattern.compile("&?"+Pattern.quote(artifactParameterName)+"=[^&]*"); } + + /** + * Gets the port from the casServiceURL ensuring to return the proper value if the default port is being used. + * @param casServiceUrl the casServerUrl to be used (i.e. "https://example.com/context/j_spring_security_cas_check") + * @return the port that is configured for the casServerUrl + */ + private static int getServicePort(URL casServiceUrl) { + int port = casServiceUrl.getPort(); + if(port == -1) { + port = casServiceUrl.getDefaultPort(); + } + return port; + } } \ No newline at end of file diff --git a/cas/src/main/java/org/springframework/security/cas/web/authentication/ServiceAuthenticationDetailsSource.java b/cas/src/main/java/org/springframework/security/cas/web/authentication/ServiceAuthenticationDetailsSource.java index 546a160e2b..f40686cfb7 100644 --- a/cas/src/main/java/org/springframework/security/cas/web/authentication/ServiceAuthenticationDetailsSource.java +++ b/cas/src/main/java/org/springframework/security/cas/web/authentication/ServiceAuthenticationDetailsSource.java @@ -15,12 +15,18 @@ */ package org.springframework.security.cas.web.authentication; +import java.net.MalformedURLException; import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.security.authentication.AuthenticationDetailsSource; import org.springframework.security.cas.ServiceProperties; +import org.springframework.util.Assert; /** * The {@code AuthenticationDetailsSource} that is set on the @@ -33,20 +39,33 @@ import org.springframework.security.cas.ServiceProperties; * @author Rob Winch */ public class ServiceAuthenticationDetailsSource implements AuthenticationDetailsSource { + ServiceAuthenticationDetails>, ApplicationContextAware { //~ Instance fields ================================================================================================ private final Pattern artifactPattern; + private ServiceProperties serviceProperties; + //~ Constructors =================================================================================================== /** * Creates an implementation that uses the default CAS artifactParameterName. + * @deprecated Use ServiceAuthenticationDetailsSource(ServiceProperties) */ + @Deprecated public ServiceAuthenticationDetailsSource() { this(ServiceProperties.DEFAULT_CAS_ARTIFACT_PARAMETER); } + /** + * Creates an implementation that uses the specified ServiceProperites and the default CAS artifactParameterName. + * + * @param serviceProperties The ServiceProperties to use to construct the serviceUrl. + */ + public ServiceAuthenticationDetailsSource(ServiceProperties serviceProperties) { + this(serviceProperties,ServiceProperties.DEFAULT_CAS_ARTIFACT_PARAMETER); + } + /** * Creates an implementation that uses the specified artifactParameterName * @@ -54,11 +73,27 @@ public class ServiceAuthenticationDetailsSource implements AuthenticationDetails * the artifactParameterName that is removed from the current * URL. The result becomes the service url. Cannot be null and * cannot be an empty String. + * @deprecated Use ServiceAuthenticationDetailsSource(ServiceProperties,String) */ public ServiceAuthenticationDetailsSource(final String artifactParameterName) { this.artifactPattern = DefaultServiceAuthenticationDetails.createArtifactPattern(artifactParameterName); } + /** + * Creates an implementation that uses the specified artifactParameterName + * + * @param serviceProperties The ServiceProperties to use to construct the serviceUrl. + * @param artifactParameterName + * the artifactParameterName that is removed from the current + * URL. The result becomes the service url. Cannot be null and + * cannot be an empty String. + */ + public ServiceAuthenticationDetailsSource(ServiceProperties serviceProperties, String artifactParameterName) { + Assert.notNull(serviceProperties, "serviceProperties cannot be null"); + this.serviceProperties = serviceProperties; + this.artifactPattern = DefaultServiceAuthenticationDetails.createArtifactPattern(artifactParameterName); + } + //~ Methods ======================================================================================================== /** @@ -66,6 +101,17 @@ public class ServiceAuthenticationDetailsSource implements AuthenticationDetails * @return the {@code ServiceAuthenticationDetails} containing information about the current request */ public ServiceAuthenticationDetails buildDetails(HttpServletRequest context) { - return new DefaultServiceAuthenticationDetails(context,artifactPattern); + try { + return new DefaultServiceAuthenticationDetails(serviceProperties.getService(),context,artifactPattern); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } + } + + @Override + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + if(serviceProperties == null) { + serviceProperties = applicationContext.getBean(ServiceProperties.class); + } } } \ No newline at end of file diff --git a/cas/src/test/java/org/springframework/security/cas/web/ServicePropertiesTests.java b/cas/src/test/java/org/springframework/security/cas/web/ServicePropertiesTests.java index 7c4f3bde1c..ec327ff49b 100644 --- a/cas/src/test/java/org/springframework/security/cas/web/ServicePropertiesTests.java +++ b/cas/src/test/java/org/springframework/security/cas/web/ServicePropertiesTests.java @@ -36,10 +36,13 @@ public class ServicePropertiesTests { } @Test - public void allowNullServiceWhenAuthenticateAllTokens() throws Exception { + public void nullServiceWhenAuthenticateAllTokens() throws Exception { ServiceProperties sp = new ServiceProperties(); sp.setAuthenticateAllArtifacts(true); - sp.afterPropertiesSet(); + try { + sp.afterPropertiesSet(); + fail("Expected Exception"); + }catch(IllegalArgumentException success) {} sp.setAuthenticateAllArtifacts(false); try { sp.afterPropertiesSet(); diff --git a/cas/src/test/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetailsTests.java b/cas/src/test/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetailsTests.java index 095caa4195..f8d5d648e3 100644 --- a/cas/src/test/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetailsTests.java +++ b/cas/src/test/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetailsTests.java @@ -18,11 +18,21 @@ import static org.junit.Assert.assertEquals; import java.util.regex.Pattern; +import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.support.ClassPathXmlApplicationContext; +import org.springframework.context.support.GenericApplicationContext; +import org.springframework.context.support.GenericXmlApplicationContext; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.cas.ServiceProperties; import org.springframework.security.web.util.UrlUtils; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; /** * @@ -32,9 +42,13 @@ public class DefaultServiceAuthenticationDetailsTests { private DefaultServiceAuthenticationDetails details; private MockHttpServletRequest request; private Pattern artifactPattern; + private String casServiceUrl; + + private ConfigurableApplicationContext context; @Before public void setUp() { + casServiceUrl = "https://localhost:8443/j_spring_security_cas"; request = new MockHttpServletRequest(); request.setScheme("https"); request.setServerName("localhost"); @@ -44,45 +58,82 @@ public class DefaultServiceAuthenticationDetailsTests { } - @Test - public void getServiceUrlNullQuery() throws Exception { - details = new DefaultServiceAuthenticationDetails(request,artifactPattern); - assertEquals(UrlUtils.buildFullRequestUrl(request),details.getServiceUrl()); + @After + public void cleanup() { + if(context != null) { + context.close(); + } } @Test - public void getServiceUrlTicketOnlyParam() { + public void getServiceUrlNullQuery() throws Exception { + details = new DefaultServiceAuthenticationDetails(casServiceUrl, request,artifactPattern); + assertEquals(UrlUtils.buildFullRequestUrl(request), details.getServiceUrl()); + } + + @Test + public void getServiceUrlTicketOnlyParam() throws Exception { request.setQueryString("ticket=123"); - details = new DefaultServiceAuthenticationDetails(request,artifactPattern); + details = new DefaultServiceAuthenticationDetails(casServiceUrl,request,artifactPattern); String serviceUrl = details.getServiceUrl(); request.setQueryString(null); assertEquals(UrlUtils.buildFullRequestUrl(request),serviceUrl); } @Test - public void getServiceUrlTicketFirstMultiParam() { + public void getServiceUrlTicketFirstMultiParam() throws Exception { request.setQueryString("ticket=123&other=value"); - details = new DefaultServiceAuthenticationDetails(request,artifactPattern); + details = new DefaultServiceAuthenticationDetails(casServiceUrl, request,artifactPattern); String serviceUrl = details.getServiceUrl(); request.setQueryString("other=value"); assertEquals(UrlUtils.buildFullRequestUrl(request),serviceUrl); } @Test - public void getServiceUrlTicketLastMultiParam() { + public void getServiceUrlTicketLastMultiParam() throws Exception { request.setQueryString("other=value&ticket=123"); - details = new DefaultServiceAuthenticationDetails(request,artifactPattern); + details = new DefaultServiceAuthenticationDetails(casServiceUrl,request,artifactPattern); String serviceUrl = details.getServiceUrl(); request.setQueryString("other=value"); assertEquals(UrlUtils.buildFullRequestUrl(request),serviceUrl); } @Test - public void getServiceUrlTicketMiddleMultiParam() { + public void getServiceUrlTicketMiddleMultiParam() throws Exception { request.setQueryString("other=value&ticket=123&last=this"); - details = new DefaultServiceAuthenticationDetails(request,artifactPattern); + details = new DefaultServiceAuthenticationDetails(casServiceUrl,request,artifactPattern); String serviceUrl = details.getServiceUrl(); request.setQueryString("other=value&last=this"); assertEquals(UrlUtils.buildFullRequestUrl(request),serviceUrl); } + + @Test + public void getServiceUrlDoesNotUseHostHeader() throws Exception { + casServiceUrl = "https://example.com/j_spring_security_cas"; + request.setServerName("evil.com"); + details = new DefaultServiceAuthenticationDetails(casServiceUrl, request,artifactPattern); + assertEquals("https://example.com/cas-sample/secure/",details.getServiceUrl()); + } + + @Test + public void getServiceUrlDoesNotUseHostHeaderPassivity() { + casServiceUrl = "https://example.com/j_spring_security_cas"; + request.setServerName("evil.com"); + ServiceAuthenticationDetails details = loadServiceAuthenticationDetails("defaultserviceauthenticationdetails-passivity.xml"); + assertEquals("https://example.com/cas-sample/secure/", details.getServiceUrl()); + } + + @Test + public void getServiceUrlDoesNotUseHostHeaderExplicit() { + casServiceUrl = "https://example.com/j_spring_security_cas"; + request.setServerName("evil.com"); + ServiceAuthenticationDetails details = loadServiceAuthenticationDetails("defaultserviceauthenticationdetails-explicit.xml"); + assertEquals("https://example.com/cas-sample/secure/", details.getServiceUrl()); + } + + private ServiceAuthenticationDetails loadServiceAuthenticationDetails(String resourceName) { + context = new GenericXmlApplicationContext(getClass(), resourceName); + ServiceAuthenticationDetailsSource source = context.getBean(ServiceAuthenticationDetailsSource.class); + return source.buildDetails(request); + } } diff --git a/cas/src/test/resources/org/springframework/security/cas/web/authentication/defaultserviceauthenticationdetails-explicit.xml b/cas/src/test/resources/org/springframework/security/cas/web/authentication/defaultserviceauthenticationdetails-explicit.xml new file mode 100644 index 0000000000..ebddb43e82 --- /dev/null +++ b/cas/src/test/resources/org/springframework/security/cas/web/authentication/defaultserviceauthenticationdetails-explicit.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/cas/src/test/resources/org/springframework/security/cas/web/authentication/defaultserviceauthenticationdetails-passivity.xml b/cas/src/test/resources/org/springframework/security/cas/web/authentication/defaultserviceauthenticationdetails-passivity.xml new file mode 100644 index 0000000000..0501a4f6ec --- /dev/null +++ b/cas/src/test/resources/org/springframework/security/cas/web/authentication/defaultserviceauthenticationdetails-passivity.xml @@ -0,0 +1,15 @@ + + + + + + + + + + \ No newline at end of file diff --git a/cas/template.mf b/cas/template.mf index 0b20a5b55c..3eb88c2517 100644 --- a/cas/template.mf +++ b/cas/template.mf @@ -14,7 +14,7 @@ Import-Template: org.springframework.security.core.*;version="${secRange}", org.springframework.security.authentication.*;version="${secRange}", org.springframework.security.web.*;version="${secRange}", - org.springframework.beans.factory;version="${springRange}", + org.springframework.beans.*;version="${springRange}", org.springframework.cache.*;version="${springRange}";resolution:=optional, org.springframework.context.*;version="${springRange}", org.springframework.dao;version="${springRange}", diff --git a/docs/manual/src/asciidoc/index.adoc b/docs/manual/src/asciidoc/index.adoc index 3c4f4e7eec..e7bfed7e59 100644 --- a/docs/manual/src/asciidoc/index.adoc +++ b/docs/manual/src/asciidoc/index.adoc @@ -5559,7 +5559,9 @@ The next step is to specify `serviceProperties` and the `authenticationDetailsSo + "org.springframework.security.cas.web.authentication.ServiceAuthenticationDetailsSource"> + + ----