Skip to content

Commit 39a198f

Browse files
tvernumtvernum
committed
Add more context to index access denied errors
Access denied messages for indices were overly brief and missed two pieces of useful information: 1. The names of the indices for which access was denied 2. The privileges that could be used to grant that access This change improves the access denied messages for index based actions by adding the index and privilege names. Privilege names are listed in order from least-privilege to most-privileged so that the first recommended path to resolution is also the lowest privilege change. Relates: elastic#42166 Backport of: elastic#60357
1 parent 54aaada commit 39a198f

File tree

12 files changed

+416
-151
lines changed

12 files changed

+416
-151
lines changed

‎server/src/main/java/org/elasticsearch/common/util/iterable/Iterables.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Iterator;
2525
import java.util.List;
2626
import java.util.Objects;
27+
import java.util.function.Predicate;
2728
import java.util.stream.Stream;
2829
import java.util.stream.StreamSupport;
2930

@@ -103,6 +104,17 @@ public static <T> T get(Iterable<T> iterable, int position) {
103104
}
104105
}
105106

107+
public static <T> int indexOf(Iterable<T> iterable, Predicate<T> predicate) {
108+
int i = 0;
109+
for (T element : iterable) {
110+
if (predicate.test(element)) {
111+
return i;
112+
}
113+
i++;
114+
}
115+
return -1;
116+
}
117+
106118
public static long size(Iterable<?> iterable) {
107119
return StreamSupport.stream(iterable.spliterator(), true).count();
108120
}

‎server/src/test/java/org/elasticsearch/common/util/iterable/IterablesTests.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@
2626
import java.util.Iterator;
2727
import java.util.List;
2828
import java.util.NoSuchElementException;
29+
import java.util.stream.Collectors;
30+
import java.util.stream.Stream;
2931

32+
import static org.hamcrest.Matchers.is;
3033
import static org.hamcrest.object.HasToString.hasToString;
3134

3235
public class IterablesTests extends ESTestCase {
@@ -86,6 +89,19 @@ public void testFlatten() {
8689
assertEquals(1, count);
8790
}
8891

92+
public void testIndexOf() {
93+
final List<String> list = Stream.generate(() -> randomAlphaOfLengthBetween(3, 9))
94+
.limit(randomIntBetween(10, 30))
95+
.distinct()
96+
.collect(Collectors.toList());
97+
for (int i = 0; i < list.size(); i++) {
98+
final String val = list.get(i);
99+
assertThat(Iterables.indexOf(list, val::equals), is(i));
100+
}
101+
assertThat(Iterables.indexOf(list, s -> false), is(-1));
102+
assertThat(Iterables.indexOf(list, s -> true), is(0));
103+
}
104+
89105
private void test(Iterable<String> iterable) {
90106
try {
91107
Iterables.get(iterable, -1);

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
import org.elasticsearch.action.ActionListener;
1010
import org.elasticsearch.cluster.metadata.IndexAbstraction;
11+
import org.elasticsearch.common.Nullable;
12+
import org.elasticsearch.common.Strings;
1113
import org.elasticsearch.transport.TransportRequest;
1214
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesRequest;
1315
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesResponse;
@@ -292,6 +294,14 @@ public boolean isAuditable() {
292294
return auditable;
293295
}
294296

297+
/**
298+
* Returns additional context about an authorization failure, if {@link #isGranted()} is false.
299+
*/
300+
@Nullable
301+
public String getFailureContext() {
302+
return null;
303+
}
304+
295305
/**
296306
* Returns a new authorization result that is granted and auditable
297307
*/
@@ -321,6 +331,19 @@ public IndexAuthorizationResult(boolean auditable, IndicesAccessControl indicesA
321331
this.indicesAccessControl = indicesAccessControl;
322332
}
323333

334+
@Override
335+
public String getFailureContext() {
336+
if (isGranted()) {
337+
return null;
338+
} else {
339+
return getFailureDescription(indicesAccessControl.getDeniedIndices());
340+
}
341+
}
342+
343+
public static String getFailureDescription(Collection<?> deniedIndices) {
344+
return "on indices [" + Strings.collectionToCommaDelimitedString(deniedIndices) + "]";
345+
}
346+
324347
public IndicesAccessControl getIndicesAccessControl() {
325348
return indicesAccessControl;
326349
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/IndicesAccessControl.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111
import org.elasticsearch.xpack.core.security.authz.permission.DocumentPermissions;
1212
import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions;
1313

14+
import java.util.Collection;
1415
import java.util.Collections;
1516
import java.util.HashMap;
1617
import java.util.Map;
1718
import java.util.Set;
19+
import java.util.stream.Collectors;
1820

1921
/**
2022
* Encapsulates the field and document permissions per concrete index based on the current request.
@@ -51,6 +53,13 @@ public boolean isGranted() {
5153
return granted;
5254
}
5355

56+
public Collection<?> getDeniedIndices() {
57+
return Collections.unmodifiableSet(this.indexPermissions.entrySet().stream()
58+
.filter(e -> e.getValue().granted == false)
59+
.map(Map.Entry::getKey)
60+
.collect(Collectors.toSet()));
61+
}
62+
5463
/**
5564
* Encapsulates the field and document permissions for an index.
5665
*/

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilege.java‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,36 +14,39 @@
1414
import org.elasticsearch.action.admin.indices.close.CloseIndexAction;
1515
import org.elasticsearch.action.admin.indices.create.AutoCreateAction;
1616
import org.elasticsearch.action.admin.indices.create.CreateIndexAction;
17-
import org.elasticsearch.action.admin.indices.resolve.ResolveIndexAction;
18-
import org.elasticsearch.xpack.core.action.CreateDataStreamAction;
19-
import org.elasticsearch.xpack.core.action.DeleteDataStreamAction;
20-
import org.elasticsearch.xpack.core.action.GetDataStreamAction;
2117
import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction;
2218
import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsAction;
2319
import org.elasticsearch.action.admin.indices.exists.types.TypesExistsAction;
2420
import org.elasticsearch.action.admin.indices.get.GetIndexAction;
2521
import org.elasticsearch.action.admin.indices.mapping.get.GetFieldMappingsAction;
2622
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsAction;
2723
import org.elasticsearch.action.admin.indices.mapping.put.AutoPutMappingAction;
24+
import org.elasticsearch.action.admin.indices.resolve.ResolveIndexAction;
2825
import org.elasticsearch.action.admin.indices.settings.get.GetSettingsAction;
2926
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryAction;
3027
import org.elasticsearch.common.Strings;
31-
import org.elasticsearch.common.collect.MapBuilder;
28+
import org.elasticsearch.xpack.core.action.CreateDataStreamAction;
29+
import org.elasticsearch.xpack.core.action.DeleteDataStreamAction;
30+
import org.elasticsearch.xpack.core.action.GetDataStreamAction;
3231
import org.elasticsearch.xpack.core.ccr.action.ForgetFollowerAction;
3332
import org.elasticsearch.xpack.core.ccr.action.PutFollowAction;
3433
import org.elasticsearch.xpack.core.ccr.action.UnfollowAction;
3534
import org.elasticsearch.xpack.core.ilm.action.ExplainLifecycleAction;
3635
import org.elasticsearch.xpack.core.security.support.Automatons;
3736

3837
import java.util.Arrays;
38+
import java.util.Collection;
3939
import java.util.Collections;
4040
import java.util.HashSet;
4141
import java.util.Locale;
4242
import java.util.Map;
4343
import java.util.Set;
4444
import java.util.concurrent.ConcurrentHashMap;
4545
import java.util.function.Predicate;
46+
import java.util.stream.Collectors;
4647

48+
import static org.elasticsearch.common.collect.Map.ofEntries;
49+
import static org.elasticsearch.common.collect.Map.entry;
4750
import static org.elasticsearch.xpack.core.security.support.Automatons.patterns;
4851
import static org.elasticsearch.xpack.core.security.support.Automatons.unionAndMinimize;
4952

@@ -99,27 +102,26 @@ public final class IndexPrivilege extends Privilege {
99102
public static final IndexPrivilege MAINTENANCE = new IndexPrivilege("maintenance", MAINTENANCE_AUTOMATON);
100103
public static final IndexPrivilege AUTO_CONFIGURE = new IndexPrivilege("auto_configure", AUTO_CONFIGURE_AUTOMATON);
101104

102-
private static final Map<String, IndexPrivilege> VALUES = MapBuilder.<String, IndexPrivilege>newMapBuilder()
103-
.put("none", NONE)
104-
.put("all", ALL)
105-
.put("manage", MANAGE)
106-
.put("create_index", CREATE_INDEX)
107-
.put("monitor", MONITOR)
108-
.put("read", READ)
109-
.put("index", INDEX)
110-
.put("delete", DELETE)
111-
.put("write", WRITE)
112-
.put("create", CREATE)
113-
.put("create_doc", CREATE_DOC)
114-
.put("delete_index", DELETE_INDEX)
115-
.put("view_index_metadata", VIEW_METADATA)
116-
.put("read_cross_cluster", READ_CROSS_CLUSTER)
117-
.put("manage_follow_index", MANAGE_FOLLOW_INDEX)
118-
.put("manage_leader_index", MANAGE_LEADER_INDEX)
119-
.put("manage_ilm", MANAGE_ILM)
120-
.put("maintenance", MAINTENANCE)
121-
.put("auto_configure", AUTO_CONFIGURE)
122-
.immutableMap();
105+
private static final Map<String, IndexPrivilege> VALUES = sortByAccessLevel(ofEntries(
106+
entry("none", NONE),
107+
entry("all", ALL),
108+
entry("manage", MANAGE),
109+
entry("create_index", CREATE_INDEX),
110+
entry("monitor", MONITOR),
111+
entry("read", READ),
112+
entry("index", INDEX),
113+
entry("delete", DELETE),
114+
entry("write", WRITE),
115+
entry("create", CREATE),
116+
entry("create_doc", CREATE_DOC),
117+
entry("delete_index", DELETE_INDEX),
118+
entry("view_index_metadata", VIEW_METADATA),
119+
entry("read_cross_cluster", READ_CROSS_CLUSTER),
120+
entry("manage_follow_index", MANAGE_FOLLOW_INDEX),
121+
entry("manage_leader_index", MANAGE_LEADER_INDEX),
122+
entry("manage_ilm", MANAGE_ILM),
123+
entry("maintenance", MAINTENANCE),
124+
entry("auto_configure", AUTO_CONFIGURE)));
123125

124126
public static final Predicate<String> ACTION_MATCHER = ALL.predicate();
125127
public static final Predicate<String> CREATE_INDEX_MATCHER = CREATE_INDEX.predicate();
@@ -157,7 +159,7 @@ private static IndexPrivilege resolve(Set<String> name) {
157159
if (ACTION_MATCHER.test(part)) {
158160
actions.add(actionToPattern(part));
159161
} else {
160-
IndexPrivilege indexPrivilege = VALUES.get(part);
162+
IndexPrivilege indexPrivilege = part == null ? null : VALUES.get(part);
161163
if (indexPrivilege != null && size == 1) {
162164
return indexPrivilege;
163165
} else if (indexPrivilege != null) {
@@ -187,4 +189,16 @@ public static Set<String> names() {
187189
return Collections.unmodifiableSet(VALUES.keySet());
188190
}
189191

192+
/**
193+
* Returns the names of privileges that grant the specified action.
194+
* @return A collection of names, ordered (to the extent possible) from least privileged (e.g. {@link #CREATE_DOC})
195+
* to most privileged (e.g. {@link #ALL})
196+
* @see Privilege#sortByAccessLevel
197+
*/
198+
public static Collection<String> findPrivilegesThatGrant(String action) {
199+
return Collections.unmodifiableList(VALUES.entrySet().stream()
200+
.filter(e -> e.getValue().predicate.test(action))
201+
.map(e -> e.getKey())
202+
.collect(Collectors.toList()));
203+
}
190204
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/Privilege.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,16 @@
66
package org.elasticsearch.xpack.core.security.authz.privilege;
77

88
import org.apache.lucene.util.automaton.Automaton;
9+
import org.apache.lucene.util.automaton.Operations;
910
import org.elasticsearch.xpack.core.security.support.Automatons;
1011

1112
import java.util.Collections;
13+
import java.util.Comparator;
14+
import java.util.HashMap;
15+
import java.util.Map;
1216
import java.util.Set;
17+
import java.util.SortedMap;
18+
import java.util.TreeMap;
1319
import java.util.function.Predicate;
1420

1521
import static org.elasticsearch.xpack.core.security.support.Automatons.patterns;
@@ -74,4 +80,21 @@ public String toString() {
7480
public Automaton getAutomaton() {
7581
return automaton;
7682
}
83+
84+
/**
85+
* Sorts the map of privileges from least-privilege to most-privilege
86+
*/
87+
static <T extends Privilege> SortedMap<String, T> sortByAccessLevel(Map<String, T> privileges) {
88+
// How many other privileges is this privilege a subset of. Those with a higher count are considered to be a lower privilege
89+
final Map<String, Long> subsetCount = new HashMap<>(privileges.size());
90+
privileges.forEach((name, priv) -> subsetCount.put(name,
91+
privileges.values().stream().filter(p2 -> p2 != priv && Operations.subsetOf(priv.automaton, p2.automaton)).count())
92+
);
93+
94+
final Comparator<String> compare = Comparator.<String>comparingLong(key -> subsetCount.getOrDefault(key, 0L)).reversed()
95+
.thenComparing(Comparator.naturalOrder());
96+
final TreeMap<String, T> tree = new TreeMap<>(compare);
97+
tree.putAll(privileges);
98+
return Collections.unmodifiableSortedMap(tree);
99+
}
77100
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/IndexPrivilegeTests.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.core.security.authz.privilege;
8+
9+
import org.elasticsearch.action.admin.indices.refresh.RefreshAction;
10+
import org.elasticsearch.action.admin.indices.shrink.ShrinkAction;
11+
import org.elasticsearch.action.admin.indices.stats.IndicesStatsAction;
12+
import org.elasticsearch.action.delete.DeleteAction;
13+
import org.elasticsearch.action.index.IndexAction;
14+
import org.elasticsearch.action.search.SearchAction;
15+
import org.elasticsearch.action.update.UpdateAction;
16+
import org.elasticsearch.common.util.iterable.Iterables;
17+
import org.elasticsearch.test.ESTestCase;
18+
19+
import org.elasticsearch.common.collect.List;
20+
import java.util.Set;
21+
22+
import static org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege.findPrivilegesThatGrant;
23+
import static org.hamcrest.Matchers.equalTo;
24+
import static org.hamcrest.Matchers.lessThan;
25+
26+
public class IndexPrivilegeTests extends ESTestCase {
27+
28+
/**
29+
* The {@link IndexPrivilege#values()} map is sorted so that privilege names that offer the _least_ access come before those that
30+
* offer _more_ access. There is no guarantee of ordering between privileges that offer non-overlapping privileges.
31+
*/
32+
public void testOrderingOfPrivilegeNames() throws Exception {
33+
final Set<String> names = IndexPrivilege.values().keySet();
34+
final int all = Iterables.indexOf(names, "all"::equals);
35+
final int manage = Iterables.indexOf(names, "manage"::equals);
36+
final int monitor = Iterables.indexOf(names, "monitor"::equals);
37+
final int read = Iterables.indexOf(names, "read"::equals);
38+
final int write = Iterables.indexOf(names, "write"::equals);
39+
final int index = Iterables.indexOf(names, "index"::equals);
40+
final int create_doc = Iterables.indexOf(names, "create_doc"::equals);
41+
final int delete = Iterables.indexOf(names, "delete"::equals);
42+
43+
assertThat(read, lessThan(all));
44+
assertThat(manage, lessThan(all));
45+
assertThat(monitor, lessThan(manage));
46+
assertThat(write, lessThan(all));
47+
assertThat(index, lessThan(write));
48+
assertThat(create_doc, lessThan(index));
49+
assertThat(delete, lessThan(write));
50+
}
51+
52+
public void testFindPrivilegesThatGrant() {
53+
assertThat(findPrivilegesThatGrant(SearchAction.NAME), equalTo(List.of("read", "all")));
54+
assertThat(findPrivilegesThatGrant(IndexAction.NAME), equalTo(List.of("create_doc", "create", "index", "write", "all")));
55+
assertThat(findPrivilegesThatGrant(UpdateAction.NAME), equalTo(List.of("index", "write", "all")));
56+
assertThat(findPrivilegesThatGrant(DeleteAction.NAME), equalTo(List.of("delete", "write", "all")));
57+
assertThat(findPrivilegesThatGrant(IndicesStatsAction.NAME), equalTo(List.of("monitor", "manage", "all")));
58+
assertThat(findPrivilegesThatGrant(RefreshAction.NAME), equalTo(List.of("maintenance", "manage", "all")));
59+
assertThat(findPrivilegesThatGrant(ShrinkAction.NAME), equalTo(List.of("manage", "all")));
60+
}
61+
62+
}

‎x-pack/plugin/ilm/qa/with-security/src/test/java/org/elasticsearch/xpack/security/PermissionsIT.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,10 @@ public void testCanManageIndexWithNoPermissions() throws Exception {
143143
assertThat(indexExplain.get("failed_step"), equalTo("wait-for-shard-history-leases"));
144144
Map<String, String> stepInfo = (Map<String, String>) indexExplain.get("step_info");
145145
assertThat(stepInfo.get("type"), equalTo("security_exception"));
146-
assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized for user [test_ilm]"));
146+
assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized" +
147+
" for user [test_ilm]" +
148+
" on indices [not-ilm]," +
149+
" this action is granted by the privileges [monitor,manage,all]"));
147150
}
148151
});
149152
}

‎x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsRestIT.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,7 @@ public void testLookbackWithoutPermissions() throws Exception {
805805
new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId));
806806
String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity());
807807
assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " +
808-
"action [indices:data/read/search] is unauthorized for user [ml_admin_plus_data]\""));
808+
"action [indices:data/read/search] is unauthorized for user [ml_admin_plus_data] on indices [network-data]"));
809809
}
810810

811811
public void testLookbackWithPipelineBucketAgg() throws Exception {
@@ -953,7 +953,8 @@ public void testLookbackWithoutPermissionsAndRollup() throws Exception {
953953
new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId));
954954
String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity());
955955
assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " +
956-
"action [indices:data/read/xpack/rollup/search] is unauthorized for user [ml_admin_plus_data]\""));
956+
"action [indices:data/read/xpack/rollup/search] is unauthorized for user [ml_admin_plus_data] " +
957+
"on indices [airline-data-aggs-rollup]"));
957958
}
958959

959960
public void testLookbackWithSingleBucketAgg() throws Exception {

0 commit comments

Comments
 (0)