Repository: SAP/cloud-security-xsuaa-integration
Pull request #953 (2022-07-28 10:04:00)

Bump java-bom from 0.3.2 to 0.4.0
Bumps [java-bom](https://github.com/SAP/btp-environment-variable-access) from 0.3.2 to 0.4.0.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/SAP/btp-environment-variable-access/releases">java-bom's releases</a>.</em></p>
<blockquote>
<h2>Version 0.4.0</h2>
<h2>Fixed Issues</h2>
<ul>
<li><code>java-sap-vcap-services</code>:
<ul>
<li>The <code>SapVcapServicesServiceBindingAccessor</code> is now exposed via the service locator pattern</li>
</ul>
</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/e193df5cad49d0dbc7d65e8a538b77c25a7ec9bf"><code>e193df5</code></a> bump version 0.3.2 -&gt; 0.4.0</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/1974157da7181fd48fa77907669739088235007c"><code>1974157</code></a> Register SapVcapServicesServiceBindingAccessor to ServiceLoader (<a href="https://github-redirect.dependabot.com/SAP/btp-environment-variable-access/issues/38">#38</a>)</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/34001913fa4c3c7eaf2872439e2bb8b9da05da8a"><code>3400191</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/SAP/btp-environment-variable-access/issues/37">#37</a> from SAP/dependabot/maven/junit-jupiter.version-5.9.0</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/4c50368151d8bfcb5e5371ac01e99fe863c33376"><code>4c50368</code></a> Bump junit-jupiter.version from 5.8.2 to 5.9.0</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/dd8163e7c37f47fcb5277fa47ecf906d4bf32af1"><code>dd8163e</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/SAP/btp-environment-variable-access/issues/36">#36</a> from SAP/test-commit-signing-action</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/ec5da0c0f2201e039cf27ec580ed94df1d54fc86"><code>ec5da0c</code></a> Update .github/workflows/release.yml</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/7c31817acec396abf89c27a23f359df3585ca801"><code>7c31817</code></a> Merge branch 'main' into test-commit-signing-action</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/0f7456245599fab6c978015ceba2cadb06a2f01f"><code>0f74562</code></a> bump version 0.3.1 -&gt; 0.3.2</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/7ba512ca2115a07df2116232ba1430a1800bbb1e"><code>7ba512c</code></a> test gpg signing commits</li>
<li><a href="https://github.com/SAP/btp-environment-variable-access/commit/17e2af07953eb1b13a38c879c58b0895b1b8f487"><code>17e2af0</code></a> Merge pull request <a href="https://github-redirect.dependabot.com/SAP/btp-environment-variable-access/issues/35">#35</a> from SAP/adjust-poms-for-release-workflow</li>
<li>See full diff in <a href="https://github.com/SAP/btp-environment-variable-access/compare/0.3.2...0.4.0">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=com.sap.cloud.environment.servicebinding:java-bom&package-manager=maven&previous-version=0.3.2&new-version=0.4.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)


</details>


Patch:
diff --git a/pom.xml b/pom.xml
index 3df93a8b..e46353e6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -66,7 +66,7 @@
 		<log4j2.version>2.18.0</log4j2.version>
 		<slf4j.api.version>1.7.36</slf4j.api.version> <!--see also here http://www.slf4j.org/faq.html#changesInVersion18 -->
 		<org.json.version>20220320</org.json.version>
-		<sap.cloud.env.servicebinding.version>0.3.2</sap.cloud.env.servicebinding.version>
+		<sap.cloud.env.servicebinding.version>0.4.0</sap.cloud.env.servicebinding.version>
 		<google.jsr305.version>3.0.2</google.jsr305.version>
 		<apache.httpclient.version>4.5.13</apache.httpclient.version>
 		<caffeine.version>2.9.3</caffeine.version>
diff --git a/token-client/README.md b/token-client/README.md
index 48ebd5d5..d4fb7b27 100644
--- a/token-client/README.md
+++ b/token-client/README.md
@@ -285,10 +285,12 @@ For spring applications using rest template, you can set
 ### Common Pitfalls
 
 #### New warning `In productive environment provide well configured HttpClientFactory service` 
-As of version ``2.10`` a warning `In productive environment provide well configured HttpClientFactory service` is exposed to the application log in case there is no own `HttpClientFactory` implementation that serves a well-configured ``ClosableRestClient``.
+As of version ``2.10`` a warning `In productive environment provide well configured HttpClientFactory service` is exposed to the application log in case there is no user provided `HttpClientFactory` implementation that serves a well-configured ``CloseableRestClient``.
 
 > Instead of using the default Apache Rest Client config for productive and high available applications we recommend you to customize your http client carefully. You may need to configure the timeouts to specify how long to wait until a connection is established and how long a socket should be kept open (i.e. how long to wait for the (next) data package). As the SSL handshake is time-consuming, it might be recommended to configure an HTTP connection pool to reuse connections by keeping the sockets open. See also [Baeldung: HttpClient Connection Management"](https://www.baeldung.com/httpclient-connection-management).<br>
 
+:bangbang: For your custom `CloseableHttpClient` implementation always disable redirects :bangbang:
+
 In case you like to overwrite [`DefaultHttpClientFactory`](/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java) you can register your own implementation of `HttpClientFactory` interface as following:
 
 - Create a SPI configuration file with name `com.sap.cloud.security.client.HttpClientFactory` in ``src/main/resources/META-INF/services`` directory.  
diff --git a/token-client/pom.xml b/token-client/pom.xml
index 0b1475b7..9a85a4e2 100644
--- a/token-client/pom.xml
+++ b/token-client/pom.xml
@@ -109,7 +109,12 @@
 			<version>2.7.9</version>
 			<scope>test</scope>
 		</dependency>
-	</dependencies>
+        <dependency>
+            <groupId>com.github.tomakehurst</groupId>
+            <artifactId>wiremock-jre8</artifactId>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
 
 	<build>
 		<plugins>
diff --git a/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java b/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
index 62475390..788a3079 100644
--- a/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
+++ b/token-client/src/main/java/com/sap/cloud/security/client/DefaultHttpClientFactory.java
@@ -39,13 +39,14 @@ public class DefaultHttpClientFactory implements HttpClientFactory {
 	static final int MAX_CONNECTIONS_PER_ROUTE = 4; // 2 is default
 	static final int MAX_CONNECTIONS = 20;
 	private static final int DEFAULT_TIMEOUT = (int) TimeUnit.SECONDS.toMillis(5);
-	private final RequestConfig timeoutConfig;
+	private final RequestConfig customConfig;
 
 	public DefaultHttpClientFactory() {
-		timeoutConfig = RequestConfig.custom()
+		customConfig = RequestConfig.custom()
 				.setConnectTimeout(DEFAULT_TIMEOUT)
 				.setConnectionRequestTimeout(DEFAULT_TIMEOUT)
 				.setSocketTimeout(DEFAULT_TIMEOUT)
+				.setRedirectsEnabled(false)
 				.build();
 	}
 
@@ -61,7 +62,7 @@ public class DefaultHttpClientFactory implements HttpClientFactory {
 			SslConnection connectionPool = sslConnectionPool.computeIfAbsent(clientId,
 					s -> new SslConnection(clientIdentity));
 			return HttpClients.custom()
-					.setDefaultRequestConfig(timeoutConfig)
+					.setDefaultRequestConfig(customConfig)
 					.setConnectionManager(connectionPool.poolingConnectionManager)
 					.setSSLContext(connectionPool.context)
 					.setSSLSocketFactory(connectionPool.sslSocketFactory)
@@ -69,7 +70,7 @@ public class DefaultHttpClientFactory implements HttpClientFactory {
 		}
 		LOGGER.warn(
 				"In productive environment provide well configured HttpClientFactory service, don't use default http client");
-		return HttpClients.createDefault();
+		return HttpClients.custom().disableRedirectHandling().build();
 	}
 
 	private static class SslConnection {


Test patch:
diff --git a/token-client/src/test/java/com/sap/cloud/security/client/DefaultHttpClientFactoryTest.java b/token-client/src/test/java/com/sap/cloud/security/client/DefaultHttpClientFactoryTest.java
index ac338607..b54906d7 100644
--- a/token-client/src/test/java/com/sap/cloud/security/client/DefaultHttpClientFactoryTest.java
+++ b/token-client/src/test/java/com/sap/cloud/security/client/DefaultHttpClientFactoryTest.java
@@ -1,11 +1,15 @@
 package com.sap.cloud.security.client;
 
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.sap.cloud.security.config.ClientCredentials;
 import com.sap.cloud.security.config.ClientIdentity;
 import nl.altindag.log.LogCaptor;
 import org.apache.commons.io.IOUtils;
+import org.apache.http.HttpHeaders;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
 import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.impl.client.CloseableHttpClient;
 import org.apache.http.util.EntityUtils;
@@ -17,16 +21,17 @@ import org.mockito.Mockito;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 
+import static com.github.tomakehurst.wiremock.client.WireMock.*;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.when;
 
 class DefaultHttpClientFactoryTest {
 
-	public static final HttpGet HTTP_GET = new HttpGet(java.net.URI.create("https://www.sap.com/"));
-	private static ClientIdentity config = Mockito.mock(ClientIdentity.class);
-	private static ClientIdentity config2 = Mockito.mock(ClientIdentity.class);
-	private DefaultHttpClientFactory cut = new DefaultHttpClientFactory();
+	public static final HttpGet HTTP_GET = new HttpGet(java.net.URI.create("https://www.sap.com/index.html"));
+	private static final ClientIdentity config = Mockito.mock(ClientIdentity.class);
+	private static final ClientIdentity config2 = Mockito.mock(ClientIdentity.class);
+	private final DefaultHttpClientFactory cut = new DefaultHttpClientFactory();
 	private static LogCaptor logCaptor;
 
 	@BeforeAll
@@ -105,7 +110,7 @@ class DefaultHttpClientFactoryTest {
 	void assertWarnWhenCalledMoreThanOnce() {
 		cut.createClient(config);
 		cut.createClient(config2);
-		assertThat(logCaptor.getWarnLogs().size()).isEqualTo(0);
+		assertThat(logCaptor.getWarnLogs()).isEmpty();
 
 		cut.createClient(config);
 		assertThat(logCaptor.getWarnLogs().get(0))
@@ -114,7 +119,7 @@ class DefaultHttpClientFactoryTest {
 		cut.createClient(null);
 		logCaptor.clearLogs();
 		cut.createClient(null);
-		assertThat(logCaptor.getWarnLogs().size()).isEqualTo(2);
+		assertThat(logCaptor.getWarnLogs()).hasSize(2);
 		assertThat(logCaptor.getWarnLogs().get(0))
 				.startsWith("Application has already created HttpClient for clientId = null, please check.");
 	}
@@ -123,4 +128,24 @@ class DefaultHttpClientFactoryTest {
 		return IOUtils.resourceToString(file, StandardCharsets.UTF_8);
 	}
 
+	@Test
+	void disableRedirects() throws IOException {
+		WireMockServer wireMockServer = new WireMockServer(8000);
+		wireMockServer.stubFor(get(urlEqualTo("/redirect"))
+				.willReturn(aResponse().withHeader(HttpHeaders.LOCATION, "https://sap.com").withStatus(HttpStatus.SC_MOVED_PERMANENTLY))
+		);
+		wireMockServer.start();
+		try {
+			CloseableHttpClient client = cut.createClient(config);
+			CloseableHttpResponse resp = client.execute(new HttpGet("http://localhost:8000/redirect"));
+			assertThat(resp.getStatusLine().getStatusCode()).isEqualTo(301);
+
+			CloseableHttpClient client2 = cut.createClient(new ClientCredentials("client", "secret"));
+			CloseableHttpResponse resp2 = client2.execute(new HttpGet("http://localhost:8000/redirect"));
+			assertThat(resp2.getStatusLine().getStatusCode()).isEqualTo(301);
+		} finally {
+			wireMockServer.stop();
+		}
+	}
+
 }
\ No newline at end of file