Skip to content

DEVEXP-447 Better error when HTTP is used instead of HTTPS #1561

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.

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
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -165,26 +165,38 @@ public void setMaxDelay(int maxDelay) {
this.maxDelay = maxDelay;
}

private FailedRequest extractErrorFields(Response response) {
if (response == null) return null;
try {
if (response.code() == STATUS_UNAUTHORIZED) {
FailedRequest failure = new FailedRequest();
failure.setMessageString("Unauthorized");
failure.setStatusString("Failed Auth");
return failure;
}
String responseBody = getEntity(response.body(), String.class);
InputStream is = new ByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8));
FailedRequest handler = FailedRequest.getFailedRequest(response.code(), response.header(HEADER_CONTENT_TYPE), is);
if (handler.getMessage() == null) {
handler.setMessageString(responseBody);
}
return handler;
} finally {
closeResponse(response);
}
}
private FailedRequest extractErrorFields(Response response) {
if (response == null) return null;
try {
if (response.code() == STATUS_UNAUTHORIZED) {
FailedRequest failure = new FailedRequest();
failure.setMessageString("Unauthorized");
failure.setStatusString("Failed Auth");
return failure;
}

final String responseBody = getEntity(response.body(), String.class);
// If HTTP is used but HTTPS is required, MarkLogic returns a text/html response that is not suitable to
// return to a user. But it will contain the below error message, which is much nicer to return to the user.
final String sslErrorMessage = "You have attempted to access an HTTPS server using HTTP";
if (response.code() == STATUS_FORBIDDEN && responseBody != null && responseBody.contains(sslErrorMessage)) {
FailedRequest failure = new FailedRequest();
failure.setMessageString(sslErrorMessage + ".");
failure.setStatusString("Forbidden");
failure.setStatusCode(STATUS_FORBIDDEN);
return failure;
}

InputStream is = new ByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8));
FailedRequest handler = FailedRequest.getFailedRequest(response.code(), response.header(HEADER_CONTENT_TYPE), is);
if (handler.getMessage() == null) {
handler.setMessageString(responseBody);
}
return handler;
} finally {
closeResponse(response);
}
}

@Override
public void connect(String host, int port, String basePath, String database, SecurityContext securityContext){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@

import com.marklogic.client.DatabaseClient;
import com.marklogic.client.DatabaseClientFactory;
import com.marklogic.client.ForbiddenUserException;
import com.marklogic.client.MarkLogicIOException;
import com.marklogic.client.test.junit5.RequireSSLExtension;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.TrustManager;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

@ExtendWith(RequireSSLExtension.class)
class CheckSSLConnectionTest {
Expand Down Expand Up @@ -58,8 +61,34 @@ void defaultSslContext() throws Exception {
.withSSLHostnameVerifier(DatabaseClientFactory.SSLHostnameVerifier.ANY)
.build();

assertThrows(MarkLogicIOException.class, () -> client.checkConnection(),
MarkLogicIOException ex = assertThrows(MarkLogicIOException.class, () -> client.checkConnection(),
"The connection should fail because the JVM's default SSL Context does not have a CA certificate that " +
"corresponds to the test-only certificate that the app server is using for this test");

assertTrue(ex.getCause() instanceof SSLHandshakeException, "Unexpected cause: " + ex.getCause());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made some improvements to this test while I was adding the new test below.

String message = ex.getCause().getMessage();
assertTrue(message.contains("PKIX path building failed"), "The call should have failed because the JVM's " +
"default SSL context does not have a CA certificate for the app server's certificate; " +
"unexpected error: " + message);
}

@Test
void noSslContext() {
DatabaseClient client = Common.newClientBuilder().build();

DatabaseClient.ConnectionResult result = client.checkConnection();
assertEquals("Forbidden", result.getErrorMessage(), "MarkLogic is expected to return a 403 Forbidden when the " +
"user tries to access an HTTPS app server using HTTP");
assertEquals(403, result.getStatusCode());

ForbiddenUserException ex = assertThrows(ForbiddenUserException.class,
() -> client.newServerEval().javascript("fn.currentDate()").evalAs(String.class));

assertEquals(
"Local message: User is not allowed to apply resource at eval. Server Message: You have attempted to access an HTTPS server using HTTP.",
ex.getMessage(),
"The user should get a clear message on why the connection failed as opposed to the previous error " +
"message of 'Server (not a REST instance?)'."
);
}
}