Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests after Apache HttpClient5 / HttpCore5 update since those use deprecated APIs #5035

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Fix tests after Apache HttpClient5 / HttpCore5 update since those use deprecated APIs #5035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/KeySetRetriever.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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.core5.http.HttpEntity;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -223,7 +224,7 @@ private CloseableHttpClient createHttpClient(HttpCacheStorage httpCacheStorage)

if (sslConfig != null) {
final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(sslConfig.toSSLConnectionSocketFactory())
.setTlsSocketStrategy(new DefaultClientTlsStrategy(sslConfig.getSslContext()))
.build();

builder.setConnectionManager(cm);
Expand Down
30 changes: 15 additions & 15 deletions src/main/java/org/opensearch/security/auditlog/sink/WebhookSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@
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.DefaultHostnameVerifier;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
import org.apache.hc.client5.http.ssl.TrustAllStrategy;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.io.SocketConfig;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
import org.apache.hc.core5.ssl.SSLContextBuilder;
import org.apache.hc.core5.ssl.TrustStrategy;
import org.apache.http.HttpStatus;

import org.opensearch.common.settings.Settings;
Expand Down Expand Up @@ -368,27 +369,20 @@ CloseableHttpClient getHttpClient() {
.setConnectionRequestTimeout(timeout, TimeUnit.SECONDS)
.build();

final TrustStrategy trustAllStrategy = new TrustStrategy() {
@Override
public boolean isTrusted(X509Certificate[] chain, String authType) {
return true;
}
};

try {

HttpClientBuilder hcb = HttpClients.custom().setDefaultRequestConfig(config);
if (!verifySSL) {
SSLContext sslContext = SSLContextBuilder.create().loadTrustMaterial(trustAllStrategy).build();
final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(
SSLContext sslContext = SSLContextBuilder.create().loadTrustMaterial(TrustAllStrategy.INSTANCE).build();
final DefaultClientTlsStrategy sslsf = new DefaultClientTlsStrategy(
sslContext,
null,
null,
SSLBufferMode.STATIC,
NoopHostnameVerifier.INSTANCE
);

final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(sslsf)
.setTlsSocketStrategy(sslsf)
.setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeout, TimeUnit.SECONDS).build())
.build();
hcb.setConnectionManager(cm);
Expand All @@ -399,10 +393,16 @@ public boolean isTrusted(X509Certificate[] chain, String authType) {
return HttpClients.custom().setDefaultRequestConfig(config).build();
}
SSLContext sslContext = SSLContextBuilder.create().loadTrustMaterial(effectiveTruststore, null).build();
final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory(sslContext, null, null, new DefaultHostnameVerifier());
final DefaultClientTlsStrategy sslsf = new DefaultClientTlsStrategy(
sslContext,
null,
null,
SSLBufferMode.STATIC,
new DefaultHostnameVerifier()
);

final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create()
.setSSLSocketFactory(sslsf)
.setTlsSocketStrategy(sslsf)
.setDefaultSocketConfig(SocketConfig.custom().setSoTimeout(timeout, TimeUnit.SECONDS).build())
.build();
hcb.setConnectionManager(cm);
Expand Down
26 changes: 9 additions & 17 deletions src/main/java/org/opensearch/security/httpclient/HttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,21 @@
import java.util.stream.Collectors;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;

import com.google.common.collect.Lists;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.core5.function.Factory;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.message.BasicHeader;
import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
import org.apache.hc.core5.reactor.ssl.TlsDetails;
import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
import org.apache.hc.core5.ssl.PrivateKeyDetails;
import org.apache.hc.core5.ssl.PrivateKeyStrategy;
import org.apache.hc.core5.ssl.SSLContextBuilder;
Expand Down Expand Up @@ -280,19 +278,13 @@ public String chooseAlias(Map<String, PrivateKeyDetails> aliases, SSLParameters
final HostnameVerifier hnv = verifyHostnames ? new DefaultHostnameVerifier() : NoopHostnameVerifier.INSTANCE;

final SSLContext sslContext = sslContextBuilder.build();
TlsStrategy tlsStrategy = ClientTlsStrategyBuilder.create()
.setSslContext(sslContext)
.setTlsVersions(supportedProtocols)
.setCiphers(supportedCipherSuites)
.setHostnameVerifier(hnv)
// See please https://issues.apache.org/jira/browse/HTTPCLIENT-2219
.setTlsDetailsFactory(new Factory<SSLEngine, TlsDetails>() {
@Override
public TlsDetails create(final SSLEngine sslEngine) {
return new TlsDetails(sslEngine.getSession(), sslEngine.getApplicationProtocol());
}
})
.build();
final TlsStrategy tlsStrategy = new DefaultClientTlsStrategy(
sslContext,
supportedProtocols,
supportedCipherSuites,
SSLBufferMode.STATIC,
hnv
);

final AsyncClientConnectionManager cm = PoolingAsyncClientConnectionManagerBuilder.create().setTlsStrategy(tlsStrategy).build();
httpClientBuilder.setConnectionManager(cm);
Expand Down
39 changes: 21 additions & 18 deletions src/test/java/com/amazon/dlic/auth/http/jwt/keybyoidc/MockIpdServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,27 @@ class MockIpdServer implements Closeable {
this.ssl = ssl;
this.jwks = jwks;

ServerBootstrap serverBootstrap = ServerBootstrap.bootstrap()
.setListenerPort(port)
.register(CTX_DISCOVER, new HttpRequestHandler() {

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleDiscoverRequest(request, response, context);
}
})
.register(CTX_KEYS, new HttpRequestHandler() {

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleKeysRequest(request, response, context);
}
});
ServerBootstrap serverBootstrap = ServerBootstrap.bootstrap().setListenerPort(port).setRequestRouter((request, context) -> {
if (request.getRequestUri().startsWith(CTX_DISCOVER)) {
return new HttpRequestHandler() {
@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleDiscoverRequest(request, response, context);
}
};
} else if (request.getRequestUri().startsWith(CTX_KEYS)) {
return new HttpRequestHandler() {
@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleKeysRequest(request, response, context);
}
};
} else {
return null;
}
});

if (ssl) {
serverBootstrap = serverBootstrap.setSslContext(createSSLContext()).setSslSetupHandler(new Callback<SSLParameters>() {
Expand Down
57 changes: 29 additions & 28 deletions src/test/java/com/amazon/dlic/auth/http/saml/MockSamlIdpServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,34 +195,35 @@ class MockSamlIdpServer implements Closeable {

this.loadSigningKeys("saml/kirk-keystore.jks", "kirk");

ServerBootstrap serverBootstrap = ServerBootstrap.bootstrap()
.setListenerPort(port)
.register(CTX_METADATA, new HttpRequestHandler() {

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {

handleMetadataRequest(request, response, context);

}
})
.register(CTX_SAML_SSO, new HttpRequestHandler() {

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleSsoRequest(request, response, context);
}
})
.register(CTX_SAML_SLO, new HttpRequestHandler() {

@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleSloRequest(request, response, context);
}
});
ServerBootstrap serverBootstrap = ServerBootstrap.bootstrap().setListenerPort(port).setRequestRouter((request, context) -> {
if (request.getRequestUri().startsWith(CTX_METADATA)) {
return new HttpRequestHandler() {
@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleMetadataRequest(request, response, context);
}
};
} else if (request.getRequestUri().startsWith(CTX_SAML_SSO)) {
return new HttpRequestHandler() {
@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleSsoRequest(request, response, context);
}
};
} else if (request.getRequestUri().startsWith(CTX_SAML_SLO)) {
return new HttpRequestHandler() {
@Override
public void handle(ClassicHttpRequest request, ClassicHttpResponse response, HttpContext context) throws HttpException,
IOException {
handleSloRequest(request, response, context);
}
};
} else {
return null;
}
});

if (ssl) {

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/security/InitializationIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public void testWhoAmIForceHttp1() throws Exception {
Response whoAmIRes = restHighLevelClient.getLowLevelClient().performRequest(new Request("GET", "/_plugins/_security/whoami"));
assertThat(200, is(whoAmIRes.getStatusLine().getStatusCode()));
// The HTTP/1.1 is forced and should be used instead
assertThat(HttpVersion.HTTP_1_1, is(whoAmIRes.getStatusLine().getProtocolVersion()));
assertThat(whoAmIRes.getStatusLine().getProtocolVersion(), is(HttpVersion.HTTP_1_1));
cwperks marked this conversation as resolved.
Show resolved Hide resolved
JsonNode whoAmIResNode = DefaultObjectMapper.objectMapper.readTree(whoAmIRes.getEntity().getContent());
String whoAmIResponsePayload = whoAmIResNode.toPrettyString();
assertThat(whoAmIResponsePayload, whoAmIResNode.get("dn").asText(), is("CN=spock,OU=client,O=client,L=Test,C=DE"));
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/security/auditlog/sink/SinkProviderTLSTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void testTlsConfigurationNoFallback() throws Exception {
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down
20 changes: 10 additions & 10 deletions src/test/java/org/opensearch/security/auditlog/sink/WebhookAuditLogTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public void postGetHttpTest() throws Exception {
server = ServerBootstrap.bootstrap()
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -355,7 +355,7 @@ public void httpsTestWithoutTLSServer() throws Exception {
server = ServerBootstrap.bootstrap()
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.register("*", handler)
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -393,8 +393,8 @@ public void httpsTest() throws Exception {
server = ServerBootstrap.bootstrap()
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setServerSocketFactory(createSSLContext().getServerSocketFactory())
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -481,8 +481,8 @@ public void httpsTestPemDefault() throws Exception {
server = ServerBootstrap.bootstrap()
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setServerSocketFactory(createSSLContext().getServerSocketFactory())
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -610,8 +610,8 @@ public void httpsTestPemEndpoint() throws Exception {
server = ServerBootstrap.bootstrap()
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setServerSocketFactory(createSSLContext().getServerSocketFactory())
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down Expand Up @@ -717,8 +717,8 @@ public void httpsTestPemContentEndpoint() throws Exception {
server = ServerBootstrap.bootstrap()
.setListenerPort(port)
.setHttpProcessor(HttpProcessors.server("Test/1.1"))
.setSslContext(createSSLContext())
.register("*", handler)
.setServerSocketFactory(createSSLContext().getServerSocketFactory())
.setRequestRouter((request, context) -> handler)
.create();

server.start();
Expand Down
13 changes: 6 additions & 7 deletions src/test/java/org/opensearch/security/test/AbstractSecurityUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope.Scope;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import org.apache.hc.client5.http.config.TlsConfig;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder;
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
import org.apache.hc.core5.function.Factory;
Expand Down Expand Up @@ -197,14 +197,13 @@ public TlsDetails create(final SSLEngine sslEngine) {
})
.build();

final AsyncClientConnectionManager cm = PoolingAsyncClientConnectionManagerBuilder.create()
.setTlsStrategy(tlsStrategy)
.build();
builder.setConnectionManager(cm);
final PoolingAsyncClientConnectionManagerBuilder cm = PoolingAsyncClientConnectionManagerBuilder.create()
.setTlsStrategy(tlsStrategy);

if (httpVersionPolicy != null) {
builder.setVersionPolicy(httpVersionPolicy);
cm.setDefaultTlsConfig(TlsConfig.custom().setVersionPolicy(httpVersionPolicy).build());
}
return builder;
return builder.setConnectionManager(cm.build());
});
return new RestHighLevelClient(restClientBuilder);
} catch (Exception e) {
Expand Down
4 changes: 3 additions & 1 deletion src/test/java/org/opensearch/security/test/helper/rest/RestHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,9 @@ public TlsDetails create(final SSLEngine sslEngine) {
hcb.setConnectionManager(cm);
}

final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom().setResponseTimeout(Timeout.ofSeconds(60));
final RequestConfig.Builder requestConfigBuilder = RequestConfig.custom()
.setResponseTimeout(Timeout.ofSeconds(60))
.setProtocolUpgradeEnabled(false);
cwperks marked this conversation as resolved.
Show resolved Hide resolved

return hcb.setDefaultRequestConfig(requestConfigBuilder.build()).disableAutomaticRetries().build();
}
Expand Down
Loading