Adjust based on review.

docs/src/main/asciidoc/spring-cloud-openfeign.adoc

@@ -813,9 +813,9 @@ Sometimes, when load balancing is enabled for Feign clients, you may want to use
 feign.oauth2.load-balanced=true
 
 When the flag is set to true, and the oauth2 client context resource details are present, a bean of class `OAuth2AccessTokenInterceptor` is created. Before each request, the interceptor resolves the required access token and includes it as a header.
-`OAuth2AccessTokenInterceptor` uses the `AuthorizedClientServiceOAuth2AuthorizedClientManager` to get `OAuth2AuthorizedClient` that holds an `OAuth2AccessToken`. If the user has specified an OAuth2 `clientId` using the `spring.cloud.openfeign.oauth2.clientId` property, it will be used to retrieve the token. If the token is not retrieved or the `clientId` has not been specified, the `serviceId` retrieved from the `url` host segment will be used.
+`OAuth2AccessTokenInterceptor` uses the `AuthorizedClientServiceOAuth2AuthorizedClientManager` to get `OAuth2AuthorizedClient` that holds an `OAuth2AccessToken`. If the user has specified an OAuth2 `clientRegistrationId` using the `spring.cloud.openfeign.oauth2.clientRegistrationId` property, it will be used to retrieve the token. If the token is not retrieved or the `clientRegistrationId` has not been specified, the `serviceId` retrieved from the `url` host segment will be used.
 
-TIP:: Using the `serviceId` as OAuth2 client id is convenient for load-balanced Feign clients. For non-load-balanced ones, the property-based `clientId` is a suitable approach.
+TIP:: Using the `serviceId` as OAuth2 client registrationId is convenient for load-balanced Feign clients. For non-load-balanced ones, the property-based `clientRegistrationId` is a suitable approach.
 
 === Transform the load-balanced HTTP request
 

spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignAutoConfiguration.java

@@ -75,6 +75,7 @@ import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Import;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Sort;
+import org.springframework.security.oauth2.client.OAuth2AuthorizedClientManager;
 import org.springframework.security.oauth2.client.OAuth2AuthorizedClientService;
 import org.springframework.security.oauth2.client.OAuth2ClientContext;
 import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
@@ -380,13 +381,13 @@ public class FeignAutoConfiguration {
 		}
 
 		@Bean
-		@ConditionalOnBean({OAuth2AuthorizedClientService.class, ClientRegistrationRepository.class})
+		@ConditionalOnBean({ OAuth2AuthorizedClientService.class, ClientRegistrationRepository.class })
 		public OAuth2AccessTokenInterceptor defaultOAuth2AccessTokenInterceptor(
-			@Value("${spring.cloud.openfeign.oauth2.clientId:}") String clientId,
-			OAuth2AuthorizedClientService oAuth2AuthorizedClientService,
-			ClientRegistrationRepository clientRegistrationRepository) {
-			return new OAuth2AccessTokenInterceptor(clientId, oAuth2AuthorizedClientService,
-				clientRegistrationRepository);
+				@Value("${spring.cloud.openfeign.oauth2.clientRegistrationId:}") String clientRegistrationId,
+				OAuth2AuthorizedClientService oAuth2AuthorizedClientService,
+				ClientRegistrationRepository clientRegistrationRepository) {
+			return new OAuth2AccessTokenInterceptor(clientRegistrationId, oAuth2AuthorizedClientService,
+					clientRegistrationRepository);
 		}
 
 	}

spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/security/OAuth2AccessTokenInterceptor.java

@@ -17,7 +17,6 @@
 package org.springframework.cloud.openfeign.security;
 
 import java.net.URI;
-import java.time.Instant;
 import java.util.Optional;
 
 import feign.RequestInterceptor;
@@ -69,9 +68,7 @@ public class OAuth2AccessTokenInterceptor implements RequestInterceptor {
 
 	private final String header;
 
-	private final String clientId;
-
-	private final OAuth2AuthorizedClientService oAuth2AuthorizedClientService;
+	private final String clientRegistrationId;
 
 	private OAuth2AuthorizedClientManager authorizedClientManager;
 
@@ -87,20 +84,20 @@ public class OAuth2AccessTokenInterceptor implements RequestInterceptor {
 		this(null, oAuth2AuthorizedClientService, clientRegistrationRepository);
 	}
 
-	public OAuth2AccessTokenInterceptor(String clientId, OAuth2AuthorizedClientService oAuth2AuthorizedClientService,
+	public OAuth2AccessTokenInterceptor(String clientRegistrationId,
+			OAuth2AuthorizedClientService oAuth2AuthorizedClientService,
 			ClientRegistrationRepository clientRegistrationRepository) {
-		this(BEARER, AUTHORIZATION, clientId, oAuth2AuthorizedClientService, clientRegistrationRepository);
+		this(BEARER, AUTHORIZATION, clientRegistrationId, oAuth2AuthorizedClientService, clientRegistrationRepository);
 	}
 
-	public OAuth2AccessTokenInterceptor(String tokenType, String header, String clientId,
+	public OAuth2AccessTokenInterceptor(String tokenType, String header, String clientRegistrationId,
 			OAuth2AuthorizedClientService oAuth2AuthorizedClientService,
 			ClientRegistrationRepository clientRegistrationRepository) {
 		this.tokenType = tokenType;
 		this.header = header;
-		this.clientId = clientId;
-		this.oAuth2AuthorizedClientService = oAuth2AuthorizedClientService;
+		this.clientRegistrationId = clientRegistrationId;
 		this.authorizedClientManager = new AuthorizedClientServiceOAuth2AuthorizedClientManager(
-				clientRegistrationRepository, this.oAuth2AuthorizedClientService);
+				clientRegistrationRepository, oAuth2AuthorizedClientService);
 	}
 
 	@Override
@@ -113,8 +110,8 @@ public class OAuth2AccessTokenInterceptor implements RequestInterceptor {
 
 	public OAuth2AccessToken getToken(RequestTemplate template) {
 		// If specified, try to use them to get token.
-		if (StringUtils.hasText(clientId)) {
-			OAuth2AccessToken token = getToken(clientId);
+		if (StringUtils.hasText(clientRegistrationId)) {
+			OAuth2AccessToken token = getToken(clientRegistrationId);
 			if (token != null) {
 				return token;
 			}
@@ -129,8 +126,8 @@ public class OAuth2AccessTokenInterceptor implements RequestInterceptor {
 		throw new IllegalStateException("OAuth2 token has not been successfully acquired.");
 	}
 
-	protected OAuth2AccessToken getToken(String clientId) {
-		if (!StringUtils.hasText(clientId)) {
+	protected OAuth2AccessToken getToken(String clientRegistrationId) {
+		if (!StringUtils.hasText(clientRegistrationId)) {
 			return null;
 		}
 
@@ -139,26 +136,10 @@ public class OAuth2AccessTokenInterceptor implements RequestInterceptor {
 			principal = ANONYMOUS_AUTHENTICATION;
 		}
 
-		// Already exist
-		OAuth2AuthorizedClient oAuth2AuthorizedClient = oAuth2AuthorizedClientService.loadAuthorizedClient(clientId,
-				principal.getName());
-		if (oAuth2AuthorizedClient != null) {
-			OAuth2AccessToken accessToken = oAuth2AuthorizedClient.getAccessToken();
-			if (accessToken != null && notExpired(accessToken)) {
-				return accessToken;
-			}
-		}
-
-		OAuth2AuthorizeRequest authorizeRequest = OAuth2AuthorizeRequest.withClientRegistrationId(clientId)
+		OAuth2AuthorizeRequest authorizeRequest = OAuth2AuthorizeRequest.withClientRegistrationId(clientRegistrationId)
 				.principal(principal).build();
 		OAuth2AuthorizedClient authorizedClient = authorizedClientManager.authorize(authorizeRequest);
-		return Optional.ofNullable(authorizedClient).map(OAuth2AuthorizedClient::getAccessToken)
-				.filter(this::notExpired).orElse(null);
-	}
-
-	protected boolean notExpired(OAuth2AccessToken token) {
-		return Optional.ofNullable(token).map(OAuth2AccessToken::getExpiresAt)
-				.map(expire -> expire.isAfter(Instant.now())).orElse(false);
+		return Optional.ofNullable(authorizedClient).map(OAuth2AuthorizedClient::getAccessToken).orElse(null);
 	}
 
 	private static String getServiceId(RequestTemplate template) {

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignAutoConfigurationTests.java

@@ -139,13 +139,13 @@ class FeignAutoConfigurationTests {
 	@Test
 	void shouldInstantiateFeignOAuth2FeignRequestInterceptor() {
 		runner.withPropertyValues("spring.cloud.openfeign.oauth2.enabled=true",
-				"spring.cloud.openfeign.oauth2.clientId=feign-client")
-			.withBean(OAuth2AuthorizedClientService.class, () -> mock(OAuth2AuthorizedClientService.class))
-			.withBean(ClientRegistrationRepository.class, () -> mock(ClientRegistrationRepository.class))
-			.run(ctx -> {
-				assertOauth2AccessTokenInterceptorExists(ctx);
-				assertThatOauth2AccessTokenInterceptorHasSpecifiedIdsPropertyWithValue(ctx, "feign-client");
-			});
+				"spring.cloud.openfeign.oauth2.clientRegistrationId=feign-client")
+				.withBean(OAuth2AuthorizedClientService.class, () -> mock(OAuth2AuthorizedClientService.class))
+				.withBean(ClientRegistrationRepository.class, () -> mock(ClientRegistrationRepository.class))
+				.run(ctx -> {
+					assertOauth2AccessTokenInterceptorExists(ctx);
+					assertThatOauth2AccessTokenInterceptorHasSpecifiedIdsPropertyWithValue(ctx, "feign-client");
+				});
 	}
 
 	private void assertOauth2FeignRequestInterceptorExists(ConfigurableApplicationContext ctx) {
@@ -177,20 +177,19 @@ class FeignAutoConfigurationTests {
 	}
 
 	private void assertThatOauth2AccessTokenInterceptorHasSpecifiedIdsPropertyWithValue(
-		ConfigurableApplicationContext ctx, String expectedValue) {
+			ConfigurableApplicationContext ctx, String expectedValue) {
 		final OAuth2AccessTokenInterceptor bean = ctx.getBean(OAuth2AccessTokenInterceptor.class);
-		assertThat(bean).hasFieldOrPropertyWithValue("clientId", expectedValue);
+		assertThat(bean).hasFieldOrPropertyWithValue("clientRegistrationId", expectedValue);
 	}
 
 	private void assertOnlyOneTargeterPresent(ConfigurableApplicationContext ctx, Class<?> beanClass) {
-		assertThat(ctx.getBeansOfType(Targeter.class)).hasSize(1)
-			.hasValueSatisfying(new Condition<>(
+		assertThat(ctx.getBeansOfType(Targeter.class)).hasSize(1).hasValueSatisfying(new Condition<>(
 				beanClass::isInstance, String.format("Targeter should be an instance of %s", beanClass)));
 
 	}
 
 	private void assertThatFeignCircuitBreakerTargeterHasGroupEnabledPropertyWithValue(
-		ConfigurableApplicationContext ctx, boolean expectedValue) {
+			ConfigurableApplicationContext ctx, boolean expectedValue) {
 		final FeignCircuitBreakerTargeter bean = ctx.getBean(FeignCircuitBreakerTargeter.class);
 		assertThat(bean).hasFieldOrPropertyWithValue("circuitBreakerGroupEnabled", expectedValue);
 	}

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/security/OAuth2AccessTokenInterceptorTests.java

@@ -32,14 +32,11 @@ import org.springframework.security.oauth2.client.registration.ClientRegistratio
 import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
 import org.springframework.security.oauth2.core.AuthorizationGrantType;
 import org.springframework.security.oauth2.core.OAuth2AccessToken;
-import org.springframework.util.AlternativeJdkIdGenerator;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.argThat;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -52,6 +49,9 @@ import static org.mockito.Mockito.when;
  */
 class OAuth2AccessTokenInterceptorTests {
 
+	private final ClientRegistrationRepository mockClientRegistrationRepository = mock(
+			ClientRegistrationRepository.class);
+
 	private final OAuth2AuthorizedClientService mockOAuth2AuthorizedClientService = mock(
 			OAuth2AuthorizedClientService.class);
 
@@ -62,7 +62,7 @@ class OAuth2AccessTokenInterceptorTests {
 
 	private RequestTemplate requestTemplate;
 
-	private static final String DEFAULT_CLIENT_ID = "feign-client";
+	private static final String DEFAULT_CLIENT_REGISTRATION_ID = "feign-client";
 
 	@BeforeEach
 	void setUp() {
@@ -74,9 +74,8 @@ class OAuth2AccessTokenInterceptorTests {
 
 	@Test
 	void shouldThrowExceptionWhenNoTokenAcquired() {
-		when(mockOAuth2AuthorizedClientService.loadAuthorizedClient(anyString(), anyString())).thenReturn(null);
 		oAuth2AccessTokenInterceptor = new OAuth2AccessTokenInterceptor(mockOAuth2AuthorizedClientService,
-				mock(ClientRegistrationRepository.class));
+				mockClientRegistrationRepository);
 		when(mockOAuth2AuthorizedClientManager.authorize(any())).thenReturn(null);
 		oAuth2AccessTokenInterceptor.setAuthorizedClientManager(mockOAuth2AuthorizedClientManager);
 
@@ -87,12 +86,9 @@ class OAuth2AccessTokenInterceptorTests {
 
 	@Test
 	void shouldAcquireValidToken() {
-		when(mockOAuth2AuthorizedClientService.loadAuthorizedClient(anyString(), anyString())).thenReturn(null);
 		oAuth2AccessTokenInterceptor = new OAuth2AccessTokenInterceptor(mockOAuth2AuthorizedClientService,
-				mock(ClientRegistrationRepository.class));
-		when(mockOAuth2AuthorizedClientManager.authorize(
-				argThat((OAuth2AuthorizeRequest request) -> ("test").equals(request.getClientRegistrationId()))))
-						.thenReturn(validTokenOAuth2AuthorizedClient());
+				mockClientRegistrationRepository);
+		when(mockOAuth2AuthorizedClientManager.authorize(any())).thenReturn(validTokenOAuth2AuthorizedClient());
 		oAuth2AccessTokenInterceptor.setAuthorizedClientManager(mockOAuth2AuthorizedClientManager);
 
 		oAuth2AccessTokenInterceptor.apply(requestTemplate);
@@ -101,25 +97,14 @@ class OAuth2AccessTokenInterceptorTests {
 	}
 
 	@Test
-	void shouldThrowExceptionWhenExpiredTokenAcquired() {
-		when(mockOAuth2AuthorizedClientService.loadAuthorizedClient(anyString(), anyString())).thenReturn(null);
+	void shouldAcquireValidTokenFromServiceId() {
 		oAuth2AccessTokenInterceptor = new OAuth2AccessTokenInterceptor(mockOAuth2AuthorizedClientService,
-				mock(ClientRegistrationRepository.class));
-		when(mockOAuth2AuthorizedClientManager.authorize(any())).thenReturn(expiredTokenOAuth2AuthorizedClient());
+				mockClientRegistrationRepository);
+		when(mockOAuth2AuthorizedClientManager.authorize(
+				argThat((OAuth2AuthorizeRequest request) -> ("test").equals(request.getClientRegistrationId()))))
+						.thenReturn(validTokenOAuth2AuthorizedClient());
 		oAuth2AccessTokenInterceptor.setAuthorizedClientManager(mockOAuth2AuthorizedClientManager);
 
-		assertThatExceptionOfType(IllegalStateException.class)
-				.isThrownBy(() -> oAuth2AccessTokenInterceptor.apply(requestTemplate))
-				.withMessage("OAuth2 token has not been successfully acquired.");
-	}
-
-	@Test
-	void shouldAcquireTokenFromAuthorizedClient() {
-		when(mockOAuth2AuthorizedClientService.loadAuthorizedClient(eq("test"), anyString()))
-				.thenReturn(validTokenOAuth2AuthorizedClient());
-		oAuth2AccessTokenInterceptor = new OAuth2AccessTokenInterceptor(mockOAuth2AuthorizedClientService,
-				mock(ClientRegistrationRepository.class));
-
 		oAuth2AccessTokenInterceptor.apply(requestTemplate);
 
 		assertThat(requestTemplate.headers().get("Authorization")).contains("Bearer Valid Token");
@@ -127,10 +112,12 @@ class OAuth2AccessTokenInterceptorTests {
 
 	@Test
 	void shouldAcquireValidTokenFromSpecifiedClientId() {
-		when(mockOAuth2AuthorizedClientService.loadAuthorizedClient(eq("testId"), anyString()))
-				.thenReturn(validTokenOAuth2AuthorizedClient());
-		oAuth2AccessTokenInterceptor = new OAuth2AccessTokenInterceptor("testId", mockOAuth2AuthorizedClientService,
-				mock(ClientRegistrationRepository.class));
+		oAuth2AccessTokenInterceptor = new OAuth2AccessTokenInterceptor(DEFAULT_CLIENT_REGISTRATION_ID,
+				mockOAuth2AuthorizedClientService, mockClientRegistrationRepository);
+		when(mockOAuth2AuthorizedClientManager
+				.authorize(argThat((OAuth2AuthorizeRequest request) -> (DEFAULT_CLIENT_REGISTRATION_ID)
+						.equals(request.getClientRegistrationId())))).thenReturn(validTokenOAuth2AuthorizedClient());
+		oAuth2AccessTokenInterceptor.setAuthorizedClientManager(mockOAuth2AuthorizedClientManager);
 
 		oAuth2AccessTokenInterceptor.apply(requestTemplate);
 
@@ -142,23 +129,13 @@ class OAuth2AccessTokenInterceptorTests {
 				Instant.now().plusSeconds(60L));
 	}
 
-	private OAuth2AccessToken expiredToken() {
-		return new OAuth2AccessToken(OAuth2AccessToken.TokenType.BEARER, "Expired Token",
-				Instant.now().minusSeconds(61L), Instant.now().minusSeconds(60L));
-	}
-
 	private OAuth2AuthorizedClient validTokenOAuth2AuthorizedClient() {
 		return new OAuth2AuthorizedClient(defaultClientRegistration(), "anonymousUser", validToken());
 	}
 
-	private OAuth2AuthorizedClient expiredTokenOAuth2AuthorizedClient() {
-		return new OAuth2AuthorizedClient(defaultClientRegistration(), "anonymousUser", expiredToken());
-	}
-
 	private ClientRegistration defaultClientRegistration() {
-		return ClientRegistration.withRegistrationId(new AlternativeJdkIdGenerator().generateId().toString())
-				.clientId(DEFAULT_CLIENT_ID).tokenUri("mock token uri")
-				.authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS).build();
+		return ClientRegistration.withRegistrationId(DEFAULT_CLIENT_REGISTRATION_ID).clientId("clientId")
+				.tokenUri("mock token uri").authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS).build();
 	}
 
 }