Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add manage_own_api_key cluster privilege #45696

Merged
merged 34 commits into from
Aug 23, 2019
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
80fa13f
Add support for authentication based predicate for cluster permission
Aug 7, 2019
60f436b
Merge branch 'manage-own-api-key-privilege' into moap-authentication-…
Aug 16, 2019
1af1308
address review comments
Aug 20, 2019
8896dcd
remove unwanted code
Aug 20, 2019
4976d78
remove unwanted code change, raise a separate pr for it
Aug 20, 2019
54490a7
fix tests
Aug 20, 2019
9033996
precommit error
Aug 20, 2019
541cfad
oh precommit, ran build on another branch, fixed it
Aug 20, 2019
aa623c3
Add `manage_own_api_key` cluster privilege
Aug 13, 2019
efc2c2b
add tests
Aug 16, 2019
7db504c
fix tests
Aug 19, 2019
adc6d69
change manage own api cluster privielge
Aug 20, 2019
2497c6f
precommit error
Aug 20, 2019
6bd259a
no need to override
Aug 20, 2019
66fc5b3
Merge branch 'manage-own-api-key-privilege' into moap-authentication-…
Aug 21, 2019
508a718
address review comment
Aug 21, 2019
29cefd2
add a base class for action based permission check to enforce checks
Aug 21, 2019
0eba55f
protected instead of public
Aug 21, 2019
de88e11
Merge branch 'manage-own-api-key-privilege' into moap-authentication-…
Aug 21, 2019
69b56c6
add unit test check for creator.realm
Aug 21, 2019
a94fa92
make the condition explicit
Aug 21, 2019
d5a295c
Merge branch 'moap-authentication-based-permission-check' into moap-m…
Aug 21, 2019
73498ea
address review comments
Aug 22, 2019
2f7933c
Merge branch 'manage-own-api-key-privilege' into moap-mo-cp
Aug 22, 2019
5c9c422
test name and test correctness
Aug 22, 2019
186e599
Merge branch 'manage-own-api-key-privilege' into moap-mo-cp
Aug 22, 2019
45b0192
remove extra blank lines
Aug 22, 2019
dc512c2
check owner flag in manage own cluster privilege, add tests
Aug 22, 2019
d740034
Update x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/…
bizybot Aug 22, 2019
7a9cfb7
fix compilation error
Aug 22, 2019
569cae6
Merge branch 'manage-own-api-key-privilege' into moap-mo-cp
Aug 22, 2019
1a2a371
address review comments
Aug 23, 2019
43cf4e3
static method instead of instance method
Aug 23, 2019
1e59c70
add unsupported class name to exception
Aug 23, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ A successful call returns an object with "cluster" and "index" fields.
"manage_ingest_pipelines",
"manage_ml",
"manage_oidc",
"manage_own_api_key",
"manage_pipeline",
"manage_rollup",
"manage_saml",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import org.apache.lucene.util.automaton.Automaton;
import org.apache.lucene.util.automaton.Operations;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege;
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
import org.elasticsearch.xpack.core.security.support.Automatons;

import java.util.ArrayList;
Expand All @@ -34,14 +34,16 @@ private ClusterPermission(final Set<ClusterPrivilege> clusterPrivileges,
}

/**
* Checks permission to a cluster action for a given request.
* Checks permission to a cluster action for a given request in the context of given
* authentication.
*
* @param action cluster action
* @param request {@link TransportRequest}
* @param authentication {@link Authentication}
* @return {@code true} if the access is allowed else returns {@code false}
*/
public boolean check(final String action, final TransportRequest request) {
return checks.stream().anyMatch(permission -> permission.check(action, request));
public boolean check(final String action, final TransportRequest request, final Authentication authentication) {
return checks.stream().anyMatch(permission -> permission.check(action, request, authentication));
}

/**
Expand Down Expand Up @@ -90,11 +92,13 @@ public Builder add(final ClusterPrivilege clusterPrivilege, final Set<String> al
return this;
}

public Builder add(final ConfigurableClusterPrivilege configurableClusterPrivilege, final Predicate<String> actionPredicate,
final Predicate<TransportRequest> requestPredicate) {
return add(configurableClusterPrivilege, new ActionRequestPredicatePermissionCheck(configurableClusterPrivilege,
actionPredicate,
requestPredicate));
public Builder add(final ClusterPrivilege clusterPrivilege, final Set<String> allowedActionPatterns,
final Set<String> excludeActionPatterns, final Predicate<TransportRequest> requestPredicate) {
final Automaton allowedAutomaton = Automatons.patterns(allowedActionPatterns);
final Automaton excludedAutomaton = Automatons.patterns(excludeActionPatterns);
final Automaton actionAutomaton = Automatons.minusAndMinimize(allowedAutomaton, excludedAutomaton);

return add(clusterPrivilege, new ActionRequestBasedPermissionCheck(clusterPrivilege, actionAutomaton, requestPredicate));
}

public Builder add(final ClusterPrivilege clusterPrivilege, final PermissionCheck permissionCheck) {
Expand Down Expand Up @@ -124,13 +128,15 @@ public ClusterPermission build() {
*/
public interface PermissionCheck {
/**
* Checks permission to a cluster action for a given request.
* Checks permission to a cluster action for a given request in the context of given
* authentication.
*
* @param action action name
* @param request {@link TransportRequest}
* @param authentication {@link Authentication}
* @return {@code true} if the specified action for given request is allowed else returns {@code false}
*/
boolean check(String action, TransportRequest request);
boolean check(String action, TransportRequest request, Authentication authentication);

/**
* Checks whether specified {@link PermissionCheck} is implied by this {@link PermissionCheck}.<br>
Expand All @@ -145,52 +151,80 @@ public interface PermissionCheck {
boolean implies(PermissionCheck otherPermissionCheck);
}

// Automaton based permission check
private static class AutomatonPermissionCheck implements PermissionCheck {
/**
* Base for implementing cluster action based {@link PermissionCheck}.
* It enforces the checks at cluster action level and then hands it off to the implementations
* to enforce checks based on {@link TransportRequest} and/or {@link Authentication}.
*/
public abstract static class ActionBasedPermissionCheck implements PermissionCheck {
private final Automaton automaton;
private final Predicate<String> actionPredicate;

AutomatonPermissionCheck(final Automaton automaton) {
public ActionBasedPermissionCheck(final Automaton automaton) {
this.automaton = automaton;
this.actionPredicate = Automatons.predicate(automaton);
}

@Override
public boolean check(final String action, final TransportRequest request) {
return actionPredicate.test(action);
public final boolean check(final String action, final TransportRequest request, final Authentication authentication) {
return actionPredicate.test(action) && doCheck(action, request, authentication);
}

protected abstract boolean doCheck(String action, TransportRequest request, Authentication authentication);

@Override
public boolean implies(final PermissionCheck permissionCheck) {
if (permissionCheck instanceof AutomatonPermissionCheck) {
return Operations.subsetOf(((AutomatonPermissionCheck) permissionCheck).automaton, this.automaton);
public final boolean implies(final PermissionCheck permissionCheck) {
if (permissionCheck instanceof ActionBasedPermissionCheck) {
return Operations.subsetOf(((ActionBasedPermissionCheck) permissionCheck).automaton, this.automaton) &&
doImplies(permissionCheck);
}
return false;
}

protected abstract boolean doImplies(PermissionCheck permissionCheck);
}

// Automaton based permission check
private static class AutomatonPermissionCheck extends ActionBasedPermissionCheck {

AutomatonPermissionCheck(final Automaton automaton) {
super(automaton);
}

@Override
protected boolean doCheck(String action, TransportRequest request, Authentication authentication) {
return true;
}

@Override
protected boolean doImplies(PermissionCheck permissionCheck) {
return permissionCheck instanceof AutomatonPermissionCheck;
}

}

// action and request based permission check
private static class ActionRequestPredicatePermissionCheck implements PermissionCheck {
// action, request based permission check
private static class ActionRequestBasedPermissionCheck extends ActionBasedPermissionCheck {
private final ClusterPrivilege clusterPrivilege;
final Predicate<String> actionPredicate;
final Predicate<TransportRequest> requestPredicate;
private final Predicate<TransportRequest> requestPredicate;

ActionRequestPredicatePermissionCheck(final ClusterPrivilege clusterPrivilege, final Predicate<String> actionPredicate,
final Predicate<TransportRequest> requestPredicate) {
this.clusterPrivilege = clusterPrivilege;
this.actionPredicate = actionPredicate;
ActionRequestBasedPermissionCheck(ClusterPrivilege clusterPrivilege, final Automaton automaton,
final Predicate<TransportRequest> requestPredicate) {
super(automaton);
this.requestPredicate = requestPredicate;
this.clusterPrivilege = clusterPrivilege;
}

@Override
public boolean check(final String action, final TransportRequest request) {
return actionPredicate.test(action) && requestPredicate.test(request);
protected boolean doCheck(String action, TransportRequest request, Authentication authentication) {
return requestPredicate.test(request);
}

@Override
public boolean implies(final PermissionCheck permissionCheck) {
if (permissionCheck instanceof ActionRequestPredicatePermissionCheck) {
final ActionRequestPredicatePermissionCheck otherCheck = (ActionRequestPredicatePermissionCheck) permissionCheck;
protected boolean doImplies(final PermissionCheck permissionCheck) {
if (permissionCheck instanceof ActionRequestBasedPermissionCheck) {
final ActionRequestBasedPermissionCheck otherCheck =
(ActionRequestBasedPermissionCheck) permissionCheck;
return this.clusterPrivilege.equals(otherCheck.clusterPrivilege);
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.apache.lucene.util.automaton.Automaton;
import org.elasticsearch.cluster.metadata.AliasOrIndex;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor;
import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege;
Expand Down Expand Up @@ -122,15 +123,18 @@ public ResourcePrivilegesMap checkIndicesPrivileges(Set<String> checkForIndexPat
}

/**
* Check if cluster permissions allow for the given action, also checks whether the limited by role allows the given actions
* Check if cluster permissions allow for the given action,
* also checks whether the limited by role allows the given actions in the context of given
* authentication.
*
* @param action cluster action
* @param request {@link TransportRequest}
* @param authentication {@link Authentication}
* @return {@code true} if action is allowed else returns {@code false}
*/
@Override
public boolean checkClusterAction(String action, TransportRequest request) {
return super.checkClusterAction(action, request) && limitedBy.checkClusterAction(action, request);
public boolean checkClusterAction(String action, TransportRequest request, Authentication authentication) {
return super.checkClusterAction(action, request, authentication) && limitedBy.checkClusterAction(action, request, authentication);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl;
import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege;
Expand Down Expand Up @@ -121,14 +122,16 @@ public ResourcePrivilegesMap checkIndicesPrivileges(Set<String> checkForIndexPat
}

/**
* Check if cluster permissions allow for the given action
* Check if cluster permissions allow for the given action in the context of given
* authentication.
*
* @param action cluster action
* @param request {@link TransportRequest}
* @param authentication {@link Authentication}
* @return {@code true} if action is allowed else returns {@code false}
*/
public boolean checkClusterAction(String action, TransportRequest request) {
return cluster.check(action, request);
public boolean checkClusterAction(String action, TransportRequest request, Authentication authentication) {
return cluster.check(action, request, authentication);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ public class ClusterPrivilegeResolver {
public static final NamedClusterPrivilege MANAGE_SLM = new ActionClusterPrivilege("manage_slm", MANAGE_SLM_PATTERN);
public static final NamedClusterPrivilege READ_SLM = new ActionClusterPrivilege("read_slm", READ_SLM_PATTERN);

public static final NamedClusterPrivilege MANAGE_OWN_API_KEY = ManageOwnApiKeyClusterPrivilege.INSTANCE;

private static final Map<String, NamedClusterPrivilege> VALUES = Stream.of(
NONE,
ALL,
Expand Down Expand Up @@ -131,7 +133,8 @@ public class ClusterPrivilegeResolver {
MANAGE_ILM,
READ_ILM,
MANAGE_SLM,
READ_SLM).collect(Collectors.toUnmodifiableMap(NamedClusterPrivilege::name, Function.identity()));
READ_SLM,
MANAGE_OWN_API_KEY).collect(Collectors.toUnmodifiableMap(NamedClusterPrivilege::name, Function.identity()));

/**
* Resolves a {@link NamedClusterPrivilege} from a given name if it exists.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ private static void expectFieldName(XContentParser parser, ParseField... fields)
* of applications (identified by a wildcard-aware application-name).
*/
public static class ManageApplicationPrivileges implements ConfigurableClusterPrivilege {

private static final Predicate<String> ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/privilege/*");
public static final String WRITEABLE_NAME = "manage-application-privileges";

private final Set<String> applicationNames;
Expand All @@ -145,6 +143,7 @@ public ManageApplicationPrivileges(Set<String> applicationNames) {
}
return false;
};

}

@Override
Expand Down Expand Up @@ -215,7 +214,7 @@ public int hashCode() {

@Override
public ClusterPermission.Builder buildPermission(final ClusterPermission.Builder builder) {
return builder.add(this, ACTION_PREDICATE, requestPredicate);
return builder.add(this, Set.of("cluster:admin/xpack/security/privilege/*"), Set.of(), requestPredicate);
}

private interface Fields {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
*
* 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.common.Strings;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.GetApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.InvalidateApiKeyRequest;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission;
import org.elasticsearch.xpack.core.security.support.Automatons;

/**
* Named cluster privilege for managing API keys owned by the current authenticated user.
*/
public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege {
public static final ManageOwnApiKeyClusterPrivilege INSTANCE = new ManageOwnApiKeyClusterPrivilege();
private static final String PRIVILEGE_NAME = "manage_own_api_key";

private ManageOwnApiKeyClusterPrivilege() {
}

@Override
public String name() {
return PRIVILEGE_NAME;
}

@Override
public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) {
return builder.add(this, ManageOwnClusterPermissionCheck.INSTANCE);
}

private static final class ManageOwnClusterPermissionCheck extends ClusterPermission.ActionBasedPermissionCheck {
public static final ManageOwnClusterPermissionCheck INSTANCE = new ManageOwnClusterPermissionCheck();

private ManageOwnClusterPermissionCheck() {
super(Automatons.patterns("cluster:admin/xpack/security/api_key/*"));
}

@Override
protected boolean doCheck(String action, TransportRequest request, Authentication authentication) {
if (request instanceof CreateApiKeyRequest) {
return true;
} else if (request instanceof GetApiKeyRequest) {
final GetApiKeyRequest getApiKeyRequest = (GetApiKeyRequest) request;
return checkIfUserIsOwnerOfApiKeys(authentication, getApiKeyRequest.getApiKeyId(), getApiKeyRequest.getUserName(),
getApiKeyRequest.getRealmName());
} else if (request instanceof InvalidateApiKeyRequest) {
final InvalidateApiKeyRequest invalidateApiKeyRequest = (InvalidateApiKeyRequest) request;
return checkIfUserIsOwnerOfApiKeys(authentication, invalidateApiKeyRequest.getId(),
invalidateApiKeyRequest.getUserName(),
invalidateApiKeyRequest.getRealmName());
}
return false;
}

@Override
protected boolean doImplies(ClusterPermission.PermissionCheck permissionCheck) {
return permissionCheck instanceof ManageOwnClusterPermissionCheck;
}

private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, String apiKeyId, String username, String realmName) {
if (isCurrentAuthenticationUsingSameApiKeyIdFromRequest(authentication, apiKeyId)) {
return true;
} else {
/*
* TODO bizybot we need to think on how we can propagate appropriate error message to the end user when username, realm name
* is missing. This is similar to the problem of propagating right error messages in case of access denied.
*/
String authenticatedUserPrincipal = authentication.getUser().principal();
String authenticatedUserRealm = authentication.getAuthenticatedBy().getName();
if (Strings.hasText(username) && Strings.hasText(realmName)) {
return username.equals(authenticatedUserPrincipal) && realmName.equals(authenticatedUserRealm);
}
}
return false;
}

private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authentication authentication, String apiKeyId) {
if (authentication.getAuthenticatedBy().getType().equals("_es_api_key")) {
// API key id from authentication must match the id from request
String authenticatedApiKeyId = (String) authentication.getMetadata().get("_security_api_key_id");
if (Strings.hasText(apiKeyId)) {
return apiKeyId.equals(authenticatedApiKeyId);
}
}
return false;
}
}
}
Loading