From 2665a92107b85170ca5ddabcc9aeb975bae8886a Mon Sep 17 00:00:00 2001
From: Rob Winch <362503+rwinch@users.noreply.github.com>
Date: Fri, 17 Jan 2025 10:56:42 -0600
Subject: [PATCH] Ensure that ClientSettings cannot be null

This ensures that ClientRegistration.Builder.ClientSettings cannot be null.
This has a slight advantage in terms of null safety to making this check
happen in the build method since the Builder does not have a null field
either.

Issue gh-16382
---
 .../registration/ClientRegistration.java      |  6 ++---
 .../registration/ClientRegistrationTests.java | 23 +++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java
index c8fa34e682..8c5c09c140 100644
--- a/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java
+++ b/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java
@@ -378,7 +378,7 @@ public final class ClientRegistration implements Serializable {
 
 		private String clientName;
 
-		private ClientSettings clientSettings;
+		private ClientSettings clientSettings = ClientSettings.builder().build();
 
 		private Builder(String registrationId) {
 			this.registrationId = registrationId;
@@ -614,6 +614,7 @@ public final class ClientRegistration implements Serializable {
 		 * @return the {@link Builder}
 		 */
 		public Builder clientSettings(ClientSettings clientSettings) {
+			Assert.notNull(clientSettings, "clientSettings cannot be null");
 			this.clientSettings = clientSettings;
 			return this;
 		}
@@ -651,8 +652,7 @@ public final class ClientRegistration implements Serializable {
 			clientRegistration.providerDetails = createProviderDetails(clientRegistration);
 			clientRegistration.clientName = StringUtils.hasText(this.clientName) ? this.clientName
 					: this.registrationId;
-			clientRegistration.clientSettings = (this.clientSettings == null) ? ClientSettings.builder().build()
-					: this.clientSettings;
+			clientRegistration.clientSettings = this.clientSettings;
 			return clientRegistration;
 		}
 
diff --git a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java
index 070e2040bd..2c4ee41a07 100644
--- a/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java
+++ b/oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationTests.java
@@ -753,4 +753,27 @@ public class ClientRegistrationTests {
 		assertThat(clientRegistration.getClientAuthenticationMethod()).isEqualTo(clientAuthenticationMethod);
 	}
 
+	@Test
+	void clientSettingsWhenNullThenThrowIllegalArgumentException() {
+		assertThatIllegalArgumentException()
+			.isThrownBy(() -> ClientRegistration.withRegistrationId(REGISTRATION_ID).clientSettings(null));
+	}
+
+	// gh-16382
+	@Test
+	void buildWhenDefaultClientSettingsThenDefaulted() {
+		ClientRegistration clientRegistration = ClientRegistration.withRegistrationId(REGISTRATION_ID)
+			.clientId(CLIENT_ID)
+			.authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE)
+			.redirectUri(REDIRECT_URI)
+			.authorizationUri(AUTHORIZATION_URI)
+			.tokenUri(TOKEN_URI)
+			.build();
+
+		// should not be null
+		assertThat(clientRegistration.getClientSettings()).isNotNull();
+		// proof key should be false for passivity
+		assertThat(clientRegistration.getClientSettings().isRequireProofKey()).isFalse();
+	}
+
 }