From 6e12f43b16c8139d8e626832e9acf696dc991a1c Mon Sep 17 00:00:00 2001 From: "Marvin S. Addison" Date: Mon, 4 Mar 2013 10:43:55 -0500 Subject: [PATCH] CASC-204 Prevent renew misconfiguration. Prevent renew from being configured via filter init param, which can lead to a half-configured state where authentication filter is configured for renew without validation filter. With this change in place, renew MUST be configured by a global configuration facility such as context parameter or JNDI to ensure proper configuration. --- .../util/AbstractConfigurationFilter.java | 4 ++ .../AuthenticationFilterTests.java | 43 +++++++++++--- .../Cas10TicketValidationFilterTests.java | 58 +++++++++++++++++++ ...yReceivingTicketValidationFilterTests.java | 30 +++++++++- .../Saml11TicketValidationFilterTests.java | 58 +++++++++++++++++++ 5 files changed, 183 insertions(+), 10 deletions(-) create mode 100644 cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas10TicketValidationFilterTests.java create mode 100644 cas-client-core/src/test/java/org/jasig/cas/client/validation/Saml11TicketValidationFilterTests.java diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractConfigurationFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractConfigurationFilter.java index e9e2183..e242510 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractConfigurationFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractConfigurationFilter.java @@ -67,6 +67,10 @@ public abstract class AbstractConfigurationFilter implements Filter { final String value = filterConfig.getInitParameter(propertyName); if (CommonUtils.isNotBlank(value)) { + if ("renew".equals(propertyName)) { + throw new IllegalArgumentException( + "Renew MUST be specified via context parameter or JNDI environment to avoid misconfiguration."); + } logger.info("Property [{}] loaded from FilterConfig.getInitParameter with value [{}]", propertyName, value); return value; } diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/authentication/AuthenticationFilterTests.java b/cas-client-core/src/test/java/org/jasig/cas/client/authentication/AuthenticationFilterTests.java index 9b41a74..b2a520b 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/authentication/AuthenticationFilterTests.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/authentication/AuthenticationFilterTests.java @@ -18,6 +18,15 @@ */ package org.jasig.cas.client.authentication; +import java.io.IOException; +import java.lang.reflect.Field; +import java.net.URLEncoder; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; + import junit.framework.TestCase; import org.jasig.cas.client.util.AbstractCasFilter; import org.jasig.cas.client.validation.AssertionImpl; @@ -25,13 +34,7 @@ import org.springframework.mock.web.MockFilterConfig; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockHttpSession; - -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import java.io.IOException; -import java.net.URLEncoder; +import org.springframework.mock.web.MockServletContext; /** * Tests for the AuthenticationFilter. @@ -177,4 +180,30 @@ public final class AuthenticationFilterTests extends TestCase { assertNull(session.getAttribute(DefaultGatewayResolverImpl.CONST_CAS_GATEWAY)); assertNull(response2.getRedirectedUrl()); } + + public void testRenewInitParamThrows() throws Exception { + final AuthenticationFilter f = new AuthenticationFilter(); + final MockFilterConfig config = new MockFilterConfig(); + config.addInitParameter("casServerLoginUrl", CAS_LOGIN_URL); + config.addInitParameter("service", "https://localhost:8443/service"); + config.addInitParameter("renew", "true"); + try { + f.init(config); + fail("Should have thrown IllegalArgumentException."); + } catch (final IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Renew MUST")); + } + } + + public void testAllowsRenewContextParam() throws Exception { + final AuthenticationFilter f = new AuthenticationFilter(); + final MockServletContext context = new MockServletContext(); + context.addInitParameter("casServerLoginUrl", "https://cas.example.com/login"); + context.addInitParameter("service", "https://localhost:8443/service"); + context.addInitParameter("renew", "true"); + f.init(new MockFilterConfig(context)); + final Field renewField = AuthenticationFilter.class.getDeclaredField("renew"); + renewField.setAccessible(true); + assertTrue((Boolean) renewField.get(f)); + } } diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas10TicketValidationFilterTests.java b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas10TicketValidationFilterTests.java new file mode 100644 index 0000000..e15e7f2 --- /dev/null +++ b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas10TicketValidationFilterTests.java @@ -0,0 +1,58 @@ +/* + * Licensed to Jasig under one or more contributor license + * agreements. See the NOTICE file distributed with this work + * for additional information regarding copyright ownership. + * Jasig licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a + * copy of the License at the following location: + * * + * http://www.apache.org/licenses/LICENSE-2.0 + * * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.jasig.cas.client.validation; + +import org.junit.Test; +import org.springframework.mock.web.MockFilterConfig; +import org.springframework.mock.web.MockServletContext; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Unit test for {@link Cas10TicketValidationFilter}. + * + * @author Marvin S. Addison + */ +public class Cas10TicketValidationFilterTests { + @Test + public void testThrowsRenewInitParam() throws Exception { + final Cas10TicketValidationFilter f = new Cas10TicketValidationFilter(); + final MockFilterConfig config = new MockFilterConfig(); + config.addInitParameter("casServerUrlPrefix", "https://cas.example.com"); + config.addInitParameter("renew", "true"); + try { + f.init(config); + fail("Should have thrown IllegalArgumentException."); + } catch (final IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Renew MUST")); + } + } + + @Test + public void testAllowsRenewContextParam() throws Exception { + final Cas10TicketValidationFilter f = new Cas10TicketValidationFilter(); + final MockServletContext context = new MockServletContext(); + context.addInitParameter("casServerUrlPrefix", "https://cas.example.com"); + context.addInitParameter("renew", "true"); + final TicketValidator validator = f.getTicketValidator(new MockFilterConfig(context)); + assertTrue(validator instanceof Cas10TicketValidator); + assertTrue(((Cas10TicketValidator) validator).isRenew()); + } +} diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilterTests.java b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilterTests.java index 073ad3f..eeb59dd 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilterTests.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilterTests.java @@ -18,15 +18,16 @@ */ package org.jasig.cas.client.validation; +import java.util.Timer; +import java.util.TimerTask; + import junit.framework.TestCase; import org.jasig.cas.client.proxy.CleanUpTimerTask; import org.jasig.cas.client.proxy.ProxyGrantingTicketStorage; import org.jasig.cas.client.proxy.ProxyGrantingTicketStorageImpl; import org.jasig.cas.client.util.MethodFlag; import org.springframework.mock.web.MockFilterConfig; - -import java.util.Timer; -import java.util.TimerTask; +import org.springframework.mock.web.MockServletContext; /** * Unit test for {@link org.jasig.cas.client.validation.Cas20ProxyReceivingTicketValidationFilter} @@ -179,6 +180,29 @@ public void testCallsCleanAllOnSchedule() throws Exception { assertNotNull(filter.getTicketValidator(config3)); } + public void testRenewInitParamThrows() throws Exception { + final Cas20ProxyReceivingTicketValidationFilter f = new Cas20ProxyReceivingTicketValidationFilter(); + final MockFilterConfig config = new MockFilterConfig(); + config.addInitParameter("casServerUrlPrefix", "https://cas.example.com"); + config.addInitParameter("renew", "true"); + try { + f.init(config); + fail("Should have thrown IllegalArgumentException."); + } catch (final IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Renew MUST")); + } + } + + public void testAllowsRenewContextParam() throws Exception { + final Cas20ProxyReceivingTicketValidationFilter f = new Cas20ProxyReceivingTicketValidationFilter(); + final MockServletContext context = new MockServletContext(); + context.addInitParameter("casServerUrlPrefix", "https://cas.example.com"); + context.addInitParameter("renew", "true"); + final TicketValidator validator = f.getTicketValidator(new MockFilterConfig(context)); + assertTrue(validator instanceof AbstractUrlBasedTicketValidator); + assertTrue(((AbstractUrlBasedTicketValidator) validator).isRenew()); + } + /** * construct a working {@link org.jasig.cas.client.validation.Cas20ProxyReceivingTicketValidationFilter} */ diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/validation/Saml11TicketValidationFilterTests.java b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Saml11TicketValidationFilterTests.java new file mode 100644 index 0000000..cb64c63 --- /dev/null +++ b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Saml11TicketValidationFilterTests.java @@ -0,0 +1,58 @@ +/* + * Licensed to Jasig under one or more contributor license + * agreements. See the NOTICE file distributed with this work + * for additional information regarding copyright ownership. + * Jasig licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a + * copy of the License at the following location: + * * + * http://www.apache.org/licenses/LICENSE-2.0 + * * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.jasig.cas.client.validation; + +import org.junit.Test; +import org.springframework.mock.web.MockFilterConfig; +import org.springframework.mock.web.MockServletContext; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Unit test for {@link Saml11TicketValidationFilter}. + * + * @author Marvin S. Addison + */ +public class Saml11TicketValidationFilterTests { + @Test + public void testRenewInitParamThrows() throws Exception { + final Saml11TicketValidationFilter f = new Saml11TicketValidationFilter(); + final MockFilterConfig config = new MockFilterConfig(); + config.addInitParameter("casServerUrlPrefix", "https://cas.example.com"); + config.addInitParameter("renew", "true"); + try { + f.init(config); + fail("Should have thrown IllegalArgumentException."); + } catch (final IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Renew MUST")); + } + } + + @Test + public void testAllowsRenewContextParam() throws Exception { + final Saml11TicketValidationFilter f = new Saml11TicketValidationFilter(); + final MockServletContext context = new MockServletContext(); + context.addInitParameter("casServerUrlPrefix", "https://cas.example.com"); + context.addInitParameter("renew", "true"); + final TicketValidator validator = f.getTicketValidator(new MockFilterConfig(context)); + assertTrue(validator instanceof Saml11TicketValidator); + assertTrue(((Saml11TicketValidator) validator).isRenew()); + } +}