From ace712b5a72df009747f5e4996d613814bd4c11b Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Tue, 10 Jul 2012 14:52:20 -0700 Subject: [PATCH 001/103] CASC-182 - reset the redirectAfterValidation parameter to false when useSession is false. --- .../client/validation/AbstractTicketValidationFilter.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java index 71b62e7ed..e29a321d7 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java @@ -103,6 +103,13 @@ protected void initInternal(final FilterConfig filterConfig) throws ServletExcep log.trace("Setting redirectAfterValidation parameter: " + this.redirectAfterValidation); setUseSession(parseBoolean(getPropertyFromInitParams(filterConfig, "useSession", "true"))); log.trace("Setting useSession parameter: " + this.useSession); + + if (!this.useSession && this.redirectAfterValidation) { + log.warn("redirectAfterValidation parameter may not be true when useSession parameter is false."); + setRedirectAfterValidation(false); + log.warn("Setting redirectAfterValidation parameter to " + this.redirectAfterValidation); + } + setTicketValidator(getTicketValidator(filterConfig)); super.initInternal(filterConfig); } From 6b3590ce88f180ec7923cc78f3041c9fee9f6167 Mon Sep 17 00:00:00 2001 From: Misagh Moayyed Date: Wed, 11 Jul 2012 10:12:20 -0700 Subject: [PATCH 002/103] CASC-182: Combined logging statements into one. --- .../cas/client/validation/AbstractTicketValidationFilter.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java index e29a321d7..e9e1c5c54 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java @@ -105,9 +105,8 @@ protected void initInternal(final FilterConfig filterConfig) throws ServletExcep log.trace("Setting useSession parameter: " + this.useSession); if (!this.useSession && this.redirectAfterValidation) { - log.warn("redirectAfterValidation parameter may not be true when useSession parameter is false."); + log.warn("redirectAfterValidation parameter may not be true when useSession parameter is false. Resetting it to false in order to prevent infinite redirects."); setRedirectAfterValidation(false); - log.warn("Setting redirectAfterValidation parameter to " + this.redirectAfterValidation); } setTicketValidator(getTicketValidator(filterConfig)); From 67999a7bf26ad2747dbd1a57058805f9a6e0f4fd Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Mon, 23 Jul 2012 22:09:09 -0400 Subject: [PATCH 003/103] CASC-184 upgrade to OpenSAML2 --- cas-client-core/pom.xml | 6 +- .../validation/Saml11TicketValidator.java | 119 ++++++++++-------- 2 files changed, 70 insertions(+), 55 deletions(-) diff --git a/cas-client-core/pom.xml b/cas-client-core/pom.xml index 2b25337bb..752b4bf71 100644 --- a/cas-client-core/pom.xml +++ b/cas-client-core/pom.xml @@ -24,10 +24,9 @@ org.opensaml opensaml - 1.1 + ${opensaml.version} jar - provided - true + compile @@ -90,5 +89,6 @@ 2.5.6.SEC01 + 2.5.1-1 diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java index 117cbe3ec..9213f9149 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java @@ -22,7 +22,18 @@ import org.jasig.cas.client.authentication.AttributePrincipal; import org.jasig.cas.client.authentication.AttributePrincipalImpl; import org.jasig.cas.client.util.CommonUtils; +import org.joda.time.DateTime; import org.opensaml.*; +import org.opensaml.common.IdentifierGenerator; +import org.opensaml.common.impl.SecureRandomIdentifierGenerator; +import org.opensaml.saml1.core.*; +import org.opensaml.xml.io.Unmarshaller; +import org.opensaml.xml.io.UnmarshallerFactory; +import org.opensaml.xml.io.UnmarshallingException; +import org.opensaml.xml.parse.BasicParserPool; +import org.opensaml.xml.parse.XMLParserException; +import org.w3c.dom.Document; +import org.w3c.dom.Element; import java.io.*; import java.net.HttpURLConnection; @@ -44,8 +55,21 @@ public final class Saml11TicketValidator extends AbstractUrlBasedTicketValidator /** Time tolerance to allow for time drifting. */ private long tolerance = 1000L; + private final BasicParserPool basicParserPool; + + private final IdentifierGenerator identifierGenerator; + + public Saml11TicketValidator(final String casServerUrlPrefix) { super(casServerUrlPrefix); + try { + DefaultBootstrap.bootstrap(); + this.basicParserPool = new BasicParserPool(); + this.basicParserPool.setNamespaceAware(true); + this.identifierGenerator = new SecureRandomIdentifierGenerator(); + } catch (final Exception e) { + throw new RuntimeException(e); + } } protected String getUrlSuffix() { @@ -62,9 +86,7 @@ protected void populateUrlAttributeMap(final Map urlParameters) @Override protected void setDisableXmlSchemaValidation(final boolean disabled) { if (disabled) { - // according to our reading of the SAML 1.1 code, this should disable the schema checking. However, there may be a couple - // of error messages that slip through on start up! - XML.parserPool.setDefaultSchemas(null, null); + this.basicParserPool.setSchema(null); } } @@ -80,69 +102,76 @@ protected Assertion parseResponseFromServer(final String response) throws Ticket try { final String removeStartOfSoapBody = response.substring(response.indexOf("") + 15); final String removeEndOfSoapBody = removeStartOfSoapBody.substring(0, removeStartOfSoapBody.indexOf("")); - final SAMLResponse samlResponse = new SAMLResponse(new ByteArrayInputStream(getBytes(removeEndOfSoapBody))); + final Document responseDocument = this.basicParserPool.parse(new ByteArrayInputStream(getBytes(removeEndOfSoapBody))); + final Element responseRoot = responseDocument.getDocumentElement(); + final UnmarshallerFactory unmarshallerFactory = Configuration.getUnmarshallerFactory(); + final Unmarshaller unmarshaller = unmarshallerFactory.getUnmarshaller(responseRoot); - if (!samlResponse.getAssertions().hasNext()) { + final Response samlResponse = (Response) unmarshaller.unmarshall(responseRoot); + + final List assertions = samlResponse.getAssertions(); + if (assertions.isEmpty()) { throw new TicketValidationException("No assertions found."); } - for (final Iterator iter = samlResponse.getAssertions(); iter.hasNext();) { - final SAMLAssertion assertion = (SAMLAssertion) iter.next(); + for (final org.opensaml.saml1.core.Assertion assertion : assertions) { if (!isValidAssertion(assertion)) { continue; } - final SAMLAuthenticationStatement authenticationStatement = getSAMLAuthenticationStatement(assertion); + final AuthenticationStatement authenticationStatement = getSAMLAuthenticationStatement(assertion); if (authenticationStatement == null) { throw new TicketValidationException("No AuthentiationStatement found in SAML Assertion."); } - final SAMLSubject subject = authenticationStatement.getSubject(); + final Subject subject = authenticationStatement.getSubject(); if (subject == null) { throw new TicketValidationException("No Subject found in SAML Assertion."); } - final SAMLAttribute[] attributes = getAttributesFor(assertion, subject); + final List attributes = getAttributesFor(assertion, subject); final Map personAttributes = new HashMap(); - for (final SAMLAttribute samlAttribute : attributes) { + for (final Attribute samlAttribute : attributes) { final List values = getValuesFrom(samlAttribute); - personAttributes.put(samlAttribute.getName(), values.size() == 1 ? values.get(0) : values); + personAttributes.put(samlAttribute.getAttributeName(), values.size() == 1 ? values.get(0) : values); } - final AttributePrincipal principal = new AttributePrincipalImpl(subject.getNameIdentifier().getName(), personAttributes); + final AttributePrincipal principal = new AttributePrincipalImpl(subject.getNameIdentifier().getNameIdentifier(), personAttributes); final Map authenticationAttributes = new HashMap(); - authenticationAttributes.put("samlAuthenticationStatement::authMethod", authenticationStatement.getAuthMethod()); + authenticationAttributes.put("samlAuthenticationStatement::authMethod", authenticationStatement.getAuthenticationMethod()); return new AssertionImpl(principal, authenticationAttributes); } - } catch (final SAMLException e) { + } catch (final UnmarshallingException e) { + throw new TicketValidationException(e); + } catch (final XMLParserException e) { throw new TicketValidationException(e); } throw new TicketValidationException("No Assertion found within valid time range. Either there's a replay of the ticket or there's clock drift. Check tolerance range, or server/client synchronization."); } - private boolean isValidAssertion(final SAMLAssertion assertion) { - final Date notBefore = assertion.getNotBefore(); - final Date notOnOrAfter = assertion.getNotOnOrAfter(); + private boolean isValidAssertion(final org.opensaml.saml1.core.Assertion assertion) { + final DateTime notBefore = assertion.getConditions().getNotBefore(); + final DateTime notOnOrAfter = assertion.getConditions().getNotOnOrAfter(); - if (assertion.getNotBefore() == null || assertion.getNotOnOrAfter() == null) { + if (notBefore == null || notOnOrAfter == null) { log.debug("Assertion has no bounding dates. Will not process."); return false; } final long currentTime = getCurrentTimeInUtc().getTime(); - if (currentTime + tolerance < notBefore.getTime()) { + if (currentTime + tolerance < notBefore.getMillis()) { log.debug("skipping assertion that's not yet valid..."); return false; } - if (notOnOrAfter.getTime() <= currentTime - tolerance) { + if (notOnOrAfter.getMillis() <= currentTime - tolerance) { log.debug("skipping expired assertion..."); return false; } @@ -150,40 +179,32 @@ private boolean isValidAssertion(final SAMLAssertion assertion) { return true; } - private SAMLAuthenticationStatement getSAMLAuthenticationStatement(final SAMLAssertion assertion) { - for (final Iterator iter = assertion.getStatements(); iter.hasNext();) { - final SAMLStatement statement = (SAMLStatement) iter.next(); + private AuthenticationStatement getSAMLAuthenticationStatement(final org.opensaml.saml1.core.Assertion assertion) { + final List statements = assertion.getAuthenticationStatements(); - if (statement instanceof SAMLAuthenticationStatement) { - return (SAMLAuthenticationStatement) statement; - } + if (statements.isEmpty()) { + return null; } - return null; + return statements.get(0); } - private SAMLAttribute[] getAttributesFor(final SAMLAssertion assertion, final SAMLSubject subject) { - final List attributes = new ArrayList(); - for (final Iterator iter = assertion.getStatements(); iter.hasNext();) { - final SAMLStatement statement = (SAMLStatement) iter.next(); - - if (statement instanceof SAMLAttributeStatement) { - final SAMLAttributeStatement attributeStatement = (SAMLAttributeStatement) statement; - // used because SAMLSubject does not implement equals - if (subject.getNameIdentifier().getName().equals(attributeStatement.getSubject().getNameIdentifier().getName())) { - for (final Iterator iter2 = attributeStatement.getAttributes(); iter2.hasNext();) - attributes.add((SAMLAttribute) iter2.next()); - } + private List getAttributesFor(final org.opensaml.saml1.core.Assertion assertion, final Subject subject) { + final List attributes = new ArrayList(); + for (final AttributeStatement attribute : assertion.getAttributeStatements()) { + if (subject.getNameIdentifier().getNameIdentifier().equals(attribute.getSubject().getNameIdentifier().getNameIdentifier())) { + attributes.addAll(attribute.getAttributes()); } } - return attributes.toArray(new SAMLAttribute[attributes.size()]); + return attributes; } - private List getValuesFrom(final SAMLAttribute attribute) { + private List getValuesFrom(final Attribute attribute) { final List list = new ArrayList(); - for (final Iterator iter = attribute.getValues(); iter.hasNext();) { - list.add(iter.next()); + // TODO I'm not actually sure if this is safe! + for (final Object o : attribute.getAttributeValues()) { + list.add(o.toString()); } return list; } @@ -195,16 +216,10 @@ private Date getCurrentTimeInUtc() { } protected String retrieveResponseFromServer(final URL validationUrl, final String ticket) { - - String MESSAGE_TO_SEND; - - try { - MESSAGE_TO_SEND = "" + final String MESSAGE_TO_SEND = "" + "" + ticket + ""; - } catch (final SAMLException e) { - throw new RuntimeException(e); - } + HttpURLConnection conn = null; From 082aafa9ca8df305a77e82967117e9696482009d Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Tue, 24 Jul 2012 22:13:25 -0400 Subject: [PATCH 004/103] CASC-184 improved SAML support. Also execute old AND new XML in the unit tests. cr for the first round of changes: serac --- .../validation/Saml11TicketValidator.java | 51 +++++++++++-------- .../Saml11TicketValidatorTests.java | 45 ++++++++++++++-- 2 files changed, 69 insertions(+), 27 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java index 9213f9149..dcfa8cfe4 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java @@ -23,10 +23,14 @@ import org.jasig.cas.client.authentication.AttributePrincipalImpl; import org.jasig.cas.client.util.CommonUtils; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; +import org.joda.time.Interval; import org.opensaml.*; import org.opensaml.common.IdentifierGenerator; import org.opensaml.common.impl.SecureRandomIdentifierGenerator; import org.opensaml.saml1.core.*; +import org.opensaml.ws.soap.soap11.Envelope; +import org.opensaml.xml.ConfigurationException; import org.opensaml.xml.io.Unmarshaller; import org.opensaml.xml.io.UnmarshallerFactory; import org.opensaml.xml.io.UnmarshallingException; @@ -52,6 +56,15 @@ */ public final class Saml11TicketValidator extends AbstractUrlBasedTicketValidator { + static { + try { + // we really only need to do this once, so this is why its here. + DefaultBootstrap.bootstrap(); + } catch (final ConfigurationException e) { + throw new RuntimeException(e); + } + } + /** Time tolerance to allow for time drifting. */ private long tolerance = 1000L; @@ -62,10 +75,10 @@ public final class Saml11TicketValidator extends AbstractUrlBasedTicketValidator public Saml11TicketValidator(final String casServerUrlPrefix) { super(casServerUrlPrefix); + this.basicParserPool = new BasicParserPool(); + this.basicParserPool.setNamespaceAware(true); + try { - DefaultBootstrap.bootstrap(); - this.basicParserPool = new BasicParserPool(); - this.basicParserPool.setNamespaceAware(true); this.identifierGenerator = new SecureRandomIdentifierGenerator(); } catch (final Exception e) { throw new RuntimeException(e); @@ -100,14 +113,13 @@ protected byte[] getBytes(final String text) { protected Assertion parseResponseFromServer(final String response) throws TicketValidationException { try { - final String removeStartOfSoapBody = response.substring(response.indexOf("") + 15); - final String removeEndOfSoapBody = removeStartOfSoapBody.substring(0, removeStartOfSoapBody.indexOf("")); - final Document responseDocument = this.basicParserPool.parse(new ByteArrayInputStream(getBytes(removeEndOfSoapBody))); + + final Document responseDocument = this.basicParserPool.parse(new ByteArrayInputStream(getBytes(response))); final Element responseRoot = responseDocument.getDocumentElement(); final UnmarshallerFactory unmarshallerFactory = Configuration.getUnmarshallerFactory(); final Unmarshaller unmarshaller = unmarshallerFactory.getUnmarshaller(responseRoot); - - final Response samlResponse = (Response) unmarshaller.unmarshall(responseRoot); + final Envelope envelope = (Envelope) unmarshaller.unmarshall(responseRoot); + final Response samlResponse = (Response) envelope.getBody().getOrderedChildren().get(0); final List assertions = samlResponse.getAssertions(); if (assertions.isEmpty()) { @@ -164,19 +176,21 @@ private boolean isValidAssertion(final org.opensaml.saml1.core.Assertion asserti return false; } - final long currentTime = getCurrentTimeInUtc().getTime(); + final DateTime currentTime = new DateTime(DateTimeZone.UTC); + final Interval validityRange = new Interval(notBefore.minus(this.tolerance), notOnOrAfter.plus(this.tolerance)); - if (currentTime + tolerance < notBefore.getMillis()) { - log.debug("skipping assertion that's not yet valid..."); - return false; + if (validityRange.contains(currentTime)) { + log.debug("Current time is within the interval validity."); + return true; } - if (notOnOrAfter.getMillis() <= currentTime - tolerance) { - log.debug("skipping expired assertion..."); + if (currentTime.isBefore(validityRange.getStart())) { + log.debug("skipping assertion that's not yet valid..."); return false; } - return true; + log.debug("skipping expired assertion..."); + return false; } private AuthenticationStatement getSAMLAuthenticationStatement(final org.opensaml.saml1.core.Assertion assertion) { @@ -202,19 +216,12 @@ private List getAttributesFor(final org.opensaml.saml1.core.Assertion private List getValuesFrom(final Attribute attribute) { final List list = new ArrayList(); - // TODO I'm not actually sure if this is safe! for (final Object o : attribute.getAttributeValues()) { list.add(o.toString()); } return list; } - private Date getCurrentTimeInUtc() { - final Calendar c = Calendar.getInstance(); - c.setTimeZone(TimeZone.getTimeZone("UTC")); - return c.getTime(); - } - protected String retrieveResponseFromServer(final URL validationUrl, final String ticket) { final String MESSAGE_TO_SEND = "" + "" + ticket diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/validation/Saml11TicketValidatorTests.java b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Saml11TicketValidatorTests.java index d55b548ae..8ec7977c0 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/validation/Saml11TicketValidatorTests.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Saml11TicketValidatorTests.java @@ -21,6 +21,9 @@ import org.jasig.cas.client.PublicTestHttpServer; import org.jasig.cas.client.util.CommonUtils; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; +import org.joda.time.Interval; import org.junit.*; import java.io.UnsupportedEncodingException; @@ -52,7 +55,7 @@ public static void cleanUp() throws Exception { }*/ @Test - public void testValidationFailedResponse() throws UnsupportedEncodingException { + public void testCompatibilityValidationFailedResponse() throws UnsupportedEncodingException { final String RESPONSE = "testtestPrincipalurn:oasis:names:tc:SAML:1.0:cm:artifact"; + final String RESPONSE = "testtestPrincipalurn:oasis:names:tc:SAML:1.0:cm:artifact"; server.content = RESPONSE.getBytes(server.encoding); try { final Assertion a = this.validator.validate("test", "test"); @@ -88,4 +90,37 @@ public void testValidationSuccessWithNoAttributes() throws UnsupportedEncodingEx fail(e.toString()); } } + + @Test + public void openSaml2GeneratedResponse() throws UnsupportedEncodingException { + final Interval range = currentTimeRangeInterval(); + final Date now = new Date(); + + final String response = "" + + "" + + "" + + "" + + "" + + "https://example.com/test-client/secure/" + + "" + + "testPrincipalurn:oasis:names:tc:SAML:1.0:cm:artifacttestPrincipalurn:oasis:names:tc:SAML:1.0:cm:artifact12345" + + "" + + "ACTIVE" + + "" + + "employee" + + "staff" + + "student"; + + server.content = response.getBytes(server.encoding); + try { + final Assertion a = this.validator.validate("test", "test"); + assertEquals("testPrincipal", a.getPrincipal().getName()); + } catch (final TicketValidationException e) { + fail(e.toString()); + } + } + + private Interval currentTimeRangeInterval() { + return new Interval(new DateTime(DateTimeZone.UTC).minus(5000), new DateTime(DateTimeZone.UTC).plus(200000000)); + } } From 0adcbcfc0adc84accd7195bc9681ec3a8801661b Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Tue, 24 Jul 2012 22:39:06 -0400 Subject: [PATCH 005/103] CASC-169 change to hashmap to allow values to be added --- .../cas/client/validation/Cas20ServiceTicketValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Cas20ServiceTicketValidator.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Cas20ServiceTicketValidator.java index 9b132f82c..627ca9a46 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Cas20ServiceTicketValidator.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Cas20ServiceTicketValidator.java @@ -130,7 +130,7 @@ protected final Assertion parseResponseFromServer(final String response) throws protected Map extractCustomAttributes(final String xml) { if (!xml.contains("")) { - return Collections.emptyMap(); + return new HashMap(); } final Map attributes = new HashMap(); From 03e552cf39eb27047775fad60b3d4c35bd0a8f7c Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Tue, 24 Jul 2012 23:01:35 -0400 Subject: [PATCH 006/103] CASC-185 add authentication time to assertion and also actually use the validity period for Assertion. --- .../org/jasig/cas/client/validation/Assertion.java | 8 ++++++++ .../jasig/cas/client/validation/AssertionImpl.java | 12 ++++++++++-- .../cas/client/validation/Saml11TicketValidator.java | 5 ++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Assertion.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Assertion.java index 75ac70d40..1e835eeff 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Assertion.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Assertion.java @@ -48,6 +48,14 @@ public interface Assertion extends Serializable { */ Date getValidUntilDate(); + /** + * The date the authentication actually occurred on. If its unable to be determined, it should be set to the current + * time. + * + * @return the authentication date, or the current time if it can't be determined. + */ + Date getAuthenticationDate(); + /** * The key/value pairs associated with this assertion. * diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AssertionImpl.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AssertionImpl.java index a845fe871..c15a6e340 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AssertionImpl.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AssertionImpl.java @@ -46,6 +46,8 @@ public final class AssertionImpl implements Assertion { /** The date the assertion is valid until. */ private final Date validUntilDate; + private final Date authenticationDate; + /** Map of key/value pairs associated with this assertion. I.e. authentication type. */ private final Map attributes; @@ -77,7 +79,7 @@ public AssertionImpl(final AttributePrincipal principal) { * @param attributes the key/value pairs for this attribute. */ public AssertionImpl(final AttributePrincipal principal, final Map attributes) { - this(principal, new Date(), null, attributes); + this(principal, new Date(), null, new Date(), attributes); } /** @@ -88,16 +90,22 @@ public AssertionImpl(final AttributePrincipal principal, final Map attributes) { + public AssertionImpl(final AttributePrincipal principal, final Date validFromDate, final Date validUntilDate, final Date authenticationDate, final Map attributes) { this.principal = principal; this.validFromDate = validFromDate; this.validUntilDate = validUntilDate; this.attributes = attributes; + this.authenticationDate = authenticationDate; CommonUtils.assertNotNull(this.principal, "principal cannot be null."); CommonUtils.assertNotNull(this.validFromDate, "validFromDate cannot be null."); CommonUtils.assertNotNull(this.attributes, "attributes cannot be null."); } + + public Date getAuthenticationDate() { + return this.authenticationDate; + } + public Date getValidFromDate() { return this.validFromDate; } diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java index dcfa8cfe4..cbbc4fbc4 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java @@ -156,7 +156,10 @@ protected Assertion parseResponseFromServer(final String response) throws Ticket final Map authenticationAttributes = new HashMap(); authenticationAttributes.put("samlAuthenticationStatement::authMethod", authenticationStatement.getAuthenticationMethod()); - return new AssertionImpl(principal, authenticationAttributes); + final DateTime notBefore = assertion.getConditions().getNotBefore(); + final DateTime notOnOrAfter = assertion.getConditions().getNotOnOrAfter(); + final DateTime authenticationInstant = authenticationStatement.getAuthenticationInstant(); + return new AssertionImpl(principal, notBefore.toDate(), notOnOrAfter.toDate(), authenticationInstant.toDate(), authenticationAttributes); } } catch (final UnmarshallingException e) { throw new TicketValidationException(e); From 0ff39cc542973a192ec03902dab81fa217e0a36e Mon Sep 17 00:00:00 2001 From: "Marvin S. Addison" Date: Wed, 25 Jul 2012 16:58:36 -0400 Subject: [PATCH 007/103] 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. --- .../jasig/cas/client/jaas/CasLoginModule.java | 82 ++++++++++++++----- .../cas/client/jaas/CasLoginModuleTests.java | 68 +++++++++++++-- .../src/test/resources/log4j.properties | 30 +++++++ 3 files changed, 151 insertions(+), 29 deletions(-) create mode 100644 cas-client-core/src/test/resources/log4j.properties 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 58a3e0e36..565d0ecc3 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.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 @@ * 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: + *

      + *
    • {@link Calendar#HOUR}
      • + *
    • {@link Calendar#MINUTE}
      • + *
    • {@link Calendar#SECOND}
      • + *
    • {@link Calendar#MILLISECOND}
      • + *
    + */ + 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 final void initialize(final Subject subject, final CallbackHandler handle } 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 final boolean login() throws LoginException { 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 final boolean commit() throws LoginException { 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 @@ private void removeCredentialsOfType(final Class clazz) { 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 void run() { } } } + 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 623730af7..1f84d239e 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 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 void testAssertionCaching() throws Exception { 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 void testAssertionCaching() throws Exception { 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 void testAssertionCaching() throws Exception { 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 000000000..8ccad3bf1 --- /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 From e2e374d14dd8e388fcf671814ae7b03f37804086 Mon Sep 17 00:00:00 2001 From: "Marvin S. Addison" Date: Thu, 26 Jul 2012 10:25:06 -0400 Subject: [PATCH 008/103] CASC-166 Address code review feedback. Use ConcurrentHashMap to avoid explicit synchronization. Use TimeUnit to allow more user-friendly configuration of the units of the cache timeout (e.g. MINUTES, SECONDS) and rename option from cacheTimeoutUnits to cacheTimeoutUnit for consistency. --- .../jasig/cas/client/jaas/CasLoginModule.java | 108 ++++++++---------- .../cas/client/jaas/CasLoginModuleTests.java | 4 +- 2 files changed, 49 insertions(+), 63 deletions(-) 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 565d0ecc3..02454e854 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 @@ -34,6 +34,7 @@ import java.util.Iterator; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; import javax.security.auth.Subject; import javax.security.auth.callback.Callback; @@ -84,10 +85,8 @@ * 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.
    • + *
  • cacheTimeoutUnit (optional) - Assertion cache timeout unit. Must be one of {@link TimeUnit} enumeration + * names, e.g. DAYS, HOURS, MINUTES, SECONDS, MILLISECONDS. Default unit is MINUTES.
    • * * *

    @@ -139,7 +138,7 @@ 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; + public static final TimeUnit DEFAULT_CACHE_TIMEOUT_UNIT = TimeUnit.MINUTES; /** * Stores mapping of ticket to assertion to support JAAS providers that @@ -191,16 +190,8 @@ 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: - *

      - *
    • {@link Calendar#HOUR}
      • - *
    • {@link Calendar#MINUTE}
      • - *
    • {@link Calendar#SECOND}
      • - *
    • {@link Calendar#MILLISECOND}
      • - *
    - */ - protected int cacheTimeoutUnits = DEFAULT_CACHE_TIMEOUT_UNITS; + /** Units of cache timeout. */ + protected TimeUnit cacheTimeoutUnit = DEFAULT_CACHE_TIMEOUT_UNIT; /** * Initializes the CAS login module. @@ -222,17 +213,20 @@ 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.
    • + *
  • cacheTimeoutUnit (optional) - Assertion cache timeout unit. Must be one of {@link TimeUnit} enumeration + * names, e.g. DAYS, HOURS, MINUTES, SECONDS, MILLISECONDS. Default unit is MINUTES.
    • * */ - public final void initialize(final Subject subject, final CallbackHandler handler, final Map state, final Map options) { + public final void initialize( + final Subject subject, + final CallbackHandler handler, + final Map state, + final Map options) { + this.assertion = null; this.callbackHandler = handler; this.subject = subject; - this.sharedState = (Map) state; + this.sharedState = (Map) state; this.sharedState = new HashMap(state); String ticketValidatorClass = null; @@ -268,15 +262,9 @@ public final void initialize(final Subject subject, final CallbackHandler handle } 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); - } + } else if ("cacheTimeoutUnit".equals(key)) { + this.cacheTimeoutUnit = Enum.valueOf(TimeUnit.class, (String) options.get(key)); + log.debug("Set cacheTimeoutUnit=" + this.cacheTimeoutUnit); } } @@ -325,20 +313,19 @@ public final boolean login() throws LoginException { throw (LoginException) new LoginException("IO exception in callback handler: " + e).initCause(e); } catch (final UnsupportedCallbackException e) { log.info("Login failed due to unsupported callback: " + e); - throw (LoginException) new LoginException("Callback handler does not support PasswordCallback and TextInputCallback.").initCause(e); + throw (LoginException) new LoginException( + "Callback handler does not support PasswordCallback and TextInputCallback.").initCause(e); } if (ticketCallback.getPassword() != null) { this.ticket = new TicketCredential(new String(ticketCallback.getPassword())); - final String service = CommonUtils.isNotBlank(serviceCallback.getName()) ? serviceCallback.getName() : this.service; + final String service = CommonUtils.isNotBlank( + serviceCallback.getName()) ? serviceCallback.getName() : this.service; if (this.cacheAssertions) { - // 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); - } + this.assertion = ASSERTION_CACHE.get(ticket); + if (this.assertion != null) { + log.debug("Assertion found in cache."); } } @@ -346,7 +333,8 @@ public final boolean login() throws LoginException { log.debug("CAS assertion is null; ticket validation required."); if (CommonUtils.isBlank(service)) { log.info("Login failed because required CAS service parameter not provided."); - throw new LoginException("Neither login module nor callback handler provided required service parameter."); + throw new LoginException( + "Neither login module nor callback handler provided required service parameter."); } try { if (log.isDebugEnabled()) { @@ -413,7 +401,8 @@ public final boolean commit() throws LoginException { throw new LoginException("Ticket credential not found."); } - final AssertionPrincipal casPrincipal = new AssertionPrincipal(this.assertion.getPrincipal().getName(), this.assertion); + final AssertionPrincipal casPrincipal = new AssertionPrincipal( + this.assertion.getPrincipal().getName(), this.assertion); this.subject.getPrincipals().add(casPrincipal); // Add group containing principal as sole member @@ -458,10 +447,7 @@ public final boolean commit() throws LoginException { if (log.isDebugEnabled()) { log.debug("Caching assertion for principal " + this.assertion.getPrincipal()); } - // Multiple threads may be accessing the static cache concurrently - synchronized (ASSERTION_CACHE) { - ASSERTION_CACHE.put(this.ticket, this.assertion); - } + ASSERTION_CACHE.put(this.ticket, this.assertion); } } else { // Login must have failed if there is no assertion defined @@ -522,10 +508,12 @@ protected void postLogout() { * @return Ticket validator with properties set. */ private TicketValidator createTicketValidator(final String className, final Map propertyMap) { - CommonUtils.assertTrue(propertyMap.containsKey("casServerUrlPrefix"), "Required property casServerUrlPrefix not found."); + CommonUtils.assertTrue( + propertyMap.containsKey("casServerUrlPrefix"), "Required property casServerUrlPrefix not found."); final Class validatorClass = ReflectUtils.loadClass(className); - final TicketValidator validator = ReflectUtils.newInstance(validatorClass, propertyMap.get("casServerUrlPrefix")); + final TicketValidator validator = ReflectUtils.newInstance( + validatorClass, propertyMap.get("casServerUrlPrefix")); try { final BeanInfo info = Introspector.getBeanInfo(validatorClass); @@ -570,7 +558,8 @@ private static Object convertIfNecessary(final PropertyDescriptor pd, final Stri } else if (long.class.equals(pd.getPropertyType())) { return new Long(value); } else { - throw new IllegalArgumentException("No conversion strategy exists for property " + pd.getName() + " of type " + pd.getPropertyType()); + throw new IllegalArgumentException( + "No conversion strategy exists for property " + pd.getName() + " of type " + pd.getPropertyType()); } } @@ -598,21 +587,18 @@ 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(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()); - } - iter.remove(); + final Iterator> iter = ASSERTION_CACHE.entrySet().iterator(); + final Calendar cutoff = Calendar.getInstance(); + cutoff.setTimeInMillis(System.currentTimeMillis() - this.cacheTimeoutUnit.toMillis(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()); } + iter.remove(); } } } 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 1f84d239e..ede32b005 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 @@ -208,8 +208,8 @@ public void testAssertionCachingExpiration() throws Exception { 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"); + // Cache timeout is 1 second + options.put("cacheTimeoutUnit", "SECONDS"); options.put("cacheTimeout", "1"); server.content = SUCCESS_RESPONSE.getBytes(server.encoding); From 1ebab8755c21ea845906900830cee97a2791448a Mon Sep 17 00:00:00 2001 From: Bernd Eckenfels Date: Fri, 21 Sep 2012 01:32:06 +0200 Subject: [PATCH 009/103] make default value of useRedirect visible. Add some JavaDoc for the options. --- .../AbstractTicketValidationFilter.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java index e9e1c5c54..87e35895a 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java @@ -38,10 +38,12 @@ *

    * This filter can be configured with the following values: *

      - *
    • redirectAfterValidation - redirect the CAS client to the same URL without the ticket.
      • + *
    • redirectAfterValidation - redirect the CAS client to the same URL without the ticket. (default: true)
      • *
    • exceptionOnValidationFailure - throw an exception if the validation fails. Otherwise, continue - * processing.
      • - *
    • useSession - store any of the useful information in a session attribute.
      • + * processing. (default: true) + *
    • useSession - store any of the useful information in a session attribute. (default: true)
      • + *
    • hostnameVerifier - name of class implementing a {@link HostnameVerifier}.
      • + *
    • hostnameVerifierConfig - name of configuration class (constructor argument of verifier).
      • *
    * * @author Scott Battaglia @@ -56,13 +58,17 @@ public abstract class AbstractTicketValidationFilter extends AbstractCasFilter { /** * Specify whether the filter should redirect the user agent after a * successful validation to remove the ticket parameter from the query - * string. + * string. Will be forced to true when {@link #useSession} is false. */ - private boolean redirectAfterValidation = false; + private boolean redirectAfterValidation = true; /** Determines whether an exception is thrown when there is a ticket validation failure. */ private boolean exceptionOnValidationFailure = true; + /** + * Specify whether the Assertion should be stored in a session + * attribute {@link AbstractCasFilter#CONST_CAS_ASSERTION} + */ private boolean useSession = true; /** From b4845cabd6fe56abce9b28a601ca968b93926982 Mon Sep 17 00:00:00 2001 From: Bernd Eckenfels Date: Fri, 21 Sep 2012 04:11:22 +0200 Subject: [PATCH 010/103] Corrected JavaDoc (force if useSession=false force redirect=false) --- .../AbstractTicketValidationFilter.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java index 87e35895a..7f650468b 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java @@ -38,7 +38,8 @@ *

    * This filter can be configured with the following values: *