Improve shard level request cache efficiency (elastic#69505) (elastic…

…#69507)

Shard level request cache is improved to work correctly at all time. Also ensure profiling and suggester are properly disabled when not supported.

server/src/main/java/org/elasticsearch/search/internal/ShardSearchRequest.java

@@ -66,7 +66,7 @@ public class ShardSearchRequest extends TransportRequest implements IndicesReque
     private final Scroll scroll;
     private final String[] types;
     private final float indexBoost;
-    private final Boolean requestCache;
+    private Boolean requestCache;
     private final long nowInMillis;
     private final boolean allowPartialSearchResults;
     private final OriginalIndices originalIndices;
@@ -345,6 +345,10 @@ public Boolean requestCache() {
         return requestCache;
     }
 
+    public void requestCache(Boolean requestCache) {
+        this.requestCache = requestCache;
+    }
+
     public boolean allowPartialSearchResults() {
         return allowPartialSearchResults;
     }

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/DocumentLevelSecurityTests.java

@@ -129,7 +129,8 @@ protected String configUsers() {
             "user1:" + usersPasswdHashed + "\n" +
             "user2:" + usersPasswdHashed + "\n" +
             "user3:" + usersPasswdHashed + "\n" +
-            "user4:" + usersPasswdHashed + "\n";
+            "user4:" + usersPasswdHashed + "\n" +
+            "user5:" + usersPasswdHashed + "\n";
     }
 
     @Override
@@ -138,7 +139,8 @@ protected String configUsersRoles() {
                 "role1:user1,user2,user3\n" +
                 "role2:user1,user3\n" +
                 "role3:user2,user3\n" +
-                "role4:user4\n";
+                "role4:user4\n" +
+                "role5:user5\n";
     }
 
     @Override
@@ -171,7 +173,17 @@ protected String configRoles() {
                 "    - names: '*'\n" +
                 "      privileges: [ ALL ]\n" +
                 // query that can match nested documents
-                "      query: '{\"bool\": { \"must_not\": { \"term\" : {\"field1\" : \"value2\"}}}}'";
+                "      query: '{\"bool\": { \"must_not\": { \"term\" : {\"field1\" : \"value2\"}}}}'\n" +
+                "role5:\n" +
+                "  cluster: [ all ]\n" +
+                "  indices:\n" +
+                "    - names: [ 'test' ]\n" +
+                "      privileges: [ read ]\n" +
+                "      query: '{\"term\" : {\"field2\" : \"value2\"}}'\n" +
+                "    - names: [ 'fls-index' ]\n" +
+                "      privileges: [ read ]\n" +
+                "      field_security:\n" +
+                "         grant: [ 'field1', 'other_field', 'suggest_field2' ]\n";
     }
 
     @Override
@@ -1278,6 +1290,15 @@ public void testSuggesters() throws Exception {
                         .endObject()).get();
         refresh("test");
 
+        assertAcked(client().admin().indices().prepareCreate("fls-index")
+            .setSettings(Settings.builder()
+                .put("index.number_of_shards", 1)
+                .put("index.number_of_replicas", 0)
+            )
+            .addMapping("type1", "field1", "type=text", "suggest_field1", "type=text", "suggest_field2", "type=completion",
+                "yet_another", "type=text")
+        );
+
         // Term suggester:
         SearchResponse response = client()
                 .prepareSearch("test")
@@ -1293,9 +1314,13 @@ public void testSuggesters() throws Exception {
         assertThat(termSuggestion.getEntries().get(0).getOptions().size(), equalTo(1));
         assertThat(termSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));
 
+        final String[] indices =
+            randomFrom(org.elasticsearch.common.collect.List.of(
+                new String[] { "test" }, new String[] { "fls-index", "test" }, new String[] { "test", "fls-index" }));
+
         Exception e = expectThrows(ElasticsearchSecurityException.class, () -> client()
-                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
-                .prepareSearch("test")
+                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
+                .prepareSearch(indices)
                 .suggest(new SuggestBuilder()
                         .setGlobalText("valeu")
                         .addSuggestion("_name1", new TermSuggestionBuilder("suggest_field1"))
@@ -1318,8 +1343,8 @@ public void testSuggesters() throws Exception {
         assertThat(phraseSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));
 
         e = expectThrows(ElasticsearchSecurityException.class, () -> client()
-                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
-                .prepareSearch("test")
+                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
+                .prepareSearch(indices)
                 .suggest(new SuggestBuilder()
                         .setGlobalText("valeu")
                         .addSuggestion("_name1", new PhraseSuggestionBuilder("suggest_field1"))
@@ -1342,8 +1367,8 @@ public void testSuggesters() throws Exception {
         assertThat(completionSuggestion.getEntries().get(0).getOptions().get(0).getText().string(), equalTo("value"));
 
         e = expectThrows(ElasticsearchSecurityException.class, () -> client()
-                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
-                .prepareSearch("test")
+                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
+                .prepareSearch(indices)
                 .suggest(new SuggestBuilder()
                         .setGlobalText("valeu")
                         .addSuggestion("_name1", new CompletionSuggestionBuilder("suggest_field2"))
@@ -1373,6 +1398,14 @@ public void testProfile() throws Exception {
                         .endObject()).get();
         refresh("test");
 
+        assertAcked(client().admin().indices().prepareCreate("fls-index")
+            .setSettings(Settings.builder()
+                .put("index.number_of_shards", 1)
+                .put("index.number_of_replicas", 0)
+            )
+            .addMapping("type1", "field1", "type=text", "other_field", "type=text", "yet_another", "type=text")
+        );
+
         SearchResponse response = client()
                 .prepareSearch("test")
                 .setProfile(true)
@@ -1389,9 +1422,12 @@ public void testProfile() throws Exception {
 //        ProfileResult profileResult = queryProfileShardResult.getQueryResults().get(0);
 //        assertThat(profileResult.getLuceneDescription(), equalTo("(other_field:value)^0.8"));
 
+        final String[] indices =
+            randomFrom(org.elasticsearch.common.collect.List.of(
+                new String[] { "test" }, new String[] { "fls-index", "test" }, new String[] { "test", "fls-index" }));
         Exception e = expectThrows(ElasticsearchSecurityException.class, () -> client()
-                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user2", USERS_PASSWD)))
-                .prepareSearch("test")
+                .filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user5", USERS_PASSWD)))
+                .prepareSearch(indices)
                 .setProfile(true)
                 .setQuery(new FuzzyQueryBuilder("other_field", "valeu"))
                 .get());

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/FieldAndDocumentLevelSecurityRequestInterceptor.java

@@ -11,9 +11,11 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.IndicesRequest;
 import org.elasticsearch.common.MemoizedSupplier;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.license.XPackLicenseState.Feature;
+import org.elasticsearch.transport.TransportActionProxy;
 import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine;
 import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
 import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.RequestInfo;
@@ -39,28 +41,37 @@ abstract class FieldAndDocumentLevelSecurityRequestInterceptor implements Reques
     @Override
     public void intercept(RequestInfo requestInfo, AuthorizationEngine authorizationEngine, AuthorizationInfo authorizationInfo,
                           ActionListener<Void> listener) {
-        if (requestInfo.getRequest() instanceof IndicesRequest) {
+        if (requestInfo.getRequest() instanceof IndicesRequest && false == TransportActionProxy.isProxyAction(requestInfo.getAction())) {
             IndicesRequest indicesRequest = (IndicesRequest) requestInfo.getRequest();
             boolean shouldIntercept = licenseState.isSecurityEnabled();
             MemoizedSupplier<Boolean> licenseChecker = new MemoizedSupplier<>(() -> licenseState.checkFeature(Feature.SECURITY_DLS_FLS));
             if (supports(indicesRequest) && shouldIntercept) {
                 final IndicesAccessControl indicesAccessControl =
                     threadContext.getTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY);
-                for (String index : requestIndices(indicesRequest)) {
+                boolean fieldLevelSecurityEnabled = false;
+                boolean documentLevelSecurityEnabled = false;
+                final String[] requestIndices = requestIndices(indicesRequest);
+                for (String index : requestIndices) {
                     IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(index);
                     if (indexAccessControl != null) {
-                        boolean fieldLevelSecurityEnabled = indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
-                        boolean documentLevelSecurityEnabled = indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
-                        if ((fieldLevelSecurityEnabled || documentLevelSecurityEnabled) && licenseChecker.get()) {
-                            logger.trace("intercepted request for index [{}] with field level access controls [{}] " +
-                                "document level access controls [{}]. disabling conflicting features",
-                                index, fieldLevelSecurityEnabled, documentLevelSecurityEnabled);
-                            disableFeatures(indicesRequest, fieldLevelSecurityEnabled, documentLevelSecurityEnabled, listener);
-                            return;
+                        fieldLevelSecurityEnabled =
+                            fieldLevelSecurityEnabled || indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
+                        documentLevelSecurityEnabled =
+                            documentLevelSecurityEnabled || indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
+                        if (fieldLevelSecurityEnabled && documentLevelSecurityEnabled) {
+                            break;
                         }
                     }
-                    logger.trace("intercepted request for index [{}] without field or document level access controls", index);
                 }
+                if ((fieldLevelSecurityEnabled || documentLevelSecurityEnabled) && licenseChecker.get()) {
+                    logger.trace("intercepted request for indices [{}] with field level access controls [{}] " +
+                            "document level access controls [{}]. disabling conflicting features",
+                        Strings.arrayToDelimitedString(requestIndices, ","), fieldLevelSecurityEnabled, documentLevelSecurityEnabled);
+                    disableFeatures(indicesRequest, fieldLevelSecurityEnabled, documentLevelSecurityEnabled, listener);
+                    return;
+                }
+                logger.trace("intercepted request for indices [{}] without field or document level access controls",
+                    Strings.arrayToDelimitedString(requestIndices, ","));
             }
         }
         listener.onResponse(null);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/interceptor/SearchRequestInterceptor.java

@@ -12,10 +12,12 @@
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.rest.RestStatus;
+import org.elasticsearch.search.builder.SearchSourceBuilder;
+import org.elasticsearch.search.internal.ShardSearchRequest;
 import org.elasticsearch.threadpool.ThreadPool;
 
 /**
- * If field level security is enabled this interceptor disables the request cache for search requests.
+ * If field level security is enabled this interceptor disables the request cache for search and shardSearch requests.
  */
 public class SearchRequestInterceptor extends FieldAndDocumentLevelSecurityRequestInterceptor {
 
@@ -26,14 +28,25 @@ public SearchRequestInterceptor(ThreadPool threadPool, XPackLicenseState license
     @Override
     public void disableFeatures(IndicesRequest indicesRequest, boolean fieldLevelSecurityEnabled, boolean documentLevelSecurityEnabled,
                                 ActionListener<Void> listener) {
-        final SearchRequest request = (SearchRequest) indicesRequest;
-        request.requestCache(false);
+        assert indicesRequest instanceof SearchRequest || indicesRequest instanceof ShardSearchRequest
+            : "request must be either SearchRequest or ShardSearchRequest";
+
+        final SearchSourceBuilder source;
+        if (indicesRequest instanceof SearchRequest) {
+            final SearchRequest request = (SearchRequest) indicesRequest;
+            request.requestCache(false);
+            source = request.source();
+        } else {
+            final ShardSearchRequest request = (ShardSearchRequest) indicesRequest;
+            request.requestCache(false);
+            source = request.source();
+        }
 
         if (documentLevelSecurityEnabled) {
-            if (request.source() != null && request.source().suggest() != null) {
+            if (source != null && source.suggest() != null) {
                 listener.onFailure(new ElasticsearchSecurityException("Suggest isn't supported if document level security is enabled",
                         RestStatus.BAD_REQUEST));
-            } else if (request.source() != null && request.source().profile()) {
+            } else if (source != null && source.profile()) {
                 listener.onFailure(new ElasticsearchSecurityException("A search request cannot be profiled if document level security " +
                     "is enabled", RestStatus.BAD_REQUEST));
             } else {
@@ -46,6 +59,6 @@ public void disableFeatures(IndicesRequest indicesRequest, boolean fieldLevelSec
 
     @Override
     public boolean supports(IndicesRequest request) {
-        return request instanceof SearchRequest;
+        return request instanceof SearchRequest || request instanceof ShardSearchRequest;
     }
 }

x-pack/qa/multi-cluster-search-security/build.gradle

@@ -1,3 +1,4 @@
+import org.elasticsearch.gradle.info.BuildParams
 import org.elasticsearch.gradle.test.RestIntegTestTask
 
 apply plugin: 'elasticsearch.testclusters'
@@ -19,6 +20,9 @@ tasks.register('remote-cluster', RestIntegTestTask) {
   systemProperty 'tests.rest.suite', 'remote_cluster'
 }
 
+// randomise between sniff and proxy modes
+boolean proxyMode = (new Random(Long.parseUnsignedLong(BuildParams.testSeed.tokenize(':').get(0), 16))).nextBoolean()
+
 testClusters {
   'remote-cluster' {
     testDistribution = 'DEFAULT'
@@ -38,8 +42,15 @@ testClusters {
     setting 'xpack.watcher.enabled', 'false'
     setting 'xpack.ml.enabled', 'false'
     setting 'xpack.license.self_generated.type', 'trial'
-    setting 'cluster.remote.my_remote_cluster.seeds', {
-      testClusters.'remote-cluster'.getAllTransportPortURI().collect { "\"$it\"" }.toString()
+    if (proxyMode) {
+      setting 'cluster.remote.my_remote_cluster.mode', 'proxy'
+      setting 'cluster.remote.my_remote_cluster.proxy_address', {
+        "\"${testClusters.'remote-cluster'.getAllTransportPortURI().get(0)}\""
+      }
+    } else {
+      setting 'cluster.remote.my_remote_cluster.seeds', {
+        testClusters.'remote-cluster'.getAllTransportPortURI().collect { "\"$it\"" }.toString()
+      }
     }
     setting 'cluster.remote.connections_per_cluster', "1"
 
@@ -51,6 +62,14 @@ tasks.register('mixed-cluster', RestIntegTestTask) {
   dependsOn 'remote-cluster'
   useCluster testClusters.'remote-cluster'
   systemProperty 'tests.rest.suite', 'multi_cluster'
+  if (proxyMode) {
+    systemProperty 'tests.rest.blacklist', [
+      'multi_cluster/10_basic/Add transient remote cluster based on the preset cluster',
+      'multi_cluster/20_info/Add transient remote cluster based on the preset cluster and check remote info',
+      'multi_cluster/20_info/Fetch remote cluster info for existing cluster',
+      'multi_cluster/70_connection_mode_configuration/*',
+    ].join(",")
+  }
 }
 
 tasks.register("integTest") {

x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/multi_cluster/100_resolve_index.yml

@@ -34,11 +34,12 @@
   - match: {indices.7.attributes.0: open }
   - match: {indices.8.name: my_remote_cluster:secured_via_alias}
   - match: {indices.8.attributes.0: open}
-  - match: {indices.9.name: my_remote_cluster:single_doc_index}
-  - match: {indices.10.attributes.0: open}
-  - match: {indices.10.name: my_remote_cluster:test_index}
-  - match: {indices.10.aliases.0: aliased_test_index}
-  - match: {indices.10.attributes.0: open}
+  - match: {indices.9.name: my_remote_cluster:shared_index}
+  - match: {indices.10.name: my_remote_cluster:single_doc_index}
+  - match: {indices.11.attributes.0: open}
+  - match: {indices.11.name: my_remote_cluster:test_index}
+  - match: {indices.11.aliases.0: aliased_test_index}
+  - match: {indices.11.attributes.0: open}
   - match: {aliases.0.name: my_remote_cluster:.security}
   - match: {aliases.0.indices.0: .security-7}
   - match: {aliases.1.name: my_remote_cluster:aliased_closed_index}

x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/multi_cluster/110_dls_fls.yml

@@ -0,0 +1,76 @@
+---
+setup:
+  - skip:
+      features: headers
+
+  - do:
+      cluster.health:
+        wait_for_status: yellow
+
+  - do:
+      security.put_user:
+        username: "dls_fls_user"
+        body:  >
+          {
+            "password": "s3krit-password",
+            "roles" : [ "dls_fls_role" ]
+          }
+
+---
+teardown:
+  - do:
+      security.delete_user:
+        username: "dls_fls_user"
+        ignore: 404
+
+---
+"Search with document and field level security":
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        request_cache: true
+        index: my_remote_cluster:shared_index
+
+  - match: { hits.total: 2}
+  - length: { hits.hits.0._source: 3 }
+  - match: { hits.hits.0._source.secret: "sesame" }
+
+  - do:
+      headers: { Authorization: "Basic ZGxzX2Zsc191c2VyOnMza3JpdC1wYXNzd29yZA==" }
+      search:
+        rest_total_hits_as_int: true
+        ccs_minimize_roundtrips: false
+        request_cache: true
+        index: my_remote_cluster:shared_index
+
+  - match: { hits.total: 1}
+  - length: { hits.hits.0._source: 2 }
+  - is_true: hits.hits.0._source.public
+  - match: { hits.hits.0._source.name: "doc 1" }
+
+---
+"Async search with document and field level security":
+  - skip:
+      version: " - 7.6.99"
+      reason: "async search available since 7.7"
+
+  - do:
+      async_search.submit:
+        index: my_remote_cluster:shared_index
+        wait_for_completion_timeout: 10s
+
+  - match: { response.hits.total.value: 2}
+  - length: { response.hits.hits.0._source: 3 }
+  - match: { response.hits.hits.0._source.secret: "sesame" }
+
+  - do:
+      headers: { Authorization: "Basic ZGxzX2Zsc191c2VyOnMza3JpdC1wYXNzd29yZA==" }
+      async_search.submit:
+        index: my_remote_cluster:shared_index
+        wait_for_completion_timeout: 10s
+
+  - match: { response.hits.total.value: 1}
+  - length: { response.hits.hits.0._source: 2 }
+  - is_true: response.hits.hits.0._source.public
+  - match: { response.hits.hits.0._source.name: "doc 1" }
+