Moar

n1v0lg · n1v0lg · commit 95af75e10de6 · 2025-01-31T10:59:14.000+01:00
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java b/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);
     }
 
     /**
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java
@@ -145,7 +145,8 @@ public boolean hasFieldOrDocumentLevelSecurity() {
     }
 
     private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String action) {
-        final Set<String> ordinaryIndices = new HashSet<>();
+        final Set<String> dataAccessOrdinaryIndices = new HashSet<>();
+        final Set<String> failureAccessOrdinaryIndices = new HashSet<>();
         final Set<String> restrictedIndices = new HashSet<>();
         final Set<String> grantMappingUpdatesOnIndices = new HashSet<>();
         final Set<String> grantMappingUpdatesOnRestrictedIndices = new HashSet<>();
@@ -155,7 +156,11 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String
                 if (group.allowRestrictedIndices) {
                     restrictedIndices.addAll(Arrays.asList(group.indices()));
                 } else {
-                    ordinaryIndices.addAll(Arrays.asList(group.indices()));
+                    if (group.failureStoreOnly) {
+                        failureAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
+                    } else {
+                        dataAccessOrdinaryIndices.addAll(Arrays.asList(group.indices()));
+                    }
                 }
             } else if (isMappingUpdateAction && containsPrivilegeThatGrantsMappingUpdatesForBwc(group)) {
                 // special BWC case for certain privileges: allow put mapping on indices and aliases (but not on data streams), even if
@@ -167,40 +172,56 @@ private IsResourceAuthorizedPredicate buildIndexMatcherPredicateForAction(String
                 }
             }
         }
-        final StringMatcher nameMatcher = indexMatcher(ordinaryIndices, restrictedIndices);
+        final StringMatcher dataAccessNameMatcher = indexMatcher(dataAccessOrdinaryIndices, restrictedIndices);
+        // TODO handle restricted indices for failure access
+        final StringMatcher failureAccessNameMatcher = indexMatcher(failureAccessOrdinaryIndices, Set.of());
         final StringMatcher bwcSpecialCaseMatcher = indexMatcher(grantMappingUpdatesOnIndices, grantMappingUpdatesOnRestrictedIndices);
-        return new IsResourceAuthorizedPredicate(nameMatcher, bwcSpecialCaseMatcher);
+        return new IsResourceAuthorizedPredicate(dataAccessNameMatcher, failureAccessNameMatcher, bwcSpecialCaseMatcher);
     }
 
     /**
      * This encapsulates the authorization test for resources.
      * There is an additional test for resources that are missing or that are not a datastream or a backing index.
      */
-    public static class IsResourceAuthorizedPredicate implements BiPredicate<String, IndexAbstraction> {
+    public static class IsResourceAuthorizedPredicate {
 
         private final BiPredicate<String, IndexAbstraction> biPredicate;
+        private final BiPredicate<String, IndexAbstraction> failureAccessBiPredicate;
 
         // public for tests
-        public IsResourceAuthorizedPredicate(StringMatcher resourceNameMatcher, StringMatcher additionalNonDatastreamNameMatcher) {
+        public IsResourceAuthorizedPredicate(
+            StringMatcher resourceNameMatcher,
+            StringMatcher failureAccessNameMatcher,
+            StringMatcher additionalNonDatastreamNameMatcher
+        ) {
             this((String name, @Nullable IndexAbstraction indexAbstraction) -> {
                 assert indexAbstraction == null || name.equals(indexAbstraction.getName());
                 return resourceNameMatcher.test(name)
                     || (isPartOfDatastream(indexAbstraction) == false && additionalNonDatastreamNameMatcher.test(name));
+            }, (String name, @Nullable IndexAbstraction indexAbstraction) -> {
+                assert indexAbstraction == null || name.equals(indexAbstraction.getName());
+                return failureAccessNameMatcher.test(name);
             });
         }
 
-        private IsResourceAuthorizedPredicate(BiPredicate<String, IndexAbstraction> biPredicate) {
+        private IsResourceAuthorizedPredicate(
+            BiPredicate<String, IndexAbstraction> biPredicate,
+            BiPredicate<String, IndexAbstraction> failureAccessBiPredicate
+        ) {
             this.biPredicate = biPredicate;
+            this.failureAccessBiPredicate = failureAccessBiPredicate;
         }
 
         /**
         * Given another {@link IsResourceAuthorizedPredicate} instance in {@param other},
         * return a new {@link IsResourceAuthorizedPredicate} instance that is equivalent to the conjunction of
         * authorization tests of that other instance and this one.
         */
-        @Override
-        public final IsResourceAuthorizedPredicate and(BiPredicate<? super String, ? super IndexAbstraction> other) {
-            return new IsResourceAuthorizedPredicate(this.biPredicate.and(other));
+        public final IsResourceAuthorizedPredicate and(IsResourceAuthorizedPredicate other) {
+            return new IsResourceAuthorizedPredicate(
+                this.biPredicate.and(other.biPredicate),
+                this.failureAccessBiPredicate.and(other.failureAccessBiPredicate)
+            );
         }
 
         /**
@@ -209,7 +230,11 @@ public final IsResourceAuthorizedPredicate and(BiPredicate<? super String, ? sup
          * Returns {@code true} if access to the given resource is authorized or {@code false} otherwise.
          */
         public final boolean test(IndexAbstraction indexAbstraction) {
-            return test(indexAbstraction.getName(), indexAbstraction);
+            return test(indexAbstraction.getName(), indexAbstraction, null);
+        }
+
+        public final boolean test(IndexAbstraction indexAbstraction, @Nullable String selector) {
+            return test(indexAbstraction.getName(), indexAbstraction, selector);
         }
 
         /**
@@ -218,9 +243,19 @@ public final boolean test(IndexAbstraction indexAbstraction) {
          * if it doesn't.
          * Returns {@code true} if access to the given resource is authorized or {@code false} otherwise.
          */
-        @Override
         public boolean test(String name, @Nullable IndexAbstraction indexAbstraction) {
-            return biPredicate.test(name, indexAbstraction);
+            return test(name, indexAbstraction, null);
+        }
+
+        public boolean test(String name, @Nullable IndexAbstraction indexAbstraction, @Nullable String selector) {
+            if (selector == null || IndexComponentSelector.DATA.getKey().equals(selector)) {
+                return biPredicate.test(name, indexAbstraction);
+            } else if (IndexComponentSelector.FAILURES.getKey().equals(selector)) {
+                return failureAccessBiPredicate.test(name, indexAbstraction);
+            } else {
+                assert selector.equals(IndexComponentSelector.ALL_APPLICABLE.getKey()) : "unexpected selector [" + selector + "]";
+                return biPredicate.test(name, indexAbstraction) && failureAccessBiPredicate.test(name, indexAbstraction);
+            }
         }
 
         private static boolean isPartOfDatastream(IndexAbstraction indexAbstraction) {
@@ -354,7 +389,7 @@ public Automaton allowedActionsMatcher(String index) {
      * Represents the set of data required about an IndexAbstraction (index/alias/datastream) in order to perform authorization on that
      * object (including setting up the necessary data structures for Field and Document Level Security).
      */
-    private static class IndexResource {
+    public static class IndexResource {
         /**
          * The name of the IndexAbstraction on which authorization is being performed
          */
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java
@@ -433,6 +433,7 @@ static SimpleRole buildFromRoleDescriptor(
         );
 
         for (RoleDescriptor.IndicesPrivileges indexPrivilege : roleDescriptor.getIndicesPrivileges()) {
+            // TODO properly handle this
             if (Arrays.asList(indexPrivilege.getIndices()).contains("*")) {
                 builder.add(
                     fieldPermissionsCache.getFieldPermissions(
@@ -441,7 +442,6 @@ static SimpleRole buildFromRoleDescriptor(
                     indexPrivilege.getQuery() == null ? null : Collections.singleton(indexPrivilege.getQuery()),
                     IndexPrivilege.get(Sets.newHashSet(indexPrivilege.getPrivileges())),
                     indexPrivilege.allowRestrictedIndices(),
-                    // TODO properly handle this
                     true,
                     indexPrivilege.getIndices()
                 );
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/StringMatcher.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/StringMatcher.java
@@ -50,6 +50,10 @@ public static StringMatcher of(Iterable<String> patterns) {
         return StringMatcher.builder().includeAll(patterns).build();
     }
 
+    public static StringMatcher never() {
+        return MATCH_NOTHING;
+    }
+
     public static StringMatcher of(String... patterns) {
         return StringMatcher.builder().includeAll(patterns).build();
     }
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java
@@ -106,6 +106,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;
@@ -868,7 +869,7 @@ static AuthorizedIndices resolveAuthorizedIndicesFromRole(
         // do not include data streams for actions that do not operate on data streams
         TransportRequest request = requestInfo.getRequest();
         final boolean includeDataStreams = (request instanceof IndicesRequest) && ((IndicesRequest) request).includeDataStreams();
-
+        // TODO need a function not a supplier
         return new AuthorizedIndices(() -> {
             Consumer<Collection<String>> timeChecker = timerSupplier.get();
             Set<String> indicesAndAliases = new HashSet<>();
@@ -898,18 +899,18 @@ 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);
+                return predicate.test(name, null, selector);
             } else {
                 // 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.
                 return (indexAbstraction.getParentDataStream() != null && predicate.test(indexAbstraction.getParentDataStream()))
-                    || predicate.test(indexAbstraction);
+                    || predicate.test(indexAbstraction, selector);
             }
         });
     }
@@ -1037,9 +1038,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);
         }
@@ -1050,8 +1051,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);
         }
     }
 }
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/accesscontrol/IndicesPermissionTests.java
@@ -694,6 +694,7 @@ public void testResourceAuthorizedPredicateForDatastreams() {
         );
         IndicesPermission.IsResourceAuthorizedPredicate predicate = new IndicesPermission.IsResourceAuthorizedPredicate(
             StringMatcher.of("other"),
+            StringMatcher.never(),
             StringMatcher.of(dataStreamName, backingIndex.getName(), concreteIndex.getName(), alias.getName())
         );
         assertThat(predicate.test(dataStream), is(false));
@@ -709,10 +710,12 @@ public void testResourceAuthorizedPredicateForDatastreams() {
     public void testResourceAuthorizedPredicateAnd() {
         IndicesPermission.IsResourceAuthorizedPredicate predicate1 = new IndicesPermission.IsResourceAuthorizedPredicate(
             StringMatcher.of("c", "a"),
+            StringMatcher.never(),
             StringMatcher.of("b", "d")
         );
         IndicesPermission.IsResourceAuthorizedPredicate predicate2 = new IndicesPermission.IsResourceAuthorizedPredicate(
             StringMatcher.of("c", "b"),
+            StringMatcher.never(),
             StringMatcher.of("a", "d")
         );
         Metadata.Builder mb = Metadata.builder(

Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,10 @@ public static StringMatcher of(Iterable<String> patterns) {`
`50`	`50`	`return StringMatcher.builder().includeAll(patterns).build();`
`51`	`51`	`}`
`52`	`52`
	`53`	`+ public static StringMatcher never() {`
	`54`	`+ return MATCH_NOTHING;`
	`55`	`+ }`
	`56`	`+`
`53`	`57`	`public static StringMatcher of(String... patterns) {`
`54`	`58`	`return StringMatcher.builder().includeAll(patterns).build();`
`55`	`59`	`}`