Skip to content

Commit 95c2403

Browse files
ascopesjzheaux
ascopes
authored and
jzheaux
committed
Fixed potential NullPointerException in opaque token introspection
It appears Nimbus does not check the presence of the Content-Type header before parsing it in some versions, and since prior to this commit, the code is .toString()-ing the result, a malformed response (such as that from a misbehaving cloud gateway) that does not include a Content-Type would currently throw a NullPointerException. In addition to this, I have added a little more information to the log output for this module on the standard and reactive implementations to aid in debugging authorization/authentication issues much more easily.
1 parent dd43d91 commit 95c2403

File tree

7 files changed

+126
-42
lines changed

7 files changed

+126
-42
lines changed

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/NimbusOpaqueTokenIntrospector.java

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.List;
2525
import java.util.Map;
2626

27+
import com.nimbusds.oauth2.sdk.ErrorObject;
2728
import com.nimbusds.oauth2.sdk.TokenIntrospectionResponse;
2829
import com.nimbusds.oauth2.sdk.TokenIntrospectionSuccessResponse;
2930
import com.nimbusds.oauth2.sdk.http.HTTPResponse;
@@ -159,11 +160,32 @@ private ResponseEntity<String> makeRequest(RequestEntity<?> requestEntity) {
159160
}
160161

161162
private HTTPResponse adaptToNimbusResponse(ResponseEntity<String> responseEntity) {
163+
MediaType contentType = responseEntity.getHeaders().getContentType();
164+
165+
if (contentType == null) {
166+
this.logger.trace("Did not receive Content-Type from introspection endpoint in response");
167+
168+
throw new OAuth2IntrospectionException(
169+
"Introspection endpoint response was invalid, as no Content-Type header was provided");
170+
}
171+
172+
// Nimbus expects JSON, but does not appear to validate this header first.
173+
if (!contentType.isCompatibleWith(MediaType.APPLICATION_JSON)) {
174+
this.logger.trace("Did not receive JSON-compatible Content-Type from introspection endpoint in response");
175+
176+
throw new OAuth2IntrospectionException("Introspection endpoint response was invalid, as content type '"
177+
+ contentType + "' is not compatible with JSON");
178+
}
179+
162180
HTTPResponse response = new HTTPResponse(responseEntity.getStatusCodeValue());
163-
response.setHeader(HttpHeaders.CONTENT_TYPE, responseEntity.getHeaders().getContentType().toString());
181+
response.setHeader(HttpHeaders.CONTENT_TYPE, contentType.toString());
164182
response.setContent(responseEntity.getBody());
183+
165184
if (response.getStatusCode() != HTTPResponse.SC_OK) {
166-
throw new OAuth2IntrospectionException("Introspection endpoint responded with " + response.getStatusCode());
185+
this.logger.trace("Introspection endpoint returned non-OK status code");
186+
187+
throw new OAuth2IntrospectionException(
188+
"Introspection endpoint responded with HTTP status code " + response.getStatusCode());
167189
}
168190
return response;
169191
}
@@ -179,7 +201,10 @@ private TokenIntrospectionResponse parseNimbusResponse(HTTPResponse response) {
179201

180202
private TokenIntrospectionSuccessResponse castToNimbusSuccess(TokenIntrospectionResponse introspectionResponse) {
181203
if (!introspectionResponse.indicatesSuccess()) {
182-
throw new OAuth2IntrospectionException("Token introspection failed");
204+
ErrorObject errorObject = introspectionResponse.toErrorResponse().getErrorObject();
205+
String message = "Token introspection failed with response " + errorObject.toJSONObject().toJSONString();
206+
this.logger.trace(message);
207+
throw new OAuth2IntrospectionException(message);
183208
}
184209
return (TokenIntrospectionSuccessResponse) introspectionResponse;
185210
}

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/NimbusReactiveOpaqueTokenIntrospector.java

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,13 @@
2424
import java.util.List;
2525
import java.util.Map;
2626

27+
import com.nimbusds.oauth2.sdk.ErrorObject;
2728
import com.nimbusds.oauth2.sdk.TokenIntrospectionResponse;
2829
import com.nimbusds.oauth2.sdk.TokenIntrospectionSuccessResponse;
2930
import com.nimbusds.oauth2.sdk.http.HTTPResponse;
3031
import com.nimbusds.oauth2.sdk.id.Audience;
32+
import org.apache.commons.logging.Log;
33+
import org.apache.commons.logging.LogFactory;
3134
import reactor.core.publisher.Mono;
3235

3336
import org.springframework.core.io.buffer.DataBuffer;
@@ -54,6 +57,8 @@
5457
*/
5558
public class NimbusReactiveOpaqueTokenIntrospector implements ReactiveOpaqueTokenIntrospector {
5659

60+
private final Log logger = LogFactory.getLog(getClass());
61+
5762
private final URI introspectionUri;
5863

5964
private final WebClient webClient;
@@ -113,14 +118,31 @@ private Mono<ClientResponse> makeRequest(String token) {
113118
}
114119

115120
private Mono<HTTPResponse> adaptToNimbusResponse(ClientResponse responseEntity) {
121+
MediaType contentType = responseEntity.headers().contentType().orElseThrow(() -> {
122+
this.logger.trace("Did not receive Content-Type from introspection endpoint in response");
123+
124+
throw new OAuth2IntrospectionException(
125+
"Introspection endpoint response was invalid, as no Content-Type header was provided");
126+
});
127+
128+
// Nimbus expects JSON, but does not appear to validate this header first.
129+
if (!contentType.isCompatibleWith(MediaType.APPLICATION_JSON)) {
130+
this.logger.trace("Did not receive JSON-compatible Content-Type from introspection endpoint in response");
131+
132+
throw new OAuth2IntrospectionException("Introspection endpoint response was invalid, as content type '"
133+
+ contentType + "' is not compatible with JSON");
134+
}
135+
116136
HTTPResponse response = new HTTPResponse(responseEntity.rawStatusCode());
117-
response.setHeader(HttpHeaders.CONTENT_TYPE, responseEntity.headers().contentType().get().toString());
137+
response.setHeader(HttpHeaders.CONTENT_TYPE, contentType.toString());
118138
if (response.getStatusCode() != HTTPResponse.SC_OK) {
139+
this.logger.trace("Introspection endpoint returned non-OK status code");
140+
119141
// @formatter:off
120142
return responseEntity.bodyToFlux(DataBuffer.class)
121143
.map(DataBufferUtils::release)
122144
.then(Mono.error(new OAuth2IntrospectionException(
123-
"Introspection endpoint responded with " + response.getStatusCode()))
145+
"Introspection endpoint responded with HTTP status code " + response.getStatusCode()))
124146
);
125147
// @formatter:on
126148
}
@@ -138,7 +160,10 @@ private TokenIntrospectionResponse parseNimbusResponse(HTTPResponse response) {
138160

139161
private TokenIntrospectionSuccessResponse castToNimbusSuccess(TokenIntrospectionResponse introspectionResponse) {
140162
if (!introspectionResponse.indicatesSuccess()) {
141-
throw new OAuth2IntrospectionException("Token introspection failed");
163+
ErrorObject errorObject = introspectionResponse.toErrorResponse().getErrorObject();
164+
String message = "Token introspection failed with response " + errorObject.toJSONObject().toJSONString();
165+
this.logger.trace(message);
166+
throw new OAuth2IntrospectionException(message);
142167
}
143168
return (TokenIntrospectionSuccessResponse) introspectionResponse;
144169
}
@@ -147,6 +172,7 @@ private void validate(String token, TokenIntrospectionSuccessResponse response)
147172
// relying solely on the authorization server to validate this token (not checking
148173
// 'exp', for example)
149174
if (!response.isActive()) {
175+
this.logger.trace("Did not validate token since it is inactive");
150176
throw new BadOpaqueTokenException("Provided token isn't active");
151177
}
152178
}

‎oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospector.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ private Map<String, Object> adaptToNimbusResponse(ResponseEntity<Map<String, Obj
158158
Map<String, Object> claims = responseEntity.getBody();
159159
// relying solely on the authorization server to validate this token (not checking
160160
// 'exp', for example)
161+
if (claims == null) {
162+
return Collections.emptyMap();
163+
}
164+
161165
boolean active = (boolean) claims.compute(OAuth2TokenIntrospectionClaimNames.ACTIVE, (k, v) -> {
162166
if (v instanceof String) {
163167
return Boolean.parseBoolean((String) v);

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/NimbusOpaqueTokenIntrospectorTests.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import okhttp3.mockwebserver.MockWebServer;
3232
import okhttp3.mockwebserver.RecordedRequest;
3333
import org.junit.jupiter.api.Test;
34+
import org.junit.jupiter.params.ParameterizedTest;
35+
import org.junit.jupiter.params.provider.ValueSource;
3436

3537
import org.springframework.core.convert.converter.Converter;
3638
import org.springframework.http.HttpHeaders;
@@ -45,6 +47,7 @@
4547
import static org.assertj.core.api.Assertions.assertThat;
4648
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
4749
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
50+
import static org.assertj.core.api.Assumptions.assumeThat;
4851
import static org.mockito.ArgumentMatchers.any;
4952
import static org.mockito.ArgumentMatchers.eq;
5053
import static org.mockito.BDDMockito.given;
@@ -308,6 +311,37 @@ public void setRequestEntityConverterWhenNonNullConverterGivenThenConverterUsed(
308311
verify(requestEntityConverter).convert(tokenToIntrospect);
309312
}
310313

314+
@Test
315+
public void handleMissingContentType() {
316+
RestOperations restOperations = mock(RestOperations.class);
317+
ResponseEntity<String> stubResponse = ResponseEntity.ok(ACTIVE_RESPONSE);
318+
given(restOperations.exchange(any(RequestEntity.class), eq(String.class))).willReturn(stubResponse);
319+
OpaqueTokenIntrospector introspectionClient = new NimbusOpaqueTokenIntrospector(INTROSPECTION_URL,
320+
restOperations);
321+
322+
// Protect against potential regressions where a default content type might be
323+
// added by default.
324+
assumeThat(stubResponse.getHeaders().getContentType()).isNull();
325+
326+
assertThatExceptionOfType(OAuth2IntrospectionException.class)
327+
.isThrownBy(() -> introspectionClient.introspect("sometokenhere"));
328+
}
329+
330+
@ParameterizedTest(name = "{displayName} when Content-Type={0}")
331+
@ValueSource(strings = { MediaType.APPLICATION_CBOR_VALUE, MediaType.TEXT_MARKDOWN_VALUE,
332+
MediaType.APPLICATION_XML_VALUE, MediaType.APPLICATION_OCTET_STREAM_VALUE })
333+
public void handleNonJsonContentType(String type) {
334+
RestOperations restOperations = mock(RestOperations.class);
335+
ResponseEntity<String> stubResponse = ResponseEntity.ok().contentType(MediaType.parseMediaType(type))
336+
.body(ACTIVE_RESPONSE);
337+
given(restOperations.exchange(any(RequestEntity.class), eq(String.class))).willReturn(stubResponse);
338+
OpaqueTokenIntrospector introspectionClient = new NimbusOpaqueTokenIntrospector(INTROSPECTION_URL,
339+
restOperations);
340+
341+
assertThatExceptionOfType(OAuth2IntrospectionException.class)
342+
.isThrownBy(() -> introspectionClient.introspect("sometokenhere"));
343+
}
344+
311345
private static ResponseEntity<String> response(String content) {
312346
HttpHeaders headers = new HttpHeaders();
313347
headers.setContentType(MediaType.APPLICATION_JSON);

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/NimbusReactiveOpaqueTokenIntrospectorTests.java

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import okhttp3.mockwebserver.MockWebServer;
3131
import okhttp3.mockwebserver.RecordedRequest;
3232
import org.junit.jupiter.api.Test;
33+
import org.junit.jupiter.params.ParameterizedTest;
34+
import org.junit.jupiter.params.provider.ValueSource;
3335
import reactor.core.publisher.Mono;
3436

3537
import org.springframework.http.HttpHeaders;
@@ -234,7 +236,35 @@ public void constructorWhenRestOperationsIsNullThenIllegalArgumentException() {
234236
.isThrownBy(() -> new NimbusReactiveOpaqueTokenIntrospector(INTROSPECTION_URL, null));
235237
}
236238

239+
@Test
240+
public void handleMissingContentType() {
241+
WebClient client = mockResponse(ACTIVE_RESPONSE, null);
242+
243+
ReactiveOpaqueTokenIntrospector introspectionClient = new NimbusReactiveOpaqueTokenIntrospector(
244+
INTROSPECTION_URL, client);
245+
246+
assertThatExceptionOfType(OAuth2IntrospectionException.class)
247+
.isThrownBy(() -> introspectionClient.introspect("sometokenhere").block());
248+
}
249+
250+
@ParameterizedTest(name = "{displayName} when Content-Type={0}")
251+
@ValueSource(strings = { MediaType.APPLICATION_CBOR_VALUE, MediaType.TEXT_MARKDOWN_VALUE,
252+
MediaType.APPLICATION_XML_VALUE, MediaType.APPLICATION_OCTET_STREAM_VALUE })
253+
public void handleNonJsonContentType(String type) {
254+
WebClient client = mockResponse(ACTIVE_RESPONSE, type);
255+
256+
ReactiveOpaqueTokenIntrospector introspectionClient = new NimbusReactiveOpaqueTokenIntrospector(
257+
INTROSPECTION_URL, client);
258+
259+
assertThatExceptionOfType(OAuth2IntrospectionException.class)
260+
.isThrownBy(() -> introspectionClient.introspect("sometokenhere").block());
261+
}
262+
237263
private WebClient mockResponse(String response) {
264+
return mockResponse(response, MediaType.APPLICATION_JSON_VALUE);
265+
}
266+
267+
private WebClient mockResponse(String response, String mediaType) {
238268
WebClient real = WebClient.builder().build();
239269
WebClient.RequestBodyUriSpec spec = spy(real.post());
240270
WebClient webClient = spy(WebClient.class);
@@ -244,7 +274,7 @@ private WebClient mockResponse(String response) {
244274
given(clientResponse.statusCode()).willReturn(HttpStatus.OK);
245275
given(clientResponse.bodyToMono(String.class)).willReturn(Mono.just(response));
246276
ClientResponse.Headers headers = mock(ClientResponse.Headers.class);
247-
given(headers.contentType()).willReturn(Optional.of(MediaType.APPLICATION_JSON));
277+
given(headers.contentType()).willReturn(Optional.ofNullable(mediaType).map(MediaType::parseMediaType));
248278
given(clientResponse.headers()).willReturn(headers);
249279
given(spec.exchange()).willReturn(Mono.just(clientResponse));
250280
return webClient;

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringOpaqueTokenIntrospectorTests.java

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,6 @@ public class SpringOpaqueTokenIntrospectorTests {
101101
+ " }";
102102
// @formatter:on
103103

104-
// @formatter:off
105-
private static final String MALFORMED_ISSUER_RESPONSE = "{\n"
106-
+ " \"active\" : \"true\",\n"
107-
+ " \"iss\" : \"badissuer\"\n"
108-
+ " }";
109-
// @formatter:on
110-
111104
// @formatter:off
112105
private static final String MALFORMED_SCOPE_RESPONSE = "{\n"
113106
+ " \"active\": true,\n"
@@ -129,8 +122,6 @@ public class SpringOpaqueTokenIntrospectorTests {
129122

130123
private static final ResponseEntity<Map<String, Object>> INVALID = response(INVALID_RESPONSE);
131124

132-
private static final ResponseEntity<Map<String, Object>> MALFORMED_ISSUER = response(MALFORMED_ISSUER_RESPONSE);
133-
134125
private static final ResponseEntity<Map<String, Object>> MALFORMED_SCOPE = response(MALFORMED_SCOPE_RESPONSE);
135126

136127
@Test
@@ -240,16 +231,6 @@ public void introspectWhenIntrospectionTokenReturnsInvalidResponseThenInvalidTok
240231
.isThrownBy(() -> introspectionClient.introspect("token"));
241232
}
242233

243-
@Test
244-
public void introspectWhenIntrospectionTokenReturnsMalformedIssuerResponseThenInvalidToken() {
245-
RestOperations restOperations = mock(RestOperations.class);
246-
OpaqueTokenIntrospector introspectionClient = new SpringOpaqueTokenIntrospector(INTROSPECTION_URL,
247-
restOperations);
248-
given(restOperations.exchange(any(RequestEntity.class), eq(STRING_OBJECT_MAP))).willReturn(MALFORMED_ISSUER);
249-
assertThatExceptionOfType(OAuth2IntrospectionException.class)
250-
.isThrownBy(() -> introspectionClient.introspect("token"));
251-
}
252-
253234
// gh-7563
254235
@Test
255236
public void introspectWhenIntrospectionTokenReturnsMalformedScopeThenEmptyAuthorities() {

‎oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/introspection/SpringReactiveOpaqueTokenIntrospectorTests.java

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,6 @@ public class SpringReactiveOpaqueTokenIntrospectorTests {
9696
+ " }";
9797
// @formatter:on
9898

99-
// @formatter:off
100-
private static final String MALFORMED_ISSUER_RESPONSE = "{\n"
101-
+ " \"active\" : \"true\",\n"
102-
+ " \"iss\" : \"badissuer\"\n"
103-
+ " }";
104-
// @formatter:on
105-
10699
private final ObjectMapper mapper = new ObjectMapper();
107100

108101
@Test
@@ -197,15 +190,6 @@ public void authenticateWhenIntrospectionTokenReturnsInvalidResponseThenInvalidT
197190
// @formatter:on
198191
}
199192

200-
@Test
201-
public void authenticateWhenIntrospectionTokenReturnsMalformedIssuerResponseThenInvalidToken() {
202-
WebClient webClient = mockResponse(MALFORMED_ISSUER_RESPONSE);
203-
SpringReactiveOpaqueTokenIntrospector introspectionClient = new SpringReactiveOpaqueTokenIntrospector(
204-
INTROSPECTION_URL, webClient);
205-
assertThatExceptionOfType(OAuth2IntrospectionException.class)
206-
.isThrownBy(() -> introspectionClient.introspect("token").block());
207-
}
208-
209193
@Test
210194
public void constructorWhenIntrospectionUriIsEmptyThenIllegalArgumentException() {
211195
assertThatIllegalArgumentException()

0 commit comments

Comments
 (0)