Revert "Fix bug where admin can read system index (#4774)"

This reverts commit 5cb4dd2.
opensearch-project · Oct 30, 2024 · c225740 · c225740
1 parent 6a7c785
commit c225740
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 32 deletions.
diff --git 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
@@ -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/...iguration/SystemIndexSearcherWrapper.java → ...uration/SecurityIndexSearcherWrapper.java b/...iguration/SystemIndexSearcherWrapper.java → ...uration/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);