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.

Sign up for GitHub

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 support for authentication based predicate for cluster permission #45431

Merged
merged 12 commits into from
Aug 22, 2019
Merged

Add support for authentication based predicate for cluster permission #45431

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 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
66fc5b3
Merge branch 'manage-own-api-key-privilege' into moap-authentication-…
Aug 21, 2019
de88e11
Merge branch 'manage-own-api-key-privilege' into moap-authentication-…
Aug 21, 2019
96661a2
address review comments
Aug 22, 2019
cf18cf0
address review comments
Aug 22, 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
58 changes: 33 additions & 25 deletions ...c/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java
View file
Open in desktop
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) {
bizybot marked this conversation as resolved.
Show resolved Hide resolved
final Automaton allowedAutomaton = Automatons.patterns(allowedActionPatterns);
final Automaton excludedAutomaton = Automatons.patterns(excludeActionPatterns);
bizybot marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -156,7 +162,7 @@ private static class AutomatonPermissionCheck implements PermissionCheck {
}

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

Expand All @@ -169,29 +175,31 @@ public boolean implies(final PermissionCheck permissionCheck) {
}
}

// action and request based permission check
private static class ActionRequestPredicatePermissionCheck implements PermissionCheck {
// action, request based permission check
private static class ActionRequestBasedPermissionCheck extends AutomatonPermissionCheck {
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);
public boolean check(final String action, final TransportRequest request, final Authentication authentication) {
return super.check(action, request, authentication) && requestPredicate.test(request);
}

@Override
public boolean implies(final PermissionCheck permissionCheck) {
if (permissionCheck instanceof ActionRequestPredicatePermissionCheck) {
final ActionRequestPredicatePermissionCheck otherCheck = (ActionRequestPredicatePermissionCheck) permissionCheck;
return this.clusterPrivilege.equals(otherCheck.clusterPrivilege);
if (super.implies(permissionCheck)) {
if (permissionCheck instanceof ActionRequestBasedPermissionCheck) {
final ActionRequestBasedPermissionCheck otherCheck =
(ActionRequestBasedPermissionCheck) permissionCheck;
return this.clusterPrivilege.equals(otherCheck.clusterPrivilege);
}
}
return false;
}
Expand Down
10 changes: 7 additions & 3 deletions ...ore/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java
View file
Open in desktop
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
9 changes: 6 additions & 3 deletions ...lugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java
View file
Open in desktop
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
5 changes: 2 additions & 3 deletions .../org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java
View file
Open in desktop
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
45 changes: 26 additions & 19 deletions ...t/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.test.ESTestCase;
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.ClusterPrivilegeResolver;
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
import org.elasticsearch.xpack.core.security.support.Automatons;
import org.junit.Before;
import org.mockito.Mockito;

import java.io.IOException;
import java.util.Objects;
Expand All @@ -26,9 +25,11 @@

import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.is;
import static org.mockito.Mockito.mock;

public class ClusterPermissionTests extends ESTestCase {
private TransportRequest mockTransportRequest = Mockito.mock(TransportRequest.class);
private TransportRequest mockTransportRequest;
private Authentication mockAuthentication;
private ClusterPrivilege cpThatDoesNothing = new ClusterPrivilege() {
@Override
public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) {
Expand All @@ -38,7 +39,8 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build

@Before
public void setup() {
mockTransportRequest = Mockito.mock(TransportRequest.class);
mockTransportRequest = mock(TransportRequest.class);
mockAuthentication = mock(Authentication.class);
}

public void testClusterPermissionBuilder() {
Expand Down Expand Up @@ -78,46 +80,52 @@ public void testClusterPermissionCheck() {
builder = mockConfigurableClusterPrivilege2.buildPermission(builder);
final ClusterPermission clusterPermission = builder.build();

assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest), is(true));
assertThat(clusterPermission.check("cluster:admin/ilm/stop", mockTransportRequest), is(true));
assertThat(clusterPermission.check("cluster:admin/xpack/security/privilege/get", mockTransportRequest), is(true));
assertThat(clusterPermission.check("cluster:admin/snapshot/status", mockTransportRequest), is(false));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest, mockAuthentication),
is(true));
assertThat(clusterPermission.check("cluster:admin/ilm/stop", mockTransportRequest, mockAuthentication), is(true));
assertThat(clusterPermission.check("cluster:admin/xpack/security/privilege/get", mockTransportRequest, mockAuthentication),
is(true));
assertThat(clusterPermission.check("cluster:admin/snapshot/status", mockTransportRequest, mockAuthentication), is(false));
}

public void testClusterPermissionCheckWithEmptyActionPatterns() {
final ClusterPermission.Builder builder = ClusterPermission.builder();
builder.add(cpThatDoesNothing, Set.of(), Set.of());
final ClusterPermission clusterPermission = builder.build();

assertThat(clusterPermission.check("cluster:admin/ilm/start", mockTransportRequest), is(false));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest), is(false));
assertThat(clusterPermission.check("cluster:admin/ilm/start", mockTransportRequest, mockAuthentication), is(false));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest, mockAuthentication),
is(false));
}

public void testClusterPermissionCheckWithExcludeOnlyActionPatterns() {
final ClusterPermission.Builder builder = ClusterPermission.builder();
builder.add(cpThatDoesNothing, Set.of(), Set.of("cluster:some/thing/to/exclude"));
final ClusterPermission clusterPermission = builder.build();

assertThat(clusterPermission.check("cluster:admin/ilm/start", mockTransportRequest), is(false));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest), is(false));
assertThat(clusterPermission.check("cluster:admin/ilm/start", mockTransportRequest, mockAuthentication), is(false));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest, mockAuthentication),
is(false));
}

public void testClusterPermissionCheckWithActionPatterns() {
final ClusterPermission.Builder builder = ClusterPermission.builder();
builder.add(cpThatDoesNothing, Set.of("cluster:admin/*"), Set.of("cluster:admin/ilm/*"));
final ClusterPermission clusterPermission = builder.build();

assertThat(clusterPermission.check("cluster:admin/ilm/start", mockTransportRequest), is(false));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest), is(true));
assertThat(clusterPermission.check("cluster:admin/ilm/start", mockTransportRequest, mockAuthentication), is(false));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest, mockAuthentication),
is(true));
}

public void testClusterPermissionCheckWithActionPatternsAndNoExludePatterns() {
final ClusterPermission.Builder builder = ClusterPermission.builder();
builder.add(cpThatDoesNothing, Set.of("cluster:admin/*"), Set.of());
final ClusterPermission clusterPermission = builder.build();

assertThat(clusterPermission.check("cluster:admin/ilm/start", mockTransportRequest), is(true));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest), is(true));
assertThat(clusterPermission.check("cluster:admin/ilm/start", mockTransportRequest, mockAuthentication), is(true));
assertThat(clusterPermission.check("cluster:admin/xpack/security/token/invalidate", mockTransportRequest, mockAuthentication),
is(true));
}

public void testNoneClusterPermissionIsImpliedByNone() {
Expand Down Expand Up @@ -223,7 +231,6 @@ public void testClusterPermissionSubsetIsImpliedByAllClusterPermission() {
}

private static class MockConfigurableClusterPrivilege implements ConfigurableClusterPrivilege {
static final Predicate<String> ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/privilege/*");
private Predicate<TransportRequest> requestPredicate;

MockConfigurableClusterPrivilege(Predicate<TransportRequest> requestPredicate) {
Expand Down Expand Up @@ -269,13 +276,13 @@ public int hashCode() {
@Override
public String toString() {
return "MockConfigurableClusterPrivilege{" +
"requestPredicate=" + requestPredicate +
"requestAuthnPredicate=" + requestPredicate +
'}';
}

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