Merge pull request elastic/elasticsearch#3091 from javanna/fix/reindex_repleaceable

Security: add tests for delete and update by query

Original commit: elastic/x-pack-elasticsearch@6f6786a26e

elasticsearch/qa/reindex-tests-with-security/build.gradle

@@ -2,6 +2,8 @@ apply plugin: 'elasticsearch.rest-test'
 
 dependencies {
   testCompile project(path: ':x-plugins:elasticsearch:x-pack', configuration: 'runtime')
+  testCompile project(path: ':x-plugins:elasticsearch:x-pack', configuration: 'testArtifacts')
+  testCompile project(path: ':modules:reindex')
 }
 
 integTest {

elasticsearch/qa/reindex-tests-with-security/src/test/java/org/elasticsearch/xpack/security/ReindexWithSecurityIT.java

@@ -0,0 +1,128 @@
+/*
+ * 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.xpack.security;
+
+import org.elasticsearch.action.admin.indices.alias.Alias;
+import org.elasticsearch.common.network.NetworkModule;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.IndexNotFoundException;
+import org.elasticsearch.index.reindex.BulkIndexByScrollResponse;
+import org.elasticsearch.index.reindex.DeleteByQueryAction;
+import org.elasticsearch.index.reindex.ReindexAction;
+import org.elasticsearch.index.reindex.ReindexPlugin;
+import org.elasticsearch.index.reindex.UpdateByQueryAction;
+import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.test.SecurityIntegTestCase;
+import org.junit.Before;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+
+public class ReindexWithSecurityIT extends SecurityIntegTestCase {
+
+    private boolean useSecurity3;
+
+    @Override
+    @Before
+    public void setUp() throws Exception {
+        super.setUp();
+        useSecurity3 = randomBoolean();
+    }
+
+    @Override
+    protected Collection<Class<? extends Plugin>> nodePlugins() {
+        Collection<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins());
+        plugins.add(ReindexPlugin.class);
+        return Collections.unmodifiableCollection(plugins);
+    }
+
+    @Override
+    protected Collection<Class<? extends Plugin>> transportClientPlugins() {
+        Collection<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins());
+        plugins.add(ReindexPlugin.class);
+        return Collections.unmodifiableCollection(plugins);
+    }
+
+    @Override
+    protected Settings externalClusterClientSettings() {
+        Settings.Builder builder = Settings.builder().put(super.externalClusterClientSettings());
+        if (useSecurity3) {
+            builder.put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME3);
+        } else {
+            builder.put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME4);
+        }
+        builder.put(Security.USER_SETTING.getKey(), "test_admin:changeme");
+        return builder.build();
+    }
+
+    public void testDeleteByQuery() {
+        createIndices("test1", "test2", "test3");
+
+        BulkIndexByScrollResponse response = DeleteByQueryAction.INSTANCE.newRequestBuilder(client()).source("test1", "test2").get();
+        assertNotNull(response);
+
+        response = DeleteByQueryAction.INSTANCE.newRequestBuilder(client()).source("test*").get();
+        assertNotNull(response);
+
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class,
+                () -> DeleteByQueryAction.INSTANCE.newRequestBuilder(client()).source("test1", "index1").get());
+        assertEquals("no such index", e.getMessage());
+    }
+
+    public void testUpdateByQuery() {
+        createIndices("test1", "test2", "test3");
+
+        BulkIndexByScrollResponse response = UpdateByQueryAction.INSTANCE.newRequestBuilder(client()).source("test1", "test2").get();
+        assertNotNull(response);
+
+        response = UpdateByQueryAction.INSTANCE.newRequestBuilder(client()).source("test*").get();
+        assertNotNull(response);
+
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class,
+                () -> UpdateByQueryAction.INSTANCE.newRequestBuilder(client()).source("test1", "index1").get());
+        assertEquals("no such index", e.getMessage());
+    }
+
+    public void testReindex() {
+        createIndices("test1", "test2", "test3", "dest");
+
+        BulkIndexByScrollResponse response = ReindexAction.INSTANCE.newRequestBuilder(client()).source("test1", "test2")
+                .destination("dest").get();
+        assertNotNull(response);
+
+        response = ReindexAction.INSTANCE.newRequestBuilder(client()).source("test*").destination("dest").get();
+        assertNotNull(response);
+
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class,
+                () -> ReindexAction.INSTANCE.newRequestBuilder(client()).source("test1", "index1").destination("dest").get());
+        assertEquals("no such index", e.getMessage());
+    }
+
+    private void createIndices(String... indices) {
+        if (randomBoolean()) {
+            //no aliases
+            createIndex(indices);
+        } else {
+            if (randomBoolean()) {
+                //one alias per index with suffix "-alias"
+                for (String index : indices) {
+                    client().admin().indices().prepareCreate(index).setSettings(indexSettings()).addAlias(new Alias(index + "-alias"));
+                }
+            } else {
+                //same alias pointing to all indices
+                for (String index : indices) {
+                    client().admin().indices().prepareCreate(index).setSettings(indexSettings()).addAlias(new Alias("alias"));
+                }
+            }
+        }
+
+        for (String index : indices) {
+            client().prepareIndex(index, "type").setSource("field", "value").get();
+        }
+        refresh();
+    }
+}

elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authz/indicesresolver/DefaultIndicesAndAliasesResolver.java

@@ -49,13 +49,10 @@ public class DefaultIndicesAndAliasesResolver implements IndicesAndAliasesResolv
 
     @Override
     public Set<String> resolve(User user, String action, TransportRequest request, MetaData metaData) {
-
         boolean isIndicesRequest = request instanceof CompositeIndicesRequest || request instanceof IndicesRequest;
-        assert isIndicesRequest : "Request [" + request + "] is not an Indices request, but should be.";
-
         // if for some reason we are missing an action... just for safety we'll reject
-        if (!isIndicesRequest) {
+        if (isIndicesRequest == false) {
-            return Collections.emptySet();
+            throw new IllegalStateException("Request [" + request + "] is not an Indices request, but should be.");
         }
 
         if (request instanceof CompositeIndicesRequest) {
@@ -74,7 +71,7 @@ public class DefaultIndicesAndAliasesResolver implements IndicesAndAliasesResolv
         final Set<String> indices;
         if (indicesRequest instanceof PutMappingRequest
                 && ((PutMappingRequest) indicesRequest).getConcreteIndex() != null) {
-            /**
+            /*
              * This is a special case since PutMappingRequests from dynamic mapping updates have a concrete index
              * if this index is set and it's in the list of authorized indices we are good and don't need to put
              * the list of indices in there, if we do so it will result in an invalid request and the update will fail.

elasticsearch/x-pack/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java

@@ -153,7 +153,7 @@ public abstract class SecurityIntegTestCase extends ESIntegTestCase {
             // TODO: disable this assertion for now, due to random runs with mock plugins. perhaps run without mock plugins?
 //            assertThat(nodeInfo.getPlugins().getInfos(), hasSize(2));
             Collection<String> pluginNames =
-                    nodeInfo.getPlugins().getPluginInfos().stream().map(p -> p.getName()).collect(Collectors.toList());
+                    nodeInfo.getPlugins().getPluginInfos().stream().map(p -> p.getClassname()).collect(Collectors.toList());
             assertThat("plugin [" + xpackPluginClass().getName() + "] not found in [" + pluginNames + "]", pluginNames,
                 hasItem(xpackPluginClass().getName()));
         }

elasticsearch/x-pack/security/src/test/java/org/elasticsearch/xpack/security/authz/indicesresolver/IndicesAndAliasesResolverIntegrationTests.java

@@ -24,9 +24,9 @@ import static org.elasticsearch.test.SecurityTestsUtils.assertAuthorizationExcep
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItems;
-import static org.hamcrest.Matchers.is;
 
 public class IndicesAndAliasesResolverIntegrationTests extends SecurityIntegTestCase {
+
     @Override
     protected String configRoles() {
         return SecuritySettingsSource.DEFAULT_ROLE + ":\n" +
@@ -57,50 +57,30 @@ public class IndicesAndAliasesResolverIntegrationTests extends SecurityIntegTest
     public void testSearchNonAuthorizedWildcard() {
         //wildcard doesn't match any authorized index
         createIndices("test1", "test2", "index1", "index2");
-        try {
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> client().prepareSearch("index*").get());
-            client().prepareSearch("index*").get();
+        assertEquals("no such index", e.getMessage());
-            fail("Expected IndexNotFoundException");
-        } catch (IndexNotFoundException e) {
-            assertThat(e.getMessage(), is("no such index"));
-        }
     }
 
     public void testEmptyClusterSearchForAll() {
-        try {
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> client().prepareSearch().get());
-            client().prepareSearch().get();
+        assertEquals("no such index", e.getMessage());
-            fail("Expected IndexNotFoundException");
-        } catch (IndexNotFoundException e) {
-            assertThat(e.getMessage(), is("no such index"));
-        }
     }
 
     public void testEmptyClusterSearchForWildcard() {
-        try {
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> client().prepareSearch("*").get());
-            client().prepareSearch("*").get();
+        assertEquals("no such index", e.getMessage());
-            fail("Expected IndexNotFoundException");
-        } catch (IndexNotFoundException e) {
-            assertThat(e.getMessage(), is("no such index"));
-        }
     }
 
     public void testEmptyAuthorizedIndicesSearchForAll() {
         createIndices("index1", "index2");
-        try {
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> client().prepareSearch().get());
-            client().prepareSearch().get();
+        assertEquals("no such index", e.getMessage());
-            fail("Expected IndexNotFoundException");
-        } catch (IndexNotFoundException e) {
-            assertThat(e.getMessage(), is("no such index"));
-        }
     }
 
     public void testEmptyAuthorizedIndicesSearchForWildcard() {
         createIndices("index1", "index2");
-        try {
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class, () -> client().prepareSearch("*").get());
-            client().prepareSearch("*").get();
+        assertEquals("no such index", e.getMessage());
-            fail("Expected IndexNotFoundException");
-        } catch (IndexNotFoundException e) {
-            assertThat(e.getMessage(), is("no such index"));
-        }
     }
 
     public void testExplicitNonAuthorizedIndex() {
@@ -187,14 +167,10 @@ public class IndicesAndAliasesResolverIntegrationTests extends SecurityIntegTest
     public void testMultiSearchWildcard() {
         //test4 is missing but authorized, only that specific item fails
         createIndices("test1", "test2", "test3", "index1");
-        try {
+        IndexNotFoundException e = expectThrows(IndexNotFoundException.class,
-            client().prepareMultiSearch()
+                () -> client().prepareMultiSearch().add(Requests.searchRequest())
-                    .add(Requests.searchRequest())
+                        .add(Requests.searchRequest("index*")).get());
-                    .add(Requests.searchRequest("index*")).get();
+        assertEquals("no such index", e.getMessage());
-            fail("Expected IndexNotFoundException");
-        } catch (IndexNotFoundException e) {
-            assertThat(e.getMessage(), is("no such index"));
-        }
     }
 
     private static void assertReturnedIndices(SearchResponse searchResponse, String... indices) {
@@ -207,12 +183,8 @@ public class IndicesAndAliasesResolverIntegrationTests extends SecurityIntegTest
     }
 
     private static void assertThrowsAuthorizationException(ActionRequestBuilder actionRequestBuilder) {
-        try {
+        ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, actionRequestBuilder::get);
-            actionRequestBuilder.get();
+        assertAuthorizationException(e, containsString("is unauthorized for user ["));
-            fail("search should fail due to attempt to access non authorized indices");
-        } catch(ElasticsearchSecurityException e) {
-            assertAuthorizationException(e, containsString("is unauthorized for user ["));
-        }
     }
 
     private void createIndices(String... indices) {
@@ -233,7 +205,6 @@ public class IndicesAndAliasesResolverIntegrationTests extends SecurityIntegTest
             }
         }
 
-        ensureGreen();
         for (String index : indices) {
             client().prepareIndex(index, "type").setSource("field", "value").get();
         }