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() {