Allow any plugin system request when plugins.security.system_indices.enabled is set to false (#5579)

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>

Author: cwperks

.github/workflows/plugin_install.yml

@@ -42,13 +42,24 @@ jobs:
 
       - name: Run Opensearch with A Single Plugin
         uses: derek-ho/start-opensearch@v7
+        if: ${{ runner.os != 'Windows' }}
         with:
           opensearch-version: ${{ env.OPENSEARCH_VERSION }}
           plugins: "file:$(pwd)/${{ env.PLUGIN_NAME }}.zip"
           security-enabled: true
           admin-password: ${{ steps.generate-password.outputs.password }}
           jdk-version: 21
 
+      - name: Run Opensearch with A Single Plugin
+        uses: derek-ho/start-opensearch@v7
+        if: ${{ runner.os == 'Windows' }}
+        with:
+          opensearch-version: ${{ env.OPENSEARCH_VERSION }}
+          plugins: "file:///$((Get-Location).Path -replace '\\\\','/')/${{ env.PLUGIN_NAME }}.zip"
+          security-enabled: true
+          admin-password: ${{ steps.generate-password.outputs.password }}
+          jdk-version: 21
+
       - name: Run sanity tests
         uses: gradle/gradle-build-action@v3
         with:

CHANGELOG.md

@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 * [Resource Sharing] Fixes accessible resource ids search by marking created_by.user field as keyword search instead of text ([#5574](https://github.com/opensearch-project/security/pull/5574))
 * [Resource Sharing] Reverts @Inject pattern usage for ResourceSharingExtension to client accessor pattern. ([#5576](https://github.com/opensearch-project/security/pull/5576))
 * Inject user custom attributes when injecting user and role information to the thread context ([#5560](https://github.com/opensearch-project/security/pull/5560))
+* Allow any plugin system request when `plugins.security.system_indices.enabled` is set to `false` ([#5579](https://github.com/opensearch-project/security/pull/5579))
 
 ### Refactoring
 

src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java

@@ -13,7 +13,6 @@
 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;
@@ -57,13 +56,6 @@ public class SystemIndexDisabledTests {
         )
         .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())) {
@@ -79,10 +71,12 @@ public void testPluginShouldBeAbleToIndexIntoAnySystemIndexWhenProtectionIsDisab
                 response.getBody(),
                 not(
                     containsString(
-                        "no permissions for [] and User [name=plugin:org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1"
+                        "no permissions for [indices:data/write/bulk[s], indices:data/write/index] and User [name=plugin:org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1"
                     )
                 )
             );
+
+            assertThat(response.getBody(), not(containsString("\"errors\":true")));
         }
     }
 }

src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java

@@ -305,34 +305,41 @@ private void evaluateSystemIndicesAccess(
         }
 
         // the following section should only be run for index actions
-        if (this.isSystemIndexEnabled && user.isPluginUser() && !isClusterPerm(action)) {
-            Set<String> matchingPluginIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern(
-                user.getName().replace("plugin:", ""),
-                requestedResolved.getAllIndices()
-            );
-            if (requestedResolved.getAllIndices().equals(matchingPluginIndices)) {
-                // plugin is authorized to perform any actions on its own registered system indices
-                presponse.allowed = true;
-                presponse.markComplete();
-                return;
-            } else {
-                Set<String> matchingSystemIndices = SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.getAllIndices());
-                matchingSystemIndices.removeAll(matchingPluginIndices);
-                // See if request matches other system indices not belong to the plugin
-                if (!matchingSystemIndices.isEmpty()) {
-                    if (log.isInfoEnabled()) {
-                        log.info(
-                            "Plugin {} can only perform {} on it's own registered System Indices. System indices from request that match plugin's registered system indices: {}",
-                            user.getName(),
-                            action,
-                            matchingPluginIndices
-                        );
-                    }
-                    presponse.allowed = false;
-                    presponse.getMissingPrivileges();
+        if (user.isPluginUser() && !isClusterPerm(action)) {
+            if (this.isSystemIndexEnabled) {
+                Set<String> matchingPluginIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern(
+                    user.getName().replace("plugin:", ""),
+                    requestedResolved.getAllIndices()
+                );
+                if (requestedResolved.getAllIndices().equals(matchingPluginIndices)) {
+                    // plugin is authorized to perform any actions on its own registered system indices
+                    presponse.allowed = true;
                     presponse.markComplete();
                     return;
+                } else {
+                    Set<String> matchingSystemIndices = SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.getAllIndices());
+                    matchingSystemIndices.removeAll(matchingPluginIndices);
+                    // See if request matches other system indices not belong to the plugin
+                    if (!matchingSystemIndices.isEmpty()) {
+                        if (log.isInfoEnabled()) {
+                            log.info(
+                                "Plugin {} can only perform {} on it's own registered System Indices. System indices from request that match plugin's registered system indices: {}",
+                                user.getName(),
+                                action,
+                                matchingPluginIndices
+                            );
+                        }
+                        presponse.allowed = false;
+                        presponse.getMissingPrivileges();
+                        presponse.markComplete();
+                        return;
+                    }
                 }
+            } else {
+                // no system index protection and request originating from plugin, allow
+                presponse.allowed = true;
+                presponse.markComplete();
+                return;
             }
         }