Address reviewer requested changes

Closes gh-17806

Signed-off-by: Bernard Budano <[email protected]>

Author: bbudano

docs/modules/ROOT/pages/features/integrations/rest/http-interface.adoc

@@ -5,7 +5,7 @@ Spring Security's OAuth Support can integrate with `RestClient` and `WebClient`
 
 [[configuration]]
 == Configuration
-After xref:features/integrations/rest/http-interface.adoc#configuration-restclient[RestClient] or xref:features/integrations/rest/http-interface.adoc#configuration-webclient[WebClient] specific configuration, usage of xref:features/integrations/rest/http-interface.adoc[] only requires adding a xref:features/integrations/rest/http-interface.adoc#client-registration-id[`@ClientRegistrationId`] to methods that require OAuth.
+After xref:features/integrations/rest/http-interface.adoc#configuration-restclient[RestClient] or xref:features/integrations/rest/http-interface.adoc#configuration-webclient[WebClient] specific configuration, usage of xref:features/integrations/rest/http-interface.adoc[] only requires adding a xref:features/integrations/rest/http-interface.adoc#client-registration-id[`@ClientRegistrationId`] to methods that require OAuth or their declaring HTTP interface.
 
 Since the presence of xref:features/integrations/rest/http-interface.adoc#client-registration-id[`@ClientRegistrationId`] determines if and how the OAuth token will be resolved, it is safe to add Spring Security's OAuth support any configuration.
 

docs/modules/ROOT/pages/whats-new.adoc

@@ -49,6 +49,7 @@ http.csrf((csrf) -> csrf.spa());
 * Added OAuth2 Support for xref:features/integrations/rest/http-interface.adoc[HTTP Interface Integration]
 * Added support for custom `JwkSource` in `NimbusJwtDecoder`, allowing usage of Nimbus's `JwkSourceBuilder` API
 * Added builder for `NimbusJwtEncoder`, supports specifying an EC or RSA key pair or a secret key
+* Added support for `@ClientRegistrationId` at class level, eliminating the need for method level repetition
 
 == SAML 2.0
 

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/client/ClientRegistrationIdProcessor.java

@@ -17,12 +17,12 @@
 package org.springframework.security.oauth2.client.web.client;
 
 import java.lang.reflect.Method;
-import java.util.Optional;
 
 import org.jspecify.annotations.Nullable;
 
 import org.springframework.core.MethodParameter;
-import org.springframework.core.annotation.AnnotationUtils;
+import org.springframework.security.core.annotation.SecurityAnnotationScanner;
+import org.springframework.security.core.annotation.SecurityAnnotationScanners;
 import org.springframework.security.oauth2.client.annotation.ClientRegistrationId;
 import org.springframework.security.oauth2.client.web.ClientAttributes;
 import org.springframework.web.service.invoker.HttpRequestValues;
@@ -38,20 +38,21 @@ public final class ClientRegistrationIdProcessor implements HttpRequestValues.Pr
 
 	public static ClientRegistrationIdProcessor DEFAULT_INSTANCE = new ClientRegistrationIdProcessor();
 
-	private ClientRegistrationIdProcessor() {
-	}
+	private SecurityAnnotationScanner<ClientRegistrationId> securityAnnotationScanner = SecurityAnnotationScanners
+		.requireUnique(ClientRegistrationId.class);
 
 	@Override
 	public void process(Method method, MethodParameter[] parameters, @Nullable Object[] arguments,
 			HttpRequestValues.Builder builder) {
-		ClientRegistrationId registeredId = Optional
-			.ofNullable(AnnotationUtils.findAnnotation(method, ClientRegistrationId.class))
-			.orElseGet(() -> AnnotationUtils.findAnnotation(method.getDeclaringClass(), ClientRegistrationId.class));
+		ClientRegistrationId registeredId = this.securityAnnotationScanner.scan(method, method.getDeclaringClass());
 
 		if (registeredId != null) {
 			String registrationId = registeredId.registrationId();
 			builder.configureAttributes(ClientAttributes.clientRegistrationId(registrationId));
 		}
 	}
 
+	private ClientRegistrationIdProcessor() {
+	}
+
 }

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/client/ClientRegistrationIdProcessorTests.java

@@ -22,12 +22,14 @@
 
 import org.junit.jupiter.api.Test;
 
+import org.springframework.core.annotation.AnnotationConfigurationException;
 import org.springframework.security.oauth2.client.annotation.ClientRegistrationId;
 import org.springframework.security.oauth2.client.web.ClientAttributes;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.web.service.invoker.HttpRequestValues;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
 /**
  * Unit tests for {@link ClientRegistrationIdProcessor}.
@@ -56,9 +58,8 @@ void processWhenClientRegistrationIdPresentThenSet() {
 	@Test
 	void processWhenMetaClientRegistrationIdPresentThenSet() {
 		HttpRequestValues.Builder builder = HttpRequestValues.builder();
-		Method hasMetaClientRegistrationId = ReflectionUtils.findMethod(RestService.class,
-				"hasMetaClientRegistrationId");
-		this.processor.process(hasMetaClientRegistrationId, null, null, builder);
+		Method hasClientRegistrationId = ReflectionUtils.findMethod(RestService.class, "hasMetaClientRegistrationId");
+		this.processor.process(hasClientRegistrationId, null, null, builder);
 
 		String registrationId = ClientAttributes.resolveClientRegistrationId(builder.build().getAttributes());
 		assertThat(registrationId).isEqualTo(REGISTRATION_ID);
@@ -67,8 +68,8 @@ void processWhenMetaClientRegistrationIdPresentThenSet() {
 	@Test
 	void processWhenNoClientRegistrationIdPresentThenNull() {
 		HttpRequestValues.Builder builder = HttpRequestValues.builder();
-		Method noClientRegistrationId = ReflectionUtils.findMethod(RestService.class, "noClientRegistrationId");
-		this.processor.process(noClientRegistrationId, null, null, builder);
+		Method hasClientRegistrationId = ReflectionUtils.findMethod(RestService.class, "noClientRegistrationId");
+		this.processor.process(hasClientRegistrationId, null, null, builder);
 
 		String registrationId = ClientAttributes.resolveClientRegistrationId(builder.build().getAttributes());
 		assertThat(registrationId).isNull();
@@ -77,14 +78,24 @@ void processWhenNoClientRegistrationIdPresentThenNull() {
 	@Test
 	void processWhenClientRegistrationIdPresentOnDeclaringClassThenSet() {
 		HttpRequestValues.Builder builder = HttpRequestValues.builder();
-		Method declaringClassHasClientRegistrationId = ReflectionUtils.findMethod(AnnotatedRestService.class,
+		Method declaringClassHasClientRegistrationId = ReflectionUtils.findMethod(TypeAnnotatedRestService.class,
 				"declaringClassHasClientRegistrationId");
 		this.processor.process(declaringClassHasClientRegistrationId, null, null, builder);
 
 		String registrationId = ClientAttributes.resolveClientRegistrationId(builder.build().getAttributes());
 		assertThat(registrationId).isEqualTo(REGISTRATION_ID);
 	}
 
+	@Test
+	void processWhenDuplicateClientRegistrationIdPresentOnAggregateServiceThenException() {
+		HttpRequestValues.Builder builder = HttpRequestValues.builder();
+		Method shouldFailDueToDuplicateClientRegistrationId = ReflectionUtils.findMethod(AggregateRestService.class,
+				"shouldFailDueToDuplicateClientRegistrationId");
+
+		assertThatExceptionOfType(AnnotationConfigurationException.class).isThrownBy(
+				() -> this.processor.process(shouldFailDueToDuplicateClientRegistrationId, null, null, builder));
+	}
+
 	interface RestService {
 
 		@ClientRegistrationId(REGISTRATION_ID)
@@ -104,10 +115,26 @@ interface RestService {
 	}
 
 	@ClientRegistrationId(REGISTRATION_ID)
-	interface AnnotatedRestService {
+	interface TypeAnnotatedRestService {
 
 		void declaringClassHasClientRegistrationId();
 
 	}
 
+	@ClientRegistrationId("a")
+	interface ARestService {
+
+	}
+
+	@ClientRegistrationId("b")
+	interface BRestService {
+
+	}
+
+	interface AggregateRestService extends ARestService, BRestService {
+
+		void shouldFailDueToDuplicateClientRegistrationId();
+
+	}
+
 }