Browse Source

JwtDecoders Supports Hostnames with Underscores

In the process of verifying gh-15852, another issue with URI was discovered.
This commit adds tests to the uri-computing methods and changes them to use
UriComponents instead of URI.

Issue gh-15852
Josh Cummings 6 months ago
parent
commit
3d15be1b06

+ 10 - 4
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistrations.java

@@ -37,6 +37,7 @@ import org.springframework.security.oauth2.core.oidc.IdTokenClaimNames;
 import org.springframework.util.Assert;
 import org.springframework.web.client.HttpClientErrorException;
 import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.UriComponents;
 import org.springframework.web.util.UriComponentsBuilder;
 
 /**
@@ -211,13 +212,18 @@ public final class ClientRegistrations {
 		};
 	}
 
-	private static Supplier<ClientRegistration.Builder> oidcRfc8414(URI issuer) {
+	private static Supplier<ClientRegistration.Builder> oidcRfc8414(String issuer) {
+		URI uri = oidcRfc8414Uri(issuer);
+		return getRfc8414Builder(issuer, uri);
+	}
+
+	static URI oidcRfc8414Uri(String issuer) {
+		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
 		// @formatter:off
-		URI uri = UriComponentsBuilder.fromUri(issuer)
-				.replacePath(OIDC_METADATA_PATH + issuer.getPath())
+		return UriComponentsBuilder.newInstance().uriComponents(uri)
+				.replacePath(OIDC_METADATA_PATH + uri.getPath())
 				.build(Collections.emptyMap());
 		// @formatter:on
-		return getRfc8414Builder(issuer, uri);
 	}
 
 	private static Supplier<ClientRegistration.Builder> oauth(URI issuer) {

+ 20 - 21
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/JwtDecoderProviderConfigurationUtils.java

@@ -16,8 +16,6 @@
 
 package org.springframework.security.oauth2.jwt;
 
-import java.net.URI;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -83,13 +81,11 @@ final class JwtDecoderProviderConfigurationUtils {
 	}
 
 	static Map<String, Object> getConfigurationForOidcIssuerLocation(String oidcIssuerLocation) {
-		UriComponents uri = UriComponentsBuilder.fromUriString(oidcIssuerLocation).build();
-		return getConfiguration(oidcIssuerLocation, rest, oidc(uri));
+		return getConfiguration(oidcIssuerLocation, rest, oidc(oidcIssuerLocation));
 	}
 
 	static Map<String, Object> getConfigurationForIssuerLocation(String issuer, RestOperations rest) {
-		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
-		return getConfiguration(issuer, rest, oidc(uri), oidcRfc8414(uri), oauth(uri));
+		return getConfiguration(issuer, rest, oidc(issuer), oidcRfc8414(issuer), oauth(issuer));
 	}
 
 	static Map<String, Object> getConfigurationForIssuerLocation(String issuer) {
@@ -161,11 +157,11 @@ final class JwtDecoderProviderConfigurationUtils {
 		return "(unavailable)";
 	}
 
-	private static Map<String, Object> getConfiguration(String issuer, RestOperations rest, URI... uris) {
+	private static Map<String, Object> getConfiguration(String issuer, RestOperations rest, UriComponents... uris) {
 		String errorMessage = "Unable to resolve the Configuration with the provided Issuer of " + "\"" + issuer + "\"";
-		for (URI uri : uris) {
+		for (UriComponents uri : uris) {
 			try {
-				RequestEntity<Void> request = RequestEntity.get(uri).build();
+				RequestEntity<Void> request = RequestEntity.get(uri.toUriString()).build();
 				ResponseEntity<Map<String, Object>> response = rest.exchange(request, STRING_OBJECT_MAP);
 				Map<String, Object> configuration = response.getBody();
 				Assert.isTrue(configuration.get("jwks_uri") != null, "The public JWK set URI must not be null");
@@ -185,27 +181,30 @@ final class JwtDecoderProviderConfigurationUtils {
 		throw new IllegalArgumentException(errorMessage);
 	}
 
-	private static URI oidc(UriComponents issuer) {
+	static UriComponents oidc(String issuer) {
+		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
 		// @formatter:off
-		return UriComponentsBuilder.newInstance().uriComponents(issuer)
-				.replacePath(issuer.getPath() + OIDC_METADATA_PATH)
-				.build(Collections.emptyMap());
+		return UriComponentsBuilder.newInstance().uriComponents(uri)
+				.replacePath(uri.getPath() + OIDC_METADATA_PATH)
+				.build();
 		// @formatter:on
 	}
 
-	private static URI oidcRfc8414(UriComponents issuer) {
+	static UriComponents oidcRfc8414(String issuer) {
+		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
 		// @formatter:off
-		return UriComponentsBuilder.newInstance().uriComponents(issuer)
-				.replacePath(OIDC_METADATA_PATH + issuer.getPath())
-				.build(Collections.emptyMap());
+		return UriComponentsBuilder.newInstance().uriComponents(uri)
+				.replacePath(OIDC_METADATA_PATH + uri.getPath())
+				.build();
 		// @formatter:on
 	}
 
-	private static URI oauth(UriComponents issuer) {
+	static UriComponents oauth(String issuer) {
+		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
 		// @formatter:off
-		return UriComponentsBuilder.newInstance().uriComponents(issuer)
-				.replacePath(OAUTH_METADATA_PATH + issuer.getPath())
-				.build(Collections.emptyMap());
+		return UriComponentsBuilder.newInstance().uriComponents(uri)
+				.replacePath(OAUTH_METADATA_PATH + uri.getPath())
+				.build();
 		// @formatter:on
 	}
 

+ 18 - 18
oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/ReactiveJwtDecoderProviderConfigurationUtils.java

@@ -16,8 +16,6 @@
 
 package org.springframework.security.oauth2.jwt;
 
-import java.net.URI;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -94,38 +92,40 @@ final class ReactiveJwtDecoderProviderConfigurationUtils {
 	}
 
 	static Mono<Map<String, Object>> getConfigurationForIssuerLocation(String issuer, WebClient web) {
-		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
-		return getConfiguration(issuer, web, oidc(uri), oidcRfc8414(uri), oauth(uri));
+		return getConfiguration(issuer, web, oidc(issuer), oidcRfc8414(issuer), oauth(issuer));
 	}
 
-	private static URI oidc(UriComponents issuer) {
+	static UriComponents oidc(String issuer) {
+		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
 		// @formatter:off
-		return UriComponentsBuilder.newInstance().uriComponents(issuer)
-				.replacePath(issuer.getPath() + OIDC_METADATA_PATH)
-				.build(Collections.emptyMap());
+		return UriComponentsBuilder.newInstance().uriComponents(uri)
+				.replacePath(uri.getPath() + OIDC_METADATA_PATH)
+				.build();
 		// @formatter:on
 	}
 
-	private static URI oidcRfc8414(UriComponents issuer) {
+	static UriComponents oidcRfc8414(String issuer) {
+		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
 		// @formatter:off
-		return UriComponentsBuilder.newInstance().uriComponents(issuer)
-				.replacePath(OIDC_METADATA_PATH + issuer.getPath())
-				.build(Collections.emptyMap());
+		return UriComponentsBuilder.newInstance().uriComponents(uri)
+				.replacePath(OIDC_METADATA_PATH + uri.getPath())
+				.build();
 		// @formatter:on
 	}
 
-	private static URI oauth(UriComponents issuer) {
+	static UriComponents oauth(String issuer) {
+		UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
 		// @formatter:off
-		return UriComponentsBuilder.newInstance().uriComponents(issuer)
-				.replacePath(OAUTH_METADATA_PATH + issuer.getPath())
-				.build(Collections.emptyMap());
+		return UriComponentsBuilder.newInstance().uriComponents(uri)
+				.replacePath(OAUTH_METADATA_PATH + uri.getPath())
+				.build();
 		// @formatter:on
 	}
 
-	private static Mono<Map<String, Object>> getConfiguration(String issuer, WebClient web, URI... uris) {
+	private static Mono<Map<String, Object>> getConfiguration(String issuer, WebClient web, UriComponents... uris) {
 		String errorMessage = "Unable to resolve the Configuration with the provided Issuer of " + "\"" + issuer + "\"";
 		return Flux.just(uris)
-			.concatMap((uri) -> web.get().uri(uri).retrieve().bodyToMono(STRING_OBJECT_MAP))
+			.concatMap((uri) -> web.get().uri(uri.toUriString()).retrieve().bodyToMono(STRING_OBJECT_MAP))
 			.flatMap((configuration) -> {
 				if (configuration.get("jwks_uri") == null) {
 					return Mono.error(() -> new IllegalArgumentException("The public JWK set URI must not be null"));

+ 13 - 0
oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/JwtDecoderProviderConfigurationUtilsTests.java

@@ -35,6 +35,7 @@ import org.junit.jupiter.api.Test;
 import org.springframework.security.oauth2.jose.TestKeys;
 import org.springframework.security.oauth2.jose.jws.JwsAlgorithms;
 import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm;
+import org.springframework.web.util.UriComponents;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
@@ -90,4 +91,16 @@ public class JwtDecoderProviderConfigurationUtilsTests {
 		assertThat(algorithms).containsOnly(SignatureAlgorithm.RS256);
 	}
 
+	// gh-15852
+	@Test
+	public void oidcWhenHostContainsUnderscoreThenRetains() {
+		UriComponents oidc = JwtDecoderProviderConfigurationUtils.oidc("https://elated_sutherland:8080/path");
+		assertThat(oidc.getHost()).isEqualTo("elated_sutherland");
+		UriComponents oauth = JwtDecoderProviderConfigurationUtils.oauth("https://elated_sutherland:8080/path");
+		assertThat(oauth.getHost()).isEqualTo("elated_sutherland");
+		UriComponents oidcRfc8414 = JwtDecoderProviderConfigurationUtils
+			.oidcRfc8414("https://elated_sutherland:8080/path");
+		assertThat(oidcRfc8414.getHost()).isEqualTo("elated_sutherland");
+	}
+
 }

+ 13 - 0
oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/ReactiveJwtDecoderProviderConfigurationUtilsTests.java

@@ -37,6 +37,7 @@ import org.junit.jupiter.api.Test;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.MediaType;
 import org.springframework.web.reactive.function.client.WebClient;
+import org.springframework.web.util.UriComponents;
 import org.springframework.web.util.UriComponentsBuilder;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -227,6 +228,18 @@ public class ReactiveJwtDecoderProviderConfigurationUtilsTests {
 		// @formatter:on
 	}
 
+	// gh-15852
+	@Test
+	public void oidcWhenHostContainsUnderscoreThenRetains() {
+		UriComponents oidc = ReactiveJwtDecoderProviderConfigurationUtils.oidc("https://elated_sutherland:8080/path");
+		assertThat(oidc.getHost()).isEqualTo("elated_sutherland");
+		UriComponents oauth = ReactiveJwtDecoderProviderConfigurationUtils.oauth("https://elated_sutherland:8080/path");
+		assertThat(oauth.getHost()).isEqualTo("elated_sutherland");
+		UriComponents oidcRfc8414 = ReactiveJwtDecoderProviderConfigurationUtils
+			.oidcRfc8414("https://elated_sutherland:8080/path");
+		assertThat(oidcRfc8414.getHost()).isEqualTo("elated_sutherland");
+	}
+
 	private void prepareConfigurationResponse() {
 		String body = String.format(DEFAULT_RESPONSE_TEMPLATE, this.issuer, this.issuer);
 		prepareConfigurationResponse(body);