Skip to content

Commit 589d4b2

Browse files
committed
Add more context to cluster access denied messages
In elastic#60357 we improved the error message when access to perform an action on an index was denied by including the index name and the privileges that would grant the action. This commit extends the second part of that change (the list of privileges that would resolve the problem) to situations when a cluster action is denied. This implementation for cluster privileges is slightly more complex than that of index privileges because cluster privileges can be dependent on parameters in the request, not just the action name. For example, "manage_own_api_key" should be suggested as a matching privilege when a user attempts to create an API key, or delete their own API key, but should not be suggested when that same user attempts to delete another user's API key. Relates: elastic#42166
1 parent f2651e4 commit 589d4b2

File tree

10 files changed

+359
-155
lines changed

10 files changed

+359
-155
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,14 @@ protected boolean extendedCheck(String action, TransportRequest request, Authent
205205

206206
@Override
207207
protected boolean doImplies(ActionBasedPermissionCheck permissionCheck) {
208-
return permissionCheck instanceof AutomatonPermissionCheck;
208+
/*
209+
* We know that "permissionCheck" has an automaton which is a subset of ours.
210+
* Which means "permissionCheck" _cannot_ grant an action that we don't (see ActionBasedPermissionCheck#check)
211+
* Since we grant _all_ requests on actions within our automaton, we must therefore grant _all_ actions+requests that the other
212+
* permission check grants.
213+
*/
214+
return true;
209215
}
210-
211216
}
212217

213218
// action, request based permission check

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ public class ActionClusterPrivilege implements NamedClusterPrivilege {
1919
private final String name;
2020
private final Set<String> allowedActionPatterns;
2121
private final Set<String> excludedActionPatterns;
22+
private final ClusterPermission permission;
2223

2324
/**
2425
* Constructor for {@link ActionClusterPrivilege} defining what cluster actions are accessible for the user with this privilege.
@@ -43,6 +44,7 @@ public ActionClusterPrivilege(final String name, final Set<String> allowedAction
4344
this.name = name;
4445
this.allowedActionPatterns = allowedActionPatterns;
4546
this.excludedActionPatterns = excludedActionPatterns;
47+
this.permission = buildPermission(ClusterPermission.builder()).build();
4648
}
4749

4850
@Override
@@ -62,4 +64,9 @@ public Set<String> getExcludedActionPatterns() {
6264
public ClusterPermission.Builder buildPermission(final ClusterPermission.Builder builder) {
6365
return builder.add(this, allowedActionPatterns, excludedActionPatterns);
6466
}
67+
68+
@Override
69+
public ClusterPermission permission() {
70+
return permission;
71+
}
6572
}

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

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.elasticsearch.action.ingest.GetPipelineAction;
1919
import org.elasticsearch.action.ingest.SimulatePipelineAction;
2020
import org.elasticsearch.common.Strings;
21+
import org.elasticsearch.transport.TransportRequest;
2122
import org.elasticsearch.xpack.core.ilm.action.GetLifecycleAction;
2223
import org.elasticsearch.xpack.core.ilm.action.GetStatusAction;
2324
import org.elasticsearch.xpack.core.ilm.action.StartILMAction;
@@ -28,16 +29,21 @@
2829
import org.elasticsearch.xpack.core.security.action.token.InvalidateTokenAction;
2930
import org.elasticsearch.xpack.core.security.action.token.RefreshTokenAction;
3031
import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction;
32+
import org.elasticsearch.xpack.core.security.authc.Authentication;
3133
import org.elasticsearch.xpack.core.slm.action.GetSnapshotLifecycleAction;
3234

35+
import java.util.Collection;
3336
import java.util.Collections;
37+
import java.util.Comparator;
38+
import java.util.HashMap;
39+
import java.util.List;
3440
import java.util.Locale;
3541
import java.util.Map;
3642
import java.util.Objects;
3743
import java.util.Set;
38-
import java.util.function.Function;
44+
import java.util.SortedMap;
45+
import java.util.TreeMap;
3946
import java.util.stream.Collectors;
40-
import java.util.stream.Stream;
4147

4248
/**
4349
* Translates cluster privilege names into concrete implementations
@@ -140,7 +146,7 @@ public class ClusterPrivilegeResolver {
140146
public static final NamedClusterPrivilege MANAGE_LOGSTASH_PIPELINES = new ActionClusterPrivilege("manage_logstash_pipelines",
141147
Set.of("cluster:admin/logstash/pipeline/*"));
142148

143-
private static final Map<String, NamedClusterPrivilege> VALUES = Stream.of(
149+
private static final Map<String, NamedClusterPrivilege> VALUES = sortByAccessLevel(List.of(
144150
NONE,
145151
ALL,
146152
MONITOR,
@@ -178,7 +184,7 @@ public class ClusterPrivilegeResolver {
178184
DELEGATE_PKI,
179185
MANAGE_OWN_API_KEY,
180186
MANAGE_ENRICH,
181-
MANAGE_LOGSTASH_PIPELINES).collect(Collectors.toUnmodifiableMap(NamedClusterPrivilege::name, Function.identity()));
187+
MANAGE_LOGSTASH_PIPELINES));
182188

183189
/**
184190
* Resolves a {@link NamedClusterPrivilege} from a given name if it exists.
@@ -219,4 +225,36 @@ private static String actionToPattern(String text) {
219225
return text + "*";
220226
}
221227

228+
/**
229+
* Returns the names of privileges that grant the specified action and request, for the given authentication context.
230+
* @return A collection of names, ordered (to the extent possible) from least privileged (e.g. {@link #MONITOR})
231+
* to most privileged (e.g. {@link #ALL})
232+
* @see #sortByAccessLevel(Collection)
233+
* @see org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission#check(String, TransportRequest, Authentication)
234+
*/
235+
public static Collection<String> findPrivilegesThatGrant(String action, TransportRequest request, Authentication authentication) {
236+
return VALUES.entrySet().stream()
237+
.filter(e -> e.getValue().permission().check(action, request, authentication))
238+
.map(Map.Entry::getKey)
239+
.collect(Collectors.toUnmodifiableList());
240+
}
241+
242+
/**
243+
* Sorts the collection of privileges from least-privilege to most-privilege (to the extent possible),
244+
* returning them in a sorted map keyed by name.
245+
*/
246+
static SortedMap<String, NamedClusterPrivilege> sortByAccessLevel(Collection<NamedClusterPrivilege> privileges) {
247+
// How many other privileges does this privilege imply. Those with a higher count are considered to be a higher privilege
248+
final Map<String, Long> impliesCount = new HashMap<>(privileges.size());
249+
privileges.forEach(priv -> impliesCount.put(priv.name(),
250+
privileges.stream().filter(p2 -> p2 != priv && priv.permission().implies(p2.permission())).count())
251+
);
252+
253+
final Comparator<String> compare = Comparator.<String>comparingLong(key -> impliesCount.getOrDefault(key, 0L))
254+
.thenComparing(Comparator.naturalOrder());
255+
final TreeMap<String, NamedClusterPrivilege> tree = new TreeMap<>(compare);
256+
privileges.forEach(p -> tree.put(p.name(), p));
257+
return Collections.unmodifiableSortedMap(tree);
258+
}
259+
222260
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege {
2828
private static final String PRIVILEGE_NAME = "manage_own_api_key";
2929
private static final String API_KEY_ID_KEY = "_security_api_key_id";
3030

31+
private final ClusterPermission permission;
32+
3133
private ManageOwnApiKeyClusterPrivilege() {
34+
permission = this.buildPermission(ClusterPermission.builder()).build();
3235
}
3336

3437
@Override
@@ -41,6 +44,11 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build
4144
return builder.add(this, ManageOwnClusterPermissionCheck.INSTANCE);
4245
}
4346

47+
@Override
48+
public ClusterPermission permission() {
49+
return permission;
50+
}
51+
4452
private static final class ManageOwnClusterPermissionCheck extends ClusterPermission.ActionBasedPermissionCheck {
4553
public static final ManageOwnClusterPermissionCheck INSTANCE = new ManageOwnClusterPermissionCheck();
4654

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,24 @@
99
package org.elasticsearch.xpack.core.security.authz.privilege;
1010

1111
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
12+
import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission;
1213

1314
/**
1415
* A {@link ClusterPrivilege} that has a name. The named cluster privileges can be referred simply by name within a
1516
* {@link RoleDescriptor#getClusterPrivileges()}.
1617
*/
1718
public interface NamedClusterPrivilege extends ClusterPrivilege {
1819
String name();
20+
21+
/**
22+
* Returns a permission that represents this privilege only.
23+
* When building a role (or role-like object) that has many privileges, it is more efficient to build a shared permission using the
24+
* {@link #buildPermission(ClusterPermission.Builder)} method instead. This method is intended to allow callers to interrogate the
25+
* runtime permissions specifically granted by this privilege.
26+
* It is acceptable (and encouraged) for implementations of this method to cache (or precompute) the {@link ClusterPermission}
27+
* and return the same object on each call.
28+
* @see #buildPermission(ClusterPermission.Builder)
29+
*/
30+
ClusterPermission permission();
31+
1932
}

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege;
1717
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver;
1818
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
19+
import org.elasticsearch.xpack.core.security.authz.privilege.NamedClusterPrivilege;
1920
import org.junit.Before;
2021

2122
import java.io.IOException;
23+
import java.util.List;
2224
import java.util.Objects;
2325
import java.util.Set;
2426
import java.util.function.Predicate;
27+
import java.util.stream.Collectors;
2528

2629
import static org.hamcrest.Matchers.containsInAnyOrder;
2730
import static org.hamcrest.Matchers.is;
@@ -230,6 +233,33 @@ public void testClusterPermissionSubsetIsImpliedByAllClusterPermission() {
230233
assertThat(allClusterPermission.implies(otherClusterPermission), is(true));
231234
}
232235

236+
public void testImpliesOnSecurityPrivilegeHierarchy() {
237+
final List<ClusterPermission> highToLow = List.of(
238+
ClusterPrivilegeResolver.ALL.permission(),
239+
ClusterPrivilegeResolver.MANAGE_SECURITY.permission(),
240+
ClusterPrivilegeResolver.MANAGE_API_KEY.permission(),
241+
ClusterPrivilegeResolver.MANAGE_OWN_API_KEY.permission()
242+
);
243+
244+
for (int i = 0; i < highToLow.size(); i++) {
245+
ClusterPermission high = highToLow.get(i);
246+
for (int j = i; j < highToLow.size(); j++) {
247+
ClusterPermission low = highToLow.get(j);
248+
assertThat("Permission " + name(high) + " should imply " + name(low), high.implies(low), is(true));
249+
}
250+
}
251+
}
252+
253+
private String name(ClusterPermission permission) {
254+
return permission.privileges().stream().map(priv -> {
255+
if (priv instanceof NamedClusterPrivilege) {
256+
return ((NamedClusterPrivilege) priv).name();
257+
} else {
258+
return priv.toString();
259+
}
260+
}).collect(Collectors.joining(","));
261+
}
262+
233263
private static class MockConfigurableClusterPrivilege implements ConfigurableClusterPrivilege {
234264
private Predicate<TransportRequest> requestPredicate;
235265

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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.test.ESTestCase;
10+
11+
import java.util.ArrayList;
12+
import java.util.Collections;
13+
import java.util.List;
14+
import java.util.SortedMap;
15+
16+
import static org.hamcrest.Matchers.contains;
17+
18+
public class ClusterPrivilegeResolverTests extends ESTestCase {
19+
20+
public void testSortByAccessLevel() throws Exception {
21+
final List<NamedClusterPrivilege> privileges = new ArrayList<>(List.of(
22+
ClusterPrivilegeResolver.ALL,
23+
ClusterPrivilegeResolver.MONITOR,
24+
ClusterPrivilegeResolver.MANAGE,
25+
ClusterPrivilegeResolver.MANAGE_OWN_API_KEY,
26+
ClusterPrivilegeResolver.MANAGE_API_KEY,
27+
ClusterPrivilegeResolver.MANAGE_SECURITY
28+
));
29+
Collections.shuffle(privileges, random());
30+
final SortedMap<String, NamedClusterPrivilege> sorted = ClusterPrivilegeResolver.sortByAccessLevel(privileges);
31+
// This is:
32+
// "manage_own_api_key", "monitor" (neither of which grant anything else in the list), sorted by name
33+
// "manage" and "manage_api_key",(which each grant 1 other privilege in the list), sorted by name
34+
// "manage_security" and "all", sorted by access level ("all" implies "manage_security")
35+
assertThat(sorted.keySet(), contains("manage_own_api_key", "monitor", "manage", "manage_api_key", "manage_security", "all"));
36+
}
37+
38+
}

0 commit comments

Comments
 (0)