Short circuit privilege evaluation for bulk requests without index re…

…solution (#926)

src/main/java/com/amazon/opendistroforelasticsearch/security/privileges/PrivilegesEvaluator.java

@@ -57,6 +57,7 @@
 import org.elasticsearch.action.admin.indices.mapping.put.PutMappingAction;
 import org.elasticsearch.action.bulk.BulkAction;
 import org.elasticsearch.action.bulk.BulkItemRequest;
+import org.elasticsearch.action.bulk.BulkRequest;
 import org.elasticsearch.action.bulk.BulkShardRequest;
 import org.elasticsearch.action.delete.DeleteAction;
 import org.elasticsearch.action.get.MultiGetAction;
@@ -220,6 +221,22 @@ public PrivilegesEvaluatorResponse evaluate(final User user, String action0, fin
             log.debug("mapped roles: {}",mappedRoles.toString());
         }
 
+        if (request instanceof BulkRequest && (Strings.isNullOrEmpty(user.getRequestedTenant()))) {
+            // Shortcut for bulk actions. The details are checked on the lower level of the BulkShardRequests (Action indices:data/write/bulk[s]).
+            // This shortcut is only possible if the default tenant is selected, as we might need to rewrite the request for non-default tenants.
+            // No further access check for the default tenant is necessary, as access will be also checked on the TransportShardBulkAction level.
+
+            if (!securityRoles.impliesClusterPermissionPermission(action0)) {
+                presponse.missingPrivileges.add(action0);
+                presponse.allowed = false;
+                log.info("No cluster-level perm match for {} [Action [{}]] [RolesChecked {}]. No permissions for {}", user, action0,
+                        securityRoles.getRoleNames(), presponse.missingPrivileges);
+            } else {
+                presponse.allowed = true;
+            }
+            return presponse;
+        }
+
         final Resolved requestedResolved = irr.resolveRequest(request);
 
         if (log.isDebugEnabled()) {

src/test/java/com/amazon/opendistroforelasticsearch/security/HttpIntegrationTests.java

@@ -33,6 +33,7 @@
 import java.io.File;
 import java.nio.charset.StandardCharsets;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import org.apache.commons.io.FileUtils;
 import org.apache.http.HttpStatus;
 import org.apache.http.NoHttpResponseException;
@@ -60,6 +61,9 @@
 import com.amazon.opendistroforelasticsearch.security.test.helper.rest.RestHelper;
 import com.amazon.opendistroforelasticsearch.security.test.helper.rest.RestHelper.HttpResponse;
 
+
+import static com.amazon.opendistroforelasticsearch.security.DefaultObjectMapper.readTree;
+
 public class HttpIntegrationTests extends SingleClusterTest {
 
     @Test
@@ -543,9 +547,32 @@ public void testBulk() throws Exception {
         System.out.println(res.getBody());
         Assert.assertEquals(HttpStatus.SC_OK, res.getStatusCode());  
         Assert.assertTrue(res.getBody().contains("\"errors\":false"));
-        Assert.assertTrue(res.getBody().contains("\"status\":201"));  
+        Assert.assertTrue(res.getBody().contains("\"status\":201"));
     }
-
+
+    @Test
+    public void testBulkWithOneIndexFailure() throws Exception {
+        final Settings settings = Settings.builder()
+                .put(ConfigConstants.OPENDISTRO_SECURITY_ROLES_MAPPING_RESOLUTION, "BOTH")
+                .build();
+        setup(Settings.EMPTY, new DynamicSecurityConfig().setSecurityRoles("roles_bulk.yml"), settings);
+        final RestHelper rh = nonSslRestHelper();
+
+        String bulkBody =
+                "{ \"index\" : { \"_index\" : \"test\", \"_id\" : \"1\" } }"+System.lineSeparator()+
+                        "{ \"a\" : \"b\" }" +System.lineSeparator()+
+                        "{ \"index\" : { \"_index\" : \"myindex\", \"_id\" : \"1\" } }"+System.lineSeparator()+
+                        "{ \"a\" : \"b\" }"+System.lineSeparator();
+
+        HttpResponse res = rh.executePostRequest("_bulk?refresh=true", bulkBody, encodeBasicHeader("bulk_test_user", "nagilum"));
+        System.out.println(res.getBody());
+        JsonNode jsonNode = readTree(res.getBody());
+        Assert.assertEquals(HttpStatus.SC_OK, res.getStatusCode());
+        Assert.assertTrue(jsonNode.get("errors").booleanValue());
+        Assert.assertEquals(201, jsonNode.get("items").get(0).get("index").get("status").intValue());
+        Assert.assertEquals(403, jsonNode.get("items").get(1).get("index").get("status").intValue());
+    }
+
     @Test
     public void test557() throws Exception {
         final Settings settings = Settings.builder()

src/test/java/com/amazon/opendistroforelasticsearch/security/IntegrationTests.java

@@ -30,6 +30,7 @@
 
 package com.amazon.opendistroforelasticsearch.security;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import io.netty.handler.ssl.OpenSsl;
 
 import java.lang.Thread.UncaughtExceptionHandler;
@@ -68,6 +69,8 @@
 import com.amazon.opendistroforelasticsearch.security.test.helper.rest.RestHelper;
 import com.amazon.opendistroforelasticsearch.security.test.helper.rest.RestHelper.HttpResponse;
 
+import static com.amazon.opendistroforelasticsearch.security.DefaultObjectMapper.readTree;
+
 public class IntegrationTests extends SingleClusterTest {
 
     @Test
@@ -868,5 +871,20 @@ public void testSecurityIndexSecurity() throws Exception {
 //                encodeBasicHeader("nagilum", "nagilum"));
 //        Assert.assertTrue(res.getStatusCode() >= 400);
 
+        String bulkBody = "{ \"index\" : { \"_index\" : \".opendistro_security\", \"_id\" : \"1\" } }\n"
+                + "{ \"field1\" : \"value1\" }\n"
+                + "{ \"index\" : { \"_index\" : \".opendistro_security\", \"_id\" : \"2\" } }\n"
+                + "{ \"field2\" : \"value2\" }\n"
+                + "{ \"index\" : { \"_index\" : \"myindex\", \"_id\" : \"2\" } }\n"
+                + "{ \"field2\" : \"value2\" }\n"
+                + "{ \"delete\" : { \"_index\" : \".opendistro_security\", \"_id\" : \"config\" } }\n";
+        res = rh.executePostRequest("_bulk?refresh=true&pretty", bulkBody, encodeBasicHeader("nagilum", "nagilum"));
+        JsonNode jsonNode = readTree(res.getBody());
+        System.out.println(res.getBody());
+        Assert.assertEquals(HttpStatus.SC_OK, res.getStatusCode());
+        Assert.assertEquals(403, jsonNode.get("items").get(0).get("index").get("status").intValue());
+        Assert.assertEquals(403, jsonNode.get("items").get(1).get("index").get("status").intValue());
+        Assert.assertEquals(201, jsonNode.get("items").get(2).get("index").get("status").intValue());
+        Assert.assertEquals(403, jsonNode.get("items").get(3).get("delete").get("status").intValue());
     }
 }

src/test/resources/internal_users.yml

@@ -331,3 +331,6 @@ foo_index:
 foo_all:
   hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
   #password is: nagilum
+bulk_test_user:
+  hash: $2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m
+  #password is: nagilum

src/test/resources/roles_bulk.yml

@@ -20,3 +20,15 @@ bulk:
     - "indices:admin/create"
     - "indices:data/write/bulk[s]"
   tenant_permissions: []
+
+bulk_test_user_role:
+  reserved: false
+  hidden: false
+  cluster_permissions:
+    - "*"
+  index_permissions:
+    - index_patterns:
+        - "test"
+      allowed_actions:
+        - "*"
+  tenant_permissions: []

src/test/resources/roles_mapping.yml

@@ -384,4 +384,7 @@ xyz_sr_hidden:
   users:
     - "test_user"
   and_backend_roles: []
-  description: "Migrated from v6"
+  description: "Migrated from v6"
+bulk_test_user_role:
+  users:
+    - "bulk_test_user"