Skip to content

Commit

Permalink
[Backport 2.x] Add do not fail on forbidden test cases around the sta…
Browse files Browse the repository at this point in the history
…ts API (opensearch-project#3825) (opensearch-project#3828)

Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
peternied authored Dec 14, 2023
1 parent 76a722b commit 6b13050
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Object> users = response.getBodyAs(Map.class);
assertThat(users, allOf(aMapWithSize(3), hasKey(ADMIN_USER_NAME), hasKey(NEW_USER), hasKey(LIMITED_USER)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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")));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand All @@ -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"));
Expand All @@ -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"));
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -51,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;
Expand All @@ -62,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;
Expand Down Expand Up @@ -110,17 +106,8 @@ public TestRestClient(InetSocketAddress nodeHttpAddress, List<Header> headers, S
this.sourceInetAddress = sourceInetAddress;
}

public HttpResponse get(String path, List<NameValuePair> 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) {
Expand Down

0 comments on commit 6b13050

Please sign in to comment.