From 128a3042d77c3a2c6b95d3579480ee50ed1b6a09 Mon Sep 17 00:00:00 2001
From: Derek Ho <dxho@amazon.com>
Date: Wed, 30 Oct 2024 16:38:34 -0400
Subject: [PATCH] Revert "Fix bug where admin can read system index (#4774)"
 (#4864)

---
 .../opensearch/security/SystemIndexTests.java | 25 -------------------
 .../SecurityFlsDlsIndexSearcherWrapper.java   |  2 +-
 ...java => SecurityIndexSearcherWrapper.java} | 10 +++-----
 3 files changed, 5 insertions(+), 32 deletions(-)
 rename src/main/java/org/opensearch/security/configuration/{SystemIndexSearcherWrapper.java => SecurityIndexSearcherWrapper.java} (93%)

diff --git a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java
index ae068255da..a6f6abe474 100644
--- a/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java
+++ b/src/integrationTest/java/org/opensearch/security/SystemIndexTests.java
@@ -88,29 +88,4 @@ public void adminShouldNotBeAbleToDeleteSecurityIndex() {
             assertThat(response4.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus()));
         }
     }
-
-    @Test
-    public void regularUserShouldGetNoResultsWhenSearchingSystemIndex() {
-        // Create system index and index a dummy document as the super admin user, data returned to super admin
-        try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
-            HttpResponse response1 = client.put(".system-index1");
-
-            assertThat(response1.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
-            String doc = "{\"field\":\"value\"}";
-            HttpResponse adminPostResponse = client.postJson(".system-index1/_doc/1?refresh=true", doc);
-            assertThat(adminPostResponse.getStatusCode(), equalTo(RestStatus.CREATED.getStatus()));
-            HttpResponse response2 = client.get(".system-index1/_search");
-
-            assertThat(response2.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
-            assertThat(response2.getBody(), response2.getBody().contains("\"hits\":{\"total\":{\"value\":1,\"relation\":\"eq\"}"));
-        }
-
-        // Regular users should not be able to read it
-        try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
-            // regular user cannot read system index
-            HttpResponse response1 = client.get(".system-index1/_search");
-
-            assertThat(response1.getBody(), response1.getBody().contains("\"hits\":{\"total\":{\"value\":0,\"relation\":\"eq\"}"));
-        }
-    }
 }
diff --git a/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java
index e889368315..f66ff7a2c0 100644
--- a/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java
+++ b/src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java
@@ -41,7 +41,7 @@
 import org.opensearch.security.support.HeaderHelper;
 import org.opensearch.security.support.SecurityUtils;
 
-public class SecurityFlsDlsIndexSearcherWrapper extends SystemIndexSearcherWrapper {
+public class SecurityFlsDlsIndexSearcherWrapper extends SecurityIndexSearcherWrapper {
 
     public final Logger log = LogManager.getLogger(this.getClass());
 
diff --git a/src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java
similarity index 93%
rename from src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java
rename to src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java
index 8e89b60712..7a40e5dbd0 100644
--- a/src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java
+++ b/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java
@@ -39,7 +39,6 @@
 import org.opensearch.core.common.transport.TransportAddress;
 import org.opensearch.core.index.Index;
 import org.opensearch.index.IndexService;
-import org.opensearch.indices.SystemIndexRegistry;
 import org.opensearch.security.privileges.PrivilegesEvaluator;
 import org.opensearch.security.securityconf.ConfigModel;
 import org.opensearch.security.securityconf.SecurityRoles;
@@ -50,7 +49,7 @@
 
 import org.greenrobot.eventbus.Subscribe;
 
-public class SystemIndexSearcherWrapper implements CheckedFunction<DirectoryReader, DirectoryReader, IOException> {
+public class SecurityIndexSearcherWrapper implements CheckedFunction<DirectoryReader, DirectoryReader, IOException> {
 
     protected final Logger log = LogManager.getLogger(this.getClass());
     protected final ThreadContext threadContext;
@@ -69,7 +68,7 @@ public class SystemIndexSearcherWrapper implements CheckedFunction<DirectoryRead
     private final Boolean systemIndexPermissionEnabled;
 
     // constructor is called per index, so avoid costly operations here
-    public SystemIndexSearcherWrapper(
+    public SecurityIndexSearcherWrapper(
         final IndexService indexService,
         final Settings settings,
         final AdminDNs adminDNs,
@@ -153,8 +152,7 @@ protected final boolean isBlockedProtectedIndexRequest() {
     }
 
     protected final boolean isBlockedSystemIndexRequest() {
-        boolean matchesSystemIndexRegisteredWithCore = !SystemIndexRegistry.matchesSystemIndexPattern(Set.of(index.getName())).isEmpty();
-        boolean isSystemIndex = systemIndexMatcher.test(index.getName()) || matchesSystemIndexRegisteredWithCore;
+        boolean isSystemIndex = systemIndexMatcher.test(index.getName());
         if (!isSystemIndex) {
             return false;
         }
@@ -163,7 +161,7 @@ protected final boolean isBlockedSystemIndexRequest() {
             final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
             if (user == null) {
                 // allow request without user from plugin.
-                return systemIndexMatcher.test(index.getName()) || matchesSystemIndexRegisteredWithCore;
+                return systemIndexMatcher.test(index.getName());
             }
             final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);
             final Set<String> mappedRoles = evaluator.mapRoles(user, caller);