More fixes

Author: n1v0lg

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

@@ -76,6 +76,10 @@ default Index getWriteIndex(IndexRequest request, Metadata metadata) {
         return getWriteIndex();
     }
 
+    default boolean isFailureIndexOfDataStream() {
+        return false;
+    }
+
     /**
      * @return the data stream to which this index belongs or <code>null</code> if this is not a concrete index or
      * if it is a concrete index that does not belong to a data stream.
@@ -193,6 +197,11 @@ public boolean isSystem() {
             return isSystem;
         }
 
+        @Override
+        public boolean isFailureIndexOfDataStream() {
+            return dataStream != null && dataStream.isFailureStoreIndex(getName());
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;

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

@@ -21,7 +21,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.function.Predicate;
+import java.util.function.BiPredicate;
 import java.util.function.Supplier;
 
 public class IndexAbstractionResolver {
@@ -37,7 +37,7 @@ public List<String> resolveIndexAbstractions(
         IndicesOptions indicesOptions,
         Metadata metadata,
         Supplier<Set<String>> allAuthorizedAndAvailable,
-        Predicate<String> isAuthorized,
+        BiPredicate<String, String> isAuthorized,
         boolean includeDataStreams
     ) {
         List<String> finalIndices = new ArrayList<>();
@@ -103,7 +103,7 @@ && isIndexVisible(
                 resolveSelectorsAndCombine(indexAbstraction, selectorString, indicesOptions, resolvedIndices, metadata);
                 if (minus) {
                     finalIndices.removeAll(resolvedIndices);
-                } else if (indicesOptions.ignoreUnavailable() == false || isAuthorized.test(indexAbstraction)) {
+                } else if (indicesOptions.ignoreUnavailable() == false || isAuthorized.test(indexAbstraction, selectorString)) {
                     // Unauthorized names are considered unavailable, so if `ignoreUnavailable` is `true` they should be silently
                     // discarded from the `finalIndices` list. Other "ways of unavailable" must be handled by the action
                     // handler, see: https://github.com/elastic/elasticsearch/issues/90215

server/src/test/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolverTests.java

@@ -236,6 +236,6 @@ private List<String> resolveAbstractionsSelectorAllowed(List<String> expressions
     }
 
     private List<String> resolveAbstractions(List<String> expressions, IndicesOptions indicesOptions, Supplier<Set<String>> mask) {
-        return indexAbstractionResolver.resolveIndexAbstractions(expressions, indicesOptions, metadata, mask, (idx) -> true, true);
+        return indexAbstractionResolver.resolveIndexAbstractions(expressions, indicesOptions, metadata, mask, (idx, sel) -> true, true);
     }
 }

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

@@ -295,7 +295,11 @@ interface AuthorizedIndices {
         /**
          * Checks if an index-like resource name is authorized, for an action by a user. The resource might or might not exist.
          */
-        boolean check(String name);
+        default boolean check(String name) {
+            return check(name, null);
+        }
+
+        boolean check(String name, String selector);
     }
 
     /**

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

@@ -193,7 +193,12 @@ public static class IsResourceAuthorizedPredicate {
         // public for tests
         public IsResourceAuthorizedPredicate(StringMatcher resourceNameMatcher, StringMatcher additionalNonDatastreamNameMatcher) {
             this((String name, @Nullable IndexAbstraction indexAbstraction) -> {
-                assert indexAbstraction == null || name.equals(indexAbstraction.getName());
+                // TODO this is hacky
+                assert indexAbstraction == null
+                    || (name.equals(indexAbstraction.getName())
+                        || name.equals(
+                            IndexNameExpressionResolver.combineSelector(indexAbstraction.getName(), IndexComponentSelector.FAILURES)
+                        ));
                 return resourceNameMatcher.test(name)
                     || (isPartOfDatastream(indexAbstraction) == false && additionalNonDatastreamNameMatcher.test(name));
             });
@@ -240,7 +245,8 @@ public final boolean testDataStreamForFailureAccess(IndexAbstraction indexAbstra
             assert indexAbstraction != null && indexAbstraction.getType() == IndexAbstraction.Type.DATA_STREAM;
             return biPredicate.test(
                 IndexNameExpressionResolver.combineSelector(indexAbstraction.getName(), IndexComponentSelector.FAILURES),
-                null
+                // this cannot be `null`, since otherwise this is not treated as a data stream
+                indexAbstraction
             );
         }
 

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

@@ -37,6 +37,7 @@ public class FailureStoreSecurityRestIT extends SecurityOnTrialLicenseRestTestCa
 
     @SuppressWarnings("unchecked")
     public void testFailureStoreAccess() throws IOException {
+        // TODO test API keys
         String dataAccessRole = "data_access";
         String failureStoreAccessRole = "failure_store_access";
 
@@ -88,11 +89,14 @@ public void testFailureStoreAccess() throws IOException {
             performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search")),
             failedDocId
         );
-        // TODO fix this
-        // assertContainsDocIds(
-        // performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search?ignore_unavailable=true")),
-        // failedDocId
-        // );
+        assertContainsDocIds(
+            performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test1::failures/_search?ignore_unavailable=true")),
+            failedDocId
+        );
+        assertContainsDocIds(
+            performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + failureIndexName + "/_search?ignore_unavailable=true")),
+            failedDocId
+        );
 
         expectThrows404(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test12::failures/_search")));
         expectThrows404(() -> performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2::failures/_search")));
@@ -109,9 +113,9 @@ public void testFailureStoreAccess() throws IOException {
         assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2::data/_search?ignore_unavailable=true")));
         assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/test2/_search?ignore_unavailable=true")));
         // TODO fix this
-        // assertEmpty(
-        // performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search?ignore_unavailable=true"))
-        // );
+        assertEmpty(
+            performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/" + dataIndexName + "/_search?ignore_unavailable=true"))
+        );
 
         // assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*1::data/_search")));
         // assertEmpty(performRequest(FAILURE_STORE_ACCESS_USER, new Request("GET", "/*1/_search")));

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

@@ -31,6 +31,7 @@
 import org.elasticsearch.action.search.TransportClosePointInTimeAction;
 import org.elasticsearch.action.search.TransportMultiSearchAction;
 import org.elasticsearch.action.search.TransportSearchScrollAction;
+import org.elasticsearch.action.support.IndexComponentSelector;
 import org.elasticsearch.action.termvectors.MultiTermVectorsAction;
 import org.elasticsearch.cluster.metadata.DataStream;
 import org.elasticsearch.cluster.metadata.IndexAbstraction;
@@ -106,6 +107,7 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.function.BiPredicate;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
@@ -907,13 +909,20 @@ static AuthorizedIndices resolveAuthorizedIndicesFromRole(
             }
             timeChecker.accept(indicesAndAliases);
             return indicesAndAliases;
-        }, name -> {
+        }, (name, selector) -> {
             final IndexAbstraction indexAbstraction = lookup.get(name);
             if (indexAbstraction == null) {
                 // test access (by name) to a resource that does not currently exist
                 // the action handler must handle the case of accessing resources that do not exist
                 return predicate.test(name, null);
             } else {
+                if (indexAbstraction.isFailureIndexOfDataStream()
+                    && predicate.testDataStreamForFailureAccess(indexAbstraction.getParentDataStream())) {
+                    return true;
+                }
+                if (IndexComponentSelector.FAILURES.getKey().equals(selector)) {
+                    return predicate.testDataStreamForFailureAccess(indexAbstraction);
+                }
                 // 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.
@@ -1046,9 +1055,9 @@ private static boolean isAsyncRelatedAction(String action) {
     static final class AuthorizedIndices implements AuthorizationEngine.AuthorizedIndices {
 
         private final CachedSupplier<Set<String>> allAuthorizedAndAvailableSupplier;
-        private final Predicate<String> isAuthorizedPredicate;
+        private final BiPredicate<String, String> isAuthorizedPredicate;
 
-        AuthorizedIndices(Supplier<Set<String>> allAuthorizedAndAvailableSupplier, Predicate<String> isAuthorizedPredicate) {
+        AuthorizedIndices(Supplier<Set<String>> allAuthorizedAndAvailableSupplier, BiPredicate<String, String> isAuthorizedPredicate) {
             this.allAuthorizedAndAvailableSupplier = CachedSupplier.wrap(allAuthorizedAndAvailableSupplier);
             this.isAuthorizedPredicate = Objects.requireNonNull(isAuthorizedPredicate);
         }
@@ -1059,8 +1068,8 @@ public Supplier<Set<String>> all() {
         }
 
         @Override
-        public boolean check(String name) {
-            return this.isAuthorizedPredicate.test(name);
+        public boolean check(String name, String selector) {
+            return isAuthorizedPredicate.test(name, selector);
         }
     }
 }

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java

@@ -1455,7 +1455,6 @@ public void testBackingIndicesAreIncludedForAuthorizedDataStreams() {
 
     public void testExplicitMappingUpdatesAreNotGrantedWithIngestPrivileges() {
         final String dataStreamName = "my_data_stream";
-        User user = new User(randomAlphaOfLengthBetween(4, 12));
         Role role = Role.builder(RESTRICTED_INDICES, "test1")
             .cluster(Collections.emptySet(), Collections.emptyList())
             .add(IndexPrivilege.CREATE, "my_*")