Skip to content

Commit a8c25fd

Browse files
authored
Optimize FLS/DLS setup in IndicePermission authz (#77832)
This change optimizes the creation and tracking of FieldPermissions and DocumentLevelPermissions in IndiciesPermission.authorize so that the method executes more quickly when dealing with large numbers of indices that do not make use of FLS/DLS The core of this change is a recognition that 1. Most usage of Elasticsearch does not rely on DLS/FLS and therefore the FieldPermissions and DocumentLevelPermissions objects will be the default/allow-all objects only. 2. In cases where DLS/FLS are used, most security configurations will have a single set of DLS/FLS permissions per index However, prior to this change the internal data structures were optimized for cases where there were multiple FLS/DLS rules to merge and apply. Performance for the overwhelming majority of use cases can be improved by optimizing for the single-rule scenario and treating the mergine of DLS/FLS rules as an exceptional case.
1 parent 2cfdadc commit a8c25fd

File tree

1 file changed

+37
-12
lines changed

1 file changed

+37
-12
lines changed

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

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -383,21 +383,45 @@ public Map<String, IndicesAccessControl.IndexAccessControl> authorize(
383383
if (actionCheck || bwcMappingActionCheck) {
384384
// propagate DLS and FLS permissions over the concrete indices
385385
for (String index : concreteIndices) {
386-
Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.computeIfAbsent(index, (k) -> new HashSet<>());
387-
fieldPermissionsByIndex.put(resource.name, fieldPermissions);
388-
fieldPermissions.add(group.getFieldPermissions());
386+
final Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.compute(index, (k, existingSet) -> {
387+
if (existingSet == null) {
388+
// Most indices rely on the default (empty) field permissions object, so we optimize for that case
389+
// Using an immutable single item set is significantly faster because it avoids any of the hashing
390+
// and backing set creation.
391+
return Set.of(group.getFieldPermissions());
392+
} else if (existingSet.size() == 1) {
393+
FieldPermissions fp = group.getFieldPermissions();
394+
if (existingSet.contains(fp)) {
395+
return existingSet;
396+
}
397+
// This index doesn't have a single field permissions object, replace the singleton with a real Set
398+
final Set<FieldPermissions> hashSet = new HashSet<>(existingSet);
399+
hashSet.add(fp);
400+
return hashSet;
401+
} else {
402+
existingSet.add(group.getFieldPermissions());
403+
return existingSet;
404+
}
405+
});
389406

390-
DocumentLevelPermissions permissions =
391-
roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions());
392-
roleQueriesByIndex.putIfAbsent(resource.name, permissions);
407+
DocumentLevelPermissions docPermissions;
393408
if (group.hasQuery()) {
394-
permissions.addAll(group.getQuery());
409+
docPermissions = roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions());
410+
docPermissions.addAll(group.getQuery());
395411
} else {
396412
// if more than one permission matches for a concrete index here and if
397413
// a single permission doesn't have a role query then DLS will not be
398414
// applied even when other permissions do have a role query
399-
permissions.setAllowAll();
415+
docPermissions = DocumentLevelPermissions.ALLOW_ALL;
416+
// don't worry about what's already there - just overwrite it, it avoids doing a 2nd hash lookup.
417+
roleQueriesByIndex.put(index, docPermissions);
418+
}
419+
420+
if (index.equals(resource.name) == false) {
421+
fieldPermissionsByIndex.put(resource.name, fieldPermissions);
422+
roleQueriesByIndex.put(resource.name, docPermissions);
400423
}
424+
401425
}
402426
if (false == actionCheck) {
403427
for (String privilegeName : group.privilege.name()) {
@@ -547,6 +571,11 @@ public Automaton getIndexMatcherAutomaton() {
547571

548572
private static class DocumentLevelPermissions {
549573

574+
public static final DocumentLevelPermissions ALLOW_ALL = new DocumentLevelPermissions();
575+
static {
576+
ALLOW_ALL.allowAll = true;
577+
}
578+
550579
private Set<BytesReference> queries = null;
551580
private boolean allowAll = false;
552581

@@ -562,9 +591,5 @@ private void addAll(Set<BytesReference> query) {
562591
private boolean isAllowAll() {
563592
return allowAll;
564593
}
565-
566-
private void setAllowAll() {
567-
this.allowAll = true;
568-
}
569594
}
570595
}

0 commit comments

Comments
 (0)