diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java new file mode 100644 index 0000000000..109e5228cf --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java @@ -0,0 +1,88 @@ +/* + * 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.security.systemindex; + +import java.util.List; +import java.util.Map; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.core.rest.RestStatus; +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 static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +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.test.framework.TestSecurityConfig.Role.ALL_ACCESS; +import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; + +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class SystemIndexDisabledTests { + + public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal"); + + @ClassRule + public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .anonymousAuth(false) + .authc(AUTHC_DOMAIN) + .users(USER_ADMIN) + .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) + .nodeSettings( + Map.of( + SECURITY_RESTAPI_ROLES_ENABLED, + List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), + SECURITY_SYSTEM_INDICES_ENABLED_KEY, + false + ) + ) + .build(); + + @Before + public void setup() { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + client.delete(".system-index1"); + } + } + + @Test + public void testPluginShouldBeAbleToIndexIntoAnySystemIndexWhenProtectionIsDisabled() { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + client.put(".system-index1"); + client.put(".system-index2"); + } + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { + HttpResponse response = client.put("try-create-and-bulk-mixed-index"); + + response.assertStatusCode(RestStatus.OK.getStatus()); + + assertThat( + response.getBody(), + not( + containsString( + "no permissions for [] and User [name=plugin:org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1" + ) + ) + ); + } + } +} diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java index 13325b3029..8294d912e9 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java @@ -13,6 +13,7 @@ import java.util.Map; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import com.fasterxml.jackson.databind.JsonNode; import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; @@ -170,6 +171,73 @@ public void testPluginShouldBeAbleToBulkIndexDocumentIntoItsSystemIndex() { } } + @Test + public void testPluginShouldBeAbleSearchOnItsSystemIndex() { + JsonNode searchResponse1; + JsonNode searchResponse2; + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { + HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1); + + assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + + HttpResponse searchResponse = client.get("search-on-system-index/" + SYSTEM_INDEX_1); + + assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2)); + + searchResponse1 = searchResponse.bodyAsJsonNode(); + } + + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + HttpResponse searchResponse = client.get(SYSTEM_INDEX_1 + "/_search"); + + assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2)); + + searchResponse2 = searchResponse.bodyAsJsonNode(); + } + + JsonNode hits1 = searchResponse1.get("hits"); + JsonNode hits2 = searchResponse2.get("hits"); + assertThat(hits1.toPrettyString(), equalTo(hits2.toPrettyString())); + } + + @Test + public void testPluginShouldBeAbleGetOnItsSystemIndex() { + JsonNode getResponse1; + JsonNode getResponse2; + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { + HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1); + + assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + + HttpResponse searchResponse = client.get("search-on-system-index/" + SYSTEM_INDEX_1); + + assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2)); + + String docId = searchResponse.getTextFromJsonBody("/hits/hits/0/_id"); + + HttpResponse getResponse = client.get("get-on-system-index/" + SYSTEM_INDEX_1 + "/" + docId); + + getResponse1 = getResponse.bodyAsJsonNode(); + } + + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + HttpResponse searchResponse = client.get(SYSTEM_INDEX_1 + "/_search"); + + assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2)); + + String docId = searchResponse.getTextFromJsonBody("/hits/hits/0/_id"); + + HttpResponse getResponse = client.get(SYSTEM_INDEX_1 + "/_doc/" + docId); + + getResponse2 = getResponse.bodyAsJsonNode(); + } + assertThat(getResponse1.toPrettyString(), equalTo(getResponse2.toPrettyString())); + } + @Test public void testPluginShouldNotBeAbleToBulkIndexDocumentIntoMixOfSystemIndexWhereAtLeastOneDoesNotBelongToPlugin() { try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestGetOnSystemIndexAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestGetOnSystemIndexAction.java new file mode 100644 index 0000000000..86ed4ce4d7 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestGetOnSystemIndexAction.java @@ -0,0 +1,62 @@ +/* + * 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.security.systemindex.sampleplugin; + +import java.util.List; + +import org.opensearch.action.get.GetRequest; +import org.opensearch.client.node.NodeClient; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestRequest; + +import static java.util.Collections.singletonList; +import static org.opensearch.rest.RestRequest.Method.GET; + +public class RestGetOnSystemIndexAction extends BaseRestHandler { + + private final RunAsSubjectClient pluginClient; + + public RestGetOnSystemIndexAction(RunAsSubjectClient pluginClient) { + this.pluginClient = pluginClient; + } + + @Override + public List routes() { + return singletonList(new Route(GET, "/get-on-system-index/{index}/{docId}")); + } + + @Override + public String getName() { + return "test_get_on_system_index_action"; + } + + @Override + public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { + String indexName = request.param("index"); + String docId = request.param("docId"); + return new RestChannelConsumer() { + + @Override + public void accept(RestChannel channel) throws Exception { + GetRequest getRequest = new GetRequest(indexName); + getRequest.id(docId); + pluginClient.get(getRequest, ActionListener.wrap(r -> { + channel.sendResponse(new BytesRestResponse(RestStatus.OK, r.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS))); + }, fr -> { channel.sendResponse(new BytesRestResponse(RestStatus.FORBIDDEN, String.valueOf(fr))); })); + } + }; + } +} diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestSearchOnSystemIndexAction.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestSearchOnSystemIndexAction.java new file mode 100644 index 0000000000..4f9bcf6cba --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestSearchOnSystemIndexAction.java @@ -0,0 +1,65 @@ +/* + * 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.security.systemindex.sampleplugin; + +import java.util.List; + +import org.opensearch.action.search.SearchRequest; +import org.opensearch.client.node.NodeClient; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.index.query.QueryBuilders; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestRequest; +import org.opensearch.search.builder.SearchSourceBuilder; + +import static java.util.Collections.singletonList; +import static org.opensearch.rest.RestRequest.Method.GET; + +public class RestSearchOnSystemIndexAction extends BaseRestHandler { + + private final RunAsSubjectClient pluginClient; + + public RestSearchOnSystemIndexAction(RunAsSubjectClient pluginClient) { + this.pluginClient = pluginClient; + } + + @Override + public List routes() { + return singletonList(new Route(GET, "/search-on-system-index/{index}")); + } + + @Override + public String getName() { + return "test_search_on_system_index_action"; + } + + @Override + public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) { + String indexName = request.param("index"); + return new RestChannelConsumer() { + + @Override + public void accept(RestChannel channel) throws Exception { + SearchRequest searchRequest = new SearchRequest(indexName); + SearchSourceBuilder sourceBuilder = new SearchSourceBuilder(); + sourceBuilder.query(QueryBuilders.matchAllQuery()); + searchRequest.source(sourceBuilder); + pluginClient.search(searchRequest, ActionListener.wrap(r -> { + channel.sendResponse(new BytesRestResponse(RestStatus.OK, r.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS))); + }, fr -> { channel.sendResponse(new BytesRestResponse(RestStatus.FORBIDDEN, String.valueOf(fr))); })); + } + }; + } +} diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java index 9113a50a5f..edd90d0568 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java @@ -88,7 +88,9 @@ public List getRestHandlers( new RestIndexDocumentIntoSystemIndexAction(client), new RestRunClusterHealthAction(client), new RestBulkIndexDocumentIntoSystemIndexAction(client, pluginClient), - new RestBulkIndexDocumentIntoMixOfSystemIndexAction(client, pluginClient) + new RestBulkIndexDocumentIntoMixOfSystemIndexAction(client, pluginClient), + new RestSearchOnSystemIndexAction(pluginClient), + new RestGetOnSystemIndexAction(pluginClient) ); } diff --git a/src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java index b87c92c356..080e2a8a08 100644 --- a/src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java +++ b/src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java @@ -163,7 +163,7 @@ protected final boolean isBlockedSystemIndexRequest() { if (systemIndexPermissionEnabled) { final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); - if (user == null) { + if (HeaderHelper.isInternalOrPluginRequest(threadContext)) { // allow request without user from plugin. return systemIndexMatcher.test(index.getName()) || matchesSystemIndexRegisteredWithCore; } @@ -180,8 +180,7 @@ protected final boolean isBlockedSystemIndexRequest() { protected final boolean isAdminDnOrPluginRequest() { final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); - if (user == null) { - // allow request without user from plugin. + if (HeaderHelper.isInternalOrPluginRequest(threadContext)) { return true; } else if (adminDns.isAdmin(user)) { return true; diff --git a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java index f6a786c514..ba03ff2f25 100644 --- a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java @@ -304,7 +304,7 @@ private void evaluateSystemIndicesAccess( // cluster actions will return true for requestedResolved.isLocalAll() // the following section should only be run for index actions - if (user.isPluginUser() && !requestedResolved.isLocalAll()) { + if (this.isSystemIndexEnabled && user.isPluginUser() && !requestedResolved.isLocalAll()) { Set matchingSystemIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern( user.getName().replace("plugin:", ""), requestedResolved.getAllIndices() diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/DlsFlsBaseContext.java b/src/main/java/org/opensearch/security/privileges/dlsfls/DlsFlsBaseContext.java index 232e2d7422..8a27fef255 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/DlsFlsBaseContext.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/DlsFlsBaseContext.java @@ -39,7 +39,7 @@ public DlsFlsBaseContext(PrivilegesEvaluator privilegesEvaluator, ThreadContext public PrivilegesEvaluationContext getPrivilegesEvaluationContext() { User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); - if (user == null || adminDNs.isAdmin(user)) { + if (HeaderHelper.isInternalOrPluginRequest(threadContext) || adminDNs.isAdmin(user)) { return null; } diff --git a/src/main/java/org/opensearch/security/support/HeaderHelper.java b/src/main/java/org/opensearch/security/support/HeaderHelper.java index bbb44664fa..3b237f5891 100644 --- a/src/main/java/org/opensearch/security/support/HeaderHelper.java +++ b/src/main/java/org/opensearch/security/support/HeaderHelper.java @@ -33,6 +33,7 @@ import com.google.common.base.Strings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.security.user.User; public class HeaderHelper { @@ -52,6 +53,18 @@ public static boolean isExtensionRequest(final ThreadContext context) { } // CS-ENFORCE-SINGLE + public static boolean isInternalOrPluginRequest(final ThreadContext threadContext) { + // If user is empty, this indicates a system-level request which should be permitted + // If the requests originates from a plugin this will also return true + final User user = (User) threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); + + if (user == null || user.isPluginUser()) { + return true; + } + + return false; + } + public static String getSafeFromHeader(final ThreadContext context, final String headerName) { if (context == null || headerName == null || headerName.isEmpty()) {