Merge branch 'master' into improve-delete-pipeline-check

weizijun · Sep 26, 2021 · 6ba29fb · 6ba29fb
2 parents 94b7c9f + a8c25fd
commit 6ba29fb
Showing 1 changed file with 37 additions and 12 deletions.
diff --git a/...c/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java b/...c/main/java/org/elasticsearch/xpack/core/security/authz/permission/IndicesPermission.java
@@ -383,21 +383,45 @@ public Map<String, IndicesAccessControl.IndexAccessControl> authorize(
                     if (actionCheck || bwcMappingActionCheck) {
                         // propagate DLS and FLS permissions over the concrete indices
                         for (String index : concreteIndices) {
-                            Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.computeIfAbsent(index, (k) -> new HashSet<>());
-                            fieldPermissionsByIndex.put(resource.name, fieldPermissions);
-                            fieldPermissions.add(group.getFieldPermissions());
+                            final Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.compute(index, (k, existingSet) -> {
+                                if (existingSet == null) {
+                                    // Most indices rely on the default (empty) field permissions object, so we optimize for that case
+                                    // Using an immutable single item set is significantly faster because it avoids any of the hashing
+                                    // and backing set creation.
+                                    return Set.of(group.getFieldPermissions());
+                                } else if (existingSet.size() == 1) {
+                                    FieldPermissions fp = group.getFieldPermissions();
+                                    if (existingSet.contains(fp)) {
+                                        return existingSet;
+                                    }
+                                    // This index doesn't have a single field permissions object, replace the singleton with a real Set
+                                    final Set<FieldPermissions> hashSet = new HashSet<>(existingSet);
+                                    hashSet.add(fp);
+                                    return hashSet;
+                                } else {
+                                    existingSet.add(group.getFieldPermissions());
+                                    return existingSet;
+                                }
+                            });
 
-                            DocumentLevelPermissions permissions =
-                                    roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions());
-                            roleQueriesByIndex.putIfAbsent(resource.name, permissions);
+                            DocumentLevelPermissions docPermissions;
                             if (group.hasQuery()) {
-                                permissions.addAll(group.getQuery());
+                                docPermissions = roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions());
+                                docPermissions.addAll(group.getQuery());
                             } else {
                                 // if more than one permission matches for a concrete index here and if
                                 // a single permission doesn't have a role query then DLS will not be
                                 // applied even when other permissions do have a role query
-                                permissions.setAllowAll();
+                                docPermissions = DocumentLevelPermissions.ALLOW_ALL;
+                                // don't worry about what's already there - just overwrite it, it avoids doing a 2nd hash lookup.
+                                roleQueriesByIndex.put(index, docPermissions);
+                            }
+
+                            if (index.equals(resource.name) == false) {
+                                fieldPermissionsByIndex.put(resource.name, fieldPermissions);
+                                roleQueriesByIndex.put(resource.name, docPermissions);
                             }
+
                         }
                         if (false == actionCheck) {
                             for (String privilegeName : group.privilege.name()) {
@@ -547,6 +571,11 @@ public Automaton getIndexMatcherAutomaton() {
 
     private static class DocumentLevelPermissions {
 
+        public static final DocumentLevelPermissions ALLOW_ALL = new DocumentLevelPermissions();
+        static {
+            ALLOW_ALL.allowAll = true;
+        }
+
         private Set<BytesReference> queries = null;
         private boolean allowAll = false;
 
@@ -562,9 +591,5 @@ private void addAll(Set<BytesReference> query) {
         private boolean isAllowAll() {
             return allowAll;
         }
-
-        private void setAllowAll() {
-            this.allowAll = true;
-        }
     }
 }