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/HttpClient5OAuth2TokenService.java b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java new file mode 100644 index 000000000..b111ad3e8 --- /dev/null +++ b/cloudplatform/connectivity-oauth/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenService.java @@ -0,0 +1,348 @@ +/* + * 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.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.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.message.BasicNameValuePair; +import org.apache.hc.core5.ssl.SSLContextBuilder; +import org.json.JSONObject; + +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; +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; + +import lombok.extern.slf4j.Slf4j; + +/** + * OAuth2 token service implementation using Apache HttpClient 5. + *

+ * This class extends {@link AbstractOAuth2TokenService} and provides the HTTP client specific logic to perform token + * requests using Apache HttpClient 5 instead of HttpClient 4. + */ +@Slf4j +class HttpClient5OAuth2TokenService extends AbstractOAuth2TokenService +{ + private static final char[] EMPTY_PASSWORD = {}; + + 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 HTTP client to use for token requests. + */ + 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. + */ + 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 HttpPost httpPost = createHttpPost(tokenUri, createRequestHeaders(headers), parameters); + 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 token endpoint", statusCode); + if( HttpStatus.SC_OK == statusCode ) { + 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); + 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 OAuth2ServiceException e ) { + 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; + } + } + + 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 ) + { + log.debug("access token request {} - {}", headers, parameters.entrySet().stream().map(e -> { + 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 + { + 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(); + 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 ) { + final var exception = new OAuth2ServiceException("Unexpected error parsing URI: " + e.getMessage()); + exception.initCause(e); + throw exception; + } + logRequest(headers, parameters); + return httpPost; + } + + 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); + 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 + { + try { + return Long.parseLong(expiresIn); + } + catch( final NumberFormatException e ) { + final var exception = + new OAuth2ServiceException( + String.format("Cannot convert expires_in from response (%s) to long", expiresIn)); + exception.initCause(e); + throw exception; + } + } + + 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 { + log.info("Retry again in {} ms", sleepTime); + Thread.sleep(sleepTime); + } + catch( final InterruptedException e ) { + 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 ) { + final var exception = + new HttpClientException( + "Failed to create HTTPS HttpClient5 with certificate authentication: " + e.getMessage()); + exception.initCause(e); + throw exception; + } + } + + @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 ) { + final var exception = + new HttpClientException("Failed to create HTTPS HttpClient5 with KeyStore: " + e.getMessage()); + exception.initCause(e); + throw exception; + } + } +} 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..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,7 +10,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; -import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import com.auth0.jwt.interfaces.DecodedJWT; import com.github.benmanes.caffeine.cache.Cache; @@ -33,10 +33,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 +111,13 @@ private OAuth2TokenService createTokenService( @Nonnull final CacheKey ignored ) tokenCacheParameters.getTokenExpirationDelta(), false); // disable cache statistics - if( !(identity instanceof ZtisClientIdentity) ) { - return new DefaultOAuth2TokenService(HttpClientFactory.create(identity), 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); - 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), - 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(httpClient, tokenCacheConfiguration); } @Nonnull 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..115cb6310 --- /dev/null +++ b/cloudplatform/connectivity-oauth/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/HttpClient5OAuth2TokenServiceTest.java @@ -0,0 +1,695 @@ +/* + * 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.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.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.HashMap; +import java.util.Map; + +import javax.annotation.Nonnull; + +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; +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; +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 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; + +/** + * 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(); + + 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() + throws HttpClientException + { + final CloseableHttpClient result = HttpClient5OAuth2TokenService.createHttpClient(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 = mock(KeyStore.class); + 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(); + } + } + + @Test + @DisplayName( "requestAccessToken should successfully retrieve access token with valid response" ) + void testRequestAccessTokenSuccess() + throws IOException + { + // 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": "test-token", + "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(ClassicHttpRequest.class), anyHttpClientResponseHandler())) + .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"); + 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); + } + }), anyHttpClientResponseHandler()); + } + + @Test + @DisplayName( "requestAccessToken should handle minimal valid response" ) + void testRequestAccessTokenMinimalResponse() + 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"); + + 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), anyHttpClientResponseHandler())).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 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"); + + 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), anyHttpClientResponseHandler())).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 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), anyHttpClientResponseHandler())) + .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 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"); + + 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), anyHttpClientResponseHandler())).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 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"); + + 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), anyHttpClientResponseHandler())).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 IOException + { + // 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), anyHttpClientResponseHandler())).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"); + }), anyHttpClientResponseHandler()); + } + + @Test + @DisplayName( "requestAccessToken should properly handle complex parameters" ) + void testRequestAccessTokenWithComplexParameters() + throws IOException + { + // 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), anyHttpClientResponseHandler())).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; + } + }), anyHttpClientResponseHandler()); + } + + @Test + @DisplayName( "requestAccessToken should handle empty parameters map" ) + void testRequestAccessTokenWithEmptyParameters() + throws IOException + { + // 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), anyHttpClientResponseHandler())).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 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 Exception + { + 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; + } +} 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..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 @@ -240,7 +240,11 @@ 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) + assertThat(result.getCause()) + .rootCause() + .isInstanceOfAny(HttpClientException.class, java.security.GeneralSecurityException.class); } @Test