Skip to content

Commit 9ba6309

Browse files
rhamedyrhamedy
committed
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
1 parent a4430aa commit 9ba6309

File tree

5 files changed

+171
-27
lines changed

5 files changed

+171
-27
lines changed

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

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.LinkedHashMap;
2222
import java.util.List;
2323
import java.util.Map;
24+
import java.util.function.Supplier;
2425

2526
import com.nimbusds.oauth2.sdk.GrantType;
2627
import com.nimbusds.oauth2.sdk.ParseException;
@@ -86,10 +87,7 @@ public final class ClientRegistrations {
8687
*/
8788
public static ClientRegistration.Builder fromOidcIssuerLocation(String issuer) {
8889
Assert.hasText(issuer, "issuer cannot be empty");
89-
Map<String, Object> configuration = getConfiguration(issuer, oidc(URI.create(issuer)));
90-
OIDCProviderMetadata metadata = parse(configuration, OIDCProviderMetadata::parse);
91-
return withProviderConfiguration(metadata, issuer)
92-
.userInfoUri(metadata.getUserInfoEndpointURI().toASCIIString());
90+
return getBuilder(issuer, oidc(URI.create(issuer)));
9391
}
9492

9593
/**
@@ -137,42 +135,68 @@ public static ClientRegistration.Builder fromOidcIssuerLocation(String issuer) {
137135
public static ClientRegistration.Builder fromIssuerLocation(String issuer) {
138136
Assert.hasText(issuer, "issuer cannot be empty");
139137
URI uri = URI.create(issuer);
140-
Map<String, Object> configuration = getConfiguration(issuer, oidc(uri), oidcRfc8414(uri), oauth(uri));
141-
AuthorizationServerMetadata metadata = parse(configuration, AuthorizationServerMetadata::parse);
142-
ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer);
143-
String userinfoEndpoint = (String) configuration.get("userinfo_endpoint");
144-
if (userinfoEndpoint != null) {
145-
builder.userInfoUri(userinfoEndpoint);
146-
}
147-
return builder;
138+
return getBuilder(issuer, oidc(uri), oidcRfc8414(uri), oauth(uri));
148139
}
149140

150-
private static URI oidc(URI issuer) {
151-
return UriComponentsBuilder.fromUri(issuer)
141+
private static Supplier<ClientRegistration.Builder> oidc(URI issuer) {
142+
URI uri = UriComponentsBuilder.fromUri(issuer)
152143
.replacePath(issuer.getPath() + OIDC_METADATA_PATH).build(Collections.emptyMap());
144+
145+
return () -> {
146+
RequestEntity<Void> request = RequestEntity.get(uri).build();
147+
Map<String, Object> configuration = rest.exchange(request, typeReference).getBody();
148+
OIDCProviderMetadata metadata = parse(configuration, OIDCProviderMetadata::parse);
149+
return withProviderConfiguration(metadata, issuer.toASCIIString())
150+
.jwkSetUri(metadata.getJWKSetURI().toASCIIString())
151+
.userInfoUri(metadata.getUserInfoEndpointURI().toASCIIString());
152+
};
153153
}
154154

155-
private static URI oidcRfc8414(URI issuer) {
156-
return UriComponentsBuilder.fromUri(issuer)
155+
private static Supplier<ClientRegistration.Builder> oidcRfc8414(URI issuer) {
156+
URI uri = UriComponentsBuilder.fromUri(issuer)
157157
.replacePath(OIDC_METADATA_PATH + issuer.getPath()).build(Collections.emptyMap());
158+
return getRfc8414Builder(issuer, uri);
158159
}
159160

160-
private static URI oauth(URI issuer) {
161-
return UriComponentsBuilder.fromUri(issuer)
161+
private static Supplier<ClientRegistration.Builder> oauth(URI issuer) {
162+
URI uri = UriComponentsBuilder.fromUri(issuer)
162163
.replacePath(OAUTH_METADATA_PATH + issuer.getPath()).build(Collections.emptyMap());
164+
return getRfc8414Builder(issuer, uri);
165+
}
166+
167+
private static Supplier<ClientRegistration.Builder> getRfc8414Builder(URI issuer, URI uri) {
168+
return () -> {
169+
RequestEntity<Void> request = RequestEntity.get(uri).build();
170+
Map<String, Object> configuration = rest.exchange(request, typeReference).getBody();
171+
AuthorizationServerMetadata metadata = parse(configuration, AuthorizationServerMetadata::parse);
172+
ClientRegistration.Builder builder = withProviderConfiguration(metadata, issuer.toASCIIString());
173+
174+
URI jwkSetUri = metadata.getJWKSetURI();
175+
if (jwkSetUri != null) {
176+
builder.jwkSetUri(jwkSetUri.toASCIIString());
177+
}
178+
179+
String userinfoEndpoint = (String) configuration.get("userinfo_endpoint");
180+
if (userinfoEndpoint != null) {
181+
builder.userInfoUri(userinfoEndpoint);
182+
}
183+
return builder;
184+
};
163185
}
164186

165-
private static Map<String, Object> getConfiguration(String issuer, URI... uris) {
187+
@SafeVarargs
188+
private static ClientRegistration.Builder getBuilder(String issuer, Supplier<ClientRegistration.Builder>... suppliers) {
166189
String errorMessage = "Unable to resolve Configuration with the provided Issuer of \"" + issuer + "\"";
167-
for (URI uri : uris) {
190+
for (Supplier<ClientRegistration.Builder> supplier : suppliers) {
168191
try {
169-
RequestEntity<Void> request = RequestEntity.get(uri).build();
170-
return rest.exchange(request, typeReference).getBody();
192+
return supplier.get();
171193
} catch (HttpClientErrorException e) {
172194
if (!e.getStatusCode().is4xxClientError()) {
173195
throw e;
174196
}
175197
// else try another endpoint
198+
} catch (IllegalArgumentException | IllegalStateException e) {
199+
throw e;
176200
} catch (RuntimeException e) {
177201
throw new IllegalArgumentException(errorMessage, e);
178202
}
@@ -219,7 +243,6 @@ private static ClientRegistration.Builder withProviderConfiguration(Authorizatio
219243
.clientAuthenticationMethod(method)
220244
.redirectUriTemplate("{baseUrl}/{action}/oauth2/code/{registrationId}")
221245
.authorizationUri(metadata.getAuthorizationEndpointURI().toASCIIString())
222-
.jwkSetUri(metadata.getJWKSetURI().toASCIIString())
223246
.providerConfigurationMetadata(configurationMetadata)
224247
.tokenUri(metadata.getTokenEndpointURI().toASCIIString())
225248
.clientName(issuer);

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,33 @@ private void assertIssuerMetadata(ClientRegistration registration,
168168
"grant_types_supported", "token_endpoint", "token_endpoint_auth_methods_supported", "userinfo_endpoint");
169169
}
170170

171+
// gh-7512
172+
@Test
173+
public void issuerWhenResponseMissingJwksUriThenThrowsIllegalArgumentException() throws Exception {
174+
this.response.remove("jwks_uri");
175+
assertThatThrownBy(() -> registration("").build())
176+
.isInstanceOf(IllegalArgumentException.class)
177+
.hasMessageContaining("The public JWK set URI must not be null");
178+
}
179+
180+
// gh-7512
181+
@Test
182+
public void issuerWhenOidcFallbackResponseMissingJwksUriThenThrowsIllegalArgumentException() throws Exception {
183+
this.response.remove("jwks_uri");
184+
assertThatThrownBy(() -> registrationOidcFallback("issuer1", null).build())
185+
.isInstanceOf(IllegalArgumentException.class)
186+
.hasMessageContaining("The public JWK set URI must not be null");
187+
}
188+
189+
// gh-7512
190+
@Test
191+
public void issuerWhenOAuth2ResponseMissingJwksUriThenThenSuccess() throws Exception {
192+
this.response.remove("jwks_uri");
193+
ClientRegistration registration = registrationOAuth2("", null).build();
194+
ClientRegistration.ProviderDetails provider = registration.getProviderDetails();
195+
assertThat(provider.getJwkSetUri()).isNull();
196+
}
197+
171198
@Test
172199
public void issuerWhenContainsTrailingSlashThenSuccess() throws Exception {
173200
assertThat(registration("")).isNotNull();

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
* issuer and method invoked.
3434
*
3535
* @author Thomas Vitale
36+
* @author Rafiullah Hamedy
3637
* @since 5.2
3738
*/
3839
class JwtDecoderProviderConfigurationUtils {
@@ -69,7 +70,15 @@ private static Map<String, Object> getConfiguration(String issuer, URI... uris)
6970
try {
7071
RequestEntity<Void> request = RequestEntity.get(uri).build();
7172
ResponseEntity<Map<String, Object>> response = rest.exchange(request, typeReference);
72-
return response.getBody();
73+
Map<String, Object> configuration = response.getBody();
74+
75+
if (configuration.get("jwks_uri") == null) {
76+
throw new IllegalArgumentException("The public JWK set URI must not be null");
77+
}
78+
79+
return configuration;
80+
} catch (IllegalArgumentException e) {
81+
throw e;
7382
} catch (RuntimeException e) {
7483
if (!(e instanceof HttpClientErrorException &&
7584
((HttpClientErrorException) e).getStatusCode().is4xxClientError())) {

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@
3333
import org.springframework.http.MediaType;
3434
import org.springframework.web.util.UriComponentsBuilder;
3535

36+
import com.fasterxml.jackson.core.JsonProcessingException;
37+
import com.fasterxml.jackson.core.type.TypeReference;
38+
import com.fasterxml.jackson.databind.JsonMappingException;
39+
import com.fasterxml.jackson.databind.ObjectMapper;
40+
3641
import static org.assertj.core.api.Assertions.assertThat;
3742
import static org.assertj.core.api.Assertions.assertThatCode;
3843

@@ -163,6 +168,36 @@ public void issuerWhenOAuth2ResponseIsNonCompliantThenThrowsRuntimeException() {
163168
.isInstanceOf(RuntimeException.class);
164169
}
165170

171+
// gh-7512
172+
@Test
173+
public void issuerWhenResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
174+
throws JsonMappingException, JsonProcessingException {
175+
prepareConfigurationResponse(this.buildResponseWithMissingJwksUri());
176+
assertThatCode(() -> JwtDecoders.fromOidcIssuerLocation(this.issuer))
177+
.isInstanceOf(IllegalArgumentException.class)
178+
.hasMessage("The public JWK set URI must not be null");
179+
}
180+
181+
// gh-7512
182+
@Test
183+
public void issuerWhenOidcFallbackResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
184+
throws JsonMappingException, JsonProcessingException {
185+
prepareConfigurationResponseOidc(this.buildResponseWithMissingJwksUri());
186+
assertThatCode(() -> JwtDecoders.fromIssuerLocation(this.issuer))
187+
.isInstanceOf(IllegalArgumentException.class)
188+
.hasMessage("The public JWK set URI must not be null");
189+
}
190+
191+
// gh-7512
192+
@Test
193+
public void issuerWhenOAuth2ResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
194+
throws JsonMappingException, JsonProcessingException {
195+
prepareConfigurationResponseOAuth2(this.buildResponseWithMissingJwksUri());
196+
assertThatCode(() -> JwtDecoders.fromIssuerLocation(this.issuer))
197+
.isInstanceOf(IllegalArgumentException.class)
198+
.hasMessage("The public JWK set URI must not be null");
199+
}
200+
166201
@Test
167202
public void issuerWhenResponseIsMalformedThenThrowsRuntimeException() {
168203
prepareConfigurationResponse("malformed");
@@ -294,4 +329,12 @@ private MockResponse response(String body) {
294329
.setBody(body)
295330
.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE);
296331
}
332+
333+
public String buildResponseWithMissingJwksUri() throws JsonMappingException, JsonProcessingException {
334+
ObjectMapper mapper = new ObjectMapper();
335+
Map<String, Object> response = mapper.readValue(DEFAULT_RESPONSE_TEMPLATE,
336+
new TypeReference<Map<String, Object>>(){});
337+
response.remove("jwks_uri");
338+
return mapper.writeValueAsString(response);
339+
}
297340
}

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

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -33,12 +33,18 @@
3333
import org.springframework.http.MediaType;
3434
import org.springframework.web.util.UriComponentsBuilder;
3535

36+
import com.fasterxml.jackson.core.JsonProcessingException;
37+
import com.fasterxml.jackson.core.type.TypeReference;
38+
import com.fasterxml.jackson.databind.JsonMappingException;
39+
import com.fasterxml.jackson.databind.ObjectMapper;
40+
3641
import static org.assertj.core.api.Assertions.assertThatCode;
3742

3843
/**
3944
* Tests for {@link ReactiveJwtDecoders}
4045
*
4146
* @author Josh Cummings
47+
* @author Rafiullah Hamedy
4248
*/
4349
public class ReactiveJwtDecodersTests {
4450
/**
@@ -76,14 +82,12 @@ public class ReactiveJwtDecodersTests {
7682

7783
private MockWebServer server;
7884
private String issuer;
79-
private String jwkSetUri;
8085

8186
@Before
8287
public void setup() throws Exception {
8388
this.server = new MockWebServer();
8489
this.server.start();
8590
this.issuer = createIssuerFromServer();
86-
this.jwkSetUri = this.issuer + ".well-known/jwks.json";
8791
this.issuer += "path";
8892
}
8993

@@ -147,6 +151,36 @@ public void issuerWhenOAuth2ResponseIsNonCompliantThenThrowsRuntimeException() {
147151
.isInstanceOf(RuntimeException.class);
148152
}
149153

154+
// gh-7512
155+
@Test
156+
public void issuerWhenResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
157+
throws JsonMappingException, JsonProcessingException {
158+
prepareConfigurationResponse(this.buildResponseWithMissingJwksUri());
159+
assertThatCode(() -> ReactiveJwtDecoders.fromOidcIssuerLocation(this.issuer))
160+
.isInstanceOf(IllegalArgumentException.class)
161+
.hasMessage("The public JWK set URI must not be null");
162+
}
163+
164+
// gh-7512
165+
@Test
166+
public void issuerWhenOidcFallbackResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
167+
throws JsonMappingException, JsonProcessingException {
168+
prepareConfigurationResponseOidc(this.buildResponseWithMissingJwksUri());
169+
assertThatCode(() -> ReactiveJwtDecoders.fromIssuerLocation(this.issuer))
170+
.isInstanceOf(IllegalArgumentException.class)
171+
.hasMessage("The public JWK set URI must not be null");
172+
}
173+
174+
// gh-7512
175+
@Test
176+
public void issuerWhenOAuth2ResponseDoesNotContainJwksUriThenThrowsIllegalArgumentException()
177+
throws JsonMappingException, JsonProcessingException {
178+
prepareConfigurationResponseOAuth2(this.buildResponseWithMissingJwksUri());
179+
assertThatCode(() -> ReactiveJwtDecoders.fromIssuerLocation(this.issuer))
180+
.isInstanceOf(IllegalArgumentException.class)
181+
.hasMessage("The public JWK set URI must not be null");
182+
}
183+
150184
@Test
151185
public void issuerWhenResponseIsMalformedThenThrowsRuntimeException() {
152186
prepareConfigurationResponse("malformed");
@@ -281,4 +315,12 @@ private MockResponse response(String body) {
281315
.setBody(body)
282316
.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE);
283317
}
318+
319+
public String buildResponseWithMissingJwksUri() throws JsonMappingException, JsonProcessingException {
320+
ObjectMapper mapper = new ObjectMapper();
321+
Map<String, Object> response = mapper.readValue(DEFAULT_RESPONSE_TEMPLATE,
322+
new TypeReference<Map<String, Object>>(){});
323+
response.remove("jwks_uri");
324+
return mapper.writeValueAsString(response);
325+
}
284326
}

0 commit comments

Comments
 (0)