[Backport 2.x] Use SystemIndexRegistry from core to determine if requ…

…est contains system indices (opensearch-project#4550)

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

src/integrationTest/java/org/opensearch/security/SystemIndexTests.java

@@ -0,0 +1,83 @@
+/*
+ * 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;
+
+import java.util.List;
+import java.util.Map;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.opensearch.core.rest.RestStatus;
+import org.opensearch.security.http.ExampleSystemIndexPlugin;
+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.equalTo;
+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 SystemIndexTests {
+
+    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(ExampleSystemIndexPlugin.class)
+        .nodeSettings(
+            Map.of(
+                SECURITY_RESTAPI_ROLES_ENABLED,
+                List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()),
+                SECURITY_SYSTEM_INDICES_ENABLED_KEY,
+                true
+            )
+        )
+        .build();
+
+    @Test
+    public void adminShouldNotBeAbleToDeleteSecurityIndex() {
+        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+            HttpResponse response = client.delete(".opendistro_security");
+
+            assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus()));
+
+            // Create regular index
+            client.put("test-index");
+
+            // regular user can delete non-system index
+            HttpResponse response2 = client.delete("test-index");
+
+            assertThat(response2.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
+
+            // regular use can create system index
+            HttpResponse response3 = client.put(".system-index1");
+
+            assertThat(response3.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
+
+            // regular user cannot delete system index
+            HttpResponse response4 = client.delete(".system-index1");
+
+            assertThat(response4.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus()));
+        }
+    }
+}

src/integrationTest/java/org/opensearch/security/http/ExampleSystemIndexPlugin.java

@@ -0,0 +1,27 @@
+/*
+ * 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.http;
+
+import java.util.Collection;
+import java.util.Collections;
+
+import org.opensearch.common.settings.Settings;
+import org.opensearch.indices.SystemIndexDescriptor;
+import org.opensearch.plugins.Plugin;
+import org.opensearch.plugins.SystemIndexPlugin;
+
+public class ExampleSystemIndexPlugin extends Plugin implements SystemIndexPlugin {
+
+    @Override
+    public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
+        final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(".system-index1", "System index 1");
+        return Collections.singletonList(systemIndexDescriptor);
+    }
+}

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

@@ -136,7 +136,7 @@ public class PrivilegesEvaluator {
     private ConfigModel configModel;
     private final IndexResolverReplacer irr;
     private final SnapshotRestoreEvaluator snapshotRestoreEvaluator;
-    private final SecurityIndexAccessEvaluator securityIndexAccessEvaluator;
+    private final SystemIndexAccessEvaluator systemIndexAccessEvaluator;
     private final ProtectedIndexAccessEvaluator protectedIndexAccessEvaluator;
     private final TermsAggregationEvaluator termsAggregationEvaluator;
     private final PitPrivilegesEvaluator pitPrivilegesEvaluator;
@@ -172,7 +172,7 @@ public PrivilegesEvaluator(
         this.clusterInfoHolder = clusterInfoHolder;
         this.irr = irr;
         snapshotRestoreEvaluator = new SnapshotRestoreEvaluator(settings, auditLog);
-        securityIndexAccessEvaluator = new SecurityIndexAccessEvaluator(settings, auditLog, irr);
+        systemIndexAccessEvaluator = new SystemIndexAccessEvaluator(settings, auditLog, irr);
         protectedIndexAccessEvaluator = new ProtectedIndexAccessEvaluator(settings, auditLog);
         termsAggregationEvaluator = new TermsAggregationEvaluator();
         pitPrivilegesEvaluator = new PitPrivilegesEvaluator();
@@ -328,7 +328,7 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
         }
 
         // Security index access
-        if (securityIndexAccessEvaluator.evaluate(
+        if (systemIndexAccessEvaluator.evaluate(
             request,
             task,
             action0,

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

@@ -41,6 +41,7 @@
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.indices.SystemIndexRegistry;
 import org.opensearch.security.auditlog.AuditLog;
 import org.opensearch.security.resolver.IndexResolverReplacer;
 import org.opensearch.security.resolver.IndexResolverReplacer.Resolved;
@@ -56,7 +57,7 @@
  * - The term `protected system indices` used here translates to system indices
  *   which have an added layer of security and cannot be accessed by anyone except Super Admin
  */
-public class SecurityIndexAccessEvaluator {
+public class SystemIndexAccessEvaluator {
 
     Logger log = LogManager.getLogger(this.getClass());
 
@@ -72,7 +73,7 @@ public class SecurityIndexAccessEvaluator {
     private final boolean isSystemIndexEnabled;
     private final boolean isSystemIndexPermissionEnabled;
 
-    public SecurityIndexAccessEvaluator(final Settings settings, AuditLog auditLog, IndexResolverReplacer irr) {
+    public SystemIndexAccessEvaluator(final Settings settings, AuditLog auditLog, IndexResolverReplacer irr) {
         this.securityIndex = settings.get(
             ConfigConstants.SECURITY_CONFIG_INDEX_NAME,
             ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX
@@ -83,6 +84,7 @@ public SecurityIndexAccessEvaluator(final Settings settings, AuditLog auditLog,
         this.systemIndexMatcher = WildcardMatcher.from(
             settings.getAsList(ConfigConstants.SECURITY_SYSTEM_INDICES_KEY, ConfigConstants.SECURITY_SYSTEM_INDICES_DEFAULT)
         );
+
         this.superAdminAccessOnlyIndexMatcher = WildcardMatcher.from(this.securityIndex);
         this.isSystemIndexEnabled = settings.getAsBoolean(
             ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY,
@@ -168,15 +170,16 @@ private boolean requestContainsAnySystemIndices(final Resolved requestedResolved
      * It will always return security index if it is present in the request, as security index is protected regardless
      * of feature being enabled or disabled
      * @param requestedResolved request which contains indices to be matched against system indices
-     * @return the list of protected system indices present in the request
+     * @return the set of protected system indices present in the request
      */
-    private List<String> getAllSystemIndices(final Resolved requestedResolved) {
-        final List<String> systemIndices = requestedResolved.getAllIndices()
+    private Set<String> getAllSystemIndices(final Resolved requestedResolved) {
+        final Set<String> systemIndices = requestedResolved.getAllIndices()
             .stream()
             .filter(securityIndex::equals)
-            .collect(Collectors.toList());
+            .collect(Collectors.toSet());
         if (isSystemIndexEnabled) {
             systemIndices.addAll(systemIndexMatcher.getMatchAny(requestedResolved.getAllIndices(), Collectors.toList()));
+            systemIndices.addAll(SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.getAllIndices().toArray(String[]::new)));
         }
         return systemIndices;
     }
@@ -210,7 +213,7 @@ private List<String> getAllProtectedSystemIndices(final Resolved requestedResolv
     private boolean requestContainsAnyRegularIndices(final Resolved requestedResolved) {
         Set<String> allIndices = requestedResolved.getAllIndices();
 
-        List<String> allSystemIndices = getAllSystemIndices(requestedResolved);
+        Set<String> allSystemIndices = getAllSystemIndices(requestedResolved);
         List<String> allProtectedSystemIndices = getAllProtectedSystemIndices(requestedResolved);
 
         return allIndices.stream().anyMatch(index -> !allSystemIndices.contains(index) && !allProtectedSystemIndices.contains(index));

src/test/java/org/opensearch/security/privileges/SecurityIndexAccessEvaluatorTest.java → src/test/java/org/opensearch/security/privileges/SystemIndexAccessEvaluatorTest.java

@@ -56,7 +56,7 @@
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
-public class SecurityIndexAccessEvaluatorTest {
+public class SystemIndexAccessEvaluatorTest {
 
     @Mock
     private AuditLog auditLog;
@@ -73,7 +73,7 @@ public class SecurityIndexAccessEvaluatorTest {
     @Mock
     ClusterService cs;
 
-    private SecurityIndexAccessEvaluator evaluator;
+    private SystemIndexAccessEvaluator evaluator;
     private static final String UNPROTECTED_ACTION = "indices:data/read";
     private static final String PROTECTED_ACTION = "indices:data/write";
 
@@ -137,7 +137,7 @@ public void setup(
 
         // when trying to resolve Index Names
 
-        evaluator = new SecurityIndexAccessEvaluator(
+        evaluator = new SystemIndexAccessEvaluator(
             Settings.builder()
                 .put(ConfigConstants.SECURITY_SYSTEM_INDICES_KEY, TEST_SYSTEM_INDEX)
                 .put(ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY, isSystemIndexEnabled)