Skip to content

JwtDecoders should support issuer hostnames containing underscores #15853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.springframework.security.oauth2.client.registration;

import java.net.URI;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -37,6 +36,7 @@
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;

/**
Expand Down Expand Up @@ -145,7 +145,7 @@ public static ClientRegistration.Builder fromOidcConfiguration(Map<String, Objec
*/
public static ClientRegistration.Builder fromOidcIssuerLocation(String issuer) {
Assert.hasText(issuer, "issuer cannot be empty");
return getBuilder(issuer, oidc(URI.create(issuer)));
return getBuilder(issuer, oidc(issuer));
}

/**
Expand Down Expand Up @@ -188,21 +188,17 @@ public static ClientRegistration.Builder fromOidcIssuerLocation(String issuer) {
*/
public static ClientRegistration.Builder fromIssuerLocation(String issuer) {
Assert.hasText(issuer, "issuer cannot be empty");
URI uri = URI.create(issuer);
return getBuilder(issuer, oidc(uri), oidcRfc8414(uri), oauth(uri));
return getBuilder(issuer, oidc(issuer), oidcRfc8414(issuer), oauth(issuer));
}

private static Supplier<ClientRegistration.Builder> oidc(URI issuer) {
// @formatter:off
URI uri = UriComponentsBuilder.fromUri(issuer)
.replacePath(issuer.getPath() + OIDC_METADATA_PATH)
.build(Collections.emptyMap());
static Supplier<ClientRegistration.Builder> oidc(String issuer) {
UriComponents uri = oidcUri(issuer);
// @formatter:on
return () -> {
RequestEntity<Void> request = RequestEntity.get(uri).build();
RequestEntity<Void> request = RequestEntity.get(uri.toUriString()).build();
Map<String, Object> configuration = rest.exchange(request, typeReference).getBody();
OIDCProviderMetadata metadata = parse(configuration, OIDCProviderMetadata::parse);
ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer.toASCIIString())
ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer)
.jwkSetUri(metadata.getJWKSetURI().toASCIIString());
if (metadata.getUserInfoEndpointURI() != null) {
builder.userInfoUri(metadata.getUserInfoEndpointURI().toASCIIString());
Expand All @@ -211,30 +207,48 @@ private static Supplier<ClientRegistration.Builder> oidc(URI issuer) {
};
}

private static Supplier<ClientRegistration.Builder> oidcRfc8414(URI issuer) {
static UriComponents oidcUri(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
URI uri = UriComponentsBuilder.fromUri(issuer)
.replacePath(OIDC_METADATA_PATH + issuer.getPath())
.build(Collections.emptyMap());
return UriComponentsBuilder.newInstance().uriComponents(uri)
.replacePath(uri.getPath() + OIDC_METADATA_PATH)
.build();
}

static Supplier<ClientRegistration.Builder> oidcRfc8414(String issuer) {
UriComponents uri = oidcRfc8414Uri(issuer);
// @formatter:on
return getRfc8414Builder(issuer, uri);
}

private static Supplier<ClientRegistration.Builder> oauth(URI issuer) {
static UriComponents oidcRfc8414Uri(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
URI uri = UriComponentsBuilder.fromUri(issuer)
.replacePath(OAUTH_METADATA_PATH + issuer.getPath())
.build(Collections.emptyMap());
// @formatter:on
return UriComponentsBuilder.newInstance().uriComponents(uri)
.replacePath(OIDC_METADATA_PATH + uri.getPath())
.build();
}

static Supplier<ClientRegistration.Builder> oauth(String issuer) {
UriComponents uri = oauthUri(issuer);
return getRfc8414Builder(issuer, uri);
}

private static Supplier<ClientRegistration.Builder> getRfc8414Builder(URI issuer, URI uri) {
static UriComponents oauthUri(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
return UriComponentsBuilder.newInstance().uriComponents(uri)
.replacePath(OAUTH_METADATA_PATH + uri.getPath())
.build();
// @formatter:on
}

private static Supplier<ClientRegistration.Builder> getRfc8414Builder(String issuer, UriComponents uri) {
return () -> {
RequestEntity<Void> request = RequestEntity.get(uri).build();
RequestEntity<Void> request = RequestEntity.get(uri.toUriString()).build();
Map<String, Object> configuration = rest.exchange(request, typeReference).getBody();
AuthorizationServerMetadata metadata = parse(configuration, AuthorizationServerMetadata::parse);
ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer.toASCIIString());
ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer);
URI jwkSetUri = metadata.getJWKSetURI();
if (jwkSetUri != null) {
builder.jwkSetUri(jwkSetUri.toASCIIString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.springframework.http.MediaType;
import org.springframework.security.oauth2.core.AuthorizationGrantType;
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
import org.springframework.web.util.UriComponents;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
Expand Down Expand Up @@ -569,6 +570,17 @@ public void issuerWhenOidcConfigurationTlsClientAuthMethodThenSuccess() throws E
.isEqualTo(ClientAuthenticationMethod.CLIENT_SECRET_BASIC);
}

// gh-15852
@Test
public void oidcWhenHostContainsUnderscoreThenRetains() {
UriComponents oidc = ClientRegistrations.oidcUri("https://elated_sutherland:8080/path");
assertThat(oidc.getHost()).isEqualTo("elated_sutherland");
UriComponents oauth = ClientRegistrations.oauthUri("https://elated_sutherland:8080/path");
assertThat(oauth.getHost()).isEqualTo("elated_sutherland");
UriComponents oidcRfc8414 = ClientRegistrations.oidcRfc8414Uri("https://elated_sutherland:8080/path");
assertThat(oidcRfc8414.getHost()).isEqualTo("elated_sutherland");
}

private ClientRegistration.Builder registration(String path) throws Exception {
this.issuer = createIssuerFromServer(path);
this.response.put("issuer", this.issuer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -45,6 +43,7 @@
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestOperations;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.util.UriComponents;
import org.springframework.web.util.UriComponentsBuilder;

/**
Expand Down Expand Up @@ -82,12 +81,11 @@ private JwtDecoderProviderConfigurationUtils() {
}

static Map<String, Object> getConfigurationForOidcIssuerLocation(String oidcIssuerLocation) {
return getConfiguration(oidcIssuerLocation, rest, oidc(URI.create(oidcIssuerLocation)));
return getConfiguration(oidcIssuerLocation, rest, oidc(oidcIssuerLocation));
}

static Map<String, Object> getConfigurationForIssuerLocation(String issuer, RestOperations rest) {
URI uri = URI.create(issuer);
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) {
Expand Down Expand Up @@ -159,11 +157,11 @@ private static String getMetadataIssuer(Map<String, Object> configuration) {
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");
Expand All @@ -183,27 +181,30 @@ private static Map<String, Object> getConfiguration(String issuer, RestOperation
throw new IllegalArgumentException(errorMessage);
}

private static URI oidc(URI issuer) {
static UriComponents oidc(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
return UriComponentsBuilder.fromUri(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(URI issuer) {
static UriComponents oidcRfc8414(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
return UriComponentsBuilder.fromUri(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(URI issuer) {
static UriComponents oauth(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
return UriComponentsBuilder.fromUri(issuer)
.replacePath(OAUTH_METADATA_PATH + issuer.getPath())
.build(Collections.emptyMap());
return UriComponentsBuilder.newInstance().uriComponents(uri)
.replacePath(OAUTH_METADATA_PATH + uri.getPath())
.build();
// @formatter:on
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,6 +39,7 @@
import org.springframework.util.Assert;
import org.springframework.web.reactive.function.client.WebClient;
import org.springframework.web.reactive.function.client.WebClientResponseException;
import org.springframework.web.util.UriComponents;
import org.springframework.web.util.UriComponentsBuilder;

final class ReactiveJwtDecoderProviderConfigurationUtils {
Expand Down Expand Up @@ -93,38 +92,40 @@ else if (jwk.getKeyType() == KeyType.EC) {
}

static Mono<Map<String, Object>> getConfigurationForIssuerLocation(String issuer, WebClient web) {
URI uri = URI.create(issuer);
return getConfiguration(issuer, web, oidc(uri), oidcRfc8414(uri), oauth(uri));
return getConfiguration(issuer, web, oidc(issuer), oidcRfc8414(issuer), oauth(issuer));
}

private static URI oidc(URI issuer) {
static UriComponents oidc(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
return UriComponentsBuilder.fromUri(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(URI issuer) {
static UriComponents oidcRfc8414(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
return UriComponentsBuilder.fromUri(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(URI issuer) {
static UriComponents oauth(String issuer) {
UriComponents uri = UriComponentsBuilder.fromUriString(issuer).build();
// @formatter:off
return UriComponentsBuilder.fromUri(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"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
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;
Expand Down Expand Up @@ -90,4 +91,16 @@ public void getSignatureAlgorithmsWhenAlgorithmThenParses() throws Exception {
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");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand Down Expand Up @@ -227,6 +228,18 @@ public void issuerWhenOidcFallbackRequestedIssuerIsUnresponsiveThenThrowsIllegal
// @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);
Expand Down