diff --git a/server/src/main/java/org/elasticsearch/common/util/iterable/Iterables.java b/server/src/main/java/org/elasticsearch/common/util/iterable/Iterables.java index 6d1dab4a9d010..e7c002438c576 100644 --- a/server/src/main/java/org/elasticsearch/common/util/iterable/Iterables.java +++ b/server/src/main/java/org/elasticsearch/common/util/iterable/Iterables.java @@ -24,6 +24,7 @@ import java.util.Iterator; import java.util.List; import java.util.Objects; +import java.util.function.Predicate; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -103,6 +104,17 @@ public static T get(Iterable iterable, int position) { } } + public static int indexOf(Iterable iterable, Predicate predicate) { + int i = 0; + for (T element : iterable) { + if (predicate.test(element)) { + return i; + } + i++; + } + return -1; + } + public static long size(Iterable iterable) { return StreamSupport.stream(iterable.spliterator(), true).count(); } diff --git a/server/src/test/java/org/elasticsearch/common/util/iterable/IterablesTests.java b/server/src/test/java/org/elasticsearch/common/util/iterable/IterablesTests.java index 6501c7caa1d64..1d536a3c01d2c 100644 --- a/server/src/test/java/org/elasticsearch/common/util/iterable/IterablesTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/iterable/IterablesTests.java @@ -26,7 +26,10 @@ import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import static org.hamcrest.Matchers.is; import static org.hamcrest.object.HasToString.hasToString; public class IterablesTests extends ESTestCase { @@ -86,6 +89,19 @@ public void testFlatten() { assertEquals(1, count); } + public void testIndexOf() { + final List list = Stream.generate(() -> randomAlphaOfLengthBetween(3, 9)) + .limit(randomIntBetween(10, 30)) + .distinct() + .collect(Collectors.toList()); + for (int i = 0; i < list.size(); i++) { + final String val = list.get(i); + assertThat(Iterables.indexOf(list, val::equals), is(i)); + } + assertThat(Iterables.indexOf(list, s -> false), is(-1)); + assertThat(Iterables.indexOf(list, s -> true), is(0)); + } + private void test(Iterable iterable) { try { Iterables.get(iterable, -1); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java index 8b2ae8ec14d7a..275dbdef3ef0a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java @@ -8,6 +8,8 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.cluster.metadata.IndexAbstraction; +import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.Strings; import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesRequest; import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesResponse; @@ -292,6 +294,14 @@ public boolean isAuditable() { return auditable; } + /** + * Returns additional context about an authorization failure, if {@link #isGranted()} is false. + */ + @Nullable + public String getFailureContext() { + return null; + } + /** * Returns a new authorization result that is granted and auditable */ @@ -321,6 +331,19 @@ public IndexAuthorizationResult(boolean auditable, IndicesAccessControl indicesA this.indicesAccessControl = indicesAccessControl; } + @Override + public String getFailureContext() { + if (isGranted()) { + return null; + } else { + return getFailureDescription(indicesAccessControl.getDeniedIndices()); + } + } + + public static String getFailureDescription(Collection deniedIndices) { + return "on indices [" + Strings.collectionToCommaDelimitedString(deniedIndices) + "]"; + } + public IndicesAccessControl getIndicesAccessControl() { return indicesAccessControl; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/IndicesAccessControl.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/IndicesAccessControl.java index 604508f95c5af..697df38bd07dc 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/IndicesAccessControl.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/IndicesAccessControl.java @@ -11,10 +11,12 @@ import org.elasticsearch.xpack.core.security.authz.permission.DocumentPermissions; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; /** * Encapsulates the field and document permissions per concrete index based on the current request. @@ -51,6 +53,13 @@ public boolean isGranted() { return granted; } + public Collection getDeniedIndices() { + return Collections.unmodifiableSet(this.indexPermissions.entrySet().stream() + .filter(e -> e.getValue().granted == false) + .map(Map.Entry::getKey) + .collect(Collectors.toSet())); + } + /** * Encapsulates the field and document permissions for an index. */ diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java index 18768a521185e..f7747b5daa0b6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java @@ -14,10 +14,6 @@ import org.elasticsearch.action.admin.indices.close.CloseIndexAction; import org.elasticsearch.action.admin.indices.create.AutoCreateAction; import org.elasticsearch.action.admin.indices.create.CreateIndexAction; -import org.elasticsearch.action.admin.indices.resolve.ResolveIndexAction; -import org.elasticsearch.xpack.core.action.CreateDataStreamAction; -import org.elasticsearch.xpack.core.action.DeleteDataStreamAction; -import org.elasticsearch.xpack.core.action.GetDataStreamAction; import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction; import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsAction; import org.elasticsearch.action.admin.indices.exists.types.TypesExistsAction; @@ -25,10 +21,13 @@ import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsAction; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsAction; import org.elasticsearch.action.admin.indices.mapping.put.AutoPutMappingAction; +import org.elasticsearch.action.admin.indices.resolve.ResolveIndexAction; import org.elasticsearch.action.admin.indices.settings.get.GetSettingsAction; import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryAction; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.collect.MapBuilder; +import org.elasticsearch.xpack.core.action.CreateDataStreamAction; +import org.elasticsearch.xpack.core.action.DeleteDataStreamAction; +import org.elasticsearch.xpack.core.action.GetDataStreamAction; import org.elasticsearch.xpack.core.ccr.action.ForgetFollowerAction; import org.elasticsearch.xpack.core.ccr.action.PutFollowAction; import org.elasticsearch.xpack.core.ccr.action.UnfollowAction; @@ -36,6 +35,7 @@ import org.elasticsearch.xpack.core.security.support.Automatons; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.Locale; @@ -43,7 +43,10 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; +import java.util.stream.Collectors; +import static org.elasticsearch.common.collect.Map.ofEntries; +import static org.elasticsearch.common.collect.Map.entry; import static org.elasticsearch.xpack.core.security.support.Automatons.patterns; import static org.elasticsearch.xpack.core.security.support.Automatons.unionAndMinimize; @@ -99,27 +102,26 @@ public final class IndexPrivilege extends Privilege { public static final IndexPrivilege MAINTENANCE = new IndexPrivilege("maintenance", MAINTENANCE_AUTOMATON); public static final IndexPrivilege AUTO_CONFIGURE = new IndexPrivilege("auto_configure", AUTO_CONFIGURE_AUTOMATON); - private static final Map VALUES = MapBuilder.newMapBuilder() - .put("none", NONE) - .put("all", ALL) - .put("manage", MANAGE) - .put("create_index", CREATE_INDEX) - .put("monitor", MONITOR) - .put("read", READ) - .put("index", INDEX) - .put("delete", DELETE) - .put("write", WRITE) - .put("create", CREATE) - .put("create_doc", CREATE_DOC) - .put("delete_index", DELETE_INDEX) - .put("view_index_metadata", VIEW_METADATA) - .put("read_cross_cluster", READ_CROSS_CLUSTER) - .put("manage_follow_index", MANAGE_FOLLOW_INDEX) - .put("manage_leader_index", MANAGE_LEADER_INDEX) - .put("manage_ilm", MANAGE_ILM) - .put("maintenance", MAINTENANCE) - .put("auto_configure", AUTO_CONFIGURE) - .immutableMap(); + private static final Map VALUES = sortByAccessLevel(ofEntries( + entry("none", NONE), + entry("all", ALL), + entry("manage", MANAGE), + entry("create_index", CREATE_INDEX), + entry("monitor", MONITOR), + entry("read", READ), + entry("index", INDEX), + entry("delete", DELETE), + entry("write", WRITE), + entry("create", CREATE), + entry("create_doc", CREATE_DOC), + entry("delete_index", DELETE_INDEX), + entry("view_index_metadata", VIEW_METADATA), + entry("read_cross_cluster", READ_CROSS_CLUSTER), + entry("manage_follow_index", MANAGE_FOLLOW_INDEX), + entry("manage_leader_index", MANAGE_LEADER_INDEX), + entry("manage_ilm", MANAGE_ILM), + entry("maintenance", MAINTENANCE), + entry("auto_configure", AUTO_CONFIGURE))); public static final Predicate ACTION_MATCHER = ALL.predicate(); public static final Predicate CREATE_INDEX_MATCHER = CREATE_INDEX.predicate(); @@ -157,7 +159,7 @@ private static IndexPrivilege resolve(Set name) { if (ACTION_MATCHER.test(part)) { actions.add(actionToPattern(part)); } else { - IndexPrivilege indexPrivilege = VALUES.get(part); + IndexPrivilege indexPrivilege = part == null ? null : VALUES.get(part); if (indexPrivilege != null && size == 1) { return indexPrivilege; } else if (indexPrivilege != null) { @@ -187,4 +189,16 @@ public static Set names() { return Collections.unmodifiableSet(VALUES.keySet()); } + /** + * Returns the names of privileges that grant the specified action. + * @return A collection of names, ordered (to the extent possible) from least privileged (e.g. {@link #CREATE_DOC}) + * to most privileged (e.g. {@link #ALL}) + * @see Privilege#sortByAccessLevel + */ + public static Collection findPrivilegesThatGrant(String action) { + return Collections.unmodifiableList(VALUES.entrySet().stream() + .filter(e -> e.getValue().predicate.test(action)) + .map(e -> e.getKey()) + .collect(Collectors.toList())); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/Privilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/Privilege.java index 54db92dacae88..2f1eb0728f129 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/Privilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/Privilege.java @@ -6,10 +6,16 @@ package org.elasticsearch.xpack.core.security.authz.privilege; import org.apache.lucene.util.automaton.Automaton; +import org.apache.lucene.util.automaton.Operations; import org.elasticsearch.xpack.core.security.support.Automatons; import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.function.Predicate; import static org.elasticsearch.xpack.core.security.support.Automatons.patterns; @@ -74,4 +80,21 @@ public String toString() { public Automaton getAutomaton() { return automaton; } + + /** + * Sorts the map of privileges from least-privilege to most-privilege + */ + static SortedMap sortByAccessLevel(Map privileges) { + // How many other privileges is this privilege a subset of. Those with a higher count are considered to be a lower privilege + final Map subsetCount = new HashMap<>(privileges.size()); + privileges.forEach((name, priv) -> subsetCount.put(name, + privileges.values().stream().filter(p2 -> p2 != priv && Operations.subsetOf(priv.automaton, p2.automaton)).count()) + ); + + final Comparator compare = Comparator.comparingLong(key -> subsetCount.getOrDefault(key, 0L)).reversed() + .thenComparing(Comparator.naturalOrder()); + final TreeMap tree = new TreeMap<>(compare); + tree.putAll(privileges); + return Collections.unmodifiableSortedMap(tree); + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilegeTests.java new file mode 100644 index 0000000000000..28e9e230af5be --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilegeTests.java @@ -0,0 +1,62 @@ +/* + * 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.xpack.core.security.authz.privilege; + +import org.elasticsearch.action.admin.indices.refresh.RefreshAction; +import org.elasticsearch.action.admin.indices.shrink.ShrinkAction; +import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction; +import org.elasticsearch.action.delete.DeleteAction; +import org.elasticsearch.action.index.IndexAction; +import org.elasticsearch.action.search.SearchAction; +import org.elasticsearch.action.update.UpdateAction; +import org.elasticsearch.common.util.iterable.Iterables; +import org.elasticsearch.test.ESTestCase; + +import org.elasticsearch.common.collect.List; +import java.util.Set; + +import static org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege.findPrivilegesThatGrant; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.lessThan; + +public class IndexPrivilegeTests extends ESTestCase { + + /** + * The {@link IndexPrivilege#values()} map is sorted so that privilege names that offer the _least_ access come before those that + * offer _more_ access. There is no guarantee of ordering between privileges that offer non-overlapping privileges. + */ + public void testOrderingOfPrivilegeNames() throws Exception { + final Set names = IndexPrivilege.values().keySet(); + final int all = Iterables.indexOf(names, "all"::equals); + final int manage = Iterables.indexOf(names, "manage"::equals); + final int monitor = Iterables.indexOf(names, "monitor"::equals); + final int read = Iterables.indexOf(names, "read"::equals); + final int write = Iterables.indexOf(names, "write"::equals); + final int index = Iterables.indexOf(names, "index"::equals); + final int create_doc = Iterables.indexOf(names, "create_doc"::equals); + final int delete = Iterables.indexOf(names, "delete"::equals); + + assertThat(read, lessThan(all)); + assertThat(manage, lessThan(all)); + assertThat(monitor, lessThan(manage)); + assertThat(write, lessThan(all)); + assertThat(index, lessThan(write)); + assertThat(create_doc, lessThan(index)); + assertThat(delete, lessThan(write)); + } + + public void testFindPrivilegesThatGrant() { + assertThat(findPrivilegesThatGrant(SearchAction.NAME), equalTo(List.of("read", "all"))); + assertThat(findPrivilegesThatGrant(IndexAction.NAME), equalTo(List.of("create_doc", "create", "index", "write", "all"))); + assertThat(findPrivilegesThatGrant(UpdateAction.NAME), equalTo(List.of("index", "write", "all"))); + assertThat(findPrivilegesThatGrant(DeleteAction.NAME), equalTo(List.of("delete", "write", "all"))); + assertThat(findPrivilegesThatGrant(IndicesStatsAction.NAME), equalTo(List.of("monitor", "manage", "all"))); + assertThat(findPrivilegesThatGrant(RefreshAction.NAME), equalTo(List.of("maintenance", "manage", "all"))); + assertThat(findPrivilegesThatGrant(ShrinkAction.NAME), equalTo(List.of("manage", "all"))); + } + +} diff --git a/x-pack/plugin/ilm/qa/with-security/src/javaRestTest/java/org/elasticsearch/xpack/security/PermissionsIT.java b/x-pack/plugin/ilm/qa/with-security/src/javaRestTest/java/org/elasticsearch/xpack/security/PermissionsIT.java index 3ec7ea39f6619..d57384dbaff9c 100644 --- a/x-pack/plugin/ilm/qa/with-security/src/javaRestTest/java/org/elasticsearch/xpack/security/PermissionsIT.java +++ b/x-pack/plugin/ilm/qa/with-security/src/javaRestTest/java/org/elasticsearch/xpack/security/PermissionsIT.java @@ -143,7 +143,8 @@ public void testCanManageIndexWithNoPermissions() throws Exception { assertThat(indexExplain.get("failed_step"), equalTo("wait-for-shard-history-leases")); Map stepInfo = (Map) indexExplain.get("step_info"); assertThat(stepInfo.get("type"), equalTo("security_exception")); - assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized for user [test_ilm]")); + assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized for user [test_ilm]" + + " on indices [not-ilm], this action is granted by the privileges [monitor,manage,all]")); } }); } diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsRestIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsRestIT.java index 24d985ff120a1..2e478cbeb3651 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsRestIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsRestIT.java @@ -818,7 +818,7 @@ public void testLookbackWithoutPermissions() throws Exception { new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId)); String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity()); assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " + - "action [indices:data/read/search] is unauthorized for user [ml_admin_plus_data]\"")); + "action [indices:data/read/search] is unauthorized for user [ml_admin_plus_data] on indices [network-data],")); } public void testLookbackWithPipelineBucketAgg() throws Exception { @@ -966,7 +966,8 @@ public void testLookbackWithoutPermissionsAndRollup() throws Exception { new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId)); String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity()); assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " + - "action [indices:data/read/xpack/rollup/search] is unauthorized for user [ml_admin_plus_data]\"")); + "action [indices:data/read/xpack/rollup/search] is unauthorized for user [ml_admin_plus_data] " + + "on indices [airline-data-aggs-rollup], ")); } public void testLookbackWithSingleBucketAgg() throws Exception { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index a4b731eef6be2..cc51cc84ef5d9 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -85,7 +85,9 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; +import static java.util.Collections.singletonList; import static org.elasticsearch.action.support.ContextPreservingActionListener.wrapPreservingContext; +import static org.elasticsearch.common.Strings.collectionToCommaDelimitedString; import static org.elasticsearch.xpack.core.security.SecurityField.setting; import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.ACTION_SCOPE_AUTHORIZATION_KEYS; import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.AUTHORIZATION_INFO_KEY; @@ -276,7 +278,7 @@ private void authorizeAction(final RequestInfo requestInfo, final String request listener.onResponse(null); }, listener::onFailure, requestInfo, requestId, authzInfo), threadContext); authzEngine.authorizeClusterAction(requestInfo, authzInfo, clusterAuthzListener); - } else if (IndexPrivilege.ACTION_MATCHER.test(action)) { + } else if (isIndexAction(action)) { final Metadata metadata = clusterService.state().metadata(); final AsyncSupplier> authorizedIndicesSupplier = new CachingAsyncSupplier<>(authzIndicesListener -> authzEngine.loadAuthorizedIndices(requestInfo, authzInfo, metadata.getIndicesLookup(), @@ -541,7 +543,8 @@ private void authorizeBulkItems(RequestInfo requestInfo, AuthorizationInfo authz if (indexAccessControl == null || indexAccessControl.isGranted() == false) { auditTrail.explicitIndexAccessEvent(requestId, AuditLevel.ACCESS_DENIED, authentication, itemAction, resolvedIndex, item.getClass().getSimpleName(), request.remoteAddress(), authzInfo); - item.abort(resolvedIndex, denialException(authentication, itemAction, null)); + item.abort(resolvedIndex, denialException(authentication, itemAction, + IndexAuthorizationResult.getFailureDescription(singletonList(resolvedIndex)), null)); } else if (audit.get()) { auditTrail.explicitIndexAccessEvent(requestId, AuditLevel.ACCESS_GRANTED, authentication, itemAction, resolvedIndex, item.getClass().getSimpleName(), request.remoteAddress(), authzInfo); @@ -561,8 +564,8 @@ private void authorizeBulkItems(RequestInfo requestInfo, AuthorizationInfo authz groupedActionListener.onResponse(new Tuple<>(bulkItemAction, indexAuthorizationResult)), groupedActionListener::onFailure)); }); - }, listener::onFailure)); }, listener::onFailure)); + }, listener::onFailure)); } private static IllegalArgumentException illegalArgument(String message) { @@ -570,6 +573,10 @@ private static IllegalArgumentException illegalArgument(String message) { return new IllegalArgumentException(message); } + private static boolean isIndexAction(String action) { + return IndexPrivilege.ACTION_MATCHER.test(action); + } + private static String getAction(BulkItemRequest item) { final DocWriteRequest docWriteRequest = item.request(); switch (docWriteRequest.opType()) { @@ -598,6 +605,11 @@ private void putTransientIfNonExisting(String key, Object value) { } private ElasticsearchSecurityException denialException(Authentication authentication, String action, Exception cause) { + return denialException(authentication, action, null, cause); + } + + private ElasticsearchSecurityException denialException(Authentication authentication, String action, @Nullable String context, + Exception cause) { final User authUser = authentication.getUser().authenticatedUser(); // Special case for anonymous user if (isAnonymousEnabled && anonymousUser.equals(authUser)) { @@ -605,23 +617,33 @@ private ElasticsearchSecurityException denialException(Authentication authentica return authcFailureHandler.authenticationRequired(action, threadContext); } } + + String userText = "user [" + authUser.principal() + "]"; // check for run as if (authentication.getUser().isRunAs()) { - logger.debug("action [{}] is unauthorized for user [{}] run as [{}]", action, authUser.principal(), - authentication.getUser().principal()); - return authorizationError("action [{}] is unauthorized for user [{}] run as [{}]", cause, action, authUser.principal(), - authentication.getUser().principal()); + userText = userText + " run as [" + authentication.getUser().principal() + "]"; } // check for authentication by API key if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { final String apiKeyId = (String) authentication.getMetadata().get(ApiKeyService.API_KEY_ID_KEY); assert apiKeyId != null : "api key id must be present in the metadata"; - logger.debug("action [{}] is unauthorized for API key id [{}] of user [{}]", action, apiKeyId, authUser.principal()); - return authorizationError("action [{}] is unauthorized for API key id [{}] of user [{}]", cause, action, apiKeyId, - authUser.principal()); + userText = "API key id [" + apiKeyId + "] of " + userText; } - logger.debug("action [{}] is unauthorized for user [{}]", action, authUser.principal()); - return authorizationError("action [{}] is unauthorized for user [{}]", cause, action, authUser.principal()); + + String message = "action [" + action + "] is unauthorized for " + userText; + if (context != null) { + message = message + " " + context; + } + + if(isIndexAction(action)) { + final Collection privileges = IndexPrivilege.findPrivilegesThatGrant(action); + if (privileges != null && privileges.size() > 0) { + message = message + ", this action is granted by the privileges [" + collectionToCommaDelimitedString(privileges) + "]"; + } + } + + logger.debug(message); + return authorizationError(message, cause); } private class AuthorizationResultListener implements ActionListener { @@ -654,21 +676,21 @@ public void onResponse(T result) { failureConsumer.accept(e); } } else { - handleFailure(result.isAuditable(), null); + handleFailure(result.isAuditable(), result.getFailureContext(), null); } } @Override public void onFailure(Exception e) { - handleFailure(true, e); + handleFailure(true, null, e); } - private void handleFailure(boolean audit, @Nullable Exception e) { + private void handleFailure(boolean audit, @Nullable String context, @Nullable Exception e) { if (audit) { auditTrailService.get().accessDenied(requestId, requestInfo.getAuthentication(), requestInfo.getAction(), requestInfo.getRequest(), authzInfo); } - failureConsumer.accept(denialException(requestInfo.getAuthentication(), requestInfo.getAction(), e)); + failureConsumer.accept(denialException(requestInfo.getAuthentication(), requestInfo.getAction(), context, e)); } } 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 b06152d13b35e..a4eb1515581d8 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 @@ -44,6 +44,9 @@ import org.elasticsearch.action.bulk.BulkItemRequest; import org.elasticsearch.action.bulk.BulkRequest; import org.elasticsearch.action.bulk.BulkShardRequest; +import org.elasticsearch.action.bulk.BulkShardResponse; +import org.elasticsearch.action.bulk.MappingUpdatePerformer; +import org.elasticsearch.action.bulk.TransportShardBulkAction; import org.elasticsearch.action.delete.DeleteAction; import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.get.GetAction; @@ -65,11 +68,13 @@ import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.action.support.replication.TransportReplicationAction; import org.elasticsearch.action.termvectors.MultiTermVectorsAction; import org.elasticsearch.action.termvectors.MultiTermVectorsRequest; import org.elasticsearch.action.termvectors.TermVectorsAction; import org.elasticsearch.action.termvectors.TermVectorsRequest; import org.elasticsearch.action.update.UpdateAction; +import org.elasticsearch.action.update.UpdateHelper; import org.elasticsearch.action.update.UpdateRequest; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.AliasMetadata; @@ -91,9 +96,11 @@ import org.elasticsearch.common.util.concurrent.ThreadContext.StoredContext; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.IndexNotFoundException; +import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.license.XPackLicenseState.Feature; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportActionProxy; @@ -165,13 +172,17 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.CountDownLatch; +import java.util.function.Consumer; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Predicate; import static java.util.Arrays.asList; +import static java.util.Collections.singletonMap; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationException; import static org.elasticsearch.test.SecurityTestsUtils.assertThrowsAuthorizationExceptionRunAs; +import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail.PRINCIPAL_ROLES_FIELD_NAME; +import static org.elasticsearch.test.TestMatchers.throwableWithMessage; import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.AUTHORIZATION_INFO_KEY; import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.INDICES_PERMISSIONS_KEY; import static org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField.ORIGINATING_ACTION_KEY; @@ -179,6 +190,7 @@ import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.SECURITY_MAIN_ALIAS; import static org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail.PRINCIPAL_ROLES_FIELD_NAME; import static org.hamcrest.Matchers.arrayContainingInAnyOrder; +import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; @@ -588,6 +600,7 @@ public void testUserWithNoRolesCannotSql() throws IOException { authzInfoRoles(Role.EMPTY.names())); verifyNoMoreInteractions(auditTrail); } + /** * Verifies that the behaviour tested in {@link #testUserWithNoRolesCanPerformRemoteSearch} * does not work for requests that are not remote-index-capable. @@ -868,6 +881,73 @@ public void testCreateIndexWithAlias() throws IOException { verify(state, times(1)).metadata(); } + public void testDenialErrorMessagesForSearchAction() throws IOException { + RoleDescriptor role = new RoleDescriptor("some_indices_" + randomAlphaOfLengthBetween(3, 6), null, new IndicesPrivileges[]{ + IndicesPrivileges.builder().indices("all*").privileges("all").build(), + IndicesPrivileges.builder().indices("read*").privileges("read").build(), + IndicesPrivileges.builder().indices("write*").privileges("write").build() + }, null); + User user = new User(randomAlphaOfLengthBetween(6, 8), role.getName()); + final Authentication authentication = createAuthentication(user); + roleMap.put(role.getName(), role); + + AuditUtil.getOrGenerateRequestId(threadContext); + + TransportRequest request = new SearchRequest("all-1", "read-2", "write-3", "other-4"); + + ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class, + () -> authorize(authentication, SearchAction.NAME, request)); + assertThat(securityException, throwableWithMessage( + containsString("[" + SearchAction.NAME + "] is unauthorized for user [" + user.principal() + "] on indices ["))); + assertThat(securityException, throwableWithMessage(containsString("write-3"))); + assertThat(securityException, throwableWithMessage(containsString("other-4"))); + assertThat(securityException, throwableWithMessage(not(containsString("all-1")))); + assertThat(securityException, throwableWithMessage(not(containsString("read-2")))); + assertThat(securityException, throwableWithMessage(containsString(", this action is granted by the privileges [read,all]"))); + } + + public void testDenialErrorMessagesForBulkIngest() throws Exception { + final String index = randomAlphaOfLengthBetween(5, 12); + RoleDescriptor role = new RoleDescriptor("some_indices_" + randomAlphaOfLengthBetween(3, 6), null, new IndicesPrivileges[]{ + IndicesPrivileges.builder().indices(index).privileges(BulkAction.NAME).build() + }, null); + User user = new User(randomAlphaOfLengthBetween(6, 8), role.getName()); + final Authentication authentication = createAuthentication(user); + roleMap.put(role.getName(), role); + + AuditUtil.getOrGenerateRequestId(threadContext); + + final BulkShardRequest request = new BulkShardRequest( + new ShardId(index, randomAlphaOfLength(24), 1), + WriteRequest.RefreshPolicy.NONE, + new BulkItemRequest[]{ + new BulkItemRequest(0, + new IndexRequest(index).id("doc-1").opType(DocWriteRequest.OpType.CREATE).source(singletonMap("field", "value"))), + new BulkItemRequest(1, + new IndexRequest(index).id("doc-2").opType(DocWriteRequest.OpType.INDEX).source(singletonMap("field", "value"))), + new BulkItemRequest(2, new DeleteRequest(index, "doc-3")) + }); + + authorize(authentication, TransportShardBulkAction.ACTION_NAME, request); + + MappingUpdatePerformer mappingUpdater = (m, s, t, l) -> l.onResponse(null); + Consumer> waitForMappingUpdate = l -> l.onResponse(null); + PlainActionFuture> future = new PlainActionFuture<>(); + IndexShard indexShard = mock(IndexShard.class); + TransportShardBulkAction.performOnPrimary(request, indexShard, new UpdateHelper(mock(ScriptService.class)), + System::currentTimeMillis, mappingUpdater, waitForMappingUpdate, future, threadPool, ThreadPool.Names.WRITE); + + TransportReplicationAction.PrimaryResult result = future.get(); + BulkShardResponse response = result.finalResponseIfSuccessful; + assertThat(response, notNullValue()); + assertThat(response.getResponses(), arrayWithSize(3)); + assertThat(response.getResponses()[0].getFailureMessage(), containsString("unauthorized for user [" + user.principal() + "]")); + assertThat(response.getResponses()[0].getFailureMessage(), containsString("on indices [" + index + "]")); + assertThat(response.getResponses()[0].getFailureMessage(), containsString("[create_doc,create,index,write,all]") ); + assertThat(response.getResponses()[1].getFailureMessage(), containsString("[create,index,write,all]") ); + assertThat(response.getResponses()[2].getFailureMessage(), containsString("[delete,write,all]") ); + } + public void testDenialForAnonymousUser() throws IOException { TransportRequest request = new GetIndexRequest().indices("b"); ClusterState state = mockEmptyMetadata();