Pārlūkot izejas kodu

ID Token validation supports clock skew

Fixes gh-5839
Joe Grandja 6 gadi atpakaļ
vecāks
revīzija
f234a5fbdb

+ 19 - 3
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenValidator.java

@@ -22,10 +22,12 @@ import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult;
 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;
@@ -44,7 +46,9 @@ import java.util.stream.Collectors;
  * @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_CLOCK_SKEW = Duration.ofSeconds(60);
 	private final ClientRegistration clientRegistration;
+	private Duration clockSkew = DEFAULT_CLOCK_SKEW;
 
 	public OidcIdTokenValidator(ClientRegistration clientRegistration) {
 		Assert.notNull(clientRegistration, "clientRegistration cannot be null");
@@ -93,15 +97,14 @@ public final class OidcIdTokenValidator implements OAuth2TokenValidator<Jwt> {
 
 		// 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.clockSkew).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.clockSkew).isBefore(idToken.getIssuedAt())) {
 			invalidClaims.put(IdTokenClaimNames.IAT, idToken.getIssuedAt());
 		}
 
@@ -119,6 +122,19 @@ public final class OidcIdTokenValidator implements OAuth2TokenValidator<Jwt> {
 		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 clockSkew the maximum acceptable clock skew
+	 */
+	public final void setClockSkew(Duration clockSkew) {
+		Assert.notNull(clockSkew, "clockSkew cannot be null");
+		this.clockSkew = clockSkew;
+	}
+
 	private static OAuth2Error invalidIdToken(Map<String, Object> invalidClaims) {
 		String claimsDetail = invalidClaims.entrySet().stream()
 				.map(it -> it.getKey() + " (" + it.getValue() + ")")

+ 41 - 20
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenValidatorTests.java

@@ -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 clockSkew = Duration.ofSeconds(60);
 
 	@Before
 	public void setup() {
@@ -55,12 +56,12 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@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)
@@ -69,7 +70,7 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@Test
-	public void validateIdTokenWhenSubNullThenHasErrors() {
+	public void validateWhenSubNullThenHasErrors() {
 		this.claims.remove(IdTokenClaimNames.SUB);
 		assertThat(this.validateIdToken())
 				.hasSize(1)
@@ -78,7 +79,7 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@Test
-	public void validateIdTokenWhenAudNullThenHasErrors() {
+	public void validateWhenAudNullThenHasErrors() {
 		this.claims.remove(IdTokenClaimNames.AUD);
 		assertThat(this.validateIdToken())
 				.hasSize(1)
@@ -87,7 +88,7 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@Test
-	public void validateIdTokenWhenIssuedAtNullThenHasErrors() {
+	public void validateWhenIssuedAtNullThenHasErrors() {
 		this.issuedAt = null;
 		assertThat(this.validateIdToken())
 				.hasSize(1)
@@ -96,7 +97,7 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@Test
-	public void validateIdTokenWhenExpiresAtNullThenHasErrors() {
+	public void validateWhenExpiresAtNullThenHasErrors() {
 		this.expiresAt = null;
 		assertThat(this.validateIdToken())
 				.hasSize(1)
@@ -105,7 +106,7 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@Test
-	public void validateIdTokenWhenAudMultipleAndAzpNullThenHasErrors() {
+	public void validateWhenAudMultipleAndAzpNullThenHasErrors() {
 		this.claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id", "other"));
 		assertThat(this.validateIdToken())
 				.hasSize(1)
@@ -114,7 +115,7 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@Test
-	public void validateIdTokenWhenAzpNotClientIdThenHasErrors() {
+	public void validateWhenAzpNotClientIdThenHasErrors() {
 		this.claims.put(IdTokenClaimNames.AZP, "other");
 		assertThat(this.validateIdToken())
 				.hasSize(1)
@@ -123,14 +124,14 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@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())
@@ -140,7 +141,7 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@Test
-	public void validateIdTokenWhenAudNotClientIdThenHasErrors() {
+	public void validateWhenAudNotClientIdThenHasErrors() {
 		this.claims.put(IdTokenClaimNames.AUD, Collections.singletonList("other-client"));
 		assertThat(this.validateIdToken())
 				.hasSize(1)
@@ -149,9 +150,18 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@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.clockSkew = 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.clockSkew = Duration.ofSeconds(0);
 		assertThat(this.validateIdToken())
 				.hasSize(1)
 				.extracting(OAuth2Error::getDescription)
@@ -159,9 +169,18 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@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.clockSkew = 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.clockSkew = Duration.ofMinutes(0);
 		assertThat(this.validateIdToken())
 				.hasSize(1)
 				.extracting(OAuth2Error::getDescription)
@@ -169,9 +188,10 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@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.clockSkew = Duration.ofSeconds(0);
 		assertThat(this.validateIdToken())
 				.hasSize(1)
 				.extracting(OAuth2Error::getDescription)
@@ -179,7 +199,7 @@ public class OidcIdTokenValidatorTests {
 	}
 
 	@Test
-	public void validateIdTokenWhenMissingClaimsThenHasErrors() {
+	public void validateWhenMissingClaimsThenHasErrors() {
 		this.claims.remove(IdTokenClaimNames.SUB);
 		this.claims.remove(IdTokenClaimNames.AUD);
 		this.issuedAt = null;
@@ -196,6 +216,7 @@ public class OidcIdTokenValidatorTests {
 	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.setClockSkew(this.clockSkew);
 		return validator.validate(idToken).getErrors();
 	}
 }