From 794716126106bc0fa3d710971ba507cd3b4156b3 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Tue, 12 Dec 2023 16:35:25 -0600 Subject: [PATCH 1/2] [Backport 2.x] Add do not fail on forbidden test cases around the stats API (#3825) Signed-off-by: Peter Nied (cherry picked from commit 676dacef288366084a14ff582d589547840c57cc) --- .../security/DefaultConfigurationTests.java | 2 +- .../security/DoNotFailOnForbiddenTests.java | 46 ++++++++++++++++++- .../security/PointInTimeOperationTest.java | 4 +- .../security/SecurityConfigurationTests.java | 10 ++-- .../org/opensearch/security/SslOnlyTests.java | 2 +- .../http/CommonProxyAuthenticationTests.java | 2 +- .../http/ExtendedProxyAuthenticationTest.java | 4 +- .../framework/cluster/TestRestClient.java | 13 +----- 8 files changed, 58 insertions(+), 25 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java b/src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java index 043d3908e9..8bb5b96145 100644 --- a/src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java +++ b/src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java @@ -69,7 +69,7 @@ public void shouldLoadDefaultConfiguration() { } try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) { client.confirmCorrectCredentials(ADMIN_USER_NAME); - HttpResponse response = client.get("/_plugins/_security/api/internalusers"); + HttpResponse response = client.get("_plugins/_security/api/internalusers"); response.assertStatusCode(200); Map users = response.getBodyAs(Map.class); assertThat(users, allOf(aMapWithSize(3), hasKey(ADMIN_USER_NAME), hasKey(NEW_USER), hasKey(LIMITED_USER))); diff --git a/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java b/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java index 207564daaa..3a50a4b1f6 100644 --- a/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java +++ b/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java @@ -39,10 +39,14 @@ import org.opensearch.client.Response; import org.opensearch.client.RestHighLevelClient; import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.TestSecurityConfig.Role; import org.opensearch.test.framework.TestSecurityConfig.User; import org.opensearch.test.framework.cluster.ClusterManager; import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; +import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; +import static org.apache.http.HttpStatus.SC_CREATED; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.aMapWithSize; import static org.hamcrest.Matchers.allOf; @@ -51,6 +55,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.opensearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions.Type.ADD; import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; @@ -127,13 +132,17 @@ public class DoNotFailOnForbiddenTests { .on(MARVELOUS_SONGS) ); + private static final User STATS_USER = new User("stats_user").roles( + new Role("test_role").clusterPermissions("cluster:monitor/*").indexPermissions("read", "indices:monitor/*").on("hi1") + ); + private static final String BOTH_INDEX_ALIAS = "both-indices"; private static final String FORBIDDEN_INDEX_ALIAS = "forbidden-index"; @ClassRule public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) .authc(AUTHC_HTTPBASIC_INTERNAL) - .users(ADMIN_USER, LIMITED_USER) + .users(ADMIN_USER, LIMITED_USER, STATS_USER) .anonymousAuth(false) .doNotFailOnForbidden(true) .build(); @@ -434,4 +443,39 @@ public void shouldPerformCatIndices_positive() throws IOException { } } + @Test + public void checkStatsApi() { + // As admin creates 2 documents in different indices, can find both indices in search, cat indice & stats APIs + try (final TestRestClient client = cluster.getRestClient(ADMIN_USER.getName(), ADMIN_USER.getPassword())) { + final HttpResponse createDoc1 = client.postJson("hi1/_doc?refresh=true", "{\"hi\":\"Hello1\"}"); + createDoc1.assertStatusCode(SC_CREATED); + final HttpResponse createDoc2 = client.postJson("hi2/_doc?refresh=true", "{\"hi\":\"Hello2\"}"); + createDoc2.assertStatusCode(SC_CREATED); + + final HttpResponse search = client.postJson("hi*/_search", "{}"); + assertThat("Unexpected document results in search:" + search.getBody(), search.getBody(), containsString("2")); + + final HttpResponse catIndices = client.get("_cat/indices"); + assertThat("Expected cat indices: " + catIndices.getBody(), catIndices.getBody(), containsString("hi1")); + assertThat("Expected cat indices: " + catIndices.getBody(), catIndices.getBody(), containsString("hi2")); + + final HttpResponse stats = client.get("hi*/_stats?filter_path=indices.*.uuid"); + assertThat("Expected stats indices: " + stats.getBody(), stats.getBody(), containsString("hi1")); + assertThat("Expected stats indices: " + stats.getBody(), stats.getBody(), containsString("hi2")); + } + + // As user who can only see the index "hi1" make sure that DNFOF is filtering out "hi2" + try (final TestRestClient client = cluster.getRestClient(STATS_USER.getName(), STATS_USER.getPassword())) { + final HttpResponse search = client.postJson("hi*/_search", "{}"); + assertThat("Unexpected document results in search:" + search.getBody(), search.getBody(), containsString("1")); + + final HttpResponse catIndices = client.get("_cat/indices"); + assertThat("Expected cat indices: " + catIndices.getBody(), catIndices.getBody(), containsString("hi1")); + assertThat("Unexpected cat indices: " + catIndices.getBody(), catIndices.getBody(), not(containsString("hi2"))); + + final HttpResponse stats = client.get("hi*/_stats?filter_path=indices.*.uuid"); + assertThat("Expected stats indices: " + stats.getBody(), stats.getBody(), containsString("hi1")); + assertThat("Unexpected stats indices: " + stats.getBody(), stats.getBody(), not(containsString("hi2"))); + } + } } diff --git a/src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java b/src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java index 81c6ae2d3e..8dfc281eb1 100644 --- a/src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java +++ b/src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java @@ -380,7 +380,7 @@ public void listPitSegmentsCreatedWithIndexAlias_negative() throws IOException { @Test public void listAllPitSegments_positive() { try (TestRestClient restClient = cluster.getRestClient(POINT_IN_TIME_USER)) { - HttpResponse response = restClient.get("/_cat/pit_segments/_all"); + HttpResponse response = restClient.get("_cat/pit_segments/_all"); response.assertStatusCode(OK.getStatus()); } @@ -389,7 +389,7 @@ public void listAllPitSegments_positive() { @Test public void listAllPitSegments_negative() { try (TestRestClient restClient = cluster.getRestClient(LIMITED_POINT_IN_TIME_USER)) { - HttpResponse response = restClient.get("/_cat/pit_segments/_all"); + HttpResponse response = restClient.get("_cat/pit_segments/_all"); response.assertStatusCode(FORBIDDEN.getStatus()); } diff --git a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java index cc95f191f7..76ea02494e 100644 --- a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java +++ b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java @@ -119,7 +119,7 @@ public void shouldCreateUserViaRestApi_failure() { @Test public void shouldAuthenticateAsAdminWithCertificate_positive() { try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - HttpResponse httpResponse = client.get("/_plugins/_security/whoami"); + HttpResponse httpResponse = client.get("_plugins/_security/whoami"); httpResponse.assertStatusCode(200); assertThat(httpResponse.getTextFromJsonBody("/is_admin"), equalTo("true")); @@ -130,7 +130,7 @@ public void shouldAuthenticateAsAdminWithCertificate_positive() { public void shouldAuthenticateAsAdminWithCertificate_negativeSelfSignedCertificate() { TestCertificates testCertificates = cluster.getTestCertificates(); try (TestRestClient client = cluster.getRestClient(testCertificates.createSelfSignedCertificate("CN=bond"))) { - HttpResponse httpResponse = client.get("/_plugins/_security/whoami"); + HttpResponse httpResponse = client.get("_plugins/_security/whoami"); httpResponse.assertStatusCode(200); assertThat(httpResponse.getTextFromJsonBody("/is_admin"), equalTo("false")); @@ -141,7 +141,7 @@ public void shouldAuthenticateAsAdminWithCertificate_negativeSelfSignedCertifica public void shouldAuthenticateAsAdminWithCertificate_negativeIncorrectDn() { TestCertificates testCertificates = cluster.getTestCertificates(); try (TestRestClient client = cluster.getRestClient(testCertificates.createAdminCertificate("CN=non_admin"))) { - HttpResponse httpResponse = client.get("/_plugins/_security/whoami"); + HttpResponse httpResponse = client.get("_plugins/_security/whoami"); httpResponse.assertStatusCode(200); assertThat(httpResponse.getTextFromJsonBody("/is_admin"), equalTo("false")); @@ -199,7 +199,7 @@ public void shouldStillWorkAfterUpdateOfSecurityConfig() { @Test public void shouldAccessIndexWithPlaceholder_positive() { try (TestRestClient client = cluster.getRestClient(LIMITED_USER)) { - HttpResponse httpResponse = client.get("/" + LIMITED_USER_INDEX + "/_doc/" + ID_1); + HttpResponse httpResponse = client.get(LIMITED_USER_INDEX + "/_doc/" + ID_1); httpResponse.assertStatusCode(200); } @@ -208,7 +208,7 @@ public void shouldAccessIndexWithPlaceholder_positive() { @Test public void shouldAccessIndexWithPlaceholder_negative() { try (TestRestClient client = cluster.getRestClient(LIMITED_USER)) { - HttpResponse httpResponse = client.get("/" + PROHIBITED_INDEX + "/_doc/" + ID_2); + HttpResponse httpResponse = client.get(PROHIBITED_INDEX + "/_doc/" + ID_2); httpResponse.assertStatusCode(403); } diff --git a/src/integrationTest/java/org/opensearch/security/SslOnlyTests.java b/src/integrationTest/java/org/opensearch/security/SslOnlyTests.java index 25feffb2b4..2ea5b4c0b2 100644 --- a/src/integrationTest/java/org/opensearch/security/SslOnlyTests.java +++ b/src/integrationTest/java/org/opensearch/security/SslOnlyTests.java @@ -59,7 +59,7 @@ public void shouldGetIndicesWithoutAuthentication() { try (TestRestClient client = cluster.getRestClient()) { // request does not contains credential - HttpResponse response = client.get("/_cat/indices"); + HttpResponse response = client.get("_cat/indices"); // successful response is returned because the security plugin in SSL only mode // does not perform authentication and authorization diff --git a/src/integrationTest/java/org/opensearch/security/http/CommonProxyAuthenticationTests.java b/src/integrationTest/java/org/opensearch/security/http/CommonProxyAuthenticationTests.java index 49ded4f2a9..d98acf8895 100644 --- a/src/integrationTest/java/org/opensearch/security/http/CommonProxyAuthenticationTests.java +++ b/src/integrationTest/java/org/opensearch/security/http/CommonProxyAuthenticationTests.java @@ -31,7 +31,7 @@ */ abstract class CommonProxyAuthenticationTests { - protected static final String RESOURCE_AUTH_INFO = "/_opendistro/_security/authinfo"; + protected static final String RESOURCE_AUTH_INFO = "_opendistro/_security/authinfo"; protected static final TestSecurityConfig.User USER_ADMIN = new TestSecurityConfig.User("admin").roles(ALL_ACCESS); protected static final String ATTRIBUTE_DEPARTMENT = "department"; diff --git a/src/integrationTest/java/org/opensearch/security/http/ExtendedProxyAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/ExtendedProxyAuthenticationTest.java index 6fcc7eac83..7c361828d6 100644 --- a/src/integrationTest/java/org/opensearch/security/http/ExtendedProxyAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/ExtendedProxyAuthenticationTest.java @@ -230,7 +230,7 @@ public void shouldRetrieveUserRolesAndAttributesSoThatAccessToPersonalIndexIsPos .header(HEADER_DEPARTMENT, DEPARTMENT_BRIDGE); try (TestRestClient client = cluster.createGenericClientRestClient(testRestClientConfiguration)) { - HttpResponse response = client.get("/" + PERSONAL_INDEX_NAME_SPOCK + "/_search"); + HttpResponse response = client.get(PERSONAL_INDEX_NAME_SPOCK + "/_search"); response.assertStatusCode(200); assertThat(response.getLongFromJsonBody(POINTER_TOTAL_HITS), equalTo(1L)); @@ -251,7 +251,7 @@ public void shouldRetrieveUserRolesAndAttributesSoThatAccessToPersonalIndexIsPos .header(HEADER_DEPARTMENT, DEPARTMENT_BRIDGE); try (TestRestClient client = cluster.createGenericClientRestClient(testRestClientConfiguration)) { - HttpResponse response = client.get("/" + PERSONAL_INDEX_NAME_KIRK + "/_search"); + HttpResponse response = client.get(PERSONAL_INDEX_NAME_KIRK + "/_search"); response.assertStatusCode(403); } diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index c3bad71e0d..5e154afbc5 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -32,8 +32,6 @@ import java.io.UnsupportedEncodingException; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.URI; -import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; @@ -110,17 +108,8 @@ public TestRestClient(InetSocketAddress nodeHttpAddress, List
headers, S this.sourceInetAddress = sourceInetAddress; } - public HttpResponse get(String path, List queryParameters, Header... headers) { - try { - URI uri = new URIBuilder(getHttpServerUri()).setPath(path).addParameters(queryParameters).build(); - return executeRequest(new HttpGet(uri), headers); - } catch (URISyntaxException ex) { - throw new RuntimeException("Incorrect URI syntax", ex); - } - } - public HttpResponse get(String path, Header... headers) { - return get(path, Collections.emptyList(), headers); + return executeRequest(new HttpGet(getHttpServerUri() + "/" + path), headers); } public HttpResponse getAuthInfo(Header... headers) { From c1fc3666204fa6c010177ec2a7c7853b45baf1ab Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 14 Dec 2023 19:46:56 +0000 Subject: [PATCH 2/2] Fix spotless issues Signed-off-by: Peter Nied --- .../org/opensearch/test/framework/cluster/TestRestClient.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index 5e154afbc5..df69332dde 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -49,7 +49,6 @@ import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpHeaders; -import org.apache.http.NameValuePair; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpDelete; @@ -60,7 +59,6 @@ import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.client.utils.URIBuilder; import org.apache.http.conn.routing.HttpRoutePlanner; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.CloseableHttpClient;