Skip to content

Commit 821f36b

Browse files
ywangdtvernum
andauthored
Optimize FLS/DLS setup in IndicePermission authz (#77832) (#78297)
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. Co-authored-by: Tim Vernum <[email protected]>
1 parent 7102376 commit 821f36b

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
@@ -312,21 +312,45 @@ public Map<String, IndicesAccessControl.IndexAccessControl> authorize(
312312
if (actionCheck || bwcMappingActionCheck) {
313313
// propagate DLS and FLS permissions over the concrete indices
314314
for (String index : concreteIndices) {
315-
Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.computeIfAbsent(index, (k) -> new HashSet<>());
316-
fieldPermissionsByIndex.put(resource.name, fieldPermissions);
317-
fieldPermissions.add(group.getFieldPermissions());
315+
final Set<FieldPermissions> fieldPermissions = fieldPermissionsByIndex.compute(index, (k, existingSet) -> {
316+
if (existingSet == null) {
317+
// Most indices rely on the default (empty) field permissions object, so we optimize for that case
318+
// Using an immutable single item set is significantly faster because it avoids any of the hashing
319+
// and backing set creation.
320+
return org.elasticsearch.core.Set.of(group.getFieldPermissions());
321+
} else if (existingSet.size() == 1) {
322+
FieldPermissions fp = group.getFieldPermissions();
323+
if (existingSet.contains(fp)) {
324+
return existingSet;
325+
}
326+
// This index doesn't have a single field permissions object, replace the singleton with a real Set
327+
final Set<FieldPermissions> hashSet = new HashSet<>(existingSet);
328+
hashSet.add(fp);
329+
return hashSet;
330+
} else {
331+
existingSet.add(group.getFieldPermissions());
332+
return existingSet;
333+
}
334+
});
318335

319-
DocumentLevelPermissions permissions =
320-
roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions());
321-
roleQueriesByIndex.putIfAbsent(resource.name, permissions);
336+
DocumentLevelPermissions docPermissions;
322337
if (group.hasQuery()) {
323-
permissions.addAll(group.getQuery());
338+
docPermissions = roleQueriesByIndex.computeIfAbsent(index, (k) -> new DocumentLevelPermissions());
339+
docPermissions.addAll(group.getQuery());
324340
} else {
325341
// if more than one permission matches for a concrete index here and if
326342
// a single permission doesn't have a role query then DLS will not be
327343
// applied even when other permissions do have a role query
328-
permissions.setAllowAll(true);
344+
docPermissions = DocumentLevelPermissions.ALLOW_ALL;
345+
// don't worry about what's already there - just overwrite it, it avoids doing a 2nd hash lookup.
346+
roleQueriesByIndex.put(index, docPermissions);
347+
}
348+
349+
if (index.equals(resource.name) == false) {
350+
fieldPermissionsByIndex.put(resource.name, fieldPermissions);
351+
roleQueriesByIndex.put(resource.name, docPermissions);
329352
}
353+
330354
}
331355
if (false == actionCheck) {
332356
for (String privilegeName : group.privilege.name()) {
@@ -509,6 +533,11 @@ private static Predicate<IndexAbstraction> buildIndexMatcherPredicateForAction(S
509533

510534
private static class DocumentLevelPermissions {
511535

536+
public static final DocumentLevelPermissions ALLOW_ALL = new DocumentLevelPermissions();
537+
static {
538+
ALLOW_ALL.allowAll = true;
539+
}
540+
512541
private Set<BytesReference> queries = null;
513542
private boolean allowAll = false;
514543

@@ -524,9 +553,5 @@ private void addAll(Set<BytesReference> query) {
524553
private boolean isAllowAll() {
525554
return allowAll;
526555
}
527-
528-
private void setAllowAll(boolean allowAll) {
529-
this.allowAll = allowAll;
530-
}
531556
}
532557
}

0 commit comments

Comments
 (0)