Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ID Token validation supports clock skew #6375

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
import org.springframework.security.oauth2.core.oidc.IdTokenClaimNames;
import org.springframework.security.oauth2.core.oidc.OidcIdToken;
import org.springframework.security.oauth2.jwt.Jwt;
import org.springframework.security.oauth2.jwt.JwtClaimNames;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;

import java.net.URL;
import java.time.Duration;
import java.time.Instant;
import java.util.HashMap;
import java.util.List;
Expand All @@ -44,7 +46,9 @@
* @see <a target="_blank" href="http://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation">ID Token Validation</a>
*/
public final class OidcIdTokenValidator implements OAuth2TokenValidator<Jwt> {
private static final Duration DEFAULT_MAX_CLOCK_SKEW = Duration.ofSeconds(60);
private final ClientRegistration clientRegistration;
private Duration maxClockSkew = DEFAULT_MAX_CLOCK_SKEW;

public OidcIdTokenValidator(ClientRegistration clientRegistration) {
Assert.notNull(clientRegistration, "clientRegistration cannot be null");
Expand Down Expand Up @@ -93,15 +97,14 @@ public OAuth2TokenValidatorResult validate(Jwt idToken) {

// 9. The current time MUST be before the time represented by the exp Claim.
Instant now = Instant.now();
if (!now.isBefore(idToken.getExpiresAt())) {
if (now.minus(this.maxClockSkew).isAfter(idToken.getExpiresAt())) {
invalidClaims.put(IdTokenClaimNames.EXP, idToken.getExpiresAt());
}

// 10. The iat Claim can be used to reject tokens that were issued too far away from the current time,
// limiting the amount of time that nonces need to be stored to prevent attacks.
// The acceptable range is Client specific.
Instant maxIssuedAt = now.plusSeconds(30);
if (idToken.getIssuedAt().isAfter(maxIssuedAt)) {
if (now.plus(this.maxClockSkew).isBefore(idToken.getIssuedAt())) {
invalidClaims.put(IdTokenClaimNames.IAT, idToken.getIssuedAt());
}

Expand All @@ -119,6 +122,19 @@ public OAuth2TokenValidatorResult validate(Jwt idToken) {
return OAuth2TokenValidatorResult.success();
}

/**
* Sets the maximum acceptable clock skew. The default is 60 seconds.
* The clock skew is used when validating the {@link JwtClaimNames#EXP exp}
* and {@link JwtClaimNames#IAT iat} claims.
*
* @since 5.2
* @param maxClockSkew the maximum acceptable clock skew
*/
public final void setMaxClockSkew(Duration maxClockSkew) {
Assert.notNull(maxClockSkew, "maxClockSkew cannot be null");
this.maxClockSkew = maxClockSkew;
}

private static OAuth2Error invalidIdToken(Map<String, Object> invalidClaims) {
String claimsDetail = invalidClaims.entrySet().stream()
.map(it -> it.getKey() + " (" + it.getValue() + ")")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public class OidcIdTokenValidatorTests {
private Map<String, Object> claims = new HashMap<>();
private Instant issuedAt = Instant.now();
private Instant expiresAt = this.issuedAt.plusSeconds(3600);
private Duration maxClockSkew = Duration.ofSeconds(60);

@Before
public void setup() {
Expand All @@ -55,12 +56,12 @@ public void setup() {
}

@Test
public void validateIdTokenWhenValidThenNoErrors() {
public void validateWhenValidThenNoErrors() {
assertThat(this.validateIdToken()).isEmpty();
}

@Test
public void validateIdTokenWhenIssuerNullThenHasErrors() {
public void validateWhenIssuerNullThenHasErrors() {
this.claims.remove(IdTokenClaimNames.ISS);
assertThat(this.validateIdToken())
.hasSize(1)
Expand All @@ -69,7 +70,7 @@ public void validateIdTokenWhenIssuerNullThenHasErrors() {
}

@Test
public void validateIdTokenWhenSubNullThenHasErrors() {
public void validateWhenSubNullThenHasErrors() {
this.claims.remove(IdTokenClaimNames.SUB);
assertThat(this.validateIdToken())
.hasSize(1)
Expand All @@ -78,7 +79,7 @@ public void validateIdTokenWhenSubNullThenHasErrors() {
}

@Test
public void validateIdTokenWhenAudNullThenHasErrors() {
public void validateWhenAudNullThenHasErrors() {
this.claims.remove(IdTokenClaimNames.AUD);
assertThat(this.validateIdToken())
.hasSize(1)
Expand All @@ -87,7 +88,7 @@ public void validateIdTokenWhenAudNullThenHasErrors() {
}

@Test
public void validateIdTokenWhenIssuedAtNullThenHasErrors() {
public void validateWhenIssuedAtNullThenHasErrors() {
this.issuedAt = null;
assertThat(this.validateIdToken())
.hasSize(1)
Expand All @@ -96,7 +97,7 @@ public void validateIdTokenWhenIssuedAtNullThenHasErrors() {
}

@Test
public void validateIdTokenWhenExpiresAtNullThenHasErrors() {
public void validateWhenExpiresAtNullThenHasErrors() {
this.expiresAt = null;
assertThat(this.validateIdToken())
.hasSize(1)
Expand All @@ -105,7 +106,7 @@ public void validateIdTokenWhenExpiresAtNullThenHasErrors() {
}

@Test
public void validateIdTokenWhenAudMultipleAndAzpNullThenHasErrors() {
public void validateWhenAudMultipleAndAzpNullThenHasErrors() {
this.claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id", "other"));
assertThat(this.validateIdToken())
.hasSize(1)
Expand All @@ -114,7 +115,7 @@ public void validateIdTokenWhenAudMultipleAndAzpNullThenHasErrors() {
}

@Test
public void validateIdTokenWhenAzpNotClientIdThenHasErrors() {
public void validateWhenAzpNotClientIdThenHasErrors() {
this.claims.put(IdTokenClaimNames.AZP, "other");
assertThat(this.validateIdToken())
.hasSize(1)
Expand All @@ -123,14 +124,14 @@ public void validateIdTokenWhenAzpNotClientIdThenHasErrors() {
}

@Test
public void validateIdTokenWhenMultipleAudAzpClientIdThenNoErrors() {
public void validateWhenMultipleAudAzpClientIdThenNoErrors() {
this.claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id", "other"));
this.claims.put(IdTokenClaimNames.AZP, "client-id");
assertThat(this.validateIdToken()).isEmpty();
}

@Test
public void validateIdTokenWhenMultipleAudAzpNotClientIdThenHasErrors() {
public void validateWhenMultipleAudAzpNotClientIdThenHasErrors() {
this.claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id-1", "client-id-2"));
this.claims.put(IdTokenClaimNames.AZP, "other-client");
assertThat(this.validateIdToken())
Expand All @@ -140,7 +141,7 @@ public void validateIdTokenWhenMultipleAudAzpNotClientIdThenHasErrors() {
}

@Test
public void validateIdTokenWhenAudNotClientIdThenHasErrors() {
public void validateWhenAudNotClientIdThenHasErrors() {
this.claims.put(IdTokenClaimNames.AUD, Collections.singletonList("other-client"));
assertThat(this.validateIdToken())
.hasSize(1)
Expand All @@ -149,37 +150,56 @@ public void validateIdTokenWhenAudNotClientIdThenHasErrors() {
}

@Test
public void validateIdTokenWhenExpiredThenHasErrors() {
this.issuedAt = Instant.now().minus(Duration.ofMinutes(1));
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(1));
public void validateWhenExpiredAnd60secClockSkewThenNoErrors() {
this.issuedAt = Instant.now().minus(Duration.ofSeconds(60));
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(30));
this.maxClockSkew = Duration.ofSeconds(60);
assertThat(this.validateIdToken()).isEmpty();
}

@Test
public void validateWhenExpiredAnd0secClockSkewThenHasErrors() {
this.issuedAt = Instant.now().minus(Duration.ofSeconds(60));
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(30));
this.maxClockSkew = Duration.ofSeconds(0);
assertThat(this.validateIdToken())
.hasSize(1)
.extracting(OAuth2Error::getDescription)
.allMatch(msg -> msg.contains(IdTokenClaimNames.EXP));
}

@Test
public void validateIdTokenWhenIssuedAtWayInFutureThenHasErrors() {
public void validateWhenIssuedAt5minAheadAnd5minClockSkewThenNoErrors() {
this.issuedAt = Instant.now().plus(Duration.ofMinutes(5));
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(1));
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(60));
this.maxClockSkew = Duration.ofMinutes(5);
assertThat(this.validateIdToken()).isEmpty();
}

@Test
public void validateWhenIssuedAt1minAheadAnd0minClockSkewThenHasErrors() {
this.issuedAt = Instant.now().plus(Duration.ofMinutes(1));
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(60));
this.maxClockSkew = Duration.ofMinutes(0);
assertThat(this.validateIdToken())
.hasSize(1)
.extracting(OAuth2Error::getDescription)
.allMatch(msg -> msg.contains(IdTokenClaimNames.IAT));
}

@Test
public void validateIdTokenWhenExpiresAtBeforeNowThenHasErrors() {
this.issuedAt = Instant.now().minusSeconds(10);
this.expiresAt = Instant.from(this.issuedAt).plusSeconds(5);
public void validateWhenExpiresAtBeforeNowThenHasErrors() {
this.issuedAt = Instant.now().minus(Duration.ofSeconds(10));
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(5));
this.maxClockSkew = Duration.ofSeconds(0);
assertThat(this.validateIdToken())
.hasSize(1)
.extracting(OAuth2Error::getDescription)
.allMatch(msg -> msg.contains(IdTokenClaimNames.EXP));
}

@Test
public void validateIdTokenWhenMissingClaimsThenHasErrors() {
public void validateWhenMissingClaimsThenHasErrors() {
this.claims.remove(IdTokenClaimNames.SUB);
this.claims.remove(IdTokenClaimNames.AUD);
this.issuedAt = null;
Expand All @@ -196,6 +216,7 @@ public void validateIdTokenWhenMissingClaimsThenHasErrors() {
private Collection<OAuth2Error> validateIdToken() {
Jwt idToken = new Jwt("token123", this.issuedAt, this.expiresAt, this.headers, this.claims);
OidcIdTokenValidator validator = new OidcIdTokenValidator(this.registration.build());
validator.setMaxClockSkew(this.maxClockSkew);
return validator.validate(idToken).getErrors();
}
}