From 1d1b1ed7bb0d0c99e693c3571231d1f2b8b7cb7c Mon Sep 17 00:00:00 2001
From: 10000-ki <76486121+10000-ki@users.noreply.github.com>
Date: Tue, 18 Jun 2024 00:07:50 +0900
Subject: [PATCH] Check block request only if system index (#4430)

Signed-off-by: 10000-ki <10000ki6472@gmail.com>
---
 .../SecurityIndexSearcherWrapper.java         |  7 +++++-
 .../AbstractSystemIndicesTests.java           |  9 ++++++++
 .../SystemIndexPermissionEnabledTests.java    | 22 +++++++++++++++++++
 src/test/resources/system_indices/roles.yml   |  2 ++
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java b/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java
index 0602cb0bed..7a40e5dbd0 100644
--- a/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java
+++ b/src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java
@@ -152,6 +152,11 @@ protected final boolean isBlockedProtectedIndexRequest() {
     }
 
     protected final boolean isBlockedSystemIndexRequest() {
+        boolean isSystemIndex = systemIndexMatcher.test(index.getName());
+        if (!isSystemIndex) {
+            return false;
+        }
+
         if (systemIndexPermissionEnabled) {
             final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
             if (user == null) {
@@ -163,7 +168,7 @@ protected final boolean isBlockedSystemIndexRequest() {
             final SecurityRoles securityRoles = evaluator.getSecurityRoles(mappedRoles);
             return !securityRoles.isPermittedOnSystemIndex(index.getName());
         }
-        return systemIndexMatcher.test(index.getName());
+        return true;
     }
 
     protected final boolean isAdminDnOrPluginRequest() {
diff --git a/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java b/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java
index 2e2e71e082..f5af73265e 100644
--- a/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java
+++ b/src/test/java/org/opensearch/security/system_indices/AbstractSystemIndicesTests.java
@@ -54,6 +54,7 @@ public abstract class AbstractSystemIndicesTests extends SingleClusterTest {
         SYSTEM_INDEX_WITH_NO_ASSOCIATED_ROLE_PERMISSIONS,
         ACCESSIBLE_ONLY_BY_SUPER_ADMIN
     );
+    static final List<String> NO_SYSTEM_INDICES = List.of(".no_system_index_1", ".no_system_index_2");
 
     static final List<String> INDICES_FOR_CREATE_REQUEST = List.of(".system_index_2");
     static final String matchAllQuery = "{\n\"query\": {\"match_all\": {}}}";
@@ -117,6 +118,14 @@ void createTestIndicesAndDocs() {
                         .source("{ \"foo\": \"bar\" }", XContentType.JSON)
                 ).actionGet();
             }
+
+            for (String index : NO_SYSTEM_INDICES) {
+                tc.index(
+                    new IndexRequest(index).setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
+                        .id("document1")
+                        .source("{ \"foo\": \"bar\" }", XContentType.JSON)
+                ).actionGet();
+            }
         }
     }
 
diff --git a/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java b/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java
index 7db072761f..f8e29b2bbd 100644
--- a/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java
+++ b/src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java
@@ -106,6 +106,28 @@ public void testSearchAsNormalUserWithoutSystemIndexAccess() {
         validateForbiddenResponse(response, "indices:data/read/search", normalUserWithoutSystemIndex);
     }
 
+    @Test
+    public void testNormalIndexShouldAlwaysBeSearchable() throws Exception {
+        RestHelper restHelper = sslRestHelper();
+
+        // search system indices
+        for (String index : NO_SYSTEM_INDICES) {
+            RestHelper.HttpResponse responseWithoutSystemIndexPermission = restHelper.executeGetRequest(
+                index + "/_search",
+                "",
+                normalUserWithoutSystemIndexHeader
+            );
+            validateSearchResponse(responseWithoutSystemIndexPermission, 1);
+
+            RestHelper.HttpResponse responseWithSystemIndexPermission = restHelper.executeGetRequest(
+                index + "/_search",
+                "",
+                normalUserHeader
+            );
+            validateSearchResponse(responseWithSystemIndexPermission, 1);
+        }
+    }
+
     /**
      *  DELETE document + index
      */
diff --git a/src/test/resources/system_indices/roles.yml b/src/test/resources/system_indices/roles.yml
index 82ac33a92d..c10d7b08bb 100644
--- a/src/test/resources/system_indices/roles.yml
+++ b/src/test/resources/system_indices/roles.yml
@@ -20,6 +20,7 @@ normal_role:
   index_permissions:
     - index_patterns:
         - '.system*'
+        - '.no_system*'
       allowed_actions:
         - '*'
         - 'system:admin/system_index'
@@ -31,5 +32,6 @@ normal_role_without_system_index:
   index_permissions:
     - index_patterns:
         - '.system*'
+        - '.no_system*'
       allowed_actions:
         - '*'