From 94a75b18f52e94e0dfeab868ed0f43b16078d7ca Mon Sep 17 00:00:00 2001 From: javanna Date: Tue, 10 Jul 2018 13:46:26 +0200 Subject: [PATCH 1/5] Remove aliases resolution limitations when security is enabled Resolving wildcards in aliases expression is challenging as we may end up with no aliases to replace the original expression with, but if we replace with an empty array that means _all which is quite the opposite. Now that we support and serialize the original requested aliases, whenever aliases are replaced we will be able to know what was initially requested. MetaData#findAliases can then be updated to not return anything in case it gets empty aliases, but the original aliases were not empty. That means that empty aliases are interpreted as _all only if they were originally requested that way. Relates to #31516 --- .../elasticsearch/action/AliasesRequest.java | 2 + .../indices/alias/IndicesAliasesRequest.java | 12 +++- .../alias/TransportIndicesAliasesAction.java | 4 +- .../alias/get/TransportGetAliasesAction.java | 2 +- .../indices/get/TransportGetIndexAction.java | 5 +- .../cluster/metadata/MetaData.java | 37 +++++++++++- .../cluster/metadata/MetaDataTests.java | 59 ++++++++++++++++++ x-pack/docs/en/security/limitations.asciidoc | 2 - .../authz/IndicesAndAliasesResolver.java | 20 ++----- .../security/authz/IndexAliasesTests.java | 60 +++++++++++-------- .../authz/IndicesAndAliasesResolverTests.java | 23 +++---- .../build.gradle | 5 +- 12 files changed, 164 insertions(+), 67 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/AliasesRequest.java b/server/src/main/java/org/elasticsearch/action/AliasesRequest.java index bf7ceb28d5023..997e368868b40 100644 --- a/server/src/main/java/org/elasticsearch/action/AliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/AliasesRequest.java @@ -32,6 +32,8 @@ public interface AliasesRequest extends IndicesRequest.Replaceable { */ String[] aliases(); + String[] getOriginalAliases(); + /** * Replaces current aliases with the provided aliases. * diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java index 9249550871c12..34dcce7a86729 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java @@ -214,6 +214,7 @@ private static ObjectParser parser(String name, Supplier aliases = new HashSet<>(); for (AliasActions action : actions) { String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(state, request.indicesOptions(), action.indices()); - Collections.addAll(aliases, action.aliases()); + Collections.addAll(aliases, action.getOriginalAliases()); for (String index : concreteIndices) { switch (action.actionType()) { case ADD: @@ -142,7 +142,7 @@ private static String[] concreteAliases(AliasActions action, MetaData metaData, if (action.expandAliasesWildcards()) { //for DELETE we expand the aliases String[] indexAsArray = {concreteIndex}; - ImmutableOpenMap> aliasMetaData = metaData.findAliases(action.aliases(), indexAsArray); + ImmutableOpenMap> aliasMetaData = metaData.findAliases(action, indexAsArray); List finalAliases = new ArrayList<>(); for (ObjectCursor> curAliases : aliasMetaData.values()) { for (AliasMetaData aliasMeta: curAliases.value) { diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java index 1bacd652ee734..2b71e85a53761 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java @@ -63,7 +63,7 @@ protected GetAliasesResponse newResponse() { @Override protected void masterOperation(GetAliasesRequest request, ClusterState state, ActionListener listener) { String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(state, request); - ImmutableOpenMap> aliases = state.metaData().findAliases(request.aliases(), concreteIndices); + ImmutableOpenMap> aliases = state.metaData().findAliases(request, concreteIndices); listener.onResponse(new GetAliasesResponse(postProcess(request, concreteIndices, aliases))); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java index 060c345454abb..584ad0bc55ac9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/get/TransportGetIndexAction.java @@ -32,15 +32,14 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsFilter; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.common.settings.IndexScopedSettings; import java.io.IOException; import java.util.List; @@ -110,7 +109,7 @@ protected void doMasterOperation(final GetIndexRequest request, String[] concret break; case ALIASES: if (!doneAliases) { - aliasesResult = state.metaData().findAliases(Strings.EMPTY_ARRAY, concreteIndices); + aliasesResult = state.metaData().findAllAliases(concreteIndices); doneAliases = true; } break; diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java index 4ed2adc9a1c9f..b772027432e15 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -24,6 +24,7 @@ import com.carrotsearch.hppc.cursors.ObjectObjectCursor; import org.apache.logging.log4j.Logger; import org.apache.lucene.util.CollectionUtil; +import org.elasticsearch.action.AliasesRequest; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterState.FeatureAware; import org.elasticsearch.cluster.Diff; @@ -247,22 +248,54 @@ public SortedMap getAliasAndIndexLookup() { return aliasAndIndexLookup; } + /** + * Finds the specific index aliases that point to the specified concrete indices or match partially with the indices via wildcards. + * + * @param concreteIndices The concrete indexes the index aliases must point to order to be returned. + * @return a map of index to a list of alias metadata, the list corresponding to a concrete index will be empty if no aliases are + * present for that index + */ + public ImmutableOpenMap> findAllAliases(String[] concreteIndices) { + return findAliases(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, concreteIndices); + } + /** * Finds the specific index aliases that match with the specified aliases directly or partially via wildcards and * that point to the specified concrete indices or match partially with the indices via wildcards. * - * @param aliases The names of the index aliases to find + * @param aliasesRequest The request to find aliases for + * @param concreteIndices The concrete indexes the index aliases must point to order to be returned. + * @return a map of index to a list of alias metadata, the list corresponding to a concrete index will be empty if no aliases are + * present for that index + */ + public ImmutableOpenMap> findAliases(final AliasesRequest aliasesRequest, String[] concreteIndices) { + return findAliases(aliasesRequest.getOriginalAliases(), aliasesRequest.aliases(), concreteIndices); + } + + /** + * Finds the specific index aliases that match with the specified aliases directly or partially via wildcards and + * that point to the specified concrete indices or match partially with the indices via wildcards. + * + * @param aliases The aliases to look for + * @param originalAliases The original aliases that the user originally requested * @param concreteIndices The concrete indexes the index aliases must point to order to be returned. * @return a map of index to a list of alias metadata, the list corresponding to a concrete index will be empty if no aliases are * present for that index */ - public ImmutableOpenMap> findAliases(final String[] aliases, String[] concreteIndices) { + private ImmutableOpenMap> findAliases(String[] originalAliases, String[] aliases, + String[] concreteIndices) { assert aliases != null; + assert originalAliases != null; assert concreteIndices != null; if (concreteIndices.length == 0) { return ImmutableOpenMap.of(); } + //if aliases were provided but they got replaced with empty aliases, return empty map + if (originalAliases.length > 0 && aliases.length == 0) { + return ImmutableOpenMap.of(); + } + boolean matchAllAliases = matchAllAliases(aliases); ImmutableOpenMap.Builder> mapBuilder = ImmutableOpenMap.builder(); for (String index : concreteIndices) { diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java index 32dd4324ff835..0bc81c9813504 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java @@ -20,6 +20,7 @@ package org.elasticsearch.cluster.metadata; import org.elasticsearch.Version; +import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; @@ -41,6 +42,7 @@ import java.io.IOException; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -50,6 +52,63 @@ public class MetaDataTests extends ESTestCase { + public void testFindAliases() { + MetaData metaData = MetaData.builder().put(IndexMetaData.builder("index") + .settings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)) + .numberOfShards(1) + .numberOfReplicas(0) + .putAlias(AliasMetaData.builder("alias1").build()) + .putAlias(AliasMetaData.builder("alias2").build())).build(); + + { + ImmutableOpenMap> aliases = metaData.findAliases(new GetAliasesRequest(), Strings.EMPTY_ARRAY); + assertThat(aliases.size(), equalTo(0)); + } + { + ImmutableOpenMap> aliases = metaData.findAliases(new GetAliasesRequest(), new String[]{"index"}); + assertThat(aliases.size(), equalTo(1)); + List aliasMetaDataList = aliases.get("index"); + assertThat(aliasMetaDataList.size(), equalTo(2)); + assertThat(aliasMetaDataList.get(0).alias(), equalTo("alias1")); + assertThat(aliasMetaDataList.get(1).alias(), equalTo("alias2")); + } + { + GetAliasesRequest getAliasesRequest = new GetAliasesRequest("alias1"); + getAliasesRequest.replaceAliases(Strings.EMPTY_ARRAY); + ImmutableOpenMap> aliases = metaData.findAliases(getAliasesRequest, new String[]{"index"}); + assertThat(aliases.size(), equalTo(0)); + } + { + ImmutableOpenMap> aliases = + metaData.findAliases(new GetAliasesRequest("alias*"), new String[]{"index"}); + assertThat(aliases.size(), equalTo(1)); + List aliasMetaDataList = aliases.get("index"); + assertThat(aliasMetaDataList.size(), equalTo(2)); + assertThat(aliasMetaDataList.get(0).alias(), equalTo("alias1")); + assertThat(aliasMetaDataList.get(1).alias(), equalTo("alias2")); + } + { + ImmutableOpenMap> aliases = + metaData.findAliases(new GetAliasesRequest("alias1"), new String[]{"index"}); + assertThat(aliases.size(), equalTo(1)); + List aliasMetaDataList = aliases.get("index"); + assertThat(aliasMetaDataList.size(), equalTo(1)); + assertThat(aliasMetaDataList.get(0).alias(), equalTo("alias1")); + } + { + ImmutableOpenMap> aliases = metaData.findAllAliases(new String[]{"index"}); + assertThat(aliases.size(), equalTo(1)); + List aliasMetaDataList = aliases.get("index"); + assertThat(aliasMetaDataList.size(), equalTo(2)); + assertThat(aliasMetaDataList.get(0).alias(), equalTo("alias1")); + assertThat(aliasMetaDataList.get(1).alias(), equalTo("alias2")); + } + { + ImmutableOpenMap> aliases = metaData.findAllAliases(Strings.EMPTY_ARRAY); + assertThat(aliases.size(), equalTo(0)); + } + } + public void testIndexAndAliasWithSameName() { IndexMetaData.Builder builder = IndexMetaData.builder("index") .settings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)) diff --git a/x-pack/docs/en/security/limitations.asciidoc b/x-pack/docs/en/security/limitations.asciidoc index c127ee3d7967c..fb8b826d5dd58 100644 --- a/x-pack/docs/en/security/limitations.asciidoc +++ b/x-pack/docs/en/security/limitations.asciidoc @@ -19,8 +19,6 @@ with {security} enabled. Elasticsearch clusters with {security} enabled apply the `/_all` wildcard, and all other wildcards, to the indices that the current user has privileges for, not the set of all indices on the cluster. -While creating or retrieving aliases by providing wildcard expressions for alias names, if there are no existing authorized aliases -that match the wildcard expression provided an IndexNotFoundException is returned. [float] === Multi Document APIs diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java index 77170f7a1cbfb..2247cbe02a812 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolver.java @@ -20,7 +20,6 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.ClusterSettings; @@ -200,6 +199,8 @@ ResolvedIndices resolveIndicesAndAliases(IndicesRequest indicesRequest, MetaData if (aliasesRequest.expandAliasesWildcards()) { List aliases = replaceWildcardsWithAuthorizedAliases(aliasesRequest.aliases(), loadAuthorizedAliases(authorizedIndices.get(), metaData)); + //it may be that we replace aliases with an empty array, in case there are no authorized aliases for the action. + //MetaData#findAliases will return nothing when some alias was originally requested, which was replaced with empty. aliasesRequest.replaceAliases(aliases.toArray(new String[aliases.size()])); } if (indicesReplacedWithNoIndices) { @@ -240,8 +241,7 @@ static String getPutMappingIndexOrAlias(PutMappingRequest request, AuthorizedInd } else { // the user is not authorized to put mappings for this index, but could have been // authorized for a write using an alias that triggered a dynamic mapping update - ImmutableOpenMap> foundAliases = - metaData.findAliases(Strings.EMPTY_ARRAY, new String[] { concreteIndexName }); + ImmutableOpenMap> foundAliases = metaData.findAllAliases(new String[] { concreteIndexName }); List aliasMetaData = foundAliases.get(concreteIndexName); if (aliasMetaData != null) { Optional foundAlias = aliasMetaData.stream() @@ -279,14 +279,12 @@ private List replaceWildcardsWithAuthorizedAliases(String[] aliases, Lis List finalAliases = new ArrayList<>(); //IndicesAliasesRequest doesn't support empty aliases (validation fails) but GetAliasesRequest does (in which case empty means _all) - boolean matchAllAliases = aliases.length == 0; - if (matchAllAliases) { + if (aliases.length == 0) { finalAliases.addAll(authorizedAliases); } for (String aliasPattern : aliases) { if (aliasPattern.equals(MetaData.ALL)) { - matchAllAliases = true; finalAliases.addAll(authorizedAliases); } else if (Regex.isSimpleMatchPattern(aliasPattern)) { for (String authorizedAlias : authorizedAliases) { @@ -298,16 +296,6 @@ private List replaceWildcardsWithAuthorizedAliases(String[] aliases, Lis finalAliases.add(aliasPattern); } } - - //Throw exception if the wildcards expansion to authorized aliases resulted in no indices. - //We always need to replace wildcards for security reasons, to make sure that the operation is executed on the aliases that we - //authorized it to execute on. Empty set gets converted to _all by es core though, and unlike with indices, here we don't have - //a special expression to replace empty set with, which gives us the guarantee that nothing will be returned. - //This is because existing aliases can contain all kinds of special characters, they are only validated since 5.1. - if (finalAliases.isEmpty()) { - String indexName = matchAllAliases ? MetaData.ALL : Arrays.toString(aliases); - throw new IndexNotFoundException(indexName); - } return finalAliases; } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndexAliasesTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndexAliasesTests.java index c6cb8bb662c7c..036f1667e14b8 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndexAliasesTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndexAliasesTests.java @@ -15,6 +15,7 @@ import org.elasticsearch.client.Client; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.rest.action.admin.indices.AliasesNotFoundException; import org.elasticsearch.test.SecurityIntegTestCase; import org.junit.Before; @@ -235,15 +236,19 @@ public void testDeleteAliasesCreateAndAliasesPermission() { //ok: user has manage_aliases on test_* assertAcked(client().filterWithHeader(headers).admin().indices().prepareAliases().removeAlias("test_1", "test_alias_*").get()); - //fails: all aliases have been deleted, no existing aliases match test_alias_* - IndexNotFoundException indexNotFoundException = expectThrows(IndexNotFoundException.class, + { + //fails: all aliases have been deleted, no existing aliases match test_alias_* + AliasesNotFoundException exception = expectThrows(AliasesNotFoundException.class, client().filterWithHeader(headers).admin().indices().prepareAliases().removeAlias("test_1", "test_alias_*")::get); - assertThat(indexNotFoundException.toString(), containsString("[test_alias_*]")); + assertThat(exception.getMessage(), equalTo("aliases [test_alias_*] missing")); + } - //fails: all aliases have been deleted, no existing aliases match _all - indexNotFoundException = expectThrows(IndexNotFoundException.class, + { + //fails: all aliases have been deleted, no existing aliases match _all + AliasesNotFoundException exception = expectThrows(AliasesNotFoundException.class, client().filterWithHeader(headers).admin().indices().prepareAliases().removeAlias("test_1", "_all")::get); - assertThat(indexNotFoundException.toString(), containsString("[_all]")); + assertThat(exception.getMessage(), equalTo("aliases [_all] missing")); + } //fails: user doesn't have manage_aliases on alias_1 assertThrowsAuthorizationException(client().filterWithHeader(headers).admin().indices().prepareAliases() @@ -383,24 +388,27 @@ public void testGetAliasesCreateAndAliasesPermission2() { getAliasesResponse = client.admin().indices().prepareGetAliases().setAliases("test_alias").get(); assertEquals(0, getAliasesResponse.getAliases().size()); - //fails: no existing aliases to replace wildcards - IndexNotFoundException indexNotFoundException = expectThrows(IndexNotFoundException.class, - client.admin().indices().prepareGetAliases().setIndices("test_1").setAliases("test_*")::get); - assertThat(indexNotFoundException.toString(), containsString("[test_*]")); - - //fails: no existing aliases to replace _all - indexNotFoundException = expectThrows(IndexNotFoundException.class, - client.admin().indices().prepareGetAliases().setIndices("test_1").setAliases("_all")::get); - assertThat(indexNotFoundException.toString(), containsString("[_all]")); - - //fails: no existing aliases to replace empty aliases - indexNotFoundException = expectThrows(IndexNotFoundException.class, - client.admin().indices().prepareGetAliases().setIndices("test_1")::get); - assertThat(indexNotFoundException.toString(), containsString("[_all]")); - - //fails: no existing aliases to replace empty aliases - indexNotFoundException = expectThrows(IndexNotFoundException.class, client.admin().indices().prepareGetAliases()::get); - assertThat(indexNotFoundException.toString(), containsString("[_all]")); + { + //fails: no existing aliases to replace wildcards + assertThrowsAuthorizationException( + client.admin().indices().prepareGetAliases().setIndices("test_1").setAliases("test_*")::get, + GetAliasesAction.NAME, "create_test_aliases_alias"); + } + { + //fails: no existing aliases to replace _all + assertThrowsAuthorizationException(client.admin().indices().prepareGetAliases().setIndices("test_1").setAliases("_all")::get, + GetAliasesAction.NAME, "create_test_aliases_alias"); + } + { + //fails: no existing aliases to replace empty aliases + assertThrowsAuthorizationException(client.admin().indices().prepareGetAliases().setIndices("test_1")::get, + GetAliasesAction.NAME, "create_test_aliases_alias"); + } + { + //fails: no existing aliases to replace empty aliases + GetAliasesResponse response = client.admin().indices().prepareGetAliases().get(); + assertThat(response.getAliases().size(), equalTo(0)); + } } public void testCreateIndexThenAliasesCreateAndAliasesPermission3() { @@ -447,9 +455,9 @@ public void testDeleteAliasesCreateAndAliasesPermission3() { assertAcked(client.admin().indices().prepareAliases().removeAlias("test_*", "_all")); //fails: all aliases have been deleted, _all can't be resolved to any existing authorized aliases - IndexNotFoundException indexNotFoundException = expectThrows(IndexNotFoundException.class, + AliasesNotFoundException exception = expectThrows(AliasesNotFoundException.class, client.admin().indices().prepareAliases().removeAlias("test_1", "_all")::get); - assertThat(indexNotFoundException.toString(), containsString("[_all]")); + assertThat(exception.getMessage(), equalTo("aliases [_all] missing")); } public void testGetAliasesCreateAndAliasesPermission3() { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java index d7c974bdc6e2a..bd5acdec818ec 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java @@ -80,6 +80,7 @@ import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_INDEX_NAME; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.arrayContainingInAnyOrder; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.emptyIterable; import static org.hamcrest.Matchers.equalTo; @@ -781,10 +782,11 @@ public void testResolveAllAliasesWildcardsIndicesAliasesRequestDeleteActions() { public void testResolveAliasesWildcardsIndicesAliasesRequestDeleteActionsNoAuthorizedIndices() { IndicesAliasesRequest request = new IndicesAliasesRequest(); request.addAliasAction(AliasActions.remove().index("foo*").alias("foo*")); - //no authorized aliases match bar*, hence this action fails and makes the whole request fail + //no authorized aliases match bar*, hence aliases are replaced with empty string for that action request.addAliasAction(AliasActions.remove().index("*bar").alias("bar*")); - expectThrows(IndexNotFoundException.class, () -> resolveIndices( - request, buildAuthorizedIndices(user, IndicesAliasesAction.NAME))); + resolveIndices(request, buildAuthorizedIndices(user, IndicesAliasesAction.NAME)); + assertThat(request.getAliasActions().get(0).aliases().length, equalTo(1)); + assertThat(request.getAliasActions().get(1).aliases().length, equalTo(0)); } public void testResolveWildcardsIndicesAliasesRequestAddAndDeleteActions() { @@ -1086,12 +1088,11 @@ public void testResolveAliasesWildcardsGetAliasesRequest() { public void testResolveAliasesWildcardsGetAliasesRequestNoAuthorizedIndices() { GetAliasesRequest request = new GetAliasesRequest(); - //no authorized aliases match bar*, hence the request fails + //no authorized aliases match bar*, hence aliases are replaced with empty array request.aliases("bar*"); request.indices("*bar"); - IndexNotFoundException e = expectThrows(IndexNotFoundException.class, - () -> resolveIndices(request, buildAuthorizedIndices(user, GetAliasesAction.NAME))); - assertEquals("no such index", e.getMessage()); + resolveIndices(request, buildAuthorizedIndices(user, GetAliasesAction.NAME)); + assertThat(request.aliases().length, equalTo(0)); } public void testResolveAliasesAllGetAliasesRequestNoAuthorizedIndices() { @@ -1100,10 +1101,10 @@ public void testResolveAliasesAllGetAliasesRequestNoAuthorizedIndices() { request.aliases("_all"); } request.indices("non_existing"); - //current user is not authorized for any index, foo* resolves to no indices, the request fails - IndexNotFoundException e = expectThrows(IndexNotFoundException.class, - () -> resolveIndices(request, buildAuthorizedIndices(userNoIndices, GetAliasesAction.NAME))); - assertEquals("no such index", e.getMessage()); + //current user is not authorized for any index, foo* resolves to no indices, aliases are replaced with empty array + ResolvedIndices resolvedIndices = resolveIndices(request, buildAuthorizedIndices(userNoIndices, GetAliasesAction.NAME)); + assertThat(resolvedIndices.getLocal(), contains("non_existing")); + assertThat(request.aliases().length, equalTo(0)); } /** diff --git a/x-pack/qa/core-rest-tests-with-security/build.gradle b/x-pack/qa/core-rest-tests-with-security/build.gradle index 7f2706a773aa9..35b08de01252a 100644 --- a/x-pack/qa/core-rest-tests-with-security/build.gradle +++ b/x-pack/qa/core-rest-tests-with-security/build.gradle @@ -12,10 +12,9 @@ integTest { integTestRunner { systemProperty 'tests.rest.blacklist', - ['cat.aliases/10_basic/Empty cluster', + [ 'index/10_with_id/Index with ID', - 'indices.get_alias/10_basic/Get alias against closed indices', - 'indices.get_alias/20_empty/Check empty aliases when getting all aliases via /_alias', + 'indices.get_alias/10_basic/Get alias against closed indices' ].join(',') systemProperty 'tests.rest.cluster.username', System.getProperty('tests.rest.cluster.username', 'test_user') From d8553aba4d6d94eed890d3c50c0e5e9c02541bf4 Mon Sep 17 00:00:00 2001 From: javanna Date: Tue, 10 Jul 2018 13:52:16 +0200 Subject: [PATCH 2/5] adjust version conditionals this change is breaking, will not be backported to 6.x --- .../action/admin/indices/alias/IndicesAliasesRequest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java index 34dcce7a86729..22e8554ed6aa6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/alias/IndicesAliasesRequest.java @@ -238,6 +238,8 @@ public AliasActions(StreamInput in) throws IOException { indexRouting = in.readOptionalString(); if (in.getVersion().onOrAfter(Version.V_6_4_0)) { writeIndex = in.readOptionalBoolean(); + } + if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { originalAliases = in.readStringArray(); } } @@ -253,6 +255,8 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalString(indexRouting); if (out.getVersion().onOrAfter(Version.V_6_4_0)) { out.writeOptionalBoolean(writeIndex); + } + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { out.writeStringArray(originalAliases); } } From 5e927d6019e465070ee18038ef37995b3139c9da Mon Sep 17 00:00:00 2001 From: javanna Date: Mon, 16 Jul 2018 14:13:47 +0200 Subject: [PATCH 3/5] add javadocsi --- .../src/main/java/org/elasticsearch/action/AliasesRequest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/action/AliasesRequest.java b/server/src/main/java/org/elasticsearch/action/AliasesRequest.java index 997e368868b40..419287f28eb22 100644 --- a/server/src/main/java/org/elasticsearch/action/AliasesRequest.java +++ b/server/src/main/java/org/elasticsearch/action/AliasesRequest.java @@ -32,6 +32,9 @@ public interface AliasesRequest extends IndicesRequest.Replaceable { */ String[] aliases(); + /** + * Returns the aliases as they were originally requested, before any potential name resolution + */ String[] getOriginalAliases(); /** From a6b5c786a39e1433ca28304f773787358476d3fa Mon Sep 17 00:00:00 2001 From: javanna Date: Tue, 17 Jul 2018 13:51:01 +0200 Subject: [PATCH 4/5] revert some of the changes --- docs/reference/migration/migrate_7_0/api.asciidoc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/reference/migration/migrate_7_0/api.asciidoc b/docs/reference/migration/migrate_7_0/api.asciidoc index f7b6f9b2e00a9..efef39c83d0a9 100644 --- a/docs/reference/migration/migrate_7_0/api.asciidoc +++ b/docs/reference/migration/migrate_7_0/api.asciidoc @@ -79,3 +79,11 @@ the only behavior in 8.0.0, this parameter is deprecated in 7.0.0 for removal in ==== The deprecated stored script contexts have now been removed When putting stored scripts, support for storing them with the deprecated `template` context or without a context is now removed. Scripts must be stored using the `script` context as mentioned in the documentation. + +==== Get Aliases API limitations when xpack security is enabled removed + +When xpack security is enabled, the Get Aliases API behaviour and response +code returned are now aligned to those of vanilla Elasticsearch. Previously +a 404 - NOT FOUND (IndexNotFoundException) could be returned in case the +current user was not authorized for any alias. An empty response with status +200 - OK is now returned instead. From 85f6c11c56976b4ebc1dd514f94232ba17dc42a2 Mon Sep 17 00:00:00 2001 From: javanna Date: Tue, 17 Jul 2018 22:14:51 +0200 Subject: [PATCH 5/5] update docs --- docs/reference/migration/migrate_7_0/api.asciidoc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/api.asciidoc b/docs/reference/migration/migrate_7_0/api.asciidoc index efef39c83d0a9..689b941ef6b6b 100644 --- a/docs/reference/migration/migrate_7_0/api.asciidoc +++ b/docs/reference/migration/migrate_7_0/api.asciidoc @@ -80,10 +80,10 @@ the only behavior in 8.0.0, this parameter is deprecated in 7.0.0 for removal in When putting stored scripts, support for storing them with the deprecated `template` context or without a context is now removed. Scripts must be stored using the `script` context as mentioned in the documentation. -==== Get Aliases API limitations when xpack security is enabled removed +==== Get Aliases API limitations when {security} is enabled removed -When xpack security is enabled, the Get Aliases API behaviour and response -code returned are now aligned to those of vanilla Elasticsearch. Previously -a 404 - NOT FOUND (IndexNotFoundException) could be returned in case the -current user was not authorized for any alias. An empty response with status -200 - OK is now returned instead. +The behavior and response codes of the get aliases API no longer vary +depending on whether {security} is enabled. Previously a +404 - NOT FOUND (IndexNotFoundException) could be returned in case the +current user was not authorized for any alias. An empty response with +status 200 - OK is now returned instead at all times.