Skip to content

Commit

Permalink
Add notBeforeSkew option
Browse files Browse the repository at this point in the history
When the clock of the identity provider is ahead of the relying party, a SAML assertion may end up having a notBefore date that appears to be in the future. This causes the assertion to be rejected. It would be nice if we could tell SamlClient that we're willing to accept some skew in server time to prevent these assertions failing.

Because assertions already take into account that there may be latency between servers, it does not seem necesary to me to skew the notOnOrAfter date. If we do want this it would be very simple to skew both notBefore and notOnOrAfter, though.

ADFS provides an option notBeforeSkew that allows you to accept assertions that appear to be in the future by a certain amount. This pull request introduces a similar feature to saml-client.
  • Loading branch information
SpectralDuck authored and malaporte committed Sep 18, 2018
1 parent 2e0f4b2 commit 58cb515
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 4 deletions.
27 changes: 23 additions & 4 deletions src/main/java/com/coveo/saml/SamlClient.java
Expand Up @@ -70,6 +70,7 @@ public enum SamlIdpBinding {
private String responseIssuer;
private List<Credential> credentials;
private DateTime now; // used for testing only
private long notBeforeSkew = 0L;
private SamlIdpBinding samlBinding;

/**
Expand All @@ -90,6 +91,21 @@ public void setDateTimeNow(DateTime now) {
this.now = now;
}

/**
* Sets by how much the current time can be before the assertion's notBefore.
*
* Used to mitigate clock differences between the identity provider and relying party.
*
* @param notBeforeSkew non-negative amount of skew (in milliseconds) to allow between the
* current time and the assertion's notBefore date. Default: 0
*/
public void setNotBeforeSkew(long notBeforeSkew) {
if (notBeforeSkew < 0) {
throw new IllegalArgumentException("Skew must be non-negative");
}
this.notBeforeSkew = notBeforeSkew;
}

/**
* Constructs an SAML client using explicit parameters.
*
Expand Down Expand Up @@ -453,14 +469,17 @@ private void validateAssertion(Response response) throws SamlException {
private void enforceConditions(Conditions conditions) throws SamlException {
DateTime now = this.now != null ? this.now : DateTime.now();

if (now.isBefore(conditions.getNotBefore())) {
DateTime notBefore = conditions.getNotBefore();
DateTime skewedNotBefore = notBefore.minus(notBeforeSkew);
if (now.isBefore(skewedNotBefore)) {
throw new SamlException(
"The assertion cannot be used before " + conditions.getNotBefore().toString());
"The assertion cannot be used before " + notBefore.toString());
}

if (now.isAfter(conditions.getNotOnOrAfter())) {
DateTime notOnOrAfter = conditions.getNotOnOrAfter();
if (now.isAfter(notOnOrAfter)) {
throw new SamlException(
"The assertion cannot be used after " + conditions.getNotOnOrAfter().toString());
"The assertion cannot be used after " + notOnOrAfter.toString());
}
}

Expand Down
24 changes: 24 additions & 0 deletions src/test/java/com/coveo/saml/SamlClientTest.java
Expand Up @@ -183,4 +183,28 @@ public void itIsNotVulnerableToCommentAttackFromOpenSAML() throws Throwable {
// ensure that the identity that ends up being returned is the proper one.
assertEquals("mlaporte@coveo.com", response.getNameID());
}

@Test
public void decodeAndValidateSamlResponseWorksWithNowAfterSkewedNotBefore() throws Throwable {
SamlClient client =
SamlClient.fromMetadata(
"myidentifier", "http://some/url", getXml("adfs.xml"), SamlClient.SamlIdpBinding.POST);
int skew = 60 * 60 * 1000;
client.setDateTimeNow(ASSERTION_DATE.minus(skew));
client.setNotBeforeSkew(skew);
SamlResponse response = client.decodeAndValidateSamlResponse(AN_ENCODED_RESPONSE);
assertEquals("mlaporte@coveo.com", response.getNameID());
}


@Test(expected = SamlException.class)
public void decodeAndValidateSamlResponseRejectsNowBeforeNotBefore() throws Throwable {
SamlClient client =
SamlClient.fromMetadata(
"myidentifier", "http://some/url", getXml("adfs.xml"), SamlClient.SamlIdpBinding.POST);
int skew = 60 * 60 * 1000;
client.setDateTimeNow(ASSERTION_DATE.minus(skew));
SamlResponse response = client.decodeAndValidateSamlResponse(AN_ENCODED_RESPONSE);
assertEquals("mlaporte@coveo.com", response.getNameID());
}
}

0 comments on commit 58cb515

Please sign in to comment.