Skip to content

Commit f234a5f

Browse files
jgrandjajgrandja
committed
ID Token validation supports clock skew
Fixes gh-5839
1 parent 575d943 commit f234a5f

File tree

2 files changed

+60
-23
lines changed

2 files changed

+60
-23
lines changed

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenValidator.java

+19-3
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@
2222
import org.springframework.security.oauth2.core.oidc.IdTokenClaimNames;
2323
import org.springframework.security.oauth2.core.oidc.OidcIdToken;
2424
import org.springframework.security.oauth2.jwt.Jwt;
25+
import org.springframework.security.oauth2.jwt.JwtClaimNames;
2526
import org.springframework.util.Assert;
2627
import org.springframework.util.CollectionUtils;
2728

2829
import java.net.URL;
30+
import java.time.Duration;
2931
import java.time.Instant;
3032
import java.util.HashMap;
3133
import java.util.List;
@@ -44,7 +46,9 @@
4446
* @see <a target="_blank" href="http://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation">ID Token Validation</a>
4547
*/
4648
public final class OidcIdTokenValidator implements OAuth2TokenValidator<Jwt> {
49+
private static final Duration DEFAULT_CLOCK_SKEW = Duration.ofSeconds(60);
4750
private final ClientRegistration clientRegistration;
51+
private Duration clockSkew = DEFAULT_CLOCK_SKEW;
4852

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

9498
// 9. The current time MUST be before the time represented by the exp Claim.
9599
Instant now = Instant.now();
96-
if (!now.isBefore(idToken.getExpiresAt())) {
100+
if (now.minus(this.clockSkew).isAfter(idToken.getExpiresAt())) {
97101
invalidClaims.put(IdTokenClaimNames.EXP, idToken.getExpiresAt());
98102
}
99103

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

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

125+
/**
126+
* Sets the maximum acceptable clock skew. The default is 60 seconds.
127+
* The clock skew is used when validating the {@link JwtClaimNames#EXP exp}
128+
* and {@link JwtClaimNames#IAT iat} claims.
129+
*
130+
* @since 5.2
131+
* @param clockSkew the maximum acceptable clock skew
132+
*/
133+
public final void setClockSkew(Duration clockSkew) {
134+
Assert.notNull(clockSkew, "clockSkew cannot be null");
135+
this.clockSkew = clockSkew;
136+
}
137+
122138
private static OAuth2Error invalidIdToken(Map<String, Object> invalidClaims) {
123139
String claimsDetail = invalidClaims.entrySet().stream()
124140
.map(it -> it.getKey() + " (" + it.getValue() + ")")

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/oidc/authentication/OidcIdTokenValidatorTests.java

+41-20
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public class OidcIdTokenValidatorTests {
4545
private Map<String, Object> claims = new HashMap<>();
4646
private Instant issuedAt = Instant.now();
4747
private Instant expiresAt = this.issuedAt.plusSeconds(3600);
48+
private Duration clockSkew = Duration.ofSeconds(60);
4849

4950
@Before
5051
public void setup() {
@@ -55,12 +56,12 @@ public void setup() {
5556
}
5657

5758
@Test
58-
public void validateIdTokenWhenValidThenNoErrors() {
59+
public void validateWhenValidThenNoErrors() {
5960
assertThat(this.validateIdToken()).isEmpty();
6061
}
6162

6263
@Test
63-
public void validateIdTokenWhenIssuerNullThenHasErrors() {
64+
public void validateWhenIssuerNullThenHasErrors() {
6465
this.claims.remove(IdTokenClaimNames.ISS);
6566
assertThat(this.validateIdToken())
6667
.hasSize(1)
@@ -69,7 +70,7 @@ public void validateIdTokenWhenIssuerNullThenHasErrors() {
6970
}
7071

7172
@Test
72-
public void validateIdTokenWhenSubNullThenHasErrors() {
73+
public void validateWhenSubNullThenHasErrors() {
7374
this.claims.remove(IdTokenClaimNames.SUB);
7475
assertThat(this.validateIdToken())
7576
.hasSize(1)
@@ -78,7 +79,7 @@ public void validateIdTokenWhenSubNullThenHasErrors() {
7879
}
7980

8081
@Test
81-
public void validateIdTokenWhenAudNullThenHasErrors() {
82+
public void validateWhenAudNullThenHasErrors() {
8283
this.claims.remove(IdTokenClaimNames.AUD);
8384
assertThat(this.validateIdToken())
8485
.hasSize(1)
@@ -87,7 +88,7 @@ public void validateIdTokenWhenAudNullThenHasErrors() {
8788
}
8889

8990
@Test
90-
public void validateIdTokenWhenIssuedAtNullThenHasErrors() {
91+
public void validateWhenIssuedAtNullThenHasErrors() {
9192
this.issuedAt = null;
9293
assertThat(this.validateIdToken())
9394
.hasSize(1)
@@ -96,7 +97,7 @@ public void validateIdTokenWhenIssuedAtNullThenHasErrors() {
9697
}
9798

9899
@Test
99-
public void validateIdTokenWhenExpiresAtNullThenHasErrors() {
100+
public void validateWhenExpiresAtNullThenHasErrors() {
100101
this.expiresAt = null;
101102
assertThat(this.validateIdToken())
102103
.hasSize(1)
@@ -105,7 +106,7 @@ public void validateIdTokenWhenExpiresAtNullThenHasErrors() {
105106
}
106107

107108
@Test
108-
public void validateIdTokenWhenAudMultipleAndAzpNullThenHasErrors() {
109+
public void validateWhenAudMultipleAndAzpNullThenHasErrors() {
109110
this.claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id", "other"));
110111
assertThat(this.validateIdToken())
111112
.hasSize(1)
@@ -114,7 +115,7 @@ public void validateIdTokenWhenAudMultipleAndAzpNullThenHasErrors() {
114115
}
115116

116117
@Test
117-
public void validateIdTokenWhenAzpNotClientIdThenHasErrors() {
118+
public void validateWhenAzpNotClientIdThenHasErrors() {
118119
this.claims.put(IdTokenClaimNames.AZP, "other");
119120
assertThat(this.validateIdToken())
120121
.hasSize(1)
@@ -123,14 +124,14 @@ public void validateIdTokenWhenAzpNotClientIdThenHasErrors() {
123124
}
124125

125126
@Test
126-
public void validateIdTokenWhenMultipleAudAzpClientIdThenNoErrors() {
127+
public void validateWhenMultipleAudAzpClientIdThenNoErrors() {
127128
this.claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id", "other"));
128129
this.claims.put(IdTokenClaimNames.AZP, "client-id");
129130
assertThat(this.validateIdToken()).isEmpty();
130131
}
131132

132133
@Test
133-
public void validateIdTokenWhenMultipleAudAzpNotClientIdThenHasErrors() {
134+
public void validateWhenMultipleAudAzpNotClientIdThenHasErrors() {
134135
this.claims.put(IdTokenClaimNames.AUD, Arrays.asList("client-id-1", "client-id-2"));
135136
this.claims.put(IdTokenClaimNames.AZP, "other-client");
136137
assertThat(this.validateIdToken())
@@ -140,7 +141,7 @@ public void validateIdTokenWhenMultipleAudAzpNotClientIdThenHasErrors() {
140141
}
141142

142143
@Test
143-
public void validateIdTokenWhenAudNotClientIdThenHasErrors() {
144+
public void validateWhenAudNotClientIdThenHasErrors() {
144145
this.claims.put(IdTokenClaimNames.AUD, Collections.singletonList("other-client"));
145146
assertThat(this.validateIdToken())
146147
.hasSize(1)
@@ -149,37 +150,56 @@ public void validateIdTokenWhenAudNotClientIdThenHasErrors() {
149150
}
150151

151152
@Test
152-
public void validateIdTokenWhenExpiredThenHasErrors() {
153-
this.issuedAt = Instant.now().minus(Duration.ofMinutes(1));
154-
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(1));
153+
public void validateWhenExpiredAnd60secClockSkewThenNoErrors() {
154+
this.issuedAt = Instant.now().minus(Duration.ofSeconds(60));
155+
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(30));
156+
this.clockSkew = Duration.ofSeconds(60);
157+
assertThat(this.validateIdToken()).isEmpty();
158+
}
159+
160+
@Test
161+
public void validateWhenExpiredAnd0secClockSkewThenHasErrors() {
162+
this.issuedAt = Instant.now().minus(Duration.ofSeconds(60));
163+
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(30));
164+
this.clockSkew = Duration.ofSeconds(0);
155165
assertThat(this.validateIdToken())
156166
.hasSize(1)
157167
.extracting(OAuth2Error::getDescription)
158168
.allMatch(msg -> msg.contains(IdTokenClaimNames.EXP));
159169
}
160170

161171
@Test
162-
public void validateIdTokenWhenIssuedAtWayInFutureThenHasErrors() {
172+
public void validateWhenIssuedAt5minAheadAnd5minClockSkewThenNoErrors() {
163173
this.issuedAt = Instant.now().plus(Duration.ofMinutes(5));
164-
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(1));
174+
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(60));
175+
this.clockSkew = Duration.ofMinutes(5);
176+
assertThat(this.validateIdToken()).isEmpty();
177+
}
178+
179+
@Test
180+
public void validateWhenIssuedAt1minAheadAnd0minClockSkewThenHasErrors() {
181+
this.issuedAt = Instant.now().plus(Duration.ofMinutes(1));
182+
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(60));
183+
this.clockSkew = Duration.ofMinutes(0);
165184
assertThat(this.validateIdToken())
166185
.hasSize(1)
167186
.extracting(OAuth2Error::getDescription)
168187
.allMatch(msg -> msg.contains(IdTokenClaimNames.IAT));
169188
}
170189

171190
@Test
172-
public void validateIdTokenWhenExpiresAtBeforeNowThenHasErrors() {
173-
this.issuedAt = Instant.now().minusSeconds(10);
174-
this.expiresAt = Instant.from(this.issuedAt).plusSeconds(5);
191+
public void validateWhenExpiresAtBeforeNowThenHasErrors() {
192+
this.issuedAt = Instant.now().minus(Duration.ofSeconds(10));
193+
this.expiresAt = this.issuedAt.plus(Duration.ofSeconds(5));
194+
this.clockSkew = Duration.ofSeconds(0);
175195
assertThat(this.validateIdToken())
176196
.hasSize(1)
177197
.extracting(OAuth2Error::getDescription)
178198
.allMatch(msg -> msg.contains(IdTokenClaimNames.EXP));
179199
}
180200

181201
@Test
182-
public void validateIdTokenWhenMissingClaimsThenHasErrors() {
202+
public void validateWhenMissingClaimsThenHasErrors() {
183203
this.claims.remove(IdTokenClaimNames.SUB);
184204
this.claims.remove(IdTokenClaimNames.AUD);
185205
this.issuedAt = null;
@@ -196,6 +216,7 @@ public void validateIdTokenWhenMissingClaimsThenHasErrors() {
196216
private Collection<OAuth2Error> validateIdToken() {
197217
Jwt idToken = new Jwt("token123", this.issuedAt, this.expiresAt, this.headers, this.claims);
198218
OidcIdTokenValidator validator = new OidcIdTokenValidator(this.registration.build());
219+
validator.setClockSkew(this.clockSkew);
199220
return validator.validate(idToken).getErrors();
200221
}
201222
}

0 commit comments

Comments
 (0)