From f29cc62829a2c92f16af09c1e586c17ca4c7408e Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Tue, 20 Jan 2015 14:18:30 +0100 Subject: [PATCH] Authorization: split analyze api into cluster level action and original indices action The analyze api allows to specify an index, to retrieve analyzers or token filters from a specific index. That is why it is categorized as indices level action. That said the index is optional and when not specified the action is executed at the cluster level. We have to remap the name of the action in that case, to make sure that it requires a different privilege under cluster: cluster:admin/analyze instead of indices:admin/analyze . Closes elastic/elasticsearch#566 Closes elastic/elasticsearch#565 Closes elastic/elasticsearch#592 Original commit: elastic/x-pack-elasticsearch@9073b30d08f1f0d5ed7346878557c17b8189862d --- .../shield/action/ShieldActionFilter.java | 2 - .../shield/action/ShieldActionMapper.java | 26 ++++-- .../authz/InternalAuthorizationService.java | 10 +- .../elasticsearch/shield/authz/Privilege.java | 2 +- .../action/ShieldActionMapperTests.java | 92 +++++++++++++++++++ .../shield/authz/AnalyzeTests.java | 90 ++++++++++++++++++ .../transport/KnownActionsTests.java | 8 +- 7 files changed, 210 insertions(+), 20 deletions(-) create mode 100644 src/test/java/org/elasticsearch/shield/action/ShieldActionMapperTests.java create mode 100644 src/test/java/org/elasticsearch/shield/authz/AnalyzeTests.java diff --git a/src/main/java/org/elasticsearch/shield/action/ShieldActionFilter.java b/src/main/java/org/elasticsearch/shield/action/ShieldActionFilter.java index 0bdb609fdad..027a35f581e 100644 --- a/src/main/java/org/elasticsearch/shield/action/ShieldActionFilter.java +++ b/src/main/java/org/elasticsearch/shield/action/ShieldActionFilter.java @@ -38,8 +38,6 @@ import java.util.List; */ public class ShieldActionFilter extends AbstractComponent implements ActionFilter { - public static final String CLUSTER_PERMISSION_SCROLL_CLEAR_ALL_NAME = "cluster:admin/indices/scroll/clear_all"; - private static final Predicate READ_ACTION_MATCHER = Privilege.Index.READ.predicate(); private final AuthenticationService authcService; diff --git a/src/main/java/org/elasticsearch/shield/action/ShieldActionMapper.java b/src/main/java/org/elasticsearch/shield/action/ShieldActionMapper.java index 19382937a2e..a5843033f16 100644 --- a/src/main/java/org/elasticsearch/shield/action/ShieldActionMapper.java +++ b/src/main/java/org/elasticsearch/shield/action/ShieldActionMapper.java @@ -5,6 +5,8 @@ */ package org.elasticsearch.shield.action; +import org.elasticsearch.action.admin.indices.analyze.AnalyzeAction; +import org.elasticsearch.action.admin.indices.analyze.AnalyzeRequest; import org.elasticsearch.action.search.ClearScrollAction; import org.elasticsearch.action.search.ClearScrollRequest; import org.elasticsearch.transport.TransportRequest; @@ -17,16 +19,28 @@ import org.elasticsearch.transport.TransportRequest; */ public class ShieldActionMapper { + static final String CLUSTER_PERMISSION_SCROLL_CLEAR_ALL_NAME = "cluster:admin/indices/scroll/clear_all"; + static final String CLUSTER_PERMISSION_ANALYZE = "cluster:admin/analyze"; + /** * Returns the shield specific action name given the incoming action name and request */ public String action(String action, TransportRequest request) { - if (action.equals(ClearScrollAction.NAME)) { - assert request instanceof ClearScrollRequest; - boolean isClearAllScrollRequest = ((ClearScrollRequest) request).scrollIds().contains("_all"); - if (isClearAllScrollRequest) { - return ShieldActionFilter.CLUSTER_PERMISSION_SCROLL_CLEAR_ALL_NAME; - } + switch (action) { + case ClearScrollAction.NAME: + assert request instanceof ClearScrollRequest; + boolean isClearAllScrollRequest = ((ClearScrollRequest) request).scrollIds().contains("_all"); + if (isClearAllScrollRequest) { + return CLUSTER_PERMISSION_SCROLL_CLEAR_ALL_NAME; + } + break; + case AnalyzeAction.NAME: + assert request instanceof AnalyzeRequest; + String[] indices = ((AnalyzeRequest) request).indices(); + if (indices == null || indices.length == 0) { + return CLUSTER_PERMISSION_ANALYZE; + } + break; } return action; } diff --git a/src/main/java/org/elasticsearch/shield/authz/InternalAuthorizationService.java b/src/main/java/org/elasticsearch/shield/authz/InternalAuthorizationService.java index 7ee34f87196..7d03437514a 100644 --- a/src/main/java/org/elasticsearch/shield/authz/InternalAuthorizationService.java +++ b/src/main/java/org/elasticsearch/shield/authz/InternalAuthorizationService.java @@ -7,12 +7,11 @@ package org.elasticsearch.shield.authz; import org.elasticsearch.action.CompositeIndicesRequest; import org.elasticsearch.action.IndicesRequest; -import org.elasticsearch.action.admin.indices.analyze.AnalyzeRequest; -import org.elasticsearch.action.search.ClearScrollAction; -import org.elasticsearch.action.search.SearchScrollAction; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexRequestHelper; +import org.elasticsearch.action.search.ClearScrollAction; +import org.elasticsearch.action.search.SearchScrollAction; import org.elasticsearch.cluster.ClusterService; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.base.Predicate; @@ -144,10 +143,7 @@ public class InternalAuthorizationService extends AbstractComponent implements A } Set indexNames = resolveIndices(user, action, request); - //Note: some APIs (e.g. analyze) are categorized under indices, but their indices are optional. - //In that case the resolved indices set is empty (for now). See https://github.com/elasticsearch/elasticsearch-shield/issues/566 - assert !indexNames.isEmpty() || request instanceof AnalyzeRequest - : "no indices request other than the analyze api has optional indices thus the resolved indices must not be empty"; + assert !indexNames.isEmpty() : "every indices request needs to have its indices set thus the resolved indices must not be empty"; if (!authorizeIndices(action, indexNames, permission.indices())) { throw denial(user, action, request); diff --git a/src/main/java/org/elasticsearch/shield/authz/Privilege.java b/src/main/java/org/elasticsearch/shield/authz/Privilege.java index 9ac53ef7cd6..7e85130199e 100644 --- a/src/main/java/org/elasticsearch/shield/authz/Privilege.java +++ b/src/main/java/org/elasticsearch/shield/authz/Privilege.java @@ -216,7 +216,7 @@ public abstract class Privilege

> { static Cluster[] values() { return values; - }; + } private static final LoadingCache cache = CacheBuilder.newBuilder().build( new CacheLoader() { diff --git a/src/test/java/org/elasticsearch/shield/action/ShieldActionMapperTests.java b/src/test/java/org/elasticsearch/shield/action/ShieldActionMapperTests.java new file mode 100644 index 00000000000..e5ec57c3a91 --- /dev/null +++ b/src/test/java/org/elasticsearch/shield/action/ShieldActionMapperTests.java @@ -0,0 +1,92 @@ +/* + * 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.action; + +import org.elasticsearch.action.admin.indices.analyze.AnalyzeAction; +import org.elasticsearch.action.admin.indices.analyze.AnalyzeRequest; +import org.elasticsearch.action.search.ClearScrollAction; +import org.elasticsearch.action.search.ClearScrollRequest; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.elasticsearch.transport.KnownActionsTests; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.CoreMatchers.equalTo; + +public class ShieldActionMapperTests extends ElasticsearchTestCase { + + @Test + public void testThatAllOrdinaryActionsRemainTheSame() { + List actions = new ArrayList<>(); + actions.addAll(KnownActionsTests.loadKnownActions()); + actions.addAll(KnownActionsTests.loadKnownHandlers()); + + ShieldActionMapper shieldActionMapper = new ShieldActionMapper(); + int iterations = randomIntBetween(10, 100); + for (int i = 0; i < iterations; i++) { + String randomAction; + do { + if (randomBoolean()) { + randomAction = randomFrom(actions); + } else { + randomAction = randomAsciiOfLength(randomIntBetween(1, 30)); + } + } while (randomAction.equals(ClearScrollAction.NAME) || + randomAction.equals(AnalyzeAction.NAME)); + + assertThat(shieldActionMapper.action(randomAction, null), equalTo(randomAction)); + } + } + + @Test + public void testClearScroll() { + ShieldActionMapper shieldActionMapper = new ShieldActionMapper(); + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + int scrollIds = randomIntBetween(1, 10); + for (int i = 0; i < scrollIds; i++) { + clearScrollRequest.addScrollId(randomAsciiOfLength(randomIntBetween(1, 30))); + } + assertThat(shieldActionMapper.action(ClearScrollAction.NAME, clearScrollRequest), equalTo(ClearScrollAction.NAME)); + } + + @Test + public void testClearScrollAll() { + ShieldActionMapper shieldActionMapper = new ShieldActionMapper(); + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + int scrollIds = randomIntBetween(0, 10); + for (int i = 0; i < scrollIds; i++) { + clearScrollRequest.addScrollId(randomAsciiOfLength(randomIntBetween(1, 30))); + } + clearScrollRequest.addScrollId("_all"); + //make sure that wherever the _all is among the scroll ids the action name gets translated + Collections.shuffle(clearScrollRequest.getScrollIds(), getRandom()); + + assertThat(shieldActionMapper.action(ClearScrollAction.NAME, clearScrollRequest), equalTo(ShieldActionMapper.CLUSTER_PERMISSION_SCROLL_CLEAR_ALL_NAME)); + } + + @Test + public void testIndicesAnalyze() { + ShieldActionMapper shieldActionMapper = new ShieldActionMapper(); + AnalyzeRequest analyzeRequest; + if (randomBoolean()) { + analyzeRequest = new AnalyzeRequest(randomAsciiOfLength(randomIntBetween(1, 30)), "text"); + } else { + analyzeRequest = new AnalyzeRequest("text"); + analyzeRequest.index(randomAsciiOfLength(randomIntBetween(1, 30))); + } + assertThat(shieldActionMapper.action(AnalyzeAction.NAME, analyzeRequest), equalTo(AnalyzeAction.NAME)); + } + + @Test + public void testClusterAnalyze() { + ShieldActionMapper shieldActionMapper = new ShieldActionMapper(); + AnalyzeRequest analyzeRequest = new AnalyzeRequest("text"); + assertThat(shieldActionMapper.action(AnalyzeAction.NAME, analyzeRequest), equalTo(ShieldActionMapper.CLUSTER_PERMISSION_ANALYZE)); + } +} diff --git a/src/test/java/org/elasticsearch/shield/authz/AnalyzeTests.java b/src/test/java/org/elasticsearch/shield/authz/AnalyzeTests.java new file mode 100644 index 00000000000..6d9da710e0e --- /dev/null +++ b/src/test/java/org/elasticsearch/shield/authz/AnalyzeTests.java @@ -0,0 +1,90 @@ +/* + * 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.authz; + +import org.elasticsearch.shield.authc.support.SecuredString; +import org.elasticsearch.test.ShieldIntegrationTest; +import org.junit.Test; + +import static org.elasticsearch.shield.authc.support.UsernamePasswordToken.BASIC_AUTH_HEADER; +import static org.elasticsearch.shield.authc.support.UsernamePasswordToken.basicAuthHeaderValue; +import static org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope; +import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope; +import static org.hamcrest.CoreMatchers.containsString; + +@ClusterScope(scope = Scope.SUITE) +public class AnalyzeTests extends ShieldIntegrationTest { + + @Override + protected String configUsers() { + return super.configUsers() + + "analyze_indices:{plain}test123\n" + + "analyze_cluster:{plain}test123\n"; + } + + @Override + protected String configUsersRoles() { + return super.configUsersRoles() + + "analyze_indices:analyze_indices\n" + + "analyze_cluster:analyze_cluster\n"; + } + + @Override + protected String configRoles() { + return super.configRoles()+ "\n" + + //role that has analyze indices privileges only + "analyze_indices:\n" + + " indices:\n" + + " 'test_*': indices:admin/analyze\n" + + "analyze_cluster:\n" + + " cluster:\n" + + " - cluster:admin/analyze\n"; + } + + @Test + public void testAnalyzeWithIndices() { + //this test tries to execute different analyze api variants from a user that has analyze privileges only on a specific index namespace + + createIndex("test_1"); + ensureGreen(); + + //ok: user has permissions for analyze on test_* + client().admin().indices().prepareAnalyze("this is my text").setIndex("test_1").setAnalyzer("standard") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("analyze_indices", new SecuredString("test123".toCharArray()))).get(); + + try { + //fails: user doesn't have permissions for analyze on index non_authorized + client().admin().indices().prepareAnalyze("this is my text").setIndex("non_authorized").setAnalyzer("standard") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("analyze_indices", new SecuredString("test123".toCharArray()))).get(); + } catch(AuthorizationException e) { + assertThat(e.getMessage(), containsString("action [indices:admin/analyze] is unauthorized for user [analyze_indices]")); + } + + try { + //fails: user doesn't have permissions for cluster level analyze + client().admin().indices().prepareAnalyze("this is my text").setAnalyzer("standard") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("analyze_indices", new SecuredString("test123".toCharArray()))).get(); + } catch(AuthorizationException e) { + assertThat(e.getMessage(), containsString("action [cluster:admin/analyze] is unauthorized for user [analyze_indices]")); + } + } + + @Test + public void testAnalyzeWithoutIndices() { + //this test tries to execute different analyze api variants from a user that has analyze privileges only at cluster level + + try { + //fails: user doesn't have permissions for analyze on index test_1 + client().admin().indices().prepareAnalyze("this is my text").setIndex("test_1").setAnalyzer("standard") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("analyze_cluster", new SecuredString("test123".toCharArray()))).get(); + } catch(AuthorizationException e) { + assertThat(e.getMessage(), containsString("action [indices:admin/analyze] is unauthorized for user [analyze_cluster]")); + } + + client().admin().indices().prepareAnalyze("this is my text").setAnalyzer("standard") + .putHeader(BASIC_AUTH_HEADER, basicAuthHeaderValue("analyze_cluster", new SecuredString("test123".toCharArray()))).get(); + } +} diff --git a/src/test/java/org/elasticsearch/transport/KnownActionsTests.java b/src/test/java/org/elasticsearch/transport/KnownActionsTests.java index 8d5b2963757..c3d787de555 100644 --- a/src/test/java/org/elasticsearch/transport/KnownActionsTests.java +++ b/src/test/java/org/elasticsearch/transport/KnownActionsTests.java @@ -52,14 +52,14 @@ public class KnownActionsTests extends ShieldIntegrationTest { @Test public void testAllCodeActionsAreKnown() throws Exception { for (String action : codeActions) { - assertThat("elasticsearch core action [" + action + "] is unknown to shield", knownActions, hasItem(action)); + assertThat("classpath action [" + action + "] is unknown to shield", knownActions, hasItem(action)); } } @Test public void testAllKnownActionsAreValid() { for (String knownAction : knownActions) { - assertThat("shield known action [" + knownAction + "] is unknown to core", codeActions, hasItems(knownAction)); + assertThat("shield known action [" + knownAction + "] is not among the classpath actions", codeActions, hasItems(knownAction)); } } @@ -71,7 +71,7 @@ public class KnownActionsTests extends ShieldIntegrationTest { } } - private static ImmutableSet loadKnownActions() { + public static ImmutableSet loadKnownActions() { final ImmutableSet.Builder knownActionsBuilder = ImmutableSet.builder(); try (InputStream input = KnownActionsTests.class.getResourceAsStream("actions")) { Streams.readAllLines(input, new Callback() { @@ -86,7 +86,7 @@ public class KnownActionsTests extends ShieldIntegrationTest { return knownActionsBuilder.build(); } - private static ImmutableSet loadKnownHandlers() { + public static ImmutableSet loadKnownHandlers() { final ImmutableSet.Builder knownHandlersBuilder = ImmutableSet.builder(); try (InputStream input = KnownActionsTests.class.getResourceAsStream("handlers")) { Streams.readAllLines(input, new Callback() {