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.

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

@@ -24,6 +24,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
+import com.nimbusds.oauth2.sdk.ErrorObject;
 import com.nimbusds.oauth2.sdk.TokenIntrospectionResponse;
 import com.nimbusds.oauth2.sdk.TokenIntrospectionSuccessResponse;
 import com.nimbusds.oauth2.sdk.http.HTTPResponse;
@@ -159,11 +160,32 @@ public class NimbusOpaqueTokenIntrospector implements OpaqueTokenIntrospector {
 	}
 
 	private HTTPResponse adaptToNimbusResponse(ResponseEntity<String> responseEntity) {
+		MediaType contentType = responseEntity.getHeaders().getContentType();
+
+		if (contentType == null) {
+			this.logger.trace("Did not receive Content-Type from introspection endpoint in response");
+
+			throw new OAuth2IntrospectionException(
+					"Introspection endpoint response was invalid, as no Content-Type header was provided");
+		}
+
+		// Nimbus expects JSON, but does not appear to validate this header first.
+		if (!contentType.isCompatibleWith(MediaType.APPLICATION_JSON)) {
+			this.logger.trace("Did not receive JSON-compatible Content-Type from introspection endpoint in response");
+
+			throw new OAuth2IntrospectionException("Introspection endpoint response was invalid, as content type '"
+					+ contentType + "' is not compatible with JSON");
+		}
+
 		HTTPResponse response = new HTTPResponse(responseEntity.getStatusCodeValue());
-		response.setHeader(HttpHeaders.CONTENT_TYPE, responseEntity.getHeaders().getContentType().toString());
+		response.setHeader(HttpHeaders.CONTENT_TYPE, contentType.toString());
 		response.setContent(responseEntity.getBody());
+
 		if (response.getStatusCode() != HTTPResponse.SC_OK) {
-			throw new OAuth2IntrospectionException("Introspection endpoint responded with " + response.getStatusCode());
+			this.logger.trace("Introspection endpoint returned non-OK status code");
+
+			throw new OAuth2IntrospectionException(
+					"Introspection endpoint responded with HTTP status code " + response.getStatusCode());
 		}
 		return response;
 	}
@@ -179,7 +201,10 @@ public class NimbusOpaqueTokenIntrospector implements OpaqueTokenIntrospector {
 
 	private TokenIntrospectionSuccessResponse castToNimbusSuccess(TokenIntrospectionResponse introspectionResponse) {
 		if (!introspectionResponse.indicatesSuccess()) {
-			throw new OAuth2IntrospectionException("Token introspection failed");
+			ErrorObject errorObject = introspectionResponse.toErrorResponse().getErrorObject();
+			String message = "Token introspection failed with response " + errorObject.toJSONObject().toJSONString();
+			this.logger.trace(message);
+			throw new OAuth2IntrospectionException(message);
 		}
 		return (TokenIntrospectionSuccessResponse) introspectionResponse;
 	}

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

@@ -24,10 +24,13 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
+import com.nimbusds.oauth2.sdk.ErrorObject;
 import com.nimbusds.oauth2.sdk.TokenIntrospectionResponse;
 import com.nimbusds.oauth2.sdk.TokenIntrospectionSuccessResponse;
 import com.nimbusds.oauth2.sdk.http.HTTPResponse;
 import com.nimbusds.oauth2.sdk.id.Audience;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import reactor.core.publisher.Mono;
 
 import org.springframework.core.io.buffer.DataBuffer;
@@ -54,6 +57,8 @@ import org.springframework.web.reactive.function.client.WebClient;
  */
 public class NimbusReactiveOpaqueTokenIntrospector implements ReactiveOpaqueTokenIntrospector {
 
+	private final Log logger = LogFactory.getLog(getClass());
+
 	private final URI introspectionUri;
 
 	private final WebClient webClient;
@@ -113,14 +118,31 @@ public class NimbusReactiveOpaqueTokenIntrospector implements ReactiveOpaqueToke
 	}
 
 	private Mono<HTTPResponse> adaptToNimbusResponse(ClientResponse responseEntity) {
+		MediaType contentType = responseEntity.headers().contentType().orElseThrow(() -> {
+			this.logger.trace("Did not receive Content-Type from introspection endpoint in response");
+
+			throw new OAuth2IntrospectionException(
+					"Introspection endpoint response was invalid, as no Content-Type header was provided");
+		});
+
+		// Nimbus expects JSON, but does not appear to validate this header first.
+		if (!contentType.isCompatibleWith(MediaType.APPLICATION_JSON)) {
+			this.logger.trace("Did not receive JSON-compatible Content-Type from introspection endpoint in response");
+
+			throw new OAuth2IntrospectionException("Introspection endpoint response was invalid, as content type '"
+					+ contentType + "' is not compatible with JSON");
+		}
+
 		HTTPResponse response = new HTTPResponse(responseEntity.rawStatusCode());
-		response.setHeader(HttpHeaders.CONTENT_TYPE, responseEntity.headers().contentType().get().toString());
+		response.setHeader(HttpHeaders.CONTENT_TYPE, contentType.toString());
 		if (response.getStatusCode() != HTTPResponse.SC_OK) {
+			this.logger.trace("Introspection endpoint returned non-OK status code");
+
 			// @formatter:off
 			return responseEntity.bodyToFlux(DataBuffer.class)
 					.map(DataBufferUtils::release)
 					.then(Mono.error(new OAuth2IntrospectionException(
-							"Introspection endpoint responded with " + response.getStatusCode()))
+							"Introspection endpoint responded with HTTP status code " + response.getStatusCode()))
 					);
 			// @formatter:on
 		}
@@ -138,7 +160,10 @@ public class NimbusReactiveOpaqueTokenIntrospector implements ReactiveOpaqueToke
 
 	private TokenIntrospectionSuccessResponse castToNimbusSuccess(TokenIntrospectionResponse introspectionResponse) {
 		if (!introspectionResponse.indicatesSuccess()) {
-			throw new OAuth2IntrospectionException("Token introspection failed");
+			ErrorObject errorObject = introspectionResponse.toErrorResponse().getErrorObject();
+			String message = "Token introspection failed with response " + errorObject.toJSONObject().toJSONString();
+			this.logger.trace(message);
+			throw new OAuth2IntrospectionException(message);
 		}
 		return (TokenIntrospectionSuccessResponse) introspectionResponse;
 	}
@@ -147,6 +172,7 @@ public class NimbusReactiveOpaqueTokenIntrospector implements ReactiveOpaqueToke
 		// relying solely on the authorization server to validate this token (not checking
 		// 'exp', for example)
 		if (!response.isActive()) {
+			this.logger.trace("Did not validate token since it is inactive");
 			throw new BadOpaqueTokenException("Provided token isn't active");
 		}
 	}

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

@@ -158,6 +158,10 @@ public class SpringOpaqueTokenIntrospector implements OpaqueTokenIntrospector {
 		Map<String, Object> claims = responseEntity.getBody();
 		// relying solely on the authorization server to validate this token (not checking
 		// 'exp', for example)
+		if (claims == null) {
+			return Collections.emptyMap();
+		}
+
 		boolean active = (boolean) claims.compute(OAuth2TokenIntrospectionClaimNames.ACTIVE, (k, v) -> {
 			if (v instanceof String) {
 				return Boolean.parseBoolean((String) v);

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

@@ -31,6 +31,8 @@ import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 import okhttp3.mockwebserver.RecordedRequest;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.http.HttpHeaders;
@@ -45,6 +47,7 @@ import org.springframework.web.client.RestOperations;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.assertj.core.api.Assumptions.assumeThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.BDDMockito.given;
@@ -308,6 +311,37 @@ public class NimbusOpaqueTokenIntrospectorTests {
 		verify(requestEntityConverter).convert(tokenToIntrospect);
 	}
 
+	@Test
+	public void handleMissingContentType() {
+		RestOperations restOperations = mock(RestOperations.class);
+		ResponseEntity<String> stubResponse = ResponseEntity.ok(ACTIVE_RESPONSE);
+		given(restOperations.exchange(any(RequestEntity.class), eq(String.class))).willReturn(stubResponse);
+		OpaqueTokenIntrospector introspectionClient = new NimbusOpaqueTokenIntrospector(INTROSPECTION_URL,
+				restOperations);
+
+		// Protect against potential regressions where a default content type might be
+		// added by default.
+		assumeThat(stubResponse.getHeaders().getContentType()).isNull();
+
+		assertThatExceptionOfType(OAuth2IntrospectionException.class)
+				.isThrownBy(() -> introspectionClient.introspect("sometokenhere"));
+	}
+
+	@ParameterizedTest(name = "{displayName} when Content-Type={0}")
+	@ValueSource(strings = { MediaType.APPLICATION_CBOR_VALUE, MediaType.TEXT_MARKDOWN_VALUE,
+			MediaType.APPLICATION_XML_VALUE, MediaType.APPLICATION_OCTET_STREAM_VALUE })
+	public void handleNonJsonContentType(String type) {
+		RestOperations restOperations = mock(RestOperations.class);
+		ResponseEntity<String> stubResponse = ResponseEntity.ok().contentType(MediaType.parseMediaType(type))
+				.body(ACTIVE_RESPONSE);
+		given(restOperations.exchange(any(RequestEntity.class), eq(String.class))).willReturn(stubResponse);
+		OpaqueTokenIntrospector introspectionClient = new NimbusOpaqueTokenIntrospector(INTROSPECTION_URL,
+				restOperations);
+
+		assertThatExceptionOfType(OAuth2IntrospectionException.class)
+				.isThrownBy(() -> introspectionClient.introspect("sometokenhere"));
+	}
+
 	private static ResponseEntity<String> response(String content) {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setContentType(MediaType.APPLICATION_JSON);

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

@@ -30,6 +30,8 @@ import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 import okhttp3.mockwebserver.RecordedRequest;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import reactor.core.publisher.Mono;
 
 import org.springframework.http.HttpHeaders;
@@ -234,7 +236,35 @@ public class NimbusReactiveOpaqueTokenIntrospectorTests {
 				.isThrownBy(() -> new NimbusReactiveOpaqueTokenIntrospector(INTROSPECTION_URL, null));
 	}
 
+	@Test
+	public void handleMissingContentType() {
+		WebClient client = mockResponse(ACTIVE_RESPONSE, null);
+
+		ReactiveOpaqueTokenIntrospector introspectionClient = new NimbusReactiveOpaqueTokenIntrospector(
+				INTROSPECTION_URL, client);
+
+		assertThatExceptionOfType(OAuth2IntrospectionException.class)
+				.isThrownBy(() -> introspectionClient.introspect("sometokenhere").block());
+	}
+
+	@ParameterizedTest(name = "{displayName} when Content-Type={0}")
+	@ValueSource(strings = { MediaType.APPLICATION_CBOR_VALUE, MediaType.TEXT_MARKDOWN_VALUE,
+			MediaType.APPLICATION_XML_VALUE, MediaType.APPLICATION_OCTET_STREAM_VALUE })
+	public void handleNonJsonContentType(String type) {
+		WebClient client = mockResponse(ACTIVE_RESPONSE, type);
+
+		ReactiveOpaqueTokenIntrospector introspectionClient = new NimbusReactiveOpaqueTokenIntrospector(
+				INTROSPECTION_URL, client);
+
+		assertThatExceptionOfType(OAuth2IntrospectionException.class)
+				.isThrownBy(() -> introspectionClient.introspect("sometokenhere").block());
+	}
+
 	private WebClient mockResponse(String response) {
+		return mockResponse(response, MediaType.APPLICATION_JSON_VALUE);
+	}
+
+	private WebClient mockResponse(String response, String mediaType) {
 		WebClient real = WebClient.builder().build();
 		WebClient.RequestBodyUriSpec spec = spy(real.post());
 		WebClient webClient = spy(WebClient.class);
@@ -244,7 +274,7 @@ public class NimbusReactiveOpaqueTokenIntrospectorTests {
 		given(clientResponse.statusCode()).willReturn(HttpStatus.OK);
 		given(clientResponse.bodyToMono(String.class)).willReturn(Mono.just(response));
 		ClientResponse.Headers headers = mock(ClientResponse.Headers.class);
-		given(headers.contentType()).willReturn(Optional.of(MediaType.APPLICATION_JSON));
+		given(headers.contentType()).willReturn(Optional.ofNullable(mediaType).map(MediaType::parseMediaType));
 		given(clientResponse.headers()).willReturn(headers);
 		given(spec.exchange()).willReturn(Mono.just(clientResponse));
 		return webClient;

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

@@ -101,13 +101,6 @@ public class SpringOpaqueTokenIntrospectorTests {
 			+ "     }";
 	// @formatter:on
 
-	// @formatter:off
-	private static final String MALFORMED_ISSUER_RESPONSE = "{\n"
-			+ "     \"active\" : \"true\",\n"
-			+ "     \"iss\" : \"badissuer\"\n"
-			+ "    }";
-	// @formatter:on
-
 	// @formatter:off
 	private static final String MALFORMED_SCOPE_RESPONSE = "{\n"
 			+ "      \"active\": true,\n"
@@ -129,8 +122,6 @@ public class SpringOpaqueTokenIntrospectorTests {
 
 	private static final ResponseEntity<Map<String, Object>> INVALID = response(INVALID_RESPONSE);
 
-	private static final ResponseEntity<Map<String, Object>> MALFORMED_ISSUER = response(MALFORMED_ISSUER_RESPONSE);
-
 	private static final ResponseEntity<Map<String, Object>> MALFORMED_SCOPE = response(MALFORMED_SCOPE_RESPONSE);
 
 	@Test
@@ -240,16 +231,6 @@ public class SpringOpaqueTokenIntrospectorTests {
 				.isThrownBy(() -> introspectionClient.introspect("token"));
 	}
 
-	@Test
-	public void introspectWhenIntrospectionTokenReturnsMalformedIssuerResponseThenInvalidToken() {
-		RestOperations restOperations = mock(RestOperations.class);
-		OpaqueTokenIntrospector introspectionClient = new SpringOpaqueTokenIntrospector(INTROSPECTION_URL,
-				restOperations);
-		given(restOperations.exchange(any(RequestEntity.class), eq(STRING_OBJECT_MAP))).willReturn(MALFORMED_ISSUER);
-		assertThatExceptionOfType(OAuth2IntrospectionException.class)
-				.isThrownBy(() -> introspectionClient.introspect("token"));
-	}
-
 	// gh-7563
 	@Test
 	public void introspectWhenIntrospectionTokenReturnsMalformedScopeThenEmptyAuthorities() {

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

@@ -96,13 +96,6 @@ public class SpringReactiveOpaqueTokenIntrospectorTests {
 			+ "     }";
 	// @formatter:on
 
-	// @formatter:off
-	private static final String MALFORMED_ISSUER_RESPONSE = "{\n"
-			+ "     \"active\" : \"true\",\n"
-			+ "     \"iss\" : \"badissuer\"\n"
-			+ "    }";
-	// @formatter:on
-
 	private final ObjectMapper mapper = new ObjectMapper();
 
 	@Test
@@ -197,15 +190,6 @@ public class SpringReactiveOpaqueTokenIntrospectorTests {
 		// @formatter:on
 	}
 
-	@Test
-	public void authenticateWhenIntrospectionTokenReturnsMalformedIssuerResponseThenInvalidToken() {
-		WebClient webClient = mockResponse(MALFORMED_ISSUER_RESPONSE);
-		SpringReactiveOpaqueTokenIntrospector introspectionClient = new SpringReactiveOpaqueTokenIntrospector(
-				INTROSPECTION_URL, webClient);
-		assertThatExceptionOfType(OAuth2IntrospectionException.class)
-				.isThrownBy(() -> introspectionClient.introspect("token").block());
-	}
-
 	@Test
 	public void constructorWhenIntrospectionUriIsEmptyThenIllegalArgumentException() {
 		assertThatIllegalArgumentException()