[2.16] Revert change to use System Index Registry from core to determ…

…ine if request matches system indices (#4616)
opensearch-project · Aug 2, 2024 · 3076016 · 3076016
1 parent deef041
commit 3076016
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 128 deletions.
diff --git a/release-notes/opensearch-security.release-notes-2.16.0.0.md b/release-notes/opensearch-security.release-notes-2.16.0.0.md
@@ -4,7 +4,6 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.16.0
 
 ### Enhancements
 * Add support for PBKDF2 for password hashing & add support for configuring BCrypt and PBKDF2 ([#4524](https://github.com/opensearch-project/security/pull/4524))
-* Use SystemIndexRegistry from core to determine if request contains system indices ([#4471](https://github.com/opensearch-project/security/pull/4471))
 * Separated DLS/FLS privilege evaluation from action privilege evaluation ([#4490](https://github.com/opensearch-project/security/pull/4490))
 * Update PULL_REQUEST_TEMPLATE to include an API spec change in the checklist. ([#4533](https://github.com/opensearch-project/security/pull/4533))
 * Update PATCH API to fail validation if nothing changes ([#4530](https://github.com/opensearch-project/security/pull/4530))
@@ -26,7 +25,6 @@ Compatible with OpenSearch and OpenSearch Dashboards version 2.16.0
 ### Maintenance
 * Remove unused dependancy Apache CXF ([#4580](https://github.com/opensearch-project/security/pull/4580))
 * Remove unnecessary return statements ([#4558](https://github.com/opensearch-project/security/pull/4558))
-* Pass set to SystemIndexRegistry.matchesSystemIndexPattern ([#4569](https://github.com/opensearch-project/security/pull/4569))
 * Refactor and update existing ml roles ([#4151](https://github.com/opensearch-project/security/pull/4151))
 * Replace JUnit assertEquals() with Hamcrest matchers assertThat() ([#4544](https://github.com/opensearch-project/security/pull/4544))
 * Update Gradle to 8.9 ([#4553](https://github.com/opensearch-project/security/pull/4553))

diff --git a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java
diff --git a/src/integrationTest/java/org/opensearch/security/http/ExampleSystemIndexPlugin.java b/src/integrationTest/java/org/opensearch/security/http/ExampleSystemIndexPlugin.java
diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/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 SystemIndexAccessEvaluator systemIndexAccessEvaluator;
+    private final SecurityIndexAccessEvaluator securityIndexAccessEvaluator;
     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);
-        systemIndexAccessEvaluator = new SystemIndexAccessEvaluator(settings, auditLog, irr);
+        securityIndexAccessEvaluator = new SecurityIndexAccessEvaluator(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 (systemIndexAccessEvaluator.evaluate(
+        if (securityIndexAccessEvaluator.evaluate(
             request,
             task,
             action0,

diff --git a/...rivileges/SystemIndexAccessEvaluator.java → ...vileges/SecurityIndexAccessEvaluator.java b/...rivileges/SystemIndexAccessEvaluator.java → ...vileges/SecurityIndexAccessEvaluator.java
@@ -41,7 +41,6 @@
 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;
@@ -57,7 +56,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 SystemIndexAccessEvaluator {
+public class SecurityIndexAccessEvaluator {
 
     Logger log = LogManager.getLogger(this.getClass());
 
@@ -73,7 +72,7 @@ public class SystemIndexAccessEvaluator {
     private final boolean isSystemIndexEnabled;
     private final boolean isSystemIndexPermissionEnabled;
 
-    public SystemIndexAccessEvaluator(final Settings settings, AuditLog auditLog, IndexResolverReplacer irr) {
+    public SecurityIndexAccessEvaluator(final Settings settings, AuditLog auditLog, IndexResolverReplacer irr) {
         this.securityIndex = settings.get(
             ConfigConstants.SECURITY_CONFIG_INDEX_NAME,
             ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX
@@ -84,7 +83,6 @@ public SystemIndexAccessEvaluator(final Settings settings, AuditLog auditLog, In
         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,
@@ -170,16 +168,15 @@ 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 set of protected system indices present in the request
+     * @return the list of protected system indices present in the request
      */
-    private Set<String> getAllSystemIndices(final Resolved requestedResolved) {
-        final Set<String> systemIndices = requestedResolved.getAllIndices()
+    private List<String> getAllSystemIndices(final Resolved requestedResolved) {
+        final List<String> systemIndices = requestedResolved.getAllIndices()
             .stream()
             .filter(securityIndex::equals)
-            .collect(Collectors.toSet());
+            .collect(Collectors.toList());
         if (isSystemIndexEnabled) {
             systemIndices.addAll(systemIndexMatcher.getMatchAny(requestedResolved.getAllIndices(), Collectors.toList()));
-            systemIndices.addAll(SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.getAllIndices()));
         }
         return systemIndices;
     }
@@ -213,7 +210,7 @@ private List<String> getAllProtectedSystemIndices(final Resolved requestedResolv
     private boolean requestContainsAnyRegularIndices(final Resolved requestedResolved) {
         Set<String> allIndices = requestedResolved.getAllIndices();
 
-        Set<String> allSystemIndices = getAllSystemIndices(requestedResolved);
+        List<String> allSystemIndices = getAllSystemIndices(requestedResolved);
         List<String> allProtectedSystemIndices = getAllProtectedSystemIndices(requestedResolved);
 
         return allIndices.stream().anyMatch(index -> !allSystemIndices.contains(index) && !allProtectedSystemIndices.contains(index));

diff --git a/...leges/SystemIndexAccessEvaluatorTest.java → ...ges/SecurityIndexAccessEvaluatorTest.java b/...leges/SystemIndexAccessEvaluatorTest.java → ...ges/SecurityIndexAccessEvaluatorTest.java
@@ -56,7 +56,7 @@
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
-public class SystemIndexAccessEvaluatorTest {
+public class SecurityIndexAccessEvaluatorTest {
 
     @Mock
     private AuditLog auditLog;
@@ -73,7 +73,7 @@ public class SystemIndexAccessEvaluatorTest {
     @Mock
     ClusterService cs;
 
-    private SystemIndexAccessEvaluator evaluator;
+    private SecurityIndexAccessEvaluator 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 SystemIndexAccessEvaluator(
+        evaluator = new SecurityIndexAccessEvaluator(
             Settings.builder()
                 .put(ConfigConstants.SECURITY_SYSTEM_INDICES_KEY, TEST_SYSTEM_INDEX)
                 .put(ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY, isSystemIndexEnabled)