Check block request only if system index (opensearch-project#4430)

Signed-off-by: 10000-ki <[email protected]>
cwperks · Jun 17, 2024 · 1d1b1ed · 1d1b1ed
1 parent 61f9d22
commit 1d1b1ed
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -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:
         - '*'