From 082aafa9ca8df305a77e82967117e9696482009d Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Tue, 24 Jul 2012 22:13:25 -0400 Subject: [PATCH] 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 | 53 +++++++++++-------- .../Saml11TicketValidatorTests.java | 45 ++++++++++++++-- 2 files changed, 70 insertions(+), 28 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 9213f91..dcfa8cf 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.AttributePrincipal; 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 @@ import javax.net.ssl.HttpsURLConnection; */ 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 @@ public final class Saml11TicketValidator extends AbstractUrlBasedTicketValidator 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 @@ public final class Saml11TicketValidator extends AbstractUrlBasedTicketValidator 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()) { + if (validityRange.contains(currentTime)) { + log.debug("Current time is within the interval validity."); + return true; + } + + if (currentTime.isBefore(validityRange.getStart())) { log.debug("skipping assertion that's not yet valid..."); return false; } - if (notOnOrAfter.getMillis() <= currentTime - tolerance) { - log.debug("skipping expired assertion..."); - 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 @@ public final class Saml11TicketValidator extends AbstractUrlBasedTicketValidator 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 d55b548..8ec7977 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 @@ package org.jasig.cas.client.validation; 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 final class Saml11TicketValidatorTests extends AbstractTicketValidatorTes }*/ @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 final class Saml11TicketValidatorTests extends AbstractTicketValidatorTes 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)); + } }