Skip to content

Commit 1c53a78

Browse files
jgrandjajgrandja
committed
Fix access token expiry check with clock skew
Fixes gh-7511
1 parent 62c7de0 commit 1c53a78

12 files changed

+161
-31
lines changed

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/ClientCredentialsOAuth2AuthorizedClientProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public OAuth2AuthorizedClient authorize(OAuth2AuthorizationContext context) {
8686
}
8787

8888
private boolean hasTokenExpired(AbstractOAuth2Token token) {
89-
return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
89+
return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
9090
}
9191

9292
/**

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/ClientCredentialsReactiveOAuth2AuthorizedClientProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public Mono<OAuth2AuthorizedClient> authorize(OAuth2AuthorizationContext context
8282
}
8383

8484
private boolean hasTokenExpired(AbstractOAuth2Token token) {
85-
return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
85+
return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
8686
}
8787

8888
/**

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/PasswordOAuth2AuthorizedClientProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public OAuth2AuthorizedClient authorize(OAuth2AuthorizationContext context) {
104104
}
105105

106106
private boolean hasTokenExpired(AbstractOAuth2Token token) {
107-
return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
107+
return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
108108
}
109109

110110
/**

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/PasswordReactiveOAuth2AuthorizedClientProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public Mono<OAuth2AuthorizedClient> authorize(OAuth2AuthorizationContext context
102102
}
103103

104104
private boolean hasTokenExpired(AbstractOAuth2Token token) {
105-
return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
105+
return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
106106
}
107107

108108
/**

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/RefreshTokenOAuth2AuthorizedClientProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public OAuth2AuthorizedClient authorize(OAuth2AuthorizationContext context) {
9494
}
9595

9696
private boolean hasTokenExpired(AbstractOAuth2Token token) {
97-
return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
97+
return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
9898
}
9999

100100
/**

‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/RefreshTokenReactiveOAuth2AuthorizedClientProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public Mono<OAuth2AuthorizedClient> authorize(OAuth2AuthorizationContext context
9393
}
9494

9595
private boolean hasTokenExpired(AbstractOAuth2Token token) {
96-
return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
96+
return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
9797
}
9898

9999
/**

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/ClientCredentialsOAuth2AuthorizedClientProviderTests.java

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -154,21 +154,32 @@ public void authorizeWhenClientCredentialsAndTokenNotExpiredThenNotReauthorize()
154154
assertThat(this.authorizedClientProvider.authorize(authorizationContext)).isNull();
155155
}
156156

157+
// gh-7511
157158
@Test
158-
public void authorizeWhenClientCredentialsAndTokenNotExpiredByClockSkewThenNotReauthorize() {
159-
ClientCredentialsOAuth2AuthorizedClientProvider authorizedClientProvider =
160-
new ClientCredentialsOAuth2AuthorizedClientProvider();
161-
authorizedClientProvider.setClockSkew(Duration.ofHours(24));
162-
Instant issuedAt = Instant.now().minus(Duration.ofDays(1));
163-
OAuth2AccessToken expiredToken = new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER, "token",
164-
issuedAt, issuedAt.plus(Duration.ofHours(1)));
159+
public void authorizeWhenClientCredentialsAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
160+
Instant now = Instant.now();
161+
Instant issuedAt = now.minus(Duration.ofMinutes(60));
162+
Instant expiresAt = now.minus(Duration.ofMinutes(1));
163+
OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
164+
OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
165165
OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(
166-
this.clientRegistration, this.principal.getName(), expiredToken);
166+
this.clientRegistration, this.principal.getName(), expiresInOneMinAccessToken);
167+
168+
// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
169+
this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
170+
171+
OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse().build();
172+
when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(accessTokenResponse);
167173

168174
OAuth2AuthorizationContext authorizationContext =
169175
OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
170176
.principal(this.principal)
171177
.build();
172-
assertThat(authorizedClientProvider.authorize(authorizationContext)).isNull();
178+
179+
OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext);
180+
181+
assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
182+
assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
183+
assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
173184
}
174185
}

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/ClientCredentialsReactiveOAuth2AuthorizedClientProviderTests.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,33 @@ public void authorizeWhenClientCredentialsAndTokenNotExpiredThenNotReauthorize()
154154
.build();
155155
assertThat(this.authorizedClientProvider.authorize(authorizationContext).block()).isNull();
156156
}
157+
158+
// gh-7511
159+
@Test
160+
public void authorizeWhenClientCredentialsAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
161+
Instant now = Instant.now();
162+
Instant issuedAt = now.minus(Duration.ofMinutes(60));
163+
Instant expiresAt = now.minus(Duration.ofMinutes(1));
164+
OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
165+
OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
166+
OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(
167+
this.clientRegistration, this.principal.getName(), expiresInOneMinAccessToken);
168+
169+
// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
170+
this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
171+
172+
OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse().build();
173+
when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
174+
175+
OAuth2AuthorizationContext authorizationContext =
176+
OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
177+
.principal(this.principal)
178+
.build();
179+
180+
OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext).block();
181+
182+
assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
183+
assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
184+
assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
185+
}
157186
}

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/PasswordOAuth2AuthorizedClientProviderTests.java

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -188,24 +188,34 @@ public void authorizeWhenPasswordAndAuthorizedWithRefreshTokenAndTokenExpiredThe
188188
assertThat(this.authorizedClientProvider.authorize(authorizationContext)).isNull();
189189
}
190190

191+
// gh-7511
191192
@Test
192-
public void authorizeWhenPasswordAndAuthorizedWithoutRefreshTokenAndTokenNotExpiredByClockSkewThenNotReauthorize() {
193-
PasswordOAuth2AuthorizedClientProvider authorizedClientProvider =
194-
new PasswordOAuth2AuthorizedClientProvider();
195-
authorizedClientProvider.setClockSkew(Duration.ofHours(24));
196-
Instant issuedAt = Instant.now().minus(Duration.ofDays(1));
197-
Instant expiresAt = issuedAt.plus(Duration.ofMinutes(60));
198-
OAuth2AccessToken accessToken = new OAuth2AccessToken(
199-
OAuth2AccessToken.TokenType.BEARER, "access-token-expired", issuedAt, expiresAt);
193+
public void authorizeWhenPasswordAndAuthorizedAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
194+
Instant now = Instant.now();
195+
Instant issuedAt = now.minus(Duration.ofMinutes(60));
196+
Instant expiresAt = now.minus(Duration.ofMinutes(1));
197+
OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
198+
OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
200199
OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(
201-
this.clientRegistration, this.principal.getName(), accessToken); // without refresh token
200+
this.clientRegistration, this.principal.getName(), expiresInOneMinAccessToken); // without refresh token
201+
202+
// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
203+
this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
204+
205+
OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse().build();
206+
when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(accessTokenResponse);
202207

203208
OAuth2AuthorizationContext authorizationContext =
204209
OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
205210
.attribute(OAuth2AuthorizationContext.USERNAME_ATTRIBUTE_NAME, "username")
206211
.attribute(OAuth2AuthorizationContext.PASSWORD_ATTRIBUTE_NAME, "password")
207212
.principal(this.principal)
208213
.build();
209-
assertThat(authorizedClientProvider.authorize(authorizationContext)).isNull();
214+
215+
OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext);
216+
217+
assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
218+
assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
219+
assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
210220
}
211221
}

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/PasswordReactiveOAuth2AuthorizedClientProviderTests.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,4 +188,35 @@ public void authorizeWhenPasswordAndAuthorizedWithRefreshTokenAndTokenExpiredThe
188188
.build();
189189
assertThat(this.authorizedClientProvider.authorize(authorizationContext).block()).isNull();
190190
}
191+
192+
// gh-7511
193+
@Test
194+
public void authorizeWhenPasswordAndAuthorizedAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
195+
Instant now = Instant.now();
196+
Instant issuedAt = now.minus(Duration.ofMinutes(60));
197+
Instant expiresAt = now.minus(Duration.ofMinutes(1));
198+
OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
199+
OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
200+
OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(
201+
this.clientRegistration, this.principal.getName(), expiresInOneMinAccessToken); // without refresh token
202+
203+
// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
204+
this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
205+
206+
OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse().build();
207+
when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
208+
209+
OAuth2AuthorizationContext authorizationContext =
210+
OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
211+
.attribute(OAuth2AuthorizationContext.USERNAME_ATTRIBUTE_NAME, "username")
212+
.attribute(OAuth2AuthorizationContext.PASSWORD_ATTRIBUTE_NAME, "password")
213+
.principal(this.principal)
214+
.build();
215+
216+
OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext).block();
217+
218+
assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
219+
assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
220+
assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
221+
}
191222
}

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/RefreshTokenOAuth2AuthorizedClientProviderTests.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,38 @@ public void authorizeWhenAuthorizedAndAccessTokenNotExpiredThenNotReauthorize()
134134
assertThat(this.authorizedClientProvider.authorize(authorizationContext)).isNull();
135135
}
136136

137+
// gh-7511
138+
@Test
139+
public void authorizeWhenAuthorizedAndAccessTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
140+
OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse()
141+
.refreshToken("new-refresh-token")
142+
.build();
143+
when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(accessTokenResponse);
144+
145+
Instant now = Instant.now();
146+
Instant issuedAt = now.minus(Duration.ofMinutes(60));
147+
Instant expiresAt = now.minus(Duration.ofMinutes(1));
148+
OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
149+
OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
150+
OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(this.clientRegistration, this.principal.getName(),
151+
expiresInOneMinAccessToken, this.authorizedClient.getRefreshToken());
152+
153+
// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
154+
this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
155+
156+
OAuth2AuthorizationContext authorizationContext =
157+
OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
158+
.principal(this.principal)
159+
.build();
160+
161+
OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext);
162+
163+
assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
164+
assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
165+
assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
166+
assertThat(reauthorizedClient.getRefreshToken()).isEqualTo(accessTokenResponse.getRefreshToken());
167+
}
168+
137169
@Test
138170
public void authorizeWhenAuthorizedAndAccessTokenExpiredThenReauthorize() {
139171
OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse()

‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/RefreshTokenReactiveOAuth2AuthorizedClientProviderTests.java

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,19 +135,36 @@ public void authorizeWhenAuthorizedAndAccessTokenNotExpiredThenNotReauthorize()
135135
assertThat(this.authorizedClientProvider.authorize(authorizationContext).block()).isNull();
136136
}
137137

138+
// gh-7511
138139
@Test
139-
public void authorizeWhenAuthorizedAndAccessTokenNotExpiredByClockSkewThenNotReauthorize() {
140-
RefreshTokenReactiveOAuth2AuthorizedClientProvider authorizedClientProvider
141-
= new RefreshTokenReactiveOAuth2AuthorizedClientProvider();
142-
authorizedClientProvider.setClockSkew(Duration.ofHours(24));
140+
public void authorizeWhenAuthorizedAndAccessTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
141+
OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse()
142+
.refreshToken("new-refresh-token")
143+
.build();
144+
when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
145+
146+
Instant now = Instant.now();
147+
Instant issuedAt = now.minus(Duration.ofMinutes(60));
148+
Instant expiresAt = now.minus(Duration.ofMinutes(1));
149+
OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
150+
OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
143151
OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(this.clientRegistration, this.principal.getName(),
144-
this.authorizedClient.getAccessToken(), this.authorizedClient.getRefreshToken());
152+
expiresInOneMinAccessToken, this.authorizedClient.getRefreshToken());
153+
154+
// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
155+
this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
145156

146157
OAuth2AuthorizationContext authorizationContext =
147158
OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
148159
.principal(this.principal)
149160
.build();
150-
assertThat(authorizedClientProvider.authorize(authorizationContext).block()).isNull();
161+
162+
OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext).block();
163+
164+
assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
165+
assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
166+
assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
167+
assertThat(reauthorizedClient.getRefreshToken()).isEqualTo(accessTokenResponse.getRefreshToken());
151168
}
152169

153170
@Test

0 commit comments

Comments
 (0)