Handle restricted indices

Author: n1v0lg

test/test-clusters/src/main/resources/default_test_roles.yml

@@ -6,6 +6,9 @@ _es_test_root:
     - names: [ "*" ]
       allow_restricted_indices: true
       privileges: [ "ALL" ]
+    - names: [ "*::failures" ]
+      allow_restricted_indices: true
+      privileges: [ "ALL" ]
   run_as: [ "*" ]
   applications:
     - application: "*"

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

@@ -147,14 +147,21 @@ public boolean hasFieldOrDocumentLevelSecurity() {
     private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String action) {
         final Set<String> dataAccessOrdinaryIndices = new HashSet<>();
         final Set<String> failureAccessOrdinaryIndices = new HashSet<>();
-        final Set<String> restrictedIndices = new HashSet<>();
+        final Set<String> dataAccessRestrictedIndices = new HashSet<>();
+        final Set<String> failureAccessRestrictedIndices = new HashSet<>();
+
+        // TODO do we need to worry about failure access here?
         final Set<String> grantMappingUpdatesOnIndices = new HashSet<>();
         final Set<String> grantMappingUpdatesOnRestrictedIndices = new HashSet<>();
         final boolean isMappingUpdateAction = isMappingUpdateAction(action);
         for (final Group group : groups) {
             if (group.actionMatcher.test(action)) {
                 if (group.allowRestrictedIndices) {
-                    restrictedIndices.addAll(Arrays.asList(group.indices()));
+                    if (group.failureStoreOnly) {
+                        failureAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
+                    } else {
+                        dataAccessRestrictedIndices.addAll(Arrays.asList(group.indices()));
+                    }
                 } else {
                     if (group.failureStoreOnly) {
                         failureAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
@@ -172,9 +179,8 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String
                 }
             }
         }
-        final StringMatcher dataAccessNameMatcher = indexMatcher(dataAccessOrdinaryIndices, restrictedIndices);
-        // TODO handle restricted indices for failure access
-        final StringMatcher failureAccessNameMatcher = indexMatcher(failureAccessOrdinaryIndices, Set.of());
+        final StringMatcher dataAccessNameMatcher = indexMatcher(dataAccessOrdinaryIndices, dataAccessRestrictedIndices);
+        final StringMatcher failureAccessNameMatcher = indexMatcher(failureAccessOrdinaryIndices, failureAccessRestrictedIndices);
         final StringMatcher bwcSpecialCaseMatcher = indexMatcher(grantMappingUpdatesOnIndices, grantMappingUpdatesOnRestrictedIndices);
         return new IsResourceAuthorizedPredicate(dataAccessNameMatcher, failureAccessNameMatcher, bwcSpecialCaseMatcher);
     }

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

@@ -569,15 +569,33 @@ public static void buildRoleFromDescriptors(
             }
         });
         restrictedIndicesPrivilegesMap.forEach((key, privilege) -> {
-            // TODO handle failure indices
-            builder.add(
-                fieldPermissionsCache.getFieldPermissions(privilege.fieldPermissionsDefinition),
-                privilege.query,
-                IndexPrivilege.get(privilege.privileges),
-                true,
-                false,
-                privilege.indices.toArray(Strings.EMPTY_ARRAY)
-            );
+            // TODO double-check if this is always the case
+            assert privilege.indices.isEmpty() == false : "indices must not be empty";
+
+            // For a privilege with both failure and non-failure indices, we need to split them into two separate groups
+            if (privilege.failureIndices.isEmpty() == false) {
+                var failureIndices = newHashSet(privilege.failureIndices);
+                builder.add(
+                    fieldPermissionsCache.getFieldPermissions(privilege.fieldPermissionsDefinition),
+                    privilege.query,
+                    IndexPrivilege.get(privilege.privileges),
+                    true,
+                    true,
+                    failureIndices.stream().map(CompositeRolesStore::stripFailuresSuffix).toList().toArray(Strings.EMPTY_ARRAY)
+                );
+                privilege.removeFailureIndices();
+            }
+            // If we still have privileges left, add a group for them
+            if (privilege.indices.isEmpty() == false) {
+                builder.add(
+                    fieldPermissionsCache.getFieldPermissions(privilege.fieldPermissionsDefinition),
+                    privilege.query,
+                    IndexPrivilege.get(privilege.privileges),
+                    true,
+                    false,
+                    privilege.indices.toArray(Strings.EMPTY_ARRAY)
+                );
+            }
         });
 
         remoteIndicesPrivilegesByCluster.forEach((clusterAliasKey, remoteIndicesPrivilegesForCluster) -> {
@@ -635,6 +653,7 @@ private static String stripFailuresSuffix(String input) {
         if (input.endsWith(literalSuffix)) {
             return input.substring(0, input.length() - literalSuffix.length());
         } else if (input.endsWith(regexSuffix)) {
+            // TODO need to add back "/" since otherwise we break the regex
             return input.substring(0, input.length() - regexSuffix.length());
         }
         assert false : "unexpected failure index name: " + input;