Make jwks_uri optional for RFC 8414 and required for OpenID Connect

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

@@ -21,6 +21,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Supplier;
 
 import com.nimbusds.oauth2.sdk.GrantType;
 import com.nimbusds.oauth2.sdk.ParseException;
@@ -86,10 +87,7 @@ public final class ClientRegistrations {
 	 */
 	public static ClientRegistration.Builder fromOidcIssuerLocation(String issuer) {
 		Assert.hasText(issuer, "issuer cannot be empty");
-		Map<String, Object> configuration = getConfiguration(issuer, oidc(URI.create(issuer)));
-		OIDCProviderMetadata metadata = parse(configuration, OIDCProviderMetadata::parse);
-		return withProviderConfiguration(metadata, issuer)
-				.userInfoUri(metadata.getUserInfoEndpointURI().toASCIIString());
+		return getBuilder(issuer, oidc(URI.create(issuer)));
 	}
 
 	/**
@@ -137,42 +135,68 @@ 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);
-		Map<String, Object> configuration = getConfiguration(issuer, oidc(uri), oidcRfc8414(uri), oauth(uri));
-		AuthorizationServerMetadata metadata = parse(configuration, AuthorizationServerMetadata::parse);
-		ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer);
-		String userinfoEndpoint = (String) configuration.get("userinfo_endpoint");
-		if (userinfoEndpoint != null) {
-			builder.userInfoUri(userinfoEndpoint);
-		}
-		return builder;
+		return getBuilder(issuer, oidc(uri), oidcRfc8414(uri), oauth(uri));
 	}
 
-	private static URI oidc(URI issuer) {
-		return UriComponentsBuilder.fromUri(issuer)
+	private static Supplier<ClientRegistration.Builder> oidc(URI issuer) {
+		URI uri = UriComponentsBuilder.fromUri(issuer)
 				.replacePath(issuer.getPath() + OIDC_METADATA_PATH).build(Collections.emptyMap());
+
+		return () -> {
+			RequestEntity<Void> request = RequestEntity.get(uri).build();
+			Map<String, Object> configuration = rest.exchange(request, typeReference).getBody();
+			OIDCProviderMetadata metadata = parse(configuration, OIDCProviderMetadata::parse);
+			return withProviderConfiguration(metadata, issuer.toASCIIString())
+					.jwkSetUri(metadata.getJWKSetURI().toASCIIString())
+					.userInfoUri(metadata.getUserInfoEndpointURI().toASCIIString());
+		};
 	}
 
-	private static URI oidcRfc8414(URI issuer) {
-		return UriComponentsBuilder.fromUri(issuer)
+	private static Supplier<ClientRegistration.Builder> oidcRfc8414(URI issuer) {
+		URI uri = UriComponentsBuilder.fromUri(issuer)
 				.replacePath(OIDC_METADATA_PATH + issuer.getPath()).build(Collections.emptyMap());
+		return getRfc8414Builder(issuer, uri);
 	}
 
-	private static URI oauth(URI issuer) {
-		return UriComponentsBuilder.fromUri(issuer)
+	private static Supplier<ClientRegistration.Builder> oauth(URI issuer) {
+		URI uri = UriComponentsBuilder.fromUri(issuer)
 				.replacePath(OAUTH_METADATA_PATH + issuer.getPath()).build(Collections.emptyMap());
+		return getRfc8414Builder(issuer, uri);
+	}
+
+	private static Supplier<ClientRegistration.Builder> getRfc8414Builder(URI issuer, URI uri) {
+		return () -> {
+			RequestEntity<Void> request = RequestEntity.get(uri).build();
+			Map<String, Object> configuration = rest.exchange(request, typeReference).getBody();
+			AuthorizationServerMetadata metadata = parse(configuration, AuthorizationServerMetadata::parse);
+			ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer.toASCIIString());
+
+			URI jwkSetUri = metadata.getJWKSetURI();
+			if (jwkSetUri != null) {
+				builder.jwkSetUri(jwkSetUri.toASCIIString());
+			}
+
+			String userinfoEndpoint = (String) configuration.get("userinfo_endpoint");
+			if (userinfoEndpoint != null) {
+				builder.userInfoUri(userinfoEndpoint);
+			}
+			return builder;
+		};
 	}
 
-	private static Map<String, Object> getConfiguration(String issuer, URI... uris) {
+	@SafeVarargs
+	private static ClientRegistration.Builder getBuilder(String issuer, Supplier<ClientRegistration.Builder>... suppliers) {
 		String errorMessage = "Unable to resolve Configuration with the provided Issuer of \"" + issuer + "\"";
-		for (URI uri : uris) {
+		for (Supplier<ClientRegistration.Builder> supplier : suppliers) {
 			try {
-				RequestEntity<Void> request = RequestEntity.get(uri).build();
-				return rest.exchange(request, typeReference).getBody();
+				return supplier.get();
 			} catch (HttpClientErrorException e) {
 				if (!e.getStatusCode().is4xxClientError()) {
 					throw e;
 				}
 				// else try another endpoint
+			} catch (IllegalArgumentException | IllegalStateException e) {
+				throw e;
 			} catch (RuntimeException e) {
 				throw new IllegalArgumentException(errorMessage, e);
 			}
@@ -219,7 +243,6 @@ private static ClientRegistration.Builder withProviderConfiguration(Authorizatio
 				.clientAuthenticationMethod(method)
 				.redirectUriTemplate("{baseUrl}/{action}/oauth2/code/{registrationId}")
 				.authorizationUri(metadata.getAuthorizationEndpointURI().toASCIIString())
-				.jwkSetUri(metadata.getJWKSetURI().toASCIIString())
 				.providerConfigurationMetadata(configurationMetadata)
 				.tokenUri(metadata.getTokenEndpointURI().toASCIIString())
 				.clientName(issuer);

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/registration/ClientRegistrationsTest.java

@@ -168,6 +168,33 @@ private void assertIssuerMetadata(ClientRegistration registration,
 				"grant_types_supported", "token_endpoint", "token_endpoint_auth_methods_supported", "userinfo_endpoint");
 	}
 
+	// gh-7512
+	@Test
+	public void issuerWhenResponseMissingJwksUriThenThrowsIllegalArgumentException() throws Exception {
+		this.response.remove("jwks_uri");
+		assertThatThrownBy(() -> registration("").build())
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessageContaining("The public JWK set URI must not be null");
+	}
+
+	// gh-7512
+	@Test
+	public void issuerWhenOidcFallbackResponseMissingJwksUriThenThrowsIllegalArgumentException() throws Exception {
+		this.response.remove("jwks_uri");
+		assertThatThrownBy(() -> registrationOidcFallback("issuer1", null).build())
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessageContaining("The public JWK set URI must not be null");
+	}
+
+	// gh-7512
+	@Test
+	public void issuerWhenOAuth2ResponseMissingJwksUriThenThenSuccess() throws Exception {
+		this.response.remove("jwks_uri");
+		ClientRegistration registration = registrationOAuth2("", null).build();
+		ClientRegistration.ProviderDetails provider = registration.getProviderDetails();
+		assertThat(provider.getJwkSetUri()).isNull();
+	}
+
 	@Test
 	public void issuerWhenContainsTrailingSlashThenSuccess() throws Exception {
 		assertThat(registration("")).isNotNull();

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

@@ -33,6 +33,7 @@
  * issuer and method invoked.
  *
  * @author Thomas Vitale
+ * @author Rafiullah Hamedy
  * @since 5.2
  */
 class JwtDecoderProviderConfigurationUtils {
@@ -69,7 +70,15 @@ private static Map<String, Object> getConfiguration(String issuer, URI... uris)
 			try {
 				RequestEntity<Void> request = RequestEntity.get(uri).build();
 				ResponseEntity<Map<String, Object>> response = rest.exchange(request, typeReference);
-				return response.getBody();
+				Map<String, Object> configuration = response.getBody();
+
+				if (configuration.get("jwks_uri") == null) {
+					throw new IllegalArgumentException("The public JWK set URI must not be null");
+				}
+
+				return configuration;
+			} catch (IllegalArgumentException e) {
+				throw e;
 			} catch (RuntimeException e) {
 				if (!(e instanceof HttpClientErrorException &&
 						((HttpClientErrorException) e).getStatusCode().is4xxClientError())) {

oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/JwtDecodersTests.java

@@ -33,6 +33,11 @@
 import org.springframework.http.MediaType;
 import org.springframework.web.util.UriComponentsBuilder;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.JsonMappingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 
@@ -163,6 +168,36 @@ public void issuerWhenOAuth2ResponseIsNonCompliantThenThrowsRuntimeException() {
 				.isInstanceOf(RuntimeException.class);
 	}
 
+	// gh-7512
+	@Test
+	public void issuerWhenResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
+			throws JsonMappingException, JsonProcessingException {
+		prepareConfigurationResponse(this.buildResponseWithMissingJwksUri());
+		assertThatCode(() -> JwtDecoders.fromOidcIssuerLocation(this.issuer))
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessage("The public JWK set URI must not be null");
+	}
+
+	// gh-7512
+	@Test
+	public void issuerWhenOidcFallbackResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
+			throws JsonMappingException, JsonProcessingException {
+		prepareConfigurationResponseOidc(this.buildResponseWithMissingJwksUri());
+		assertThatCode(() -> JwtDecoders.fromIssuerLocation(this.issuer))
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessage("The public JWK set URI must not be null");
+	}
+
+	// gh-7512
+	@Test
+	public void issuerWhenOAuth2ResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
+			throws JsonMappingException, JsonProcessingException {
+		prepareConfigurationResponseOAuth2(this.buildResponseWithMissingJwksUri());
+		assertThatCode(() -> JwtDecoders.fromIssuerLocation(this.issuer))
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessage("The public JWK set URI must not be null");
+	}
+
 	@Test
 	public void issuerWhenResponseIsMalformedThenThrowsRuntimeException() {
 		prepareConfigurationResponse("malformed");
@@ -294,4 +329,12 @@ private MockResponse response(String body) {
 				.setBody(body)
 				.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE);
 	}
+
+	public String buildResponseWithMissingJwksUri() throws JsonMappingException, JsonProcessingException {
+		ObjectMapper mapper = new ObjectMapper();
+		Map<String, Object> response = mapper.readValue(DEFAULT_RESPONSE_TEMPLATE,
+				new TypeReference<Map<String, Object>>(){});
+		response.remove("jwks_uri");
+		return mapper.writeValueAsString(response);
+	}
 }

oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/ReactiveJwtDecodersTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -33,12 +33,18 @@
 import org.springframework.http.MediaType;
 import org.springframework.web.util.UriComponentsBuilder;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.JsonMappingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 import static org.assertj.core.api.Assertions.assertThatCode;
 
 /**
  * Tests for {@link ReactiveJwtDecoders}
  *
  * @author Josh Cummings
+ * @author Rafiullah Hamedy
  */
 public class ReactiveJwtDecodersTests {
 	/**
@@ -76,14 +82,12 @@ public class ReactiveJwtDecodersTests {
 
 	private MockWebServer server;
 	private String issuer;
-	private String jwkSetUri;
 
 	@Before
 	public void setup() throws Exception {
 		this.server = new MockWebServer();
 		this.server.start();
 		this.issuer = createIssuerFromServer();
-		this.jwkSetUri = this.issuer + ".well-known/jwks.json";
 		this.issuer += "path";
 	}
 
@@ -147,6 +151,36 @@ public void issuerWhenOAuth2ResponseIsNonCompliantThenThrowsRuntimeException() {
 				.isInstanceOf(RuntimeException.class);
 	}
 
+	// gh-7512
+	@Test
+	public void issuerWhenResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
+			throws JsonMappingException, JsonProcessingException {
+		prepareConfigurationResponse(this.buildResponseWithMissingJwksUri());
+		assertThatCode(() -> ReactiveJwtDecoders.fromOidcIssuerLocation(this.issuer))
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessage("The public JWK set URI must not be null");
+	}
+
+	// gh-7512
+	@Test
+	public void issuerWhenOidcFallbackResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
+			throws JsonMappingException, JsonProcessingException {
+		prepareConfigurationResponseOidc(this.buildResponseWithMissingJwksUri());
+		assertThatCode(() -> ReactiveJwtDecoders.fromIssuerLocation(this.issuer))
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessage("The public JWK set URI must not be null");
+	}
+
+	// gh-7512
+	@Test
+	public void issuerWhenOAuth2ResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
+			throws JsonMappingException, JsonProcessingException {
+		prepareConfigurationResponseOAuth2(this.buildResponseWithMissingJwksUri());
+		assertThatCode(() -> ReactiveJwtDecoders.fromIssuerLocation(this.issuer))
+				.isInstanceOf(IllegalArgumentException.class)
+				.hasMessage("The public JWK set URI must not be null");
+	}
+
 	@Test
 	public void issuerWhenResponseIsMalformedThenThrowsRuntimeException() {
 		prepareConfigurationResponse("malformed");
@@ -281,4 +315,12 @@ private MockResponse response(String body) {
 				.setBody(body)
 				.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE);
 	}
+
+	public String buildResponseWithMissingJwksUri() throws JsonMappingException, JsonProcessingException {
+		ObjectMapper mapper = new ObjectMapper();
+		Map<String, Object> response = mapper.readValue(DEFAULT_RESPONSE_TEMPLATE,
+				new TypeReference<Map<String, Object>>(){});
+		response.remove("jwks_uri");
+		return mapper.writeValueAsString(response);
+	}
 }