Fix UOE on search requests that match a sparse role query (#43668)

Search requests executed through the SecurityIndexSearcherWrapper throw an UnsupportedOperationException if they match a sparse role query. When low level cancellation is activated (which is the default since #42857), the context index searcher creates a weight that doesn't handle #scorer. This change fixes this bug and adds a test to ensure that we check this case.
elastic · Jun 27, 2019 · 329d05f · 329d05f
1 parent cd4f81e
commit 329d05f
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 4 deletions.
diff --git a/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java b/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java
@@ -152,13 +152,14 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio
                 }
 
                 @Override
-                public Scorer scorer(LeafReaderContext context) throws IOException {
+                public boolean isCacheable(LeafReaderContext ctx) {
                     throw new UnsupportedOperationException();
                 }
 
                 @Override
-                public boolean isCacheable(LeafReaderContext ctx) {
-                    throw new UnsupportedOperationException();
+                public Scorer scorer(LeafReaderContext context) throws IOException {
+                    // in case the wrapped searcher (in) uses the scorer directly
+                    return weight.scorer(context);
                 }
 
                 @Override

diff --git a/...search/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperUnitTests.java b/...search/xpack/core/security/authz/accesscontrol/SecurityIndexSearcherWrapperUnitTests.java
@@ -43,6 +43,7 @@
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
+import org.elasticsearch.index.engine.Engine;
 import org.elasticsearch.index.mapper.FieldNamesFieldMapper;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.SeqNoFieldMapper;
@@ -52,6 +53,7 @@
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.search.aggregations.LeafBucketCollector;
+import org.elasticsearch.search.internal.ContextIndexSearcher;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.IndexSettingsModule;
 import org.elasticsearch.xpack.core.security.authz.accesscontrol.DocumentSubsetReader.DocumentSubsetDirectoryReader;
@@ -537,7 +539,11 @@ public void onRemoval(ShardId shardId, Accountable accountable) {
         }
 
         DocumentSubsetDirectoryReader filteredReader = DocumentSubsetReader.wrap(reader, cache, roleQuery);
-        IndexSearcher searcher = new SecurityIndexSearcherWrapper.IndexSearcherWrapper(filteredReader);
+        IndexSearcher wrapSearcher = new SecurityIndexSearcherWrapper.IndexSearcherWrapper(filteredReader);
+        Engine.Searcher engineSearcher = new Engine.Searcher("test", wrapSearcher, () -> {});
+        ContextIndexSearcher searcher = new ContextIndexSearcher(engineSearcher,
+            wrapSearcher.getQueryCache(), wrapSearcher.getQueryCachingPolicy());
+        searcher.setCheckCancelled(() -> {});
 
         // Searching a non-existing term will trigger a null scorer
         assertEquals(0, searcher.count(new TermQuery(new Term("non_existing_field", "non_existing_value"))));