From 14699d66109a5450da238a6af44a3d9570135d74 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 21 Jan 2015 19:19:15 +0100 Subject: [PATCH] Indices resolution: empty aliases to be treated same as _all in GetAliasesRequest While IndicesAliasesRequest doesn't support empty aliases, thus only explicit _all needs to resolved to all existing authorized aliases, GetAliasesRequest does support empty aliases, thus we have to treat them the same as _all. Closes elastic/elasticsearch#606 Original commit: elastic/x-pack-elasticsearch@3e993ea2bd353ee1affcd3ed72cc3db1973e83cd --- .../DefaultIndicesResolver.java | 18 +++-- .../shield/authz/IndexAliasesTests.java | 70 +++++++++++++++++++ .../DefaultIndicesResolverTests.java | 66 ++++++++++++++++- 3 files changed, 148 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/elasticsearch/shield/authz/indicesresolver/DefaultIndicesResolver.java b/src/main/java/org/elasticsearch/shield/authz/indicesresolver/DefaultIndicesResolver.java index b39aa3db52a..ea88b57769e 100644 --- a/src/main/java/org/elasticsearch/shield/authz/indicesresolver/DefaultIndicesResolver.java +++ b/src/main/java/org/elasticsearch/shield/authz/indicesresolver/DefaultIndicesResolver.java @@ -130,6 +130,7 @@ public class DefaultIndicesResolver implements IndicesResolver authorizedAliases = loadAuthorizedAliases(authorizedIndices, metaData); } + assert aliasActions.aliases().length > 0 : "aliases must not be empty within each single alias remove action"; List aliases = replaceWildcardsWithAuthorizedAliases(aliasActions.aliases(), authorizedAliases); aliasActions.aliases(aliases.toArray(new String[aliases.size()])); } @@ -151,8 +152,16 @@ public class DefaultIndicesResolver implements IndicesResolver private List replaceWildcardsWithAuthorizedAliases(String[] aliases, List authorizedAliases) { List finalAliases = Lists.newArrayList(); + + //IndicesAliasesRequest doesn't support empty aliases (validation fails) but GetAliasesRequest does (in which case empty means _all) + boolean matchAllAliases = aliases.length == 0; + if (matchAllAliases) { + 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) { @@ -170,7 +179,8 @@ public class DefaultIndicesResolver implements IndicesResolver //to make sure that the operation is executed on the aliases that we authorized it to execute on. //If we can't replace because we got an empty set, we can only throw exception. if (finalAliases.isEmpty()) { - throw new IndexMissingException(new Index(Arrays.toString(aliases))); + Index index = matchAllAliases ? new Index(MetaData.ALL) : new Index(Arrays.toString(aliases)); + throw new IndexMissingException(index); } return finalAliases; } @@ -258,10 +268,8 @@ public class DefaultIndicesResolver implements IndicesResolver //If we can't replace because we got an empty set, we can only throw exception. //Downside of this is that a single item exception is going to make fail the composite request that holds it as a whole. if (resolvedIndices == null || resolvedIndices.isEmpty()) { - if (MetaData.isAllIndices(originalIndices)) { - throw new IndexMissingException(new Index(MetaData.ALL)); - } - throw new IndexMissingException(new Index(Arrays.toString(originalIndices))); + Index index = MetaData.isAllIndices(originalIndices) ? new Index(MetaData.ALL) : new Index(Arrays.toString(originalIndices)); + throw new IndexMissingException(index); } return resolvedIndices; } diff --git a/src/test/java/org/elasticsearch/shield/authz/IndexAliasesTests.java b/src/test/java/org/elasticsearch/shield/authz/IndexAliasesTests.java index 31d96eea542..a62fbeae466 100644 --- a/src/test/java/org/elasticsearch/shield/authz/IndexAliasesTests.java +++ b/src/test/java/org/elasticsearch/shield/authz/IndexAliasesTests.java @@ -164,6 +164,14 @@ public class IndexAliasesTests extends ShieldIntegrationTest { assertThat(e.getMessage(), containsString("[_all]")); } + try { + client().admin().indices().prepareGetAliases().setIndices("test_1").setIndicesOptions(IndicesOptions.lenientExpandOpen()) + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_only", new SecuredString("test123".toCharArray()))).get(); + fail("get alias should have failed due to missing manage_aliases privileges"); + } catch(IndexMissingException e) { + assertThat(e.getMessage(), containsString("[_all]")); + } + try { client().admin().indices().prepareGetAliases("test_alias").setIndices("test_*").setIndicesOptions(IndicesOptions.lenientExpandOpen()) .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_only", new SecuredString("test123".toCharArray()))).get(); @@ -171,6 +179,14 @@ public class IndexAliasesTests extends ShieldIntegrationTest { } catch(IndexMissingException e) { assertThat(e.getMessage(), containsString("[test_*]")); } + + try { + client().admin().indices().prepareGetAliases() + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_only", new SecuredString("test123".toCharArray()))).get(); + fail("get alias should have failed due to missing manage_aliases privileges"); + } catch(IndexMissingException e) { + assertThat(e.getMessage(), containsString("[_all]")); + } } @Test @@ -257,6 +273,15 @@ public class IndexAliasesTests extends ShieldIntegrationTest { } catch(AuthorizationException e) { assertThat(e.getMessage(), containsString("action [indices:admin/aliases] is unauthorized for user [create_test_aliases_test]")); } + + try { + //fails: user doesn't have manage_aliases on alias_1 + client().admin().indices().prepareAliases().removeAlias("test_1", new String[]{"_all", "alias_1"}) + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test", new SecuredString("test123".toCharArray()))).get(); + fail("remove alias should have failed due to missing manage_aliases privileges on alias_1"); + } catch(AuthorizationException e) { + assertThat(e.getMessage(), containsString("action [indices:admin/aliases] is unauthorized for user [create_test_aliases_test]")); + } } @Test @@ -286,6 +311,11 @@ public class IndexAliasesTests extends ShieldIntegrationTest { .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test", new SecuredString("test123".toCharArray()))), "test_1", "test_alias"); + //ok: user has manage_aliases on test_*, empty aliases gets resolved to test_alias and empty indices gets resolved to _all indices (thus test_1) + assertAliases(client().admin().indices().prepareGetAliases().setIndices("test_1") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test", new SecuredString("test123".toCharArray()))), + "test_1", "test_alias"); + //ok: user has manage_aliases on test_*, test_* aliases gets resolved to test_alias and empty indices gets resolved to _all indices (thus test_1) assertAliases(client().admin().indices().prepareGetAliases().setAliases("test_*").setIndices("test_1") .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test", new SecuredString("test123".toCharArray()))), @@ -296,6 +326,20 @@ public class IndexAliasesTests extends ShieldIntegrationTest { .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test", new SecuredString("test123".toCharArray()))), "test_1", "test_alias"); + //ok: user has manage_aliases on test_*, empty aliases gets resolved to test_alias and empty indices becomes test_1 + assertAliases(client().admin().indices().prepareGetAliases() + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test", new SecuredString("test123".toCharArray()))), + "test_1", "test_alias"); + + try { + //fails: user has manage_aliases on test_*, although _all aliases and empty indices can be resolved, the explicit non authorized alias (alias_1) causes the request to fail + client().admin().indices().prepareGetAliases().setAliases("_all", "alias_1") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test", new SecuredString("test123".toCharArray()))).get(); + fail("get alias should have failed due to missing manage_aliases privileges on alias_1"); + } catch(AuthorizationException e) { + assertThat(e.getMessage(), containsString("action [indices:admin/aliases/get] is unauthorized for user [create_test_aliases_test]")); + } + try { //fails: user doesn't have manage_aliases on alias_1 client().admin().indices().prepareGetAliases().setAliases("alias_1") @@ -443,6 +487,24 @@ public class IndexAliasesTests extends ShieldIntegrationTest { } catch(IndexMissingException e) { assertThat(e.getMessage(), containsString("[_all]")); } + + try { + //fails: no existing aliases to replace empty aliases + client().admin().indices().prepareGetAliases().setIndices("test_1") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_alias", new SecuredString("test123".toCharArray()))).get(); + fail("get alias should have failed due to missing manage_aliases privileges on test_1"); + } catch(IndexMissingException e) { + assertThat(e.getMessage(), containsString("[_all]")); + } + + try { + //fails: no existing aliases to replace empty aliases + client().admin().indices().prepareGetAliases() + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_alias", new SecuredString("test123".toCharArray()))).get(); + fail("get alias should have failed due to missing manage_aliases privileges on test_1"); + } catch(IndexMissingException e) { + assertThat(e.getMessage(), containsString("[_all]")); + } } @Test @@ -534,6 +596,14 @@ public class IndexAliasesTests extends ShieldIntegrationTest { .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test_alias", new SecuredString("test123".toCharArray()))), "test_1", "alias_1", "test_alias"); + assertAliases(client().admin().indices().prepareGetAliases().setIndices("test_1") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test_alias", new SecuredString("test123".toCharArray()))), + "test_1", "alias_1", "test_alias"); + + assertAliases(client().admin().indices().prepareGetAliases() + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test_alias", new SecuredString("test123".toCharArray()))), + "test_1", "alias_1", "test_alias"); + assertAliases(client().admin().indices().prepareGetAliases().setAliases("alias_*").setIndices("test_*") .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("create_test_aliases_test_alias", new SecuredString("test123".toCharArray()))), "test_1", "alias_1"); diff --git a/src/test/java/org/elasticsearch/shield/authz/indicesresolver/DefaultIndicesResolverTests.java b/src/test/java/org/elasticsearch/shield/authz/indicesresolver/DefaultIndicesResolverTests.java index 9128ca274d2..2b5f74067ef 100644 --- a/src/test/java/org/elasticsearch/shield/authz/indicesresolver/DefaultIndicesResolverTests.java +++ b/src/test/java/org/elasticsearch/shield/authz/indicesresolver/DefaultIndicesResolverTests.java @@ -409,6 +409,25 @@ public class DefaultIndicesResolverTests extends ElasticsearchTestCase { assertThat(request.getAliasActions().get(1).aliases(), arrayContaining("foofoobar")); } + @Test + public void testResolveAllAliasesWildcardsIndicesAliasesRequestDeleteActions() { + IndicesAliasesRequest request = new IndicesAliasesRequest(); + request.addAliasAction(AliasAction.newRemoveAliasAction("*", "_all")); + request.addAliasAction(new IndicesAliasesRequest.AliasActions(AliasAction.Type.REMOVE, "_all", new String[]{"_all", "explicit"})); + Set indices = defaultIndicesResolver.resolve(user, IndicesAliasesAction.NAME, request, metaData); + //union of all resolved indices and aliases gets returned, based on what user is authorized for + //note that the index side will end up containing matching aliases too, which is fine, as es core would do + //the same and resolve those aliases to their corresponding concrete indices (which we let core do) + String[] expectedIndices = new String[]{"bar", "foofoobar", "foofoo", "explicit"}; + assertThat(indices.size(), equalTo(expectedIndices.length)); + assertThat(indices, hasItems(expectedIndices)); + //alias foofoobar on both sides, that's fine, es core would do the same, same as above + assertThat(request.getAliasActions().get(0).indices(), arrayContaining("bar", "foofoobar", "foofoo")); + assertThat(request.getAliasActions().get(0).aliases(), arrayContaining("foofoobar")); + assertThat(request.getAliasActions().get(0).indices(), arrayContaining("bar", "foofoobar", "foofoo")); + assertThat(request.getAliasActions().get(1).aliases(), arrayContaining("foofoobar", "explicit")); + } + @Test(expected = IndexMissingException.class) public void testResolveAliasesWildcardsIndicesAliasesRequestDeleteActionsNoAuthorizedIndices() { IndicesAliasesRequest request = new IndicesAliasesRequest(); @@ -546,7 +565,9 @@ public class DefaultIndicesResolverTests extends ElasticsearchTestCase { @Test public void testResolveAllAliasesGetAliasesRequest() { GetAliasesRequest request = new GetAliasesRequest(); - request.aliases("_all"); + if (randomBoolean()) { + request.aliases("_all"); + } if (randomBoolean()) { request.indices("_all"); } @@ -560,6 +581,38 @@ public class DefaultIndicesResolverTests extends ElasticsearchTestCase { assertThat(request.aliases(), arrayContaining("foofoobar")); } + @Test + public void testResolveAllAndExplicitAliasesGetAliasesRequest() { + GetAliasesRequest request = new GetAliasesRequest(new String[]{"_all", "explicit"}); + if (randomBoolean()) { + request.indices("_all"); + } + Set indices = defaultIndicesResolver.resolve(user, GetAliasesAction.NAME, request, metaData); + //the union of all resolved indices and aliases gets returned + String[] expectedIndices = new String[]{"bar", "foofoobar", "foofoo", "explicit"}; + assertThat(indices.size(), equalTo(expectedIndices.length)); + assertThat(indices, hasItems(expectedIndices)); + //_all gets replaced with all indices that user is authorized for + assertThat(request.indices(), arrayContaining("bar", "foofoobar", "foofoo")); + assertThat(request.aliases(), arrayContaining("foofoobar", "explicit")); + } + + @Test + public void testResolveAllAndWildcardsAliasesGetAliasesRequest() { + GetAliasesRequest request = new GetAliasesRequest(new String[]{"_all", "foo*", "non_matching_*"}); + if (randomBoolean()) { + request.indices("_all"); + } + Set indices = defaultIndicesResolver.resolve(user, GetAliasesAction.NAME, request, metaData); + //the union of all resolved indices and aliases gets returned + String[] expectedIndices = new String[]{"bar", "foofoobar", "foofoo"}; + assertThat(indices.size(), equalTo(expectedIndices.length)); + assertThat(indices, hasItems(expectedIndices)); + //_all gets replaced with all indices that user is authorized for + assertThat(request.indices(), arrayContaining(expectedIndices)); + assertThat(request.aliases(), arrayContaining("foofoobar", "foofoobar")); + } + @Test public void testResolveAliasesWildcardsGetAliasesRequest() { GetAliasesRequest request = new GetAliasesRequest(); @@ -586,6 +639,17 @@ public class DefaultIndicesResolverTests extends ElasticsearchTestCase { defaultIndicesResolver.resolve(user, GetAliasesAction.NAME, request, metaData); } + @Test(expected = IndexMissingException.class) + public void testResolveAliasesAllGetAliasesRequestNoAuthorizedIndices() { + GetAliasesRequest request = new GetAliasesRequest(); + if (randomBoolean()) { + request.aliases("_all"); + } + request.indices("non_existing"); + //current user is not authorized for any index, foo* resolves to no indices, the request fails + defaultIndicesResolver.resolve(userNoIndices, GetAliasesAction.NAME, request, metaData); + } + //msearch is a CompositeIndicesRequest whose items (SearchRequests) implement IndicesRequest.Replaceable, wildcards will get replaced @Test public void testResolveMultiSearchNoWildcards() {