[java-idp-plugin-oidc-rp] branch main updated: JOIDCRP-46 - Convert HTTP client executeOpen calls to ResponseHandlers
Phil Smart
philip.smart at jisc.ac.uk
Fri Oct 6 16:06:43 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=9d5572c087be676057f5b43d850a84a6182dd97c
The following commit(s) were added to refs/heads/main by this push:
new 9d5572c JOIDCRP-46 - Convert HTTP client executeOpen calls to ResponseHandlers
9d5572c is described below
commit 9d5572c087be676057f5b43d850a84a6182dd97c
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Oct 6 17:06:35 2023 +0100
JOIDCRP-46 - Convert HTTP client executeOpen calls to ResponseHandlers
- Converted the access token and userinfo lookups to use response
handlers to decode the HTTP response.
https://shibboleth.atlassian.net/browse/JOIDCRP-46
---
.../impl/AbstractJSONResponseDecoderFunction.java | 11 ++-
.../impl/DefaultAccessTokenResponseDecoder.java | 12 ++--
.../impl/DefaultUserInfoResponseDecoder.java | 7 +-
.../impl/AbstractHttpOIDCAuthenticationAction.java | 53 +++++++-------
.../DefaultAccessTokenResponseDecoderTest.java | 19 +++--
.../impl/DefaultUserInfoResponseDecoderTest.java | 21 +++---
.../authn/oidc/rp/impl/AbstractOIDCTest.java | 5 ++
.../rp/impl/ExchangeCodeForAccessTokenTest.java | 83 ++++++----------------
.../oidc/rp/impl/UserInfoEndpointLookupTest.java | 40 ++++++-----
9 files changed, 108 insertions(+), 143 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 f71eb29..0ffbc90 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
@@ -14,11 +14,9 @@
package net.shibboleth.idp.plugin.authn.oidc.rp.decoding.impl;
-import java.util.function.Function;
-
import javax.annotation.Nonnull;
-import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -28,15 +26,14 @@ import net.shibboleth.shared.component.ComponentInitializationException;
import net.shibboleth.shared.logic.Constraint;
/**
- * Abstract class for JSON based response decoders.
+ * Abstract class for JSON based Http client 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>
+ * <p>Note, any input stream obtained by the decoder MUST ensure the stream is closed.</p>
*
* @param <T> the return type of the function.
*/
public abstract class AbstractJSONResponseDecoderFunction<T> extends AbstractInitializableComponent
- implements Function<ClassicHttpResponse, T> {
+ implements HttpClientResponseHandler<T>{
/** JSON object mapper. */
@NonnullAfterInit private ObjectMapper objectMapper;
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 0aadc7d..3a01080 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
@@ -15,15 +15,16 @@
package net.shibboleth.idp.plugin.authn.oidc.rp.decoding.impl;
+import java.io.IOException;
import java.io.InputStream;
import java.util.Map;
import javax.annotation.Nonnull;
-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.HttpException;
import org.apache.hc.core5.http.HttpStatus;
import org.slf4j.Logger;
import org.springframework.http.MediaType;
@@ -38,17 +39,18 @@ import net.minidev.json.JSONObject;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * Default access token response decoder, which converts a successful HTTP response into an
- * {@link OIDCTokenResponse}. Any decoding error is logged and {@code null} is returned.
+ * Default access token response decoder which converts a successful HTTP response into an
+ * {@link OIDCTokenResponse} and a unsuccessful response into an {@link TokenErrorResponse}.
+ * Any decoding error is logged and {@code null} is returned.
*/
public class DefaultAccessTokenResponseDecoder extends AbstractJSONResponseDecoderFunction<TokenResponse> {
/** Class logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(DefaultAccessTokenResponseDecoder.class);
+ /** {@inheritDoc} */
@Override
- @Nullable public TokenResponse apply(@Nullable final ClassicHttpResponse httpResponse) {
-
+ public TokenResponse handleResponse(final ClassicHttpResponse httpResponse) throws HttpException, IOException {
try {
if (httpResponse == null) {
log.warn("HttpResponse was null, can not process response");
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 6d98d2d..7af8df8 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
@@ -41,12 +41,13 @@ import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * Response decoder for UserInfo responses. Supports both plain JSON Object and JWT responses.
+ * The default Http client UserInfo response decoder. Supports both plain JSON Object and JWT responses.
*
* <p>Importantly,the decoder *must not ever* decode a JWT response as a plain response type, otherwise the signature
* check may not be performed downstream - although other validation for the plain object type should. That is, we
* can not rely solely on the content-type header in-case of content-type header injection attacks — the logic
- * that builds either the JWT or plain response should fail, or at least present an invalid UserInfo response token.</p>
+ * that builds either the JWT or plain response should fail, or at least present an invalid UserInfo response token.
+ * Any decoding error is logged and {@code null} is returned.</p>
*/
public class DefaultUserInfoResponseDecoder extends AbstractJSONResponseDecoderFunction<UserInfoResponse> {
@@ -58,7 +59,7 @@ public class DefaultUserInfoResponseDecoder extends AbstractJSONResponseDecoderF
// Checkstyle: CyclomaticComplexity|ReturnCount|MethodLength OFF
@Override
- public UserInfoResponse apply(@Nullable final ClassicHttpResponse httpResponse) {
+ public UserInfoResponse handleResponse(@Nullable final ClassicHttpResponse httpResponse) {
if (httpResponse == null) {
log.error("HttpResponse was null, can not process response");
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractHttpOIDCAuthenticationAction.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractHttpOIDCAuthenticationAction.java
index 9e71ff3..6b6a54e 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractHttpOIDCAuthenticationAction.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractHttpOIDCAuthenticationAction.java
@@ -24,7 +24,7 @@ import org.apache.hc.client5.http.classic.HttpClient;
import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.http.ClassicHttpRequest;
-import org.apache.hc.core5.http.ClassicHttpResponse;
+import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import org.opensaml.profile.context.ProfileRequestContext;
import org.opensaml.security.httpclient.HttpClientSecurityParameters;
import org.opensaml.security.httpclient.HttpClientSecuritySupport;
@@ -43,7 +43,8 @@ import net.shibboleth.shared.logic.Constraint;
import net.shibboleth.shared.primitive.LoggerFactory;
/**
- * An abstract class for OIDC actions that make synchronous HTTP requests and return {@link Response responses}.
+ * An abstract class for OIDC actions that make synchronous HTTP requests and return types of
+ * {@link Response responses}.
*
* @param <T> the response type of the object returned as a result of the request.
*/
@@ -57,10 +58,10 @@ public abstract class AbstractHttpOIDCAuthenticationAction<T extends Response>
/** The message encoder to encode the HTTP request into a {@link HttpUriRequest}.*/
@NonnullAfterInit private Function<ProfileRequestContext, ClassicHttpRequest> httpRequestEncoderStrategy;
- /** The message decoder to decode the HTTP response.*/
- @NonnullAfterInit private Function<ClassicHttpResponse, T> httpResponseDecoderStrategy;
+ /** The message decoder (Http response handler) to decode the HTTP response.*/
+ @NonnullAfterInit private HttpClientResponseHandler<T> httpResponseDecoderStrategy;
- /** HttpClient for contacting the endpoint. */
+ /** Http client for contacting the endpoint. */
@NonnullAfterInit private HttpClient httpClient;
/** HTTP client security parameters. */
@@ -92,20 +93,20 @@ public abstract class AbstractHttpOIDCAuthenticationAction<T extends Response>
}
/**
- * Get the HTTP response decoder strategy.
+ * Get the HTTP response decoder/handler strategy.
*
* @return the response decoder.
*/
- @NonnullAfterInit public Function<ClassicHttpResponse, T> getHttpResponseDecoderStrategy() {
+ @NonnullAfterInit public HttpClientResponseHandler<T> getHttpResponseDecoderStrategy() {
return httpResponseDecoderStrategy;
}
/**
- * Set the strategy used to map a HTTP response the response type.
+ * Set the strategy (Http client response handler) used to map a HTTP response the response type.
*
* @param strategy the strategy
*/
- public void setHttpResponseDecoderStrategy(@Nonnull final Function<ClassicHttpResponse, T> strategy) {
+ public void setHttpResponseDecoderStrategy(@Nonnull final HttpClientResponseHandler<T> strategy) {
checkSetterPreconditions();
httpResponseDecoderStrategy = Constraint.isNotNull(strategy, "Http decoder strategy can not be null");
@@ -146,9 +147,9 @@ public abstract class AbstractHttpOIDCAuthenticationAction<T extends Response>
}
/**
- * Encode the request using the supplied request encoder strategy. Execute a synchronous HTTP request
- * and decode the response using the supplied decoder strategy. If an error response is returned an exception
- * is thrown —the error object is not propagated back to the caller.
+ * Encode the request using the supplied request encoder strategy, execute a synchronous HTTP request
+ * and decode the response using the supplied Http client response decoder strategy/handler. If an error response
+ * is returned an exception is thrown—the OIDC error object is not propagated back to the caller.
*
* <p>The request encoder strategy should return null to indicate an error constructing the request.</p>
*
@@ -156,7 +157,7 @@ public abstract class AbstractHttpOIDCAuthenticationAction<T extends Response>
* @param authenticatableContext an authenticatable context to set the authenticated flag. Can be {@literal null} if
* no flag is supplied.
*
- * @return a successful decoded response.
+ * @return a successful decoded response. Never {@code null}.
*
* @throws OIDCRPException on error making the request, or if an error response is returned.
*/
@@ -167,15 +168,13 @@ public abstract class AbstractHttpOIDCAuthenticationAction<T extends Response>
if (request == null) {
throw new OIDCRPException("Unable to encode HTTP request");
}
- try (final ClassicHttpResponse response = executeHttpRequest(request, authenticatableContext)){
- final T responseObject = getHttpResponseDecoderStrategy().apply(response);
- if (responseObject == null) {
- throw new OIDCRPException("Unable to process HTTP response");
- } else if (!responseObject.indicatesSuccess()) {
- throw new OIDCRPException(formatErrorResponse(((ErrorResponse)responseObject).getErrorObject()));
- }
- return responseObject;
+ final T response = executeHttpRequest(request, authenticatableContext);
+ if (response == null) {
+ throw new OIDCRPException("Unable to process HTTP response");
+ } else if (!response.indicatesSuccess()) {
+ throw new OIDCRPException(formatErrorResponse(((ErrorResponse)response).getErrorObject()));
}
+ return response;
} catch (final IOException e) {
log.error("{} Unable to perform HTTP request and return response",getLogPrefix(),e);
throw new OIDCRPException(e);
@@ -205,19 +204,18 @@ public abstract class AbstractHttpOIDCAuthenticationAction<T extends Response>
/**
- * Performs a call to an HTTP endpoint using the configured HttpClient and security parameters.
- *
- * <p>Note, it is the responsibility of the caller to ensure the response is consumed and closed.</p>
+ * Performs a call to an Http endpoint using the configured HttpClient, HttpClientResponseHandler, and
+ * security parameters.
*
* @param request the prepared HTTP request
* @param authenticatableContext an authenticatable context to set the authenticated flag. Can be {@literal null} if
* no flag is supplied.
*
- * @return the HTTP response, never {@code null}. The response will remain open until closed.
+ * @return the encoded Http response.
*
* @throws IOException if there is an error producing a response
*/
- @Nonnull protected ClassicHttpResponse executeHttpRequest(@Nonnull final ClassicHttpRequest request,
+ @Nullable protected T executeHttpRequest(@Nonnull final ClassicHttpRequest request,
@Nullable final AbstractAuthenticatableOIDCContext authenticatableContext) throws IOException {
Constraint.isNotNull(request, "Request can not be null");
@@ -226,8 +224,7 @@ public abstract class AbstractHttpOIDCAuthenticationAction<T extends Response>
assert clientContext != null;
HttpClientSecuritySupport.marshalSecurityParameters(clientContext, httpClientSecurityParameters, true);
HttpClientSecuritySupport.addDefaultTLSTrustEngineCriteria(clientContext, request);
- final ClassicHttpResponse httpResponse = httpClient.executeOpen(null, request, clientContext);
- assert httpResponse != null;
+ final T httpResponse = httpClient.execute(request, clientContext, httpResponseDecoderStrategy);
final String scheme = request.getScheme();
assert scheme != null;
HttpClientSecuritySupport.checkTLSCredentialEvaluated(clientContext, scheme);
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 02426ed..1e23cbe 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
@@ -20,8 +20,6 @@ import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
-import java.io.IOException;
-
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.io.entity.StringEntity;
@@ -37,7 +35,6 @@ import com.nimbusds.oauth2.sdk.TokenResponse;
import com.nimbusds.openid.connect.sdk.OIDCTokenResponse;
import net.shibboleth.idp.plugin.authn.oidc.rp.impl.AbstractOIDCTest;
-import net.shibboleth.shared.component.ComponentInitializationException;
/** Tests for the DefaultAccessTokenResponseDecoder.*/
public class DefaultAccessTokenResponseDecoderTest extends AbstractOIDCTest {
@@ -56,7 +53,7 @@ public class DefaultAccessTokenResponseDecoderTest extends AbstractOIDCTest {
@SuppressWarnings("null")
@Test
- public void testEncoder_Success() throws ComponentInitializationException, UnsupportedOperationException, IOException {
+ public void testEncoder_Success() throws Exception {
decoder.initialize();
final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);
@@ -64,7 +61,7 @@ public class DefaultAccessTokenResponseDecoderTest extends AbstractOIDCTest {
ContentType.parse(MediaType.APPLICATION_JSON_VALUE)));
Mockito.when(response.getCode()).thenReturn(200);
- final TokenResponse decodedResponse = decoder.apply(response);
+ final TokenResponse decodedResponse = decoder.handleResponse(response);
assertNotNull(decodedResponse);
assertTrue(decodedResponse.indicatesSuccess());
final AccessTokenResponse tokenResponse = decodedResponse.toSuccessResponse();
@@ -75,16 +72,16 @@ public class DefaultAccessTokenResponseDecoderTest extends AbstractOIDCTest {
}
@Test
- public void testEncoder_NullResponse() throws ComponentInitializationException {
+ public void testEncoder_NullResponse() throws Exception {
decoder.initialize();
- final TokenResponse decodedResponse = decoder.apply(null);
+ final TokenResponse decodedResponse = decoder.handleResponse(null);
assertNull(decodedResponse);
}
@SuppressWarnings("null")
@Test
- public void testEncoder_InternalServerErrorResponse() throws ComponentInitializationException {
+ public void testEncoder_InternalServerErrorResponse() throws Exception {
decoder.initialize();
final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);
@@ -92,7 +89,7 @@ public class DefaultAccessTokenResponseDecoderTest extends AbstractOIDCTest {
ContentType.parse(MediaType.APPLICATION_JSON_VALUE)));
Mockito.when(response.getCode()).thenReturn(500);
- final TokenResponse decodedResponse = decoder.apply(response);
+ final TokenResponse decodedResponse = decoder.handleResponse(response);
assertNotNull(decodedResponse);
assertFalse(decodedResponse.indicatesSuccess());
@@ -101,14 +98,14 @@ public class DefaultAccessTokenResponseDecoderTest extends AbstractOIDCTest {
}
@Test
- public void testEncoder_NoMessageEntity() throws ComponentInitializationException {
+ public void testEncoder_NoMessageEntity() throws Exception {
decoder.initialize();
final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);
Mockito.when(response.getCode()).thenReturn(200);
when(response.getEntity()).thenReturn(null);
- final TokenResponse decodedResponse = decoder.apply(response);
+ final TokenResponse decodedResponse = decoder.handleResponse(response);
assertNull(decodedResponse);
}
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultUserInfoResponseDecoderTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultUserInfoResponseDecoderTest.java
index c7575e5..1cda9b5 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultUserInfoResponseDecoderTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/decoding/impl/DefaultUserInfoResponseDecoderTest.java
@@ -23,11 +23,10 @@ import static org.testng.Assert.assertTrue;
import java.io.ByteArrayInputStream;
-import org.apache.hc.core5.http.Header;
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.ProtocolException;
-import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.http.message.BasicHeader;
import org.mockito.Mockito;
import org.springframework.http.MediaType;
@@ -73,7 +72,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
when(response.getHeader(DefaultUserInfoResponseDecoder.USERINFO_ERROR_RESPONSE_HEADER))
.thenReturn(header);
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNotNull(userInfoResponse);
assertFalse(userInfoResponse.indicatesSuccess());
final ErrorObject errorMsg = ((UserInfoErrorResponse)userInfoResponse).getErrorObject();
@@ -88,7 +87,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
Mockito.when(response.getCode()).thenReturn(200);
when(response.getEntity()).thenReturn(null);
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNull(userInfoResponse);
}
@@ -102,7 +101,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
when(response.getEntity()).thenReturn(mockHttpEntity);
when(mockHttpEntity.getContentType()).thenReturn(MediaType.APPLICATION_JSON_VALUE);
when(mockHttpEntity.getContent()).thenReturn(new ByteArrayInputStream(USERINFO_RESPONSE.getBytes()));
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNotNull(userInfoResponse);
assertTrue(userInfoResponse.indicatesSuccess());
@@ -128,7 +127,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
when(mockHttpEntity.getContentType()).thenReturn(MediaType.APPLICATION_JSON_VALUE);
when(mockHttpEntity.getContent()).thenReturn(new ByteArrayInputStream(USERINFO_RESPONSE_JWS.getBytes()));
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNull(userInfoResponse);
}
@@ -143,7 +142,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
when(mockHttpEntity.getContentType()).thenReturn(ContentType.APPLICATION_JWT.getType());
when(mockHttpEntity.getContent()).thenReturn(new ByteArrayInputStream(USERINFO_RESPONSE.getBytes()));
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNull(userInfoResponse);
}
@@ -158,7 +157,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
when(mockHttpEntity.getContentType()).thenReturn("application/unknown");
when(mockHttpEntity.getContent()).thenReturn(new ByteArrayInputStream(USERINFO_RESPONSE_JWS.getBytes()));
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNull(userInfoResponse);
}
@@ -172,7 +171,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
when(mockHttpEntity.getContentType()).thenReturn(ContentType.APPLICATION_JWT.getType());
when(mockHttpEntity.getContent()).thenReturn(new ByteArrayInputStream(USERINFO_RESPONSE_JWS.getBytes()));
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNotNull(userInfoResponse);
assertTrue(userInfoResponse.indicatesSuccess());
assertEquals(userInfoResponse.toSuccessResponse().getEntityContentType(), ContentType.APPLICATION_JWT);
@@ -198,7 +197,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
when(response.getHeader(DefaultUserInfoResponseDecoder.USERINFO_ERROR_RESPONSE_HEADER))
.thenReturn(header);
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNotNull(userInfoResponse);
assertFalse(userInfoResponse.indicatesSuccess());
@@ -214,7 +213,7 @@ public class DefaultUserInfoResponseDecoderTest extends AbstractOIDCTest {
when(mockHttpEntity.getContentType()).thenReturn(ContentType.APPLICATION_JWT.getType());
when(mockHttpEntity.getContent()).thenReturn(new ByteArrayInputStream(USERINFO_RESPONSE_JWE.getBytes()));
- final UserInfoResponse userInfoResponse = decoder.apply(response);
+ final UserInfoResponse userInfoResponse = decoder.handleResponse(response);
assertNotNull(userInfoResponse);
assertTrue(userInfoResponse.indicatesSuccess());
assertEquals(userInfoResponse.toSuccessResponse().getEntityContentType(), ContentType.APPLICATION_JWT);
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractOIDCTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractOIDCTest.java
index 8e8dfcc..cee4bb2 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractOIDCTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/AbstractOIDCTest.java
@@ -154,6 +154,11 @@ public abstract class AbstractOIDCTest {
+ " \"error_description\":\"this request was bad\"\n"
+ " }";
+ @Nonnull @NotEmpty
+ protected final String USERINFO_RESPONSE_ERROR = "Bearer realm=\"example.com\",\n"
+ + " error=\"invalid_token\",\n"
+ + " error_description=\"The access token expired\"";
+
/** Mock a UserInfo JWE reponse.*/
@Nonnull @NotEmpty
protected final String USERINFO_RESPONSE_JWE = "eyJraWQiOiIzNGUzNWJiZS1iZDBiLTQyMzEtYWJlMC04Y2Z"
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExchangeCodeForAccessTokenTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExchangeCodeForAccessTokenTest.java
index 3d4e4ef..d9fafb6 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExchangeCodeForAccessTokenTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ExchangeCodeForAccessTokenTest.java
@@ -27,7 +27,7 @@ import org.apache.hc.client5.http.classic.HttpClient;
import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.ContentType;
-import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
import org.apache.hc.core5.http.protocol.HttpContext;
@@ -41,10 +41,8 @@ import org.springframework.webflow.execution.Event;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
-import com.fasterxml.jackson.databind.ObjectMapper;
import com.nimbusds.jose.util.StandardCharset;
import com.nimbusds.oauth2.sdk.TokenErrorResponse;
-import com.nimbusds.oauth2.sdk.TokenResponse;
import com.nimbusds.openid.connect.sdk.OIDCTokenResponse;
import net.shibboleth.idp.authn.context.AuthenticationContext;
@@ -87,8 +85,12 @@ public class ExchangeCodeForAccessTokenTest extends AbstractOIDCTest {
Mockito.when(httpResponse.getCode()).thenReturn(200);
Mockito.when(httpResponse.getEntity()).thenReturn(
new StringEntity(ACCESS_TOKEN_RESPONSE, ContentType.parse(MediaType.APPLICATION_JSON_VALUE)));
- Mockito.when(httpClient.executeOpen((HttpHost) Mockito.any(), (HttpUriRequest) Mockito.any(),
- (HttpContext) Mockito.any())).thenReturn(httpResponse);
+
+
+ final OIDCTokenResponse responseToken = OIDCTokenResponse.parse(convertHttpResponseToJSONObject(httpResponse));
+ Mockito.when(httpClient.execute((HttpUriRequest) Mockito.any(),
+ (HttpContext) Mockito.any(), (HttpClientResponseHandler<OIDCTokenResponse>)Mockito.any()))
+ .thenReturn(responseToken);
// create new client with mock response
exchangeAction.setHttpClient(httpClient);
@@ -109,9 +111,10 @@ public class ExchangeCodeForAccessTokenTest extends AbstractOIDCTest {
.setHeader("Content-Type", ContentType.APPLICATION_FORM_URLENCODED.getMimeType())
.setCharset(StandardCharset.UTF_8);
return rb.build();
- });
+ });
+
+ // Add a dummy response decoder, it is not used as the response is mocked.
exchangeAction.setHttpResponseDecoderStrategy(response -> {
- final ObjectMapper mapper = new ObjectMapper();
try {
return OIDCTokenResponse.parse(convertHttpResponseToJSONObject(httpResponse));
} catch (final Exception e) {
@@ -131,12 +134,12 @@ public class ExchangeCodeForAccessTokenTest extends AbstractOIDCTest {
public void testTokenExchange_WrongTokenType() throws Exception {
final HttpClient httpClient = Mockito.mock(HttpClient.class);
- final ClassicHttpResponse httpResponse = Mockito.mock(ClassicHttpResponse.class);
- Mockito.when(httpResponse.getCode()).thenReturn(200);
- Mockito.when(httpResponse.getEntity()).thenReturn(
- new StringEntity(ACCESS_TOKEN_RESPONSE_NO_IDTOKEN, ContentType.parse(MediaType.APPLICATION_JSON_VALUE)));
- Mockito.when(httpClient.executeOpen((HttpHost)Mockito.any(), (HttpUriRequest) Mockito.any(), (HttpContext) Mockito.any()))
- .thenReturn(httpResponse);
+
+ final OIDCTokenResponse responseToken = null;
+ Mockito.when(httpClient.execute((HttpUriRequest) Mockito.any(),
+ (HttpContext) Mockito.any(), (HttpClientResponseHandler<OIDCTokenResponse>)Mockito.any()))
+ .thenReturn(responseToken);
+
// create new client with mock response
exchangeAction.setHttpClient(httpClient);
@@ -159,12 +162,7 @@ public class ExchangeCodeForAccessTokenTest extends AbstractOIDCTest {
return rb.build();
});
exchangeAction.setHttpResponseDecoderStrategy(response -> {
- final ObjectMapper mapper = new ObjectMapper();
- try {
- return TokenResponse.parse(convertHttpResponseToJSONObject(httpResponse));
- } catch (final Exception e) {
- return null;
- }
+ return null;
});
exchangeAction.initialize();
@@ -184,8 +182,11 @@ public class ExchangeCodeForAccessTokenTest extends AbstractOIDCTest {
Mockito.when(httpResponse.getCode()).thenReturn(400);
Mockito.when(httpResponse.getEntity()).thenReturn(
new StringEntity(TOKEN_RESPONSE_ERROR, ContentType.parse(MediaType.APPLICATION_JSON_VALUE)));
- Mockito.when(httpClient.executeOpen((HttpHost) Mockito.any(), (HttpUriRequest) Mockito.any(), (
- HttpContext) Mockito.any())).thenReturn(httpResponse);
+
+ final TokenErrorResponse responseToken = TokenErrorResponse.parse(convertHttpResponseToJSONObject(httpResponse));
+ Mockito.when(httpClient.execute((HttpUriRequest) Mockito.any(),
+ (HttpContext) Mockito.any(), (HttpClientResponseHandler<TokenErrorResponse>)Mockito.any()))
+ .thenReturn(responseToken);
// create new client with mock response
exchangeAction.setHttpClient(httpClient);
@@ -225,44 +226,4 @@ public class ExchangeCodeForAccessTokenTest extends AbstractOIDCTest {
}
- @Test
- public void testNullTokenResponse() throws Exception {
-
- final HttpClient httpClient = Mockito.mock(HttpClient.class);
- final ClassicHttpResponse httpResponse = Mockito.mock(ClassicHttpResponse.class);
- Mockito.when(httpResponse.getCode()).thenReturn(200);
- Mockito.when(httpClient.executeOpen((HttpHost) Mockito.any(), (HttpUriRequest) Mockito.any(), (
- HttpContext) Mockito.any())).thenReturn(httpResponse);
- assert httpClient != null;
- // create new client with mock response
- exchangeAction.setHttpClient(httpClient);
-
- exchangeAction.setHttpRequestEncoderStrategy(prc -> {
- URI uri;
- try {
- uri = new URIBuilder().setScheme("https")
- .setHost("op.example.com")
- .setPath("/token")
- .build();
- } catch (final URISyntaxException e) {
- return null;
- }
-
- // Add headers and create request.
- final ClassicRequestBuilder rb = ClassicRequestBuilder.post().setUri(uri)
- .setHeader("Content-Type", ContentType.APPLICATION_FORM_URLENCODED.getMimeType())
- .setCharset(StandardCharset.UTF_8);
- return rb.build();
- });
- exchangeAction.setHttpResponseDecoderStrategy(response -> null);
-
- exchangeAction.initialize();
-
- final Event event = exchangeAction.execute(src);
- assert event != null;
- assertNotNull(event);
- assertEquals("AuthenticationException",event.getId());
-
- }
-
}
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoEndpointLookupTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoEndpointLookupTest.java
index 3ba4172..cf5bf30 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoEndpointLookupTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/UserInfoEndpointLookupTest.java
@@ -26,7 +26,7 @@ import java.net.URISyntaxException;
import org.apache.hc.client5.http.classic.HttpClient;
import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
-import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
import org.apache.hc.core5.http.protocol.HttpContext;
@@ -41,6 +41,7 @@ import org.testng.annotations.Test;
import com.nimbusds.common.contenttype.ContentType;
import com.nimbusds.jose.util.StandardCharset;
+import com.nimbusds.openid.connect.sdk.OIDCTokenResponse;
import com.nimbusds.openid.connect.sdk.UserInfoErrorResponse;
import com.nimbusds.openid.connect.sdk.UserInfoSuccessResponse;
import com.nimbusds.openid.connect.sdk.claims.UserInfo;
@@ -86,9 +87,13 @@ public class UserInfoEndpointLookupTest extends AbstractOIDCTest {
final ClassicHttpResponse httpResponse = Mockito.mock(ClassicHttpResponse.class);
Mockito.when(httpResponse.getCode()).thenReturn(200);
Mockito.when(httpResponse.getEntity()).thenReturn(new StringEntity(USERINFO_RESPONSE));
- Mockito.when(httpClient.executeOpen((HttpHost) Mockito.any(), (HttpUriRequest) Mockito.any(), (HttpContext) Mockito.any()))
- .thenReturn(httpResponse);
-
+
+ final UserInfoSuccessResponse responseToken = new
+ UserInfoSuccessResponse(new UserInfo(convertHttpResponseToJSONObject(httpResponse)));
+ Mockito.when(httpClient.execute((HttpUriRequest) Mockito.any(),
+ (HttpContext) Mockito.any(), (HttpClientResponseHandler<UserInfoSuccessResponse>)Mockito.any()))
+ .thenReturn(responseToken);
+
// create new client with mock response
action.setHttpClient(httpClient);
@@ -109,6 +114,7 @@ public class UserInfoEndpointLookupTest extends AbstractOIDCTest {
.setCharset(StandardCharset.UTF_8);
return rb.build();
});
+ // Not used, just needed to avoid null response decoder
action.setHttpResponseDecoderStrategy(response -> {
try {
return new UserInfoSuccessResponse(new UserInfo(convertHttpResponseToJSONObject(httpResponse)));
@@ -139,12 +145,11 @@ public class UserInfoEndpointLookupTest extends AbstractOIDCTest {
@Test
public void testUserInfoLookup_ErrorResponse() throws Exception {
- final HttpClient httpClient = Mockito.mock(HttpClient.class);
- final ClassicHttpResponse httpResponse = Mockito.mock(ClassicHttpResponse.class);
- Mockito.when(httpResponse.getCode()).thenReturn(401);
- Mockito.when(httpResponse.getEntity()).thenReturn(new StringEntity(TOKEN_RESPONSE_ERROR));
- Mockito.when(httpClient.executeOpen((HttpHost) Mockito.any(),(HttpUriRequest) Mockito.any(),
- (HttpContext) Mockito.any())).thenReturn(httpResponse);
+ final HttpClient httpClient = Mockito.mock(HttpClient.class);
+ final UserInfoErrorResponse responseToken = UserInfoErrorResponse.parse(USERINFO_RESPONSE_ERROR);
+ Mockito.when(httpClient.execute((HttpUriRequest) Mockito.any(),
+ (HttpContext) Mockito.any(), (HttpClientResponseHandler<UserInfoErrorResponse>)Mockito.any()))
+ .thenReturn(responseToken);
// create new client with mock response
action.setHttpClient(httpClient);
@@ -166,11 +171,10 @@ public class UserInfoEndpointLookupTest extends AbstractOIDCTest {
.setCharset(StandardCharset.UTF_8);
return rb.build();
});
+ // This is a just to avoid a null response decoder, is not used.
action.setHttpResponseDecoderStrategy(response -> {
try {
- return UserInfoErrorResponse.parse("Bearer realm=\"example.com\",\n"
- + " error=\"invalid_token\",\n"
- + " error_description=\"The access token expired\"");
+ return UserInfoErrorResponse.parse(USERINFO_RESPONSE_ERROR);
} catch (final Exception e) {
fail(e.getMessage());
return null;
@@ -190,10 +194,12 @@ public class UserInfoEndpointLookupTest extends AbstractOIDCTest {
public void testNullUserInfoResponse() throws Exception {
final HttpClient httpClient = Mockito.mock(HttpClient.class);
- final ClassicHttpResponse httpResponse = Mockito.mock(ClassicHttpResponse.class);
- Mockito.when(httpResponse.getCode()).thenReturn(200);
- Mockito.when(httpClient.executeOpen((HttpHost) Mockito.any(), (HttpUriRequest) Mockito.any(), (
- HttpContext) Mockito.any())).thenReturn(httpResponse);
+
+ final OIDCTokenResponse responseToken = null;
+ Mockito.when(httpClient.execute((HttpUriRequest) Mockito.any(),
+ (HttpContext) Mockito.any(), (HttpClientResponseHandler<OIDCTokenResponse>)Mockito.any()))
+ .thenReturn(responseToken);
+
// create new client with mock response
action.setHttpClient(httpClient);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list