Concrete index access

Author: n1v0lg

server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstraction.java

@@ -100,7 +100,7 @@ default boolean isDataStreamRelated() {
         return false;
     }
 
-    default boolean isConcreteFailureIndexOfDataStream() {
+    default boolean isFailureIndexOfDataStream() {
         return false;
     }
 
@@ -198,7 +198,7 @@ public boolean isSystem() {
         }
 
         @Override
-        public boolean isConcreteFailureIndexOfDataStream() {
+        public boolean isFailureIndexOfDataStream() {
             return getParentDataStream() != null && getParentDataStream().isFailureStoreIndex(getName());
         }
 

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java

@@ -467,14 +467,14 @@ public boolean checkIndex(Group group) {
             final DataStream ds = indexAbstraction == null ? null : indexAbstraction.getParentDataStream();
             if (ds != null) {
                 if (group.checkIndex(ds.getName())) {
-                    // TODO is this right?
-                    final IndexComponentSelector selectorToCheck = indexAbstraction.isConcreteFailureIndexOfDataStream()
+                    final IndexComponentSelector selectorToCheck = indexAbstraction.isFailureIndexOfDataStream()
                         ? IndexComponentSelector.FAILURES
                         : selector;
-                    return group.checkSelector(selectorToCheck);
+                    if (group.checkSelector(selectorToCheck)) {
+                        return true;
+                    }
                 }
             }
-            // TODO assertions around selector here?
             return group.checkIndex(name) && group.checkSelector(selector);
         }
 

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java

@@ -161,8 +161,7 @@ public class InternalUsers {
                         IndicesStatsAction.NAME + "*",
                         TransportUpdateSettingsAction.TYPE.name(),
                         DownsampleAction.NAME,
-                        TransportAddIndexBlockAction.TYPE.name(),
-                        "read_failures"
+                        TransportAddIndexBlockAction.TYPE.name()
                     )
                     .allowRestrictedIndices(false)
                     .build(),
@@ -181,8 +180,7 @@ public class InternalUsers {
                         IndicesStatsAction.NAME + "*",
                         TransportUpdateSettingsAction.TYPE.name(),
                         DownsampleAction.NAME,
-                        TransportAddIndexBlockAction.TYPE.name(),
-                        "read_failures"
+                        TransportAddIndexBlockAction.TYPE.name()
                     )
                     .allowRestrictedIndices(true)
                     .build() },
@@ -221,8 +219,7 @@ public class InternalUsers {
                         TransportBulkAction.NAME,
                         TransportIndexAction.NAME,
                         TransportSearchScrollAction.TYPE.name(),
-                        ModifyDataStreamsAction.NAME,
-                        "read_failures"
+                        ModifyDataStreamsAction.NAME
                     )
                     .allowRestrictedIndices(false)
                     .build() },
@@ -246,6 +243,7 @@ public class InternalUsers {
             new RoleDescriptor.IndicesPrivileges[] {
                 RoleDescriptor.IndicesPrivileges.builder()
                     .indices("*")
+                    // TODO it's a bug this works...
                     .privileges(LazyRolloverAction.NAME, "read_failures")
                     .allowRestrictedIndices(true)
                     .build() },

x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/FailureStoreSecurityRestIT.java

@@ -32,6 +32,7 @@
 public class FailureStoreSecurityRestIT extends SecurityOnTrialLicenseRestTestCase {
 
     private static final String DATA_ACCESS_USER = "data_access_user";
+    private static final String STAR_READ_ONLY_USER = "star_read_only_user";
     private static final String FAILURE_STORE_ACCESS_USER = "failure_store_access_user";
     private static final String BOTH_ACCESS_USER = "both_access_user";
     private static final String WRITE_ACCESS_USER = "write_access_user";
@@ -40,11 +41,13 @@ public class FailureStoreSecurityRestIT extends SecurityOnTrialLicenseRestTestCa
     @SuppressWarnings("unchecked")
     public void testFailureStoreAccess() throws IOException {
         String dataAccessRole = "data_access";
+        String starReadOnlyRole = "star_read_only_access";
         String failureStoreAccessRole = "failure_store_access";
         String bothAccessRole = "both_access";
         String writeAccessRole = "write_access";
 
         createUser(DATA_ACCESS_USER, PASSWORD, List.of(dataAccessRole));
+        createUser(STAR_READ_ONLY_USER, PASSWORD, List.of(starReadOnlyRole));
         createUser(FAILURE_STORE_ACCESS_USER, PASSWORD, List.of(failureStoreAccessRole));
         createUser(BOTH_ACCESS_USER, PASSWORD, randomBoolean() ? List.of(bothAccessRole) : List.of(dataAccessRole, failureStoreAccessRole));
         createUser(WRITE_ACCESS_USER, PASSWORD, List.of(writeAccessRole));
@@ -55,6 +58,12 @@ public void testFailureStoreAccess() throws IOException {
               "cluster": ["all"],
               "indices": [{"names": ["test*"], "privileges": ["read"]}]
             }"""), dataAccessRole);
+        upsertRole(Strings.format("""
+            {
+              "description": "Role with data access",
+              "cluster": ["all"],
+              "indices": [{"names": ["*"], "privileges": ["read"]}]
+            }"""), starReadOnlyRole);
         upsertRole(Strings.format("""
             {
               "description": "Role with failure store access",
@@ -96,6 +105,9 @@ public void testFailureStoreAccess() throws IOException {
         String dataIndexName = dataIndexNames.get(0);
         String failureIndexName = failureIndexNames.get(0);
 
+        // `*` with read access user _can_ read concrete failure index with only read
+        assertContainsDocIds(performRequest(STAR_READ_ONLY_USER, new Request("GET", "/" + failureIndexName + "/_search")), failedDocId);
+
         // user with access to failures index
         assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::failures/_search")), failedDocId);
         assertContainsDocIds(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test*::failures/_search")), failedDocId);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java

@@ -920,7 +920,7 @@ static AuthorizedIndices resolveAuthorizedIndicesFromRole(
                 // We check the parent data stream first if there is one. For testing requested indices, this is most likely
                 // more efficient than checking the index name first because we recommend grant privileges over data stream
                 // instead of backing indices.
-                if (indexAbstraction.isConcreteFailureIndexOfDataStream()
+                if (indexAbstraction.isFailureIndexOfDataStream()
                     && predicate.test(indexAbstraction.getParentDataStream(), IndexComponentSelector.FAILURES.getKey())) {
                     return true;
                 }