From 6cec3684de9abbb6ceb57e3fbb866b9904969f80 Mon Sep 17 00:00:00 2001 From: Rob Rudin Date: Mon, 23 Jan 2023 15:08:59 -0500 Subject: [PATCH] Forcing port to 443 when using MarkLogic cloud authentication 443 is written in stone for now, and thus the UX can be simpler by forcing the port to that value. Also, removed support for defaulting to 443 in DatabaseClientPropertySource. That was intended for ML Cloud users, but the better way to do it is via the DatabaseClientFactory method that's modified in this PR. --- .../client/DatabaseClientFactory.java | 7 +++++++ .../OkHttpClientBuilderFactory.java | 4 ++-- .../impl/DatabaseClientPropertySource.java | 3 --- .../marklogic/client/impl/OkHttpServices.java | 2 +- ...arkLogicCloudAuthenticationConfigurer.java | 10 ++++----- .../client/impl/okhttp/OkHttpUtil.java | 4 ++-- .../OkHttpClientBuilderFactoryTest.java | 3 +-- ...ogicCloudAuthenticationConfigurerTest.java | 21 +++++-------------- .../test/DatabaseClientBuilderTest.java | 15 ------------- 9 files changed, 23 insertions(+), 46 deletions(-) diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java b/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java index 359714b40..02ba2d2fe 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java @@ -1383,6 +1383,13 @@ static public DatabaseClient newClient(String host, int port, String basePath, S SecurityContext securityContext, DatabaseClient.ConnectionType connectionType) { RESTServices services = new OkHttpServices(); + // As of 6.1.0, the following optimization is made as it's guaranteed that if the user is connecting to a + // MarkLogic Cloud instance, then port 443 will be used. Every path for constructing a DatabaseClient goes through + // this method, ensuring that this optimization will always be applied, and thus freeing the user from having to + // worry about what port to configure when using MarkLogic Cloud. + if (securityContext instanceof MarkLogicCloudAuthContext) { + port = 443; + } services.connect(host, port, basePath, database, securityContext); if (clientConfigurator != null) { diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactory.java b/marklogic-client-api/src/main/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactory.java index 0c37fd1fa..c85abc389 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactory.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactory.java @@ -28,7 +28,7 @@ */ public interface OkHttpClientBuilderFactory { - static OkHttpClient.Builder newOkHttpClientBuilder(String host, int port, DatabaseClientFactory.SecurityContext securityContext) { - return OkHttpUtil.newOkHttpClientBuilder(host, port, securityContext); + static OkHttpClient.Builder newOkHttpClientBuilder(String host, DatabaseClientFactory.SecurityContext securityContext) { + return OkHttpUtil.newOkHttpClientBuilder(host, securityContext); } } diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java index b66a12aa2..710fd2f77 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java @@ -91,9 +91,6 @@ public DatabaseClientFactory.Bean newClientBean() { } }); bean.setSecurityContext(newSecurityContext()); - if (bean.getSecurityContext() != null && bean.getSecurityContext().getSSLContext() != null && bean.getPort() == 0) { - bean.setPort(443); - } return bean; } diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java index 65c8e0985..9a4a63863 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java @@ -197,7 +197,7 @@ public void connect(String host, int port, String basePath, String database, Sec this.database = database; this.baseUri = HttpUrlBuilder.newBaseUrl(host, port, basePath, securityContext.getSSLContext()); - OkHttpClient.Builder clientBuilder = OkHttpUtil.newOkHttpClientBuilder(host, port, securityContext); + OkHttpClient.Builder clientBuilder = OkHttpUtil.newOkHttpClientBuilder(host, securityContext); Properties props = System.getProperties(); if (props.containsKey(OKHTTP_LOGGINGINTERCEPTOR_LEVEL)) { diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurer.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurer.java index 46796e8c7..7cba1744a 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurer.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurer.java @@ -29,11 +29,9 @@ public class MarkLogicCloudAuthenticationConfigurer implements AuthenticationCon private final static Logger logger = LoggerFactory.getLogger(MarkLogicCloudAuthenticationConfigurer.class); private String host; - private int port; - public MarkLogicCloudAuthenticationConfigurer(String host, int port) { + public MarkLogicCloudAuthenticationConfigurer(String host) { this.host = host; - this.port = port; } @Override @@ -84,10 +82,12 @@ private Response callTokenEndpoint(MarkLogicCloudAuthContext securityContext) { } protected HttpUrl buildTokenUrl(MarkLogicCloudAuthContext securityContext) { + // For the near future, it's guaranteed that https and 443 will be required for connecting to MarkLogic Cloud, + // so providing the ability to customize this would be misleading. return new HttpUrl.Builder() - .scheme(securityContext.getSSLContext() != null ? "https" : "http") + .scheme("https") .host(host) - .port(port) + .port(443) .build() .resolve(securityContext.getTokenEndpoint()).newBuilder().build(); } diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/OkHttpUtil.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/OkHttpUtil.java index d8ca95e6f..9ad515242 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/OkHttpUtil.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/OkHttpUtil.java @@ -36,7 +36,7 @@ public abstract class OkHttpUtil { final private static ConnectionPool connectionPool = new ConnectionPool(); - public static OkHttpClient.Builder newOkHttpClientBuilder(String host, int port, DatabaseClientFactory.SecurityContext securityContext) { + public static OkHttpClient.Builder newOkHttpClientBuilder(String host, DatabaseClientFactory.SecurityContext securityContext) { OkHttpClient.Builder clientBuilder = OkHttpUtil.newClientBuilder(); AuthenticationConfigurer authenticationConfigurer = null; @@ -53,7 +53,7 @@ public static OkHttpClient.Builder newOkHttpClientBuilder(String host, int port, } else if (securityContext instanceof DatabaseClientFactory.SAMLAuthContext) { configureSAMLAuth((DatabaseClientFactory.SAMLAuthContext) securityContext, clientBuilder); } else if (securityContext instanceof DatabaseClientFactory.MarkLogicCloudAuthContext) { - authenticationConfigurer = new MarkLogicCloudAuthenticationConfigurer(host, port); + authenticationConfigurer = new MarkLogicCloudAuthenticationConfigurer(host); } else { throw new IllegalArgumentException("Unsupported security context: " + securityContext.getClass()); } diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactoryTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactoryTest.java index e5f201145..0d8685d78 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactoryTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactoryTest.java @@ -12,8 +12,7 @@ public class OkHttpClientBuilderFactoryTest { @Test void smokeTest() { DatabaseClientFactory.Bean bean = Common.newClientBuilder().buildBean(); - OkHttpClient.Builder builder = OkHttpClientBuilderFactory.newOkHttpClientBuilder( - bean.getHost(), bean.getPort(), bean.getSecurityContext()); + OkHttpClient.Builder builder = OkHttpClientBuilderFactory.newOkHttpClientBuilder(bean.getHost(), bean.getSecurityContext()); assertNotNull(builder); OkHttpClient client = builder.build(); diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurerTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurerTest.java index b4d57f6b5..924d7e9b3 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurerTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurerTest.java @@ -17,22 +17,11 @@ public class MarkLogicCloudAuthenticationConfigurerTest { @Test void buildTokenUrl() throws Exception { - HttpUrl tokenUrl = new MarkLogicCloudAuthenticationConfigurer("somehost", 443).buildTokenUrl( + HttpUrl tokenUrl = new MarkLogicCloudAuthenticationConfigurer("somehost").buildTokenUrl( new DatabaseClientFactory.MarkLogicCloudAuthContext("doesnt-matter") .withSSLContext(SSLContext.getDefault(), null) ); - assertEquals("https://somehost/token", tokenUrl.toString(), - "When the port is 443, OkHttp won't include it in the URL"); - } - - @Test - void buildTokenUrlWithNonStandardPort() throws Exception { - HttpUrl tokenUrl = new MarkLogicCloudAuthenticationConfigurer("somehost", 444).buildTokenUrl( - new DatabaseClientFactory.MarkLogicCloudAuthContext("doesnt-matter") - .withSSLContext(SSLContext.getDefault(), null) - ); - assertEquals("https://somehost:444/token", tokenUrl.toString(), - "If the port isn't 443, then OkHttp will include it in the URL"); + assertEquals("https://somehost/token", tokenUrl.toString()); } /** @@ -41,7 +30,7 @@ void buildTokenUrlWithNonStandardPort() throws Exception { */ @Test void buildTokenUrlWithCustomTokenPath() throws Exception { - HttpUrl tokenUrl = new MarkLogicCloudAuthenticationConfigurer("otherhost", 443).buildTokenUrl( + HttpUrl tokenUrl = new MarkLogicCloudAuthenticationConfigurer("otherhost").buildTokenUrl( new DatabaseClientFactory.MarkLogicCloudAuthContext("doesnt-matter", "/customToken", "doesnt-matter") .withSSLContext(SSLContext.getDefault(), null) ); @@ -50,7 +39,7 @@ void buildTokenUrlWithCustomTokenPath() throws Exception { @Test void newFormBody() { - FormBody body = new MarkLogicCloudAuthenticationConfigurer("doesnt-matter", 443) + FormBody body = new MarkLogicCloudAuthenticationConfigurer("doesnt-matter") .newFormBody(new DatabaseClientFactory.MarkLogicCloudAuthContext("myKey")); assertEquals("grant_type", body.name(0)); assertEquals("apikey", body.value(0)); @@ -64,7 +53,7 @@ void newFormBody() { */ @Test void newFormBodyWithOverrides() { - FormBody body = new MarkLogicCloudAuthenticationConfigurer("doesnt-matter", 443) + FormBody body = new MarkLogicCloudAuthenticationConfigurer("doesnt-matter") .newFormBody(new DatabaseClientFactory.MarkLogicCloudAuthContext("myKey", "doesnt-matter", "custom-grant-type")); assertEquals("grant_type", body.name(0)); assertEquals("custom-grant-type", body.value(0)); diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java index d056d47d0..7b79e079c 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java @@ -216,19 +216,4 @@ void sslProtocolAndTrustManagerAndHostnameVerifier() { assertEquals(Common.TRUST_ALL_MANAGER, context.getTrustManager()); assertEquals(DatabaseClientFactory.SSLHostnameVerifier.COMMON, context.getSSLHostnameVerifier()); } - - @Test - void sslContextWithNoPort() throws Exception { - bean = new DatabaseClientBuilder() - .withSecurityContextType("DIGEST") - .withSSLContext(SSLContext.getDefault()) - .buildBean(); - - assertTrue(bean.getSecurityContext() instanceof DatabaseClientFactory.DigestAuthContext); - assertNotNull(bean.getSecurityContext().getSSLContext()); - assertEquals(443, bean.getPort(), - "If an SSLContext is provided with no port, then assume 443, as that's the standard port for HTTPS calls. " + - "That makes life a little simpler for MarkLogic Cloud users as well, as they don't need to worry about " + - "setting the port."); - } }