Skip to content

Forcing port to 443 when using MarkLogic cloud authentication #1521

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

Merged
merged 1 commit into from
Jan 24, 2023
Merged

Forcing port to 443 when using MarkLogic cloud authentication #1521

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
7 changes: 7 additions & 0 deletions marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions ...api/src/main/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor Author

@rjrudin rjrudin Jan 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what ml-app-deployer users - it becomes simpler because the ML cloud auth configurer can safely assume usage of 443 for obtaining an access token, so "port" doesn't need to be provided anymore.

return OkHttpUtil.newOkHttpClientBuilder(host, securityContext);
}
}
3 changes: 0 additions & 3 deletions ...ogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
10 changes: 5 additions & 5 deletions ...rc/main/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
Expand Down
4 changes: 2 additions & 2 deletions marklogic-client-api/src/main/java/com/marklogic/client/impl/okhttp/OkHttpUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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());
}
Expand Down
3 changes: 1 addition & 2 deletions ...src/test/java/com/marklogic/client/extra/okhttpclient/OkHttpClientBuilderFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
21 changes: 5 additions & 16 deletions ...est/java/com/marklogic/client/impl/okhttp/MarkLogicCloudAuthenticationConfigurerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

@rjrudin rjrudin Jan 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test reflects a now-invalid scenario and has thus been removed.

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());
}

/**
Expand All @@ -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)
);
Expand All @@ -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));
Expand All @@ -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));
Expand Down
15 changes: 0 additions & 15 deletions marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}