Enforces cluster permission checks for all cluster actions

Enforcing means that cluster actions will not be evaluated (as a fallback) by Index permissions. This enables us to move what typically would be considered indices actions and put them under the cluster privileges (a good example for this are all the template management APIs... we want to enforce cluster admin privileges over them).

Original commit: elastic/x-pack-elasticsearch@ee870954f2

src/main/java/org/elasticsearch/shield/authz/Permission.java

@@ -46,6 +46,9 @@ public interface Permission {
 
     static class Global implements Permission {
 
+        final static Predicate<String> clusterActionMatcher = Privilege.Cluster.ALL.predicate();
+        final static Predicate<String> indicesActionMatcher = Privilege.Index.ALL.predicate();
+
         private final Cluster cluster;
         private final Indices indices;
 
@@ -67,11 +70,11 @@ public interface Permission {
         }
 
         public boolean check(User user, String action, TransportRequest request, MetaData metaData) {
-            if (cluster != null && cluster.check(user, action, request, metaData)) {
-                return true;
+            if (clusterActionMatcher.apply(action)) {
+                return cluster != null && cluster.check(user, action, request, metaData);
             }
-            if (indices != null && indices.check(user, action, request, metaData)) {
-                return true;
+            if (indicesActionMatcher.apply(action)) {
+                return indices != null && indices.check(user, action, request, metaData);
             }
             return false;
         }

src/test/java/org/elasticsearch/shield/MultipleIndicesPermissionsTests.java

@@ -6,6 +6,7 @@
 package org.elasticsearch.shield;
 
 import org.elasticsearch.action.index.IndexResponse;
+import org.elasticsearch.action.search.MultiSearchResponse;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.shield.authz.AuthorizationException;
@@ -94,5 +95,20 @@ public class MultipleIndicesPermissionsTests extends ShieldIntegrationTest {
         } catch (AuthorizationException ae) {
             // expected
         }
+
+        MultiSearchResponse msearchResponse = client.prepareMultiSearch()
+                .add(client.prepareSearch("test"))
+                .add(client.prepareSearch("test1"))
+                .get();
+        MultiSearchResponse.Item[] items = msearchResponse.getResponses();
+        assertThat(items.length, is(2));
+        assertThat(items[0].isFailure(), is(false));
+        searchResponse = items[0].getResponse();
+        assertNoFailures(searchResponse);
+        assertHitCount(searchResponse, 1);
+        assertThat(items[1].isFailure(), is(false));
+        searchResponse = items[1].getResponse();
+        assertNoFailures(searchResponse);
+        assertHitCount(searchResponse, 1);
     }
 }

src/test/java/org/elasticsearch/shield/PermissionPrecedenceTests.java

@@ -0,0 +1,126 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.shield;
+
+import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse;
+import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateResponse;
+import org.elasticsearch.client.transport.TransportClient;
+import org.elasticsearch.cluster.ClusterService;
+import org.elasticsearch.cluster.metadata.IndexTemplateMetaData;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.transport.TransportAddress;
+import org.elasticsearch.shield.authc.support.SecuredString;
+import org.elasticsearch.shield.authc.support.SecuredStringTests;
+import org.elasticsearch.shield.authz.AuthorizationException;
+import org.elasticsearch.shield.test.ShieldIntegrationTest;
+import org.junit.Test;
+
+import java.util.List;
+
+import static org.elasticsearch.shield.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.hamcrest.Matchers.hasSize;
+
+/**
+ * This test makes sure that if an action is a cluster action (according to our
+ * internal categorization in shield, then we apply the cluster priv checks and don't
+ * fallback on the indices privs at all. In particular, this is useful when we want to treat
+ * actions that are normally categorized as index actions as cluster actions - for example,
+ * index template actions.
+ */
+public class PermissionPrecedenceTests extends ShieldIntegrationTest {
+
+    static final String ROLES =
+            "admin:\n" +
+            "  cluster: all\n" +
+            "  indices:\n" +
+            "    '*': all\n" +
+            "\n" +
+            "user:\n" +
+            "  indices:\n" +
+            "    'test_*': all\n";
+
+    static final String USERS =
+            "admin:{plain}test123\n" +
+            "user:{plain}test123\n";
+
+    static final String USERS_ROLES =
+            "admin:admin\n" +
+            "user:user\n";
+
+    @Override
+    protected String configRole() {
+        return ROLES;
+    }
+
+    @Override
+    protected String configUsers() {
+        return USERS;
+    }
+
+    @Override
+    protected String configUsersRoles() {
+        return USERS_ROLES;
+    }
+
+    @Override
+    protected String getClientUsername() {
+        return "admin";
+    }
+
+    @Override
+    protected SecuredString getClientPassword() {
+        return SecuredStringTests.build("test123");
+    }
+
+    @Test
+    public void testDifferetCombinationsOfIndices() throws Exception {
+
+        ClusterService clusterService = internalCluster().getInstance(ClusterService.class);
+        TransportAddress address = clusterService.localNode().address();
+
+        TransportClient client = new TransportClient(ImmutableSettings.builder()
+                .put("cluster.name", internalCluster().getClusterName())
+                .put("node.mode", "network")
+                .put(getSSLSettingsForStore("/org/elasticsearch/shield/transport/ssl/certs/simple/testclient.jks", "testclient")))
+                .addTransportAddress(address);
+
+        // first lets try with "admin"... all should work
+
+        PutIndexTemplateResponse putResponse = client.admin().indices().preparePutTemplate("template1")
+                .setTemplate("test_*")
+                .putHeader("Authorization", basicAuthHeaderValue("admin", SecuredStringTests.build("test123")))
+                .get();
+        assertAcked(putResponse);
+
+        GetIndexTemplatesResponse getResponse = client.admin().indices().prepareGetTemplates("template1")
+                .putHeader("Authorization", basicAuthHeaderValue("admin", SecuredStringTests.build("test123")))
+                .get();
+        List<IndexTemplateMetaData> templates = getResponse.getIndexTemplates();
+        assertThat(templates, hasSize(1));
+
+        // now lets try with "user"
+
+        try {
+            client.admin().indices().preparePutTemplate("template1")
+                    .setTemplate("test_*")
+                    .putHeader("Authorization", basicAuthHeaderValue("user", SecuredStringTests.build("test123")))
+                    .get();
+            fail("expected an authorization exception as template APIs should require cluster ALL permission");
+        } catch (AuthorizationException ae) {
+            // expected;
+        }
+
+        try {
+            client.admin().indices().prepareGetTemplates("template1")
+                    .putHeader("Authorization", basicAuthHeaderValue("user", SecuredStringTests.build("test123")))
+                    .get();
+            fail("expected an authorization exception as template APIs should require cluster ALL permission");
+        } catch (AuthorizationException ae) {
+            // expected
+        }
+    }
+}

src/test/java/org/elasticsearch/shield/test/ShieldIntegrationTest.java

@@ -68,8 +68,8 @@ public abstract class ShieldIntegrationTest extends ElasticsearchIntegrationTest
                 .put("discovery.type", "zen")
                 .put("node.mode", "network")
                 .put("plugin.types", ShieldPlugin.class.getName())
-                .put("shield.authc.esusers.files.users", writeFile(folder, "users", CONFIG_STANDARD_USER))
-                .put("shield.authc.esusers.files.users_roles", writeFile(folder, "users_roles", CONFIG_STANDARD_USER_ROLES))
+                .put("shield.authc.esusers.files.users", writeFile(folder, "users", configUsers()))
+                .put("shield.authc.esusers.files.users_roles", writeFile(folder, "users_roles", configUsersRoles()))
                 .put("shield.authz.store.files.roles", writeFile(folder, "roles.yml", configRole()))
                 .put("shield.transport.n2n.ip_filter.file", writeFile(folder, "ip_filter.yml", CONFIG_IPFILTER_ALLOW_ALL))
                 .put(getSSLSettingsForStore("/org/elasticsearch/shield/transport/ssl/certs/simple/testnode.jks", "testnode"))
@@ -87,6 +87,14 @@ public abstract class ShieldIntegrationTest extends ElasticsearchIntegrationTest
         return CONFIG_ROLE_ALLOW_ALL;
     }
 
+    protected String configUsers() {
+        return CONFIG_STANDARD_USER;
+    }
+
+    protected String configUsersRoles() {
+        return CONFIG_STANDARD_USER_ROLES;
+    }
+
     @Override
     protected Settings transportClientSettings() {
         return ImmutableSettings.builder()