Skip to content

Commit 40a22d1

Browse files
authored
Rewrite FieldsExtractor using QueryVisitor API (#93390)
Lucene offers now a QueryVisitor API that is very convenient to iterate through a query tree and extract info from each subquery, like the number of fields as the FieldsExtractor did so far through instanceof checks that can only cover a limited amount of queries. This commit converts the method to use the query visitor API.
1 parent 38baa36 commit 40a22d1

File tree

3 files changed

+24
-276
lines changed

3 files changed

+24
-276
lines changed

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

-102
This file was deleted.

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

+24-15
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99

1010
import org.apache.logging.log4j.LogManager;
1111
import org.apache.logging.log4j.Logger;
12+
import org.apache.lucene.search.BooleanClause;
1213
import org.apache.lucene.search.QueryCachingPolicy;
14+
import org.apache.lucene.search.QueryVisitor;
1315
import org.apache.lucene.search.Weight;
1416
import org.elasticsearch.common.util.concurrent.ThreadContext;
1517
import org.elasticsearch.index.Index;
@@ -18,9 +20,7 @@
1820
import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField;
1921
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
2022

21-
import java.util.HashSet;
2223
import java.util.Objects;
23-
import java.util.Set;
2424

2525
/**
2626
* Opts out of the query cache if field level security is active for the current request, and it is unsafe to cache.
@@ -63,24 +63,33 @@ public Weight doCache(Weight weight, QueryCachingPolicy policy) {
6363
*/
6464
static boolean cachingIsSafe(Weight weight, IndicesAccessControl.IndexAccessControl permissions) {
6565
// support caching for common queries, by inspecting the field
66-
// TODO: If in the future there is a Query#extractFields() then we can do a better job
67-
Set<String> fields = new HashSet<>();
6866
try {
69-
FieldExtractor.extractFields(weight.getQuery(), fields);
70-
} catch (UnsupportedOperationException ok) {
71-
// we don't know how to safely extract the fields of this query, don't cache.
72-
return false;
73-
}
67+
weight.getQuery().visit(new QueryVisitor() {
68+
@Override
69+
public QueryVisitor getSubVisitor(BooleanClause.Occur occur, org.apache.lucene.search.Query parent) {
70+
return this; // we want to use the same visitor for must_not clauses too
71+
}
7472

75-
// we successfully extracted the set of fields: check each one
76-
for (String field : fields) {
77-
// don't cache any internal fields (e.g. _field_names), these are complicated.
78-
if (field.startsWith("_") || permissions.getFieldPermissions().grantsAccessTo(field) == false) {
79-
return false;
80-
}
73+
@Override
74+
public boolean acceptField(String field) {
75+
// don't cache any internal fields (e.g. _field_names), these are complicated.
76+
if (field.startsWith("_") || permissions.getFieldPermissions().grantsAccessTo(field) == false) {
77+
throw new FLSQueryNotCacheable("Query field has FLS permissions");
78+
}
79+
return super.acceptField(field);
80+
}
81+
});
82+
} catch (FLSQueryNotCacheable e) {
83+
return false;
8184
}
8285
// we can cache, all fields are ok
8386
return true;
8487
}
8588

89+
private static class FLSQueryNotCacheable extends RuntimeException {
90+
FLSQueryNotCacheable(String message) {
91+
// don't waste time filling in the stacktrace
92+
super(message, null, false, false);
93+
}
94+
}
8695
}

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

-159
This file was deleted.

0 commit comments

Comments
 (0)