Skip to content

[HTTP/2] Functional Test cases #2367

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
DarshitChanpura merged 1 commit into from
Dec 31, 2022
Merged

[HTTP/2] Functional Test cases #2367

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
30 changes: 28 additions & 2 deletions 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 @@ -33,6 +33,8 @@
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.HttpVersion;
import org.apache.hc.core5.http2.HttpVersionPolicy;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -120,14 +122,39 @@ public void testWhoAmI() throws Exception {
try (RestHighLevelClient restHighLevelClient = getRestClient(clusterInfo, "spock-keystore.jks", "truststore.jks")) {
Response whoAmIRes = restHighLevelClient.getLowLevelClient().performRequest(new Request("GET", "/_plugins/_security/whoami"));
Assert.assertEquals(whoAmIRes.getStatusLine().getStatusCode(), 200);
// Should be using HTTP/2 by default
Assert.assertEquals(whoAmIRes.getStatusLine().getProtocolVersion(), HttpVersion.HTTP_2);
JsonNode whoAmIResNode = DefaultObjectMapper.objectMapper.readTree(whoAmIRes.getEntity().getContent());
String whoAmIResponsePayload = whoAmIResNode.toPrettyString();
Assert.assertEquals(whoAmIResponsePayload, "CN=spock,OU=client,O=client,L=Test,C=DE", whoAmIResNode.get("dn").asText());
Assert.assertFalse(whoAmIResponsePayload, whoAmIResNode.get("is_admin").asBoolean());
Assert.assertFalse(whoAmIResponsePayload, whoAmIResNode.get("is_node_certificate_request").asBoolean());
}
}


@Test
Copy link
Member

@cwperks cwperks Dec 27, 2022

Choose a reason for hiding this comment

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

Curious, is there a way for a cluster admin to force HTTP/1? Would that ever be valid or desired as a config setting?

Copy link
Collaborator Author

@reta reta Dec 27, 2022

Choose a reason for hiding this comment

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

Hm ... this is a good question, I don't see why one would do that intentionally, but the RestClientBuilder could force HTTP/1.1, see please [1] fe

[1] https://github.com/opensearch-project/security/pull/2367/files#diff-25d921c48bb0ef8fb56806d0d9cbed23e72f8e9b275a9ec606bc6be78cecb37eR192

public void testWhoAmIForceHttp1() throws Exception {
final Settings settings = Settings.builder()
.put("plugins.security.ssl.http.enabled",true)
.put("plugins.security.ssl.http.keystore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("node-0-keystore.jks"))
.put("plugins.security.ssl.http.truststore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("truststore.jks"))
.build();
setup(Settings.EMPTY, new DynamicSecurityConfig().setSecurityInternalUsers("internal_empty.yml")
.setSecurityRoles("roles_deny.yml"), settings, true);

try (RestHighLevelClient restHighLevelClient = getRestClient(clusterInfo, "spock-keystore.jks", "truststore.jks", HttpVersionPolicy.FORCE_HTTP_1)) {
Response whoAmIRes = restHighLevelClient.getLowLevelClient().performRequest(new Request("GET", "/_plugins/_security/whoami"));
Assert.assertEquals(whoAmIRes.getStatusLine().getStatusCode(), 200);
// The HTTP/1.1 is forced and should be used instead
Assert.assertEquals(whoAmIRes.getStatusLine().getProtocolVersion(), HttpVersion.HTTP_1_1);
JsonNode whoAmIResNode = DefaultObjectMapper.objectMapper.readTree(whoAmIRes.getEntity().getContent());
String whoAmIResponsePayload = whoAmIResNode.toPrettyString();
Assert.assertEquals(whoAmIResponsePayload, "CN=spock,OU=client,O=client,L=Test,C=DE", whoAmIResNode.get("dn").asText());
Assert.assertFalse(whoAmIResponsePayload, whoAmIResNode.get("is_admin").asBoolean());
Assert.assertFalse(whoAmIResponsePayload, whoAmIResNode.get("is_node_certificate_request").asBoolean());
}
}

@Test
public void testConfigHotReload() throws Exception {

Expand Down Expand Up @@ -234,5 +261,4 @@ public void testDiscoveryWithoutInitialization() throws Exception {
Assert.assertEquals(clusterInfo.numNodes, clusterHelper.nodeClient().admin().cluster().health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getNumberOfNodes());
Assert.assertEquals(ClusterHealthStatus.GREEN, clusterHelper.nodeClient().admin().cluster().health(new ClusterHealthRequest().waitForGreenStatus()).actionGet().getStatus());
}

}
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public void testSSLPlainText() throws Exception {
final RuntimeException ex = Assert.assertThrows(RuntimeException.class,
() -> nonSslRestHelper().executeGetRequest("_search", encodeBasicHeader("admin", "admin")));
Assert.assertEquals("org.apache.hc.core5.http.NoHttpResponseException", ex.getCause().getClass().getName());
}, 2);
}, 1); /* no retry on NotSslRecordException exceptions */

// All of the messages should be the same as the http client is attempting multiple times.
messages.stream().forEach((message) -> {
Expand Down
7 changes: 4 additions & 3 deletions src/test/java/org/opensearch/security/dlic/dlsfls/DlsFlsCrossClusterSearchTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ public void testCcs() throws Exception {
Assert.assertTrue(ccs.getBody().contains("salary1"));
Assert.assertFalse(ccs.getBody().contains("secret1"));
Assert.assertFalse(ccs.getBody().contains("AnotherSecredField"));
Assert.assertFalse(ccs.getBody().contains("xxx1")); Assert.assertEquals(ccs.getHeaders().toString(), 1, ccs.getHeaders().size());
Assert.assertFalse(ccs.getBody().contains("xxx1"));
Assert.assertEquals(ccs.getHeaders().toString(), 2, ccs.getHeaders().size());
Copy link
Collaborator Author

@reta reta Dec 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

There are 2 headers now: content type and content length

}

@Test
Expand Down Expand Up @@ -183,7 +184,7 @@ public void testCcsDifferentConfig() throws Exception {
Assert.assertTrue(ccs.getBody().contains("__fn__crl2"));
Assert.assertFalse(ccs.getBody().contains("secret1"));
Assert.assertFalse(ccs.getBody().contains("AnotherSecredField"));
Assert.assertEquals(ccs.getHeaders().toString(), 1, ccs.getHeaders().size());
Assert.assertEquals(ccs.getHeaders().toString(), 2, ccs.getHeaders().size());
}

@Test
Expand Down Expand Up @@ -265,6 +266,6 @@ public void testCcsDifferentConfigBoth() throws Exception {
Assert.assertFalse(ccs.getBody().contains("secret1"));
Assert.assertFalse(ccs.getBody().contains("AnotherSecredField"));
Assert.assertTrue(ccs.getBody().contains("someoneelse"));
Assert.assertEquals(ccs.getHeaders().toString(), 1, ccs.getHeaders().size());
Assert.assertEquals(ccs.getHeaders().toString(), 2, ccs.getHeaders().size());
}
}
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/security/dlic/dlsfls/DlsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void testDls() throws Exception {
Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("/deals/_search?pretty", encodeBasicHeader("dept_manager", "password"))).getStatusCode());
Assert.assertTrue(res.getBody().contains("\"value\" : 1,\n \"relation"));
Assert.assertTrue(res.getBody().contains("\"failed\" : 0"));
Assert.assertEquals(res.getHeaders().toString(), 1, res.getHeaders().size());
Assert.assertEquals(res.getHeaders().toString(), 2, res.getHeaders().size());

Assert.assertEquals(HttpStatus.SC_OK, (res = rh.executeGetRequest("/deals/_search?pretty", encodeBasicHeader("admin", "admin"))).getStatusCode());
Assert.assertTrue(res.getBody().contains("\"value\" : 2,\n \"relation"));
Expand Down
10 changes: 8 additions & 2 deletions src/test/java/org/opensearch/security/sanity/tests/SecurityRestTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,20 @@ protected RestClient buildClient(Settings settings, HttpHost[] hosts) throws IOE
// create adminDN (super-admin) client
File file = new File(getClass().getClassLoader().getResource(CERT_FILE_DIRECTORY).getFile());
Path configPath = PathUtils.get(file.toURI()).getParent().toAbsolutePath();
return new SecureRestClientBuilder(settings, configPath).setSocketTimeout(60000).build();
return new SecureRestClientBuilder(settings, configPath)
.setSocketTimeout(60000)
.setConnectionRequestTimeout(180000)
Copy link
Collaborator Author

@reta reta Dec 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Should be fixed by opensearch-project/common-utils#287 but needs manual setting now

Copy link
Member

@DarshitChanpura DarshitChanpura Dec 24, 2022

Choose a reason for hiding this comment

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

Should we wait until that issue has been published? or we can create a tracker issue that reverts this change once common-utils publishes an artifact with that change

Copy link
Collaborator Author

@reta reta Dec 24, 2022

Choose a reason for hiding this comment

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

No action is needed: this change is good, it may just be redundant after opensearch-project/common-utils#287 but keeping it is fine, thanks.

.build();
}

// create client with passed user
String userName = System.getProperty("user");
String password = System.getProperty("password");

return new SecureRestClientBuilder(hosts, isHttps(), userName, password).setSocketTimeout(60000).build();
return new SecureRestClientBuilder(hosts, isHttps(), userName, password)
.setSocketTimeout(60000)
.setConnectionRequestTimeout(180000)
.build();
}
else {
RestClientBuilder builder = RestClient.builder(hosts);
Expand Down
8 changes: 8 additions & 0 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 @@ -55,6 +55,7 @@
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.http2.HttpVersionPolicy;
import org.apache.hc.core5.reactor.ssl.TlsDetails;
import org.apache.hc.core5.ssl.SSLContextBuilder;
import org.apache.hc.core5.ssl.SSLContexts;
Expand Down Expand Up @@ -144,6 +145,10 @@ public static Header encodeBasicHeader(final String username, final String passw
}

protected RestHighLevelClient getRestClient(ClusterInfo info, String keyStoreName, String trustStoreName) {
return getRestClient(info, keyStoreName, trustStoreName, null);
}

protected RestHighLevelClient getRestClient(ClusterInfo info, String keyStoreName, String trustStoreName, HttpVersionPolicy httpVersionPolicy) {
final String prefix = getResourceFolder()==null?"":getResourceFolder()+"/";

try {
Expand Down Expand Up @@ -183,6 +188,9 @@ public TlsDetails create(final SSLEngine sslEngine) {
.setTlsStrategy(tlsStrategy)
.build();
builder.setConnectionManager(cm);
if (httpVersionPolicy != null) {
builder.setVersionPolicy(httpVersionPolicy);
}
return builder;
});
return new RestHighLevelClient(restClientBuilder);
Expand Down
Loading