From fbd8882757430bf036825d54959218cb359bddd7 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 31 Dec 2024 15:06:38 -0500 Subject: [PATCH] Address code review comments Signed-off-by: Craig Perkins --- .../{ => systemindex}/SystemIndexTests.java | 34 +++++----- .../IndexDocumentIntoSystemIndexAction.java | 2 +- .../IndexDocumentIntoSystemIndexRequest.java | 2 +- .../IndexDocumentIntoSystemIndexResponse.java | 2 +- .../sampleplugin}/PluginContextSwitcher.java | 2 +- ...dexDocumentIntoMixOfSystemIndexAction.java | 7 +- ...ulkIndexDocumentIntoSystemIndexAction.java | 3 +- ...estIndexDocumentIntoSystemIndexAction.java | 2 +- .../RestRunClusterHealthAction.java | 3 +- .../sampleplugin}/RunClusterHealthAction.java | 2 +- .../RunClusterHealthRequest.java | 2 +- .../RunClusterHealthResponse.java | 2 +- .../sampleplugin}/SystemIndexPlugin1.java | 3 +- .../sampleplugin}/SystemIndexPlugin2.java | 2 +- ...ortIndexDocumentIntoSystemIndexAction.java | 3 +- .../TransportRunClusterHealthAction.java | 3 +- .../test/framework/matcher/RestMatchers.java | 68 +++++++++++++++++++ .../security/auth/BackendRegistry.java | 13 ++-- .../PrivilegesEvaluatorResponse.java | 4 -- .../privileges/SnapshotRestoreEvaluator.java | 1 - .../SystemIndexAccessEvaluator.java | 10 +-- .../transport/SecurityInterceptor.java | 4 +- .../ContextProvidingPluginSubjectTests.java | 34 ---------- .../SystemIndexAccessEvaluatorTest.java | 13 ---- .../AbstractSystemIndicesTests.java | 3 +- .../SystemIndexDisabledTests.java | 12 ++-- .../SystemIndexPermissionDisabledTests.java | 20 +++--- .../SystemIndexPermissionEnabledTests.java | 50 +++++++------- 28 files changed, 147 insertions(+), 159 deletions(-) rename src/integrationTest/java/org/opensearch/security/{ => systemindex}/SystemIndexTests.java (85%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/IndexDocumentIntoSystemIndexAction.java (92%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/IndexDocumentIntoSystemIndexRequest.java (95%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/IndexDocumentIntoSystemIndexResponse.java (96%) rename src/{main/java/org/opensearch/security/identity => integrationTest/java/org/opensearch/security/systemindex/sampleplugin}/PluginContextSwitcher.java (93%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/RestBulkIndexDocumentIntoMixOfSystemIndexAction.java (91%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/RestBulkIndexDocumentIntoSystemIndexAction.java (96%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/RestIndexDocumentIntoSystemIndexAction.java (96%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/RestRunClusterHealthAction.java (93%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/RunClusterHealthAction.java (91%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/RunClusterHealthRequest.java (94%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/RunClusterHealthResponse.java (95%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/SystemIndexPlugin1.java (97%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/SystemIndexPlugin2.java (97%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/TransportIndexDocumentIntoSystemIndexAction.java (97%) rename src/integrationTest/java/org/opensearch/security/{plugin => systemindex/sampleplugin}/TransportRunClusterHealthAction.java (97%) create mode 100644 src/integrationTest/java/org/opensearch/test/framework/matcher/RestMatchers.java diff --git a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java similarity index 85% rename from src/integrationTest/java/org/opensearch/security/SystemIndexTests.java rename to src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java index 0c77cf43f6..0c96cb078b 100644 --- a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java @@ -7,7 +7,7 @@ * compatible open source license. * */ -package org.opensearch.security; +package org.opensearch.security.systemindex; import java.util.List; import java.util.Map; @@ -19,21 +19,22 @@ import org.junit.runner.RunWith; import org.opensearch.core.rest.RestStatus; -import org.opensearch.security.plugin.SystemIndexPlugin1; -import org.opensearch.security.plugin.SystemIndexPlugin2; +import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; +import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; 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 org.opensearch.test.framework.matcher.RestMatchers; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.opensearch.security.plugin.SystemIndexPlugin1.SYSTEM_INDEX_1; -import static org.opensearch.security.plugin.SystemIndexPlugin2.SYSTEM_INDEX_2; import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; +import static org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1.SYSTEM_INDEX_1; +import static org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2.SYSTEM_INDEX_2; import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; @@ -109,11 +110,11 @@ public void testPluginShouldNotBeAbleToIndexDocumentIntoSystemIndexRegisteredByO try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_2); - assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus())); assertThat( - response.getBody(), - containsString( - "no permissions for [indices:admin/create] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1" + response, + RestMatchers.isForbidden( + "/error/root_cause/0/reason", + "no permissions for [] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1" ) ); } @@ -124,8 +125,7 @@ public void testPluginShouldBeAbleToCreateSystemIndexButUserShouldNotBeAbleToInd try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.put("try-create-and-index/" + SYSTEM_INDEX_1 + "?runAs=user"); - assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus())); - assertThat(response.getBody(), containsString("no permissions for [indices:data/write/index] and User [name=admin")); + assertThat(response, RestMatchers.isForbidden("/error/root_cause/0/reason", "no permissions for [] and User [admin")); } } @@ -134,11 +134,11 @@ public void testPluginShouldNotBeAbleToRunClusterActions() { try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { HttpResponse response = client.get("try-cluster-health/plugin"); - assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus())); assertThat( - response.getBody(), - containsString( - "no permissions for [cluster:monitor/health] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1" + response, + RestMatchers.isForbidden( + "/error/root_cause/0/reason", + "no permissions for [] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1" ) ); } @@ -182,9 +182,7 @@ public void testPluginShouldNotBeAbleToBulkIndexDocumentIntoMixOfSystemIndexWher assertThat( response.getBody(), - containsString( - "no permissions for [indices:data/write/bulk[s]] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1" - ) + containsString("no permissions for [] and User [name=plugin:org.opensearch.security.plugin.SystemIndexPlugin1") ); } } diff --git a/src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexAction.java similarity index 92% rename from src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexAction.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexAction.java index 8621f2a609..73e90e3039 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexAction.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexAction.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import org.opensearch.action.ActionType; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexRequest.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexRequest.java similarity index 95% rename from src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexRequest.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexRequest.java index d1b644ad11..ae7c345457 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexRequest.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexRequest.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import java.io.IOException; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexResponse.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexResponse.java similarity index 96% rename from src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexResponse.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexResponse.java index 9779cca252..e154742fe2 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/IndexDocumentIntoSystemIndexResponse.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/IndexDocumentIntoSystemIndexResponse.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; // CS-SUPPRESS-SINGLE: RegexpSingleline It is not possible to use phrase "cluster manager" instead of master here import java.io.IOException; diff --git a/src/main/java/org/opensearch/security/identity/PluginContextSwitcher.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/PluginContextSwitcher.java similarity index 93% rename from src/main/java/org/opensearch/security/identity/PluginContextSwitcher.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/PluginContextSwitcher.java index a16671fda4..f74c26ed9e 100644 --- a/src/main/java/org/opensearch/security/identity/PluginContextSwitcher.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/PluginContextSwitcher.java @@ -8,7 +8,7 @@ * Modifications Copyright OpenSearch Contributors. See * GitHub history for details. */ -package org.opensearch.security.identity; +package org.opensearch.security.systemindex.sampleplugin; import java.util.Objects; import java.util.concurrent.Callable; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RestBulkIndexDocumentIntoMixOfSystemIndexAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestBulkIndexDocumentIntoMixOfSystemIndexAction.java similarity index 91% rename from src/integrationTest/java/org/opensearch/security/plugin/RestBulkIndexDocumentIntoMixOfSystemIndexAction.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestBulkIndexDocumentIntoMixOfSystemIndexAction.java index 0d1dc4fe01..54aa672d4c 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/RestBulkIndexDocumentIntoMixOfSystemIndexAction.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestBulkIndexDocumentIntoMixOfSystemIndexAction.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import java.util.List; @@ -26,12 +26,11 @@ import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; -import org.opensearch.security.identity.PluginContextSwitcher; import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.PUT; -import static org.opensearch.security.plugin.SystemIndexPlugin1.SYSTEM_INDEX_1; -import static org.opensearch.security.plugin.SystemIndexPlugin2.SYSTEM_INDEX_2; +import static org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1.SYSTEM_INDEX_1; +import static org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2.SYSTEM_INDEX_2; public class RestBulkIndexDocumentIntoMixOfSystemIndexAction extends BaseRestHandler { diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RestBulkIndexDocumentIntoSystemIndexAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestBulkIndexDocumentIntoSystemIndexAction.java similarity index 96% rename from src/integrationTest/java/org/opensearch/security/plugin/RestBulkIndexDocumentIntoSystemIndexAction.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestBulkIndexDocumentIntoSystemIndexAction.java index 56fa3ef2a9..8b37e54164 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/RestBulkIndexDocumentIntoSystemIndexAction.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestBulkIndexDocumentIntoSystemIndexAction.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import java.util.List; @@ -27,7 +27,6 @@ import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; -import org.opensearch.security.identity.PluginContextSwitcher; import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.PUT; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RestIndexDocumentIntoSystemIndexAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestIndexDocumentIntoSystemIndexAction.java similarity index 96% rename from src/integrationTest/java/org/opensearch/security/plugin/RestIndexDocumentIntoSystemIndexAction.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestIndexDocumentIntoSystemIndexAction.java index e668e8bccc..9092c97988 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/RestIndexDocumentIntoSystemIndexAction.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestIndexDocumentIntoSystemIndexAction.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import java.util.List; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RestRunClusterHealthAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestRunClusterHealthAction.java similarity index 93% rename from src/integrationTest/java/org/opensearch/security/plugin/RestRunClusterHealthAction.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestRunClusterHealthAction.java index 755f3278f0..f4674b4377 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/RestRunClusterHealthAction.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestRunClusterHealthAction.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import java.util.List; @@ -17,7 +17,6 @@ import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; -import org.opensearch.security.identity.PluginContextSwitcher; import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.GET; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthAction.java similarity index 91% rename from src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthAction.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthAction.java index b819465a35..e2072eeb79 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthAction.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthAction.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import org.opensearch.action.ActionType; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthRequest.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthRequest.java similarity index 94% rename from src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthRequest.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthRequest.java index 8ae08bd6ff..099264313e 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthRequest.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthRequest.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import java.io.IOException; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthResponse.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthResponse.java similarity index 95% rename from src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthResponse.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthResponse.java index 7a855744dc..a500755e22 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/RunClusterHealthResponse.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunClusterHealthResponse.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; // CS-SUPPRESS-SINGLE: RegexpSingleline It is not possible to use phrase "cluster manager" instead of master here import java.io.IOException; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/SystemIndexPlugin1.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java similarity index 97% rename from src/integrationTest/java/org/opensearch/security/plugin/SystemIndexPlugin1.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java index 3f1112c4b6..9805619965 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/SystemIndexPlugin1.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import java.util.Arrays; import java.util.Collection; @@ -39,7 +39,6 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.script.ScriptService; -import org.opensearch.security.identity.PluginContextSwitcher; import org.opensearch.threadpool.ThreadPool; import org.opensearch.watcher.ResourceWatcherService; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/SystemIndexPlugin2.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java similarity index 97% rename from src/integrationTest/java/org/opensearch/security/plugin/SystemIndexPlugin2.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java index 8fcf23e3db..f91cbd3226 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/SystemIndexPlugin2.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import java.util.Collection; import java.util.Collections; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/TransportIndexDocumentIntoSystemIndexAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/TransportIndexDocumentIntoSystemIndexAction.java similarity index 97% rename from src/integrationTest/java/org/opensearch/security/plugin/TransportIndexDocumentIntoSystemIndexAction.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/TransportIndexDocumentIntoSystemIndexAction.java index c549da7809..6138cfbb54 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/TransportIndexDocumentIntoSystemIndexAction.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/TransportIndexDocumentIntoSystemIndexAction.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.index.IndexRequest; @@ -21,7 +21,6 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.identity.IdentityService; import org.opensearch.identity.Subject; -import org.opensearch.security.identity.PluginContextSwitcher; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; import org.opensearch.tasks.Task; diff --git a/src/integrationTest/java/org/opensearch/security/plugin/TransportRunClusterHealthAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/TransportRunClusterHealthAction.java similarity index 97% rename from src/integrationTest/java/org/opensearch/security/plugin/TransportRunClusterHealthAction.java rename to src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/TransportRunClusterHealthAction.java index 4be06933a2..310262c947 100644 --- a/src/integrationTest/java/org/opensearch/security/plugin/TransportRunClusterHealthAction.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/TransportRunClusterHealthAction.java @@ -8,7 +8,7 @@ * */ -package org.opensearch.security.plugin; +package org.opensearch.security.systemindex.sampleplugin; import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; import org.opensearch.action.admin.cluster.health.ClusterHealthResponse; @@ -19,7 +19,6 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.identity.IdentityService; import org.opensearch.identity.Subject; -import org.opensearch.security.identity.PluginContextSwitcher; import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.TransportService; diff --git a/src/integrationTest/java/org/opensearch/test/framework/matcher/RestMatchers.java b/src/integrationTest/java/org/opensearch/test/framework/matcher/RestMatchers.java new file mode 100644 index 0000000000..b885074688 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/test/framework/matcher/RestMatchers.java @@ -0,0 +1,68 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + */ +package org.opensearch.test.framework.matcher; + +import org.hamcrest.Description; +import org.hamcrest.DiagnosingMatcher; + +import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; + +public class RestMatchers { + + private RestMatchers() {} + + public static DiagnosingMatcher isForbidden(String jsonPointer, String patternString) { + return new DiagnosingMatcher() { + + @Override + public void describeTo(Description description) { + description.appendText("Response has status 403 Forbidden with a JSON response that has the value ") + .appendValue(patternString) + .appendText(" at ") + .appendValue(jsonPointer); + } + + @Override + protected boolean matches(Object item, Description mismatchDescription) { + if (!(item instanceof HttpResponse)) { + mismatchDescription.appendValue(item).appendText(" is not a HttpResponse"); + return false; + } + + HttpResponse response = (HttpResponse) item; + + if (response.getStatusCode() != 403) { + mismatchDescription.appendText("Status is not 403 Forbidden: ").appendText("\n").appendValue(item); + return false; + } + + try { + String value = response.getTextFromJsonBody(jsonPointer); + + if (value == null) { + mismatchDescription.appendText("Could not find value at " + jsonPointer).appendText("\n").appendValue(item); + return false; + } + + if (value.contains(patternString)) { + return true; + } else { + mismatchDescription.appendText("Value at " + jsonPointer + " does not match pattern: " + patternString + "\n") + .appendValue(item); + return false; + } + } catch (Exception e) { + mismatchDescription.appendText("Parsing request body failed with " + e).appendText("\n").appendValue(item); + return false; + } + } + }; + } +} diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 42aa13f505..a1b1899c32 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -391,16 +391,11 @@ public boolean authenticate(final SecurityRequestChannel request) { if (authenticated) { final User impersonatedUser = impersonate(request, authenticatedUser); - threadPool.getThreadContext() - .putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, impersonatedUser == null ? authenticatedUser : impersonatedUser); - UserSubject subject = new SecurityUser(threadPool, impersonatedUser == null ? authenticatedUser : impersonatedUser); + final User effectiveUser = impersonatedUser == null ? authenticatedUser : impersonatedUser; + threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, effectiveUser); + UserSubject subject = new SecurityUser(threadPool, effectiveUser); threadPool.getThreadContext().putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject); - auditLog.logSucceededLogin( - (impersonatedUser == null ? authenticatedUser : impersonatedUser).getName(), - false, - authenticatedUser.getName(), - request - ); + auditLog.logSucceededLogin(effectiveUser.getName(), false, authenticatedUser.getName(), request); } else { if (isDebugEnabled) { log.debug("User still not authenticated after checking {} auth domains", restAuthDomains.size()); diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java index 6523cf94a7..915514264c 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java @@ -47,10 +47,6 @@ public Set getMissingPrivileges() { return new HashSet(missingPrivileges); } - public boolean addMissingPrivileges(String action) { - return missingPrivileges.add(action); - } - public Set getMissingSecurityRoles() { return new HashSet<>(missingSecurityRoles); } diff --git a/src/main/java/org/opensearch/security/privileges/SnapshotRestoreEvaluator.java b/src/main/java/org/opensearch/security/privileges/SnapshotRestoreEvaluator.java index 8a11e433e8..23612e1a52 100644 --- a/src/main/java/org/opensearch/security/privileges/SnapshotRestoreEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/SnapshotRestoreEvaluator.java @@ -109,7 +109,6 @@ public PrivilegesEvaluatorResponse evaluate( auditLog.logSecurityIndexAttempt(request, action, task); log.warn("{} for '{}' as source index is not allowed", action, securityIndex); presponse.allowed = false; - presponse.addMissingPrivileges(action); return presponse.markComplete(); } return presponse; diff --git a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java index 00a2791ba3..af0ad33fb8 100644 --- a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java @@ -272,12 +272,10 @@ private void evaluateSystemIndicesAccess( log.debug("Service account cannot access regular indices: {}", regularIndices); } presponse.allowed = false; - presponse.addMissingPrivileges(action); presponse.markComplete(); return; } boolean containsProtectedIndex = requestContainsAnyProtectedSystemIndices(requestedResolved); - if (containsProtectedIndex) { auditLog.logSecurityIndexAttempt(request, action, task); if (log.isInfoEnabled()) { @@ -289,7 +287,6 @@ private void evaluateSystemIndicesAccess( ); } presponse.allowed = false; - presponse.addMissingPrivileges(action); presponse.markComplete(); return; } else if (containsSystemIndex @@ -310,13 +307,12 @@ private void evaluateSystemIndicesAccess( ); } presponse.allowed = false; - presponse.addMissingPrivileges(action); presponse.markComplete(); return; } } - if (user.isPluginUser()) { + if (user.isPluginUser() && !requestedResolved.isLocalAll()) { Set matchingSystemIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern( user.getName().replace("plugin:", ""), requestedResolved.getAllIndices() @@ -335,7 +331,6 @@ private void evaluateSystemIndicesAccess( ); } presponse.allowed = false; - presponse.addMissingPrivileges(action); presponse.markComplete(); } return; @@ -358,7 +353,6 @@ private void evaluateSystemIndicesAccess( auditLog.logSecurityIndexAttempt(request, action, task); log.warn("{} for '_all' indices is not allowed for a regular user", action); presponse.allowed = false; - presponse.addMissingPrivileges(action); presponse.markComplete(); } } @@ -373,7 +367,6 @@ else if (containsSystemIndex && !isSystemIndexPermissionEnabled) { log.debug("Filtered '{}' but resulting list is empty", securityIndex); } presponse.allowed = false; - presponse.addMissingPrivileges(action); presponse.markComplete(); return; } @@ -386,7 +379,6 @@ else if (containsSystemIndex && !isSystemIndexPermissionEnabled) { final String foundSystemIndexes = String.join(", ", getAllSystemIndices(requestedResolved)); log.warn("{} for '{}' index is not allowed for a regular user", action, foundSystemIndexes); presponse.allowed = false; - presponse.addMissingPrivileges(action); presponse.markComplete(); } } diff --git a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java index 099e025c31..d91b897c81 100644 --- a/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java +++ b/src/main/java/org/opensearch/security/transport/SecurityInterceptor.java @@ -131,8 +131,8 @@ public SecurityRequestHandler getHandler(String private User determineUser(Connection connection) { User user0 = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); - // pluginUser did not exist prior to 2.18.0 - if (user0 != null && user0.isPluginUser() && connection.getVersion().before(Version.V_2_18_0)) { + // pluginUser did not exist prior to 2.19.0 + if (user0 != null && user0.isPluginUser() && connection.getVersion().before(Version.V_2_19_0)) { user0 = null; } return user0; diff --git a/src/test/java/org/opensearch/security/identity/ContextProvidingPluginSubjectTests.java b/src/test/java/org/opensearch/security/identity/ContextProvidingPluginSubjectTests.java index 48851c48b3..fd337ef1eb 100644 --- a/src/test/java/org/opensearch/security/identity/ContextProvidingPluginSubjectTests.java +++ b/src/test/java/org/opensearch/security/identity/ContextProvidingPluginSubjectTests.java @@ -25,7 +25,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_USER; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThrows; public class ContextProvidingPluginSubjectTests { static class TestIdentityAwarePlugin extends Plugin implements IdentityAwarePlugin { @@ -55,37 +54,4 @@ public void testSecurityUserSubjectRunAs() throws Exception { SecurityUserTests.terminate(threadPool); } - - @Test - public void testPluginContextSwitcherRunAs() throws Exception { - final ThreadPool threadPool = new TestThreadPool(getClass().getName()); - - final Plugin testPlugin = new TestIdentityAwarePlugin(); - - final PluginContextSwitcher contextSwitcher = new PluginContextSwitcher(); - - final User pluginUser = new User("plugin:" + testPlugin.getClass().getCanonicalName()); - - ContextProvidingPluginSubject subject = new ContextProvidingPluginSubject(threadPool, Settings.EMPTY, testPlugin); - - contextSwitcher.initialize(subject); - - assertNull(threadPool.getThreadContext().getTransient(OPENDISTRO_SECURITY_USER)); - - subject.runAs(() -> { - assertThat(threadPool.getThreadContext().getTransient(OPENDISTRO_SECURITY_USER), equalTo(pluginUser)); - return null; - }); - - assertNull(threadPool.getThreadContext().getTransient(OPENDISTRO_SECURITY_USER)); - - SecurityUserTests.terminate(threadPool); - } - - @Test - public void testPluginContextSwitcherUninitializedRunAs() throws Exception { - final PluginContextSwitcher contextSwitcher = new PluginContextSwitcher(); - - assertThrows(NullPointerException.class, () -> contextSwitcher.runAs(() -> null)); - } } diff --git a/src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java b/src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java index fcf67aa5a5..58e811fa24 100644 --- a/src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java +++ b/src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java @@ -46,7 +46,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.opensearch.security.support.ConfigConstants.SYSTEM_INDEX_PERMISSION; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -152,8 +151,6 @@ public void setup( when(log.isDebugEnabled()).thenReturn(true); when(log.isInfoEnabled()).thenReturn(true); - when(presponse.addMissingPrivileges(any())).thenReturn(true); - doReturn(ImmutableSet.of(index)).when(ip).getResolvedIndexPattern(user, indexNameExpressionResolver, cs, true, false); } @@ -291,7 +288,6 @@ public void testUnprotectedActionOnSystemIndex_systemIndexPermissionEnabled_With verify(auditLog).logSecurityIndexAttempt(request, UNPROTECTED_ACTION, null); verify(log).isInfoEnabled(); verify(log).info("No {} permission for user roles {} to System Indices {}", UNPROTECTED_ACTION, securityRoles, TEST_SYSTEM_INDEX); - verify(presponse).addMissingPrivileges(UNPROTECTED_ACTION); } @Test @@ -446,7 +442,6 @@ public void testDisableCacheOrRealtimeOnSystemIndex_systemIndexPermissionEnabled ); verify(log).debug("Disable search request cache for this request"); verify(log).debug("Disable realtime for this request"); - verify(presponse, times(3)).addMissingPrivileges(UNPROTECTED_ACTION); } @Test @@ -500,7 +495,6 @@ public void testProtectedActionLocalAll_systemIndexDisabled() { verify(auditLog).logSecurityIndexAttempt(request, PROTECTED_ACTION, task); assertThat(presponse.allowed, is(false)); - verify(presponse).addMissingPrivileges(PROTECTED_ACTION); verify(presponse).markComplete(); verify(log).warn("{} for '_all' indices is not allowed for a regular user", "indices:data/write"); } @@ -515,7 +509,6 @@ public void testProtectedActionLocalAll_systemIndexPermissionDisabled() { verify(auditLog).logSecurityIndexAttempt(request, PROTECTED_ACTION, task); assertThat(presponse.allowed, is(false)); - verify(presponse).addMissingPrivileges(PROTECTED_ACTION); verify(presponse).markComplete(); verify(log).warn("{} for '_all' indices is not allowed for a regular user", PROTECTED_ACTION); } @@ -530,7 +523,6 @@ public void testProtectedActionLocalAll_systemIndexPermissionEnabled() { verify(auditLog).logSecurityIndexAttempt(request, PROTECTED_ACTION, task); assertThat(presponse.allowed, is(false)); - verify(presponse).addMissingPrivileges(PROTECTED_ACTION); verify(presponse).markComplete(); verify(log).warn("{} for '_all' indices is not allowed for a regular user", PROTECTED_ACTION); } @@ -589,7 +581,6 @@ public void testProtectedActionOnSystemIndex_systemIndexPermissionDisabled() { verify(auditLog).logSecurityIndexAttempt(request, PROTECTED_ACTION, task); assertThat(presponse.allowed, is(false)); - verify(presponse).addMissingPrivileges(PROTECTED_ACTION); verify(presponse).markComplete(); verify(log).warn("{} for '{}' index is not allowed for a regular user", PROTECTED_ACTION, TEST_SYSTEM_INDEX); } @@ -607,7 +598,6 @@ public void testProtectedActionOnSystemIndex_systemIndexPermissionEnabled_withou verify(presponse).markComplete(); verify(log).isInfoEnabled(); verify(log).info("No {} permission for user roles {} to System Indices {}", PROTECTED_ACTION, securityRoles, TEST_SYSTEM_INDEX); - verify(presponse).addMissingPrivileges(PROTECTED_ACTION); } @Test @@ -632,7 +622,6 @@ public void testProtectedActionOnProtectedSystemIndex_systemIndexDisabled() { verify(auditLog).logSecurityIndexAttempt(request, PROTECTED_ACTION, task); assertThat(presponse.allowed, is(false)); - verify(presponse).addMissingPrivileges(PROTECTED_ACTION); verify(presponse).markComplete(); verify(log).warn("{} for '{}' index is not allowed for a regular user", PROTECTED_ACTION, SECURITY_INDEX); @@ -648,7 +637,6 @@ public void testProtectedActionOnProtectedSystemIndex_systemIndexPermissionDisab verify(auditLog).logSecurityIndexAttempt(request, PROTECTED_ACTION, task); assertThat(presponse.allowed, is(false)); - verify(presponse).addMissingPrivileges(PROTECTED_ACTION); verify(presponse).markComplete(); verify(log).warn("{} for '{}' index is not allowed for a regular user", PROTECTED_ACTION, SECURITY_INDEX); @@ -684,7 +672,6 @@ private void testSecurityIndexAccess(String action) { verify(auditLog).logSecurityIndexAttempt(request, action, task); assertThat(presponse.allowed, is(false)); - verify(presponse).addMissingPrivileges(action); verify(presponse).markComplete(); verify(log).isInfoEnabled(); diff --git a/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java b/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java index 65e0c4cbb8..2e154060e9 100644 --- a/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java +++ b/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java @@ -185,7 +185,6 @@ String permissionExceptionMessage(String action, String username) { } void validateForbiddenResponse(RestHelper.HttpResponse response, String action, String user) { - assertThat(response.getStatusCode(), is(RestStatus.FORBIDDEN.getStatus())); MatcherAssert.assertThat(response.getBody(), Matchers.containsStringIgnoringCase(permissionExceptionMessage(action, user))); } @@ -195,7 +194,7 @@ void shouldBeAllowedOnlyForAuthorizedIndices(String index, RestHelper.HttpRespon boolean isRequestingAccessToNonAuthorizedSystemIndex = (!user.equals(allAccessUser) && index.equals(SYSTEM_INDEX_WITH_NO_ASSOCIATED_ROLE_PERMISSIONS)); if (isSecurityIndexRequest || isRequestingAccessToNonAuthorizedSystemIndex) { - validateForbiddenResponse(response, action, user); + validateForbiddenResponse(response, isSecurityIndexRequest ? "" : action, user); } else { assertThat(response.getStatusCode(), is(RestStatus.OK.getStatus())); } diff --git a/src/test/java/org/opensearch/security/system_indices/SystemIndexDisabledTests.java b/src/test/java/org/opensearch/security/system_indices/SystemIndexDisabledTests.java index 391519a481..eb56cc6fb6 100644 --- a/src/test/java/org/opensearch/security/system_indices/SystemIndexDisabledTests.java +++ b/src/test/java/org/opensearch/security/system_indices/SystemIndexDisabledTests.java @@ -123,7 +123,7 @@ public void testDeleteAsSuperAdmin() { @Test public void testDeleteAsAdmin() { - testDeleteWithUser(allAccessUser, allAccessUserHeader, "indices:admin/delete", "indices:data/write/delete"); + testDeleteWithUser(allAccessUser, allAccessUserHeader, "", ""); } @Test @@ -175,7 +175,7 @@ public void testCloseOpenAsAdmin() { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePostRequest(index + "/_close", "", allAccessUserHeader); - shouldBeAllowedOnlyForAuthorizedIndices(index, response, "indices:admin/close", allAccessUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, response, "", allAccessUser); // User can open the index but cannot close it response = restHelper.executePostRequest(index + "/_open", "", allAccessUserHeader); @@ -348,14 +348,14 @@ public void testSnapshotSystemIndicesAsAdmin() { assertThat(res.getStatusCode(), is(HttpStatus.SC_UNAUTHORIZED)); res = restHelper.executePostRequest(snapshotRequest + "/_restore?wait_for_completion=true", "", allAccessUserHeader); - shouldBeAllowedOnlyForAuthorizedIndices(index, res, "cluster:admin/snapshot/restore", allAccessUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, res, "", allAccessUser); res = restHelper.executePostRequest( snapshotRequest + "/_restore?wait_for_completion=true", "{ \"rename_pattern\": \"(.+)\", \"rename_replacement\": \"restored_index_with_global_state_$1\" }", allAccessUserHeader ); - shouldBeAllowedOnlyForAuthorizedIndices(index, res, "cluster:admin/snapshot/restore", allAccessUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, res, "", allAccessUser); } } @@ -384,9 +384,7 @@ private void testSnapshotWithUser(String user, Header header) { RestHelper.HttpResponse res = restHelper.executeGetRequest(snapshotRequest); assertThat(res.getStatusCode(), is(HttpStatus.SC_UNAUTHORIZED)); - String action = index.equals(ACCESSIBLE_ONLY_BY_SUPER_ADMIN) - ? "cluster:admin/snapshot/restore" - : "indices:data/write/index, indices:admin/create"; + String action = index.equals(ACCESSIBLE_ONLY_BY_SUPER_ADMIN) ? "" : "indices:data/write/index, indices:admin/create"; res = restHelper.executePostRequest(snapshotRequest + "/_restore?wait_for_completion=true", "", header); shouldBeAllowedOnlyForAuthorizedIndices(index, res, action, user); diff --git a/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionDisabledTests.java b/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionDisabledTests.java index 4d8cf41dbc..da868aa5f8 100644 --- a/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionDisabledTests.java +++ b/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionDisabledTests.java @@ -140,10 +140,10 @@ private void testDeleteWithUser(String user, Header header) { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executeDeleteRequest(index + "/_doc/document1", header); - validateForbiddenResponse(response, "indices:data/write/delete", user); + validateForbiddenResponse(response, "", user); response = restHelper.executeDeleteRequest(index, header); - validateForbiddenResponse(response, "indices:admin/delete", user); + validateForbiddenResponse(response, "", user); } } @@ -169,7 +169,7 @@ public void testCloseOpenAsAdmin() { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePostRequest(index + "/_close", "", allAccessUserHeader); - validateForbiddenResponse(response, "indices:admin/close", allAccessUser); + validateForbiddenResponse(response, "", allAccessUser); // admin cannot close any system index but can open them response = restHelper.executePostRequest(index + "/_open", "", allAccessUserHeader); @@ -192,7 +192,7 @@ private void testCloseOpenWithUser(String user, Header header) { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePostRequest(index + "/_close", "", header); - validateForbiddenResponse(response, "indices:admin/close", user); + validateForbiddenResponse(response, "", user); // normal user cannot open or close security index response = restHelper.executePostRequest(index + "/_open", "", header); @@ -284,10 +284,10 @@ private void testUpdateWithUser(String user, Header header) { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePutRequest(index + "/_mapping", newMappings, header); - validateForbiddenResponse(response, "indices:admin/mapping/put", user); + validateForbiddenResponse(response, "", user); response = restHelper.executePutRequest(index + "/_settings", updateIndexSettings, header); - validateForbiddenResponse(response, "indices:admin/settings/update", user); + validateForbiddenResponse(response, "", user); } } @@ -346,14 +346,14 @@ public void testSnapshotSystemIndicesAsAdmin() { "", allAccessUserHeader ); - validateForbiddenResponse(res, "cluster:admin/snapshot/restore", allAccessUser); + validateForbiddenResponse(res, "", allAccessUser); res = restHelper.executePostRequest( "_snapshot/" + index + "/" + index + "_1/_restore?wait_for_completion=true", "{ \"rename_pattern\": \"(.+)\", \"rename_replacement\": \"restored_index_with_global_state_$1\" }", allAccessUserHeader ); - shouldBeAllowedOnlyForAuthorizedIndices(index, res, "cluster:admin/snapshot/restore", allAccessUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, res, "", allAccessUser); } } @@ -382,7 +382,7 @@ private void testSnapshotSystemIndexWithUser(String user, Header header) { assertThat(res.getStatusCode(), is(HttpStatus.SC_UNAUTHORIZED)); res = restHelper.executePostRequest("_snapshot/" + index + "/" + index + "_1/_restore?wait_for_completion=true", "", header); - validateForbiddenResponse(res, "cluster:admin/snapshot/restore", user); + validateForbiddenResponse(res, "", user); res = restHelper.executePostRequest( "_snapshot/" + index + "/" + index + "_1/_restore?wait_for_completion=true", @@ -390,7 +390,7 @@ private void testSnapshotSystemIndexWithUser(String user, Header header) { header ); if (index.equals(ACCESSIBLE_ONLY_BY_SUPER_ADMIN)) { - validateForbiddenResponse(res, "cluster:admin/snapshot/restore", user); + validateForbiddenResponse(res, "", user); } else { validateForbiddenResponse(res, "indices:data/write/index, indices:admin/create", user); } diff --git a/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java b/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java index 90ec9eaa29..4c22b2ed50 100644 --- a/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java +++ b/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java @@ -59,7 +59,7 @@ public void testSearchAsAdmin() { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePostRequest(index + "/_search", matchAllQuery, allAccessUserHeader); // no system indices are searchable by admin - validateForbiddenResponse(response, "indices:data/read/search", allAccessUser); + validateForbiddenResponse(response, "", allAccessUser); } // search all indices @@ -78,7 +78,7 @@ public void testSearchAsNormalUser() throws Exception { // security index is only accessible by super-admin RestHelper.HttpResponse response = restHelper.executePostRequest(index + "/_search", "", normalUserHeader); if (index.equals(ACCESSIBLE_ONLY_BY_SUPER_ADMIN) || index.equals(SYSTEM_INDEX_WITH_NO_ASSOCIATED_ROLE_PERMISSIONS)) { - validateForbiddenResponse(response, "indices:data/read/search", normalUser); + validateForbiddenResponse(response, "", normalUser); } else { // got 1 hits because system index permissions are enabled validateSearchResponse(response, 1); @@ -98,7 +98,7 @@ public void testSearchAsNormalUserWithoutSystemIndexAccess() { // search system indices for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePostRequest(index + "/_search", "", normalUserWithoutSystemIndexHeader); - validateForbiddenResponse(response, "indices:data/read/search", normalUserWithoutSystemIndex); + validateForbiddenResponse(response, "", normalUserWithoutSystemIndex); } // search all indices @@ -151,10 +151,10 @@ public void testDeleteAsAdmin() { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executeDeleteRequest(index + "/_doc/document1", allAccessUserHeader); - validateForbiddenResponse(response, "indices:data/write/delete", allAccessUser); + validateForbiddenResponse(response, "", allAccessUser); response = restHelper.executeDeleteRequest(index, allAccessUserHeader); - validateForbiddenResponse(response, "indices:admin/delete", allAccessUser); + validateForbiddenResponse(response, "", allAccessUser); } } @@ -166,10 +166,10 @@ public void testDeleteAsNormalUser() { // permission for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executeDeleteRequest(index + "/_doc/document1", normalUserHeader); - shouldBeAllowedOnlyForAuthorizedIndices(index, response, "indices:data/write/delete", normalUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, response, "", normalUser); response = restHelper.executeDeleteRequest(index, normalUserHeader); - shouldBeAllowedOnlyForAuthorizedIndices(index, response, "indices:admin/delete", normalUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, response, "", normalUser); } } @@ -183,10 +183,10 @@ public void testDeleteAsNormalUserWithoutSystemIndexAccess() { index + "/_doc/document1", normalUserWithoutSystemIndexHeader ); - validateForbiddenResponse(response, "indices:data/write/delete", normalUserWithoutSystemIndex); + validateForbiddenResponse(response, "", normalUserWithoutSystemIndex); response = restHelper.executeDeleteRequest(index, normalUserWithoutSystemIndexHeader); - validateForbiddenResponse(response, "indices:admin/delete", normalUserWithoutSystemIndex); + validateForbiddenResponse(response, "", normalUserWithoutSystemIndex); } } @@ -217,11 +217,11 @@ public void testCloseOpenAsNormalUser() { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePostRequest(index + "/_close", "", normalUserHeader); - shouldBeAllowedOnlyForAuthorizedIndices(index, response, "indices:admin/close", normalUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, response, "", normalUser); // normal user cannot open or close security index response = restHelper.executePostRequest(index + "/_open", "", normalUserHeader); - shouldBeAllowedOnlyForAuthorizedIndices(index, response, "indices:admin/open", normalUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, response, "", normalUser); } } @@ -235,11 +235,11 @@ private void testCloseOpenWithUser(String user, Header header) { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePostRequest(index + "/_close", "", header); - validateForbiddenResponse(response, "indices:admin/close", user); + validateForbiddenResponse(response, "", user); // admin or normal user (without system index permission) cannot open or close any system index response = restHelper.executePostRequest(index + "/_open", "", header); - validateForbiddenResponse(response, "indices:admin/open", user); + validateForbiddenResponse(response, "", user); } } @@ -314,10 +314,10 @@ public void testUpdateAsNormalUser() { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePutRequest(index + "/_settings", updateIndexSettings, normalUserHeader); - shouldBeAllowedOnlyForAuthorizedIndices(index, response, "indices:admin/settings/update", normalUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, response, "", normalUser); response = restHelper.executePutRequest(index + "/_mapping", newMappings, normalUserHeader); - shouldBeAllowedOnlyForAuthorizedIndices(index, response, "indices:admin/mapping/put", normalUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, response, "", normalUser); } } @@ -331,10 +331,10 @@ private void testUpdateWithUser(String user, Header header) { for (String index : SYSTEM_INDICES) { RestHelper.HttpResponse response = restHelper.executePutRequest(index + "/_settings", updateIndexSettings, header); - validateForbiddenResponse(response, "indices:admin/settings/update", user); + validateForbiddenResponse(response, "", user); response = restHelper.executePutRequest(index + "/_mapping", newMappings, header); - validateForbiddenResponse(response, "indices:admin/mapping/put", user); + validateForbiddenResponse(response, "", user); } } @@ -393,14 +393,14 @@ public void testSnapshotSystemIndicesAsAdmin() { "", allAccessUserHeader ); - validateForbiddenResponse(res, "cluster:admin/snapshot/restore", allAccessUser); + validateForbiddenResponse(res, "", allAccessUser); res = restHelper.executePostRequest( "_snapshot/" + index + "/" + index + "_1/_restore?wait_for_completion=true", "{ \"rename_pattern\": \"(.+)\", \"rename_replacement\": \"restored_index_with_global_state_$1\" }", allAccessUserHeader ); - shouldBeAllowedOnlyForAuthorizedIndices(index, res, "cluster:admin/snapshot/restore", allAccessUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, res, "", allAccessUser); } } @@ -424,7 +424,7 @@ public void testSnapshotSystemIndicesAsNormalUser() { "", normalUserHeader ); - shouldBeAllowedOnlyForAuthorizedIndices(index, res, "cluster:admin/snapshot/restore", normalUser); + shouldBeAllowedOnlyForAuthorizedIndices(index, res, "", normalUser); res = restHelper.executePostRequest( "_snapshot/" + index + "/" + index + "_1/_restore?wait_for_completion=true", @@ -432,9 +432,7 @@ public void testSnapshotSystemIndicesAsNormalUser() { normalUserHeader ); - String action = index.equals(ACCESSIBLE_ONLY_BY_SUPER_ADMIN) - ? "cluster:admin/snapshot/restore" - : "indices:data/write/index, indices:admin/create"; + String action = index.equals(ACCESSIBLE_ONLY_BY_SUPER_ADMIN) ? "" : "indices:data/write/index, indices:admin/create"; validateForbiddenResponse(res, action, normalUser); } } @@ -459,16 +457,14 @@ public void testSnapshotSystemIndicesAsNormalUserWithoutSystemIndexAccess() { "", normalUserWithoutSystemIndexHeader ); - validateForbiddenResponse(res, "cluster:admin/snapshot/restore", normalUserWithoutSystemIndex); + validateForbiddenResponse(res, "", normalUserWithoutSystemIndex); res = restHelper.executePostRequest( "_snapshot/" + index + "/" + index + "_1/_restore?wait_for_completion=true", "{ \"rename_pattern\": \"(.+)\", \"rename_replacement\": \"restored_index_with_global_state_$1\" }", normalUserWithoutSystemIndexHeader ); - String action = index.equals(ACCESSIBLE_ONLY_BY_SUPER_ADMIN) - ? "cluster:admin/snapshot/restore" - : "indices:data/write/index, indices:admin/create"; + String action = index.equals(ACCESSIBLE_ONLY_BY_SUPER_ADMIN) ? "" : "indices:data/write/index, indices:admin/create"; validateForbiddenResponse(res, action, normalUserWithoutSystemIndex); } }