Ensure that plugin can search on system index when utilizing pluginSu…

…bject.runAs (opensearch-project#5032)

Signed-off-by: Craig Perkins <cwperx@amazon.com>

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"
+                    )
+                )
+            );
+        }
+    }
+}

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())) {

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<Route> 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))); }));
+            }
+        };
+    }
+}

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<Route> 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))); }));
+            }
+        };
+    }
+}

src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java

@@ -88,7 +88,9 @@ public List<RestHandler> 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)
         );
     }
 

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;

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<String> matchingSystemIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern(
                 user.getName().replace("plugin:", ""),
                 requestedResolved.getAllIndices()

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;
         }
 

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()) {