Fix OAuth2AuthorizationRequest additionalParameters/attributes Consumer

Fixes gh-8177

Author: jgrandja

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/jackson2/OAuth2AuthorizationRequestMixinTests.java

@@ -27,7 +27,6 @@
 import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 
-import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.stream.Collectors;
@@ -71,8 +70,8 @@ public void serializeWhenRequiredAttributesOnlyThenSerializes() throws Exception
 				this.authorizationRequestBuilder
 						.scopes(null)
 						.state(null)
-						.additionalParameters(Collections.emptyMap())
-						.attributes(Collections.emptyMap())
+						.additionalParameters(Map::clear)
+						.attributes(Map::clear)
 						.build();
 		String expectedJson = asJson(authorizationRequest);
 		String json = this.mapper.writeValueAsString(authorizationRequest);
@@ -119,8 +118,8 @@ public void deserializeWhenRequiredAttributesOnlyThenDeserializes() throws Excep
 				this.authorizationRequestBuilder
 						.scopes(null)
 						.state(null)
-						.additionalParameters(Collections.emptyMap())
-						.attributes(Collections.emptyMap())
+						.additionalParameters(Map::clear)
+						.attributes(Map::clear)
 						.build();
 		String json = asJson(expectedAuthorizationRequest);
 		OAuth2AuthorizationRequest authorizationRequest = this.mapper.readValue(json, OAuth2AuthorizationRequest.class);

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

@@ -437,6 +437,7 @@ public void resolveWhenAuthorizationRequestCustomizerRemovesNonceThenQueryExclud
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
 		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(OidcParameterNames.NONCE);
 		assertThat(authorizationRequest.getAttributes()).doesNotContainKey(OidcParameterNames.NONCE);
+		assertThat(authorizationRequest.getAttributes()).containsKey(OAuth2ParameterNames.REGISTRATION_ID);
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id&" +

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

@@ -29,6 +29,7 @@
 import org.springframework.security.oauth2.client.registration.TestClientRegistrations;
 import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest;
+import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
 import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
 import org.springframework.security.oauth2.core.oidc.OidcScopes;
 import org.springframework.security.oauth2.core.oidc.endpoint.OidcParameterNames;
@@ -162,6 +163,7 @@ public void resolveWhenAuthorizationRequestCustomizerRemovesNonceThenQueryExclud
 
 		assertThat(authorizationRequest.getAdditionalParameters()).doesNotContainKey(OidcParameterNames.NONCE);
 		assertThat(authorizationRequest.getAttributes()).doesNotContainKey(OidcParameterNames.NONCE);
+		assertThat(authorizationRequest.getAttributes()).containsKey(OAuth2ParameterNames.REGISTRATION_ID);
 		assertThat(authorizationRequest.getAuthorizationRequestUri())
 				.matches("https://example.com/login/oauth/authorize\\?" +
 						"response_type=code&client_id=client-id&" +

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

@@ -35,6 +35,7 @@
 import reactor.core.publisher.Mono;
 
 import java.util.Collections;
+import java.util.Map;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -96,7 +97,7 @@ public void applyWhenAuthorizationRequestEmptyThenOAuth2AuthorizationException()
 
 	@Test
 	public void applyWhenAttributesMissingThenOAuth2AuthorizationException() {
-		this.authorizationRequest.attributes(Collections.emptyMap());
+		this.authorizationRequest.attributes(Map::clear);
 		when(this.authorizationRequestRepository.removeAuthorizationRequest(any())).thenReturn(Mono.just(this.authorizationRequest.build()));
 
 		assertThatThrownBy(() -> applyConverter())

oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java

@@ -229,9 +229,9 @@ public static class Builder {
 		private String redirectUri;
 		private Set<String> scopes;
 		private String state;
-		private Consumer<Map<String, Object>> additionalParametersConsumer = params -> {};
+		private Map<String, Object> additionalParameters = new LinkedHashMap<>();
 		private Consumer<Map<String, Object>> parametersConsumer = params -> {};
-		private Consumer<Map<String, Object>> attributesConsumer = attrs -> {};
+		private Map<String, Object> attributes = new LinkedHashMap<>();
 		private String authorizationRequestUri;
 		private Function<UriBuilder, URI> authorizationRequestUriFunction = builder -> builder.build();
 		private final DefaultUriBuilderFactory uriBuilderFactory;
@@ -325,8 +325,8 @@ public Builder state(String state) {
 		 * @return the {@link Builder}
 		 */
 		public Builder additionalParameters(Map<String, Object> additionalParameters) {
-			if (additionalParameters != null) {
-				return additionalParameters(params -> params.putAll(additionalParameters));
+			if (!CollectionUtils.isEmpty(additionalParameters)) {
+				this.additionalParameters.putAll(additionalParameters);
 			}
 			return this;
 		}
@@ -340,7 +340,7 @@ public Builder additionalParameters(Map<String, Object> additionalParameters) {
 		 */
 		public Builder additionalParameters(Consumer<Map<String, Object>> additionalParametersConsumer) {
 			if (additionalParametersConsumer != null) {
-				this.additionalParametersConsumer = additionalParametersConsumer;
+				additionalParametersConsumer.accept(this.additionalParameters);
 			}
 			return this;
 		}
@@ -367,8 +367,8 @@ public Builder parameters(Consumer<Map<String, Object>> parametersConsumer) {
 		 * @return the {@link Builder}
 		 */
 		public Builder attributes(Map<String, Object> attributes) {
-			if (attributes != null) {
-				return attributes(attrs -> attrs.putAll(attributes));
+			if (!CollectionUtils.isEmpty(attributes)) {
+				this.attributes.putAll(attributes);
 			}
 			return this;
 		}
@@ -382,7 +382,7 @@ public Builder attributes(Map<String, Object> attributes) {
 		 */
 		public Builder attributes(Consumer<Map<String, Object>> attributesConsumer) {
 			if (attributesConsumer != null) {
-				this.attributesConsumer = attributesConsumer;
+				attributesConsumer.accept(this.attributes);
 			}
 			return this;
 		}
@@ -439,12 +439,8 @@ public OAuth2AuthorizationRequest build() {
 			authorizationRequest.scopes = Collections.unmodifiableSet(
 				CollectionUtils.isEmpty(this.scopes) ?
 					Collections.emptySet() : new LinkedHashSet<>(this.scopes));
-			Map<String, Object> additionalParameters = new LinkedHashMap<>();
-			this.additionalParametersConsumer.accept(additionalParameters);
-			authorizationRequest.additionalParameters = Collections.unmodifiableMap(additionalParameters);
-			Map<String, Object> attributes = new LinkedHashMap<>();
-			this.attributesConsumer.accept(attributes);
-			authorizationRequest.attributes = Collections.unmodifiableMap(attributes);
+			authorizationRequest.additionalParameters = Collections.unmodifiableMap(this.additionalParameters);
+			authorizationRequest.attributes = Collections.unmodifiableMap(this.attributes);
 			authorizationRequest.authorizationRequestUri =
 					StringUtils.hasText(this.authorizationRequestUri) ?
 							this.authorizationRequestUri : this.buildAuthorizationRequestUri();
@@ -457,7 +453,7 @@ private String buildAuthorizationRequestUri() {
 			this.parametersConsumer.accept(parameters);
 			MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>();
 			parameters.forEach((k, v) -> queryParams.set(
-					encodeQueryParam(k), encodeQueryParam(v.toString())));		// Encoded
+					encodeQueryParam(k), encodeQueryParam(String.valueOf(v))));		// Encoded
 			UriBuilder uriBuilder = this.uriBuilderFactory.uriString(this.authorizationUri)
 					.queryParams(queryParams);
 			return this.authorizationRequestUriFunction.apply(uriBuilder).toString();
@@ -477,9 +473,7 @@ private Map<String, Object> getParameters() {
 			if (this.redirectUri != null) {
 				parameters.put(OAuth2ParameterNames.REDIRECT_URI, this.redirectUri);
 			}
-			Map<String, Object> additionalParameters = new LinkedHashMap<>();
-			this.additionalParametersConsumer.accept(additionalParameters);
-			additionalParameters.forEach((k, v) -> parameters.put(k, v.toString()));
+			parameters.putAll(this.additionalParameters);
 			return parameters;
 		}
 

oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequestTests.java

@@ -121,15 +121,15 @@ public void buildWhenStateIsNullThenDoesNotThrowAnyException() {
 	}
 
 	@Test
-	public void buildWhenAdditionalParametersIsNullThenDoesNotThrowAnyException() {
+	public void buildWhenAdditionalParametersEmptyThenDoesNotThrowAnyException() {
 		assertThatCode(() ->
 				OAuth2AuthorizationRequest.authorizationCode()
 					.authorizationUri(AUTHORIZATION_URI)
 					.clientId(CLIENT_ID)
 					.redirectUri(REDIRECT_URI)
 					.scopes(SCOPES)
 					.state(STATE)
-					.additionalParameters((Map) null)
+					.additionalParameters(Map::clear)
 					.build())
 				.doesNotThrowAnyException();
 	}