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 f4eafd05e15..bafbea2e727 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java @@ -249,53 +249,45 @@ public class MetaData implements Iterable, Diffable, To } /** - * Finds the specific index aliases that point to the specified concrete indices or match partially with the indices via wildcards. + * Finds the specific index aliases that point to the requested concrete indices directly + * or that match 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 + * @param concreteIndices The concrete indices that the aliases must point to in order to be returned. + * @return A map of index name to the list of aliases metadata. If a concrete index does not have matching + * aliases then the result will not include the index's key. */ - public ImmutableOpenMap> findAllAliases(String[] concreteIndices) { - return findAliases(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, concreteIndices); + public ImmutableOpenMap> findAllAliases(final String[] concreteIndices) { + return findAliases(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. + * Finds the specific index aliases that match with the specified aliases directly or partially via wildcards, and + * that point to the specified concrete indices (directly or matching indices via wildcards). * * @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 + * @param concreteIndices The concrete indices that the aliases must point to in order to be returned. + * @return A map of index name to the list of aliases metadata. If a concrete index does not have matching + * aliases then the result will not include the index's key. */ - public ImmutableOpenMap> findAliases(final AliasesRequest aliasesRequest, String[] concreteIndices) { - return findAliases(aliasesRequest.getOriginalAliases(), aliasesRequest.aliases(), concreteIndices); + public ImmutableOpenMap> findAliases(final AliasesRequest aliasesRequest, final String[] concreteIndices) { + return findAliases(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. + * Finds the specific index aliases that match with the specified aliases directly or partially via wildcards, and + * that point to the specified concrete indices (directly or matching 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 + * @param aliases The aliases to look for. Might contain include or exclude wildcards. + * @param concreteIndices The concrete indices that the aliases must point to in order to be returned + * @return A map of index name to the list of aliases metadata. If a concrete index does not have matching + * aliases then the result will not include the index's key. */ - private ImmutableOpenMap> findAliases(String[] originalAliases, String[] aliases, - String[] concreteIndices) { + private ImmutableOpenMap> findAliases(final String[] aliases, final 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(); - } - String[] patterns = new String[aliases.length]; boolean[] include = new boolean[aliases.length]; for (int i = 0; i < aliases.length; i++) { @@ -331,7 +323,6 @@ public class MetaData implements Iterable, Diffable, To filteredValues.add(value); } } - if (filteredValues.isEmpty() == false) { // Make the list order deterministic CollectionUtil.timSort(filteredValues, Comparator.comparing(AliasMetaData::alias)); diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java index 1aaec080307..9585381029f 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java @@ -51,6 +51,7 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.RoutingFieldMapper; import org.elasticsearch.index.shard.IndexEventListener; import org.elasticsearch.indices.IndicesService; +import org.elasticsearch.indices.InvalidAliasNameException; import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matchers; import org.mockito.ArgumentCaptor; @@ -82,7 +83,7 @@ import static org.mockito.Mockito.when; public class IndexCreationTaskTests extends ESTestCase { private final IndicesService indicesService = mock(IndicesService.class); - private final AliasValidator aliasValidator = mock(AliasValidator.class); + private final AliasValidator aliasValidator = new AliasValidator(Settings.EMPTY); private final NamedXContentRegistry xContentRegistry = mock(NamedXContentRegistry.class); private final CreateIndexClusterStateUpdateRequest request = mock(CreateIndexClusterStateUpdateRequest.class); private final Logger logger = mock(Logger.class); @@ -149,6 +150,12 @@ public class IndexCreationTaskTests extends ESTestCase { assertThat(getMappingsFromResponse(), Matchers.hasKey("mapping1")); } + public void testInvalidAliasName() throws Exception { + final String[] invalidAliasNames = new String[] { "-alias1", "+alias2", "_alias3", "a#lias", "al:ias", ".", ".." }; + setupRequestAlias(new Alias(randomFrom(invalidAliasNames))); + expectThrows(InvalidAliasNameException.class, this::executeTask); + } + public void testRequestDataHavePriorityOverTemplateData() throws Exception { final CompressedXContent tplMapping = createMapping("text"); final CompressedXContent reqMapping = createMapping("keyword"); 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 a929028a34e..c2950256884 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java @@ -66,6 +66,14 @@ public class MetaDataTests extends ESTestCase { assertThat(aliases.size(), equalTo(0)); } { + final GetAliasesRequest request; + if (randomBoolean()) { + request = new GetAliasesRequest(); + } else { + request = new GetAliasesRequest(randomFrom("alias1", "alias2")); + // replacing with empty aliases behaves as if aliases were unspecified at request building + request.replaceAliases(Strings.EMPTY_ARRAY); + } ImmutableOpenMap> aliases = metaData.findAliases(new GetAliasesRequest(), new String[]{"index"}); assertThat(aliases.size(), equalTo(1)); List aliasMetaDataList = aliases.get("index"); @@ -73,12 +81,6 @@ public class MetaDataTests extends ESTestCase { 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"}); 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 cac9baf1512..26fa8405ccf 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 @@ -46,9 +46,9 @@ import static org.elasticsearch.xpack.core.security.authz.IndicesAndAliasesResol class IndicesAndAliasesResolver { - //`*,-*` what we replace indices with if we need Elasticsearch to return empty responses without throwing exception - private static final String[] NO_INDICES_ARRAY = new String[] { "*", "-*" }; - static final List NO_INDICES_LIST = Arrays.asList(NO_INDICES_ARRAY); + //`*,-*` what we replace indices and aliases with if we need Elasticsearch to return empty responses without throwing exception + static final String[] NO_INDICES_OR_ALIASES_ARRAY = new String[] { "*", "-*" }; + static final List NO_INDICES_OR_ALIASES_LIST = Arrays.asList(NO_INDICES_OR_ALIASES_ARRAY); private final IndexNameExpressionResolver nameExpressionResolver; private final RemoteClusterResolver remoteClusterResolver; @@ -165,7 +165,7 @@ class IndicesAndAliasesResolver { //this is how we tell es core to return an empty response, we can let the request through being sure //that the '-*' wildcard expression will be resolved to no indices. We can't let empty indices through //as that would be resolved to _all by es core. - replaceable.indices(NO_INDICES_ARRAY); + replaceable.indices(NO_INDICES_OR_ALIASES_ARRAY); indicesReplacedWithNoIndices = true; resolvedIndicesBuilder.addLocal(NO_INDEX_PLACEHOLDER); } else { @@ -176,8 +176,6 @@ class IndicesAndAliasesResolver { } } else { if (containsWildcards(indicesRequest)) { - //an alias can still contain '*' in its name as of 5.0. Such aliases cannot be referred to when using - //the security plugin, otherwise the following exception gets thrown throw new IllegalStateException("There are no external requests known to support wildcards that don't support replacing " + "their indices"); } @@ -198,8 +196,6 @@ class IndicesAndAliasesResolver { 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) { @@ -213,6 +209,13 @@ class IndicesAndAliasesResolver { } else { resolvedIndicesBuilder.addLocal(aliasesRequest.aliases()); } + // if no aliases are authorized, then fill in an expression that + // MetaData#findAliases evaluates to the empty alias list. You cannot put + // "nothing" (the empty list) explicitly because this is resolved by es core to + // _all + if (aliasesRequest.aliases().length == 0) { + aliasesRequest.replaceAliases(NO_INDICES_OR_ALIASES_ARRAY); + } } return resolvedIndicesBuilder.build(); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 47cf458e19a..17562b13f0d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -818,7 +818,7 @@ public class AuthorizationServiceTests extends ESTestCase { final SearchRequest searchRequest = new SearchRequest("_all"); authorize(authentication, SearchAction.NAME, searchRequest); assertEquals(2, searchRequest.indices().length); - assertEquals(IndicesAndAliasesResolver.NO_INDICES_LIST, Arrays.asList(searchRequest.indices())); + assertEquals(IndicesAndAliasesResolver.NO_INDICES_OR_ALIASES_LIST, Arrays.asList(searchRequest.indices())); } public void testGrantedNonXPackUserCanExecuteMonitoringOperationsAgainstSecurityIndex() { 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 f9d91527942..4dc0909552c 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 @@ -778,11 +778,11 @@ public class IndicesAndAliasesResolverTests extends ESTestCase { public void testResolveAliasesWildcardsIndicesAliasesRequestDeleteActionsNoAuthorizedIndices() { IndicesAliasesRequest request = new IndicesAliasesRequest(); request.addAliasAction(AliasActions.remove().index("foo*").alias("foo*")); - //no authorized aliases match bar*, hence aliases are replaced with empty string for that action + //no authorized aliases match bar*, hence aliases are replaced with no-aliases-expression for that action request.addAliasAction(AliasActions.remove().index("*bar").alias("bar*")); resolveIndices(request, buildAuthorizedIndices(user, IndicesAliasesAction.NAME)); assertThat(request.getAliasActions().get(0).aliases(), arrayContainingInAnyOrder("foofoobar", "foobarfoo")); - assertThat(request.getAliasActions().get(1).aliases().length, equalTo(0)); + assertThat(request.getAliasActions().get(1).aliases(), arrayContaining(IndicesAndAliasesResolver.NO_INDICES_OR_ALIASES_ARRAY)); } public void testResolveWildcardsIndicesAliasesRequestAddAndDeleteActions() { @@ -1084,11 +1084,11 @@ public class IndicesAndAliasesResolverTests extends ESTestCase { public void testResolveAliasesWildcardsGetAliasesRequestNoAuthorizedIndices() { GetAliasesRequest request = new GetAliasesRequest(); - //no authorized aliases match bar*, hence aliases are replaced with empty array + //no authorized aliases match bar*, hence aliases are replaced with the no-aliases-expression request.aliases("bar*"); request.indices("*bar"); resolveIndices(request, buildAuthorizedIndices(user, GetAliasesAction.NAME)); - assertThat(request.aliases().length, equalTo(0)); + assertThat(request.aliases(), arrayContaining(IndicesAndAliasesResolver.NO_INDICES_OR_ALIASES_ARRAY)); } public void testResolveAliasesExclusionWildcardsGetAliasesRequest() { @@ -1112,10 +1112,11 @@ public class IndicesAndAliasesResolverTests extends ESTestCase { request.aliases("_all"); } request.indices("non_existing"); - //current user is not authorized for any index, foo* resolves to no indices, aliases are replaced with empty array + //current user is not authorized for any index, aliases are replaced with the no-aliases-expression ResolvedIndices resolvedIndices = resolveIndices(request, buildAuthorizedIndices(userNoIndices, GetAliasesAction.NAME)); assertThat(resolvedIndices.getLocal(), contains("non_existing")); - assertThat(request.aliases().length, equalTo(0)); + assertThat(Arrays.asList(request.indices()), contains("non_existing")); + assertThat(request.aliases(), arrayContaining(IndicesAndAliasesResolver.NO_INDICES_OR_ALIASES_ARRAY)); } /** @@ -1372,7 +1373,7 @@ public class IndicesAndAliasesResolverTests extends ESTestCase { final List localIndices = resolvedIndices.getLocal(); assertEquals(1, localIndices.size()); assertEquals(IndicesAndAliasesResolverField.NO_INDEX_PLACEHOLDER, localIndices.iterator().next()); - assertEquals(IndicesAndAliasesResolver.NO_INDICES_LIST, Arrays.asList(request.indices())); + assertEquals(IndicesAndAliasesResolver.NO_INDICES_OR_ALIASES_LIST, Arrays.asList(request.indices())); assertEquals(0, resolvedIndices.getRemote().size()); }