[java-idp-plugin-oidc-rp] branch main updated: Cleanup response decoder logic
Phil Smart
philip.smart at jisc.ac.uk
Tue Aug 29 18:01:44 UTC 2023
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch main
in repository java-idp-plugin-oidc-rp.
View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-oidc-rp.git;a=commit;h=5d39a3a3c51d3857d648059f56bbff6d20a5d6fb
The following commit(s) were added to refs/heads/main by this push:
new 5d39a3a Cleanup response decoder logic
5d39a3a is described below
commit 5d39a3a3c51d3857d648059f56bbff6d20a5d6fb
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Aug 29 19:01:41 2023 +0100
Cleanup response decoder logic
---
.../impl/AbstractJSONResponseDecoderFunction.java | 3 +++
.../impl/DefaultAccessTokenResponseDecoder.java | 26 +++++++++++--------
.../impl/DefaultUserInfoResponseDecoder.java | 29 +++++++++-------------
.../DefaultAccessTokenResponseDecoderTest.java | 5 ++--
4 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/AbstractJSONResponseDecoderFunction.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/AbstractJSONResponseDecoderFunction.java
index 691f093..f71eb29 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/AbstractJSONResponseDecoderFunction.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/AbstractJSONResponseDecoderFunction.java
@@ -29,6 +29,9 @@ import net.shibboleth.shared.logic.Constraint;
/**
* Abstract class for JSON based response decoders.
+ *
+ * <p>Note, the Http Response should not be closed inside a decoder implementation, but MUST be closed by the calling
+ * class. However, any input stream obtained by the decoder MUST ensure the stream is closed.</p>
*
* @param <T> the return type of the function.
*/
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultAccessTokenResponseDecoder.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultAccessTokenResponseDecoder.java
index b0dec07..0aadc7d 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultAccessTokenResponseDecoder.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultAccessTokenResponseDecoder.java
@@ -23,6 +23,7 @@ import javax.annotation.Nullable;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpStatus;
import org.slf4j.Logger;
import org.springframework.http.MediaType;
@@ -49,9 +50,13 @@ public class DefaultAccessTokenResponseDecoder extends AbstractJSONResponseDecod
@Nullable public TokenResponse apply(@Nullable final ClassicHttpResponse httpResponse) {
try {
-
- if (httpResponse == null || httpResponse.getEntity() == null) {
- log.warn("No HTTP response, or response did not contain an entity");
+ if (httpResponse == null) {
+ log.warn("HttpResponse was null, can not process response");
+ return null;
+ }
+ final HttpEntity entity = httpResponse.getEntity();
+ if (entity == null) {
+ log.warn("HTTP response did not contain an entity");
return null;
}
@@ -60,14 +65,19 @@ public class DefaultAccessTokenResponseDecoder extends AbstractJSONResponseDecod
log.warn("HTTP response did not contain a content-type, must contain a content-type");
return null;
}
- final var mimeType = contentType.getMimeType();
+ final String mimeType = contentType.getMimeType();
assert mimeType != null;
if (MediaType.APPLICATION_JSON.compareTo(MimeType.valueOf(mimeType)) != 0) {
log.warn("Wrong content type header, expected 'application/json' found '{}'", contentType.getMimeType());
return null;
}
- try (InputStream input = httpResponse.getEntity().getContent()) {
+ try (final InputStream input = httpResponse.getEntity().getContent()) {
+ if (input == null) {
+ log.warn("HTTP response does not contain a message entity, nothing to decode");
+ return null;
+ }
+
final Map<String, Object> tokenResponseAsMap = getObjectMapper().readValue(
input, new TypeReference<Map<String, Object>>() {});
if (log.isTraceEnabled()) {
@@ -77,11 +87,7 @@ public class DefaultAccessTokenResponseDecoder extends AbstractJSONResponseDecod
if (httpStatusCode != HttpStatus.SC_OK) {
return TokenErrorResponse.parse(new JSONObject(tokenResponseAsMap));
- } else if (httpResponse.getEntity() == null || httpResponse.getEntity().getContent() == null) {
- log.warn("HTTP response does not contain a message entity, nothing to decode, status '{}'",
- httpStatusCode);
- return null;
- }
+ }
return OIDCTokenResponse.parse(new JSONObject(tokenResponseAsMap));
}
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultUserInfoResponseDecoder.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultUserInfoResponseDecoder.java
index a81f8be..6d98d2d 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultUserInfoResponseDecoder.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultUserInfoResponseDecoder.java
@@ -22,6 +22,7 @@ import javax.annotation.Nullable;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.HttpStatus;
import org.slf4j.Logger;
@@ -68,17 +69,10 @@ public class DefaultUserInfoResponseDecoder extends AbstractJSONResponseDecoderF
final int httpStatusCode = httpResponse.getCode();
if (httpStatusCode != HttpStatus.SC_OK) {
- if (httpResponse.getHeader(USERINFO_ERROR_RESPONSE_HEADER) != null) {
-
- final Header errorHeader = httpResponse.getHeader(USERINFO_ERROR_RESPONSE_HEADER);
-
- if (errorHeader == null) {
- log.warn("HTTP status code implies error response, but no error given");
- return null;
- } else {
- return UserInfoErrorResponse.parse(errorHeader.getValue());
- }
-
+ final Header errorHeader = httpResponse.getHeader(USERINFO_ERROR_RESPONSE_HEADER);
+
+ if (errorHeader != null) {
+ return UserInfoErrorResponse.parse(errorHeader.getValue());
} else {
log.warn("HTTP status code implies error response, but no error given");
return null;
@@ -86,17 +80,18 @@ public class DefaultUserInfoResponseDecoder extends AbstractJSONResponseDecoderF
} else {
// Response indicates success
- if (httpResponse.getEntity() == null || httpResponse.getEntity().getContent() == null) {
+ final HttpEntity entity = httpResponse.getEntity();
+ if (entity == null) {
log.warn("HTTP response did not contain a response entity, nothing to decode");
return null;
}
-
- if (httpResponse.getEntity().getContentType() == null) {
+ final String contentTypeString = entity.getContentType();
+ if (contentTypeString == null) {
log.warn("HTTP response did not contain a content-type, must contain a content-type");
return null;
}
- final ContentType contentType = ContentType.parse(httpResponse.getEntity().getContentType());
+ final ContentType contentType = ContentType.parse(contentTypeString);
if (contentType == null) {
log.warn("HTTP response did not contain a valid content-type");
return null;
@@ -105,7 +100,7 @@ public class DefaultUserInfoResponseDecoder extends AbstractJSONResponseDecoderF
// Is a JWT type or plain JSON object
if (ContentType.APPLICATION_JWT.matches(contentType)) {
- try (InputStream input = httpResponse.getEntity().getContent()) {
+ try (InputStream input = entity.getContent()) {
final String content = IOUtils.readInputStreamToString(input);
final JWT parsedJwt = JWTParser.parse(content);
return new UserInfoSuccessResponse(parsedJwt);
@@ -113,7 +108,7 @@ public class DefaultUserInfoResponseDecoder extends AbstractJSONResponseDecoderF
} else if (ContentType.APPLICATION_JSON.matches(contentType)){
- try (InputStream input = httpResponse.getEntity().getContent()) {
+ try (InputStream input = entity.getContent()) {
final String content = IOUtils.readInputStreamToString(input);
final Map<String, Object> claims = getObjectMapper().readValue(
content, new TypeReference<Map<String, Object>>() {});
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultAccessTokenResponseDecoderTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultAccessTokenResponseDecoderTest.java
index 26f16fd..02426ed 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultAccessTokenResponseDecoderTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultAccessTokenResponseDecoderTest.java
@@ -75,11 +75,10 @@ public class DefaultAccessTokenResponseDecoderTest extends AbstractOIDCTest {
}
@Test
- public void testEncoder_NullResponseFields() throws ComponentInitializationException {
+ public void testEncoder_NullResponse() throws ComponentInitializationException {
decoder.initialize();
- final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);
- final TokenResponse decodedResponse = decoder.apply(response);
+ final TokenResponse decodedResponse = decoder.apply(null);
assertNull(decodedResponse);
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list