Fix doc-update interceptor for indices with DLS and FLS (#61516)

This fixes the protection against updates (and bulk updates) for indices with DLS
and/or FLS, when the request uses date math expressions.

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

@@ -5,13 +5,16 @@
  */
 package org.elasticsearch.integration;
 
+import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
 import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsResponse;
 import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
+import org.elasticsearch.action.bulk.BulkResponse;
 import org.elasticsearch.action.fieldcaps.FieldCapabilities;
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest;
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
 import org.elasticsearch.action.search.SearchResponse;
+import org.elasticsearch.client.Requests;
 import org.elasticsearch.cluster.metadata.MappingMetadata;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.settings.SecureString;
@@ -23,6 +26,7 @@ import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.test.SecurityIntegTestCase;
 import org.elasticsearch.xpack.core.XPackSettings;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -34,7 +38,9 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitC
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;
 import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.BASIC_AUTH_HEADER;
 import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 
 public class DocumentAndFieldLevelSecurityTests extends SecurityIntegTestCase {
 
@@ -144,6 +150,41 @@ public class DocumentAndFieldLevelSecurityTests extends SecurityIntegTestCase {
         assertThat(response.getHits().getAt(1).getSourceAsMap().get("field2").toString(), equalTo("value2"));
     }
 
+    public void testUpdatesAreRejected() {
+        for (String indexName : Arrays.asList("<test-{2015.05.05||+1d}>", "test")) {
+            assertAcked(client().admin().indices().prepareCreate(indexName)
+                    .addMapping("type1", "id", "type=keyword", "field1", "type=text", "field2", "type=text")
+                    .setSettings(Settings.builder()
+                            .put("index.number_of_replicas", 0)
+                            .put("index.number_of_shards", 1))
+            );
+            client().prepareIndex(indexName, "type1", "1").setSource("id", "1", "field1", "value1")
+                    .setRefreshPolicy(IMMEDIATE)
+                    .get();
+
+            ElasticsearchSecurityException exception = expectThrows(ElasticsearchSecurityException.class, () -> {
+                client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER,
+                        basicAuthHeaderValue("user1", USERS_PASSWD)))
+                        .prepareUpdate(indexName, "type1", "1")
+                        .setDoc(Requests.INDEX_CONTENT_TYPE, "field2", "value2")
+                        .get();
+            });
+            assertThat(exception.getDetailedMessage(), containsString("Can't execute an update request if field or document level " +
+                    "security"));
+
+            BulkResponse bulkResponse = client().filterWithHeader(Collections.singletonMap(BASIC_AUTH_HEADER, basicAuthHeaderValue("user1",
+                    USERS_PASSWD)))
+                    .prepareBulk()
+                    .add(client().prepareUpdate(indexName, "type1", "1")
+                            .setDoc(Requests.INDEX_CONTENT_TYPE, "field2", "value2"))
+                    .get();
+            assertThat(bulkResponse.getItems().length, is(1));
+            assertThat(bulkResponse.getItems()[0].getFailureMessage(), containsString("Can't execute a bulk item request with update " +
+                    "requests" +
+                    " embedded if field or document level security is enabled"));
+        }
+    }
+
     public void testDLSIsAppliedBeforeFLS() {
         assertAcked(client().admin().indices().prepareCreate("test")
                 .addMapping("type1", "field1", "type=text", "field2", "type=text")

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

@@ -46,29 +46,30 @@ public class BulkShardRequestInterceptor implements RequestInterceptor {
         MemoizedSupplier<Boolean> licenseChecker = new MemoizedSupplier<>(() -> licenseState.checkFeature(Feature.SECURITY_DLS_FLS));
         if (requestInfo.getRequest() instanceof BulkShardRequest && shouldIntercept) {
             IndicesAccessControl indicesAccessControl = threadContext.getTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY);
-
+            BulkShardRequest bulkShardRequest = (BulkShardRequest) requestInfo.getRequest();
-            final BulkShardRequest bulkShardRequest = (BulkShardRequest) requestInfo.getRequest();
+            // this uses the {@code BulkShardRequest#index()} because the {@code bulkItemRequest#index()}
-            for (BulkItemRequest bulkItemRequest : bulkShardRequest.items()) {
+            // can still be an unresolved date math expression
-                IndicesAccessControl.IndexAccessControl indexAccessControl =
+            IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(bulkShardRequest.index());
-                    indicesAccessControl.getIndexPermissions(bulkItemRequest.index());
+            // TODO replace if condition with assertion
-                boolean found = false;
+            if (indexAccessControl != null) {
-                if (indexAccessControl != null) {
+                for (BulkItemRequest bulkItemRequest : bulkShardRequest.items()) {
-                    boolean fls = indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
+                    boolean found = false;
-                    boolean dls = indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
+                    if (bulkItemRequest.request() instanceof UpdateRequest) {
-                    if (fls || dls) {
+                        boolean fls = indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
-                        if (licenseChecker.get() && bulkItemRequest.request() instanceof UpdateRequest) {
+                        boolean dls = indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
+                        // the feature usage checker is a "last-ditch" verification, it doesn't have practical importance
+                        if ((fls || dls) && licenseChecker.get()) {
                             found = true;
-                            logger.trace("aborting bulk item update request for index [{}]", bulkItemRequest.index());
+                            logger.trace("aborting bulk item update request for index [{}]", bulkShardRequest.index());
                             bulkItemRequest.abort(bulkItemRequest.index(), new ElasticsearchSecurityException("Can't execute a bulk " +
-                                "item request with update requests embedded if field or document level security is enabled",
+                                    "item request with update requests embedded if field or document level security is enabled",
-                                RestStatus.BAD_REQUEST));
+                                    RestStatus.BAD_REQUEST));
                         }
                     }
-                }
+                    if (found == false) {
-
+                        logger.trace("intercepted bulk request for index [{}] without any update requests, continuing execution",
-                if (found == false) {
+                                bulkShardRequest.index());
-                    logger.trace("intercepted bulk request for index [{}] without any update requests, continuing execution",
+                    }
-                        bulkItemRequest.index());
                 }
             }
         }

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

@@ -45,7 +45,7 @@ abstract class FieldAndDocumentLevelSecurityRequestInterceptor implements Reques
             if (supports(indicesRequest) && shouldIntercept) {
                 final IndicesAccessControl indicesAccessControl =
                     threadContext.getTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY);
-                for (String index : indicesRequest.indices()) {
+                for (String index : requestIndices(indicesRequest)) {
                     IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(index);
                     if (indexAccessControl != null) {
                         boolean fieldLevelSecurityEnabled = indexAccessControl.getFieldPermissions().hasFieldLevelSecurity();
@@ -65,6 +65,10 @@ abstract class FieldAndDocumentLevelSecurityRequestInterceptor implements Reques
         listener.onResponse(null);
     }
 
+    String[] requestIndices(IndicesRequest indicesRequest) {
+        return indicesRequest.indices();
+    }
+
     abstract void disableFeatures(IndicesRequest request, boolean fieldLevelSecurityEnabled, boolean documentLevelSecurityEnabled,
                                   ActionListener<Void> listener);
 

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

@@ -33,6 +33,17 @@ public class UpdateRequestInterceptor extends FieldAndDocumentLevelSecurityReque
             "is enabled", RestStatus.BAD_REQUEST));
     }
 
+    @Override
+    String[] requestIndices(IndicesRequest indicesRequest) {
+        if (indicesRequest instanceof UpdateRequest) {
+            UpdateRequest updateRequest = (UpdateRequest) indicesRequest;
+            if (updateRequest.getShardId() != null) {
+                return new String[]{updateRequest.getShardId().getIndexName()};
+            }
+        }
+        return new String[0];
+    }
+
     @Override
     public boolean supports(IndicesRequest request) {
         return request instanceof UpdateRequest;