From 98e155de5f6d2ad134a323332a48340b458b9bb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Fri, 20 Feb 2026 20:21:42 +0100 Subject: [PATCH 01/14] Initial --- cloudplatform/connectivity-oauth/pom.xml | 13 + .../connectivity/HttpClient5Factory.java | 188 ++++++++++++++ .../HttpClient5OAuth2TokenService.java | 237 ++++++++++++++++++ .../connectivity/OAuth2Service.java | 31 +-- ...h2ServiceBindingDestinationLoaderTest.java | 14 +- 5 files changed, 457 insertions(+), 26 deletions(-) create mode 100644 cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5Factory.java create mode 100644 cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java diff --git a/cloudplatform/connectivity-oauth/pom.xml b/cloudplatform/connectivity-oauth/pom.xml index 25fe786ed..44cc44e5c 100644 --- a/cloudplatform/connectivity-oauth/pom.xml +++ b/cloudplatform/connectivity-oauth/pom.xml @@ -48,6 +48,7 @@ com.sap.cloud.sdk.cloudplatform connectivity-apache-httpclient4 + test com.sap.cloud.sdk.cloudplatform @@ -112,6 +113,10 @@ com.github.ben-manes.caffeine caffeine + + org.json + json + org.apache.httpcomponents httpclient @@ -122,6 +127,14 @@ + + org.apache.httpcomponents.client5 + httpclient5 + + + org.apache.httpcomponents.core5 + httpcore5 + org.apache.commons commons-lang3 diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5Factory.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5Factory.java new file mode 100644 index 000000000..f2df78f25 --- /dev/null +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5Factory.java @@ -0,0 +1,188 @@ +/* + * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. + */ + +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.security.KeyStore; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; + +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; +import org.apache.hc.client5.http.io.HttpClientConnectionManager; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.sap.cloud.security.config.ClientCertificate; +import com.sap.cloud.security.config.ClientIdentity; +import com.sap.cloud.security.mtls.SSLContextFactory; + +/** + * Factory for creating Apache HttpClient 5 instances from {@link ClientIdentity}. + *

+ * This factory handles the creation of HttpClient 5 instances configured with the appropriate SSL context based on the + * provided client identity, supporting both client secret and client certificate authentication. + */ +class HttpClient5Factory +{ + private static final Logger LOGGER = LoggerFactory.getLogger(HttpClient5Factory.class); + private static final char[] NO_PASSWORD = "".toCharArray(); + + /** + * Creates a new {@link CloseableHttpClient} configured with the given {@link ClientIdentity}. + * + * @param identity + * The client identity containing credentials (either client secret or client certificate). + * @return A configured {@link CloseableHttpClient} instance. + * @throws HttpClient5FactoryException + * If there is an error creating the HTTP client. + */ + @Nonnull + static CloseableHttpClient create( @Nonnull final ClientIdentity identity ) + throws HttpClient5FactoryException + { + return create(identity, null); + } + + /** + * Creates a new {@link CloseableHttpClient} configured with the given {@link ClientIdentity} and optional + * {@link KeyStore}. + * + * @param identity + * The client identity containing credentials (either client secret or client certificate). + * @param keyStore + * Optional KeyStore to use for mTLS. If provided, this takes precedence over the identity's certificate. + * @return A configured {@link CloseableHttpClient} instance. + * @throws HttpClient5FactoryException + * If there is an error creating the HTTP client. + */ + @Nonnull + static CloseableHttpClient create( @Nonnull final ClientIdentity identity, @Nullable final KeyStore keyStore ) + throws HttpClient5FactoryException + { + try { + final HttpClientConnectionManager connectionManager = createConnectionManager(identity, keyStore); + return HttpClients.custom().useSystemProperties().setConnectionManager(connectionManager).build(); + } + catch( final GeneralSecurityException | IOException e ) { + throw new HttpClient5FactoryException("Failed to create HttpClient5 for ClientIdentity", e); + } + } + + @Nonnull + private static + HttpClientConnectionManager + createConnectionManager( @Nonnull final ClientIdentity identity, @Nullable final KeyStore keyStore ) + throws GeneralSecurityException, + IOException + { + final TlsSocketStrategy tlsSocketStrategy = createTlsSocketStrategy(identity, keyStore); + + return PoolingHttpClientConnectionManagerBuilder.create().setTlsSocketStrategy(tlsSocketStrategy).build(); + } + + @Nonnull + private static + TlsSocketStrategy + createTlsSocketStrategy( @Nonnull final ClientIdentity identity, @Nullable final KeyStore keyStore ) + throws GeneralSecurityException, + IOException + { + final SSLContext sslContext = createSSLContext(identity, keyStore); + return new DefaultClientTlsStrategy(sslContext); + } + + @Nonnull + private static + SSLContext + createSSLContext( @Nonnull final ClientIdentity identity, @Nullable final KeyStore keyStore ) + throws GeneralSecurityException, + IOException + { + // If a KeyStore is provided directly (e.g., for ZTIS), use it for mTLS + if( keyStore != null ) { + LOGGER.debug("Creating SSL context with provided KeyStore for identity: {}", identity.getId()); + return createSSLContextFromKeyStore(keyStore); + } + + // If the identity is certificate-based and has valid certificate data, use SSLContextFactory + if( identity.isCertificateBased() && hasCertificateData(identity) ) { + LOGGER.debug("Creating SSL context with client certificate for identity: {}", identity.getId()); + return SSLContextFactory.getInstance().create(identity); + } + + // For non-certificate-based identities or identities without certificate data, return a default SSL context + LOGGER.debug("Creating default SSL context for identity: {}", identity.getId()); + return SSLContext.getDefault(); + } + + /** + * Checks if the identity has valid certificate data that can be used by SSLContextFactory. This excludes identities + * that have a KeyStore directly (like ZtisClientIdentity) which should be handled separately. + */ + private static boolean hasCertificateData( @Nonnull final ClientIdentity identity ) + { + // ZtisClientIdentity has a KeyStore but no PEM certificate data - it should be handled separately + // by passing the KeyStore directly to HttpClient5Factory.create(identity, keyStore) + if( identity instanceof SecurityLibWorkarounds.ZtisClientIdentity ) { + return false; + } + + // Check for PEM string certificate data + if( identity instanceof ClientCertificate clientCertificate ) { + final String cert = clientCertificate.getCertificate(); + final String key = clientCertificate.getKey(); + if( cert != null && !cert.isBlank() && key != null && !key.isBlank() ) { + return true; + } + } + + // Check for pre-parsed certificate chain and private key + if( identity.getCertificateChain() != null && identity.getPrivateKey() != null ) { + return true; + } + + // Check if identity has certificate/key via the base interface methods + // This handles cases where the identity is not a ClientCertificate but still has certificate data + final String cert = identity.getCertificate(); + final String key = identity.getKey(); + if( cert != null && !cert.isBlank() && key != null && !key.isBlank() ) { + return true; + } + + return false; + } + + @Nonnull + private static SSLContext createSSLContextFromKeyStore( @Nonnull final KeyStore keyStore ) + throws GeneralSecurityException + { + final KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance("SunX509"); + keyManagerFactory.init(keyStore, NO_PASSWORD); + final SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(keyManagerFactory.getKeyManagers(), null, null); + return sslContext; + } + + /** + * Exception thrown when there is an error creating an HttpClient5 instance. + */ + static class HttpClient5FactoryException extends RuntimeException + { + private static final long serialVersionUID = 1L; + + HttpClient5FactoryException( final String message, final Throwable cause ) + { + super(message, cause); + } + } +} diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java new file mode 100644 index 000000000..c48373fde --- /dev/null +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -0,0 +1,237 @@ +/* + * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. + */ + +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.*; + +import java.io.IOException; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.AbstractMap; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import javax.annotation.Nonnull; + +import org.apache.hc.client5.http.entity.UrlEncodedFormEntity; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; +import org.apache.hc.core5.http.message.BasicNameValuePair; +import org.json.JSONObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.sap.cloud.security.client.DefaultTokenClientConfiguration; +import com.sap.cloud.security.servlet.MDCHelper; +import com.sap.cloud.security.xsuaa.Assertions; +import com.sap.cloud.security.xsuaa.client.AbstractOAuth2TokenService; +import com.sap.cloud.security.xsuaa.client.OAuth2ServiceException; +import com.sap.cloud.security.xsuaa.client.OAuth2TokenResponse; +import com.sap.cloud.security.xsuaa.http.HttpHeaders; +import com.sap.cloud.security.xsuaa.tokenflows.TokenCacheConfiguration; + +/** + * Implementation of {@link com.sap.cloud.security.xsuaa.client.OAuth2TokenService} using Apache HttpClient 5. + *

+ * This class provides OAuth2 token retrieval functionality using Apache HttpClient 5 instead of HttpClient 4. + */ +public class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService +{ + private static final Logger LOGGER = LoggerFactory.getLogger(HttpClient5OAuth2TokenService.class); + private static final String USER_AGENT_HEADER = "User-Agent"; + + private final CloseableHttpClient httpClient; + private final DefaultTokenClientConfiguration config = DefaultTokenClientConfiguration.getInstance(); + + /** + * Creates a new instance with the given HTTP client and default cache configuration. + * + * @param httpClient + * The Apache HttpClient 5 instance to use for HTTP requests. + */ + public HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpClient ) + { + this(httpClient, TokenCacheConfiguration.defaultConfiguration()); + } + + /** + * Creates a new instance with the given HTTP client and cache configuration. + * + * @param httpClient + * The Apache HttpClient 5 instance to use for HTTP requests. + * @param tokenCacheConfiguration + * The cache configuration to use. + */ + public HttpClient5OAuth2TokenService( + @Nonnull final CloseableHttpClient httpClient, + @Nonnull final TokenCacheConfiguration tokenCacheConfiguration ) + { + super(tokenCacheConfiguration); + Assertions.assertNotNull(httpClient, "http client is required"); + this.httpClient = httpClient; + } + + @Override + protected + OAuth2TokenResponse + requestAccessToken( final URI tokenUri, final HttpHeaders headers, final Map parameters ) + throws OAuth2ServiceException + { + Assertions.assertNotNull(tokenUri, "Token endpoint URI must not be null!"); + return convertToOAuth2TokenResponse( + executeRequest(tokenUri, headers, parameters, config.isRetryEnabled() ? config.getMaxRetryAttempts() : 0)); + } + + private String executeRequest( + final URI tokenUri, + final HttpHeaders headers, + final Map parameters, + final int attemptsLeft ) + throws OAuth2ServiceException + { + final ClassicHttpRequest httpPost = createHttpPost(tokenUri, createRequestHeaders(headers), parameters); + LOGGER + .debug( + "Requesting access token from url {} with headers {} and {} retries left", + tokenUri, + headers, + attemptsLeft); + try { + return httpClient.execute(httpPost, response -> { + final int statusCode = response.getCode(); + final String body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); + LOGGER.debug("Received statusCode {} from {}", statusCode, tokenUri); + if( HttpStatus.SC_OK == statusCode ) { + LOGGER.debug("Successfully retrieved access token from {} with params {}.", tokenUri, parameters); + return body; + } else if( attemptsLeft > 0 && config.getRetryStatusCodes().contains(statusCode) ) { + LOGGER.warn("Request failed with status {} but is retryable. Retrying...", statusCode); + pauseBeforeNextAttempt(config.getRetryDelayTime()); + return executeRequest(tokenUri, headers, parameters, attemptsLeft - 1); + } + throw OAuth2ServiceException + .builder("Error requesting access token!") + .withStatusCode(statusCode) + .withUri(tokenUri) + .withRequestHeaders(getHeadersAsStringArray(httpPost.getHeaders())) + .withResponseHeaders(getHeadersAsStringArray(response.getHeaders())) + .withResponseBody(body) + .build(); + }); + } + catch( final IOException e ) { + if( e instanceof final OAuth2ServiceException oAuth2Exception ) { + throw oAuth2Exception; + } else { + throw OAuth2ServiceException + .builder("Error requesting access token!") + .withUri(tokenUri) + .withRequestHeaders(getHeadersAsStringArray(httpPost.getHeaders())) + .withResponseBody(e.getMessage()) + .build(); + } + } + } + + private HttpHeaders createRequestHeaders( final HttpHeaders headers ) + { + final HttpHeaders requestHeaders = new HttpHeaders(); + headers.getHeaders().forEach(h -> requestHeaders.withHeader(h.getName(), h.getValue())); + requestHeaders.withHeader(MDCHelper.CORRELATION_HEADER, MDCHelper.getOrCreateCorrelationId()); + return requestHeaders; + } + + private void logRequest( final HttpHeaders headers, final Map parameters ) + { + LOGGER.debug("access token request {} - {}", headers, parameters.entrySet().stream().map(e -> { + if( e.getKey().contains(PASSWORD) + || e.getKey().contains(CLIENT_SECRET) + || e.getKey().contains(ASSERTION) ) { + return new AbstractMap.SimpleImmutableEntry<>(e.getKey(), "****"); + } + return e; + }).toList()); + } + + private + ClassicHttpRequest + createHttpPost( final URI uri, final HttpHeaders headers, final Map parameters ) + throws OAuth2ServiceException + { + final ClassicRequestBuilder requestBuilder = ClassicRequestBuilder.post(uri); + + headers.getHeaders().forEach(header -> requestBuilder.addHeader(header.getName(), header.getValue())); + + final List basicNameValuePairs = + parameters + .entrySet() + .stream() + .map(entry -> new BasicNameValuePair(entry.getKey(), entry.getValue())) + .toList(); + + requestBuilder.setEntity(new UrlEncodedFormEntity(basicNameValuePairs, StandardCharsets.UTF_8)); + requestBuilder.addHeader(USER_AGENT_HEADER, getUserAgent()); + + logRequest(headers, parameters); + return requestBuilder.build(); + } + + private String getUserAgent() + { + // Construct a user agent string similar to what HttpClientUtil provides + final String javaVersion = System.getProperty("java.version", "unknown"); + return "cloud-security-xsuaa-integration/HttpClient5 (Java/" + javaVersion + ")"; + } + + private OAuth2TokenResponse convertToOAuth2TokenResponse( final String responseBody ) + throws OAuth2ServiceException + { + final Map accessTokenMap = new JSONObject(responseBody).toMap(); + final String accessToken = getParameter(accessTokenMap, ACCESS_TOKEN); + final String refreshToken = getParameter(accessTokenMap, REFRESH_TOKEN); + final String expiresIn = getParameter(accessTokenMap, EXPIRES_IN); + final String tokenType = getParameter(accessTokenMap, TOKEN_TYPE); + return new OAuth2TokenResponse(accessToken, convertExpiresInToLong(expiresIn), refreshToken, tokenType); + } + + private Long convertExpiresInToLong( final String expiresIn ) + throws OAuth2ServiceException + { + try { + return Long.parseLong(expiresIn); + } + catch( final NumberFormatException e ) { + throw new OAuth2ServiceException( + String.format("Cannot convert expires_in from response (%s) to long", expiresIn)); + } + } + + private String getParameter( final Map accessTokenMap, final String key ) + { + return String.valueOf(accessTokenMap.get(key)); + } + + private static String[] getHeadersAsStringArray( final Header[] headers ) + { + return headers != null ? Arrays.stream(headers).map(Header::toString).toArray(String[]::new) : new String[0]; + } + + private void pauseBeforeNextAttempt( final long sleepTime ) + { + try { + LOGGER.info("Retry again in {} ms", sleepTime); + Thread.sleep(sleepTime); + } + catch( final InterruptedException e ) { + LOGGER.warn("Thread.sleep has been interrupted. Retry starts now."); + Thread.currentThread().interrupt(); + } + } +} diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java index ea3b29d8a..a6526b9c4 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java @@ -10,8 +10,6 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.apache.http.impl.client.CloseableHttpClient; - import com.auth0.jwt.interfaces.DecodedJWT; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; @@ -33,10 +31,8 @@ import com.sap.cloud.sdk.cloudplatform.tenant.Tenant; import com.sap.cloud.sdk.cloudplatform.tenant.TenantAccessor; import com.sap.cloud.sdk.cloudplatform.tenant.TenantWithSubdomain; -import com.sap.cloud.security.client.HttpClientFactory; import com.sap.cloud.security.config.ClientIdentity; import com.sap.cloud.security.token.Token; -import com.sap.cloud.security.xsuaa.client.DefaultOAuth2TokenService; import com.sap.cloud.security.xsuaa.client.OAuth2ServiceException; import com.sap.cloud.security.xsuaa.client.OAuth2TokenResponse; import com.sap.cloud.security.xsuaa.client.OAuth2TokenService; @@ -113,29 +109,14 @@ private OAuth2TokenService createTokenService( @Nonnull final CacheKey ignored ) tokenCacheParameters.getTokenExpirationDelta(), false); // disable cache statistics - if( !(identity instanceof ZtisClientIdentity) ) { - return new DefaultOAuth2TokenService(HttpClientFactory.create(identity), tokenCacheConfiguration); - } - - final DefaultHttpDestination destination = - DefaultHttpDestination - // Giving an empty URL here as a workaround - // If we were to give the token URL here we can't change the subdomain later - // But the subdomain represents the tenant in case of IAS, so we have to change the subdomain per-tenant - .builder("") - .name("oauth-destination-ztis-" + identity.getId().hashCode()) - .keyStore(((ZtisClientIdentity) identity).getKeyStore()) - .build(); - try { - return new DefaultOAuth2TokenService( - (CloseableHttpClient) HttpClientAccessor.getHttpClient(destination), + if( identity instanceof ZtisClientIdentity ztisIdentity ) { + // For ZTIS, use the KeyStore directly from the identity + return new HttpClient5OAuth2TokenService( + HttpClient5Factory.create(identity, ztisIdentity.getKeyStore()), tokenCacheConfiguration); } - catch( final ClassCastException e ) { - final String msg = - "For the X509_ATTESTED credential type the 'HttpClientAccessor' must return instances of 'CloseableHttpClient'"; - throw new DestinationAccessException(msg, e); - } + + return new HttpClient5OAuth2TokenService(HttpClient5Factory.create(identity), tokenCacheConfiguration); } @Nonnull diff --git a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java index c38f1ab81..f6670b307 100644 --- a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java +++ b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java @@ -240,7 +240,19 @@ void testInvalidCertificate() sut.tryGetDestination(OPTIONS_WITH_EMPTY_BINDING).map(HttpDestinationProperties::getHeaders); assertThat(result.isFailure()).isTrue(); - assertThat(result.getCause()).hasRootCauseExactlyInstanceOf(HttpClientException.class); + // The root cause can be either HttpClientException (when using DefaultOAuth2TokenService with HttpClient 4) + // or a security exception like CertificateException (when using HttpClient5OAuth2TokenService) + final Throwable rootCause = getRootCause(result.getCause()); + assertThat(rootCause).isInstanceOfAny(HttpClientException.class, java.security.GeneralSecurityException.class); + } + + private static Throwable getRootCause( Throwable throwable ) + { + Throwable cause = throwable; + while( cause.getCause() != null && cause.getCause() != cause ) { + cause = cause.getCause(); + } + return cause; } @Test From aaad195ab54f6beec463da0f4f3d07472e1f37f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Fri, 20 Feb 2026 20:35:05 +0100 Subject: [PATCH 02/14] Combine Jonas' AI solution --- .../connectivity/HttpClient5Factory.java | 188 ------------------ .../HttpClient5OAuth2TokenService.java | 188 ++++++++++++++---- .../connectivity/OAuth2Service.java | 15 +- 3 files changed, 154 insertions(+), 237 deletions(-) delete mode 100644 cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5Factory.java diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5Factory.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5Factory.java deleted file mode 100644 index f2df78f25..000000000 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5Factory.java +++ /dev/null @@ -1,188 +0,0 @@ -/* - * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. - */ - -package com.sap.cloud.sdk.cloudplatform.connectivity; - -import java.io.IOException; -import java.security.GeneralSecurityException; -import java.security.KeyStore; - -import javax.annotation.Nonnull; -import javax.annotation.Nullable; -import javax.net.ssl.KeyManagerFactory; -import javax.net.ssl.SSLContext; - -import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; -import org.apache.hc.client5.http.impl.classic.HttpClients; -import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; -import org.apache.hc.client5.http.io.HttpClientConnectionManager; -import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; -import org.apache.hc.client5.http.ssl.TlsSocketStrategy; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.sap.cloud.security.config.ClientCertificate; -import com.sap.cloud.security.config.ClientIdentity; -import com.sap.cloud.security.mtls.SSLContextFactory; - -/** - * Factory for creating Apache HttpClient 5 instances from {@link ClientIdentity}. - *

- * This factory handles the creation of HttpClient 5 instances configured with the appropriate SSL context based on the - * provided client identity, supporting both client secret and client certificate authentication. - */ -class HttpClient5Factory -{ - private static final Logger LOGGER = LoggerFactory.getLogger(HttpClient5Factory.class); - private static final char[] NO_PASSWORD = "".toCharArray(); - - /** - * Creates a new {@link CloseableHttpClient} configured with the given {@link ClientIdentity}. - * - * @param identity - * The client identity containing credentials (either client secret or client certificate). - * @return A configured {@link CloseableHttpClient} instance. - * @throws HttpClient5FactoryException - * If there is an error creating the HTTP client. - */ - @Nonnull - static CloseableHttpClient create( @Nonnull final ClientIdentity identity ) - throws HttpClient5FactoryException - { - return create(identity, null); - } - - /** - * Creates a new {@link CloseableHttpClient} configured with the given {@link ClientIdentity} and optional - * {@link KeyStore}. - * - * @param identity - * The client identity containing credentials (either client secret or client certificate). - * @param keyStore - * Optional KeyStore to use for mTLS. If provided, this takes precedence over the identity's certificate. - * @return A configured {@link CloseableHttpClient} instance. - * @throws HttpClient5FactoryException - * If there is an error creating the HTTP client. - */ - @Nonnull - static CloseableHttpClient create( @Nonnull final ClientIdentity identity, @Nullable final KeyStore keyStore ) - throws HttpClient5FactoryException - { - try { - final HttpClientConnectionManager connectionManager = createConnectionManager(identity, keyStore); - return HttpClients.custom().useSystemProperties().setConnectionManager(connectionManager).build(); - } - catch( final GeneralSecurityException | IOException e ) { - throw new HttpClient5FactoryException("Failed to create HttpClient5 for ClientIdentity", e); - } - } - - @Nonnull - private static - HttpClientConnectionManager - createConnectionManager( @Nonnull final ClientIdentity identity, @Nullable final KeyStore keyStore ) - throws GeneralSecurityException, - IOException - { - final TlsSocketStrategy tlsSocketStrategy = createTlsSocketStrategy(identity, keyStore); - - return PoolingHttpClientConnectionManagerBuilder.create().setTlsSocketStrategy(tlsSocketStrategy).build(); - } - - @Nonnull - private static - TlsSocketStrategy - createTlsSocketStrategy( @Nonnull final ClientIdentity identity, @Nullable final KeyStore keyStore ) - throws GeneralSecurityException, - IOException - { - final SSLContext sslContext = createSSLContext(identity, keyStore); - return new DefaultClientTlsStrategy(sslContext); - } - - @Nonnull - private static - SSLContext - createSSLContext( @Nonnull final ClientIdentity identity, @Nullable final KeyStore keyStore ) - throws GeneralSecurityException, - IOException - { - // If a KeyStore is provided directly (e.g., for ZTIS), use it for mTLS - if( keyStore != null ) { - LOGGER.debug("Creating SSL context with provided KeyStore for identity: {}", identity.getId()); - return createSSLContextFromKeyStore(keyStore); - } - - // If the identity is certificate-based and has valid certificate data, use SSLContextFactory - if( identity.isCertificateBased() && hasCertificateData(identity) ) { - LOGGER.debug("Creating SSL context with client certificate for identity: {}", identity.getId()); - return SSLContextFactory.getInstance().create(identity); - } - - // For non-certificate-based identities or identities without certificate data, return a default SSL context - LOGGER.debug("Creating default SSL context for identity: {}", identity.getId()); - return SSLContext.getDefault(); - } - - /** - * Checks if the identity has valid certificate data that can be used by SSLContextFactory. This excludes identities - * that have a KeyStore directly (like ZtisClientIdentity) which should be handled separately. - */ - private static boolean hasCertificateData( @Nonnull final ClientIdentity identity ) - { - // ZtisClientIdentity has a KeyStore but no PEM certificate data - it should be handled separately - // by passing the KeyStore directly to HttpClient5Factory.create(identity, keyStore) - if( identity instanceof SecurityLibWorkarounds.ZtisClientIdentity ) { - return false; - } - - // Check for PEM string certificate data - if( identity instanceof ClientCertificate clientCertificate ) { - final String cert = clientCertificate.getCertificate(); - final String key = clientCertificate.getKey(); - if( cert != null && !cert.isBlank() && key != null && !key.isBlank() ) { - return true; - } - } - - // Check for pre-parsed certificate chain and private key - if( identity.getCertificateChain() != null && identity.getPrivateKey() != null ) { - return true; - } - - // Check if identity has certificate/key via the base interface methods - // This handles cases where the identity is not a ClientCertificate but still has certificate data - final String cert = identity.getCertificate(); - final String key = identity.getKey(); - if( cert != null && !cert.isBlank() && key != null && !key.isBlank() ) { - return true; - } - - return false; - } - - @Nonnull - private static SSLContext createSSLContextFromKeyStore( @Nonnull final KeyStore keyStore ) - throws GeneralSecurityException - { - final KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance("SunX509"); - keyManagerFactory.init(keyStore, NO_PASSWORD); - final SSLContext sslContext = SSLContext.getInstance("TLS"); - sslContext.init(keyManagerFactory.getKeyManagers(), null, null); - return sslContext; - } - - /** - * Exception thrown when there is an error creating an HttpClient5 instance. - */ - static class HttpClient5FactoryException extends RuntimeException - { - private static final long serialVersionUID = 1L; - - HttpClient5FactoryException( final String message, final Throwable cause ) - { - super(message, cause); - } - } -} diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index c48373fde..a6e7141c2 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -9,26 +9,35 @@ import java.io.IOException; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.security.KeyStore; import java.util.AbstractMap; import java.util.Arrays; import java.util.List; import java.util.Map; import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import javax.net.ssl.SSLContext; +import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.entity.UrlEncodedFormEntity; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; -import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.io.entity.EntityUtils; -import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; import org.apache.hc.core5.http.message.BasicNameValuePair; +import org.apache.hc.core5.ssl.SSLContextBuilder; import org.json.JSONObject; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.sap.cloud.security.client.DefaultTokenClientConfiguration; +import com.sap.cloud.security.client.HttpClientException; +import com.sap.cloud.security.config.ClientCertificate; +import com.sap.cloud.security.config.ClientIdentity; +import com.sap.cloud.security.mtls.SSLContextFactory; import com.sap.cloud.security.servlet.MDCHelper; import com.sap.cloud.security.xsuaa.Assertions; import com.sap.cloud.security.xsuaa.client.AbstractOAuth2TokenService; @@ -36,16 +45,20 @@ import com.sap.cloud.security.xsuaa.client.OAuth2TokenResponse; import com.sap.cloud.security.xsuaa.http.HttpHeaders; import com.sap.cloud.security.xsuaa.tokenflows.TokenCacheConfiguration; +import com.sap.cloud.security.xsuaa.util.HttpClientUtil; + +import lombok.extern.slf4j.Slf4j; /** - * Implementation of {@link com.sap.cloud.security.xsuaa.client.OAuth2TokenService} using Apache HttpClient 5. + * OAuth2 token service implementation using Apache HttpClient 5. *

- * This class provides OAuth2 token retrieval functionality using Apache HttpClient 5 instead of HttpClient 4. + * This class extends {@link AbstractOAuth2TokenService} and provides the HTTP client specific logic to perform token + * requests using Apache HttpClient 5 instead of HttpClient 4. */ -public class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService +@Slf4j +class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService { - private static final Logger LOGGER = LoggerFactory.getLogger(HttpClient5OAuth2TokenService.class); - private static final String USER_AGENT_HEADER = "User-Agent"; + private static final char[] EMPTY_PASSWORD = {}; private final CloseableHttpClient httpClient; private final DefaultTokenClientConfiguration config = DefaultTokenClientConfiguration.getInstance(); @@ -54,9 +67,9 @@ public class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService * Creates a new instance with the given HTTP client and default cache configuration. * * @param httpClient - * The Apache HttpClient 5 instance to use for HTTP requests. + * The HTTP client to use for token requests. */ - public HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpClient ) + HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpClient ) { this(httpClient, TokenCacheConfiguration.defaultConfiguration()); } @@ -65,11 +78,11 @@ public HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpCli * Creates a new instance with the given HTTP client and cache configuration. * * @param httpClient - * The Apache HttpClient 5 instance to use for HTTP requests. + * The HTTP client to use for token requests. * @param tokenCacheConfiguration * The cache configuration to use. */ - public HttpClient5OAuth2TokenService( + HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpClient, @Nonnull final TokenCacheConfiguration tokenCacheConfiguration ) { @@ -96,8 +109,8 @@ private String executeRequest( final int attemptsLeft ) throws OAuth2ServiceException { - final ClassicHttpRequest httpPost = createHttpPost(tokenUri, createRequestHeaders(headers), parameters); - LOGGER + final HttpPost httpPost = createHttpPost(tokenUri, createRequestHeaders(headers), parameters); + log .debug( "Requesting access token from url {} with headers {} and {} retries left", tokenUri, @@ -107,12 +120,12 @@ private String executeRequest( return httpClient.execute(httpPost, response -> { final int statusCode = response.getCode(); final String body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - LOGGER.debug("Received statusCode {} from {}", statusCode, tokenUri); + log.debug("Received statusCode {} from {}", statusCode, tokenUri); if( HttpStatus.SC_OK == statusCode ) { - LOGGER.debug("Successfully retrieved access token from {} with params {}.", tokenUri, parameters); + log.debug("Successfully retrieved access token from {} with params {}.", tokenUri, parameters); return body; } else if( attemptsLeft > 0 && config.getRetryStatusCodes().contains(statusCode) ) { - LOGGER.warn("Request failed with status {} but is retryable. Retrying...", statusCode); + log.warn("Request failed with status {} but is retryable. Retrying...", statusCode); pauseBeforeNextAttempt(config.getRetryDelayTime()); return executeRequest(tokenUri, headers, parameters, attemptsLeft - 1); } @@ -150,7 +163,7 @@ private HttpHeaders createRequestHeaders( final HttpHeaders headers ) private void logRequest( final HttpHeaders headers, final Map parameters ) { - LOGGER.debug("access token request {} - {}", headers, parameters.entrySet().stream().map(e -> { + log.debug("access token request {} - {}", headers, parameters.entrySet().stream().map(e -> { if( e.getKey().contains(PASSWORD) || e.getKey().contains(CLIENT_SECRET) || e.getKey().contains(ASSERTION) ) { @@ -160,34 +173,26 @@ private void logRequest( final HttpHeaders headers, final Map pa }).toList()); } - private - ClassicHttpRequest - createHttpPost( final URI uri, final HttpHeaders headers, final Map parameters ) - throws OAuth2ServiceException + private HttpPost createHttpPost( final URI uri, final HttpHeaders headers, final Map parameters ) + throws OAuth2ServiceException { - final ClassicRequestBuilder requestBuilder = ClassicRequestBuilder.post(uri); - - headers.getHeaders().forEach(header -> requestBuilder.addHeader(header.getName(), header.getValue())); - + final HttpPost httpPost = new HttpPost(uri); + headers.getHeaders().forEach(header -> httpPost.setHeader(header.getName(), header.getValue())); final List basicNameValuePairs = parameters .entrySet() .stream() .map(entry -> new BasicNameValuePair(entry.getKey(), entry.getValue())) .toList(); - - requestBuilder.setEntity(new UrlEncodedFormEntity(basicNameValuePairs, StandardCharsets.UTF_8)); - requestBuilder.addHeader(USER_AGENT_HEADER, getUserAgent()); - + try { + httpPost.setEntity(new UrlEncodedFormEntity(basicNameValuePairs, StandardCharsets.UTF_8)); + httpPost.addHeader(org.apache.hc.core5.http.HttpHeaders.USER_AGENT, HttpClientUtil.getUserAgent()); + } + catch( final Exception e ) { + throw new OAuth2ServiceException("Unexpected error parsing URI: " + e.getMessage()); + } logRequest(headers, parameters); - return requestBuilder.build(); - } - - private String getUserAgent() - { - // Construct a user agent string similar to what HttpClientUtil provides - final String javaVersion = System.getProperty("java.version", "unknown"); - return "cloud-security-xsuaa-integration/HttpClient5 (Java/" + javaVersion + ")"; + return httpPost; } private OAuth2TokenResponse convertToOAuth2TokenResponse( final String responseBody ) @@ -226,12 +231,111 @@ private static String[] getHeadersAsStringArray( final Header[] headers ) private void pauseBeforeNextAttempt( final long sleepTime ) { try { - LOGGER.info("Retry again in {} ms", sleepTime); + log.info("Retry again in {} ms", sleepTime); Thread.sleep(sleepTime); } catch( final InterruptedException e ) { - LOGGER.warn("Thread.sleep has been interrupted. Retry starts now."); - Thread.currentThread().interrupt(); + log.warn("Thread.sleep has been interrupted. Retry starts now."); + } + } + + /** + * Creates a CloseableHttpClient (HttpClient5) based on ClientIdentity details. + *

+ * For ClientIdentity that is certificate based it will resolve HTTPS client using the provided ClientIdentity. If + * the ClientIdentity wasn't provided or is not certificate-based, it will return default HttpClient. + * + * @param clientIdentity + * for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity + * interface should be provided + * @return HTTP or HTTPS client (HttpClient5) + * @throws HttpClientException + * in case HTTPS Client could not be setup + */ + @Nonnull + static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clientIdentity ) + throws HttpClientException + { + return createHttpClient(clientIdentity, null); + } + + /** + * Creates a CloseableHttpClient (HttpClient5) based on ClientIdentity details and optional KeyStore. + *

+ * For ClientIdentity that is certificate based it will resolve HTTPS client using the provided ClientIdentity. If a + * KeyStore is provided (e.g., for ZTIS), it will be used directly. If the ClientIdentity wasn't provided or is not + * certificate-based, it will return default HttpClient. + * + * @param clientIdentity + * for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity + * interface should be provided + * @param keyStore + * optional KeyStore to use for mTLS (e.g., for ZTIS) + * @return HTTP or HTTPS client (HttpClient5) + * @throws HttpClientException + * in case HTTPS Client could not be setup + */ + @Nonnull + static + CloseableHttpClient + createHttpClient( @Nullable final ClientIdentity clientIdentity, @Nullable final KeyStore keyStore ) + throws HttpClientException + { + // If a KeyStore is provided directly (e.g., for ZTIS), use it + if( keyStore != null ) { + log + .debug( + "Creating HTTPS HttpClient5 with provided KeyStore for client '{}'", + clientIdentity != null ? clientIdentity.getId() : "unknown"); + return createHttpClientWithKeyStore(keyStore); + } + + if( clientIdentity == null ) { + log.debug("No ClientIdentity provided, creating default HttpClient5"); + return createDefaultHttpClient(); + } + + if( !clientIdentity.isCertificateBased() ) { + log.debug("ClientIdentity is not certificate-based, creating default HttpClient5"); + return createDefaultHttpClient(); + } + + log + .debug( + "Creating HTTPS HttpClient5 with certificate-based authentication for client '{}'", + clientIdentity.getId()); + + try { + final KeyStore identityKeyStore = SSLContextFactory.getInstance().createKeyStore(clientIdentity); + return createHttpClientWithKeyStore(identityKeyStore); + } + catch( final Exception e ) { + throw new HttpClientException( + "Failed to create HTTPS HttpClient5 with certificate authentication: " + e.getMessage()); + } + } + + @Nonnull + private static CloseableHttpClient createDefaultHttpClient() + { + return HttpClients.custom().useSystemProperties().build(); + } + + @Nonnull + private static CloseableHttpClient createHttpClientWithKeyStore( @Nonnull final KeyStore keyStore ) + throws HttpClientException + { + try { + final SSLContext sslContext = SSLContextBuilder.create().loadKeyMaterial(keyStore, EMPTY_PASSWORD).build(); + + final var tlsStrategy = new DefaultClientTlsStrategy(sslContext); + final var connectionManager = + PoolingHttpClientConnectionManagerBuilder.create().setTlsSocketStrategy(tlsStrategy).build(); + + return HttpClientBuilder.create().useSystemProperties().setConnectionManager(connectionManager).build(); + } + catch( final Exception e ) { + throw new HttpClientException("Failed to create HTTPS HttpClient5 with KeyStore: " + e.getMessage()); } } } diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java index a6526b9c4..7dec1c128 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2Service.java @@ -10,6 +10,8 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; + import com.auth0.jwt.interfaces.DecodedJWT; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; @@ -109,14 +111,13 @@ private OAuth2TokenService createTokenService( @Nonnull final CacheKey ignored ) tokenCacheParameters.getTokenExpirationDelta(), false); // disable cache statistics - if( identity instanceof ZtisClientIdentity ztisIdentity ) { - // For ZTIS, use the KeyStore directly from the identity - return new HttpClient5OAuth2TokenService( - HttpClient5Factory.create(identity, ztisIdentity.getKeyStore()), - tokenCacheConfiguration); - } + // For ZTIS, use the KeyStore directly from the identity + final CloseableHttpClient httpClient = + identity instanceof final ZtisClientIdentity ztisIdentity + ? HttpClient5OAuth2TokenService.createHttpClient(identity, ztisIdentity.getKeyStore()) + : HttpClient5OAuth2TokenService.createHttpClient(identity); - return new HttpClient5OAuth2TokenService(HttpClient5Factory.create(identity), tokenCacheConfiguration); + return new HttpClient5OAuth2TokenService(httpClient, tokenCacheConfiguration); } @Nonnull From 5a7630ae88e462eaf97b090886d9ceee95151ddb Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Tue, 24 Feb 2026 14:40:58 +0100 Subject: [PATCH 03/14] first tests --- .../HttpClient5OAuth2TokenServiceTest.java | 314 ++++++++++++++++++ 1 file changed, 314 insertions(+) create mode 100644 cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java diff --git a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java new file mode 100644 index 000000000..c35cfc206 --- /dev/null +++ b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java @@ -0,0 +1,314 @@ +/* + * Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. + */ + +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.cert.CertificateException; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.parallel.Isolated; + +import com.sap.cloud.sdk.cloudplatform.connectivity.SecurityLibWorkarounds.ZtisClientIdentity; +import com.sap.cloud.sdk.testutil.TestContext; +import com.sap.cloud.security.client.HttpClientException; +import com.sap.cloud.security.config.ClientCertificate; +import com.sap.cloud.security.config.ClientCredentials; +import com.sap.cloud.security.config.ClientIdentity; + +import lombok.SneakyThrows; + +/** + * Unit tests for the HTTP client creation methods in {@link HttpClient5OAuth2TokenService}. + *

+ * These tests verify the different scenarios for creating HTTP clients based on various ClientIdentity types and + * KeyStore configurations, including proper handling of null inputs and error conditions. + */ +@Isolated( "Tests HTTP client creation which may have global implications" ) +class HttpClient5OAuth2TokenServiceTest +{ + @RegisterExtension + static TestContext context = TestContext.withThreadContext(); + + @Test + @DisplayName( "createHttpClient with null ClientIdentity should return default HTTP client" ) + void testCreateHttpClientWithNullClientIdentity() + throws HttpClientException + { + final CloseableHttpClient result = HttpClient5OAuth2TokenService.createHttpClient(null); + + assertThat(result).isNotNull(); + assertThat(result).isInstanceOf(CloseableHttpClient.class); + } + + @Test + @DisplayName( "createHttpClient with null ClientIdentity and null KeyStore should return default HTTP client" ) + void testCreateHttpClientWithNullClientIdentityAndNullKeyStore() + throws HttpClientException + { + final CloseableHttpClient result = HttpClient5OAuth2TokenService.createHttpClient(null, null); + + assertThat(result).isNotNull(); + assertThat(result).isInstanceOf(CloseableHttpClient.class); + } + + @Test + @DisplayName( "createHttpClient with non-certificate-based ClientIdentity should return default HTTP client" ) + void testCreateHttpClientWithNonCertificateBasedClientIdentity() + throws HttpClientException + { + final ClientIdentity clientCredentials = new ClientCredentials("client-id", "client-secret"); + + final CloseableHttpClient result = HttpClient5OAuth2TokenService.createHttpClient(clientCredentials); + + assertThat(result).isNotNull(); + assertThat(result).isInstanceOf(CloseableHttpClient.class); + } + + @Test + @DisplayName( "createHttpClient with non-certificate-based ClientIdentity and null KeyStore should return default HTTP client" ) + void testCreateHttpClientWithNonCertificateBasedClientIdentityAndNullKeyStore() + throws HttpClientException + { + final ClientIdentity clientCredentials = new ClientCredentials("client-id", "client-secret"); + + final CloseableHttpClient result = HttpClient5OAuth2TokenService.createHttpClient(clientCredentials, null); + + assertThat(result).isNotNull(); + assertThat(result).isInstanceOf(CloseableHttpClient.class); + } + + @Test + @DisplayName( "createHttpClient with provided KeyStore should use KeyStore regardless of ClientIdentity" ) + @SneakyThrows + void testCreateHttpClientWithProvidedKeyStore() + { + final KeyStore keyStore = createEmptyKeyStore(); + final ClientIdentity clientCredentials = new ClientCredentials("client-id", "client-secret"); + + final CloseableHttpClient result = HttpClient5OAuth2TokenService.createHttpClient(clientCredentials, keyStore); + + assertThat(result).isNotNull(); + assertThat(result).isInstanceOf(CloseableHttpClient.class); + } + + @Test + @DisplayName( "createHttpClient with only KeyStore provided should use KeyStore" ) + @SneakyThrows + void testCreateHttpClientWithOnlyKeyStore() + { + final KeyStore keyStore = createEmptyKeyStore(); + + final CloseableHttpClient result = HttpClient5OAuth2TokenService.createHttpClient(null, keyStore); + + assertThat(result).isNotNull(); + assertThat(result).isInstanceOf(CloseableHttpClient.class); + } + + @Test + @DisplayName( "createHttpClient with certificate-based ClientIdentity should handle invalid certificates gracefully" ) + void testCreateHttpClientWithCertificateBasedClientIdentity() + { + final ClientIdentity certificateIdentity = createMockCertificateBasedIdentity(); + + // These should fail because of invalid certificate format + assertThatThrownBy(() -> HttpClient5OAuth2TokenService.createHttpClient(certificateIdentity)) + .isInstanceOf(HttpClientException.class) + .hasMessageContaining("Failed to create HTTPS HttpClient5 with certificate authentication"); + + assertThatThrownBy(() -> HttpClient5OAuth2TokenService.createHttpClient(certificateIdentity, null)) + .isInstanceOf(HttpClientException.class) + .hasMessageContaining("Failed to create HTTPS HttpClient5 with certificate authentication"); + } + + @Test + @DisplayName( "createHttpClient with ZTIS ClientIdentity should handle certificate validation" ) + @SneakyThrows + void testCreateHttpClientWithZtisClientIdentity() + { + final KeyStore keyStore = createEmptyKeyStore(); + final ClientIdentity ztisIdentity = new ZtisClientIdentity("ztis-client-id", keyStore); + + // ZtisClientIdentity is certificate-based but doesn't implement certificate methods properly + // This should fail with certificate validation error + assertThatThrownBy(() -> HttpClient5OAuth2TokenService.createHttpClient(ztisIdentity)) + .isInstanceOf(HttpClientException.class) + .hasMessageContaining("Failed to create HTTPS HttpClient5 with certificate authentication"); + } + + @Test + @DisplayName( "createHttpClient with ZTIS ClientIdentity and explicit KeyStore should prefer explicit KeyStore" ) + @SneakyThrows + void testCreateHttpClientWithZtisClientIdentityAndExplicitKeyStore() + { + final KeyStore embeddedKeyStore = createEmptyKeyStore(); + final KeyStore explicitKeyStore = createEmptyKeyStore(); + final ClientIdentity ztisIdentity = new ZtisClientIdentity("ztis-client-id", embeddedKeyStore); + + final CloseableHttpClient result = + HttpClient5OAuth2TokenService.createHttpClient(ztisIdentity, explicitKeyStore); + + assertThat(result).isNotNull(); + assertThat(result).isInstanceOf(CloseableHttpClient.class); + } + + @Test + @DisplayName( "createHttpClient should handle certificate creation failures gracefully" ) + void testCreateHttpClientWithInvalidCertificateIdentity() + { + final ClientIdentity invalidCertificateIdentity = createInvalidCertificateBasedIdentity(); + + assertThatThrownBy(() -> HttpClient5OAuth2TokenService.createHttpClient(invalidCertificateIdentity)) + .isInstanceOf(HttpClientException.class) + .hasMessageContaining("Failed to create HTTPS HttpClient5 with certificate authentication"); + } + + @Test + @DisplayName( "createHttpClient with invalid KeyStore should throw HttpClientException" ) + void testCreateHttpClientWithInvalidKeyStore() + { + final KeyStore invalidKeyStore = createInvalidKeyStore(); + final ClientIdentity clientCredentials = new ClientCredentials("client-id", "client-secret"); + + assertThatThrownBy(() -> HttpClient5OAuth2TokenService.createHttpClient(clientCredentials, invalidKeyStore)) + .isInstanceOf(HttpClientException.class) + .hasMessageContaining("Failed to create HTTPS HttpClient5 with KeyStore"); + } + + @Test + @DisplayName( "Multiple calls to createHttpClient should return different instances" ) + void testCreateHttpClientReturnsNewInstances() + throws HttpClientException + { + final CloseableHttpClient client1 = HttpClient5OAuth2TokenService.createHttpClient(null); + final CloseableHttpClient client2 = HttpClient5OAuth2TokenService.createHttpClient(null); + + assertThat(client1).isNotNull(); + assertThat(client2).isNotNull(); + assertThat(client1).isNotSameAs(client2); + } + + @Test + @DisplayName( "createHttpClient with different ClientIdentity types should handle appropriately" ) + void testCreateHttpClientWithDifferentIdentityTypes() + throws HttpClientException + { + final ClientIdentity credentials = new ClientCredentials("client-id", "client-secret"); + + final CloseableHttpClient credentialsClient = HttpClient5OAuth2TokenService.createHttpClient(credentials); + + assertThat(credentialsClient).isNotNull(); + assertThat(credentialsClient).isInstanceOf(CloseableHttpClient.class); + + // Test that certificate-based identity throws exception due to invalid certificate + final ClientIdentity certificate = createMockCertificateBasedIdentity(); + assertThatThrownBy(() -> HttpClient5OAuth2TokenService.createHttpClient(certificate)) + .isInstanceOf(HttpClientException.class) + .hasMessageContaining("Failed to create HTTPS HttpClient5 with certificate authentication"); + } + + @Test + @DisplayName( "createHttpClient should handle concurrent access safely" ) + void testCreateHttpClientConcurrentAccess() + throws InterruptedException + { + final int threadCount = 10; + final Thread[] threads = new Thread[threadCount]; + final CloseableHttpClient[] results = new CloseableHttpClient[threadCount]; + final Exception[] exceptions = new Exception[threadCount]; + + for( int i = 0; i < threadCount; i++ ) { + final int index = i; + threads[i] = new Thread(() -> { + try { + results[index] = HttpClient5OAuth2TokenService.createHttpClient(null); + } + catch( final Exception e ) { + exceptions[index] = e; + } + }); + } + + // Start all threads + for( final Thread thread : threads ) { + thread.start(); + } + + // Wait for all threads to complete + for( final Thread thread : threads ) { + thread.join(); + } + + // Verify results + for( int i = 0; i < threadCount; i++ ) { + assertThat(exceptions[i]).isNull(); + assertThat(results[i]).isNotNull(); + } + } + + // Helper methods for creating test objects + + @Nonnull + private static KeyStore createEmptyKeyStore() + throws KeyStoreException, + CertificateException, + IOException, + NoSuchAlgorithmException + { + final KeyStore keyStore = KeyStore.getInstance("JKS"); + keyStore.load(null, null); + return keyStore; + } + + @Nonnull + private static ClientIdentity createMockCertificateBasedIdentity() + { + final ClientCertificate mockCertificate = mock(ClientCertificate.class); + when(mockCertificate.isCertificateBased()).thenReturn(true); + when(mockCertificate.getId()).thenReturn("certificate-client-id"); + + // Mock the certificate methods with valid string values + when(mockCertificate.getCertificate()) + .thenReturn("-----BEGIN CERTIFICATE-----\nMIIBIjANBgkqhkiG9w0BAQEF...\n-----END CERTIFICATE-----"); + when(mockCertificate.getKey()) + .thenReturn("-----BEGIN PRIVATE KEY-----\nMIIEvgIBADANBgkqhkiG9w0BAQEF...\n-----END PRIVATE KEY-----"); + + return mockCertificate; + } + + @Nonnull + private static ClientIdentity createInvalidCertificateBasedIdentity() + { + final ClientCertificate mockCertificate = mock(ClientCertificate.class); + when(mockCertificate.isCertificateBased()).thenReturn(true); + when(mockCertificate.getId()).thenReturn("invalid-certificate-client-id"); + + // Return null for certificate methods to trigger the validation error + when(mockCertificate.getCertificate()).thenReturn(null); + when(mockCertificate.getCertificateChain()).thenReturn(null); + + return mockCertificate; + } + + @Nullable + private static KeyStore createInvalidKeyStore() + { + // Return a mock KeyStore that will cause SSL context creation to fail + return mock(KeyStore.class); + } +} From bb0e49778d81d1cd5bbfe3341d0d312e72585842 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 12:58:43 +0100 Subject: [PATCH 04/14] tests for HttpClient5OAuth2TokenService.requestAccessToken --- .../HttpClient5OAuth2TokenServiceTest.java | 412 +++++++++++++++++- 1 file changed, 401 insertions(+), 11 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java index c35cfc206..d6db91843 100644 --- a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java +++ b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java @@ -6,19 +6,36 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateException; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.time.temporal.TemporalUnit; +import java.util.HashMap; +import java.util.Map; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.io.HttpClientResponseHandler; +import org.apache.hc.core5.http.io.entity.StringEntity; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -30,6 +47,9 @@ import com.sap.cloud.security.config.ClientCertificate; import com.sap.cloud.security.config.ClientCredentials; import com.sap.cloud.security.config.ClientIdentity; +import com.sap.cloud.security.xsuaa.client.OAuth2ServiceException; +import com.sap.cloud.security.xsuaa.client.OAuth2TokenResponse; +import com.sap.cloud.security.xsuaa.http.HttpHeaders; import lombok.SneakyThrows; @@ -56,17 +76,6 @@ void testCreateHttpClientWithNullClientIdentity() assertThat(result).isInstanceOf(CloseableHttpClient.class); } - @Test - @DisplayName( "createHttpClient with null ClientIdentity and null KeyStore should return default HTTP client" ) - void testCreateHttpClientWithNullClientIdentityAndNullKeyStore() - throws HttpClientException - { - final CloseableHttpClient result = HttpClient5OAuth2TokenService.createHttpClient(null, null); - - assertThat(result).isNotNull(); - assertThat(result).isInstanceOf(CloseableHttpClient.class); - } - @Test @DisplayName( "createHttpClient with non-certificate-based ClientIdentity should return default HTTP client" ) void testCreateHttpClientWithNonCertificateBasedClientIdentity() @@ -261,6 +270,387 @@ void testCreateHttpClientConcurrentAccess() } } + // Tests for requestAccessToken method + + private CloseableHttpClient mockHttpClient; + private ClassicHttpResponse mockResponse; + private HttpClient5OAuth2TokenService tokenService; + + @BeforeEach + void setUp() + { + mockHttpClient = mock(CloseableHttpClient.class); + mockResponse = mock(ClassicHttpResponse.class); + tokenService = new HttpClient5OAuth2TokenService(mockHttpClient); + } + + @Test + @DisplayName( "requestAccessToken should successfully retrieve access token with valid response" ) + void testRequestAccessTokenSuccess() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + headers.withHeader("Content-Type", "application/x-www-form-urlencoded"); + + final Map parameters = new HashMap<>(); + parameters.put("grant_type", "client_credentials"); + parameters.put("client_id", "test-client"); + parameters.put("client_secret", "test-secret"); + + final String responseBody = """ + { + "access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9", + "token_type": "Bearer", + "expires_in": 3600, + "refresh_token": "refresh-token-value" + } + """; + + when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); + when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); + + // When + final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); + + // Then + assertThat(result).isNotNull(); + assertThat(result.getAccessToken()).isEqualTo("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9"); + assertThat(result.getTokenType()).isEqualTo("Bearer"); + assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)).isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); + assertThat(result.getRefreshToken()).isEqualTo("refresh-token-value"); + + // Verify HTTP client was called with correct request + verify(mockHttpClient).execute(argThat(httpPost -> { + try { + return httpPost instanceof HttpPost + && httpPost.getUri().equals(tokenUri) + && httpPost.getFirstHeader("Content-Type").getValue().equals("application/x-www-form-urlencoded"); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + }), any(HttpClientResponseHandler.class)); + } + + @Test + @DisplayName( "requestAccessToken should handle minimal valid response" ) + void testRequestAccessTokenMinimalResponse() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + final Map parameters = Map.of("grant_type", "client_credentials"); + + final String responseBody = """ + { + "access_token": "minimal-token", + "token_type": "Bearer", + "expires_in": 3600 + } + """; + + when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); + when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); + + // When + final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); + + // Then + assertThat(result).isNotNull(); + assertThat(result.getAccessToken()).isEqualTo("minimal-token"); + assertThat(result.getTokenType()).isEqualTo("Bearer");assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)).isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); + assertThat(result.getRefreshToken()).isEqualTo("null"); // Should be "null" as string when not present + } + + @Test + @DisplayName( "requestAccessToken should throw OAuth2ServiceException for HTTP error status" ) + void testRequestAccessTokenHttpError() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + final Map parameters = Map.of("grant_type", "client_credentials"); + + final String errorResponseBody = """ + { + "error": "invalid_client", + "error_description": "Client authentication failed" + } + """; + + when(mockResponse.getCode()).thenReturn(HttpStatus.SC_UNAUTHORIZED); + when(mockResponse.getEntity()).thenReturn(new StringEntity(errorResponseBody, StandardCharsets.UTF_8)); + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); + + // When & Then + assertThatThrownBy(() -> tokenService.requestAccessToken(tokenUri, headers, parameters)) + .isInstanceOf(OAuth2ServiceException.class) + .hasMessageContaining("Error requesting access token!") + .satisfies(exception -> { + OAuth2ServiceException oauthException = (OAuth2ServiceException) exception; + assertThat(oauthException.getHttpStatusCode()).isEqualTo(HttpStatus.SC_UNAUTHORIZED); + assertThat(oauthException.getMessage()).contains("invalid_client"); + }); + } + + @Test + @DisplayName( "requestAccessToken should throw OAuth2ServiceException for IOException" ) + void testRequestAccessTokenIOException() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + final Map parameters = Map.of("grant_type", "client_credentials"); + + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenThrow(new IOException("Connection timeout")); + + // When & Then + assertThatThrownBy(() -> tokenService.requestAccessToken(tokenUri, headers, parameters)) + .isInstanceOf(OAuth2ServiceException.class) + .hasMessageContaining("Error requesting access token!") + .satisfies(exception -> { + OAuth2ServiceException oauthException = (OAuth2ServiceException) exception; + assertThat(oauthException.getMessage()).contains("Connection timeout"); + }); + } + + @Test + @DisplayName( "requestAccessToken should handle invalid JSON response" ) + void testRequestAccessTokenInvalidJson() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + final Map parameters = Map.of("grant_type", "client_credentials"); + + final String invalidJsonResponse = "{invalid: json response}"; + + when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); + when(mockResponse.getEntity()).thenReturn(new StringEntity(invalidJsonResponse, StandardCharsets.UTF_8)); + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); + + // When & Then + assertThatThrownBy(() -> tokenService.requestAccessToken(tokenUri, headers, parameters)) + .isInstanceOf(OAuth2ServiceException.class); + } + + @Test + @DisplayName( "requestAccessToken should handle invalid expires_in value" ) + void testRequestAccessTokenInvalidExpiresIn() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + final Map parameters = Map.of("grant_type", "client_credentials"); + + final String responseBody = """ + { + "access_token": "test-token", + "token_type": "Bearer", + "expires_in": "invalid-number" + } + """; + + when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); + when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); + + // When & Then + assertThatThrownBy(() -> tokenService.requestAccessToken(tokenUri, headers, parameters)) + .isInstanceOf(OAuth2ServiceException.class) + .hasMessageContaining("Cannot convert expires_in from response"); + } + + @Test + @DisplayName( "requestAccessToken should throw IllegalArgumentException for null tokenUri" ) + void testRequestAccessTokenNullTokenUri() + { + // Given + final HttpHeaders headers = new HttpHeaders(); + final Map parameters = Map.of("grant_type", "client_credentials"); + + // When & Then + assertThatThrownBy(() -> tokenService.requestAccessToken(null, headers, parameters)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Token endpoint URI must not be null!"); + } + + @Test + @DisplayName( "requestAccessToken should properly handle custom headers" ) + void testRequestAccessTokenWithCustomHeaders() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + headers.withHeader("Authorization", "Basic dGVzdDp0ZXN0"); + headers.withHeader("X-Custom-Header", "custom-value"); + + final Map parameters = Map.of("grant_type", "client_credentials"); + + final String responseBody = """ + { + "access_token": "test-token", + "token_type": "Bearer", + "expires_in": 3600 + } + """; + + when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); + when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); + + // When + final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); + + // Then + assertThat(result).isNotNull(); + assertThat(result.getAccessToken()).isEqualTo("test-token"); + + // Verify custom headers were included + verify(mockHttpClient).execute(argThat(httpPost -> { + return httpPost instanceof HttpPost + && httpPost.getFirstHeader("Authorization") != null + && httpPost.getFirstHeader("Authorization").getValue().equals("Basic dGVzdDp0ZXN0") + && httpPost.getFirstHeader("X-Custom-Header") != null + && httpPost.getFirstHeader("X-Custom-Header").getValue().equals("custom-value"); + }), any(HttpClientResponseHandler.class)); + } + + @Test + @DisplayName( "requestAccessToken should properly handle complex parameters" ) + void testRequestAccessTokenWithComplexParameters() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + + final Map parameters = new HashMap<>(); + parameters.put("grant_type", "authorization_code"); + parameters.put("client_id", "test-client"); + parameters.put("client_secret", "test-secret"); + parameters.put("code", "auth-code-123"); + parameters.put("redirect_uri", "https://app.example.com/callback"); + parameters.put("scope", "read write"); + + final String responseBody = """ + { + "access_token": "complex-token", + "token_type": "Bearer", + "expires_in": 3600, + "refresh_token": "refresh-token-123", + "scope": "read write" + } + """; + + when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); + when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); + + // When + final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); + + // Then + assertThat(result).isNotNull(); + assertThat(result.getAccessToken()).isEqualTo("complex-token"); + assertThat(result.getTokenType()).isEqualTo("Bearer"); + assertThat(result.getTokenType()).isEqualTo("Bearer");assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)).isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); + assertThat(result.getRefreshToken()).isEqualTo("refresh-token-123"); + + // Verify all parameters were included in the request + verify(mockHttpClient).execute(argThat(httpPost -> { + try { + if (!(httpPost instanceof HttpPost)) { + return false; + } + final String requestBody = new String( + httpPost.getEntity().getContent().readAllBytes(), + StandardCharsets.UTF_8 + ); + return requestBody.contains("grant_type=authorization_code") + && requestBody.contains("client_id=test-client") + && requestBody.contains("client_secret=test-secret") + && requestBody.contains("code=auth-code-123") + && requestBody.contains("redirect_uri=https%3A%2F%2Fapp.example.com%2Fcallback") + && requestBody.contains("scope=read+write"); + } catch (final IOException e) { + return false; + } + }), any(HttpClientResponseHandler.class)); + } + + @Test + @DisplayName( "requestAccessToken should handle empty parameters map" ) + void testRequestAccessTokenWithEmptyParameters() + throws Exception + { + // Given + final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); + final HttpHeaders headers = new HttpHeaders(); + final Map parameters = new HashMap<>(); + + final String responseBody = """ + { + "access_token": "empty-params-token", + "token_type": "Bearer", + "expires_in": 3600 + } + """; + + when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); + when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); + when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + .thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); + + // When + final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); + + // Then + assertThat(result).isNotNull(); + assertThat(result.getAccessToken()).isEqualTo("empty-params-token"); + } + // Helper methods for creating test objects @Nonnull From 3e481aace7146ecdea7425b4edc00a46c9504d83 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 15:45:09 +0100 Subject: [PATCH 05/14] refactor and improve --- .../HttpClient5OAuth2TokenServiceTest.java | 183 +++++++++--------- 1 file changed, 87 insertions(+), 96 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java index d6db91843..115cb6310 100644 --- a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java +++ b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java @@ -17,20 +17,16 @@ import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; -import java.security.cert.CertificateException; import java.time.Instant; import java.time.temporal.ChronoUnit; -import java.time.temporal.TemporalUnit; import java.util.HashMap; import java.util.Map; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.io.HttpClientResponseHandler; @@ -65,6 +61,18 @@ class HttpClient5OAuth2TokenServiceTest @RegisterExtension static TestContext context = TestContext.withThreadContext(); + private CloseableHttpClient mockHttpClient; + private ClassicHttpResponse mockResponse; + private HttpClient5OAuth2TokenService tokenService; + + @BeforeEach + void setUp() + { + mockHttpClient = mock(CloseableHttpClient.class); + mockResponse = mock(ClassicHttpResponse.class); + tokenService = new HttpClient5OAuth2TokenService(mockHttpClient); + } + @Test @DisplayName( "createHttpClient with null ClientIdentity should return default HTTP client" ) void testCreateHttpClientWithNullClientIdentity() @@ -191,7 +199,7 @@ void testCreateHttpClientWithInvalidCertificateIdentity() @DisplayName( "createHttpClient with invalid KeyStore should throw HttpClientException" ) void testCreateHttpClientWithInvalidKeyStore() { - final KeyStore invalidKeyStore = createInvalidKeyStore(); + final KeyStore invalidKeyStore = mock(KeyStore.class); final ClientIdentity clientCredentials = new ClientCredentials("client-id", "client-secret"); assertThatThrownBy(() -> HttpClient5OAuth2TokenService.createHttpClient(clientCredentials, invalidKeyStore)) @@ -252,17 +260,14 @@ void testCreateHttpClientConcurrentAccess() } }); } - // Start all threads for( final Thread thread : threads ) { thread.start(); } - // Wait for all threads to complete for( final Thread thread : threads ) { thread.join(); } - // Verify results for( int i = 0; i < threadCount; i++ ) { assertThat(exceptions[i]).isNull(); @@ -270,24 +275,10 @@ void testCreateHttpClientConcurrentAccess() } } - // Tests for requestAccessToken method - - private CloseableHttpClient mockHttpClient; - private ClassicHttpResponse mockResponse; - private HttpClient5OAuth2TokenService tokenService; - - @BeforeEach - void setUp() - { - mockHttpClient = mock(CloseableHttpClient.class); - mockResponse = mock(ClassicHttpResponse.class); - tokenService = new HttpClient5OAuth2TokenService(mockHttpClient); - } - @Test @DisplayName( "requestAccessToken should successfully retrieve access token with valid response" ) void testRequestAccessTokenSuccess() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); @@ -301,7 +292,7 @@ void testRequestAccessTokenSuccess() final String responseBody = """ { - "access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9", + "access_token": "test-token", "token_type": "Bearer", "expires_in": 3600, "refresh_token": "refresh-token-value" @@ -310,7 +301,7 @@ void testRequestAccessTokenSuccess() when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + when(mockHttpClient.execute(any(ClassicHttpRequest.class), anyHttpClientResponseHandler())) .thenAnswer(invocation -> { HttpClientResponseHandler handler = invocation.getArgument(1); return handler.handleResponse(mockResponse); @@ -321,9 +312,10 @@ void testRequestAccessTokenSuccess() // Then assertThat(result).isNotNull(); - assertThat(result.getAccessToken()).isEqualTo("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9"); + assertThat(result.getAccessToken()).isEqualTo("test-token"); assertThat(result.getTokenType()).isEqualTo("Bearer"); - assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)).isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); + assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)) + .isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); assertThat(result.getRefreshToken()).isEqualTo("refresh-token-value"); // Verify HTTP client was called with correct request @@ -332,16 +324,17 @@ void testRequestAccessTokenSuccess() return httpPost instanceof HttpPost && httpPost.getUri().equals(tokenUri) && httpPost.getFirstHeader("Content-Type").getValue().equals("application/x-www-form-urlencoded"); - } catch (URISyntaxException e) { + } + catch( URISyntaxException e ) { throw new RuntimeException(e); } - }), any(HttpClientResponseHandler.class)); + }), anyHttpClientResponseHandler()); } @Test @DisplayName( "requestAccessToken should handle minimal valid response" ) void testRequestAccessTokenMinimalResponse() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); @@ -358,11 +351,10 @@ void testRequestAccessTokenMinimalResponse() when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) - .thenAnswer(invocation -> { - HttpClientResponseHandler handler = invocation.getArgument(1); - return handler.handleResponse(mockResponse); - }); + when(mockHttpClient.execute(any(HttpPost.class), anyHttpClientResponseHandler())).thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); // When final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); @@ -370,14 +362,16 @@ void testRequestAccessTokenMinimalResponse() // Then assertThat(result).isNotNull(); assertThat(result.getAccessToken()).isEqualTo("minimal-token"); - assertThat(result.getTokenType()).isEqualTo("Bearer");assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)).isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); + assertThat(result.getTokenType()).isEqualTo("Bearer"); + assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)) + .isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); assertThat(result.getRefreshToken()).isEqualTo("null"); // Should be "null" as string when not present } @Test @DisplayName( "requestAccessToken should throw OAuth2ServiceException for HTTP error status" ) void testRequestAccessTokenHttpError() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); @@ -393,11 +387,10 @@ void testRequestAccessTokenHttpError() when(mockResponse.getCode()).thenReturn(HttpStatus.SC_UNAUTHORIZED); when(mockResponse.getEntity()).thenReturn(new StringEntity(errorResponseBody, StandardCharsets.UTF_8)); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) - .thenAnswer(invocation -> { - HttpClientResponseHandler handler = invocation.getArgument(1); - return handler.handleResponse(mockResponse); - }); + when(mockHttpClient.execute(any(HttpPost.class), anyHttpClientResponseHandler())).thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); // When & Then assertThatThrownBy(() -> tokenService.requestAccessToken(tokenUri, headers, parameters)) @@ -413,14 +406,14 @@ void testRequestAccessTokenHttpError() @Test @DisplayName( "requestAccessToken should throw OAuth2ServiceException for IOException" ) void testRequestAccessTokenIOException() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); final HttpHeaders headers = new HttpHeaders(); final Map parameters = Map.of("grant_type", "client_credentials"); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) + when(mockHttpClient.execute(any(HttpPost.class), anyHttpClientResponseHandler())) .thenThrow(new IOException("Connection timeout")); // When & Then @@ -436,7 +429,7 @@ void testRequestAccessTokenIOException() @Test @DisplayName( "requestAccessToken should handle invalid JSON response" ) void testRequestAccessTokenInvalidJson() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); @@ -447,11 +440,10 @@ void testRequestAccessTokenInvalidJson() when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); when(mockResponse.getEntity()).thenReturn(new StringEntity(invalidJsonResponse, StandardCharsets.UTF_8)); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) - .thenAnswer(invocation -> { - HttpClientResponseHandler handler = invocation.getArgument(1); - return handler.handleResponse(mockResponse); - }); + when(mockHttpClient.execute(any(HttpPost.class), anyHttpClientResponseHandler())).thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); // When & Then assertThatThrownBy(() -> tokenService.requestAccessToken(tokenUri, headers, parameters)) @@ -461,7 +453,7 @@ void testRequestAccessTokenInvalidJson() @Test @DisplayName( "requestAccessToken should handle invalid expires_in value" ) void testRequestAccessTokenInvalidExpiresIn() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); @@ -478,11 +470,10 @@ void testRequestAccessTokenInvalidExpiresIn() when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) - .thenAnswer(invocation -> { - HttpClientResponseHandler handler = invocation.getArgument(1); - return handler.handleResponse(mockResponse); - }); + when(mockHttpClient.execute(any(HttpPost.class), anyHttpClientResponseHandler())).thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); // When & Then assertThatThrownBy(() -> tokenService.requestAccessToken(tokenUri, headers, parameters)) @@ -507,7 +498,7 @@ void testRequestAccessTokenNullTokenUri() @Test @DisplayName( "requestAccessToken should properly handle custom headers" ) void testRequestAccessTokenWithCustomHeaders() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); @@ -527,11 +518,10 @@ void testRequestAccessTokenWithCustomHeaders() when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) - .thenAnswer(invocation -> { - HttpClientResponseHandler handler = invocation.getArgument(1); - return handler.handleResponse(mockResponse); - }); + when(mockHttpClient.execute(any(HttpPost.class), anyHttpClientResponseHandler())).thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); // When final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); @@ -547,13 +537,13 @@ void testRequestAccessTokenWithCustomHeaders() && httpPost.getFirstHeader("Authorization").getValue().equals("Basic dGVzdDp0ZXN0") && httpPost.getFirstHeader("X-Custom-Header") != null && httpPost.getFirstHeader("X-Custom-Header").getValue().equals("custom-value"); - }), any(HttpClientResponseHandler.class)); + }), anyHttpClientResponseHandler()); } @Test @DisplayName( "requestAccessToken should properly handle complex parameters" ) void testRequestAccessTokenWithComplexParameters() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); @@ -579,11 +569,10 @@ void testRequestAccessTokenWithComplexParameters() when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) - .thenAnswer(invocation -> { - HttpClientResponseHandler handler = invocation.getArgument(1); - return handler.handleResponse(mockResponse); - }); + when(mockHttpClient.execute(any(HttpPost.class), anyHttpClientResponseHandler())).thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); // When final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); @@ -592,35 +581,36 @@ void testRequestAccessTokenWithComplexParameters() assertThat(result).isNotNull(); assertThat(result.getAccessToken()).isEqualTo("complex-token"); assertThat(result.getTokenType()).isEqualTo("Bearer"); - assertThat(result.getTokenType()).isEqualTo("Bearer");assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)).isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); + assertThat(result.getTokenType()).isEqualTo("Bearer"); + assertThat(result.getExpiredAt().truncatedTo(ChronoUnit.HOURS)) + .isEqualTo(Instant.now().plusSeconds(3600).truncatedTo(ChronoUnit.HOURS)); assertThat(result.getRefreshToken()).isEqualTo("refresh-token-123"); // Verify all parameters were included in the request verify(mockHttpClient).execute(argThat(httpPost -> { try { - if (!(httpPost instanceof HttpPost)) { + if( !(httpPost instanceof HttpPost) ) { return false; } - final String requestBody = new String( - httpPost.getEntity().getContent().readAllBytes(), - StandardCharsets.UTF_8 - ); + final String requestBody = + new String(httpPost.getEntity().getContent().readAllBytes(), StandardCharsets.UTF_8); return requestBody.contains("grant_type=authorization_code") && requestBody.contains("client_id=test-client") && requestBody.contains("client_secret=test-secret") && requestBody.contains("code=auth-code-123") && requestBody.contains("redirect_uri=https%3A%2F%2Fapp.example.com%2Fcallback") && requestBody.contains("scope=read+write"); - } catch (final IOException e) { + } + catch( final IOException e ) { return false; } - }), any(HttpClientResponseHandler.class)); + }), anyHttpClientResponseHandler()); } @Test @DisplayName( "requestAccessToken should handle empty parameters map" ) void testRequestAccessTokenWithEmptyParameters() - throws Exception + throws IOException { // Given final URI tokenUri = URI.create("https://oauth.server.com/oauth/token"); @@ -637,11 +627,10 @@ void testRequestAccessTokenWithEmptyParameters() when(mockResponse.getCode()).thenReturn(HttpStatus.SC_OK); when(mockResponse.getEntity()).thenReturn(new StringEntity(responseBody, StandardCharsets.UTF_8)); - when(mockHttpClient.execute(any(HttpPost.class), any(HttpClientResponseHandler.class))) - .thenAnswer(invocation -> { - HttpClientResponseHandler handler = invocation.getArgument(1); - return handler.handleResponse(mockResponse); - }); + when(mockHttpClient.execute(any(HttpPost.class), anyHttpClientResponseHandler())).thenAnswer(invocation -> { + HttpClientResponseHandler handler = invocation.getArgument(1); + return handler.handleResponse(mockResponse); + }); // When final OAuth2TokenResponse result = tokenService.requestAccessToken(tokenUri, headers, parameters); @@ -651,14 +640,23 @@ void testRequestAccessTokenWithEmptyParameters() assertThat(result.getAccessToken()).isEqualTo("empty-params-token"); } - // Helper methods for creating test objects + /** + * Helper method to provide a properly typed HttpClientResponseHandler matcher. This avoids unchecked type warnings + * when using {@link org.mockito.ArgumentMatchers#any(Class)} with generic types. + * + * @return A matcher that accepts any HttpClientResponseHandler of type String + * @param + * The response handler type parameter + */ + @SuppressWarnings( "unchecked" ) + private static HttpClientResponseHandler anyHttpClientResponseHandler() + { + return any(HttpClientResponseHandler.class); + } @Nonnull private static KeyStore createEmptyKeyStore() - throws KeyStoreException, - CertificateException, - IOException, - NoSuchAlgorithmException + throws Exception { final KeyStore keyStore = KeyStore.getInstance("JKS"); keyStore.load(null, null); @@ -694,11 +692,4 @@ private static ClientIdentity createInvalidCertificateBasedIdentity() return mockCertificate; } - - @Nullable - private static KeyStore createInvalidKeyStore() - { - // Return a mock KeyStore that will cause SSL context creation to fail - return mock(KeyStore.class); - } } From 152b83f15416a8dbcef90f31c9934c9e1dae01d2 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 16:35:36 +0100 Subject: [PATCH 06/14] PMD --- .../HttpClient5OAuth2TokenService.java | 57 ++++++++++++------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index a6e7141c2..36af306b4 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -4,7 +4,13 @@ package com.sap.cloud.sdk.cloudplatform.connectivity; -import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.*; +import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.ACCESS_TOKEN; +import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.ASSERTION; +import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.CLIENT_SECRET; +import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.EXPIRES_IN; +import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.PASSWORD; +import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.REFRESH_TOKEN; +import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.TOKEN_TYPE; import java.io.IOException; import java.net.URI; @@ -110,19 +116,14 @@ private String executeRequest( throws OAuth2ServiceException { final HttpPost httpPost = createHttpPost(tokenUri, createRequestHeaders(headers), parameters); - log - .debug( - "Requesting access token from url {} with headers {} and {} retries left", - tokenUri, - headers, - attemptsLeft); + log.debug("Requesting access token with {} retries left", attemptsLeft); try { return httpClient.execute(httpPost, response -> { final int statusCode = response.getCode(); final String body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - log.debug("Received statusCode {} from {}", statusCode, tokenUri); + log.debug("Received statusCode {} from host {}", statusCode, tokenUri.getHost()); if( HttpStatus.SC_OK == statusCode ) { - log.debug("Successfully retrieved access token from {} with params {}.", tokenUri, parameters); + log.debug("Successfully retrieved access token from host {}.", tokenUri.getHost()); return body; } else if( attemptsLeft > 0 && config.getRetryStatusCodes().contains(statusCode) ) { log.warn("Request failed with status {} but is retryable. Retrying...", statusCode); @@ -143,12 +144,15 @@ private String executeRequest( if( e instanceof final OAuth2ServiceException oAuth2Exception ) { throw oAuth2Exception; } else { - throw OAuth2ServiceException - .builder("Error requesting access token!") - .withUri(tokenUri) - .withRequestHeaders(getHeadersAsStringArray(httpPost.getHeaders())) - .withResponseBody(e.getMessage()) - .build(); + final var exception = + OAuth2ServiceException + .builder("Error requesting access token!") + .withUri(tokenUri) + .withRequestHeaders(getHeadersAsStringArray(httpPost.getHeaders())) + .withResponseBody(e.getMessage()) + .build(); + exception.initCause(e); + throw exception; } } } @@ -189,7 +193,9 @@ private HttpPost createHttpPost( final URI uri, final HttpHeaders headers, final httpPost.addHeader(org.apache.hc.core5.http.HttpHeaders.USER_AGENT, HttpClientUtil.getUserAgent()); } catch( final Exception e ) { - throw new OAuth2ServiceException("Unexpected error parsing URI: " + e.getMessage()); + final var exception = new OAuth2ServiceException("Unexpected error parsing URI: " + e.getMessage()); + exception.initCause(e); + throw exception; } logRequest(headers, parameters); return httpPost; @@ -213,8 +219,11 @@ private Long convertExpiresInToLong( final String expiresIn ) return Long.parseLong(expiresIn); } catch( final NumberFormatException e ) { - throw new OAuth2ServiceException( - String.format("Cannot convert expires_in from response (%s) to long", expiresIn)); + final var exception = + new OAuth2ServiceException( + String.format("Cannot convert expires_in from response (%s) to long", expiresIn)); + exception.initCause(e); + throw exception; } } @@ -310,8 +319,11 @@ static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clie return createHttpClientWithKeyStore(identityKeyStore); } catch( final Exception e ) { - throw new HttpClientException( - "Failed to create HTTPS HttpClient5 with certificate authentication: " + e.getMessage()); + final var exception = + new HttpClientException( + "Failed to create HTTPS HttpClient5 with certificate authentication: " + e.getMessage()); + exception.initCause(e); + throw exception; } } @@ -335,7 +347,10 @@ private static CloseableHttpClient createHttpClientWithKeyStore( @Nonnull final return HttpClientBuilder.create().useSystemProperties().setConnectionManager(connectionManager).build(); } catch( final Exception e ) { - throw new HttpClientException("Failed to create HTTPS HttpClient5 with KeyStore: " + e.getMessage()); + final var exception = + new HttpClientException("Failed to create HTTPS HttpClient5 with KeyStore: " + e.getMessage()); + exception.initCause(e); + throw exception; } } } From 226783778e490ce611b5957cdeffddf85930799b Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 16:52:13 +0100 Subject: [PATCH 07/14] PMD --- .../HttpClient5OAuth2TokenService.java | 154 +++++++----------- 1 file changed, 57 insertions(+), 97 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index 36af306b4..3e6e24725 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -4,14 +4,6 @@ package com.sap.cloud.sdk.cloudplatform.connectivity; -import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.ACCESS_TOKEN; -import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.ASSERTION; -import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.CLIENT_SECRET; -import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.EXPIRES_IN; -import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.PASSWORD; -import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.REFRESH_TOKEN; -import static com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants.TOKEN_TYPE; - import java.io.IOException; import java.net.URI; import java.nio.charset.StandardCharsets; @@ -49,6 +41,7 @@ import com.sap.cloud.security.xsuaa.client.AbstractOAuth2TokenService; import com.sap.cloud.security.xsuaa.client.OAuth2ServiceException; import com.sap.cloud.security.xsuaa.client.OAuth2TokenResponse; +import com.sap.cloud.security.xsuaa.client.OAuth2TokenServiceConstants; import com.sap.cloud.security.xsuaa.http.HttpHeaders; import com.sap.cloud.security.xsuaa.tokenflows.TokenCacheConfiguration; import com.sap.cloud.security.xsuaa.util.HttpClientUtil; @@ -62,8 +55,7 @@ * requests using Apache HttpClient 5 instead of HttpClient 4. */ @Slf4j -class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService -{ +class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService { private static final char[] EMPTY_PASSWORD = {}; private final CloseableHttpClient httpClient; @@ -72,37 +64,30 @@ class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService /** * Creates a new instance with the given HTTP client and default cache configuration. * - * @param httpClient - * The HTTP client to use for token requests. + * @param httpClient The HTTP client to use for token requests. */ - HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpClient ) - { + HttpClient5OAuth2TokenService(@Nonnull final CloseableHttpClient httpClient) { this(httpClient, TokenCacheConfiguration.defaultConfiguration()); } /** * Creates a new instance with the given HTTP client and cache configuration. * - * @param httpClient - * The HTTP client to use for token requests. - * @param tokenCacheConfiguration - * The cache configuration to use. + * @param httpClient The HTTP client to use for token requests. + * @param tokenCacheConfiguration The cache configuration to use. */ HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpClient, - @Nonnull final TokenCacheConfiguration tokenCacheConfiguration ) - { + @Nonnull final TokenCacheConfiguration tokenCacheConfiguration) { super(tokenCacheConfiguration); Assertions.assertNotNull(httpClient, "http client is required"); this.httpClient = httpClient; } @Override - protected - OAuth2TokenResponse - requestAccessToken( final URI tokenUri, final HttpHeaders headers, final Map parameters ) - throws OAuth2ServiceException - { + protected OAuth2TokenResponse + requestAccessToken(final URI tokenUri, final HttpHeaders headers, final Map parameters) + throws OAuth2ServiceException { Assertions.assertNotNull(tokenUri, "Token endpoint URI must not be null!"); return convertToOAuth2TokenResponse( executeRequest(tokenUri, headers, parameters, config.isRetryEnabled() ? config.getMaxRetryAttempts() : 0)); @@ -112,9 +97,8 @@ private String executeRequest( final URI tokenUri, final HttpHeaders headers, final Map parameters, - final int attemptsLeft ) - throws OAuth2ServiceException - { + final int attemptsLeft) + throws OAuth2ServiceException { final HttpPost httpPost = createHttpPost(tokenUri, createRequestHeaders(headers), parameters); log.debug("Requesting access token with {} retries left", attemptsLeft); try { @@ -122,10 +106,10 @@ private String executeRequest( final int statusCode = response.getCode(); final String body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); log.debug("Received statusCode {} from host {}", statusCode, tokenUri.getHost()); - if( HttpStatus.SC_OK == statusCode ) { + if (HttpStatus.SC_OK == statusCode) { log.debug("Successfully retrieved access token from host {}.", tokenUri.getHost()); return body; - } else if( attemptsLeft > 0 && config.getRetryStatusCodes().contains(statusCode) ) { + } else if (attemptsLeft > 0 && config.getRetryStatusCodes().contains(statusCode)) { log.warn("Request failed with status {} but is retryable. Retrying...", statusCode); pauseBeforeNextAttempt(config.getRetryDelayTime()); return executeRequest(tokenUri, headers, parameters, attemptsLeft - 1); @@ -139,9 +123,8 @@ private String executeRequest( .withResponseBody(body) .build(); }); - } - catch( final IOException e ) { - if( e instanceof final OAuth2ServiceException oAuth2Exception ) { + } catch (final IOException e) { + if (e instanceof final OAuth2ServiceException oAuth2Exception) { throw oAuth2Exception; } else { final var exception = @@ -157,29 +140,26 @@ private String executeRequest( } } - private HttpHeaders createRequestHeaders( final HttpHeaders headers ) - { + private HttpHeaders createRequestHeaders(final HttpHeaders headers) { final HttpHeaders requestHeaders = new HttpHeaders(); headers.getHeaders().forEach(h -> requestHeaders.withHeader(h.getName(), h.getValue())); requestHeaders.withHeader(MDCHelper.CORRELATION_HEADER, MDCHelper.getOrCreateCorrelationId()); return requestHeaders; } - private void logRequest( final HttpHeaders headers, final Map parameters ) - { + private void logRequest(final HttpHeaders headers, final Map parameters) { log.debug("access token request {} - {}", headers, parameters.entrySet().stream().map(e -> { - if( e.getKey().contains(PASSWORD) - || e.getKey().contains(CLIENT_SECRET) - || e.getKey().contains(ASSERTION) ) { + if (e.getKey().contains(OAuth2TokenServiceConstants.PASSWORD) + || e.getKey().contains(OAuth2TokenServiceConstants.CLIENT_SECRET) + || e.getKey().contains(OAuth2TokenServiceConstants.ASSERTION)) { return new AbstractMap.SimpleImmutableEntry<>(e.getKey(), "****"); } return e; }).toList()); } - private HttpPost createHttpPost( final URI uri, final HttpHeaders headers, final Map parameters ) - throws OAuth2ServiceException - { + private HttpPost createHttpPost(final URI uri, final HttpHeaders headers, final Map parameters) + throws OAuth2ServiceException { final HttpPost httpPost = new HttpPost(uri); headers.getHeaders().forEach(header -> httpPost.setHeader(header.getName(), header.getValue())); final List basicNameValuePairs = @@ -191,8 +171,7 @@ private HttpPost createHttpPost( final URI uri, final HttpHeaders headers, final try { httpPost.setEntity(new UrlEncodedFormEntity(basicNameValuePairs, StandardCharsets.UTF_8)); httpPost.addHeader(org.apache.hc.core5.http.HttpHeaders.USER_AGENT, HttpClientUtil.getUserAgent()); - } - catch( final Exception e ) { + } catch (final Exception e) { final var exception = new OAuth2ServiceException("Unexpected error parsing URI: " + e.getMessage()); exception.initCause(e); throw exception; @@ -201,24 +180,21 @@ private HttpPost createHttpPost( final URI uri, final HttpHeaders headers, final return httpPost; } - private OAuth2TokenResponse convertToOAuth2TokenResponse( final String responseBody ) - throws OAuth2ServiceException - { + private OAuth2TokenResponse convertToOAuth2TokenResponse(final String responseBody) + throws OAuth2ServiceException { final Map accessTokenMap = new JSONObject(responseBody).toMap(); - final String accessToken = getParameter(accessTokenMap, ACCESS_TOKEN); - final String refreshToken = getParameter(accessTokenMap, REFRESH_TOKEN); - final String expiresIn = getParameter(accessTokenMap, EXPIRES_IN); - final String tokenType = getParameter(accessTokenMap, TOKEN_TYPE); + final String accessToken = getParameter(accessTokenMap, OAuth2TokenServiceConstants.ACCESS_TOKEN); + final String refreshToken = getParameter(accessTokenMap, OAuth2TokenServiceConstants.REFRESH_TOKEN); + final String expiresIn = getParameter(accessTokenMap, OAuth2TokenServiceConstants.EXPIRES_IN); + final String tokenType = getParameter(accessTokenMap, OAuth2TokenServiceConstants.TOKEN_TYPE); return new OAuth2TokenResponse(accessToken, convertExpiresInToLong(expiresIn), refreshToken, tokenType); } - private Long convertExpiresInToLong( final String expiresIn ) - throws OAuth2ServiceException - { + private Long convertExpiresInToLong(final String expiresIn) + throws OAuth2ServiceException { try { return Long.parseLong(expiresIn); - } - catch( final NumberFormatException e ) { + } catch (final NumberFormatException e) { final var exception = new OAuth2ServiceException( String.format("Cannot convert expires_in from response (%s) to long", expiresIn)); @@ -227,23 +203,19 @@ private Long convertExpiresInToLong( final String expiresIn ) } } - private String getParameter( final Map accessTokenMap, final String key ) - { + private String getParameter(final Map accessTokenMap, final String key) { return String.valueOf(accessTokenMap.get(key)); } - private static String[] getHeadersAsStringArray( final Header[] headers ) - { + private static String[] getHeadersAsStringArray(final Header[] headers) { return headers != null ? Arrays.stream(headers).map(Header::toString).toArray(String[]::new) : new String[0]; } - private void pauseBeforeNextAttempt( final long sleepTime ) - { + private void pauseBeforeNextAttempt(final long sleepTime) { try { log.info("Retry again in {} ms", sleepTime); Thread.sleep(sleepTime); - } - catch( final InterruptedException e ) { + } catch (final InterruptedException e) { log.warn("Thread.sleep has been interrupted. Retry starts now."); } } @@ -254,17 +226,14 @@ private void pauseBeforeNextAttempt( final long sleepTime ) * For ClientIdentity that is certificate based it will resolve HTTPS client using the provided ClientIdentity. If * the ClientIdentity wasn't provided or is not certificate-based, it will return default HttpClient. * - * @param clientIdentity - * for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity - * interface should be provided + * @param clientIdentity for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity + * interface should be provided * @return HTTP or HTTPS client (HttpClient5) - * @throws HttpClientException - * in case HTTPS Client could not be setup + * @throws HttpClientException in case HTTPS Client could not be setup */ @Nonnull - static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clientIdentity ) - throws HttpClientException - { + static CloseableHttpClient createHttpClient(@Nullable final ClientIdentity clientIdentity) + throws HttpClientException { return createHttpClient(clientIdentity, null); } @@ -275,23 +244,18 @@ static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clie * KeyStore is provided (e.g., for ZTIS), it will be used directly. If the ClientIdentity wasn't provided or is not * certificate-based, it will return default HttpClient. * - * @param clientIdentity - * for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity - * interface should be provided - * @param keyStore - * optional KeyStore to use for mTLS (e.g., for ZTIS) + * @param clientIdentity for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity + * interface should be provided + * @param keyStore optional KeyStore to use for mTLS (e.g., for ZTIS) * @return HTTP or HTTPS client (HttpClient5) - * @throws HttpClientException - * in case HTTPS Client could not be setup + * @throws HttpClientException in case HTTPS Client could not be setup */ @Nonnull - static - CloseableHttpClient - createHttpClient( @Nullable final ClientIdentity clientIdentity, @Nullable final KeyStore keyStore ) - throws HttpClientException - { + static CloseableHttpClient + createHttpClient(@Nullable final ClientIdentity clientIdentity, @Nullable final KeyStore keyStore) + throws HttpClientException { // If a KeyStore is provided directly (e.g., for ZTIS), use it - if( keyStore != null ) { + if (keyStore != null) { log .debug( "Creating HTTPS HttpClient5 with provided KeyStore for client '{}'", @@ -299,12 +263,12 @@ static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clie return createHttpClientWithKeyStore(keyStore); } - if( clientIdentity == null ) { + if (clientIdentity == null) { log.debug("No ClientIdentity provided, creating default HttpClient5"); return createDefaultHttpClient(); } - if( !clientIdentity.isCertificateBased() ) { + if (!clientIdentity.isCertificateBased()) { log.debug("ClientIdentity is not certificate-based, creating default HttpClient5"); return createDefaultHttpClient(); } @@ -317,8 +281,7 @@ static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clie try { final KeyStore identityKeyStore = SSLContextFactory.getInstance().createKeyStore(clientIdentity); return createHttpClientWithKeyStore(identityKeyStore); - } - catch( final Exception e ) { + } catch (final Exception e) { final var exception = new HttpClientException( "Failed to create HTTPS HttpClient5 with certificate authentication: " + e.getMessage()); @@ -328,15 +291,13 @@ static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clie } @Nonnull - private static CloseableHttpClient createDefaultHttpClient() - { + private static CloseableHttpClient createDefaultHttpClient() { return HttpClients.custom().useSystemProperties().build(); } @Nonnull - private static CloseableHttpClient createHttpClientWithKeyStore( @Nonnull final KeyStore keyStore ) - throws HttpClientException - { + private static CloseableHttpClient createHttpClientWithKeyStore(@Nonnull final KeyStore keyStore) + throws HttpClientException { try { final SSLContext sslContext = SSLContextBuilder.create().loadKeyMaterial(keyStore, EMPTY_PASSWORD).build(); @@ -345,8 +306,7 @@ private static CloseableHttpClient createHttpClientWithKeyStore( @Nonnull final PoolingHttpClientConnectionManagerBuilder.create().setTlsSocketStrategy(tlsStrategy).build(); return HttpClientBuilder.create().useSystemProperties().setConnectionManager(connectionManager).build(); - } - catch( final Exception e ) { + } catch (final Exception e) { final var exception = new HttpClientException("Failed to create HTTPS HttpClient5 with KeyStore: " + e.getMessage()); exception.initCause(e); From b8256bc9fd89eefd1a09da638c581d552d3d75c5 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 16:56:28 +0100 Subject: [PATCH 08/14] undo reformatting --- .../HttpClient5OAuth2TokenService.java | 135 +++++++++++------- 1 file changed, 84 insertions(+), 51 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index 3e6e24725..222ea4559 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -55,7 +55,8 @@ * requests using Apache HttpClient 5 instead of HttpClient 4. */ @Slf4j -class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService { +class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService +{ private static final char[] EMPTY_PASSWORD = {}; private final CloseableHttpClient httpClient; @@ -64,30 +65,37 @@ class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService { /** * Creates a new instance with the given HTTP client and default cache configuration. * - * @param httpClient The HTTP client to use for token requests. + * @param httpClient + * The HTTP client to use for token requests. */ - HttpClient5OAuth2TokenService(@Nonnull final CloseableHttpClient httpClient) { + HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpClient ) + { this(httpClient, TokenCacheConfiguration.defaultConfiguration()); } /** * Creates a new instance with the given HTTP client and cache configuration. * - * @param httpClient The HTTP client to use for token requests. - * @param tokenCacheConfiguration The cache configuration to use. + * @param httpClient + * The HTTP client to use for token requests. + * @param tokenCacheConfiguration + * The cache configuration to use. */ HttpClient5OAuth2TokenService( @Nonnull final CloseableHttpClient httpClient, - @Nonnull final TokenCacheConfiguration tokenCacheConfiguration) { + @Nonnull final TokenCacheConfiguration tokenCacheConfiguration ) + { super(tokenCacheConfiguration); Assertions.assertNotNull(httpClient, "http client is required"); this.httpClient = httpClient; } @Override - protected OAuth2TokenResponse - requestAccessToken(final URI tokenUri, final HttpHeaders headers, final Map parameters) - throws OAuth2ServiceException { + protected + OAuth2TokenResponse + requestAccessToken( final URI tokenUri, final HttpHeaders headers, final Map parameters ) + throws OAuth2ServiceException + { Assertions.assertNotNull(tokenUri, "Token endpoint URI must not be null!"); return convertToOAuth2TokenResponse( executeRequest(tokenUri, headers, parameters, config.isRetryEnabled() ? config.getMaxRetryAttempts() : 0)); @@ -97,8 +105,9 @@ private String executeRequest( final URI tokenUri, final HttpHeaders headers, final Map parameters, - final int attemptsLeft) - throws OAuth2ServiceException { + final int attemptsLeft ) + throws OAuth2ServiceException + { final HttpPost httpPost = createHttpPost(tokenUri, createRequestHeaders(headers), parameters); log.debug("Requesting access token with {} retries left", attemptsLeft); try { @@ -106,10 +115,10 @@ private String executeRequest( final int statusCode = response.getCode(); final String body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); log.debug("Received statusCode {} from host {}", statusCode, tokenUri.getHost()); - if (HttpStatus.SC_OK == statusCode) { + if( HttpStatus.SC_OK == statusCode ) { log.debug("Successfully retrieved access token from host {}.", tokenUri.getHost()); return body; - } else if (attemptsLeft > 0 && config.getRetryStatusCodes().contains(statusCode)) { + } else if( attemptsLeft > 0 && config.getRetryStatusCodes().contains(statusCode) ) { log.warn("Request failed with status {} but is retryable. Retrying...", statusCode); pauseBeforeNextAttempt(config.getRetryDelayTime()); return executeRequest(tokenUri, headers, parameters, attemptsLeft - 1); @@ -123,8 +132,9 @@ private String executeRequest( .withResponseBody(body) .build(); }); - } catch (final IOException e) { - if (e instanceof final OAuth2ServiceException oAuth2Exception) { + } + catch( final IOException e ) { + if( e instanceof final OAuth2ServiceException oAuth2Exception ) { throw oAuth2Exception; } else { final var exception = @@ -140,26 +150,29 @@ private String executeRequest( } } - private HttpHeaders createRequestHeaders(final HttpHeaders headers) { + private HttpHeaders createRequestHeaders( final HttpHeaders headers ) + { final HttpHeaders requestHeaders = new HttpHeaders(); headers.getHeaders().forEach(h -> requestHeaders.withHeader(h.getName(), h.getValue())); requestHeaders.withHeader(MDCHelper.CORRELATION_HEADER, MDCHelper.getOrCreateCorrelationId()); return requestHeaders; } - private void logRequest(final HttpHeaders headers, final Map parameters) { + private void logRequest( final HttpHeaders headers, final Map parameters ) + { log.debug("access token request {} - {}", headers, parameters.entrySet().stream().map(e -> { - if (e.getKey().contains(OAuth2TokenServiceConstants.PASSWORD) + if( e.getKey().contains(OAuth2TokenServiceConstants.PASSWORD) || e.getKey().contains(OAuth2TokenServiceConstants.CLIENT_SECRET) - || e.getKey().contains(OAuth2TokenServiceConstants.ASSERTION)) { + || e.getKey().contains(OAuth2TokenServiceConstants.ASSERTION) ) { return new AbstractMap.SimpleImmutableEntry<>(e.getKey(), "****"); } return e; }).toList()); } - private HttpPost createHttpPost(final URI uri, final HttpHeaders headers, final Map parameters) - throws OAuth2ServiceException { + private HttpPost createHttpPost( final URI uri, final HttpHeaders headers, final Map parameters ) + throws OAuth2ServiceException + { final HttpPost httpPost = new HttpPost(uri); headers.getHeaders().forEach(header -> httpPost.setHeader(header.getName(), header.getValue())); final List basicNameValuePairs = @@ -171,7 +184,8 @@ private HttpPost createHttpPost(final URI uri, final HttpHeaders headers, final try { httpPost.setEntity(new UrlEncodedFormEntity(basicNameValuePairs, StandardCharsets.UTF_8)); httpPost.addHeader(org.apache.hc.core5.http.HttpHeaders.USER_AGENT, HttpClientUtil.getUserAgent()); - } catch (final Exception e) { + } + catch( final Exception e ) { final var exception = new OAuth2ServiceException("Unexpected error parsing URI: " + e.getMessage()); exception.initCause(e); throw exception; @@ -180,8 +194,9 @@ private HttpPost createHttpPost(final URI uri, final HttpHeaders headers, final return httpPost; } - private OAuth2TokenResponse convertToOAuth2TokenResponse(final String responseBody) - throws OAuth2ServiceException { + private OAuth2TokenResponse convertToOAuth2TokenResponse( final String responseBody ) + throws OAuth2ServiceException + { final Map accessTokenMap = new JSONObject(responseBody).toMap(); final String accessToken = getParameter(accessTokenMap, OAuth2TokenServiceConstants.ACCESS_TOKEN); final String refreshToken = getParameter(accessTokenMap, OAuth2TokenServiceConstants.REFRESH_TOKEN); @@ -190,11 +205,13 @@ private OAuth2TokenResponse convertToOAuth2TokenResponse(final String responseBo return new OAuth2TokenResponse(accessToken, convertExpiresInToLong(expiresIn), refreshToken, tokenType); } - private Long convertExpiresInToLong(final String expiresIn) - throws OAuth2ServiceException { + private Long convertExpiresInToLong( final String expiresIn ) + throws OAuth2ServiceException + { try { return Long.parseLong(expiresIn); - } catch (final NumberFormatException e) { + } + catch( final NumberFormatException e ) { final var exception = new OAuth2ServiceException( String.format("Cannot convert expires_in from response (%s) to long", expiresIn)); @@ -203,19 +220,23 @@ private Long convertExpiresInToLong(final String expiresIn) } } - private String getParameter(final Map accessTokenMap, final String key) { + private String getParameter( final Map accessTokenMap, final String key ) + { return String.valueOf(accessTokenMap.get(key)); } - private static String[] getHeadersAsStringArray(final Header[] headers) { + private static String[] getHeadersAsStringArray( final Header[] headers ) + { return headers != null ? Arrays.stream(headers).map(Header::toString).toArray(String[]::new) : new String[0]; } - private void pauseBeforeNextAttempt(final long sleepTime) { + private void pauseBeforeNextAttempt( final long sleepTime ) + { try { log.info("Retry again in {} ms", sleepTime); Thread.sleep(sleepTime); - } catch (final InterruptedException e) { + } + catch( final InterruptedException e ) { log.warn("Thread.sleep has been interrupted. Retry starts now."); } } @@ -226,14 +247,17 @@ private void pauseBeforeNextAttempt(final long sleepTime) { * For ClientIdentity that is certificate based it will resolve HTTPS client using the provided ClientIdentity. If * the ClientIdentity wasn't provided or is not certificate-based, it will return default HttpClient. * - * @param clientIdentity for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity - * interface should be provided + * @param clientIdentity + * for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity + * interface should be provided * @return HTTP or HTTPS client (HttpClient5) - * @throws HttpClientException in case HTTPS Client could not be setup + * @throws HttpClientException + * in case HTTPS Client could not be setup */ @Nonnull - static CloseableHttpClient createHttpClient(@Nullable final ClientIdentity clientIdentity) - throws HttpClientException { + static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clientIdentity ) + throws HttpClientException + { return createHttpClient(clientIdentity, null); } @@ -244,18 +268,23 @@ static CloseableHttpClient createHttpClient(@Nullable final ClientIdentity clien * KeyStore is provided (e.g., for ZTIS), it will be used directly. If the ClientIdentity wasn't provided or is not * certificate-based, it will return default HttpClient. * - * @param clientIdentity for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity - * interface should be provided - * @param keyStore optional KeyStore to use for mTLS (e.g., for ZTIS) + * @param clientIdentity + * for X.509 certificate based communication {@link ClientCertificate} implementation of ClientIdentity + * interface should be provided + * @param keyStore + * optional KeyStore to use for mTLS (e.g., for ZTIS) * @return HTTP or HTTPS client (HttpClient5) - * @throws HttpClientException in case HTTPS Client could not be setup + * @throws HttpClientException + * in case HTTPS Client could not be setup */ @Nonnull - static CloseableHttpClient - createHttpClient(@Nullable final ClientIdentity clientIdentity, @Nullable final KeyStore keyStore) - throws HttpClientException { + static + CloseableHttpClient + createHttpClient( @Nullable final ClientIdentity clientIdentity, @Nullable final KeyStore keyStore ) + throws HttpClientException + { // If a KeyStore is provided directly (e.g., for ZTIS), use it - if (keyStore != null) { + if( keyStore != null ) { log .debug( "Creating HTTPS HttpClient5 with provided KeyStore for client '{}'", @@ -263,12 +292,12 @@ static CloseableHttpClient createHttpClient(@Nullable final ClientIdentity clien return createHttpClientWithKeyStore(keyStore); } - if (clientIdentity == null) { + if( clientIdentity == null ) { log.debug("No ClientIdentity provided, creating default HttpClient5"); return createDefaultHttpClient(); } - if (!clientIdentity.isCertificateBased()) { + if( !clientIdentity.isCertificateBased() ) { log.debug("ClientIdentity is not certificate-based, creating default HttpClient5"); return createDefaultHttpClient(); } @@ -281,7 +310,8 @@ static CloseableHttpClient createHttpClient(@Nullable final ClientIdentity clien try { final KeyStore identityKeyStore = SSLContextFactory.getInstance().createKeyStore(clientIdentity); return createHttpClientWithKeyStore(identityKeyStore); - } catch (final Exception e) { + } + catch( final Exception e ) { final var exception = new HttpClientException( "Failed to create HTTPS HttpClient5 with certificate authentication: " + e.getMessage()); @@ -291,13 +321,15 @@ static CloseableHttpClient createHttpClient(@Nullable final ClientIdentity clien } @Nonnull - private static CloseableHttpClient createDefaultHttpClient() { + private static CloseableHttpClient createDefaultHttpClient() + { return HttpClients.custom().useSystemProperties().build(); } @Nonnull - private static CloseableHttpClient createHttpClientWithKeyStore(@Nonnull final KeyStore keyStore) - throws HttpClientException { + private static CloseableHttpClient createHttpClientWithKeyStore( @Nonnull final KeyStore keyStore ) + throws HttpClientException + { try { final SSLContext sslContext = SSLContextBuilder.create().loadKeyMaterial(keyStore, EMPTY_PASSWORD).build(); @@ -306,7 +338,8 @@ private static CloseableHttpClient createHttpClientWithKeyStore(@Nonnull final K PoolingHttpClientConnectionManagerBuilder.create().setTlsSocketStrategy(tlsStrategy).build(); return HttpClientBuilder.create().useSystemProperties().setConnectionManager(connectionManager).build(); - } catch (final Exception e) { + } + catch( final Exception e ) { final var exception = new HttpClientException("Failed to create HTTPS HttpClient5 with KeyStore: " + e.getMessage()); exception.initCause(e); From 0703644b75e102b540882f1898f5e10d7f8fd50d Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 16:58:43 +0100 Subject: [PATCH 09/14] more checks --- .../connectivity/HttpClient5OAuth2TokenService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index 222ea4559..121f0ab3f 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -114,9 +114,9 @@ private String executeRequest( return httpClient.execute(httpPost, response -> { final int statusCode = response.getCode(); final String body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8); - log.debug("Received statusCode {} from host {}", statusCode, tokenUri.getHost()); + log.debug("Received statusCode {} from token endpoint", statusCode); if( HttpStatus.SC_OK == statusCode ) { - log.debug("Successfully retrieved access token from host {}.", tokenUri.getHost()); + log.debug("Successfully retrieved access token from token endpoint"); return body; } else if( attemptsLeft > 0 && config.getRetryStatusCodes().contains(statusCode) ) { log.warn("Request failed with status {} but is retryable. Retrying...", statusCode); From fb110c6c99dfefa108426cb3e8a400649f69ffdf Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 17:03:08 +0100 Subject: [PATCH 10/14] code format --- .../connectivity/HttpClient5OAuth2TokenService.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index 121f0ab3f..b85ed550d 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -92,9 +92,9 @@ class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService @Override protected - OAuth2TokenResponse - requestAccessToken( final URI tokenUri, final HttpHeaders headers, final Map parameters ) - throws OAuth2ServiceException + OAuth2TokenResponse + requestAccessToken( final URI tokenUri, final HttpHeaders headers, final Map parameters ) + throws OAuth2ServiceException { Assertions.assertNotNull(tokenUri, "Token endpoint URI must not be null!"); return convertToOAuth2TokenResponse( @@ -279,9 +279,9 @@ static CloseableHttpClient createHttpClient( @Nullable final ClientIdentity clie */ @Nonnull static - CloseableHttpClient - createHttpClient( @Nullable final ClientIdentity clientIdentity, @Nullable final KeyStore keyStore ) - throws HttpClientException + CloseableHttpClient + createHttpClient( @Nullable final ClientIdentity clientIdentity, @Nullable final KeyStore keyStore ) + throws HttpClientException { // If a KeyStore is provided directly (e.g., for ZTIS), use it if( keyStore != null ) { From 628c3bb6a3825471e9040e47023fa2feeee2422d Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 17:18:59 +0100 Subject: [PATCH 11/14] PMD again --- .../connectivity/HttpClient5OAuth2TokenService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index b85ed550d..6fdd433ee 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -134,8 +134,8 @@ private String executeRequest( }); } catch( final IOException e ) { - if( e instanceof final OAuth2ServiceException oAuth2Exception ) { - throw oAuth2Exception; + if( e instanceof OAuth2ServiceException ) { + throw (OAuth2ServiceException) e; } else { final var exception = OAuth2ServiceException From 826cd821f302a3c71cb02bc8ecf0314556ad6a33 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 17:31:56 +0100 Subject: [PATCH 12/14] =?UTF-8?q?PMD=20again=20=F0=9F=99=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../connectivity/HttpClient5OAuth2TokenService.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index 6fdd433ee..5c580d857 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -133,10 +133,10 @@ private String executeRequest( .build(); }); } + catch( final OAuth2ServiceException e ) { + throw e; + } catch( final IOException e ) { - if( e instanceof OAuth2ServiceException ) { - throw (OAuth2ServiceException) e; - } else { final var exception = OAuth2ServiceException .builder("Error requesting access token!") @@ -146,7 +146,6 @@ private String executeRequest( .build(); exception.initCause(e); throw exception; - } } } From 34879f0aed7bb6ad50a865c7935c693dda4cd060 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Thu, 26 Feb 2026 17:35:26 +0100 Subject: [PATCH 13/14] code format --- .../HttpClient5OAuth2TokenService.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java index 5c580d857..b111ad3e8 100644 --- a/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -137,15 +137,15 @@ private String executeRequest( throw e; } catch( final IOException e ) { - final var exception = - OAuth2ServiceException - .builder("Error requesting access token!") - .withUri(tokenUri) - .withRequestHeaders(getHeadersAsStringArray(httpPost.getHeaders())) - .withResponseBody(e.getMessage()) - .build(); - exception.initCause(e); - throw exception; + final var exception = + OAuth2ServiceException + .builder("Error requesting access token!") + .withUri(tokenUri) + .withRequestHeaders(getHeadersAsStringArray(httpPost.getHeaders())) + .withResponseBody(e.getMessage()) + .build(); + exception.initCause(e); + throw exception; } } From 0bb783896c02f5c0e9d203650de5f8ef763b14f8 Mon Sep 17 00:00:00 2001 From: Jonas Israel Date: Tue, 3 Mar 2026 09:59:57 +0100 Subject: [PATCH 14/14] improve test --- .../OAuth2ServiceBindingDestinationLoaderTest.java | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java index f6670b307..2b05514fb 100644 --- a/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java +++ b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/OAuth2ServiceBindingDestinationLoaderTest.java @@ -242,17 +242,9 @@ void testInvalidCertificate() assertThat(result.isFailure()).isTrue(); // The root cause can be either HttpClientException (when using DefaultOAuth2TokenService with HttpClient 4) // or a security exception like CertificateException (when using HttpClient5OAuth2TokenService) - final Throwable rootCause = getRootCause(result.getCause()); - assertThat(rootCause).isInstanceOfAny(HttpClientException.class, java.security.GeneralSecurityException.class); - } - - private static Throwable getRootCause( Throwable throwable ) - { - Throwable cause = throwable; - while( cause.getCause() != null && cause.getCause() != cause ) { - cause = cause.getCause(); - } - return cause; + assertThat(result.getCause()) + .rootCause() + .isInstanceOfAny(HttpClientException.class, java.security.GeneralSecurityException.class); } @Test