From 55048867a2005e48adeab24df9e223cb0c4db9fb Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 2 Mar 2023 09:27:57 +0100 Subject: [PATCH] Allow reuse of library-specific configuration code in `ClientHttpRequestFactoryFactory` and `ClientHttpConnectorFactory`. See gh-760 --- .../client/ClientHttpConnectorFactory.java | 104 +++++++++++------- .../ClientHttpRequestFactoryFactory.java | 80 ++++++++++---- ...tHttpConnectorFactoryIntegrationTests.java | 6 +- ...ValueMetadataTemplateIntegrationTests.java | 2 +- 4 files changed, 128 insertions(+), 64 deletions(-) diff --git a/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpConnectorFactory.java b/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpConnectorFactory.java index 13db6f27..b185118a 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpConnectorFactory.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpConnectorFactory.java @@ -25,6 +25,7 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import reactor.netty.http.Http11SslContextSpec; import reactor.netty.http.client.HttpClient; +import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.reactive.ClientHttpConnector; import org.springframework.http.client.reactive.JettyClientHttpConnector; import org.springframework.http.client.reactive.ReactorClientHttpConnector; @@ -81,42 +82,20 @@ public class ClientHttpConnectorFactory { Assert.notNull(options, "ClientOptions must not be null"); Assert.notNull(sslConfiguration, "SslConfiguration must not be null"); - if (REACTOR_NETTY_PRESENT) { - return ReactorNetty.usingReactorNetty(options, sslConfiguration); - } - - if (JETTY_PRESENT) { - return JettyClient.usingJetty(options, sslConfiguration); - } - - throw new IllegalStateException("No supported Reactive Http Client library available (Reactor Netty, Jetty)"); - } - - private static void configureSsl(SslConfiguration sslConfiguration, SslContextBuilder sslContextBuilder) { - try { - - if (sslConfiguration.getTrustStoreConfiguration().isPresent()) { - sslContextBuilder - .trustManager(createTrustManagerFactory(sslConfiguration.getTrustStoreConfiguration())); + if (REACTOR_NETTY_PRESENT) { + return ReactorNetty.usingReactorNetty(options, sslConfiguration); } - if (sslConfiguration.getKeyStoreConfiguration().isPresent()) { - sslContextBuilder.keyManager(createKeyManagerFactory(sslConfiguration.getKeyStoreConfiguration(), - sslConfiguration.getKeyConfiguration())); - } - - if (!sslConfiguration.getEnabledProtocols().isEmpty()) { - sslContextBuilder.protocols(sslConfiguration.getEnabledProtocols()); - } - - if (!sslConfiguration.getEnabledCipherSuites().isEmpty()) { - sslContextBuilder.ciphers(sslConfiguration.getEnabledCipherSuites()); + if (JETTY_PRESENT) { + return JettyClient.usingJetty(options, sslConfiguration); } } catch (GeneralSecurityException | IOException e) { throw new IllegalStateException(e); } + + throw new IllegalStateException("No supported Reactive Http Client library available (Reactor Netty, Jetty)"); } /** @@ -124,9 +103,21 @@ public class ClientHttpConnectorFactory { * * @author Mark Paluch */ - static class ReactorNetty { + public static class ReactorNetty { + + /** + * Create a {@link ClientHttpConnector} using Reactor Netty. + * @param options must not be {@literal null} + * @param sslConfiguration must not be {@literal null} + * @return a new and configured {@link ReactorClientHttpConnector} instance. + */ + public static ReactorClientHttpConnector usingReactorNetty(ClientOptions options, + SslConfiguration sslConfiguration) { + return new ReactorClientHttpConnector(createClient(options, sslConfiguration)); + } + + public static HttpClient createClient(ClientOptions options, SslConfiguration sslConfiguration) { - static ClientHttpConnector usingReactorNetty(ClientOptions options, SslConfiguration sslConfiguration) { HttpClient client = HttpClient.create(); if (hasSslConfiguration(sslConfiguration)) { @@ -140,24 +131,59 @@ public class ClientHttpConnectorFactory { client = client.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, Math.toIntExact(options.getConnectionTimeout().toMillis())).proxyWithSystemProperties(); - return new ReactorClientHttpConnector(client); + return client; } - } - - static class JettyClient { - - static ClientHttpConnector usingJetty(ClientOptions options, SslConfiguration sslConfiguration) { + private static void configureSsl(SslConfiguration sslConfiguration, SslContextBuilder sslContextBuilder) { try { - return new JettyClientHttpConnector(configureClient(getHttpClient(sslConfiguration), options)); + + if (sslConfiguration.getTrustStoreConfiguration().isPresent()) { + sslContextBuilder + .trustManager(createTrustManagerFactory(sslConfiguration.getTrustStoreConfiguration())); + } + + if (sslConfiguration.getKeyStoreConfiguration().isPresent()) { + sslContextBuilder.keyManager(createKeyManagerFactory(sslConfiguration.getKeyStoreConfiguration(), + sslConfiguration.getKeyConfiguration())); + } + + if (!sslConfiguration.getEnabledProtocols().isEmpty()) { + sslContextBuilder.protocols(sslConfiguration.getEnabledProtocols()); + } + + if (!sslConfiguration.getEnabledCipherSuites().isEmpty()) { + sslContextBuilder.ciphers(sslConfiguration.getEnabledCipherSuites()); + } } catch (GeneralSecurityException | IOException e) { throw new IllegalStateException(e); } } - private static org.eclipse.jetty.client.HttpClient configureClient( + } + + /** + * Utility methods to create {@link ClientHttpRequestFactory} using the Jetty Client. + * + * @author Mark Paluch + */ + static class JettyClient { + + /** + * Create a {@link ClientHttpConnector} using Jetty. + * @param options must not be {@literal null} + * @param sslConfiguration must not be {@literal null} + * @return a new and configured {@link JettyClientHttpConnector} instance. + * @throws GeneralSecurityException + * @throws IOException + */ + public static JettyClientHttpConnector usingJetty(ClientOptions options, SslConfiguration sslConfiguration) + throws GeneralSecurityException, IOException { + return new JettyClientHttpConnector(configureClient(getHttpClient(sslConfiguration), options)); + } + + public static org.eclipse.jetty.client.HttpClient configureClient( org.eclipse.jetty.client.HttpClient httpClient, ClientOptions options) { httpClient.setConnectTimeout(options.getConnectionTimeout().toMillis()); @@ -166,7 +192,7 @@ public class ClientHttpConnectorFactory { return httpClient; } - private static org.eclipse.jetty.client.HttpClient getHttpClient(SslConfiguration sslConfiguration) + public static org.eclipse.jetty.client.HttpClient getHttpClient(SslConfiguration sslConfiguration) throws IOException, GeneralSecurityException { if (hasSslConfiguration(sslConfiguration)) { diff --git a/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpRequestFactoryFactory.java b/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpRequestFactoryFactory.java index 5401d0aa..179f35f0 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpRequestFactoryFactory.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpRequestFactoryFactory.java @@ -150,14 +150,18 @@ public class ClientHttpRequestFactoryFactory { return new SimpleClientHttpRequestFactory(); } - private static SSLContext getSSLContext(SslConfiguration sslConfiguration, TrustManager[] trustManagers) + private static SSLContext getSSLContext(SslConfiguration sslConfiguration) throws GeneralSecurityException, IOException { - KeyConfiguration keyConfiguration = sslConfiguration.getKeyConfiguration(); - KeyManager[] keyManagers = sslConfiguration.getKeyStoreConfiguration().isPresent() - ? createKeyManagerFactory(sslConfiguration.getKeyStoreConfiguration(), keyConfiguration) - .getKeyManagers() - : null; + return getSSLContext(sslConfiguration.getKeyStoreConfiguration(), sslConfiguration.getKeyConfiguration(), + getTrustManagers(sslConfiguration)); + } + + static SSLContext getSSLContext(KeyStoreConfiguration keyStoreConfiguration, KeyConfiguration keyConfiguration, + @Nullable TrustManager[] trustManagers) throws GeneralSecurityException, IOException { + + KeyManager[] keyManagers = keyStoreConfiguration.isPresent() + ? createKeyManagerFactory(keyStoreConfiguration, keyConfiguration).getKeyManagers() : null; SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(keyManagers, trustManagers, null); @@ -166,7 +170,7 @@ public class ClientHttpRequestFactoryFactory { } @Nullable - private static TrustManager[] getTrustManagers(SslConfiguration sslConfiguration) + static TrustManager[] getTrustManagers(SslConfiguration sslConfiguration) throws GeneralSecurityException, IOException { return sslConfiguration.getTrustStoreConfiguration().isPresent() @@ -282,13 +286,30 @@ public class ClientHttpRequestFactoryFactory { } /** - * {@link ClientHttpRequestFactory} for Apache Http Components. + * Utilities to create a {@link ClientHttpRequestFactory} for Apache Http Components. * * @author Mark Paluch */ - static class HttpComponents { + public static class HttpComponents { - static ClientHttpRequestFactory usingHttpComponents(ClientOptions options, SslConfiguration sslConfiguration) + /** + * Create a {@link ClientHttpRequestFactory} using Apache Http Components. + * @param options must not be {@literal null} + * @param sslConfiguration must not be {@literal null} + * @return a new and configured {@link HttpComponentsClientHttpRequestFactory} + * instance. + * @throws GeneralSecurityException + * @throws IOException + */ + public static HttpComponentsClientHttpRequestFactory usingHttpComponents(ClientOptions options, + SslConfiguration sslConfiguration) throws GeneralSecurityException, IOException { + + HttpClientBuilder httpClientBuilder = getHttpClientBuilder(options, sslConfiguration); + + return new HttpComponentsClientHttpRequestFactory(httpClientBuilder.build()); + } + + public static HttpClientBuilder getHttpClientBuilder(ClientOptions options, SslConfiguration sslConfiguration) throws GeneralSecurityException, IOException { HttpClientBuilder httpClientBuilder = HttpClients.custom(); @@ -298,7 +319,7 @@ public class ClientHttpRequestFactoryFactory { if (hasSslConfiguration(sslConfiguration)) { - SSLContext sslContext = getSSLContext(sslConfiguration, getTrustManagers(sslConfiguration)); + SSLContext sslContext = getSSLContext(sslConfiguration); String[] enabledProtocols = null; @@ -330,19 +351,36 @@ public class ClientHttpRequestFactoryFactory { // Support redirects httpClientBuilder.setRedirectStrategy(new LaxRedirectStrategy()); - return new HttpComponentsClientHttpRequestFactory(httpClientBuilder.build()); + return httpClientBuilder; } } /** - * {@link ClientHttpRequestFactory} for the {@link okhttp3.OkHttpClient}. + * Utilities to create a {@link ClientHttpRequestFactory} for the + * {@link okhttp3.OkHttpClient}. * * @author Mark Paluch */ - static class OkHttp3 { + public static class OkHttp3 { - static ClientHttpRequestFactory usingOkHttp3(ClientOptions options, SslConfiguration sslConfiguration) + /** + * Create a {@link ClientHttpRequestFactory} using {@link okhttp3.OkHttpClient}. + * @param options must not be {@literal null} + * @param sslConfiguration must not be {@literal null} + * @return a new and configured {@link OkHttp3ClientHttpRequestFactory} instance. + * @throws GeneralSecurityException + * @throws IOException + */ + public static OkHttp3ClientHttpRequestFactory usingOkHttp3(ClientOptions options, + SslConfiguration sslConfiguration) throws GeneralSecurityException, IOException { + + Builder builder = getBuilder(options, sslConfiguration); + + return new OkHttp3ClientHttpRequestFactory(builder.build()); + } + + public static Builder getBuilder(ClientOptions options, SslConfiguration sslConfiguration) throws GeneralSecurityException, IOException { Builder builder = new Builder(); @@ -353,13 +391,14 @@ public class ClientHttpRequestFactoryFactory { TrustManager[] trustManagers = getTrustManagers(sslConfiguration); - if (trustManagers.length != 1 || !(trustManagers[0] instanceof X509TrustManager)) { + if (trustManagers == null || trustManagers.length != 1 + || !(trustManagers[0] instanceof X509TrustManager)) { throw new IllegalStateException( "Unexpected default trust managers:" + Arrays.toString(trustManagers)); } - X509TrustManager trustManager = (X509TrustManager) trustManagers[0]; - SSLContext sslContext = getSSLContext(sslConfiguration, trustManagers); + SSLContext sslContext = getSSLContext(sslConfiguration.getKeyStoreConfiguration(), + sslConfiguration.getKeyConfiguration(), trustManagers); ConnectionSpec.Builder sslConnectionSpecBuilder = new ConnectionSpec.Builder(sslConnectionSpec); @@ -374,15 +413,14 @@ public class ClientHttpRequestFactoryFactory { sslConnectionSpec = sslConnectionSpecBuilder.build(); - builder.sslSocketFactory(sslContext.getSocketFactory(), trustManager); + builder.sslSocketFactory(sslContext.getSocketFactory(), (X509TrustManager) trustManagers[0]); } builder.connectionSpecs(Arrays.asList(sslConnectionSpec, ConnectionSpec.CLEARTEXT)); builder.connectTimeout(options.getConnectionTimeout().toMillis(), TimeUnit.MILLISECONDS) .readTimeout(options.getReadTimeout().toMillis(), TimeUnit.MILLISECONDS); - - return new OkHttp3ClientHttpRequestFactory(builder.build()); + return builder; } } diff --git a/spring-vault-core/src/test/java/org/springframework/vault/client/ClientHttpConnectorFactoryIntegrationTests.java b/spring-vault-core/src/test/java/org/springframework/vault/client/ClientHttpConnectorFactoryIntegrationTests.java index 12663de9..bb7398e9 100644 --- a/spring-vault-core/src/test/java/org/springframework/vault/client/ClientHttpConnectorFactoryIntegrationTests.java +++ b/spring-vault-core/src/test/java/org/springframework/vault/client/ClientHttpConnectorFactoryIntegrationTests.java @@ -86,7 +86,7 @@ class ClientHttpConnectorFactoryIntegrationTests { } @Test - void jettyClientShouldWork() { + void jettyClientShouldWork() throws Exception { ClientHttpConnector factory = JettyClient.usingJetty(new ClientOptions(), Settings.createSslConfiguration()); @@ -98,7 +98,7 @@ class ClientHttpConnectorFactoryIntegrationTests { } @Test - void jettyClientWithExplicitEnabledCipherSuitesShouldWork() { + void jettyClientWithExplicitEnabledCipherSuitesShouldWork() throws Exception { List enabledCipherSuites = new ArrayList(); enabledCipherSuites.add("TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"); @@ -115,7 +115,7 @@ class ClientHttpConnectorFactoryIntegrationTests { } @Test - void jettyClientWithExplicitEnabledProtocolsShouldWork() { + void jettyClientWithExplicitEnabledProtocolsShouldWork() throws Exception { List enabledProtocols = new ArrayList(); enabledProtocols.add("TLSv1.2"); diff --git a/spring-vault-core/src/test/java/org/springframework/vault/core/VaultKeyValueMetadataTemplateIntegrationTests.java b/spring-vault-core/src/test/java/org/springframework/vault/core/VaultKeyValueMetadataTemplateIntegrationTests.java index 854de275..911bba98 100644 --- a/spring-vault-core/src/test/java/org/springframework/vault/core/VaultKeyValueMetadataTemplateIntegrationTests.java +++ b/spring-vault-core/src/test/java/org/springframework/vault/core/VaultKeyValueMetadataTemplateIntegrationTests.java @@ -160,7 +160,7 @@ class VaultKeyValueMetadataTemplateIntegrationTests extends AbstractVaultKeyValu this.kvOperations.delete(SECRET_NAME); VaultMetadataResponse metadataResponse = this.vaultKeyValueMetadataOperations.get(SECRET_NAME); Versioned.Metadata version1 = metadataResponse.getVersions().get(0); - assertThat(version1.getDeletedAt()).isBefore(Instant.now()); + assertThat(version1.getDeletedAt()).isBefore(Instant.now().plusSeconds(5)); this.vaultKeyValueMetadataOperations.delete(SECRET_NAME);