More robust and consistent allowAll indicesAccessControl (elastic#79415…

…) (elastic#79427)

* More robust and consistent allowAll indicesAccessControl (elastic#79415)

This PR ensures that AllowAllIndicesAccessControl is able to behave well
for all superclass's methods. Previously it throws NPE when it is asked
about Fls/Dls usage because it has a null index permissions map as a
placeholder. In this PR, we also get rid of the null and also mandate
non-null in the constructor of IndicesAccessControl.

In additional, whether a role has DLS/FLS and whether an
AllowAllIndicesAccessControl should be used for short circuit is
determined more consistently. In both places, whether a group has total
access to all indices is used as part of the criteria. Previously it is
possible that the role reports it has DLS/FLS while the
cindicesAccessControl does not have it. This could happen when one of
the group has DLS/FLS but another group has total access to all indices.
In this case, the code now correctly reports no DLS/FLS in both places.

Resolves: elastic#79361

* fix for 7.x quirks

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

@@ -40,7 +40,7 @@ public class IndicesAccessControl {
 
     public IndicesAccessControl(boolean granted, Map<String, IndexAccessControl> indexPermissions) {
         this.granted = granted;
-        this.indexPermissions = indexPermissions;
+        this.indexPermissions = Objects.requireNonNull(indexPermissions);
     }
 
     /**
@@ -292,7 +292,7 @@ private static class AllowAllIndicesAccessControl extends IndicesAccessControl {
         private final IndexAccessControl allowAllIndexAccessControl = new IndexAccessControl(true, null, null);
 
         private AllowAllIndicesAccessControl() {
-            super(true, null);
+            super(true, org.elasticsearch.core.Map.of());
         }
 
         @Override
@@ -301,13 +301,8 @@ public IndexAccessControl getIndexPermissions(String index) {
         }
 
         @Override
-        public boolean isGranted() {
-            return true;
-        }
-
-        @Override
-        public Collection<?> getDeniedIndices() {
-            return org.elasticsearch.core.Set.of();
+        public String toString() {
+            return "AllowAllIndicesAccessControl{}";
         }
     }
 

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

@@ -59,8 +59,8 @@ public final class IndicesPermission {
 
     public IndicesPermission(Group... groups) {
         this.groups = groups;
-        this.hasFieldOrDocumentLevelSecurity = Arrays.stream(groups)
-            .anyMatch(g -> g.hasQuery() || g.fieldPermissions.hasFieldLevelSecurity());
+        this.hasFieldOrDocumentLevelSecurity = Arrays.stream(groups).noneMatch(Group::isTotal)
+            && Arrays.stream(groups).anyMatch(g -> g.hasQuery() || g.fieldPermissions.hasFieldLevelSecurity());
     }
 
     private static StringMatcher indexMatcher(Collection<String> ordinaryIndices, Collection<String> restrictedIndices) {

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

@@ -18,6 +18,7 @@
 import java.util.Collections;
 import java.util.Set;
 
+import static org.hamcrest.Matchers.emptyIterable;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
@@ -122,4 +123,19 @@ public void testSLimitedIndicesAccessControl() {
         assertThat(result.getIndexPermissions("_index").getDocumentPermissions().getQueries(), is(nullValue()));
         assertThat(result.getIndexPermissions("_index").getDocumentPermissions().getLimitedByQueries(), equalTo(queries));
     }
+
+    public void testAllowAllIndicesAccessControl() {
+        final IndicesAccessControl allowAll = IndicesAccessControl.allowAll();
+        final IndexAccessControl indexAccessControl = allowAll.getIndexPermissions(randomAlphaOfLengthBetween(3, 8));
+        assertThat(indexAccessControl.isGranted(), is(true));
+        assertThat(indexAccessControl.getDocumentPermissions(), is(DocumentPermissions.allowAll()));
+        assertThat(indexAccessControl.getFieldPermissions(), is(FieldPermissions.DEFAULT));
+        assertThat(allowAll.getDeniedIndices(), emptyIterable());
+        assertThat(allowAll.getFieldAndDocumentLevelSecurityUsage(), is(IndicesAccessControl.DlsFlsUsage.NONE));
+        assertThat(allowAll.getIndicesWithFieldOrDocumentLevelSecurity(), emptyIterable());
+
+        final IndicesAccessControl indicesAccessControl = new IndicesAccessControl(randomBoolean(), org.elasticsearch.core.Map.of());
+        assertThat(allowAll.limitIndicesAccessControl(indicesAccessControl), is(indicesAccessControl));
+        assertThat(indicesAccessControl.limitIndicesAccessControl(allowAll), is(indicesAccessControl));
+    }
 }

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

@@ -469,6 +469,34 @@ public void testAuthorizationForMappingUpdates() {
         }
     }
 
+    public void testIndicesPermissionHasFieldOrDocumentLevelSecurity() {
+        // Make sure we have at least one of fieldPermissions and documentPermission
+        final FieldPermissions fieldPermissions = randomBoolean() ?
+            new FieldPermissions(new FieldPermissionsDefinition(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY)) :
+            FieldPermissions.DEFAULT;
+        final Set<BytesReference> queries;
+        if (fieldPermissions == FieldPermissions.DEFAULT) {
+            queries = org.elasticsearch.core.Set.of(new BytesArray("a query"));
+        } else {
+            queries = randomBoolean() ? org.elasticsearch.core.Set.of(new BytesArray("a query")) : null;
+        }
+
+        final IndicesPermission indicesPermission1 = new IndicesPermission(
+            new IndicesPermission.Group(IndexPrivilege.ALL, fieldPermissions, queries, randomBoolean(), "*"));
+        assertThat(indicesPermission1.hasFieldOrDocumentLevelSecurity(), is(true));
+
+        // IsTotal means no DLS/FLS
+        final IndicesPermission indicesPermission2 = new IndicesPermission(
+            new IndicesPermission.Group(IndexPrivilege.ALL, FieldPermissions.DEFAULT, null, true, "*"));
+        assertThat(indicesPermission2.hasFieldOrDocumentLevelSecurity(), is(false));
+
+        // IsTotal means NO DLS/FLS even when there is another group that has DLS/FLS
+        final IndicesPermission indicesPermission3 = new IndicesPermission(
+            new IndicesPermission.Group(IndexPrivilege.ALL, FieldPermissions.DEFAULT, null, true, "*"),
+            new IndicesPermission.Group(IndexPrivilege.NONE, fieldPermissions, queries, randomBoolean(), "*"));
+        assertThat(indicesPermission3.hasFieldOrDocumentLevelSecurity(), is(false));
+    }
+
     private static IndexMetadata createIndexMetadata(String name) {
         Settings.Builder settingsBuilder = Settings.builder()
             .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)