浏览代码

Fix access token expiry check with clock skew

Fixes gh-7511
Joe Grandja 5 年之前
父节点
当前提交
1c53a7859b
共有 12 个文件被更改,包括 161 次插入31 次删除
  1. 1 1
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/ClientCredentialsOAuth2AuthorizedClientProvider.java
  2. 1 1
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/ClientCredentialsReactiveOAuth2AuthorizedClientProvider.java
  3. 1 1
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/PasswordOAuth2AuthorizedClientProvider.java
  4. 1 1
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/PasswordReactiveOAuth2AuthorizedClientProvider.java
  5. 1 1
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/RefreshTokenOAuth2AuthorizedClientProvider.java
  6. 1 1
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/RefreshTokenReactiveOAuth2AuthorizedClientProvider.java
  7. 20 9
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/ClientCredentialsOAuth2AuthorizedClientProviderTests.java
  8. 29 0
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/ClientCredentialsReactiveOAuth2AuthorizedClientProviderTests.java
  9. 20 10
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/PasswordOAuth2AuthorizedClientProviderTests.java
  10. 31 0
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/PasswordReactiveOAuth2AuthorizedClientProviderTests.java
  11. 32 0
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/RefreshTokenOAuth2AuthorizedClientProviderTests.java
  12. 23 6
      oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/RefreshTokenReactiveOAuth2AuthorizedClientProviderTests.java

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

@@ -86,7 +86,7 @@ public final class ClientCredentialsOAuth2AuthorizedClientProvider implements OA
 	}
 
 	private boolean hasTokenExpired(AbstractOAuth2Token token) {
-		return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
+		return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
 	}
 
 	/**

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

@@ -82,7 +82,7 @@ public final class ClientCredentialsReactiveOAuth2AuthorizedClientProvider imple
 	}
 
 	private boolean hasTokenExpired(AbstractOAuth2Token token) {
-		return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
+		return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
 	}
 
 	/**

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

@@ -104,7 +104,7 @@ public final class PasswordOAuth2AuthorizedClientProvider implements OAuth2Autho
 	}
 
 	private boolean hasTokenExpired(AbstractOAuth2Token token) {
-		return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
+		return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
 	}
 
 	/**

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

@@ -102,7 +102,7 @@ public final class PasswordReactiveOAuth2AuthorizedClientProvider implements Rea
 	}
 
 	private boolean hasTokenExpired(AbstractOAuth2Token token) {
-		return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
+		return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
 	}
 
 	/**

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

@@ -94,7 +94,7 @@ public final class RefreshTokenOAuth2AuthorizedClientProvider implements OAuth2A
 	}
 
 	private boolean hasTokenExpired(AbstractOAuth2Token token) {
-		return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
+		return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
 	}
 
 	/**

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

@@ -93,7 +93,7 @@ public final class RefreshTokenReactiveOAuth2AuthorizedClientProvider implements
 	}
 
 	private boolean hasTokenExpired(AbstractOAuth2Token token) {
-		return token.getExpiresAt().isBefore(Instant.now(this.clock).minus(this.clockSkew));
+		return this.clock.instant().isAfter(token.getExpiresAt().minus(this.clockSkew));
 	}
 
 	/**

+ 20 - 9
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/ClientCredentialsOAuth2AuthorizedClientProviderTests.java

@@ -154,21 +154,32 @@ public class ClientCredentialsOAuth2AuthorizedClientProviderTests {
 		assertThat(this.authorizedClientProvider.authorize(authorizationContext)).isNull();
 	}
 
+	// gh-7511
 	@Test
-	public void authorizeWhenClientCredentialsAndTokenNotExpiredByClockSkewThenNotReauthorize() {
-		ClientCredentialsOAuth2AuthorizedClientProvider authorizedClientProvider =
-				new ClientCredentialsOAuth2AuthorizedClientProvider();
-		authorizedClientProvider.setClockSkew(Duration.ofHours(24));
-		Instant issuedAt = Instant.now().minus(Duration.ofDays(1));
-		OAuth2AccessToken expiredToken = new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER, "token",
-				issuedAt, issuedAt.plus(Duration.ofHours(1)));
+	public void authorizeWhenClientCredentialsAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
+		Instant now = Instant.now();
+		Instant issuedAt = now.minus(Duration.ofMinutes(60));
+		Instant expiresAt = now.minus(Duration.ofMinutes(1));
+		OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
+				OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
 		OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(
-				this.clientRegistration, this.principal.getName(), expiredToken);
+				this.clientRegistration, this.principal.getName(), expiresInOneMinAccessToken);
+
+		// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
+		this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
+
+		OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse().build();
+		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(accessTokenResponse);
 
 		OAuth2AuthorizationContext authorizationContext =
 				OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
 						.principal(this.principal)
 						.build();
-		assertThat(authorizedClientProvider.authorize(authorizationContext)).isNull();
+
+		OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext);
+
+		assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
+		assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
+		assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
 	}
 }

+ 29 - 0
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/ClientCredentialsReactiveOAuth2AuthorizedClientProviderTests.java

@@ -154,4 +154,33 @@ public class ClientCredentialsReactiveOAuth2AuthorizedClientProviderTests {
 						.build();
 		assertThat(this.authorizedClientProvider.authorize(authorizationContext).block()).isNull();
 	}
+
+	// gh-7511
+	@Test
+	public void authorizeWhenClientCredentialsAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
+		Instant now = Instant.now();
+		Instant issuedAt = now.minus(Duration.ofMinutes(60));
+		Instant expiresAt = now.minus(Duration.ofMinutes(1));
+		OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
+				OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
+		OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(
+				this.clientRegistration, this.principal.getName(), expiresInOneMinAccessToken);
+
+		// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
+		this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
+
+		OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse().build();
+		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
+
+		OAuth2AuthorizationContext authorizationContext =
+				OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
+						.principal(this.principal)
+						.build();
+
+		OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext).block();
+
+		assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
+		assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
+		assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
+	}
 }

+ 20 - 10
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/PasswordOAuth2AuthorizedClientProviderTests.java

@@ -188,17 +188,22 @@ public class PasswordOAuth2AuthorizedClientProviderTests {
 		assertThat(this.authorizedClientProvider.authorize(authorizationContext)).isNull();
 	}
 
+	// gh-7511
 	@Test
-	public void authorizeWhenPasswordAndAuthorizedWithoutRefreshTokenAndTokenNotExpiredByClockSkewThenNotReauthorize() {
-		PasswordOAuth2AuthorizedClientProvider authorizedClientProvider =
-				new PasswordOAuth2AuthorizedClientProvider();
-		authorizedClientProvider.setClockSkew(Duration.ofHours(24));
-		Instant issuedAt = Instant.now().minus(Duration.ofDays(1));
-		Instant expiresAt = issuedAt.plus(Duration.ofMinutes(60));
-		OAuth2AccessToken accessToken = new OAuth2AccessToken(
-				OAuth2AccessToken.TokenType.BEARER, "access-token-expired", issuedAt, expiresAt);
+	public void authorizeWhenPasswordAndAuthorizedAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
+		Instant now = Instant.now();
+		Instant issuedAt = now.minus(Duration.ofMinutes(60));
+		Instant expiresAt = now.minus(Duration.ofMinutes(1));
+		OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
+				OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
 		OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(
-				this.clientRegistration, this.principal.getName(), accessToken);	// without refresh token
+				this.clientRegistration, this.principal.getName(), expiresInOneMinAccessToken);		// without refresh token
+
+		// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
+		this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
+
+		OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse().build();
+		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(accessTokenResponse);
 
 		OAuth2AuthorizationContext authorizationContext =
 				OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
@@ -206,6 +211,11 @@ public class PasswordOAuth2AuthorizedClientProviderTests {
 						.attribute(OAuth2AuthorizationContext.PASSWORD_ATTRIBUTE_NAME, "password")
 						.principal(this.principal)
 						.build();
-		assertThat(authorizedClientProvider.authorize(authorizationContext)).isNull();
+
+		OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext);
+
+		assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
+		assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
+		assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
 	}
 }

+ 31 - 0
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/PasswordReactiveOAuth2AuthorizedClientProviderTests.java

@@ -188,4 +188,35 @@ public class PasswordReactiveOAuth2AuthorizedClientProviderTests {
 						.build();
 		assertThat(this.authorizedClientProvider.authorize(authorizationContext).block()).isNull();
 	}
+
+	// gh-7511
+	@Test
+	public void authorizeWhenPasswordAndAuthorizedAndTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
+		Instant now = Instant.now();
+		Instant issuedAt = now.minus(Duration.ofMinutes(60));
+		Instant expiresAt = now.minus(Duration.ofMinutes(1));
+		OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
+				OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
+		OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(
+				this.clientRegistration, this.principal.getName(), expiresInOneMinAccessToken);		// without refresh token
+
+		// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
+		this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
+
+		OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse().build();
+		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
+
+		OAuth2AuthorizationContext authorizationContext =
+				OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
+						.attribute(OAuth2AuthorizationContext.USERNAME_ATTRIBUTE_NAME, "username")
+						.attribute(OAuth2AuthorizationContext.PASSWORD_ATTRIBUTE_NAME, "password")
+						.principal(this.principal)
+						.build();
+
+		OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext).block();
+
+		assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
+		assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
+		assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
+	}
 }

+ 32 - 0
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/RefreshTokenOAuth2AuthorizedClientProviderTests.java

@@ -134,6 +134,38 @@ public class RefreshTokenOAuth2AuthorizedClientProviderTests {
 		assertThat(this.authorizedClientProvider.authorize(authorizationContext)).isNull();
 	}
 
+	// gh-7511
+	@Test
+	public void authorizeWhenAuthorizedAndAccessTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
+		OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse()
+				.refreshToken("new-refresh-token")
+				.build();
+		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(accessTokenResponse);
+
+		Instant now = Instant.now();
+		Instant issuedAt = now.minus(Duration.ofMinutes(60));
+		Instant expiresAt = now.minus(Duration.ofMinutes(1));
+		OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
+				OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
+		OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(this.clientRegistration, this.principal.getName(),
+				expiresInOneMinAccessToken, this.authorizedClient.getRefreshToken());
+
+		// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
+		this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
+
+		OAuth2AuthorizationContext authorizationContext =
+				OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
+						.principal(this.principal)
+						.build();
+
+		OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext);
+
+		assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
+		assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
+		assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
+		assertThat(reauthorizedClient.getRefreshToken()).isEqualTo(accessTokenResponse.getRefreshToken());
+	}
+
 	@Test
 	public void authorizeWhenAuthorizedAndAccessTokenExpiredThenReauthorize() {
 		OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse()

+ 23 - 6
oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/RefreshTokenReactiveOAuth2AuthorizedClientProviderTests.java

@@ -135,19 +135,36 @@ public class RefreshTokenReactiveOAuth2AuthorizedClientProviderTests {
 		assertThat(this.authorizedClientProvider.authorize(authorizationContext).block()).isNull();
 	}
 
+	// gh-7511
 	@Test
-	public void authorizeWhenAuthorizedAndAccessTokenNotExpiredByClockSkewThenNotReauthorize() {
-		RefreshTokenReactiveOAuth2AuthorizedClientProvider authorizedClientProvider
-				= new RefreshTokenReactiveOAuth2AuthorizedClientProvider();
-		authorizedClientProvider.setClockSkew(Duration.ofHours(24));
+	public void authorizeWhenAuthorizedAndAccessTokenNotExpiredButClockSkewForcesExpiryThenReauthorize() {
+		OAuth2AccessTokenResponse accessTokenResponse = TestOAuth2AccessTokenResponses.accessTokenResponse()
+				.refreshToken("new-refresh-token")
+				.build();
+		when(this.accessTokenResponseClient.getTokenResponse(any())).thenReturn(Mono.just(accessTokenResponse));
+
+		Instant now = Instant.now();
+		Instant issuedAt = now.minus(Duration.ofMinutes(60));
+		Instant expiresAt = now.minus(Duration.ofMinutes(1));
+		OAuth2AccessToken expiresInOneMinAccessToken = new OAuth2AccessToken(
+				OAuth2AccessToken.TokenType.BEARER, "access-token-1234", issuedAt, expiresAt);
 		OAuth2AuthorizedClient authorizedClient = new OAuth2AuthorizedClient(this.clientRegistration, this.principal.getName(),
-				this.authorizedClient.getAccessToken(), this.authorizedClient.getRefreshToken());
+				expiresInOneMinAccessToken, this.authorizedClient.getRefreshToken());
+
+		// Shorten the lifespan of the access token by 90 seconds, which will ultimately force it to expire on the client
+		this.authorizedClientProvider.setClockSkew(Duration.ofSeconds(90));
 
 		OAuth2AuthorizationContext authorizationContext =
 				OAuth2AuthorizationContext.withAuthorizedClient(authorizedClient)
 						.principal(this.principal)
 						.build();
-		assertThat(authorizedClientProvider.authorize(authorizationContext).block()).isNull();
+
+		OAuth2AuthorizedClient reauthorizedClient = this.authorizedClientProvider.authorize(authorizationContext).block();
+
+		assertThat(reauthorizedClient.getClientRegistration()).isSameAs(this.clientRegistration);
+		assertThat(reauthorizedClient.getPrincipalName()).isEqualTo(this.principal.getName());
+		assertThat(reauthorizedClient.getAccessToken()).isEqualTo(accessTokenResponse.getAccessToken());
+		assertThat(reauthorizedClient.getRefreshToken()).isEqualTo(accessTokenResponse.getRefreshToken());
 	}
 
 	@Test