From 2c9090155afba75b9f2d94717e97e666e896b686 Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Tue, 6 May 2014 22:35:55 -0400 Subject: [PATCH 1/3] CASC-223 SingleSignOutFilter requires init method to be called which changes the contract with previous versions Problem: Some clients such as Spring Security configure the filter via Spring configuration, meaning the handler's init method is not called vai the Filter#init method. Solution: For now, have an atomic boolean to determine if init was called or not and call it if necessary as part of the flow. --- .../client/session/SingleSignOutFilter.java | 46 ++++++++++++------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/session/SingleSignOutFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/session/SingleSignOutFilter.java index 33b2094eb..c9243e95b 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/session/SingleSignOutFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/session/SingleSignOutFilter.java @@ -19,6 +19,7 @@ package org.jasig.cas.client.session; import java.io.IOException; +import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.*; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -34,49 +35,52 @@ */ public final class SingleSignOutFilter extends AbstractConfigurationFilter { - private static final SingleSignOutHandler handler = new SingleSignOutHandler(); + private static final SingleSignOutHandler HANDLER = new SingleSignOutHandler(); + + private AtomicBoolean handlerInitialized = new AtomicBoolean(false); public void init(final FilterConfig filterConfig) throws ServletException { if (!isIgnoreInitConfiguration()) { - handler.setArtifactParameterName(getPropertyFromInitParams(filterConfig, "artifactParameterName", + HANDLER.setArtifactParameterName(getPropertyFromInitParams(filterConfig, "artifactParameterName", SingleSignOutHandler.DEFAULT_ARTIFACT_PARAMETER_NAME)); - handler.setLogoutParameterName(getPropertyFromInitParams(filterConfig, "logoutParameterName", + HANDLER.setLogoutParameterName(getPropertyFromInitParams(filterConfig, "logoutParameterName", SingleSignOutHandler.DEFAULT_LOGOUT_PARAMETER_NAME)); - handler.setFrontLogoutParameterName(getPropertyFromInitParams(filterConfig, "frontLogoutParameterName", + HANDLER.setFrontLogoutParameterName(getPropertyFromInitParams(filterConfig, "frontLogoutParameterName", SingleSignOutHandler.DEFAULT_FRONT_LOGOUT_PARAMETER_NAME)); - handler.setRelayStateParameterName(getPropertyFromInitParams(filterConfig, "relayStateParameterName", + HANDLER.setRelayStateParameterName(getPropertyFromInitParams(filterConfig, "relayStateParameterName", SingleSignOutHandler.DEFAULT_RELAY_STATE_PARAMETER_NAME)); - handler.setCasServerUrlPrefix(getPropertyFromInitParams(filterConfig, "casServerUrlPrefix", null)); - handler.setArtifactParameterOverPost(parseBoolean(getPropertyFromInitParams(filterConfig, + HANDLER.setCasServerUrlPrefix(getPropertyFromInitParams(filterConfig, "casServerUrlPrefix", null)); + HANDLER.setArtifactParameterOverPost(parseBoolean(getPropertyFromInitParams(filterConfig, "artifactParameterOverPost", "false"))); - handler.setEagerlyCreateSessions(parseBoolean(getPropertyFromInitParams(filterConfig, + HANDLER.setEagerlyCreateSessions(parseBoolean(getPropertyFromInitParams(filterConfig, "eagerlyCreateSessions", "true"))); } - handler.init(); + HANDLER.init(); + handlerInitialized.set(true); } public void setArtifactParameterName(final String name) { - handler.setArtifactParameterName(name); + HANDLER.setArtifactParameterName(name); } public void setLogoutParameterName(final String name) { - handler.setLogoutParameterName(name); + HANDLER.setLogoutParameterName(name); } public void setFrontLogoutParameterName(final String name) { - handler.setFrontLogoutParameterName(name); + HANDLER.setFrontLogoutParameterName(name); } public void setRelayStateParameterName(final String name) { - handler.setRelayStateParameterName(name); + HANDLER.setRelayStateParameterName(name); } public void setCasServerUrlPrefix(final String casServerUrlPrefix) { - handler.setCasServerUrlPrefix(casServerUrlPrefix); + HANDLER.setCasServerUrlPrefix(casServerUrlPrefix); } public void setSessionMappingStorage(final SessionMappingStorage storage) { - handler.setSessionMappingStorage(storage); + HANDLER.setSessionMappingStorage(storage); } public void doFilter(final ServletRequest servletRequest, final ServletResponse servletResponse, @@ -84,7 +88,15 @@ public void doFilter(final ServletRequest servletRequest, final ServletResponse final HttpServletRequest request = (HttpServletRequest) servletRequest; final HttpServletResponse response = (HttpServletResponse) servletResponse; - if (handler.process(request, response)) { + /** + *

Workaround for now for the fact that Spring Security will fail since it doesn't call {@link #init(javax.servlet.FilterConfig)}.

+ *

Ultimately we need to allow deployers to actually inject their fully-initialized {@link org.jasig.cas.client.session.SingleSignOutHandler}.

+ */ + if (!this.handlerInitialized.getAndSet(true)) { + HANDLER.init(); + } + + if (HANDLER.process(request, response)) { filterChain.doFilter(servletRequest, servletResponse); } } @@ -94,6 +106,6 @@ public void destroy() { } protected static SingleSignOutHandler getSingleSignOutHandler() { - return handler; + return HANDLER; } } From a4e984e4ea9e98d6b7bc014c20a11c5b5b7583d8 Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Tue, 6 May 2014 22:53:05 -0400 Subject: [PATCH 2/3] Synchronize the init method in case we have multiple concurrent requests at the same time. --- .../client/session/SingleSignOutHandler.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/session/SingleSignOutHandler.java b/cas-client-core/src/main/java/org/jasig/cas/client/session/SingleSignOutHandler.java index c93d9b30d..5f8b46c5d 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/session/SingleSignOutHandler.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/session/SingleSignOutHandler.java @@ -132,18 +132,20 @@ public void setEagerlyCreateSessions(final boolean eagerlyCreateSessions) { /** * Initializes the component for use. */ - public void init() { - CommonUtils.assertNotNull(this.artifactParameterName, "artifactParameterName cannot be null."); - CommonUtils.assertNotNull(this.logoutParameterName, "logoutParameterName cannot be null."); - CommonUtils.assertNotNull(this.frontLogoutParameterName, "frontLogoutParameterName cannot be null."); - CommonUtils.assertNotNull(this.sessionMappingStorage, "sessionMappingStorage cannot be null."); - CommonUtils.assertNotNull(this.relayStateParameterName, "relayStateParameterName cannot be null."); - CommonUtils.assertNotNull(this.casServerUrlPrefix, "casServerUrlPrefix cannot be null."); - - if (this.artifactParameterOverPost) { - this.safeParameters = Arrays.asList(this.logoutParameterName, this.artifactParameterName); - } else { - this.safeParameters = Arrays.asList(this.logoutParameterName); + public synchronized void init() { + if (this.safeParameters == null) { + CommonUtils.assertNotNull(this.artifactParameterName, "artifactParameterName cannot be null."); + CommonUtils.assertNotNull(this.logoutParameterName, "logoutParameterName cannot be null."); + CommonUtils.assertNotNull(this.frontLogoutParameterName, "frontLogoutParameterName cannot be null."); + CommonUtils.assertNotNull(this.sessionMappingStorage, "sessionMappingStorage cannot be null."); + CommonUtils.assertNotNull(this.relayStateParameterName, "relayStateParameterName cannot be null."); + CommonUtils.assertNotNull(this.casServerUrlPrefix, "casServerUrlPrefix cannot be null."); + + if (this.artifactParameterOverPost) { + this.safeParameters = Arrays.asList(this.logoutParameterName, this.artifactParameterName); + } else { + this.safeParameters = Arrays.asList(this.logoutParameterName); + } } } From a44b4c1229ff6e9b584b6c2b139d74dbbcfd1261 Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Tue, 6 May 2014 23:17:32 -0400 Subject: [PATCH 3/3] Removed no-longer valid test. --- .../jasig/cas/client/session/SingleSignOutFilterTests.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/session/SingleSignOutFilterTests.java b/cas-client-core/src/test/java/org/jasig/cas/client/session/SingleSignOutFilterTests.java index 64114f9b9..7c91f8f10 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/session/SingleSignOutFilterTests.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/session/SingleSignOutFilterTests.java @@ -60,12 +60,6 @@ public void setUp() throws Exception { response = new MockHttpServletResponse(); filterChain = new MockFilterChain(); } - - @Test(expected = IllegalArgumentException.class) - public void initWithoutCasServerUrlPrefix() throws ServletException { - filter = new SingleSignOutFilter(); - filter.init(new MockFilterConfig()); - } @Test public void tokenRequest() throws IOException, ServletException {