CASC-166 Fix race condition in cached assertion cleanup.

Perform assertion cleanup on same thread as JAAS module invocations to ensure that cleanup of expired assertions occurs before the cache is interrogated. A verifying test case accompanies this fix. The test case required a new module option, cacheTimeoutUnits, in order to complete on a time scale suitable for unit tests.
This commit is contained in:
Marvin S. Addison 2012-07-25 16:58:36 -04:00
parent 2b9cec59be
commit ec0c7d5162
3 changed files with 151 additions and 29 deletions

View File

@ -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.</li>
* <li>cacheTimeout (optional) - Assertion cache timeout in minutes.</li>
* <li>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.</li>
* </ul>
*
* <p>
@ -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<TicketCredential,Assertion> ASSERTION_CACHE = new HashMap<TicketCredential,Assertion>();
/** 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:
* <ul>
* <li>{@link Calendar#HOUR}</li>
* <li>{@link Calendar#MINUTE}</li>
* <li>{@link Calendar#SECOND}</li>
* <li>{@link Calendar#MILLISECOND}</li>
* </ul>
*/
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.</li>
* <li>cacheTimeout (optional) - assertion cache timeout in minutes.</li>
* <li>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.</li>
* </ul>
*/
public final void initialize(final Subject subject, final CallbackHandler handler, final Map<String,?> state, final Map<String, ?> 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<Map.Entry<TicketCredential,Assertion>> 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<Map.Entry<TicketCredential, Assertion>> 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 {
}
}
}

View File

@ -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 = "<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'>"
final String SUCCESS_RESPONSE = "<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'>"
+ "<cas:authenticationSuccess><cas:user>"
+ USERNAME
+ "</cas:user></cas:authenticationSuccess></cas:serviceResponse>";
final String RESPONSE2 = "<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'><cas:authenticationFailure code=\"INVALID_TICKET\">Ticket ST-300000-aA5Yuvrxzpv8Tau1cYQ7-srv1 not recognized</cas:authenticationFailure></cas:serviceResponse>";
server.content = RESPONSE1.getBytes(server.encoding);
final String FAILURE_RESPONSE = "<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'><cas:authenticationFailure code=\"INVALID_TICKET\">Ticket ST-300000-aA5Yuvrxzpv8Tau1cYQ7-srv1 not recognized</cas:authenticationFailure></cas:serviceResponse>";
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 = "<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'>"
+ "<cas:authenticationSuccess><cas:user>"
+ USERNAME
+ "</cas:user></cas:authenticationSuccess></cas:serviceResponse>";
final String FAILURE_RESPONSE = "<cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'><cas:authenticationFailure code=\"INVALID_TICKET\">Ticket ST-12345-ABCDEFGHIJKLMNOPQRSTUVWXYZ-hosta not recognized</cas:authenticationFailure></cas:serviceResponse>";
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<String, Object>(),
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<String, Object>(),
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<? extends Principal> principalClass, final String name) {
final Set<? extends Principal> principals = subject.getPrincipals(principalClass);

View File

@ -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