Skip to content

Commit f3d2431

Browse files
strehlestrehle
authored
Fix duplicate bean definition of rest templates -Alternative to PR #3430 (#3434)
* revert single use of RestTemplate in proxy calls Re-establish the beans and usage fromm 77.20.x and earlier * Refactor * simplify * Fix duplicate bean definition of rest templates before we had duplicates of: - nonTrustingRestTemplate - trustingRestTemplate * Add new options
1 parent 4f9cd08 commit f3d2431

File tree

6 files changed

+63
-60
lines changed

6 files changed

+63
-60
lines changed

‎server/src/main/java/org/cloudfoundry/identity/uaa/impl/config/RestTemplateConfig.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,20 @@ public class RestTemplateConfig {
2020
@Value("${rest.template.maxKeepAlive:0}")
2121
public int maxKeepAlive;
2222

23+
@Value("${rest.template.validateAfterInactivity:2000}")
24+
public int validateAfterInactivity;
25+
26+
@Value("${rest.template.retryCount:0}")
27+
public int retryCount;
28+
2329
@Bean
2430
public RestTemplate nonTrustingRestTemplate() {
25-
return new RestTemplate(UaaHttpRequestUtils.createRequestFactory(false, timeout, maxTotal, maxPerRoute, maxKeepAlive));
31+
return new RestTemplate(UaaHttpRequestUtils.createRequestFactory(false, timeout, timeout, maxTotal, maxPerRoute, maxKeepAlive, validateAfterInactivity, retryCount));
2632
}
2733

2834
@Bean
2935
public RestTemplate trustingRestTemplate() {
30-
return new RestTemplate(UaaHttpRequestUtils.createRequestFactory(true, timeout, maxTotal, maxPerRoute, maxKeepAlive));
36+
return new RestTemplate(UaaHttpRequestUtils.createRequestFactory(true, timeout, timeout, maxTotal, maxPerRoute, maxKeepAlive, validateAfterInactivity, retryCount));
3137
}
3238

3339
public static RestTemplateConfig createDefaults() {
@@ -36,6 +42,8 @@ public static RestTemplateConfig createDefaults() {
3642
restTemplateConfig.maxTotal = 10;
3743
restTemplateConfig.maxPerRoute = 5;
3844
restTemplateConfig.maxKeepAlive = 0;
45+
restTemplateConfig.validateAfterInactivity = 2000;
46+
restTemplateConfig.retryCount = 0;
3947
return restTemplateConfig;
4048
}
4149
}

‎server/src/main/java/org/cloudfoundry/identity/uaa/provider/saml/SamlConfiguration.java

Lines changed: 9 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,21 @@
22

33
import lombok.Data;
44
import lombok.extern.slf4j.Slf4j;
5-
import org.apache.http.conn.ssl.NoopHostnameVerifier;
6-
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
7-
import org.apache.http.impl.client.CloseableHttpClient;
8-
import org.apache.http.impl.client.HttpClients;
9-
import org.apache.http.ssl.TrustStrategy;
105
import org.cloudfoundry.identity.uaa.cache.StaleUrlCache;
116
import org.cloudfoundry.identity.uaa.cache.UrlContentCache;
7+
import org.cloudfoundry.identity.uaa.impl.config.RestTemplateConfig;
128
import org.cloudfoundry.identity.uaa.util.TimeService;
139
import org.cloudfoundry.identity.uaa.util.TimeServiceImpl;
10+
import org.cloudfoundry.identity.uaa.util.UaaHttpRequestUtils;
1411
import org.springframework.beans.factory.annotation.Autowired;
1512
import org.springframework.beans.factory.annotation.Qualifier;
1613
import org.springframework.beans.factory.annotation.Value;
1714
import org.springframework.boot.context.properties.EnableConfigurationProperties;
18-
import org.springframework.boot.web.client.RestTemplateBuilder;
1915
import org.springframework.context.annotation.Bean;
2016
import org.springframework.context.annotation.Configuration;
21-
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
17+
import org.springframework.http.client.ClientHttpRequestFactory;
2218
import org.springframework.web.client.RestTemplate;
2319

24-
import javax.net.ssl.SSLContext;
25-
import java.security.KeyManagementException;
26-
import java.security.KeyStoreException;
27-
import java.security.NoSuchAlgorithmException;
28-
import java.time.Duration;
29-
3020
@Slf4j
3121
@EnableConfigurationProperties({SamlConfigProps.class})
3222
@Configuration
@@ -126,38 +116,14 @@ public UrlContentCache urlContentCache(TimeService timeService) {
126116
return new StaleUrlCache(timeService);
127117
}
128118

129-
@Bean
130-
public RestTemplate trustingRestTemplate() throws NoSuchAlgorithmException, KeyStoreException, KeyManagementException {
131-
// skip ssl validation
132-
TrustStrategy acceptingTrustStrategy = (x509Certificates, s) -> true;
133-
SSLContext sslContext = org.apache.http.ssl.SSLContexts.custom().loadTrustMaterial(null, acceptingTrustStrategy).build();
134-
SSLConnectionSocketFactory csf = new SSLConnectionSocketFactory(sslContext, new NoopHostnameVerifier());
135-
CloseableHttpClient httpClient = HttpClients.custom().setSSLSocketFactory(csf).build();
136-
HttpComponentsClientHttpRequestFactory requestFactory = new HttpComponentsClientHttpRequestFactory();
137-
requestFactory.setHttpClient(httpClient);
138-
139-
RestTemplateBuilder restTemplateBuilder = new RestTemplateBuilder();
140-
return restTemplateBuilder
141-
.setConnectTimeout(Duration.ofMillis(socketConnectionTimeout))
142-
.setReadTimeout(Duration.ofMillis(socketReadTimeout))
143-
.requestFactory(() -> requestFactory)
144-
.build();
145-
}
146-
147-
@Bean
148-
public RestTemplate nonTrustingRestTemplate() {
149-
RestTemplateBuilder restTemplateBuilder = new RestTemplateBuilder();
150-
return restTemplateBuilder
151-
.setConnectTimeout(Duration.ofMillis(socketConnectionTimeout))
152-
.setReadTimeout(Duration.ofMillis(socketReadTimeout))
153-
.build();
154-
}
155-
156119
@Autowired
157120
@Bean
158-
public FixedHttpMetaDataProvider fixedHttpMetaDataProvider(@Qualifier("trustingRestTemplate") RestTemplate trustingRestTemplate,
159-
@Qualifier("nonTrustingRestTemplate") RestTemplate nonTrustingRestTemplate,
121+
public FixedHttpMetaDataProvider fixedHttpMetaDataProvider(
122+
@Qualifier("restTemplateConfig") RestTemplateConfig restTemplateConfig,
160123
UrlContentCache urlContentCache) {
161-
return new FixedHttpMetaDataProvider(trustingRestTemplate, nonTrustingRestTemplate, urlContentCache);
124+
// create SAML custom configuration, because of own timeout settings
125+
ClientHttpRequestFactory trustingRequestFactory = UaaHttpRequestUtils.createRequestFactory(true, socketConnectionTimeout, socketReadTimeout, restTemplateConfig);
126+
ClientHttpRequestFactory nonTrustingRequestFactory = UaaHttpRequestUtils.createRequestFactory(false, socketConnectionTimeout, socketReadTimeout, restTemplateConfig);
127+
return new FixedHttpMetaDataProvider(new RestTemplate(trustingRequestFactory), new RestTemplate(nonTrustingRequestFactory), urlContentCache);
162128
}
163129
}

‎server/src/main/java/org/cloudfoundry/identity/uaa/util/UaaHttpRequestUtils.java

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.apache.http.HeaderElement;
1818
import org.apache.http.HeaderElementIterator;
1919
import org.apache.http.HttpResponse;
20+
import org.apache.http.client.HttpRequestRetryHandler;
2021
import org.apache.http.config.Registry;
2122
import org.apache.http.config.RegistryBuilder;
2223
import org.apache.http.conn.ConnectionKeepAliveStrategy;
@@ -28,6 +29,7 @@
2829
import org.apache.http.protocol.HTTP;
2930
import org.apache.http.protocol.HttpContext;
3031
import org.apache.http.util.TextUtils;
32+
import org.cloudfoundry.identity.uaa.impl.config.RestTemplateConfig;
3133
import org.slf4j.Logger;
3234
import org.slf4j.LoggerFactory;
3335
import org.apache.http.conn.ssl.NoopHostnameVerifier;
@@ -45,6 +47,7 @@
4547
import javax.net.ssl.HostnameVerifier;
4648
import javax.net.ssl.SSLContext;
4749
import javax.servlet.http.HttpServletRequest;
50+
import java.io.IOException;
4851
import java.net.URLEncoder;
4952
import java.nio.charset.StandardCharsets;
5053
import java.security.KeyManagementException;
@@ -63,23 +66,27 @@ public abstract class UaaHttpRequestUtils {
6366
private static final Logger logger = LoggerFactory.getLogger(UaaHttpRequestUtils.class);
6467

6568
public static ClientHttpRequestFactory createRequestFactory(boolean skipSslValidation, int timeout) {
66-
return createRequestFactory(getClientBuilder(skipSslValidation, 10, 5, 0), timeout);
69+
return createRequestFactory(getClientBuilder(skipSslValidation, 10, 5, 0, 2000, 0), timeout, timeout);
6770
}
6871

69-
public static ClientHttpRequestFactory createRequestFactory(boolean skipSslValidation, int timeout, int poolSize, int defaultMaxPerRoute, int maxKeepAlive) {
70-
return createRequestFactory(getClientBuilder(skipSslValidation, poolSize, defaultMaxPerRoute, maxKeepAlive), timeout);
72+
public static ClientHttpRequestFactory createRequestFactory(boolean skipSslValidation, int timeout, int readTimeout, int poolSize, int defaultMaxPerRoute, int maxKeepAlive, int validateAfterInactivity, int retryCount) {
73+
return createRequestFactory(getClientBuilder(skipSslValidation, poolSize, defaultMaxPerRoute, maxKeepAlive, validateAfterInactivity, retryCount), timeout, readTimeout);
7174
}
7275

73-
protected static ClientHttpRequestFactory createRequestFactory(HttpClientBuilder builder, int timeoutInMs) {
76+
public static ClientHttpRequestFactory createRequestFactory(boolean skipSslValidation, int timeout, int readTimeout, RestTemplateConfig restTemplateConfig) {
77+
return createRequestFactory(getClientBuilder(skipSslValidation, restTemplateConfig.maxTotal, restTemplateConfig.maxPerRoute, restTemplateConfig.maxKeepAlive, restTemplateConfig.validateAfterInactivity, restTemplateConfig.retryCount), timeout, readTimeout);
78+
}
79+
80+
protected static ClientHttpRequestFactory createRequestFactory(HttpClientBuilder builder, int timeoutInMs, int readTimeoutInMs) {
7481
HttpComponentsClientHttpRequestFactory httpComponentsClientHttpRequestFactory = new HttpComponentsClientHttpRequestFactory(builder.build());
7582

76-
httpComponentsClientHttpRequestFactory.setReadTimeout(timeoutInMs);
83+
httpComponentsClientHttpRequestFactory.setReadTimeout(readTimeoutInMs);
7784
httpComponentsClientHttpRequestFactory.setConnectionRequestTimeout(timeoutInMs);
7885
httpComponentsClientHttpRequestFactory.setConnectTimeout(timeoutInMs);
7986
return httpComponentsClientHttpRequestFactory;
8087
}
8188

82-
protected static HttpClientBuilder getClientBuilder(boolean skipSslValidation, int poolSize, int defaultMaxPerRoute, int maxKeepAlive) {
89+
protected static HttpClientBuilder getClientBuilder(boolean skipSslValidation, int poolSize, int defaultMaxPerRoute, int maxKeepAlive, int validateAfterInactivity, int retryCount) {
8390
HttpClientBuilder builder = HttpClients.custom()
8491
.useSystemProperties()
8592
.setRedirectStrategy(new DefaultRedirectStrategy());
@@ -100,6 +107,7 @@ protected static HttpClientBuilder getClientBuilder(boolean skipSslValidation, i
100107
}
101108
cm.setMaxTotal(poolSize);
102109
cm.setDefaultMaxPerRoute(defaultMaxPerRoute);
110+
cm.setValidateAfterInactivity(validateAfterInactivity);
103111
builder.setConnectionManager(cm);
104112

105113
if (maxKeepAlive <= 0) {
@@ -108,6 +116,10 @@ protected static HttpClientBuilder getClientBuilder(boolean skipSslValidation, i
108116
builder.setKeepAliveStrategy(new UaaConnectionKeepAliveStrategy(maxKeepAlive));
109117
}
110118

119+
if (retryCount > 0) {
120+
builder.setRetryHandler(new UaaHttpRequestRetryHandler(retryCount));
121+
}
122+
111123
return builder;
112124
}
113125

@@ -178,6 +190,26 @@ public long getKeepAliveDuration(HttpResponse httpResponse, HttpContext httpCont
178190
}
179191
}
180192

193+
private static class UaaHttpRequestRetryHandler implements HttpRequestRetryHandler {
194+
195+
private final int executionCount;
196+
197+
public UaaHttpRequestRetryHandler(int executionCount) {
198+
this.executionCount = executionCount;
199+
}
200+
201+
@Override
202+
public boolean retryRequest(IOException exception, int executionCount, HttpContext context) {
203+
if (executionCount > this.executionCount) {
204+
return false;
205+
}
206+
if (exception instanceof org.apache.http.NoHttpResponseException) {
207+
return true;
208+
}
209+
return false;
210+
}
211+
}
212+
181213
@SuppressWarnings("java:S1168")
182214
private static String[] split(final String s) {
183215
if (TextUtils.isBlank(s)) {

‎server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/Saml2BearerGrantAuthenticationConverterTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.fasterxml.jackson.databind.ObjectMapper;
44
import net.shibboleth.utilities.java.support.xml.SerializeSupport;
5+
import org.cloudfoundry.identity.uaa.impl.config.RestTemplateConfig;
56
import org.cloudfoundry.identity.uaa.provider.JdbcIdentityProviderProvisioning;
67
import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManager;
78
import org.cloudfoundry.identity.uaa.zone.beans.IdentityZoneManagerImpl;
@@ -42,7 +43,6 @@
4243
import org.springframework.security.saml2.provider.service.registration.Saml2MessageBinding;
4344
import org.springframework.security.saml2.provider.service.web.RelyingPartyRegistrationResolver;
4445
import org.springframework.util.StringUtils;
45-
import org.springframework.web.client.RestTemplate;
4646
import org.w3c.dom.Element;
4747

4848
import javax.xml.namespace.QName;
@@ -77,12 +77,11 @@ class Saml2BearerGrantAuthenticationConverterTest {
7777
@BeforeEach
7878
void beforeEach() {
7979
IdentityZoneManager identityZoneManager = new IdentityZoneManagerImpl();
80-
RestTemplate restTemplate = new RestTemplate();
8180
SamlConfiguration samlConfiguration = new SamlConfiguration();
8281
JdbcIdentityProviderProvisioning providerProvisioning = mock(JdbcIdentityProviderProvisioning.class);
8382

8483
SamlIdentityProviderConfigurator identityProviderConfigurator = new SamlIdentityProviderConfigurator(
85-
providerProvisioning, identityZoneManager, samlConfiguration.fixedHttpMetaDataProvider(restTemplate, restTemplate, null)
84+
providerProvisioning, identityZoneManager, samlConfiguration.fixedHttpMetaDataProvider(RestTemplateConfig.createDefaults(), null)
8685
);
8786
SamlRelyingPartyRegistrationRepositoryConfig samlRelyingPartyRegistrationRepositoryConfig =
8887
new SamlRelyingPartyRegistrationRepositoryConfig(

‎server/src/test/java/org/cloudfoundry/identity/uaa/provider/saml/SamlIdentityProviderConfiguratorTests.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.apache.http.conn.ConnectTimeoutException;
1919
import org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider;
2020
import org.cloudfoundry.identity.uaa.cache.UrlContentCache;
21+
import org.cloudfoundry.identity.uaa.impl.config.RestTemplateConfig;
2122
import org.cloudfoundry.identity.uaa.oauth.common.util.RandomValueStringGenerator;
2223
import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
2324
import org.cloudfoundry.identity.uaa.provider.IdentityProviderProvisioning;
@@ -34,7 +35,6 @@
3435
import org.mockito.junit.jupiter.MockitoExtension;
3536
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistration;
3637
import org.springframework.web.client.ResourceAccessException;
37-
import org.springframework.web.client.RestTemplate;
3838

3939
import java.net.SocketTimeoutException;
4040
import java.security.KeyManagementException;
@@ -227,11 +227,9 @@ void returnNoIdpsInZoneForClientWithNoAllowedProviders() {
227227
}
228228

229229
FixedHttpMetaDataProvider createNonMockFixedHttpMetaDataProvider(SamlConfiguration samlConfiguration) throws NoSuchAlgorithmException, KeyStoreException, KeyManagementException {
230-
RestTemplate trustingRestTemplate = samlConfiguration.trustingRestTemplate();
231-
RestTemplate nonTrustingRestTemplate = samlConfiguration.nonTrustingRestTemplate();
232230
UrlContentCache urlContentCache = samlConfiguration.urlContentCache(samlConfiguration.timeService());
233231

234-
return samlConfiguration.fixedHttpMetaDataProvider(trustingRestTemplate, nonTrustingRestTemplate, urlContentCache);
232+
return samlConfiguration.fixedHttpMetaDataProvider(RestTemplateConfig.createDefaults(), urlContentCache);
235233
}
236234

237235
@Test

‎server/src/test/java/org/cloudfoundry/identity/uaa/util/UaaHttpRequestUtilsTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,11 @@ void httpsIpProxy() {
128128
}
129129

130130
public void testHttpProxy(String url, int expectedPort, String expectedHost, boolean wantHandlerInvoked) {
131-
HttpClientBuilder builder = UaaHttpRequestUtils.getClientBuilder(true, 20, 2, 5);
131+
HttpClientBuilder builder = UaaHttpRequestUtils.getClientBuilder(true, 20, 2, 5, 2000, 2);
132132
HttpRoutePlanner planner = (HttpRoutePlanner) ReflectionTestUtils.getField(builder.build(), "routePlanner");
133133
SystemProxyRoutePlanner routePlanner = new SystemProxyRoutePlanner(planner);
134134
builder.setRoutePlanner(routePlanner);
135-
RestTemplate template = new RestTemplate(UaaHttpRequestUtils.createRequestFactory(builder, Integer.MAX_VALUE));
135+
RestTemplate template = new RestTemplate(UaaHttpRequestUtils.createRequestFactory(builder, Integer.MAX_VALUE, Integer.MAX_VALUE));
136136
try {
137137
template.getForObject(url, String.class);
138138
} catch (Exception ignored) {

0 commit comments

Comments
 (0)