Browse Source

Polish gh-1326

Joe Grandja 1 year ago
parent
commit
05f1371668

+ 2 - 3
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientConfigurationAuthenticationProvider.java

@@ -53,7 +53,6 @@ import org.springframework.util.StringUtils;
  * @see OAuth2AuthorizationService
  * @see OidcClientRegistrationAuthenticationToken
  * @see OidcClientRegistrationAuthenticationProvider
- * @see RegisteredClientOidcClientRegistrationConverter
  * @see <a href="https://openid.net/specs/openid-connect-registration-1_0.html#ClientConfigurationEndpoint">4. Client Configuration Endpoint</a>
  */
 public final class OidcClientConfigurationAuthenticationProvider implements AuthenticationProvider {
@@ -79,9 +78,9 @@ public final class OidcClientConfigurationAuthenticationProvider implements Auth
 	}
 
 	/**
-	 * Sets the {@link Converter} used for converting an {@link RegisteredClient} to a {@link OidcClientRegistration}.
+	 * Sets the {@link Converter} used for converting a {@link RegisteredClient} to an {@link OidcClientRegistration}.
 	 *
-	 * @param clientRegistrationConverter the {@link Converter} used for converting an {@link RegisteredClient} to a {@link OidcClientRegistration}
+	 * @param clientRegistrationConverter the {@link Converter} used for converting a {@link RegisteredClient} to an {@link OidcClientRegistration}
 	 * @since 1.2.0
 	 */
 	public void setClientRegistrationConverter(Converter<RegisteredClient, OidcClientRegistration> clientRegistrationConverter) {

+ 2 - 2
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientRegistrationAuthenticationProvider.java

@@ -168,9 +168,9 @@ public final class OidcClientRegistrationAuthenticationProvider implements Authe
 	}
 
 	/**
-	 * Sets the {@link Converter} used for converting an {@link RegisteredClient} to a {@link OidcClientRegistration}.
+	 * Sets the {@link Converter} used for converting a {@link RegisteredClient} to an {@link OidcClientRegistration}.
 	 *
-	 * @param clientRegistrationConverter the {@link Converter} used for converting an {@link RegisteredClient} to a {@link OidcClientRegistration}
+	 * @param clientRegistrationConverter the {@link Converter} used for converting a {@link RegisteredClient} to an {@link OidcClientRegistration}
 	 * @since 1.2.0
 	 */
 	public void setClientRegistrationConverter(Converter<RegisteredClient, OidcClientRegistration> clientRegistrationConverter) {

+ 6 - 4
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/converter/OidcClientRegistrationRegisteredClientConverter.java

@@ -15,6 +15,10 @@
  */
 package org.springframework.security.oauth2.server.authorization.oidc.converter;
 
+import java.time.Instant;
+import java.util.Base64;
+import java.util.UUID;
+
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.security.crypto.keygen.Base64StringKeyGenerator;
 import org.springframework.security.crypto.keygen.StringKeyGenerator;
@@ -29,11 +33,9 @@ import org.springframework.security.oauth2.server.authorization.settings.ClientS
 import org.springframework.security.oauth2.server.authorization.settings.TokenSettings;
 import org.springframework.util.CollectionUtils;
 
-import java.time.Instant;
-import java.util.Base64;
-import java.util.UUID;
-
 /**
+ * A {@link Converter} that converts the provided {@link OidcClientRegistration} to a {@link RegisteredClient}.
+ *
  * @author Joe Grandja
  * @author Dmitriy Dubson
  * @since 1.2.0

+ 2 - 0
oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/oidc/converter/RegisteredClientOidcClientRegistrationConverter.java

@@ -28,6 +28,8 @@ import org.springframework.util.CollectionUtils;
 import org.springframework.web.util.UriComponentsBuilder;
 
 /**
+ * A {@link Converter} that converts the provided {@link RegisteredClient} to an {@link OidcClientRegistration}.
+ *
  * @author Joe Grandja
  * @since 1.2.0
  */

+ 46 - 43
oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OidcClientRegistrationTests.java

@@ -22,13 +22,12 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
-import java.util.function.Function;
-import java.util.stream.Collectors;
+
+import jakarta.servlet.http.HttpServletResponse;
 
 import com.nimbusds.jose.jwk.JWKSet;
 import com.nimbusds.jose.jwk.source.JWKSource;
 import com.nimbusds.jose.proc.SecurityContext;
-import jakarta.servlet.http.HttpServletResponse;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 import org.junit.jupiter.api.AfterAll;
@@ -588,7 +587,7 @@ public class OidcClientRegistrationTests {
 					oidc
 						.clientRegistrationEndpoint(clientRegistration ->
 							clientRegistration
-								.authenticationProviders(configureRegisteredClientConverters())
+								.authenticationProviders(configureClientRegistrationConverters())
 						)
 				);
 			RequestMatcher endpointsMatcher = authorizationServerConfigurer.getEndpointsMatcher();
@@ -607,15 +606,14 @@ public class OidcClientRegistrationTests {
 		}
 		// @formatter:on
 
-		private Consumer<List<AuthenticationProvider>> configureRegisteredClientConverters() {
+		private Consumer<List<AuthenticationProvider>> configureClientRegistrationConverters() {
             // @formatter:off
 			return (authenticationProviders) ->
 					authenticationProviders.forEach(authenticationProvider -> {
-						List<String> customClientMetadata = List.of("custom-metadata-name-1", "custom-metadata-name-2");
-
+						List<String> supportedCustomClientMetadata = List.of("custom-metadata-name-1", "custom-metadata-name-2");
 						if (authenticationProvider instanceof OidcClientRegistrationAuthenticationProvider provider) {
-							provider.setRegisteredClientConverter(new CustomRegisteredClientConverter(customClientMetadata));
-							provider.setClientRegistrationConverter(new CustomClientRegistrationConverter(customClientMetadata));
+							provider.setRegisteredClientConverter(new CustomRegisteredClientConverter(supportedCustomClientMetadata));
+							provider.setClientRegistrationConverter(new CustomClientRegistrationConverter(supportedCustomClientMetadata));
 						}
 					});
 			// @formatter:on
@@ -695,54 +693,59 @@ public class OidcClientRegistrationTests {
 
 	}
 
-	static class CustomClientRegistrationConverter implements Converter<RegisteredClient, OidcClientRegistration> {
-		private final List<String> customMetadata;
+	private static class CustomRegisteredClientConverter implements Converter<OidcClientRegistration, RegisteredClient> {
+		private final OidcClientRegistrationRegisteredClientConverter delegate =
+				new OidcClientRegistrationRegisteredClientConverter();
+		private final List<String> supportedCustomClientMetadata;
 
-		private final RegisteredClientOidcClientRegistrationConverter delegate;
-
-		CustomClientRegistrationConverter(List<String> customMetadata) {
-			this.customMetadata = customMetadata;
-			this.delegate = new RegisteredClientOidcClientRegistrationConverter();
+		private CustomRegisteredClientConverter(List<String> supportedCustomClientMetadata) {
+			this.supportedCustomClientMetadata = supportedCustomClientMetadata;
 		}
 
-		public OidcClientRegistration convert(RegisteredClient registeredClient) {
-			var clientRegistration = delegate.convert(registeredClient);
-			Map<String, Object> claims = new HashMap<>(clientRegistration.getClaims());
-			if (!CollectionUtils.isEmpty(customMetadata)) {
-				ClientSettings clientSettings = registeredClient.getClientSettings();
-
-				claims.putAll(customMetadata.stream()
-						.filter(metadatum -> clientSettings.getSetting(metadatum) != null)
-						.collect(Collectors.toMap(Function.identity(), clientSettings::getSetting)));
+		@Override
+		public RegisteredClient convert(OidcClientRegistration clientRegistration) {
+			RegisteredClient registeredClient = this.delegate.convert(clientRegistration);
+
+			ClientSettings.Builder clientSettingsBuilder = ClientSettings
+					.withSettings(registeredClient.getClientSettings().getSettings());
+			if (!CollectionUtils.isEmpty(this.supportedCustomClientMetadata)) {
+				clientRegistration.getClaims().forEach((claim, value) -> {
+					if (this.supportedCustomClientMetadata.contains(claim)) {
+						clientSettingsBuilder.setting(claim, value);
+					}
+				});
 			}
-			return OidcClientRegistration.withClaims(claims).build();
+
+			return RegisteredClient.from(registeredClient).clientSettings(clientSettingsBuilder.build()).build();
 		}
-	}
 
-	static class CustomRegisteredClientConverter implements Converter<OidcClientRegistration, RegisteredClient> {
-		private final List<String> customMetadata;
+	}
 
-		private final OidcClientRegistrationRegisteredClientConverter delegate;
+	private static class CustomClientRegistrationConverter implements Converter<RegisteredClient, OidcClientRegistration> {
+		private final RegisteredClientOidcClientRegistrationConverter delegate =
+				new RegisteredClientOidcClientRegistrationConverter();
+		private final List<String> supportedCustomClientMetadata;
 
-		CustomRegisteredClientConverter(List<String> customMetadata) {
-			this.customMetadata = customMetadata;
-			this.delegate = new OidcClientRegistrationRegisteredClientConverter();
+		private CustomClientRegistrationConverter(List<String> supportedCustomClientMetadata) {
+			this.supportedCustomClientMetadata = supportedCustomClientMetadata;
 		}
 
-		public RegisteredClient convert(OidcClientRegistration clientRegistration) {
-			RegisteredClient convertedClient = delegate.convert(clientRegistration);
-			ClientSettings.Builder clientSettingsBuilder = ClientSettings
-					.withSettings(convertedClient.getClientSettings().getSettings());
-
-			if (!CollectionUtils.isEmpty(this.customMetadata)) {
-				clientRegistration.getClaims().forEach((claim, value) -> {
-					if (this.customMetadata.contains(claim)) {
-						clientSettingsBuilder.setting(claim, value);
+		@Override
+		public OidcClientRegistration convert(RegisteredClient registeredClient) {
+			OidcClientRegistration clientRegistration = this.delegate.convert(registeredClient);
+
+			Map<String, Object> clientMetadata = new HashMap<>(clientRegistration.getClaims());
+			if (!CollectionUtils.isEmpty(this.supportedCustomClientMetadata)) {
+				Map<String, Object> clientSettings = registeredClient.getClientSettings().getSettings();
+				this.supportedCustomClientMetadata.forEach((customClaim) -> {
+					if (clientSettings.containsKey(customClaim)) {
+						clientMetadata.put(customClaim, clientSettings.get(customClaim));
 					}
 				});
 			}
 
-			return RegisteredClient.from(convertedClient).clientSettings(clientSettingsBuilder.build()).build();
+			return OidcClientRegistration.withClaims(clientMetadata).build();
 		}
 	}
+
 }

+ 8 - 8
oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/oidc/authentication/OidcClientConfigurationAuthenticationProviderTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2020-2022 the original author or authors.
+ * Copyright 2020-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -111,6 +111,13 @@ public class OidcClientConfigurationAuthenticationProviderTests {
 		assertThat(this.authenticationProvider.supports(OidcClientRegistrationAuthenticationToken.class)).isTrue();
 	}
 
+	@Test
+	public void setClientRegistrationConverterWhenNullThenThrowIllegalArgumentException() {
+		assertThatIllegalArgumentException()
+				.isThrownBy(() -> this.authenticationProvider.setClientRegistrationConverter(null))
+				.withMessage("clientRegistrationConverter cannot be null");
+	}
+
 	@Test
 	public void authenticateWhenPrincipalNotOAuth2TokenAuthenticationTokenThenThrowOAuth2AuthenticationException() {
 		TestingAuthenticationToken principal = new TestingAuthenticationToken("principal", "credentials");
@@ -378,13 +385,6 @@ public class OidcClientConfigurationAuthenticationProviderTests {
 		assertThat(clientRegistrationResult.getRegistrationAccessToken()).isNull();
 	}
 
-	@Test
-	public void setClientRegistrationConverterWhenNullThenThrowIllegalArgumentException() {
-		assertThatIllegalArgumentException()
-				.isThrownBy(() -> this.authenticationProvider.setClientRegistrationConverter(null))
-				.withMessage("clientRegistrationConverter cannot be null");
-	}
-
 	private static Jwt createJwtClientConfiguration() {
 		return createJwt(Collections.singleton("client.read"));
 	}