diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/jaas/CasLoginModule.java b/cas-client-core/src/main/java/org/jasig/cas/client/jaas/CasLoginModule.java index 58a3e0e..565d0ec 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/jaas/CasLoginModule.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/jaas/CasLoginModule.java @@ -26,9 +26,14 @@ import java.beans.PropertyDescriptor; import java.io.IOException; import java.security.Principal; import java.security.acl.Group; -import java.util.*; -import java.util.concurrent.Executor; -import java.util.concurrent.Executors; +import java.util.Arrays; +import java.util.Calendar; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Map; +import java.util.Set; import javax.security.auth.Subject; import javax.security.auth.callback.Callback; @@ -79,6 +84,10 @@ import org.jasig.cas.client.validation.TicketValidator; * for JAAS providers that attempt to periodically reauthenticate to renew principal. * Since CAS tickets are one-time-use, a cached assertion must be provided on reauthentication. *
  • cacheTimeout (optional) - Assertion cache timeout in minutes.
    • + *
  • cacheTimeoutUnits (optional) - Assertion cache timeout units. + * MUST be one of following: + * Calendar.HOUR(10), Calendar.MINUTE(12), Calendar.SECOND(13), Calendar.MILLISECOND(14). + * Default unit is minutes.
    • * * *

    @@ -129,6 +138,9 @@ public class CasLoginModule implements LoginModule { */ public static final int DEFAULT_CACHE_TIMEOUT = 480; + /** Default assertion cache timeout unit is minutes. */ + public static final int DEFAULT_CACHE_TIMEOUT_UNITS = Calendar.MINUTE; + /** * Stores mapping of ticket to assertion to support JAAS providers that * attempt to periodically re-authenticate to renew principal. Since @@ -137,9 +149,6 @@ public class CasLoginModule implements LoginModule { */ protected static final Map ASSERTION_CACHE = new HashMap(); - /** Executor responsible for assertion cache cleanup */ - protected static Executor cacheCleanerExecutor = Executors.newSingleThreadExecutor(); - /** Logger instance */ protected final Log log = LogFactory.getLog(getClass()); @@ -182,8 +191,20 @@ public class CasLoginModule implements LoginModule { /** Assertion cache timeout in minutes */ protected int cacheTimeout = DEFAULT_CACHE_TIMEOUT; + /** + * Units of cache timeout. Must be one of following {@link Calendar} time unit constants: + *

    + */ + protected int cacheTimeoutUnits = DEFAULT_CACHE_TIMEOUT_UNITS; + /** * Initializes the CAS login module. + * * @param subject Authentication subject. * @param handler Callback handler. * @param state Shared state map. @@ -201,10 +222,12 @@ public class CasLoginModule implements LoginModule { * which by default are single use, reauthentication fails. Assertion caching addresses this * behavior. *
  • cacheTimeout (optional) - assertion cache timeout in minutes.
    • + *
  • cacheTimeoutUnits (optional) - Assertion cache timeout units. + * MUST be one of following: + * Calendar.HOUR(10), Calendar.MINUTE(12), Calendar.SECOND(13), Calendar.MILLISECOND(14). + * Default unit is minutes.
    • * */ - - public final void initialize(final Subject subject, final CallbackHandler handler, final Map state, final Map options) { this.assertion = null; this.callbackHandler = handler; @@ -245,11 +268,20 @@ public class CasLoginModule implements LoginModule { } else if ("cacheTimeout".equals(key)) { this.cacheTimeout = Integer.parseInt((String) options.get(key)); log.debug("Set cacheTimeout=" + this.cacheTimeout); + } else if ("cacheTimeoutUnits".equals(key)) { + final int units = Integer.parseInt((String) options.get(key)); + if (units == Calendar.HOUR || units == Calendar.MINUTE || + units == Calendar.SECOND || units == Calendar.MILLISECOND) { + this.cacheTimeoutUnits = units; + log.debug("Set cacheTimeoutUnits=" + this.cacheTimeoutUnits); + } else { + throw new IllegalArgumentException("Invalid time unit constant " + units); + } } } if (this.cacheAssertions) { - cacheCleanerExecutor.execute(new CacheCleaner()); + cleanCache(); } CommonUtils.assertNotNull(ticketValidatorClass, "ticketValidatorClass is required."); @@ -301,7 +333,8 @@ public class CasLoginModule implements LoginModule { final String service = CommonUtils.isNotBlank(serviceCallback.getName()) ? serviceCallback.getName() : this.service; if (this.cacheAssertions) { - synchronized(ASSERTION_CACHE) { + // Multiple threads may be accessing the static cache concurrently + synchronized (ASSERTION_CACHE) { if (ASSERTION_CACHE.get(ticket) != null) { log.debug("Assertion found in cache."); this.assertion = ASSERTION_CACHE.get(ticket); @@ -425,7 +458,10 @@ public class CasLoginModule implements LoginModule { if (log.isDebugEnabled()) { log.debug("Caching assertion for principal " + this.assertion.getPrincipal()); } - ASSERTION_CACHE.put(this.ticket, this.assertion); + // Multiple threads may be accessing the static cache concurrently + synchronized (ASSERTION_CACHE) { + ASSERTION_CACHE.put(this.ticket, this.assertion); + } } } else { // Login must have failed if there is no assertion defined @@ -554,23 +590,26 @@ public class CasLoginModule implements LoginModule { this.subject.getPrivateCredentials().removeAll(this.subject.getPrivateCredentials(clazz)); } - /** Removes expired entries from the assertion cache. */ - private class CacheCleaner implements Runnable { - public void run() { - if (log.isDebugEnabled()) { - log.debug("Cleaning assertion cache of size " + CasLoginModule.ASSERTION_CACHE.size()); - } - final Iterator> iter = - CasLoginModule.ASSERTION_CACHE.entrySet().iterator(); + + /** + * Removes expired entries from the assertion cache. + */ + private void cleanCache() { + if (log.isDebugEnabled()) { + log.debug("Cleaning assertion cache of size " + ASSERTION_CACHE.size()); + } + // Multiple threads may be accessing the static cache concurrently + synchronized (ASSERTION_CACHE) { + final Iterator> iter = ASSERTION_CACHE.entrySet().iterator(); final Calendar cutoff = Calendar.getInstance(); - cutoff.add(Calendar.MINUTE, -CasLoginModule.this.cacheTimeout); + cutoff.add(this.cacheTimeoutUnits, -this.cacheTimeout); while (iter.hasNext()) { final Assertion assertion = iter.next().getValue(); final Calendar created = Calendar.getInstance(); created.setTime(assertion.getValidFromDate()); if (created.before(cutoff)) { if (log.isDebugEnabled()) { - log.debug("Removing expired assertion for principal " + assertion.getPrincipal()); + log.debug("Removing expired assertion for principal " + assertion.getPrincipal()); } iter.remove(); } @@ -578,3 +617,4 @@ public class CasLoginModule implements LoginModule { } } } + diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/jaas/CasLoginModuleTests.java b/cas-client-core/src/test/java/org/jasig/cas/client/jaas/CasLoginModuleTests.java index 623730a..1f84d23 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/jaas/CasLoginModuleTests.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/jaas/CasLoginModuleTests.java @@ -28,13 +28,16 @@ import java.util.Set; import javax.security.auth.Subject; import javax.security.auth.login.LoginException; -import static org.junit.Assert.*; - import org.jasig.cas.client.PublicTestHttpServer; -import org.junit.AfterClass; +import org.jasig.cas.client.validation.TicketValidationException; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + /** * Unit test for {@link CasLoginModule} class. * @@ -150,15 +153,16 @@ public class CasLoginModuleTests { final String USERNAME = "username"; final String SERVICE = "https://example.com/service"; final String TICKET = "ST-300000-aA5Yuvrxzpv8Tau1cYQ7-srv1"; - final String RESPONSE1 = "" + final String SUCCESS_RESPONSE = "" + "" + USERNAME + ""; - final String RESPONSE2 = "Ticket ST-300000-aA5Yuvrxzpv8Tau1cYQ7-srv1 not recognized"; - server.content = RESPONSE1.getBytes(server.encoding); - + final String FAILURE_RESPONSE = "Ticket ST-300000-aA5Yuvrxzpv8Tau1cYQ7-srv1 not recognized"; + options.put("cacheAssertions", "true"); options.put("cacheTimeout", "1"); + + server.content = SUCCESS_RESPONSE.getBytes(server.encoding); module.initialize( subject, new ServiceAndTicketCallbackHandler(SERVICE, TICKET), @@ -173,7 +177,7 @@ public class CasLoginModuleTests { module.logout(); assertEquals(0, subject.getPrincipals().size()); assertEquals(0, subject.getPrivateCredentials().size()); - server.content = RESPONSE2.getBytes(server.encoding); + server.content = FAILURE_RESPONSE.getBytes(server.encoding); module.initialize( subject, new ServiceAndTicketCallbackHandler(SERVICE, TICKET), @@ -184,6 +188,54 @@ public class CasLoginModuleTests { assertEquals(this.subject.getPrincipals().size(), 3); assertEquals(TICKET, this.subject.getPrivateCredentials().iterator().next().toString()); } + + + /** + * Verify that cached assertions that are expired are never be accessible + * by {@link org.jasig.cas.client.jaas.CasLoginModule#login()} method. + * + * @throws Exception On errors. + */ + @Test + public void testAssertionCachingExpiration() throws Exception { + final String USERNAME = "hizzy"; + final String SERVICE = "https://example.com/service"; + final String TICKET = "ST-12345-ABCDEFGHIJKLMNOPQRSTUVWXYZ-hosta"; + final String SUCCESS_RESPONSE = "" + + "" + + USERNAME + + ""; + final String FAILURE_RESPONSE = "Ticket ST-12345-ABCDEFGHIJKLMNOPQRSTUVWXYZ-hosta not recognized"; + + options.put("cacheAssertions", "true"); + // Cache timeout is 1 second (Calendar.SECOND=13) + options.put("cacheTimeoutUnits", "13"); + options.put("cacheTimeout", "1"); + + server.content = SUCCESS_RESPONSE.getBytes(server.encoding); + module.initialize( + subject, + new ServiceAndTicketCallbackHandler(SERVICE, TICKET), + new HashMap(), + options); + assertTrue(module.login()); + module.commit(); + + Thread.sleep(1100); + // Assertion should now be expired from cache + server.content = FAILURE_RESPONSE.getBytes(server.encoding); + module.initialize( + subject, + new ServiceAndTicketCallbackHandler(SERVICE, TICKET), + new HashMap(), + options); + try { + module.login(); + fail("Should have thrown login exception."); + } catch (LoginException e) { + assertTrue(e.getCause() instanceof TicketValidationException); + } + } private boolean hasPrincipalName(final Subject subject, final Class principalClass, final String name) { final Set principals = subject.getPrincipals(principalClass); diff --git a/cas-client-core/src/test/resources/log4j.properties b/cas-client-core/src/test/resources/log4j.properties new file mode 100644 index 0000000..8ccad3b --- /dev/null +++ b/cas-client-core/src/test/resources/log4j.properties @@ -0,0 +1,30 @@ +# +# log4j configuration to get clean console listing during Maven tests +# + +# +# 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. +# + +log4j.rootCategory=INFO, stdout +log4j.logger.org.apache.xml.security=OFF +log4j.appender.stdout=org.apache.log4j.ConsoleAppender +log4j.appender.stdout.layout=org.apache.log4j.PatternLayout +log4j.appender.stdout.layout.ConversionPattern=%-5p %d{ISO8601} %t::%c{1} - %m%n + +log4j.logger.org.jasig.cas=DEBUG