Cwikius-Spring
/
spring-security
mirror of https://github.com/spring-projects/spring-security.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make jwks_uri optional for RFC 8414 and Required for OpenID Connect 

OpenID Connect Discovery 1.0 expects the OpenId Provider Metadata 
response is expected to return a valid jwks_uri, however, this field is 
optional in the Authorization Server Metadata response as per RFC 8414
specification.

Fixes gh-7512
This commit is contained in:
Rafiullah Hamedy 2019-10-30 00:09:12 -04:00 committed by Josh Cummings
parent e1fad001d9
commit 58ca81d500
5 changed files with 171 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -21,6 +21,7 @@ import java.util.Collections;
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 final class ClientRegistrations {
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));
return getBuilder(issuer, oidc(uri), oidcRfc8414(uri), oauth(uri));
}
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 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 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);
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 URI oidc(URI issuer) {
return UriComponentsBuilder.fromUri(issuer)
.replacePath(issuer.getPath() + OIDC_METADATA_PATH).build(Collections.emptyMap());
}
private static URI oidcRfc8414(URI issuer) {
return UriComponentsBuilder.fromUri(issuer)
.replacePath(OIDC_METADATA_PATH + issuer.getPath()).build(Collections.emptyMap());
}
private static URI oauth(URI issuer) {
return UriComponentsBuilder.fromUri(issuer)
.replacePath(OAUTH_METADATA_PATH + issuer.getPath()).build(Collections.emptyMap());
}
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 @@ public final class ClientRegistrations {
.clientAuthenticationMethod(method)
.redirectUriTemplate("{baseUrl}/{action}/oauth2/code/{registrationId}")
.authorizationUri(metadata.getAuthorizationEndpointURI().toASCIIString())
.jwkSetUri(metadata.getJWKSetURI().toASCIIString())
.providerConfigurationMetadata(configurationMetadata)
.tokenUri(metadata.getTokenEndpointURI().toASCIIString())
.clientName(issuer);

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

@ -168,6 +168,33 @@ public class ClientRegistrationsTest {
"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();

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

@ -33,6 +33,7 @@ import java.util.Map;
* issuer and method invoked.
*
* @author Thomas Vitale
* @author Rafiullah Hamedy
* @since 5.2
*/
class JwtDecoderProviderConfigurationUtils {
@ -69,7 +70,15 @@ class JwtDecoderProviderConfigurationUtils {
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())) {

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

@ -33,6 +33,11 @@ import org.springframework.http.HttpHeaders;
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 class JwtDecodersTests {
.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 @@ public class JwtDecodersTests {
.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);
}
}

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

@ -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.HttpHeaders;
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 class ReactiveJwtDecodersTests {
.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 @@ public class ReactiveJwtDecodersTests {
.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);
}
}