Skip to content
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

[Backport 2.x] Add do not fail on forbidden test cases around the stats API (#3825) #3828

Merged
merged 2 commits into from
Dec 14, 2023
Merged

[Backport 2.x] Add do not fail on forbidden test cases around the stats API (#3825) #3828

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7947161
[Backport 2.x] Add do not fail on forbidden test cases around the sta…
peternied Dec 12, 2023
c1fc366
Fix spotless issues
peternied Dec 14, 2023
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
2 changes: 1 addition & 1 deletion src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java
View file
Open in desktop
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
46 changes: 45 additions & 1 deletion src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java
View file
Open in desktop
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")));
}
}
}
4 changes: 2 additions & 2 deletions src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/integrationTest/java/org/opensearch/security/SslOnlyTests.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/integrationTest/java/org/opensearch/security/http/CommonProxyAuthenticationTests.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions src/integrationTest/java/org/opensearch/security/http/ExtendedProxyAuthenticationTest.java
View file
Open in desktop
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
13 changes: 1 addition & 12 deletions src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java
View file
Open in desktop
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;
peternied marked this conversation as resolved.
Show resolved Hide resolved
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -110,17 +108,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
Loading