From 80fa13faddf8cd68d01f31ec2e1e3c341d4f4610 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Wed, 7 Aug 2019 12:42:02 +1000 Subject: [PATCH 01/27] Add support for authentication based predicate for cluster permission Currently, cluster permission checks whether a cluster action is permitted and optionally in the context of a request. There are scenarios where we would want to check whether the cluster action is permitted, optionally in the context of a request and current authentication. For example, management of API keys is only restricted to the API keys owned by the current user. In this case, along with the cluster action and API key request, the check needs to perform whether the currently authenticated user is indeed allowed to operate only on owned API keys. With this commit, we are introducing one more context of the current authentication that can be considered during permission evaluation. Relates: #40031 --- .../authz/permission/ClusterPermission.java | 50 +- .../authz/permission/LimitedRole.java | 10 +- .../core/security/authz/permission/Role.java | 9 +- .../ConfigurableClusterPrivileges.java | 8 +- .../permission/ClusterPermissionTests.java | 74 ++- .../authz/permission/LimitedRoleTests.java | 25 +- .../ManageApplicationPrivilegesTests.java | 20 +- .../authz/privilege/PrivilegeTests.java | 7 +- .../authz/store/ReservedRolesStoreTests.java | 564 +++++++++--------- .../xpack/security/authz/RBACEngine.java | 2 +- .../authc/esnative/NativeRealmIntegTests.java | 6 +- .../authz/AuthorizationServiceTests.java | 5 +- .../authz/store/CompositeRolesStoreTests.java | 19 +- .../authz/store/FileRolesStoreTests.java | 6 +- 14 files changed, 434 insertions(+), 371 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index 6d6a01684760c..738ff0c895520 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -8,14 +8,15 @@ 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; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.BiPredicate; import java.util.function.Predicate; /** @@ -34,14 +35,16 @@ private ClusterPermission(final Set 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)); } /** @@ -90,11 +93,10 @@ public Builder add(final ClusterPrivilege clusterPrivilege, final Set al return this; } - public Builder add(final ConfigurableClusterPrivilege configurableClusterPrivilege, final Predicate actionPredicate, - final Predicate requestPredicate) { - return add(configurableClusterPrivilege, new ActionRequestPredicatePermissionCheck(configurableClusterPrivilege, - actionPredicate, - requestPredicate)); + public Builder add(final ClusterPrivilege clusterPrivilege, final Predicate actionPredicate, + final BiPredicate requestAuthnPredicate) { + return add(clusterPrivilege, new ActionRequestAuthenticationPredicatePermissionCheck(clusterPrivilege, + actionPredicate, requestAuthnPredicate)); } public Builder add(final ClusterPrivilege clusterPrivilege, final PermissionCheck permissionCheck) { @@ -124,13 +126,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}.
@@ -156,7 +160,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); } @@ -169,28 +173,30 @@ public boolean implies(final PermissionCheck permissionCheck) { } } - // action and request based permission check - private static class ActionRequestPredicatePermissionCheck implements PermissionCheck { + // action, request and authentication based permission check + private static class ActionRequestAuthenticationPredicatePermissionCheck implements PermissionCheck { private final ClusterPrivilege clusterPrivilege; final Predicate actionPredicate; - final Predicate requestPredicate; + final BiPredicate requestAuthnPredicate; - ActionRequestPredicatePermissionCheck(final ClusterPrivilege clusterPrivilege, final Predicate actionPredicate, - final Predicate requestPredicate) { + ActionRequestAuthenticationPredicatePermissionCheck(final ClusterPrivilege clusterPrivilege, + final Predicate actionPredicate, + final BiPredicate requestAuthnPredicate) { this.clusterPrivilege = clusterPrivilege; this.actionPredicate = actionPredicate; - this.requestPredicate = requestPredicate; + this.requestAuthnPredicate = requestAuthnPredicate; } @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 actionPredicate.test(action) && requestAuthnPredicate.test(request, authentication); } @Override public boolean implies(final PermissionCheck permissionCheck) { - if (permissionCheck instanceof ActionRequestPredicatePermissionCheck) { - final ActionRequestPredicatePermissionCheck otherCheck = (ActionRequestPredicatePermissionCheck) permissionCheck; + if (permissionCheck instanceof ActionRequestAuthenticationPredicatePermissionCheck) { + final ActionRequestAuthenticationPredicatePermissionCheck otherCheck = + (ActionRequestAuthenticationPredicatePermissionCheck) permissionCheck; return this.clusterPrivilege.equals(otherCheck.clusterPrivilege); } return false; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java index 8c7491d0a9a3d..871be8cbc6569 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRole.java @@ -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; @@ -122,15 +123,18 @@ public ResourcePrivilegesMap checkIndicesPrivileges(Set 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); } /** diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java index ef898a0876dda..94d583f616787 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java @@ -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; @@ -121,14 +122,16 @@ public ResourcePrivilegesMap checkIndicesPrivileges(Set 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); } /** diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java index 22ba4c1f2e33a..68cdfc3d1f836 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.xpack.core.security.action.privilege.ApplicationPrivilegesRequest; +import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege.Category; import org.elasticsearch.xpack.core.security.support.Automatons; @@ -30,6 +31,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.function.BiPredicate; import java.util.function.Predicate; /** @@ -131,12 +133,12 @@ public static class ManageApplicationPrivileges implements ConfigurableClusterPr private final Set applicationNames; private final Predicate applicationPredicate; - private final Predicate requestPredicate; + private final BiPredicate requestAuthnPredicate; public ManageApplicationPrivileges(Set applicationNames) { this.applicationNames = Collections.unmodifiableSet(applicationNames); this.applicationPredicate = Automatons.predicate(applicationNames); - this.requestPredicate = request -> { + this.requestAuthnPredicate = (request, authn) -> { if (request instanceof ApplicationPrivilegesRequest) { final ApplicationPrivilegesRequest privRequest = (ApplicationPrivilegesRequest) request; final Collection requestApplicationNames = privRequest.getApplicationNames(); @@ -215,7 +217,7 @@ public int hashCode() { @Override public ClusterPermission.Builder buildPermission(final ClusterPermission.Builder builder) { - return builder.add(this, ACTION_PREDICATE, requestPredicate); + return builder.add(this, ACTION_PREDICATE, requestAuthnPredicate); } private interface Fields { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java index 18eb99e97f2f7..4fe368f5e33a1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java @@ -12,23 +12,26 @@ 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; import java.util.Set; +import java.util.function.BiPredicate; import java.util.function.Predicate; 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) { @@ -38,7 +41,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() { @@ -49,9 +53,9 @@ public void testClusterPermissionBuilder() { builder = ClusterPrivilegeResolver.MANAGE_SECURITY.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = - new MockConfigurableClusterPrivilege(r -> false); + new MockConfigurableClusterPrivilege((r,a) -> false); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); builder = mockConfigurableClusterPrivilege2.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -71,17 +75,19 @@ public void testClusterPermissionCheck() { builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = - new MockConfigurableClusterPrivilege(r -> false); + new MockConfigurableClusterPrivilege((r,a) -> false); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); 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() { @@ -89,8 +95,9 @@ public void testClusterPermissionCheckWithEmptyActionPatterns() { 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() { @@ -98,8 +105,9 @@ public void testClusterPermissionCheckWithExcludeOnlyActionPatterns() { 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() { @@ -107,8 +115,9 @@ public void testClusterPermissionCheckWithActionPatterns() { 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() { @@ -116,8 +125,9 @@ public void testClusterPermissionCheckWithActionPatternsAndNoExludePatterns() { 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() { @@ -129,9 +139,9 @@ public void testNoneClusterPermissionIsImpliedByAny() { builder = ClusterPrivilegeResolver.MANAGE_SECURITY.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = - new MockConfigurableClusterPrivilege(r -> false); + new MockConfigurableClusterPrivilege((r,a) -> false); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); builder = mockConfigurableClusterPrivilege2.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -144,7 +154,7 @@ public void testClusterPermissionSubsetWithConfigurableClusterPrivilegeIsImplied builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -161,7 +171,7 @@ public void testClusterPermissionNonSubsetWithConfigurableClusterPrivilegeIsImpl builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -169,7 +179,7 @@ public void testClusterPermissionNonSubsetWithConfigurableClusterPrivilegeIsImpl builder1 = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder1); builder1 = mockConfigurableClusterPrivilege1.buildPermission(builder1); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = - new MockConfigurableClusterPrivilege(r -> false); + new MockConfigurableClusterPrivilege((r,a) -> false); builder1 = mockConfigurableClusterPrivilege2.buildPermission(builder1); final ClusterPermission otherClusterPermission = builder1.build(); @@ -207,7 +217,7 @@ public void testClusterPermissionIsImpliedBySameClusterPermission() { builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -224,10 +234,10 @@ public void testClusterPermissionSubsetIsImpliedByAllClusterPermission() { private static class MockConfigurableClusterPrivilege implements ConfigurableClusterPrivilege { static final Predicate ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/privilege/*"); - private Predicate requestPredicate; + private BiPredicate requestAuthnPredicate; - MockConfigurableClusterPrivilege(Predicate requestPredicate) { - this.requestPredicate = requestPredicate; + MockConfigurableClusterPrivilege(BiPredicate requestPredicate) { + this.requestAuthnPredicate = requestPredicate; } @Override @@ -258,24 +268,24 @@ public boolean equals(Object o) { return false; } final MockConfigurableClusterPrivilege that = (MockConfigurableClusterPrivilege) o; - return requestPredicate.equals(that.requestPredicate); + return requestAuthnPredicate.equals(that.requestAuthnPredicate); } @Override public int hashCode() { - return Objects.hash(requestPredicate); + return Objects.hash(requestAuthnPredicate); } @Override public String toString() { return "MockConfigurableClusterPrivilege{" + - "requestPredicate=" + requestPredicate + + "requestAuthnPredicate=" + requestAuthnPredicate + '}'; } @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - return builder.add(this, ACTION_PREDICATE, requestPredicate); + return builder.add(this, ACTION_PREDICATE, requestAuthnPredicate); } } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java index 4bcc581d072b1..74e06d1cbce25 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/LimitedRoleTests.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.util.set.Sets; 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.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.ApplicationPrivilegeDescriptor; @@ -128,24 +129,26 @@ public void testAuthorize() { public void testCheckClusterAction() { Role fromRole = Role.builder("a-role").cluster(Collections.singleton("manage_security"), Collections.emptyList()) - .build(); - assertThat(fromRole.checkClusterAction("cluster:admin/xpack/security/x", mock(TransportRequest.class)), is(true)); + .build(); + Authentication authentication = mock(Authentication.class); + assertThat(fromRole.checkClusterAction("cluster:admin/xpack/security/x", mock(TransportRequest.class), authentication), is(true)); { Role limitedByRole = Role.builder("limited-role") - .cluster(Collections.singleton("all"), Collections.emptyList()).build(); - assertThat(limitedByRole.checkClusterAction("cluster:admin/xpack/security/x", mock(TransportRequest.class)), is(true)); - assertThat(limitedByRole.checkClusterAction("cluster:other-action", mock(TransportRequest.class)), is(true)); + .cluster(Collections.singleton("all"), Collections.emptyList()).build(); + assertThat(limitedByRole.checkClusterAction("cluster:admin/xpack/security/x", mock(TransportRequest.class), authentication), + is(true)); + assertThat(limitedByRole.checkClusterAction("cluster:other-action", mock(TransportRequest.class), authentication), is(true)); Role role = LimitedRole.createLimitedRole(fromRole, limitedByRole); - assertThat(role.checkClusterAction("cluster:admin/xpack/security/x", mock(TransportRequest.class)), is(true)); - assertThat(role.checkClusterAction("cluster:other-action", mock(TransportRequest.class)), is(false)); + assertThat(role.checkClusterAction("cluster:admin/xpack/security/x", mock(TransportRequest.class), authentication), is(true)); + assertThat(role.checkClusterAction("cluster:other-action", mock(TransportRequest.class), authentication), is(false)); } { Role limitedByRole = Role.builder("limited-role") - .cluster(Collections.singleton("monitor"), Collections.emptyList()).build(); - assertThat(limitedByRole.checkClusterAction("cluster:monitor/me", mock(TransportRequest.class)), is(true)); + .cluster(Collections.singleton("monitor"), Collections.emptyList()).build(); + assertThat(limitedByRole.checkClusterAction("cluster:monitor/me", mock(TransportRequest.class), authentication), is(true)); Role role = LimitedRole.createLimitedRole(fromRole, limitedByRole); - assertThat(role.checkClusterAction("cluster:monitor/me", mock(TransportRequest.class)), is(false)); - assertThat(role.checkClusterAction("cluster:admin/xpack/security/x", mock(TransportRequest.class)), is(false)); + assertThat(role.checkClusterAction("cluster:monitor/me", mock(TransportRequest.class), authentication), is(false)); + assertThat(role.checkClusterAction("cluster:admin/xpack/security/x", mock(TransportRequest.class), authentication), is(false)); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageApplicationPrivilegesTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageApplicationPrivilegesTests.java index dfe1147fb2c43..10eea045aadab 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageApplicationPrivilegesTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageApplicationPrivilegesTests.java @@ -24,6 +24,7 @@ import org.elasticsearch.xpack.core.security.action.privilege.DeletePrivilegesRequest; import org.elasticsearch.xpack.core.security.action.privilege.GetPrivilegesRequest; import org.elasticsearch.xpack.core.security.action.privilege.PutPrivilegesRequest; +import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges.ManageApplicationPrivileges; @@ -40,6 +41,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.mock; public class ManageApplicationPrivilegesTests extends ESTestCase { @@ -97,14 +99,15 @@ public void testActionAndRequestPredicate() { assertThat(kibanaAndLogstashPermission, notNullValue()); assertThat(cloudAndSwiftypePermission, notNullValue()); + final Authentication authentication = mock(Authentication.class); final GetPrivilegesRequest getKibana1 = new GetPrivilegesRequest(); getKibana1.application("kibana-1"); - assertTrue(kibanaAndLogstashPermission.check("cluster:admin/xpack/security/privilege/get", getKibana1)); - assertFalse(cloudAndSwiftypePermission.check("cluster:admin/xpack/security/privilege/get", getKibana1)); + assertTrue(kibanaAndLogstashPermission.check("cluster:admin/xpack/security/privilege/get", getKibana1, authentication)); + assertFalse(cloudAndSwiftypePermission.check("cluster:admin/xpack/security/privilege/get", getKibana1, authentication)); final DeletePrivilegesRequest deleteLogstash = new DeletePrivilegesRequest("logstash", new String[]{"all"}); - assertTrue(kibanaAndLogstashPermission.check("cluster:admin/xpack/security/privilege/get", deleteLogstash)); - assertFalse(cloudAndSwiftypePermission.check("cluster:admin/xpack/security/privilege/get", deleteLogstash)); + assertTrue(kibanaAndLogstashPermission.check("cluster:admin/xpack/security/privilege/get", deleteLogstash, authentication)); + assertFalse(cloudAndSwiftypePermission.check("cluster:admin/xpack/security/privilege/get", deleteLogstash, authentication)); final PutPrivilegesRequest putKibana = new PutPrivilegesRequest(); @@ -114,11 +117,12 @@ public void testActionAndRequestPredicate() { randomAlphaOfLengthBetween(3, 6).toLowerCase(Locale.ROOT), Collections.emptySet(), Collections.emptyMap())); } putKibana.setPrivileges(kibanaPrivileges); - assertTrue(kibanaAndLogstashPermission.check("cluster:admin/xpack/security/privilege/get", putKibana)); - assertFalse(cloudAndSwiftypePermission.check("cluster:admin/xpack/security/privilege/get", putKibana)); + assertTrue(kibanaAndLogstashPermission.check("cluster:admin/xpack/security/privilege/get", putKibana, authentication)); + assertFalse(cloudAndSwiftypePermission.check("cluster:admin/xpack/security/privilege/get", putKibana, authentication)); } public void testSecurityForGetAllApplicationPrivileges() { + final Authentication authentication = mock(Authentication.class); final GetPrivilegesRequest getAll = new GetPrivilegesRequest(); getAll.application(null); getAll.privileges(new String[0]); @@ -130,8 +134,8 @@ public void testSecurityForGetAllApplicationPrivileges() { final ClusterPermission kibanaOnlyPermission = kibanaOnly.buildPermission(ClusterPermission.builder()).build(); final ClusterPermission allAppsPermission = allApps.buildPermission(ClusterPermission.builder()).build(); - assertFalse(kibanaOnlyPermission.check("cluster:admin/xpack/security/privilege/get", getAll)); - assertTrue(allAppsPermission.check("cluster:admin/xpack/security/privilege/get", getAll)); + assertFalse(kibanaOnlyPermission.check("cluster:admin/xpack/security/privilege/get", getAll, authentication)); + assertTrue(allAppsPermission.check("cluster:admin/xpack/security/privilege/get", getAll, authentication)); } private ManageApplicationPrivileges clone(ManageApplicationPrivileges original) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java index bd64d2112287f..e02c930101694 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java @@ -9,17 +9,18 @@ import org.elasticsearch.common.util.set.Sets; 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.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.support.Automatons; import org.junit.Rule; import org.junit.rules.ExpectedException; -import org.mockito.Mockito; import java.util.Set; import java.util.function.Predicate; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.mockito.Mockito.mock; public class PrivilegeTests extends ESTestCase { @Rule @@ -35,13 +36,13 @@ public void testSubActionPattern() throws Exception { private void verifyClusterActionAllowed(ClusterPrivilege clusterPrivilege, String... actions) { ClusterPermission clusterPermission = clusterPrivilege.buildPermission(ClusterPermission.builder()).build(); for (String action: actions) { - assertTrue(clusterPermission.check(action, Mockito.mock(TransportRequest.class))); + assertTrue(clusterPermission.check(action, mock(TransportRequest.class), mock(Authentication.class))); } } private void verifyClusterActionDenied(ClusterPrivilege clusterPrivilege, String... actions) { ClusterPermission clusterPermission = clusterPrivilege.buildPermission(ClusterPermission.builder()).build(); for (String action: actions) { - assertFalse(clusterPermission.check(action, Mockito.mock(TransportRequest.class))); + assertFalse(clusterPermission.check(action, mock(TransportRequest.class), mock(Authentication.class))); } } public void testCluster() throws Exception { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java index 4433b9d3750e7..4e6509d2ea823 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java @@ -124,6 +124,7 @@ import org.elasticsearch.xpack.core.security.action.token.CreateTokenAction; import org.elasticsearch.xpack.core.security.action.token.InvalidateTokenAction; import org.elasticsearch.xpack.core.security.action.user.PutUserAction; +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.IndexAccessControl; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsCache; @@ -200,33 +201,34 @@ public void testIsReserved() { public void testSnapshotUserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("snapshot_user"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role snapshotUserRole = Role.builder(roleDescriptor, null).build(); - assertThat(snapshotUserRole.cluster().check(GetRepositoriesAction.NAME, request), is(true)); - assertThat(snapshotUserRole.cluster().check(CreateSnapshotAction.NAME, request), is(true)); - assertThat(snapshotUserRole.cluster().check(SnapshotsStatusAction.NAME, request), is(true)); - assertThat(snapshotUserRole.cluster().check(GetSnapshotsAction.NAME, request), is(true)); - - assertThat(snapshotUserRole.cluster().check(PutRepositoryAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(GetIndexTemplatesAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(DeleteIndexTemplateAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(PutPipelineAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(GetPipelineAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(DeletePipelineAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(GetWatchAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(PutWatchAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(DeleteWatchAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(ExecuteWatchAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(AckWatchAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(ActivateWatchAction.NAME, request), is(false)); - assertThat(snapshotUserRole.cluster().check(WatcherServiceAction.NAME, request), is(false)); + assertThat(snapshotUserRole.cluster().check(GetRepositoriesAction.NAME, request, authentication), is(true)); + assertThat(snapshotUserRole.cluster().check(CreateSnapshotAction.NAME, request, authentication), is(true)); + assertThat(snapshotUserRole.cluster().check(SnapshotsStatusAction.NAME, request, authentication), is(true)); + assertThat(snapshotUserRole.cluster().check(GetSnapshotsAction.NAME, request, authentication), is(true)); + + assertThat(snapshotUserRole.cluster().check(PutRepositoryAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(GetIndexTemplatesAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(DeleteIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(PutPipelineAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(GetPipelineAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(DeletePipelineAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(GetWatchAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(PutWatchAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(DeleteWatchAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(ExecuteWatchAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(AckWatchAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(ActivateWatchAction.NAME, request, authentication), is(false)); + assertThat(snapshotUserRole.cluster().check(WatcherServiceAction.NAME, request, authentication), is(false)); assertThat(snapshotUserRole.indices().allowedIndicesMatcher(IndexAction.NAME).test(randomAlphaOfLengthBetween(8, 24)), is(false)); assertThat(snapshotUserRole.indices().allowedIndicesMatcher("indices:foo").test(randomAlphaOfLengthBetween(8, 24)), is(false)); @@ -247,22 +249,23 @@ public void testSnapshotUserRole() { public void testIngestAdminRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("ingest_admin"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role ingestAdminRole = Role.builder(roleDescriptor, null).build(); - assertThat(ingestAdminRole.cluster().check(PutIndexTemplateAction.NAME, request), is(true)); - assertThat(ingestAdminRole.cluster().check(GetIndexTemplatesAction.NAME, request), is(true)); - assertThat(ingestAdminRole.cluster().check(DeleteIndexTemplateAction.NAME, request), is(true)); - assertThat(ingestAdminRole.cluster().check(PutPipelineAction.NAME, request), is(true)); - assertThat(ingestAdminRole.cluster().check(GetPipelineAction.NAME, request), is(true)); - assertThat(ingestAdminRole.cluster().check(DeletePipelineAction.NAME, request), is(true)); + assertThat(ingestAdminRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(true)); + assertThat(ingestAdminRole.cluster().check(GetIndexTemplatesAction.NAME, request, authentication), is(true)); + assertThat(ingestAdminRole.cluster().check(DeleteIndexTemplateAction.NAME, request, authentication), is(true)); + assertThat(ingestAdminRole.cluster().check(PutPipelineAction.NAME, request, authentication), is(true)); + assertThat(ingestAdminRole.cluster().check(GetPipelineAction.NAME, request, authentication), is(true)); + assertThat(ingestAdminRole.cluster().check(DeletePipelineAction.NAME, request, authentication), is(true)); - assertThat(ingestAdminRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(ingestAdminRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(ingestAdminRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); + assertThat(ingestAdminRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(ingestAdminRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(ingestAdminRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); assertThat(ingestAdminRole.indices().allowedIndicesMatcher(IndexAction.NAME).test("foo"), is(false)); assertThat(ingestAdminRole.indices().allowedIndicesMatcher("indices:foo").test(randomAlphaOfLengthBetween(8, 24)), @@ -275,39 +278,40 @@ public void testIngestAdminRole() { public void testKibanaSystemRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("kibana_system"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role kibanaRole = Role.builder(roleDescriptor, null).build(); - assertThat(kibanaRole.cluster().check(ClusterHealthAction.NAME, request), is(true)); - assertThat(kibanaRole.cluster().check(ClusterStateAction.NAME, request), is(true)); - assertThat(kibanaRole.cluster().check(ClusterStatsAction.NAME, request), is(true)); - assertThat(kibanaRole.cluster().check(PutIndexTemplateAction.NAME, request), is(true)); - assertThat(kibanaRole.cluster().check(GetIndexTemplatesAction.NAME, request), is(true)); - assertThat(kibanaRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(kibanaRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(kibanaRole.cluster().check(MonitoringBulkAction.NAME, request), is(true)); + assertThat(kibanaRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(true)); + assertThat(kibanaRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(true)); + assertThat(kibanaRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(true)); + assertThat(kibanaRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(true)); + assertThat(kibanaRole.cluster().check(GetIndexTemplatesAction.NAME, request, authentication), is(true)); + assertThat(kibanaRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(kibanaRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(kibanaRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(true)); // SAML and token - assertThat(kibanaRole.cluster().check(SamlPrepareAuthenticationAction.NAME, request), is(true)); - assertThat(kibanaRole.cluster().check(SamlAuthenticateAction.NAME, request), is(true)); - assertThat(kibanaRole.cluster().check(InvalidateTokenAction.NAME, request), is(true)); - assertThat(kibanaRole.cluster().check(CreateTokenAction.NAME, request), is(true)); + assertThat(kibanaRole.cluster().check(SamlPrepareAuthenticationAction.NAME, request, authentication), is(true)); + assertThat(kibanaRole.cluster().check(SamlAuthenticateAction.NAME, request, authentication), is(true)); + assertThat(kibanaRole.cluster().check(InvalidateTokenAction.NAME, request, authentication), is(true)); + assertThat(kibanaRole.cluster().check(CreateTokenAction.NAME, request, authentication), is(true)); // Application Privileges DeletePrivilegesRequest deleteKibanaPrivileges = new DeletePrivilegesRequest("kibana-.kibana", new String[]{ "all", "read" }); DeletePrivilegesRequest deleteLogstashPrivileges = new DeletePrivilegesRequest("logstash", new String[]{ "all", "read" }); - assertThat(kibanaRole.cluster().check(DeletePrivilegesAction.NAME, deleteKibanaPrivileges), is(true)); - assertThat(kibanaRole.cluster().check(DeletePrivilegesAction.NAME, deleteLogstashPrivileges), is(false)); + assertThat(kibanaRole.cluster().check(DeletePrivilegesAction.NAME, deleteKibanaPrivileges, authentication), is(true)); + assertThat(kibanaRole.cluster().check(DeletePrivilegesAction.NAME, deleteLogstashPrivileges, authentication), is(false)); GetPrivilegesRequest getKibanaPrivileges = new GetPrivilegesRequest(); getKibanaPrivileges.application("kibana-.kibana-sales"); GetPrivilegesRequest getApmPrivileges = new GetPrivilegesRequest(); getApmPrivileges.application("apm"); - assertThat(kibanaRole.cluster().check(GetPrivilegesAction.NAME, getKibanaPrivileges), is(true)); - assertThat(kibanaRole.cluster().check(GetPrivilegesAction.NAME, getApmPrivileges), is(false)); + assertThat(kibanaRole.cluster().check(GetPrivilegesAction.NAME, getKibanaPrivileges, authentication), is(true)); + assertThat(kibanaRole.cluster().check(GetPrivilegesAction.NAME, getApmPrivileges, authentication), is(false)); PutPrivilegesRequest putKibanaPrivileges = new PutPrivilegesRequest(); putKibanaPrivileges.setPrivileges(Collections.singletonList(new ApplicationPrivilegeDescriptor( @@ -315,10 +319,10 @@ public void testKibanaSystemRole() { PutPrivilegesRequest putSwiftypePrivileges = new PutPrivilegesRequest(); putSwiftypePrivileges.setPrivileges(Collections.singletonList(new ApplicationPrivilegeDescriptor( "swiftype-kibana" , "all", Collections.emptySet(), Collections.emptyMap()))); - assertThat(kibanaRole.cluster().check(PutPrivilegesAction.NAME, putKibanaPrivileges), is(true)); - assertThat(kibanaRole.cluster().check(PutPrivilegesAction.NAME, putSwiftypePrivileges), is(false)); + assertThat(kibanaRole.cluster().check(PutPrivilegesAction.NAME, putKibanaPrivileges, authentication), is(true)); + assertThat(kibanaRole.cluster().check(PutPrivilegesAction.NAME, putSwiftypePrivileges, authentication), is(false)); - assertThat(kibanaRole.cluster().check(GetBuiltinPrivilegesAction.NAME, request), is(true)); + assertThat(kibanaRole.cluster().check(GetBuiltinPrivilegesAction.NAME, request, authentication), is(true)); // Everything else assertThat(kibanaRole.runAs().check(randomAlphaOfLengthBetween(1, 12)), is(false)); @@ -378,19 +382,20 @@ public void testKibanaSystemRole() { public void testKibanaUserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("kibana_user"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role kibanaUserRole = Role.builder(roleDescriptor, null).build(); - assertThat(kibanaUserRole.cluster().check(ClusterHealthAction.NAME, request), is(false)); - assertThat(kibanaUserRole.cluster().check(ClusterStateAction.NAME, request), is(false)); - assertThat(kibanaUserRole.cluster().check(ClusterStatsAction.NAME, request), is(false)); - assertThat(kibanaUserRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(kibanaUserRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(kibanaUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(kibanaUserRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); + assertThat(kibanaUserRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(false)); + assertThat(kibanaUserRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(false)); + assertThat(kibanaUserRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(false)); + assertThat(kibanaUserRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(kibanaUserRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(kibanaUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(kibanaUserRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); assertThat(kibanaUserRole.runAs().check(randomAlphaOfLengthBetween(1, 12)), is(false)); @@ -415,21 +420,22 @@ public void testKibanaUserRole() { public void testMonitoringUserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("monitoring_user"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role monitoringUserRole = Role.builder(roleDescriptor, null).build(); - assertThat(monitoringUserRole.cluster().check(MainAction.NAME, request), is(true)); - assertThat(monitoringUserRole.cluster().check(XPackInfoAction.NAME, request), is(true)); - assertThat(monitoringUserRole.cluster().check(ClusterHealthAction.NAME, request), is(false)); - assertThat(monitoringUserRole.cluster().check(ClusterStateAction.NAME, request), is(false)); - assertThat(monitoringUserRole.cluster().check(ClusterStatsAction.NAME, request), is(false)); - assertThat(monitoringUserRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(monitoringUserRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(monitoringUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(monitoringUserRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); + assertThat(monitoringUserRole.cluster().check(MainAction.NAME, request, authentication), is(true)); + assertThat(monitoringUserRole.cluster().check(XPackInfoAction.NAME, request, authentication), is(true)); + assertThat(monitoringUserRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(false)); + assertThat(monitoringUserRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(false)); + assertThat(monitoringUserRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(false)); + assertThat(monitoringUserRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(monitoringUserRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(monitoringUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(monitoringUserRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); assertThat(monitoringUserRole.runAs().check(randomAlphaOfLengthBetween(1, 12)), is(false)); @@ -471,28 +477,29 @@ public void testMonitoringUserRole() { public void testRemoteMonitoringAgentRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("remote_monitoring_agent"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role remoteMonitoringAgentRole = Role.builder(roleDescriptor, null).build(); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterHealthAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterStateAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterStatsAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(PutIndexTemplateAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(GetWatchAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(PutWatchAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(DeleteWatchAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(ExecuteWatchAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(AckWatchAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(ActivateWatchAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(WatcherServiceAction.NAME, request), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(GetWatchAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(PutWatchAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(DeleteWatchAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(ExecuteWatchAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(AckWatchAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(ActivateWatchAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(WatcherServiceAction.NAME, request, authentication), is(false)); // we get this from the cluster:monitor privilege - assertThat(remoteMonitoringAgentRole.cluster().check(WatcherStatsAction.NAME, request), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(WatcherStatsAction.NAME, request, authentication), is(true)); assertThat(remoteMonitoringAgentRole.runAs().check(randomAlphaOfLengthBetween(1, 12)), is(false)); @@ -530,21 +537,22 @@ public void testRemoteMonitoringAgentRole() { public void testRemoteMonitoringCollectorRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("remote_monitoring_collector"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role remoteMonitoringAgentRole = Role.builder(roleDescriptor, null).build(); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterHealthAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterStateAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterStatsAction.NAME, request), is(true)); - assertThat(remoteMonitoringAgentRole.cluster().check(GetIndexTemplatesAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(DeleteIndexTemplateAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(remoteMonitoringAgentRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(true)); + assertThat(remoteMonitoringAgentRole.cluster().check(GetIndexTemplatesAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(DeleteIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(remoteMonitoringAgentRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); assertThat(remoteMonitoringAgentRole.runAs().check(randomAlphaOfLengthBetween(1, 12)), is(false)); @@ -628,19 +636,20 @@ private void assertMonitoringOnRestrictedIndices(Role role) { public void testReportingUserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("reporting_user"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role reportingUserRole = Role.builder(roleDescriptor, null).build(); - assertThat(reportingUserRole.cluster().check(ClusterHealthAction.NAME, request), is(false)); - assertThat(reportingUserRole.cluster().check(ClusterStateAction.NAME, request), is(false)); - assertThat(reportingUserRole.cluster().check(ClusterStatsAction.NAME, request), is(false)); - assertThat(reportingUserRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(reportingUserRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(reportingUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(reportingUserRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); + assertThat(reportingUserRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(false)); + assertThat(reportingUserRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(false)); + assertThat(reportingUserRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(false)); + assertThat(reportingUserRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(reportingUserRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(reportingUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(reportingUserRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); assertThat(reportingUserRole.runAs().check(randomAlphaOfLengthBetween(1, 12)), is(false)); @@ -668,19 +677,20 @@ public void testReportingUserRole() { public void testKibanaDashboardOnlyUserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("kibana_dashboard_only_user"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role dashboardsOnlyUserRole = Role.builder(roleDescriptor, null).build(); - assertThat(dashboardsOnlyUserRole.cluster().check(ClusterHealthAction.NAME, request), is(false)); - assertThat(dashboardsOnlyUserRole.cluster().check(ClusterStateAction.NAME, request), is(false)); - assertThat(dashboardsOnlyUserRole.cluster().check(ClusterStatsAction.NAME, request), is(false)); - assertThat(dashboardsOnlyUserRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(dashboardsOnlyUserRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(dashboardsOnlyUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(dashboardsOnlyUserRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); + assertThat(dashboardsOnlyUserRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(false)); + assertThat(dashboardsOnlyUserRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(false)); + assertThat(dashboardsOnlyUserRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(false)); + assertThat(dashboardsOnlyUserRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(dashboardsOnlyUserRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(dashboardsOnlyUserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(dashboardsOnlyUserRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); assertThat(dashboardsOnlyUserRole.runAs().check(randomAlphaOfLengthBetween(1, 12)), is(false)); @@ -702,18 +712,19 @@ public void testKibanaDashboardOnlyUserRole() { public void testSuperuserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("superuser"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role superuserRole = Role.builder(roleDescriptor, null).build(); - assertThat(superuserRole.cluster().check(ClusterHealthAction.NAME, request), is(true)); - assertThat(superuserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(true)); - assertThat(superuserRole.cluster().check(PutUserAction.NAME, request), is(true)); - assertThat(superuserRole.cluster().check(PutRoleAction.NAME, request), is(true)); - assertThat(superuserRole.cluster().check(PutIndexTemplateAction.NAME, request), is(true)); - assertThat(superuserRole.cluster().check("internal:admin/foo", request), is(false)); + assertThat(superuserRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(true)); + assertThat(superuserRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(true)); + assertThat(superuserRole.cluster().check(PutUserAction.NAME, request, authentication), is(true)); + assertThat(superuserRole.cluster().check(PutRoleAction.NAME, request, authentication), is(true)); + assertThat(superuserRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(true)); + assertThat(superuserRole.cluster().check("internal:admin/foo", request, authentication), is(false)); final Settings indexSettings = Settings.builder().put("index.version.created", Version.CURRENT).build(); final String internalSecurityIndex = randomFrom(RestrictedIndicesNames.INTERNAL_SECURITY_MAIN_INDEX_6, @@ -772,19 +783,20 @@ public void testSuperuserRole() { public void testLogstashSystemRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("logstash_system"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role logstashSystemRole = Role.builder(roleDescriptor, null).build(); - assertThat(logstashSystemRole.cluster().check(ClusterHealthAction.NAME, request), is(true)); - assertThat(logstashSystemRole.cluster().check(ClusterStateAction.NAME, request), is(true)); - assertThat(logstashSystemRole.cluster().check(ClusterStatsAction.NAME, request), is(true)); - assertThat(logstashSystemRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(logstashSystemRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(logstashSystemRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(logstashSystemRole.cluster().check(MonitoringBulkAction.NAME, request), is(true)); + assertThat(logstashSystemRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(true)); + assertThat(logstashSystemRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(true)); + assertThat(logstashSystemRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(true)); + assertThat(logstashSystemRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(logstashSystemRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(logstashSystemRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(logstashSystemRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(true)); assertThat(logstashSystemRole.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); @@ -798,6 +810,7 @@ public void testLogstashSystemRole() { public void testBeatsAdminRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); final RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("beats_admin"); assertNotNull(roleDescriptor); @@ -805,13 +818,13 @@ public void testBeatsAdminRole() { final Role beatsAdminRole = Role.builder(roleDescriptor, null).build(); - assertThat(beatsAdminRole.cluster().check(ClusterHealthAction.NAME, request), is(false)); - assertThat(beatsAdminRole.cluster().check(ClusterStateAction.NAME, request), is(false)); - assertThat(beatsAdminRole.cluster().check(ClusterStatsAction.NAME, request), is(false)); - assertThat(beatsAdminRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(beatsAdminRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(beatsAdminRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(beatsAdminRole.cluster().check(MonitoringBulkAction.NAME, request), is(false)); + assertThat(beatsAdminRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(false)); + assertThat(beatsAdminRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(false)); + assertThat(beatsAdminRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(false)); + assertThat(beatsAdminRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(beatsAdminRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(beatsAdminRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(beatsAdminRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(false)); assertThat(beatsAdminRole.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); @@ -836,19 +849,20 @@ public void testBeatsAdminRole() { public void testBeatsSystemRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor(BeatsSystemUser.ROLE_NAME); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role beatsSystemRole = Role.builder(roleDescriptor, null).build(); - assertThat(beatsSystemRole.cluster().check(ClusterHealthAction.NAME, request), is(true)); - assertThat(beatsSystemRole.cluster().check(ClusterStateAction.NAME, request), is(true)); - assertThat(beatsSystemRole.cluster().check(ClusterStatsAction.NAME, request), is(true)); - assertThat(beatsSystemRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(beatsSystemRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(beatsSystemRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(beatsSystemRole.cluster().check(MonitoringBulkAction.NAME, request), is(true)); + assertThat(beatsSystemRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(true)); + assertThat(beatsSystemRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(true)); + assertThat(beatsSystemRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(true)); + assertThat(beatsSystemRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(beatsSystemRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(beatsSystemRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(beatsSystemRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(true)); assertThat(beatsSystemRole.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); @@ -869,19 +883,20 @@ public void testBeatsSystemRole() { public void testAPMSystemRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor(APMSystemUser.ROLE_NAME); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role APMSystemRole = Role.builder(roleDescriptor, null).build(); - assertThat(APMSystemRole.cluster().check(ClusterHealthAction.NAME, request), is(true)); - assertThat(APMSystemRole.cluster().check(ClusterStateAction.NAME, request), is(true)); - assertThat(APMSystemRole.cluster().check(ClusterStatsAction.NAME, request), is(true)); - assertThat(APMSystemRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(APMSystemRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(APMSystemRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); - assertThat(APMSystemRole.cluster().check(MonitoringBulkAction.NAME, request), is(true)); + assertThat(APMSystemRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(true)); + assertThat(APMSystemRole.cluster().check(ClusterStateAction.NAME, request, authentication), is(true)); + assertThat(APMSystemRole.cluster().check(ClusterStatsAction.NAME, request, authentication), is(true)); + assertThat(APMSystemRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(APMSystemRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(APMSystemRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); + assertThat(APMSystemRole.cluster().check(MonitoringBulkAction.NAME, request, authentication), is(true)); assertThat(APMSystemRole.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); @@ -912,62 +927,63 @@ public void testAPMUserRole() { public void testMachineLearningAdminRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("machine_learning_admin"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role role = Role.builder(roleDescriptor, null).build(); - assertThat(role.cluster().check(CloseJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteCalendarAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteCalendarEventAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteDatafeedAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteExpiredDataAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteFilterAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteForecastAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteModelSnapshotAction.NAME, request), is(true)); - assertThat(role.cluster().check(FinalizeJobExecutionAction.NAME, request), is(false)); // internal use only - assertThat(role.cluster().check(FindFileStructureAction.NAME, request), is(true)); - assertThat(role.cluster().check(FlushJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(ForecastJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetBucketsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetCalendarEventsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetCalendarsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetCategoriesAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetDatafeedsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetDatafeedsStatsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetFiltersAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetInfluencersAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetJobsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetJobsStatsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetModelSnapshotsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetOverallBucketsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetRecordsAction.NAME, request), is(true)); - assertThat(role.cluster().check(IsolateDatafeedAction.NAME, request), is(false)); // internal use only - assertThat(role.cluster().check(KillProcessAction.NAME, request), is(false)); // internal use only - assertThat(role.cluster().check(MlInfoAction.NAME, request), is(true)); - assertThat(role.cluster().check(OpenJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(PersistJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(PostCalendarEventsAction.NAME, request), is(true)); - assertThat(role.cluster().check(PostDataAction.NAME, request), is(true)); - assertThat(role.cluster().check(PreviewDatafeedAction.NAME, request), is(true)); - assertThat(role.cluster().check(PutCalendarAction.NAME, request), is(true)); - assertThat(role.cluster().check(PutDatafeedAction.NAME, request), is(true)); - assertThat(role.cluster().check(PutFilterAction.NAME, request), is(true)); - assertThat(role.cluster().check(PutJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(RevertModelSnapshotAction.NAME, request), is(true)); - assertThat(role.cluster().check(SetUpgradeModeAction.NAME, request), is(true)); - assertThat(role.cluster().check(StartDatafeedAction.NAME, request), is(true)); - assertThat(role.cluster().check(StopDatafeedAction.NAME, request), is(true)); - assertThat(role.cluster().check(UpdateCalendarJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(UpdateDatafeedAction.NAME, request), is(true)); - assertThat(role.cluster().check(UpdateFilterAction.NAME, request), is(true)); - assertThat(role.cluster().check(UpdateJobAction.NAME, request), is(true)); - assertThat(role.cluster().check(UpdateModelSnapshotAction.NAME, request), is(true)); - assertThat(role.cluster().check(UpdateProcessAction.NAME, request), is(false)); // internal use only - assertThat(role.cluster().check(ValidateDetectorAction.NAME, request), is(true)); - assertThat(role.cluster().check(ValidateJobConfigAction.NAME, request), is(true)); + assertThat(role.cluster().check(CloseJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteCalendarAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteCalendarEventAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteDatafeedAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteExpiredDataAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteFilterAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteForecastAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteModelSnapshotAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(FinalizeJobExecutionAction.NAME, request, authentication), is(false)); // internal use only + assertThat(role.cluster().check(FindFileStructureAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(FlushJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(ForecastJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetBucketsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetCalendarEventsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetCalendarsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetCategoriesAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetDatafeedsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetDatafeedsStatsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetFiltersAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetInfluencersAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetJobsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetJobsStatsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetModelSnapshotsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetOverallBucketsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetRecordsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(IsolateDatafeedAction.NAME, request, authentication), is(false)); // internal use only + assertThat(role.cluster().check(KillProcessAction.NAME, request, authentication), is(false)); // internal use only + assertThat(role.cluster().check(MlInfoAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(OpenJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PersistJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PostCalendarEventsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PostDataAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PreviewDatafeedAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PutCalendarAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PutDatafeedAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PutFilterAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PutJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(RevertModelSnapshotAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(SetUpgradeModeAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(StartDatafeedAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(StopDatafeedAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(UpdateCalendarJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(UpdateDatafeedAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(UpdateFilterAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(UpdateJobAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(UpdateModelSnapshotAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(UpdateProcessAction.NAME, request, authentication), is(false)); // internal use only + assertThat(role.cluster().check(ValidateDetectorAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(ValidateJobConfigAction.NAME, request, authentication), is(true)); assertThat(role.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); assertNoAccessAllowed(role, "foo"); @@ -995,62 +1011,63 @@ public void testMachineLearningAdminRole() { public void testMachineLearningUserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("machine_learning_user"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role role = Role.builder(roleDescriptor, null).build(); - assertThat(role.cluster().check(CloseJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(DeleteCalendarAction.NAME, request), is(false)); - assertThat(role.cluster().check(DeleteCalendarEventAction.NAME, request), is(false)); - assertThat(role.cluster().check(DeleteDatafeedAction.NAME, request), is(false)); - assertThat(role.cluster().check(DeleteExpiredDataAction.NAME, request), is(false)); - assertThat(role.cluster().check(DeleteFilterAction.NAME, request), is(false)); - assertThat(role.cluster().check(DeleteForecastAction.NAME, request), is(false)); - assertThat(role.cluster().check(DeleteJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(DeleteModelSnapshotAction.NAME, request), is(false)); - assertThat(role.cluster().check(FinalizeJobExecutionAction.NAME, request), is(false)); - assertThat(role.cluster().check(FindFileStructureAction.NAME, request), is(true)); - assertThat(role.cluster().check(FlushJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(ForecastJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(GetBucketsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetCalendarEventsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetCalendarsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetCategoriesAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetDatafeedsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetDatafeedsStatsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetFiltersAction.NAME, request), is(false)); - assertThat(role.cluster().check(GetInfluencersAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetJobsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetJobsStatsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetModelSnapshotsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetOverallBucketsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetRecordsAction.NAME, request), is(true)); - assertThat(role.cluster().check(IsolateDatafeedAction.NAME, request), is(false)); - assertThat(role.cluster().check(KillProcessAction.NAME, request), is(false)); - assertThat(role.cluster().check(MlInfoAction.NAME, request), is(true)); - assertThat(role.cluster().check(OpenJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(PersistJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(PostCalendarEventsAction.NAME, request), is(false)); - assertThat(role.cluster().check(PostDataAction.NAME, request), is(false)); - assertThat(role.cluster().check(PreviewDatafeedAction.NAME, request), is(false)); - assertThat(role.cluster().check(PutCalendarAction.NAME, request), is(false)); - assertThat(role.cluster().check(PutDatafeedAction.NAME, request), is(false)); - assertThat(role.cluster().check(PutFilterAction.NAME, request), is(false)); - assertThat(role.cluster().check(PutJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(RevertModelSnapshotAction.NAME, request), is(false)); - assertThat(role.cluster().check(SetUpgradeModeAction.NAME, request), is(false)); - assertThat(role.cluster().check(StartDatafeedAction.NAME, request), is(false)); - assertThat(role.cluster().check(StopDatafeedAction.NAME, request), is(false)); - assertThat(role.cluster().check(UpdateCalendarJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(UpdateDatafeedAction.NAME, request), is(false)); - assertThat(role.cluster().check(UpdateFilterAction.NAME, request), is(false)); - assertThat(role.cluster().check(UpdateJobAction.NAME, request), is(false)); - assertThat(role.cluster().check(UpdateModelSnapshotAction.NAME, request), is(false)); - assertThat(role.cluster().check(UpdateProcessAction.NAME, request), is(false)); - assertThat(role.cluster().check(ValidateDetectorAction.NAME, request), is(false)); - assertThat(role.cluster().check(ValidateJobConfigAction.NAME, request), is(false)); + assertThat(role.cluster().check(CloseJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(DeleteCalendarAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(DeleteCalendarEventAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(DeleteDatafeedAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(DeleteExpiredDataAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(DeleteFilterAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(DeleteForecastAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(DeleteJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(DeleteModelSnapshotAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(FinalizeJobExecutionAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(FindFileStructureAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(FlushJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(ForecastJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(GetBucketsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetCalendarEventsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetCalendarsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetCategoriesAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetDatafeedsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetDatafeedsStatsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetFiltersAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(GetInfluencersAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetJobsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetJobsStatsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetModelSnapshotsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetOverallBucketsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetRecordsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(IsolateDatafeedAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(KillProcessAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(MlInfoAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(OpenJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PersistJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PostCalendarEventsAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PostDataAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PreviewDatafeedAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PutCalendarAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PutDatafeedAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PutFilterAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PutJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(RevertModelSnapshotAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(SetUpgradeModeAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(StartDatafeedAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(StopDatafeedAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(UpdateCalendarJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(UpdateDatafeedAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(UpdateFilterAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(UpdateJobAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(UpdateModelSnapshotAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(UpdateProcessAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(ValidateDetectorAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(ValidateJobConfigAction.NAME, request, authentication), is(false)); assertThat(role.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); assertNoAccessAllowed(role, "foo"); @@ -1079,19 +1096,20 @@ public void testMachineLearningUserRole() { public void testDataFrameTransformsAdminRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("data_frame_transforms_admin"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role role = Role.builder(roleDescriptor, null).build(); - assertThat(role.cluster().check(DeleteDataFrameTransformAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetDataFrameTransformsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetDataFrameTransformsStatsAction.NAME, request), is(true)); - assertThat(role.cluster().check(PreviewDataFrameTransformAction.NAME, request), is(true)); - assertThat(role.cluster().check(PutDataFrameTransformAction.NAME, request), is(true)); - assertThat(role.cluster().check(StartDataFrameTransformAction.NAME, request), is(true)); - assertThat(role.cluster().check(StopDataFrameTransformAction.NAME, request), is(true)); + assertThat(role.cluster().check(DeleteDataFrameTransformAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetDataFrameTransformsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetDataFrameTransformsStatsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PreviewDataFrameTransformAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PutDataFrameTransformAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(StartDataFrameTransformAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(StopDataFrameTransformAction.NAME, request, authentication), is(true)); assertThat(role.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); assertOnlyReadAllowed(role, ".data-frame-notifications-1"); @@ -1115,19 +1133,20 @@ public void testDataFrameTransformsAdminRole() { public void testDataFrameTransformsUserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("data_frame_transforms_user"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role role = Role.builder(roleDescriptor, null).build(); - assertThat(role.cluster().check(DeleteDataFrameTransformAction.NAME, request), is(false)); - assertThat(role.cluster().check(GetDataFrameTransformsAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetDataFrameTransformsStatsAction.NAME, request), is(true)); - assertThat(role.cluster().check(PreviewDataFrameTransformAction.NAME, request), is(false)); - assertThat(role.cluster().check(PutDataFrameTransformAction.NAME, request), is(false)); - assertThat(role.cluster().check(StartDataFrameTransformAction.NAME, request), is(false)); - assertThat(role.cluster().check(StopDataFrameTransformAction.NAME, request), is(false)); + assertThat(role.cluster().check(DeleteDataFrameTransformAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(GetDataFrameTransformsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetDataFrameTransformsStatsAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(PreviewDataFrameTransformAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(PutDataFrameTransformAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(StartDataFrameTransformAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(StopDataFrameTransformAction.NAME, request, authentication), is(false)); assertThat(role.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); assertOnlyReadAllowed(role, ".data-frame-notifications-1"); @@ -1151,20 +1170,21 @@ public void testDataFrameTransformsUserRole() { public void testWatcherAdminRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("watcher_admin"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role role = Role.builder(roleDescriptor, null).build(); - assertThat(role.cluster().check(PutWatchAction.NAME, request), is(true)); - assertThat(role.cluster().check(GetWatchAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteWatchAction.NAME, request), is(true)); - assertThat(role.cluster().check(ExecuteWatchAction.NAME, request), is(true)); - assertThat(role.cluster().check(AckWatchAction.NAME, request), is(true)); - assertThat(role.cluster().check(ActivateWatchAction.NAME, request), is(true)); - assertThat(role.cluster().check(WatcherServiceAction.NAME, request), is(true)); - assertThat(role.cluster().check(WatcherStatsAction.NAME, request), is(true)); + assertThat(role.cluster().check(PutWatchAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(GetWatchAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteWatchAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(ExecuteWatchAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(AckWatchAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(ActivateWatchAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(WatcherServiceAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(WatcherStatsAction.NAME, request, authentication), is(true)); assertThat(role.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); assertThat(role.indices().allowedIndicesMatcher(IndexAction.NAME).test("foo"), is(false)); @@ -1180,20 +1200,21 @@ public void testWatcherAdminRole() { public void testWatcherUserRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("watcher_user"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role role = Role.builder(roleDescriptor, null).build(); - assertThat(role.cluster().check(PutWatchAction.NAME, request), is(false)); - assertThat(role.cluster().check(GetWatchAction.NAME, request), is(true)); - assertThat(role.cluster().check(DeleteWatchAction.NAME, request), is(false)); - assertThat(role.cluster().check(ExecuteWatchAction.NAME, request), is(false)); - assertThat(role.cluster().check(AckWatchAction.NAME, request), is(false)); - assertThat(role.cluster().check(ActivateWatchAction.NAME, request), is(false)); - assertThat(role.cluster().check(WatcherServiceAction.NAME, request), is(false)); - assertThat(role.cluster().check(WatcherStatsAction.NAME, request), is(true)); + assertThat(role.cluster().check(PutWatchAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(GetWatchAction.NAME, request, authentication), is(true)); + assertThat(role.cluster().check(DeleteWatchAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(ExecuteWatchAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(AckWatchAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(ActivateWatchAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(WatcherServiceAction.NAME, request, authentication), is(false)); + assertThat(role.cluster().check(WatcherStatsAction.NAME, request, authentication), is(true)); assertThat(role.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); assertThat(role.indices().allowedIndicesMatcher(IndexAction.NAME).test("foo"), is(false)); @@ -1252,16 +1273,17 @@ private void assertNoAccessAllowed(Role role, String index) { public void testLogstashAdminRole() { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); RoleDescriptor roleDescriptor = new ReservedRolesStore().roleDescriptor("logstash_admin"); assertNotNull(roleDescriptor); assertThat(roleDescriptor.getMetadata(), hasEntry("_reserved", true)); Role logstashAdminRole = Role.builder(roleDescriptor, null).build(); - assertThat(logstashAdminRole.cluster().check(ClusterHealthAction.NAME, request), is(false)); - assertThat(logstashAdminRole.cluster().check(PutIndexTemplateAction.NAME, request), is(false)); - assertThat(logstashAdminRole.cluster().check(ClusterRerouteAction.NAME, request), is(false)); - assertThat(logstashAdminRole.cluster().check(ClusterUpdateSettingsAction.NAME, request), is(false)); + assertThat(logstashAdminRole.cluster().check(ClusterHealthAction.NAME, request, authentication), is(false)); + assertThat(logstashAdminRole.cluster().check(PutIndexTemplateAction.NAME, request, authentication), is(false)); + assertThat(logstashAdminRole.cluster().check(ClusterRerouteAction.NAME, request, authentication), is(false)); + assertThat(logstashAdminRole.cluster().check(ClusterUpdateSettingsAction.NAME, request, authentication), is(false)); assertThat(logstashAdminRole.runAs().check(randomAlphaOfLengthBetween(1, 30)), is(false)); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index df00474f6d69d..78ff3140871c3 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -137,7 +137,7 @@ public void authorizeClusterAction(RequestInfo requestInfo, AuthorizationInfo au ActionListener listener) { if (authorizationInfo instanceof RBACAuthorizationInfo) { final Role role = ((RBACAuthorizationInfo) authorizationInfo).getRole(); - if (role.checkClusterAction(requestInfo.getAction(), requestInfo.getRequest())) { + if (role.checkClusterAction(requestInfo.getAction(), requestInfo.getRequest(), requestInfo.getAuthentication())) { listener.onResponse(AuthorizationResult.granted()); } else if (checkSameUserPermissions(requestInfo.getAction(), requestInfo.getRequest(), requestInfo.getAuthentication())) { listener.onResponse(AuthorizationResult.granted()); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java index 36c9b79538272..64d6cfd938f81 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmIntegTests.java @@ -49,6 +49,7 @@ import org.elasticsearch.xpack.core.security.action.user.GetUsersResponse; import org.elasticsearch.xpack.core.security.action.user.PutUserRequestBuilder; import org.elasticsearch.xpack.core.security.action.user.SetEnabledRequestBuilder; +import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authc.support.Hasher; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.permission.Role; @@ -369,10 +370,11 @@ public void testCreateAndUpdateRole() { } } else { final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); GetRolesResponse getRolesResponse = new GetRolesRequestBuilder(client()).names("test_role").get(); assertTrue("test_role does not exist!", getRolesResponse.hasRoles()); assertTrue("any cluster permission should be authorized", - Role.builder(getRolesResponse.roles()[0], null).build().cluster().check("cluster:admin/foo", request)); + Role.builder(getRolesResponse.roles()[0], null).build().cluster().check("cluster:admin/foo", request, authentication)); preparePutRole("test_role") .cluster("none") @@ -383,7 +385,7 @@ public void testCreateAndUpdateRole() { assertTrue("test_role does not exist!", getRolesResponse.hasRoles()); assertFalse("no cluster permission should be authorized", - Role.builder(getRolesResponse.roles()[0], null).build().cluster().check("cluster:admin/bar", request)); + Role.builder(getRolesResponse.roles()[0], null).build().cluster().check("cluster:admin/bar", request, authentication)); } } 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 d7f3252dd95a5..89c68fec5e281 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 @@ -151,6 +151,7 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.CountDownLatch; +import java.util.function.BiPredicate; import java.util.function.Predicate; import static java.util.Arrays.asList; @@ -320,7 +321,7 @@ public void testAuthorizeUsingConditionalPrivileges() throws IOException { final ConfigurableClusterPrivilege configurableClusterPrivilege = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - final Predicate requestPredicate = r -> r == request; + final BiPredicate requestPredicate = (r,a) -> r == request; final Predicate actionPredicate = Automatons.predicate(((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns()); builder.add(this, actionPredicate, requestPredicate); @@ -347,7 +348,7 @@ public void testAuthorizationDeniedWhenConditionalPrivilegesDoNotMatch() throws final ConfigurableClusterPrivilege configurableClusterPrivilege = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - final Predicate requestPredicate = r -> false; + final BiPredicate requestPredicate = (r,a) -> false; final Predicate actionPredicate = Automatons.predicate(((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns()); builder.add(this, actionPredicate,requestPredicate); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index 4ab525a43da2f..ffe76d3475650 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -546,13 +546,14 @@ public void testMergingBasicRoles() { final TransportRequest request1 = mock(TransportRequest.class); final TransportRequest request2 = mock(TransportRequest.class); final TransportRequest request3 = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); ConfigurableClusterPrivilege ccp1 = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { Predicate predicate1 = Automatons.predicate(((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns()); - builder.add(this, predicate1, req -> req == request1); + builder.add(this, predicate1, (req, authn) -> req == request1); return builder; } }; @@ -584,7 +585,7 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { Predicate predicate2 = Automatons.predicate(((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns()); - builder.add(this, predicate2, req -> req == request2); + builder.add(this, predicate2, (req, authn) -> req == request2); return builder; } }; @@ -626,12 +627,14 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build CompositeRolesStore.buildRoleFromDescriptors(Sets.newHashSet(role1, role2), cache, privilegeStore, future); Role role = future.actionGet(); - assertThat(role.cluster().check(ClusterStateAction.NAME, randomFrom(request1, request2, request3)), equalTo(true)); - assertThat(role.cluster().check(SamlAuthenticateAction.NAME, randomFrom(request1, request2, request3)), equalTo(true)); - assertThat(role.cluster().check(ClusterUpdateSettingsAction.NAME, randomFrom(request1, request2, request3)), equalTo(false)); + assertThat(role.cluster().check(ClusterStateAction.NAME, randomFrom(request1, request2, request3), authentication), equalTo(true)); + assertThat(role.cluster().check(SamlAuthenticateAction.NAME, randomFrom(request1, request2, request3), authentication), + equalTo(true)); + assertThat(role.cluster().check(ClusterUpdateSettingsAction.NAME, randomFrom(request1, request2, request3), authentication), + equalTo(false)); - assertThat(role.cluster().check(PutUserAction.NAME, randomFrom(request1, request2)), equalTo(true)); - assertThat(role.cluster().check(PutUserAction.NAME, request3), equalTo(false)); + assertThat(role.cluster().check(PutUserAction.NAME, randomFrom(request1, request2), authentication), equalTo(true)); + assertThat(role.cluster().check(PutUserAction.NAME, request3, authentication), equalTo(false)); final Predicate allowedRead = role.indices().allowedIndicesMatcher(GetAction.NAME); assertThat(allowedRead.test("abc-123"), equalTo(true)); @@ -1076,7 +1079,7 @@ public void testApiKeyAuthUsesApiKeyServiceWithScopedRole() throws IOException { PlainActionFuture roleFuture = new PlainActionFuture<>(); compositeRolesStore.getRoles(authentication.getUser(), authentication, roleFuture); Role role = roleFuture.actionGet(); - assertThat(role.checkClusterAction("cluster:admin/foo", Empty.INSTANCE), is(false)); + assertThat(role.checkClusterAction("cluster:admin/foo", Empty.INSTANCE, mock(Authentication.class)), is(false)); assertThat(effectiveRoleDescriptors.get(), is(nullValue())); verify(apiKeyService).getRoleForApiKey(eq(authentication), any(ActionListener.class)); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java index 6555dbd882377..3a2c30891008e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java @@ -20,6 +20,7 @@ import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.security.audit.logfile.CapturingLogger; +import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.authz.permission.IndicesPermission; @@ -351,14 +352,15 @@ public void testAutoReload() throws Exception { assertEquals(1, modifiedRoles.size()); assertTrue(modifiedRoles.contains("role5")); final TransportRequest request = mock(TransportRequest.class); + final Authentication authentication = mock(Authentication.class); descriptors = store.roleDescriptors(Collections.singleton("role5")); assertThat(descriptors, notNullValue()); assertEquals(1, descriptors.size()); Role role = Role.builder(descriptors.iterator().next(), null).build(); assertThat(role, notNullValue()); assertThat(role.names(), equalTo(new String[] { "role5" })); - assertThat(role.cluster().check("cluster:monitor/foo/bar", request), is(true)); - assertThat(role.cluster().check("cluster:admin/foo/bar", request), is(false)); + assertThat(role.cluster().check("cluster:monitor/foo/bar", request, authentication), is(true)); + assertThat(role.cluster().check("cluster:admin/foo/bar", request, authentication), is(false)); // truncate to remove some final Set truncatedFileRolesModified = new HashSet<>(); From 1af13084f969b205635647ba89f5f21d762612a6 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 13:36:06 +1000 Subject: [PATCH 02/27] address review comments The permission checks that are dependent on actions and optionally on request and/or on authentication, now have a way to specify the predicates. By default the implementation will tests all the predicates to be successful for the operation to be allowed. In case customization is required one has option to implement `PermissionCheck`. - Adds a permission check predicate interface that also allows implementers to specify behavior for `implies`. --- .../authz/permission/ClusterPermission.java | 84 ++++++++++---- .../ConfigurableClusterPrivileges.java | 55 ++++++--- .../permission/ClusterPermissionTests.java | 105 ++++-------------- .../authz/AuthorizationServiceTests.java | 35 ++++-- .../authz/store/CompositeRolesStoreTests.java | 33 ++++-- 5 files changed, 171 insertions(+), 141 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index 738ff0c895520..76e00723f0e0d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -16,7 +16,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.function.BiPredicate; import java.util.function.Predicate; /** @@ -93,10 +92,21 @@ public Builder add(final ClusterPrivilege clusterPrivilege, final Set al return this; } - public Builder add(final ClusterPrivilege clusterPrivilege, final Predicate actionPredicate, - final BiPredicate requestAuthnPredicate) { - return add(clusterPrivilege, new ActionRequestAuthenticationPredicatePermissionCheck(clusterPrivilege, - actionPredicate, requestAuthnPredicate)); + public Builder add(final ClusterPrivilege clusterPrivilege, final Set allowedActionPatterns, + final Set excludeActionPatterns, final PermissionCheckPredicate requestPredicate) { + return add(clusterPrivilege, allowedActionPatterns, excludeActionPatterns, requestPredicate, + new AllowAllPermissionCheckPredicate()); + } + + public Builder add(final ClusterPrivilege clusterPrivilege, final Set allowedActionPatterns, + final Set excludeActionPatterns, final PermissionCheckPredicate requestPredicate, + final PermissionCheckPredicate authenticationPredicate) { + final Automaton allowedAutomaton = Automatons.patterns(allowedActionPatterns); + final Automaton excludedAutomaton = Automatons.patterns(excludeActionPatterns); + final Automaton actionAutomaton = Automatons.minusAndMinimize(allowedAutomaton, excludedAutomaton); + + return add(clusterPrivilege, new ActionRequestAuthenticationBasedPermissionCheck(actionAutomaton, requestPredicate, + authenticationPredicate)); } public Builder add(final ClusterPrivilege clusterPrivilege, final PermissionCheck permissionCheck) { @@ -120,6 +130,35 @@ public ClusterPermission build() { } } + /** + * A {@link Predicate} which also can determine if the other {@link PermissionCheckPredicate} + * is implied by it. + */ + public interface PermissionCheckPredicate extends Predicate { + /** + * Checks whether specified {@link PermissionCheckPredicate} is implied by this {@link PermissionCheckPredicate}.
+ * This is important method to be considered during implementation as it compares {@link PermissionCheckPredicate}s. + * If {@code permissionCheckPredicate.implies(otherPermissionCheckPredicate)}, that means all the operations allowed + * by {@code otherPermissionCheckPredicate} are also allowed by {@code permissionCheckPredicate}. + * + * @param otherPermissionCheckPredicate {@link PermissionCheckPredicate} + * @return {@code true} if the specified permission check predicate is implied by this + * {@link PermissionCheckPredicate} else returns {@code false} + */ + boolean implies(PermissionCheckPredicate otherPermissionCheckPredicate); + } + + private static final class AllowAllPermissionCheckPredicate implements PermissionCheckPredicate { + @Override + public boolean implies(PermissionCheckPredicate otherPermissionCheckPredicate) { + return true; + } + @Override + public boolean test(T t) { + return true; + } + } + /** * Evaluates whether the cluster actions (optionally for a given request) * is permitted by this permission. @@ -174,30 +213,33 @@ public boolean implies(final PermissionCheck permissionCheck) { } // action, request and authentication based permission check - private static class ActionRequestAuthenticationPredicatePermissionCheck implements PermissionCheck { - private final ClusterPrivilege clusterPrivilege; - final Predicate actionPredicate; - final BiPredicate requestAuthnPredicate; - - ActionRequestAuthenticationPredicatePermissionCheck(final ClusterPrivilege clusterPrivilege, - final Predicate actionPredicate, - final BiPredicate requestAuthnPredicate) { - this.clusterPrivilege = clusterPrivilege; - this.actionPredicate = actionPredicate; - this.requestAuthnPredicate = requestAuthnPredicate; + private static class ActionRequestAuthenticationBasedPermissionCheck extends AutomatonPermissionCheck { + private final PermissionCheckPredicate requestPredicate; + private final PermissionCheckPredicate authenticationPredicate; + + ActionRequestAuthenticationBasedPermissionCheck(final Automaton automaton, + final PermissionCheckPredicate requestPredicate, + final PermissionCheckPredicate authenticationPredicate) { + super(automaton); + this.requestPredicate = requestPredicate; + this.authenticationPredicate = authenticationPredicate; } @Override public boolean check(final String action, final TransportRequest request, final Authentication authentication) { - return actionPredicate.test(action) && requestAuthnPredicate.test(request, authentication); + return super.check(action, request, authentication) && requestPredicate.test(request) && authenticationPredicate.test( + authentication); } @Override public boolean implies(final PermissionCheck permissionCheck) { - if (permissionCheck instanceof ActionRequestAuthenticationPredicatePermissionCheck) { - final ActionRequestAuthenticationPredicatePermissionCheck otherCheck = - (ActionRequestAuthenticationPredicatePermissionCheck) permissionCheck; - return this.clusterPrivilege.equals(otherCheck.clusterPrivilege); + if (super.implies(permissionCheck)) { + if (permissionCheck instanceof ActionRequestAuthenticationBasedPermissionCheck) { + final ActionRequestAuthenticationBasedPermissionCheck otherCheck = + (ActionRequestAuthenticationBasedPermissionCheck) permissionCheck; + return this.requestPredicate.implies(otherCheck.requestPredicate) && + this.authenticationPredicate.implies(otherCheck.authenticationPredicate); + } } return false; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java index 68cdfc3d1f836..b98654787e682 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java @@ -6,6 +6,8 @@ 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.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -15,9 +17,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.xpack.core.security.action.privilege.ApplicationPrivilegesRequest; -import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege.Category; import org.elasticsearch.xpack.core.security.support.Automatons; @@ -31,7 +31,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; -import java.util.function.BiPredicate; import java.util.function.Predicate; /** @@ -127,26 +126,12 @@ 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 ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/privilege/*"); public static final String WRITEABLE_NAME = "manage-application-privileges"; private final Set applicationNames; - private final Predicate applicationPredicate; - private final BiPredicate requestAuthnPredicate; public ManageApplicationPrivileges(Set applicationNames) { this.applicationNames = Collections.unmodifiableSet(applicationNames); - this.applicationPredicate = Automatons.predicate(applicationNames); - this.requestAuthnPredicate = (request, authn) -> { - if (request instanceof ApplicationPrivilegesRequest) { - final ApplicationPrivilegesRequest privRequest = (ApplicationPrivilegesRequest) request; - final Collection requestApplicationNames = privRequest.getApplicationNames(); - return requestApplicationNames.isEmpty() ? this.applicationNames.contains("*") - : requestApplicationNames.stream().allMatch(application -> applicationPredicate.test(application)); - } - return false; - }; } @Override @@ -217,12 +202,46 @@ public int hashCode() { @Override public ClusterPermission.Builder buildPermission(final ClusterPermission.Builder builder) { - return builder.add(this, ACTION_PREDICATE, requestAuthnPredicate); + return builder.add(this, Set.of("cluster:admin/xpack/security/privilege/*"), Set.of(), + new RequestPermissionCheckPredicate(applicationNames)); } private interface Fields { ParseField MANAGE = new ParseField("manage"); ParseField APPLICATIONS = new ParseField("applications"); } + + private static class RequestPermissionCheckPredicate + implements ClusterPermission.PermissionCheckPredicate { + private final Automaton applicationNamesAutomaton; + private final Set applicationNames; + private final Predicate applicationPredicate; + + RequestPermissionCheckPredicate(Set applicationNames) { + this.applicationNamesAutomaton = Automatons.patterns(applicationNames); + this.applicationNames = applicationNames; + this.applicationPredicate = Automatons.predicate(applicationNamesAutomaton); + } + + @Override + public boolean implies(ClusterPermission.PermissionCheckPredicate otherPermissionCheckPredicate) { + if (otherPermissionCheckPredicate instanceof RequestPermissionCheckPredicate) { + return Operations.subsetOf(((RequestPermissionCheckPredicate) otherPermissionCheckPredicate).applicationNamesAutomaton, + this.applicationNamesAutomaton); + } + return false; + } + + @Override + public boolean test(TransportRequest transportRequest) { + if (transportRequest instanceof ApplicationPrivilegesRequest) { + final ApplicationPrivilegesRequest privRequest = (ApplicationPrivilegesRequest) transportRequest; + final Collection requestApplicationNames = privRequest.getApplicationNames(); + return requestApplicationNames.isEmpty() ? this.applicationNames.contains("*") + : requestApplicationNames.stream().allMatch(application -> applicationPredicate.test(application)); + } + return false; + } + } } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java index 4fe368f5e33a1..a28931b060f14 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java @@ -8,22 +8,15 @@ package org.elasticsearch.xpack.core.security.authz.permission; -import org.elasticsearch.common.io.stream.StreamOutput; -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.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges; import org.junit.Before; -import java.io.IOException; -import java.util.Objects; import java.util.Set; -import java.util.function.BiPredicate; -import java.util.function.Predicate; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; @@ -52,10 +45,10 @@ public void testClusterPermissionBuilder() { builder = ClusterPrivilegeResolver.MANAGE_SECURITY.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = - new MockConfigurableClusterPrivilege((r,a) -> false); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege2 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1", "application-2")); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); builder = mockConfigurableClusterPrivilege2.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -74,10 +67,10 @@ public void testClusterPermissionCheck() { builder = ClusterPrivilegeResolver.MANAGE_SECURITY.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = - new MockConfigurableClusterPrivilege((r,a) -> false); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege2 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1", "application-2")); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); builder = mockConfigurableClusterPrivilege2.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -138,10 +131,10 @@ public void testNoneClusterPermissionIsImpliedByAny() { ClusterPermission.Builder builder = ClusterPermission.builder(); builder = ClusterPrivilegeResolver.MANAGE_SECURITY.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = - new MockConfigurableClusterPrivilege((r,a) -> false); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege2 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1", "application-2")); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); builder = mockConfigurableClusterPrivilege2.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -153,8 +146,8 @@ public void testClusterPermissionSubsetWithConfigurableClusterPrivilegeIsImplied ClusterPermission.Builder builder = ClusterPermission.builder(); builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -170,16 +163,16 @@ public void testClusterPermissionNonSubsetWithConfigurableClusterPrivilegeIsImpl ClusterPermission.Builder builder = ClusterPermission.builder(); builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); ClusterPermission.Builder builder1 = ClusterPermission.builder(); builder1 = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder1); builder1 = mockConfigurableClusterPrivilege1.buildPermission(builder1); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = - new MockConfigurableClusterPrivilege((r,a) -> false); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege2 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1", "application-2")); builder1 = mockConfigurableClusterPrivilege2.buildPermission(builder1); final ClusterPermission otherClusterPermission = builder1.build(); @@ -216,8 +209,8 @@ public void testClusterPermissionIsImpliedBySameClusterPermission() { ClusterPermission.Builder builder = ClusterPermission.builder(); builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = - new MockConfigurableClusterPrivilege((r,a) -> r == mockTransportRequest && a == mockAuthentication); + final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = + new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -232,60 +225,4 @@ public void testClusterPermissionSubsetIsImpliedByAllClusterPermission() { assertThat(allClusterPermission.implies(otherClusterPermission), is(true)); } - private static class MockConfigurableClusterPrivilege implements ConfigurableClusterPrivilege { - static final Predicate ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/privilege/*"); - private BiPredicate requestAuthnPredicate; - - MockConfigurableClusterPrivilege(BiPredicate requestPredicate) { - this.requestAuthnPredicate = requestPredicate; - } - - @Override - public Category getCategory() { - return Category.APPLICATION; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder; - } - - @Override - public String getWriteableName() { - return "mock-ccp"; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - final MockConfigurableClusterPrivilege that = (MockConfigurableClusterPrivilege) o; - return requestAuthnPredicate.equals(that.requestAuthnPredicate); - } - - @Override - public int hashCode() { - return Objects.hash(requestAuthnPredicate); - } - - @Override - public String toString() { - return "MockConfigurableClusterPrivilege{" + - "requestAuthnPredicate=" + requestAuthnPredicate + - '}'; - } - - @Override - public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - return builder.add(this, ACTION_PREDICATE, requestAuthnPredicate); - } - } } 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 89c68fec5e281..0bb076332d689 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 @@ -119,7 +119,6 @@ import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; -import org.elasticsearch.xpack.core.security.support.Automatons; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.ElasticUser; import org.elasticsearch.xpack.core.security.user.KibanaUser; @@ -151,8 +150,6 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.CountDownLatch; -import java.util.function.BiPredicate; -import java.util.function.Predicate; import static java.util.Arrays.asList; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException; @@ -321,10 +318,18 @@ public void testAuthorizeUsingConditionalPrivileges() throws IOException { final ConfigurableClusterPrivilege configurableClusterPrivilege = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - final BiPredicate requestPredicate = (r,a) -> r == request; - final Predicate actionPredicate = - Automatons.predicate(((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns()); - builder.add(this, actionPredicate, requestPredicate); + builder.add(this, ((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), Set.of(), + new ClusterPermission.PermissionCheckPredicate() { + @Override + public boolean implies(ClusterPermission.PermissionCheckPredicate otherPermissionCheckPredicate) { + return this.equals(otherPermissionCheckPredicate); + } + + @Override + public boolean test(TransportRequest r) { + return r == request; + } + }); return builder; } }; @@ -348,10 +353,18 @@ public void testAuthorizationDeniedWhenConditionalPrivilegesDoNotMatch() throws final ConfigurableClusterPrivilege configurableClusterPrivilege = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - final BiPredicate requestPredicate = (r,a) -> false; - final Predicate actionPredicate = - Automatons.predicate(((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns()); - builder.add(this, actionPredicate,requestPredicate); + builder.add(this,((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), Set.of(), + new ClusterPermission.PermissionCheckPredicate() { + @Override + public boolean implies(ClusterPermission.PermissionCheckPredicate otherPermissionCheckPredicate) { + return this.equals(otherPermissionCheckPredicate); + } + + @Override + public boolean test(TransportRequest r) { + return false; + } + }); return builder; } }; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index ffe76d3475650..d7c84926ecb8c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -51,7 +51,6 @@ import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames; -import org.elasticsearch.xpack.core.security.support.Automatons; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.SystemUser; import org.elasticsearch.xpack.core.security.user.User; @@ -551,9 +550,19 @@ public void testMergingBasicRoles() { ConfigurableClusterPrivilege ccp1 = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - Predicate predicate1 = - Automatons.predicate(((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns()); - builder.add(this, predicate1, (req, authn) -> req == request1); + builder.add(this, ((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), + Set.of(), new ClusterPermission.PermissionCheckPredicate() { + + @Override + public boolean test(TransportRequest r) { + return r == request1; + } + + @Override + public boolean implies(ClusterPermission.PermissionCheckPredicate permissionCheckPredicate) { + return this.equals(permissionCheckPredicate); + } + }); return builder; } }; @@ -583,9 +592,19 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build ConfigurableClusterPrivilege ccp2 = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - Predicate predicate2 = - Automatons.predicate(((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns()); - builder.add(this, predicate2, (req, authn) -> req == request2); + builder.add(this, ((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), + Set.of(), new ClusterPermission.PermissionCheckPredicate() { + + @Override + public boolean test(TransportRequest r) { + return r == request2; + } + + @Override + public boolean implies(ClusterPermission.PermissionCheckPredicate permissionCheckPredicate) { + return this.equals(permissionCheckPredicate); + } + }); return builder; } }; From 8896dcd8b8a64168a8bca655aad69777f7568424 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 14:25:07 +1000 Subject: [PATCH 03/27] remove unwanted code --- .../authz/permission/ClusterPermission.java | 44 +++++-------------- 1 file changed, 10 insertions(+), 34 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index 76e00723f0e0d..34caf46ee885b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -94,19 +94,11 @@ public Builder add(final ClusterPrivilege clusterPrivilege, final Set al public Builder add(final ClusterPrivilege clusterPrivilege, final Set allowedActionPatterns, final Set excludeActionPatterns, final PermissionCheckPredicate requestPredicate) { - return add(clusterPrivilege, allowedActionPatterns, excludeActionPatterns, requestPredicate, - new AllowAllPermissionCheckPredicate()); - } - - public Builder add(final ClusterPrivilege clusterPrivilege, final Set allowedActionPatterns, - final Set excludeActionPatterns, final PermissionCheckPredicate requestPredicate, - final PermissionCheckPredicate authenticationPredicate) { final Automaton allowedAutomaton = Automatons.patterns(allowedActionPatterns); final Automaton excludedAutomaton = Automatons.patterns(excludeActionPatterns); final Automaton actionAutomaton = Automatons.minusAndMinimize(allowedAutomaton, excludedAutomaton); - return add(clusterPrivilege, new ActionRequestAuthenticationBasedPermissionCheck(actionAutomaton, requestPredicate, - authenticationPredicate)); + return add(clusterPrivilege, new ActionRequestBasedPermissionCheck(actionAutomaton, requestPredicate)); } public Builder add(final ClusterPrivilege clusterPrivilege, final PermissionCheck permissionCheck) { @@ -148,17 +140,6 @@ public interface PermissionCheckPredicate extends Predicate { boolean implies(PermissionCheckPredicate otherPermissionCheckPredicate); } - private static final class AllowAllPermissionCheckPredicate implements PermissionCheckPredicate { - @Override - public boolean implies(PermissionCheckPredicate otherPermissionCheckPredicate) { - return true; - } - @Override - public boolean test(T t) { - return true; - } - } - /** * Evaluates whether the cluster actions (optionally for a given request) * is permitted by this permission. @@ -212,33 +193,28 @@ public boolean implies(final PermissionCheck permissionCheck) { } } - // action, request and authentication based permission check - private static class ActionRequestAuthenticationBasedPermissionCheck extends AutomatonPermissionCheck { + // action, request based permission check + private static class ActionRequestBasedPermissionCheck extends AutomatonPermissionCheck { private final PermissionCheckPredicate requestPredicate; - private final PermissionCheckPredicate authenticationPredicate; - ActionRequestAuthenticationBasedPermissionCheck(final Automaton automaton, - final PermissionCheckPredicate requestPredicate, - final PermissionCheckPredicate authenticationPredicate) { + ActionRequestBasedPermissionCheck(final Automaton automaton, + final PermissionCheckPredicate requestPredicate) { super(automaton); this.requestPredicate = requestPredicate; - this.authenticationPredicate = authenticationPredicate; } @Override public boolean check(final String action, final TransportRequest request, final Authentication authentication) { - return super.check(action, request, authentication) && requestPredicate.test(request) && authenticationPredicate.test( - authentication); + return super.check(action, request, authentication) && requestPredicate.test(request); } @Override public boolean implies(final PermissionCheck permissionCheck) { if (super.implies(permissionCheck)) { - if (permissionCheck instanceof ActionRequestAuthenticationBasedPermissionCheck) { - final ActionRequestAuthenticationBasedPermissionCheck otherCheck = - (ActionRequestAuthenticationBasedPermissionCheck) permissionCheck; - return this.requestPredicate.implies(otherCheck.requestPredicate) && - this.authenticationPredicate.implies(otherCheck.authenticationPredicate); + if (permissionCheck instanceof ActionRequestBasedPermissionCheck) { + final ActionRequestBasedPermissionCheck otherCheck = + (ActionRequestBasedPermissionCheck) permissionCheck; + return this.requestPredicate.implies(otherCheck.requestPredicate); } } return false; From 4976d7828c3224b731d3856e957efc8955eee8cc Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 15:51:00 +1000 Subject: [PATCH 04/27] remove unwanted code change, raise a separate pr for it --- .../authz/permission/ClusterPermission.java | 32 ++---- .../ConfigurableClusterPrivileges.java | 50 +++------ .../permission/ClusterPermissionTests.java | 102 ++++++++++++++---- 3 files changed, 104 insertions(+), 80 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index 34caf46ee885b..ec9d9e1014d27 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -93,12 +93,12 @@ public Builder add(final ClusterPrivilege clusterPrivilege, final Set al } public Builder add(final ClusterPrivilege clusterPrivilege, final Set allowedActionPatterns, - final Set excludeActionPatterns, final PermissionCheckPredicate requestPredicate) { + final Set excludeActionPatterns, final Predicate 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(actionAutomaton, requestPredicate)); + return add(clusterPrivilege, new ActionRequestBasedPermissionCheck(clusterPrivilege, actionAutomaton, requestPredicate)); } public Builder add(final ClusterPrivilege clusterPrivilege, final PermissionCheck permissionCheck) { @@ -122,24 +122,6 @@ public ClusterPermission build() { } } - /** - * A {@link Predicate} which also can determine if the other {@link PermissionCheckPredicate} - * is implied by it. - */ - public interface PermissionCheckPredicate extends Predicate { - /** - * Checks whether specified {@link PermissionCheckPredicate} is implied by this {@link PermissionCheckPredicate}.
- * This is important method to be considered during implementation as it compares {@link PermissionCheckPredicate}s. - * If {@code permissionCheckPredicate.implies(otherPermissionCheckPredicate)}, that means all the operations allowed - * by {@code otherPermissionCheckPredicate} are also allowed by {@code permissionCheckPredicate}. - * - * @param otherPermissionCheckPredicate {@link PermissionCheckPredicate} - * @return {@code true} if the specified permission check predicate is implied by this - * {@link PermissionCheckPredicate} else returns {@code false} - */ - boolean implies(PermissionCheckPredicate otherPermissionCheckPredicate); - } - /** * Evaluates whether the cluster actions (optionally for a given request) * is permitted by this permission. @@ -195,12 +177,14 @@ public boolean implies(final PermissionCheck permissionCheck) { // action, request based permission check private static class ActionRequestBasedPermissionCheck extends AutomatonPermissionCheck { - private final PermissionCheckPredicate requestPredicate; + private final ClusterPrivilege clusterPrivilege; + private final Predicate requestPredicate; - ActionRequestBasedPermissionCheck(final Automaton automaton, - final PermissionCheckPredicate requestPredicate) { + ActionRequestBasedPermissionCheck(ClusterPrivilege clusterPrivilege, final Automaton automaton, + final Predicate requestPredicate) { super(automaton); this.requestPredicate = requestPredicate; + this.clusterPrivilege = clusterPrivilege; } @Override @@ -214,7 +198,7 @@ public boolean implies(final PermissionCheck permissionCheck) { if (permissionCheck instanceof ActionRequestBasedPermissionCheck) { final ActionRequestBasedPermissionCheck otherCheck = (ActionRequestBasedPermissionCheck) permissionCheck; - return this.requestPredicate.implies(otherCheck.requestPredicate); + return this.clusterPrivilege.equals(otherCheck.clusterPrivilege); } } return false; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java index b98654787e682..8632cda623d4c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParseException; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.xpack.core.security.action.privilege.ApplicationPrivilegesRequest; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege.Category; @@ -129,9 +130,22 @@ public static class ManageApplicationPrivileges implements ConfigurableClusterPr public static final String WRITEABLE_NAME = "manage-application-privileges"; private final Set applicationNames; + private final Predicate applicationPredicate; + private final Predicate requestPredicate; public ManageApplicationPrivileges(Set applicationNames) { this.applicationNames = Collections.unmodifiableSet(applicationNames); + this.applicationPredicate = Automatons.predicate(applicationNames); + this.requestPredicate = request -> { + if (request instanceof ApplicationPrivilegesRequest) { + final ApplicationPrivilegesRequest privRequest = (ApplicationPrivilegesRequest) request; + final Collection requestApplicationNames = privRequest.getApplicationNames(); + return requestApplicationNames.isEmpty() ? this.applicationNames.contains("*") + : requestApplicationNames.stream().allMatch(application -> applicationPredicate.test(application)); + } + return false; + }; + } @Override @@ -202,46 +216,12 @@ public int hashCode() { @Override public ClusterPermission.Builder buildPermission(final ClusterPermission.Builder builder) { - return builder.add(this, Set.of("cluster:admin/xpack/security/privilege/*"), Set.of(), - new RequestPermissionCheckPredicate(applicationNames)); + return builder.add(this, Set.of("cluster:admin/xpack/security/privilege/*"), Set.of(), requestPredicate); } private interface Fields { ParseField MANAGE = new ParseField("manage"); ParseField APPLICATIONS = new ParseField("applications"); } - - private static class RequestPermissionCheckPredicate - implements ClusterPermission.PermissionCheckPredicate { - private final Automaton applicationNamesAutomaton; - private final Set applicationNames; - private final Predicate applicationPredicate; - - RequestPermissionCheckPredicate(Set applicationNames) { - this.applicationNamesAutomaton = Automatons.patterns(applicationNames); - this.applicationNames = applicationNames; - this.applicationPredicate = Automatons.predicate(applicationNamesAutomaton); - } - - @Override - public boolean implies(ClusterPermission.PermissionCheckPredicate otherPermissionCheckPredicate) { - if (otherPermissionCheckPredicate instanceof RequestPermissionCheckPredicate) { - return Operations.subsetOf(((RequestPermissionCheckPredicate) otherPermissionCheckPredicate).applicationNamesAutomaton, - this.applicationNamesAutomaton); - } - return false; - } - - @Override - public boolean test(TransportRequest transportRequest) { - if (transportRequest instanceof ApplicationPrivilegesRequest) { - final ApplicationPrivilegesRequest privRequest = (ApplicationPrivilegesRequest) transportRequest; - final Collection requestApplicationNames = privRequest.getApplicationNames(); - return requestApplicationNames.isEmpty() ? this.applicationNames.contains("*") - : requestApplicationNames.stream().allMatch(application -> applicationPredicate.test(application)); - } - return false; - } - } } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java index a28931b060f14..7801ae7647387 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermissionTests.java @@ -8,15 +8,20 @@ package org.elasticsearch.xpack.core.security.authz.permission; +import org.elasticsearch.common.io.stream.StreamOutput; +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.ConfigurableClusterPrivileges; +import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.junit.Before; +import java.io.IOException; +import java.util.Objects; import java.util.Set; +import java.util.function.Predicate; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; @@ -45,10 +50,10 @@ public void testClusterPermissionBuilder() { builder = ClusterPrivilegeResolver.MANAGE_SECURITY.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege2 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1", "application-2")); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = + new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = + new MockConfigurableClusterPrivilege(r -> false); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); builder = mockConfigurableClusterPrivilege2.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -67,10 +72,10 @@ public void testClusterPermissionCheck() { builder = ClusterPrivilegeResolver.MANAGE_SECURITY.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege2 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1", "application-2")); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = + new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = + new MockConfigurableClusterPrivilege(r -> false); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); builder = mockConfigurableClusterPrivilege2.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -131,10 +136,10 @@ public void testNoneClusterPermissionIsImpliedByAny() { ClusterPermission.Builder builder = ClusterPermission.builder(); builder = ClusterPrivilegeResolver.MANAGE_SECURITY.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege2 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1", "application-2")); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = + new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = + new MockConfigurableClusterPrivilege(r -> false); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); builder = mockConfigurableClusterPrivilege2.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -146,8 +151,8 @@ public void testClusterPermissionSubsetWithConfigurableClusterPrivilegeIsImplied ClusterPermission.Builder builder = ClusterPermission.builder(); builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = + new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -163,16 +168,16 @@ public void testClusterPermissionNonSubsetWithConfigurableClusterPrivilegeIsImpl ClusterPermission.Builder builder = ClusterPermission.builder(); builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = + new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); ClusterPermission.Builder builder1 = ClusterPermission.builder(); builder1 = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder1); builder1 = mockConfigurableClusterPrivilege1.buildPermission(builder1); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege2 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1", "application-2")); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege2 = + new MockConfigurableClusterPrivilege(r -> false); builder1 = mockConfigurableClusterPrivilege2.buildPermission(builder1); final ClusterPermission otherClusterPermission = builder1.build(); @@ -209,8 +214,8 @@ public void testClusterPermissionIsImpliedBySameClusterPermission() { ClusterPermission.Builder builder = ClusterPermission.builder(); builder = ClusterPrivilegeResolver.MANAGE_ML.buildPermission(builder); builder = ClusterPrivilegeResolver.MANAGE_ILM.buildPermission(builder); - final ConfigurableClusterPrivileges.ManageApplicationPrivileges mockConfigurableClusterPrivilege1 = - new ConfigurableClusterPrivileges.ManageApplicationPrivileges(Set.of("application-1")); + final MockConfigurableClusterPrivilege mockConfigurableClusterPrivilege1 = + new MockConfigurableClusterPrivilege(r -> r == mockTransportRequest); builder = mockConfigurableClusterPrivilege1.buildPermission(builder); final ClusterPermission clusterPermission = builder.build(); @@ -225,4 +230,59 @@ public void testClusterPermissionSubsetIsImpliedByAllClusterPermission() { assertThat(allClusterPermission.implies(otherClusterPermission), is(true)); } + private static class MockConfigurableClusterPrivilege implements ConfigurableClusterPrivilege { + private Predicate requestPredicate; + + MockConfigurableClusterPrivilege(Predicate requestPredicate) { + this.requestPredicate = requestPredicate; + } + + @Override + public Category getCategory() { + return Category.APPLICATION; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return builder; + } + + @Override + public String getWriteableName() { + return "mock-ccp"; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + final MockConfigurableClusterPrivilege that = (MockConfigurableClusterPrivilege) o; + return requestPredicate.equals(that.requestPredicate); + } + + @Override + public int hashCode() { + return Objects.hash(requestPredicate); + } + + @Override + public String toString() { + return "MockConfigurableClusterPrivilege{" + + "requestAuthnPredicate=" + requestPredicate + + '}'; + } + + @Override + public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { + return builder.add(this, Set.of("cluster:admin/xpack/security/privilege/*"), Set.of(), requestPredicate); + } + } } From 54490a7c203e075609c2a0512ae20812f7932b75 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 15:58:46 +1000 Subject: [PATCH 05/27] fix tests --- .../authz/AuthorizationServiceTests.java | 30 +++++-------------- .../authz/store/CompositeRolesStoreTests.java | 28 ++--------------- 2 files changed, 10 insertions(+), 48 deletions(-) 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 0bb076332d689..e442172a5e642 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 @@ -119,6 +119,7 @@ import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.core.security.support.Automatons; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.ElasticUser; import org.elasticsearch.xpack.core.security.user.KibanaUser; @@ -150,6 +151,7 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.CountDownLatch; +import java.util.function.Predicate; import static java.util.Arrays.asList; import static org.elasticsearch.test.SecurityTestsUtils.assertAuthenticationException; @@ -318,18 +320,9 @@ public void testAuthorizeUsingConditionalPrivileges() throws IOException { final ConfigurableClusterPrivilege configurableClusterPrivilege = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { + final Predicate requestPredicate = r -> r == request; builder.add(this, ((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), Set.of(), - new ClusterPermission.PermissionCheckPredicate() { - @Override - public boolean implies(ClusterPermission.PermissionCheckPredicate otherPermissionCheckPredicate) { - return this.equals(otherPermissionCheckPredicate); - } - - @Override - public boolean test(TransportRequest r) { - return r == request; - } - }); + requestPredicate); return builder; } }; @@ -353,18 +346,9 @@ public void testAuthorizationDeniedWhenConditionalPrivilegesDoNotMatch() throws final ConfigurableClusterPrivilege configurableClusterPrivilege = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - builder.add(this,((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), Set.of(), - new ClusterPermission.PermissionCheckPredicate() { - @Override - public boolean implies(ClusterPermission.PermissionCheckPredicate otherPermissionCheckPredicate) { - return this.equals(otherPermissionCheckPredicate); - } - - @Override - public boolean test(TransportRequest r) { - return false; - } - }); + final Predicate requestPredicate = r -> false; + builder.add(this, ((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), Set.of() + , requestPredicate); return builder; } }; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index d7c84926ecb8c..53c8f82361f13 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -550,19 +550,8 @@ public void testMergingBasicRoles() { ConfigurableClusterPrivilege ccp1 = new MockConfigurableClusterPrivilege() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - builder.add(this, ((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), - Set.of(), new ClusterPermission.PermissionCheckPredicate() { - - @Override - public boolean test(TransportRequest r) { - return r == request1; - } - - @Override - public boolean implies(ClusterPermission.PermissionCheckPredicate permissionCheckPredicate) { - return this.equals(permissionCheckPredicate); - } - }); + builder.add(this, ((ActionClusterPrivilege) ClusterPrivilegeResolver.MANAGE_SECURITY).getAllowedActionPatterns(), Set.of(), + req -> req == request1); return builder; } }; @@ -593,18 +582,7 @@ public boolean implies(ClusterPermission.PermissionCheckPredicate() { - - @Override - public boolean test(TransportRequest r) { - return r == request2; - } - - @Override - public boolean implies(ClusterPermission.PermissionCheckPredicate permissionCheckPredicate) { - return this.equals(permissionCheckPredicate); - } - }); + Set.of(), req -> req == request2); return builder; } }; From 90339969ee251af1e5889e012da498e610df5d81 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 17:47:31 +1000 Subject: [PATCH 06/27] precommit error --- .../xpack/security/authz/AuthorizationServiceTests.java | 1 - 1 file changed, 1 deletion(-) 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 e442172a5e642..b5b137a74bda0 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 @@ -119,7 +119,6 @@ import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; -import org.elasticsearch.xpack.core.security.support.Automatons; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.ElasticUser; import org.elasticsearch.xpack.core.security.user.KibanaUser; From 541cfadc517910cf7499d24fc53b29d8551ec8f3 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 18:03:31 +1000 Subject: [PATCH 07/27] oh precommit, ran build on another branch, fixed it --- .../security/authz/privilege/ConfigurableClusterPrivileges.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java index 8632cda623d4c..8b4840f2d504f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ConfigurableClusterPrivileges.java @@ -6,8 +6,6 @@ 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.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; From aa623c352b1d9533e8c1697ba7474a0bb45ceefd Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 13 Aug 2019 10:09:42 +1000 Subject: [PATCH 08/27] Add `manage_own_api_key` cluster privilege This commit adds `manage_own_api_key` cluster privilege which only allows api key cluster actions on API keys owned by the current authenticated user. --- .../privilege/ClusterPrivilegeResolver.java | 5 +- .../ManageOwnApiKeyClusterPrivilege.java | 88 +++++++++++++++++++ .../ManageOwnApiKeyClusterPrivilegeTests.java | 88 +++++++++++++++++++ 3 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java index 755d76e76aa03..4f6e2afd9ecd1 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ClusterPrivilegeResolver.java @@ -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 VALUES = Stream.of( NONE, ALL, @@ -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. diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java new file mode 100644 index 0000000000000..d5e479612efdf --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -0,0 +1,88 @@ +/* + * + * 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; + +import java.util.function.BiPredicate; +import java.util.function.Predicate; + +/** + * 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 Predicate ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/api_key/*"); + private final String name; + private final BiPredicate requestAuthnPredicate; + + private ManageOwnApiKeyClusterPrivilege() { + this.name = "manage_own_api_key"; + this.requestAuthnPredicate = (request, 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; + }; + } + + private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, String apiKeyId, String username, String realmName) { + if (isCurrentAuthenticationUsingSameApiKeyIdFromRequest(authentication, apiKeyId)) { + return true; + } else { + /* + * TODO ygaikwad 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 proper 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) { + // TODO ygaikwad replace with constants after merging other change + 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; + } + + @Override + public String name() { + return name; + } + + @Override + public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { + return builder.add(this, ACTION_PREDICATE, requestAuthnPredicate); + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java new file mode 100644 index 0000000000000..0e28f7e69199d --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -0,0 +1,88 @@ +/* + * + * 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.test.ESTestCase; +import org.elasticsearch.transport.TransportRequest; +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.user.User; + +import java.util.Map; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase { + + public void testActionRequestAuthenticationBasedPredicateWhenAuthenticatingWithApiKey() { + final ClusterPermission clusterPermission = + ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); + { + final String apiKeyId = randomAlphaOfLengthBetween(4, 7); + final Authentication authentication = createMockAuthentication("_es_api_key", "_es_api_key", + Map.of("_security_api_key_id", apiKeyId)); + final TransportRequest request = randomFrom(GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()), + InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())); + + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/something", request, authentication)); + } + { + final String apiKeyId = randomAlphaOfLengthBetween(4, 7); + final Authentication authentication = createMockAuthentication("_es_api_key", "_es_api_key", + Map.of("_security_api_key_id", randomAlphaOfLength(7))); + final TransportRequest request = randomFrom(GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()), + InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())); + + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); + } + } + + public void testActionRequestAuthenticationBasedPredicateWhenRequestContainsUsernameAndRealmName() { + final ClusterPermission clusterPermission = + ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); + { + final Authentication authentication = createMockAuthentication("realm1", "native", Map.of()); + final TransportRequest request = randomFrom(GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"), + InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe")); + + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/something", request, authentication)); + } + { + final Authentication authentication = createMockAuthentication("realm1", "native", Map.of()); + final TransportRequest request = randomFrom( + GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), + GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), + InvalidateApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), + InvalidateApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe")); + + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); + } + } + + private Authentication createMockAuthentication(String realmName, String realmType, Map metadata) { + final User user = new User("joe"); + final Authentication authentication = mock(Authentication.class); + final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); + when(authentication.getUser()).thenReturn(user); + when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); + when(authenticatedBy.getName()).thenReturn(realmName); + when(authenticatedBy.getType()).thenReturn(realmType); + when(authentication.getMetadata()).thenReturn(metadata); + return authentication; + } +} From efc2c2b784a2189536499d512e22938be28be05e Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Fri, 16 Aug 2019 13:11:16 +1000 Subject: [PATCH 09/27] add tests --- .../ManageOwnApiKeyClusterPrivilege.java | 10 ++--- .../action/TransportGetApiKeyAction.java | 5 +++ .../TransportInvalidateApiKeyAction.java | 4 ++ .../security/authz/AuthorizationService.java | 9 ++++ .../security/authc/ApiKeyIntegTests.java | 45 +++++++++++++++++-- 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index d5e479612efdf..992028858e75e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -26,11 +26,10 @@ public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege { public static final ManageOwnApiKeyClusterPrivilege INSTANCE = new ManageOwnApiKeyClusterPrivilege(); private static final Predicate ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/api_key/*"); - private final String name; + private static final String PRIVILEGE_NAME = "manage_own_api_key"; private final BiPredicate requestAuthnPredicate; private ManageOwnApiKeyClusterPrivilege() { - this.name = "manage_own_api_key"; this.requestAuthnPredicate = (request, authentication) -> { if (request instanceof CreateApiKeyRequest) { return true; @@ -52,8 +51,8 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin return true; } else { /* - * TODO ygaikwad 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 proper error messages in case of access denied. + * 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(); @@ -65,7 +64,6 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin } private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authentication authentication, String apiKeyId) { - // TODO ygaikwad replace with constants after merging other change 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"); @@ -78,7 +76,7 @@ private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authenticati @Override public String name() { - return name; + return PRIVILEGE_NAME; } @Override diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java index 9a3c0ed8326c9..f42d52a21cd3c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java @@ -48,6 +48,11 @@ protected void doExecute(Task task, GetApiKeyRequest request, ActionListener failureListener = new PlainActionFuture<>(); // for any other API key id, it must deny access - client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(1).getId(), false), failureListener); + client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(1).getId(), false), + failureListener); ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener.actionGet()); - assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", SecuritySettingsSource.TEST_SUPERUSER); + assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", SecuritySettingsSource.TEST_SUPERUSER, + responses.get(0).getId()); + } + + public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOtherKeysCreatedBySameOwner() throws InterruptedException + , ExecutionException { + List responses = createApiKeys(SecuritySettingsSource.TEST_SUPERUSER, 2, null, "manage_own_api_key"); + final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString( + (responses.get(0).getId() + ":" + responses.get(0).getKey().toString()).getBytes(StandardCharsets.UTF_8)); + Client client = client().filterWithHeader(Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue)); + PlainActionFuture listener = new PlainActionFuture<>(); + + final PlainActionFuture failureListener = new PlainActionFuture<>(); + // for any other API key id, it must deny access + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(1).getId(), false), + failureListener); + ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener.actionGet()); + assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/invalidate", SecuritySettingsSource.TEST_SUPERUSER, + responses.get(0).getId()); + + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false), + listener); + InvalidateApiKeyResponse invalidateResponse = listener.get(); + + assertThat(invalidateResponse.getInvalidatedApiKeys().size(), equalTo(1)); + assertThat(invalidateResponse.getInvalidatedApiKeys(), containsInAnyOrder(responses.get(0).getId())); + assertThat(invalidateResponse.getPreviouslyInvalidatedApiKeys().size(), equalTo(0)); + assertThat(invalidateResponse.getErrors().size(), equalTo(0)); } private void verifyGetResponse(int expectedNumberOfApiKeys, List responses, @@ -582,13 +610,17 @@ private void verifyGetResponse(String user, int expectedNumberOfApiKeys, List createApiKeys(int noOfApiKeys, TimeValue expiration) { - return createApiKeys(SecuritySettingsSource.TEST_SUPERUSER, noOfApiKeys, expiration); + return createApiKeys(SecuritySettingsSource.TEST_SUPERUSER, noOfApiKeys, expiration, "monitor"); } private List createApiKeys(String user, int noOfApiKeys, TimeValue expiration) { + return createApiKeys(user, noOfApiKeys, expiration, "monitor"); + } + + private List createApiKeys(String user, int noOfApiKeys, TimeValue expiration, String role) { List responses = new ArrayList<>(); for (int i = 0; i < noOfApiKeys; i++) { - final RoleDescriptor descriptor = new RoleDescriptor("role", new String[] { "monitor" }, null, null); + final RoleDescriptor descriptor = new RoleDescriptor("role", new String[] { role }, null, null); Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken .basicAuthHeaderValue(user, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); final CreateApiKeyResponse response = new CreateApiKeyRequestBuilder(client) @@ -602,6 +634,11 @@ private List createApiKeys(String user, int noOfApiKeys, T return responses; } + private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName, String apiKeyId) { + assertThat(ese.getMessage(), + is("action [" + action + "] is unauthorized for API key id [" + apiKeyId + "] of user [" + userName + "]")); + } + private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName) { assertThat(ese.getMessage(), is("action [" + action + "] is unauthorized for user [" + userName + "]")); } From 7db504c8ee78883878d95fc0465467e2a400f404 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 00:00:19 +1000 Subject: [PATCH 10/27] fix tests --- .../docs/en/rest-api/security/get-builtin-privileges.asciidoc | 1 + .../test/resources/rest-api-spec/test/privileges/11_builtin.yml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/docs/en/rest-api/security/get-builtin-privileges.asciidoc b/x-pack/docs/en/rest-api/security/get-builtin-privileges.asciidoc index b6afc70715a55..685f7731371ab 100644 --- a/x-pack/docs/en/rest-api/security/get-builtin-privileges.asciidoc +++ b/x-pack/docs/en/rest-api/security/get-builtin-privileges.asciidoc @@ -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", diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/11_builtin.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/11_builtin.yml index 2e23a85b7e737..df1978f443fc1 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/11_builtin.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/privileges/11_builtin.yml @@ -15,5 +15,5 @@ setup: # This is fragile - it needs to be updated every time we add a new cluster/index privilege # I would much prefer we could just check that specific entries are in the array, but we don't have # an assertion for that - - length: { "cluster" : 28 } + - length: { "cluster" : 29 } - length: { "index" : 16 } From adc6d69898e166e33646a568028a20030e624d23 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 14:17:04 +1000 Subject: [PATCH 11/27] change manage own api cluster privielge --- .../authz/permission/ClusterPermission.java | 4 +- .../ManageOwnApiKeyClusterPrivilege.java | 109 ++++++++++-------- .../ManageOwnApiKeyClusterPrivilegeTests.java | 92 ++++++++------- 3 files changed, 111 insertions(+), 94 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index ec9d9e1014d27..58037bf148e16 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -152,11 +152,11 @@ public interface PermissionCheck { } // Automaton based permission check - private static class AutomatonPermissionCheck implements PermissionCheck { + public static class AutomatonPermissionCheck implements PermissionCheck { private final Automaton automaton; private final Predicate actionPredicate; - AutomatonPermissionCheck(final Automaton automaton) { + public AutomatonPermissionCheck(final Automaton automaton) { this.automaton = automaton; this.actionPredicate = Automatons.predicate(automaton); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index 992028858e75e..d17dcbd764e8d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -17,61 +17,14 @@ import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.support.Automatons; -import java.util.function.BiPredicate; -import java.util.function.Predicate; - /** * 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 Predicate ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/api_key/*"); private static final String PRIVILEGE_NAME = "manage_own_api_key"; - private final BiPredicate requestAuthnPredicate; private ManageOwnApiKeyClusterPrivilege() { - this.requestAuthnPredicate = (request, 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; - }; - } - - 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; } @Override @@ -81,6 +34,64 @@ public String name() { @Override public ClusterPermission.Builder buildPermission(ClusterPermission.Builder builder) { - return builder.add(this, ACTION_PREDICATE, requestAuthnPredicate); + return builder.add(this, ManageOwnClusterPermissionCheck.INSTANCE); + } + + private static final class ManageOwnClusterPermissionCheck extends ClusterPermission.AutomatonPermissionCheck { + public static final ManageOwnClusterPermissionCheck INSTANCE = new ManageOwnClusterPermissionCheck(); + private ManageOwnClusterPermissionCheck() { + super(Automatons.patterns("cluster:admin/xpack/security/api_key/*")); + } + + @Override + public boolean check(final String action, final TransportRequest request, final Authentication authentication) { + if (super.check(action, request, 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 + public boolean implies(final ClusterPermission.PermissionCheck permissionCheck) { + return super.implies(permissionCheck); + } + + 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; + } } -} + } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java index 0e28f7e69199d..619352a90837a 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -23,55 +23,61 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase { - public void testActionRequestAuthenticationBasedPredicateWhenAuthenticatingWithApiKey() { + public void testAuthenticationWithApiKeyAllowsAccessToApiKeyActionsWhenItIsOwner() { final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); - { - final String apiKeyId = randomAlphaOfLengthBetween(4, 7); - final Authentication authentication = createMockAuthentication("_es_api_key", "_es_api_key", - Map.of("_security_api_key_id", apiKeyId)); - final TransportRequest request = randomFrom(GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()), - InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())); - - assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); - assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); - assertFalse(clusterPermission.check("cluster:admin/something", request, authentication)); - } - { - final String apiKeyId = randomAlphaOfLengthBetween(4, 7); - final Authentication authentication = createMockAuthentication("_es_api_key", "_es_api_key", - Map.of("_security_api_key_id", randomAlphaOfLength(7))); - final TransportRequest request = randomFrom(GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()), - InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())); - - assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); - assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); - } + + final String apiKeyId = randomAlphaOfLengthBetween(4, 7); + final Authentication authentication = createMockAuthentication("_es_api_key", "_es_api_key", + Map.of("_security_api_key_id", apiKeyId)); + final TransportRequest request = randomFrom(GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()), + InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())); + + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/something", request, authentication)); + } + + public void testAuthenticationWithApiKeyDeniesAccessToApiKeyActionsWhenItIsNotOwner() { + final ClusterPermission clusterPermission = + ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); + + final String apiKeyId = randomAlphaOfLengthBetween(4, 7); + final Authentication authentication = createMockAuthentication("_es_api_key", "_es_api_key", + Map.of("_security_api_key_id", randomAlphaOfLength(7))); + final TransportRequest request = randomFrom(GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()), + InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())); + + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); } - public void testActionRequestAuthenticationBasedPredicateWhenRequestContainsUsernameAndRealmName() { + public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner() { final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); - { - final Authentication authentication = createMockAuthentication("realm1", "native", Map.of()); - final TransportRequest request = randomFrom(GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"), - InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe")); - - assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); - assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); - assertFalse(clusterPermission.check("cluster:admin/something", request, authentication)); - } - { - final Authentication authentication = createMockAuthentication("realm1", "native", Map.of()); - final TransportRequest request = randomFrom( - GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), - GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), - InvalidateApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), - InvalidateApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe")); - - assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); - assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); - } + + final Authentication authentication = createMockAuthentication("realm1", "native", Map.of()); + final TransportRequest request = randomFrom(GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"), + InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe")); + + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/something", request, authentication)); + } + + public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsNotOwner() { + final ClusterPermission clusterPermission = + ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); + + final Authentication authentication = createMockAuthentication("realm1", "native", Map.of()); + final TransportRequest request = randomFrom( + GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), + GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), + InvalidateApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), + InvalidateApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe")); + + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); } private Authentication createMockAuthentication(String realmName, String realmType, Map metadata) { From 2497c6fa3edd467431fad320b0b4a2614adc7791 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 18:18:18 +1000 Subject: [PATCH 12/27] precommit error --- .../authz/privilege/ManageOwnApiKeyClusterPrivilege.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index d17dcbd764e8d..dba007c0ee004 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -39,6 +39,7 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build private static final class ManageOwnClusterPermissionCheck extends ClusterPermission.AutomatonPermissionCheck { public static final ManageOwnClusterPermissionCheck INSTANCE = new ManageOwnClusterPermissionCheck(); + private ManageOwnClusterPermissionCheck() { super(Automatons.patterns("cluster:admin/xpack/security/api_key/*")); } @@ -54,7 +55,8 @@ public boolean check(final String action, final TransportRequest request, final getApiKeyRequest.getRealmName()); } else if (request instanceof InvalidateApiKeyRequest) { final InvalidateApiKeyRequest invalidateApiKeyRequest = (InvalidateApiKeyRequest) request; - return checkIfUserIsOwnerOfApiKeys(authentication, invalidateApiKeyRequest.getId(), invalidateApiKeyRequest.getUserName(), + return checkIfUserIsOwnerOfApiKeys(authentication, invalidateApiKeyRequest.getId(), + invalidateApiKeyRequest.getUserName(), invalidateApiKeyRequest.getRealmName()); } } @@ -94,4 +96,4 @@ private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authenticati return false; } } - } +} From 6bd259acd8ed7f7f462a0ea104a171182f29b8bf Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Tue, 20 Aug 2019 19:08:46 +1000 Subject: [PATCH 13/27] no need to override --- .../authz/privilege/ManageOwnApiKeyClusterPrivilege.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index dba007c0ee004..3bf104736fe86 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -63,11 +63,6 @@ public boolean check(final String action, final TransportRequest request, final return false; } - @Override - public boolean implies(final ClusterPermission.PermissionCheck permissionCheck) { - return super.implies(permissionCheck); - } - private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, String apiKeyId, String username, String realmName) { if (isCurrentAuthenticationUsingSameApiKeyIdFromRequest(authentication, apiKeyId)) { return true; From 508a718789706b3fea5f8306c01f1df808468644 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Wed, 21 Aug 2019 23:41:15 +1000 Subject: [PATCH 14/27] address review comment --- .../action/TransportGetApiKeyAction.java | 11 ++++---- .../TransportInvalidateApiKeyAction.java | 10 +++---- .../xpack/security/authc/ApiKeyService.java | 3 +++ .../security/authc/ApiKeyIntegTests.java | 26 ++++++++++++++----- 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java index f42d52a21cd3c..847cdce9b95ef 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java @@ -48,14 +48,13 @@ protected void doExecute(Task task, GetApiKeyRequest request, ActionListener PASSWORD_HASHING_ALGORITHM = new Setting<>( "xpack.security.authc.api_key.hashing.algorithm", "pbkdf2", Function.identity(), v -> { if (Hasher.getAvailableAlgoStoredHash().contains(v.toLowerCase(Locale.ROOT)) == false) { @@ -520,6 +522,7 @@ private void validateApiKeyExpiration(Map source, ApiKeyCredenti : limitedByRoleDescriptors.keySet().toArray(Strings.EMPTY_ARRAY); final User apiKeyUser = new User(principal, roleNames, null, null, metadata, true); final Map authResultMetadata = new HashMap<>(); + authResultMetadata.put(API_KEY_CREATOR_REALM, creator.get("realm")); authResultMetadata.put(API_KEY_ROLE_DESCRIPTORS_KEY, roleDescriptors); authResultMetadata.put(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, limitedByRoleDescriptors); authResultMetadata.put(API_KEY_ID_KEY, credentials.getId()); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index 06c327bd1a910..26d54b37a2777 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -533,23 +533,29 @@ public void testInvalidateApiKeysOwnedByCurrentAuthenticatedUser() throws Interr verifyInvalidateResponse(noOfApiKeysForUserWithManageApiKeyRole, userWithManageApiKeyRoleApiKeys, invalidateResponse); } - public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformation() throws InterruptedException, ExecutionException { + public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformationButNotAnyOtherKeysCreatedBySameOwner() throws InterruptedException, ExecutionException { List responses = createApiKeys(2, null); final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString( (responses.get(0).getId() + ":" + responses.get(0).getKey().toString()).getBytes(StandardCharsets.UTF_8)); Client client = client().filterWithHeader(Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue)); PlainActionFuture listener = new PlainActionFuture<>(); - client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false), listener); + client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(0).getId(), randomBoolean()), listener); GetApiKeyResponse response = listener.get(); verifyGetResponse(1, responses, response, Collections.singleton(responses.get(0).getId()), null); final PlainActionFuture failureListener = new PlainActionFuture<>(); // for any other API key id, it must deny access - client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(1).getId(), false), + client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(1).getId(), randomBoolean()), failureListener); ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener.actionGet()); assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", SecuritySettingsSource.TEST_SUPERUSER, responses.get(0).getId()); + + final PlainActionFuture failureListener1 = new PlainActionFuture<>(); + client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.forOwnedApiKeys(), failureListener1); + ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener1.actionGet()); + assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", SecuritySettingsSource.TEST_SUPERUSER, + responses.get(0).getId()); } public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOtherKeysCreatedBySameOwner() throws InterruptedException @@ -558,17 +564,23 @@ public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOth final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString( (responses.get(0).getId() + ":" + responses.get(0).getKey().toString()).getBytes(StandardCharsets.UTF_8)); Client client = client().filterWithHeader(Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue)); - PlainActionFuture listener = new PlainActionFuture<>(); final PlainActionFuture failureListener = new PlainActionFuture<>(); // for any other API key id, it must deny access - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(1).getId(), false), + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(1).getId(), randomBoolean()), failureListener); ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener.actionGet()); assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/invalidate", SecuritySettingsSource.TEST_SUPERUSER, responses.get(0).getId()); - client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false), + final PlainActionFuture failureListener1 = new PlainActionFuture<>(); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.forOwnedApiKeys(), failureListener1); + ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener1.actionGet()); + assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/invalidate", SecuritySettingsSource.TEST_SUPERUSER, + responses.get(0).getId()); + + PlainActionFuture listener = new PlainActionFuture<>(); + client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(0).getId(), randomBoolean()), listener); InvalidateApiKeyResponse invalidateResponse = listener.get(); @@ -576,6 +588,8 @@ public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOth assertThat(invalidateResponse.getInvalidatedApiKeys(), containsInAnyOrder(responses.get(0).getId())); assertThat(invalidateResponse.getPreviouslyInvalidatedApiKeys().size(), equalTo(0)); assertThat(invalidateResponse.getErrors().size(), equalTo(0)); + + } private void verifyGetResponse(int expectedNumberOfApiKeys, List responses, From 29cefd284edeea4ea3e495bd4ac76331a45b5b17 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 22 Aug 2019 00:06:02 +1000 Subject: [PATCH 15/27] add a base class for action based permission check to enforce checks --- .../authz/permission/ClusterPermission.java | 62 +++++++++++++------ .../ManageOwnApiKeyClusterPrivilege.java | 33 +++++----- .../security/authc/ApiKeyIntegTests.java | 7 ++- 3 files changed, 66 insertions(+), 36 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index 58037bf148e16..be969320ebcb5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -151,32 +151,60 @@ public interface PermissionCheck { boolean implies(PermissionCheck otherPermissionCheck); } - // Automaton based permission check - public 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 actionPredicate; - public 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, final Authentication authentication) { - 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); } + public 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; } + + public abstract boolean doImplies(PermissionCheck permissionCheck); + } + + // Automaton based permission check + private static class AutomatonPermissionCheck extends ActionBasedPermissionCheck { + + AutomatonPermissionCheck(final Automaton automaton) { + super(automaton); + } + + @Override + public boolean doCheck(String action, TransportRequest request, Authentication authentication) { + return true; + } + + @Override + public boolean doImplies(PermissionCheck permissionCheck) { + return permissionCheck instanceof AutomatonPermissionCheck; + } + } // action, request based permission check - private static class ActionRequestBasedPermissionCheck extends AutomatonPermissionCheck { + private static class ActionRequestBasedPermissionCheck extends ActionBasedPermissionCheck { private final ClusterPrivilege clusterPrivilege; private final Predicate requestPredicate; @@ -188,18 +216,16 @@ private static class ActionRequestBasedPermissionCheck extends AutomatonPermissi } @Override - public boolean check(final String action, final TransportRequest request, final Authentication authentication) { - return super.check(action, request, authentication) && requestPredicate.test(request); + public boolean doCheck(String action, TransportRequest request, Authentication authentication) { + return requestPredicate.test(request); } @Override - public boolean implies(final PermissionCheck permissionCheck) { - if (super.implies(permissionCheck)) { - if (permissionCheck instanceof ActionRequestBasedPermissionCheck) { - final ActionRequestBasedPermissionCheck otherCheck = - (ActionRequestBasedPermissionCheck) permissionCheck; - return this.clusterPrivilege.equals(otherCheck.clusterPrivilege); - } + public boolean doImplies(final PermissionCheck permissionCheck) { + if (permissionCheck instanceof ActionRequestBasedPermissionCheck) { + final ActionRequestBasedPermissionCheck otherCheck = + (ActionRequestBasedPermissionCheck) permissionCheck; + return this.clusterPrivilege.equals(otherCheck.clusterPrivilege); } return false; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index 3bf104736fe86..3ba46bf90f220 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -37,7 +37,7 @@ public ClusterPermission.Builder buildPermission(ClusterPermission.Builder build return builder.add(this, ManageOwnClusterPermissionCheck.INSTANCE); } - private static final class ManageOwnClusterPermissionCheck extends ClusterPermission.AutomatonPermissionCheck { + private static final class ManageOwnClusterPermissionCheck extends ClusterPermission.ActionBasedPermissionCheck { public static final ManageOwnClusterPermissionCheck INSTANCE = new ManageOwnClusterPermissionCheck(); private ManageOwnClusterPermissionCheck() { @@ -45,24 +45,27 @@ private ManageOwnClusterPermissionCheck() { } @Override - public boolean check(final String action, final TransportRequest request, final Authentication authentication) { - if (super.check(action, request, 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()); - } + public 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 + public 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; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index 26d54b37a2777..a2cf94ed3b30a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -533,7 +533,8 @@ public void testInvalidateApiKeysOwnedByCurrentAuthenticatedUser() throws Interr verifyInvalidateResponse(noOfApiKeysForUserWithManageApiKeyRole, userWithManageApiKeyRoleApiKeys, invalidateResponse); } - public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformationButNotAnyOtherKeysCreatedBySameOwner() throws InterruptedException, ExecutionException { + public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformationButNotAnyOtherKeysCreatedBySameOwner() + throws InterruptedException, ExecutionException { List responses = createApiKeys(2, null); final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString( (responses.get(0).getId() + ":" + responses.get(0).getKey().toString()).getBytes(StandardCharsets.UTF_8)); @@ -558,8 +559,8 @@ public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformationBu responses.get(0).getId()); } - public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOtherKeysCreatedBySameOwner() throws InterruptedException - , ExecutionException { + public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOtherKeysCreatedBySameOwner() + throws InterruptedException, ExecutionException { List responses = createApiKeys(SecuritySettingsSource.TEST_SUPERUSER, 2, null, "manage_own_api_key"); final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString( (responses.get(0).getId() + ":" + responses.get(0).getKey().toString()).getBytes(StandardCharsets.UTF_8)); From 0eba55f7eda2ad4d24b0d1f05930169d0686c7a1 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 22 Aug 2019 00:10:31 +1000 Subject: [PATCH 16/27] protected instead of public --- .../security/authz/permission/ClusterPermission.java | 12 ++++++------ .../privilege/ManageOwnApiKeyClusterPrivilege.java | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index be969320ebcb5..4d71a5791e55b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -170,7 +170,7 @@ public final boolean check(final String action, final TransportRequest request, return actionPredicate.test(action) && doCheck(action, request, authentication); } - public abstract boolean doCheck(String action, TransportRequest request, Authentication authentication); + protected abstract boolean doCheck(String action, TransportRequest request, Authentication authentication); @Override public final boolean implies(final PermissionCheck permissionCheck) { @@ -181,7 +181,7 @@ public final boolean implies(final PermissionCheck permissionCheck) { return false; } - public abstract boolean doImplies(PermissionCheck permissionCheck); + protected abstract boolean doImplies(PermissionCheck permissionCheck); } // Automaton based permission check @@ -192,12 +192,12 @@ private static class AutomatonPermissionCheck extends ActionBasedPermissionCheck } @Override - public boolean doCheck(String action, TransportRequest request, Authentication authentication) { + protected boolean doCheck(String action, TransportRequest request, Authentication authentication) { return true; } @Override - public boolean doImplies(PermissionCheck permissionCheck) { + protected boolean doImplies(PermissionCheck permissionCheck) { return permissionCheck instanceof AutomatonPermissionCheck; } @@ -216,12 +216,12 @@ private static class ActionRequestBasedPermissionCheck extends ActionBasedPermis } @Override - public boolean doCheck(String action, TransportRequest request, Authentication authentication) { + protected boolean doCheck(String action, TransportRequest request, Authentication authentication) { return requestPredicate.test(request); } @Override - public boolean doImplies(final PermissionCheck permissionCheck) { + protected boolean doImplies(final PermissionCheck permissionCheck) { if (permissionCheck instanceof ActionRequestBasedPermissionCheck) { final ActionRequestBasedPermissionCheck otherCheck = (ActionRequestBasedPermissionCheck) permissionCheck; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index 3ba46bf90f220..19a585c38e6b9 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -45,7 +45,7 @@ private ManageOwnClusterPermissionCheck() { } @Override - public boolean doCheck(String action, TransportRequest request, Authentication authentication) { + protected boolean doCheck(String action, TransportRequest request, Authentication authentication) { if (request instanceof CreateApiKeyRequest) { return true; } else if (request instanceof GetApiKeyRequest) { @@ -62,7 +62,7 @@ public boolean doCheck(String action, TransportRequest request, Authentication a } @Override - public boolean doImplies(ClusterPermission.PermissionCheck permissionCheck) { + protected boolean doImplies(ClusterPermission.PermissionCheck permissionCheck) { return permissionCheck instanceof ManageOwnClusterPermissionCheck; } From 69b56c63750e56389b4d326c0f947036023d3f4b Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 22 Aug 2019 08:04:34 +1000 Subject: [PATCH 17/27] add unit test check for creator.realm --- .../elasticsearch/xpack/security/authc/ApiKeyServiceTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java index 0491d20d74c8a..031f5ccec0696 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java @@ -191,6 +191,7 @@ public void testValidateApiKey() throws Exception { sourceMap.put("limited_by_role_descriptors", Collections.singletonMap("limited role", Collections.singletonMap("cluster", "all"))); Map creatorMap = new HashMap<>(); creatorMap.put("principal", "test_user"); + creatorMap.put("realm", "realm1"); creatorMap.put("metadata", Collections.emptyMap()); sourceMap.put("creator", creatorMap); sourceMap.put("api_key_invalidated", false); @@ -209,6 +210,7 @@ public void testValidateApiKey() throws Exception { assertThat(result.getMetadata().get(ApiKeyService.API_KEY_ROLE_DESCRIPTORS_KEY), equalTo(sourceMap.get("role_descriptors"))); assertThat(result.getMetadata().get(ApiKeyService.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), equalTo(sourceMap.get("limited_by_role_descriptors"))); + assertThat(result.getMetadata().get(ApiKeyService.API_KEY_CREATOR_REALM), is("realm1")); sourceMap.put("expiration_time", Clock.systemUTC().instant().plus(1L, ChronoUnit.HOURS).toEpochMilli()); future = new PlainActionFuture<>(); @@ -222,6 +224,7 @@ public void testValidateApiKey() throws Exception { assertThat(result.getMetadata().get(ApiKeyService.API_KEY_ROLE_DESCRIPTORS_KEY), equalTo(sourceMap.get("role_descriptors"))); assertThat(result.getMetadata().get(ApiKeyService.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY), equalTo(sourceMap.get("limited_by_role_descriptors"))); + assertThat(result.getMetadata().get(ApiKeyService.API_KEY_CREATOR_REALM), is("realm1")); sourceMap.put("expiration_time", Clock.systemUTC().instant().minus(1L, ChronoUnit.HOURS).toEpochMilli()); future = new PlainActionFuture<>(); From a94fa92a79ce9d2a571875a0e941e7a79e1f330d Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 22 Aug 2019 08:08:43 +1000 Subject: [PATCH 18/27] make the condition explicit --- .../authz/privilege/ManageOwnApiKeyClusterPrivilege.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index 19a585c38e6b9..690dad207cf24 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -76,7 +76,10 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin */ String authenticatedUserPrincipal = authentication.getUser().principal(); String authenticatedUserRealm = authentication.getAuthenticatedBy().getName(); - if (Strings.hasText(username) && Strings.hasText(realmName)) { + if (authenticatedUserRealm.equals("_es_api_key")) { + // API key cannot own any other API key so deny access + return false; + } else if (Strings.hasText(username) && Strings.hasText(realmName)) { return username.equals(authenticatedUserPrincipal) && realmName.equals(authenticatedUserRealm); } } From 73498ead08fe519f1802c26c626921afd24b2922 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 22 Aug 2019 13:09:22 +1000 Subject: [PATCH 19/27] address review comments --- .../authz/permission/ClusterPermission.java | 16 ++++++++-------- .../ManageOwnApiKeyClusterPrivilege.java | 12 +++++++----- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java index 4d71a5791e55b..10d474ac3a760 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java @@ -167,21 +167,21 @@ public ActionBasedPermissionCheck(final Automaton automaton) { @Override public final boolean check(final String action, final TransportRequest request, final Authentication authentication) { - return actionPredicate.test(action) && doCheck(action, request, authentication); + return actionPredicate.test(action) && extendedCheck(action, request, authentication); } - protected abstract boolean doCheck(String action, TransportRequest request, Authentication authentication); + protected abstract boolean extendedCheck(String action, TransportRequest request, Authentication authentication); @Override public final boolean implies(final PermissionCheck permissionCheck) { if (permissionCheck instanceof ActionBasedPermissionCheck) { return Operations.subsetOf(((ActionBasedPermissionCheck) permissionCheck).automaton, this.automaton) && - doImplies(permissionCheck); + doImplies((ActionBasedPermissionCheck) permissionCheck); } return false; } - protected abstract boolean doImplies(PermissionCheck permissionCheck); + protected abstract boolean doImplies(ActionBasedPermissionCheck permissionCheck); } // Automaton based permission check @@ -192,12 +192,12 @@ private static class AutomatonPermissionCheck extends ActionBasedPermissionCheck } @Override - protected boolean doCheck(String action, TransportRequest request, Authentication authentication) { + protected boolean extendedCheck(String action, TransportRequest request, Authentication authentication) { return true; } @Override - protected boolean doImplies(PermissionCheck permissionCheck) { + protected boolean doImplies(ActionBasedPermissionCheck permissionCheck) { return permissionCheck instanceof AutomatonPermissionCheck; } @@ -216,12 +216,12 @@ private static class ActionRequestBasedPermissionCheck extends ActionBasedPermis } @Override - protected boolean doCheck(String action, TransportRequest request, Authentication authentication) { + protected boolean extendedCheck(String action, TransportRequest request, Authentication authentication) { return requestPredicate.test(request); } @Override - protected boolean doImplies(final PermissionCheck permissionCheck) { + protected boolean doImplies(final ActionBasedPermissionCheck permissionCheck) { if (permissionCheck instanceof ActionRequestBasedPermissionCheck) { final ActionRequestBasedPermissionCheck otherCheck = (ActionRequestBasedPermissionCheck) permissionCheck; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index 690dad207cf24..e571097fa7747 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -23,6 +23,8 @@ public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege { public static final ManageOwnApiKeyClusterPrivilege INSTANCE = new ManageOwnApiKeyClusterPrivilege(); private static final String PRIVILEGE_NAME = "manage_own_api_key"; + private static final String API_KEY_REALM_TYPE = "_es_api_key"; + private static final String API_KEY_ID_KEY = "_security_api_key_id"; private ManageOwnApiKeyClusterPrivilege() { } @@ -45,7 +47,7 @@ private ManageOwnClusterPermissionCheck() { } @Override - protected boolean doCheck(String action, TransportRequest request, Authentication authentication) { + protected boolean extendedCheck(String action, TransportRequest request, Authentication authentication) { if (request instanceof CreateApiKeyRequest) { return true; } else if (request instanceof GetApiKeyRequest) { @@ -62,7 +64,7 @@ protected boolean doCheck(String action, TransportRequest request, Authenticatio } @Override - protected boolean doImplies(ClusterPermission.PermissionCheck permissionCheck) { + protected boolean doImplies(ClusterPermission.ActionBasedPermissionCheck permissionCheck) { return permissionCheck instanceof ManageOwnClusterPermissionCheck; } @@ -76,7 +78,7 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin */ String authenticatedUserPrincipal = authentication.getUser().principal(); String authenticatedUserRealm = authentication.getAuthenticatedBy().getName(); - if (authenticatedUserRealm.equals("_es_api_key")) { + if (authenticatedUserRealm.equals(API_KEY_REALM_TYPE)) { // API key cannot own any other API key so deny access return false; } else if (Strings.hasText(username) && Strings.hasText(realmName)) { @@ -87,9 +89,9 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin } private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authentication authentication, String apiKeyId) { - if (authentication.getAuthenticatedBy().getType().equals("_es_api_key")) { + if (authentication.getAuthenticatedBy().getType().equals(API_KEY_REALM_TYPE)) { // API key id from authentication must match the id from request - String authenticatedApiKeyId = (String) authentication.getMetadata().get("_security_api_key_id"); + String authenticatedApiKeyId = (String) authentication.getMetadata().get(API_KEY_ID_KEY); if (Strings.hasText(apiKeyId)) { return apiKeyId.equals(authenticatedApiKeyId); } From 5c9c422abe70e92b4afee311a26dde44ee1be504 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 22 Aug 2019 22:17:07 +1000 Subject: [PATCH 20/27] test name and test correctness --- .../ManageOwnApiKeyClusterPrivilegeTests.java | 53 ++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java index 619352a90837a..dbccc2daefc21 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -28,14 +28,14 @@ public void testAuthenticationWithApiKeyAllowsAccessToApiKeyActionsWhenItIsOwner ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); final String apiKeyId = randomAlphaOfLengthBetween(4, 7); - final Authentication authentication = createMockAuthentication("_es_api_key", "_es_api_key", + final Authentication authentication = createMockAuthentication("joe","_es_api_key", "_es_api_key", Map.of("_security_api_key_id", apiKeyId)); - final TransportRequest request = randomFrom(GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()), - InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())); + final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); + final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); - assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); - assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); - assertFalse(clusterPermission.check("cluster:admin/something", request, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); + assertFalse(clusterPermission.check("cluster:admin/something", mock(TransportRequest.class), authentication)); } public void testAuthenticationWithApiKeyDeniesAccessToApiKeyActionsWhenItIsNotOwner() { @@ -43,45 +43,48 @@ public void testAuthenticationWithApiKeyDeniesAccessToApiKeyActionsWhenItIsNotOw ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); final String apiKeyId = randomAlphaOfLengthBetween(4, 7); - final Authentication authentication = createMockAuthentication("_es_api_key", "_es_api_key", + final Authentication authentication = createMockAuthentication("joe","_es_api_key", "_es_api_key", Map.of("_security_api_key_id", randomAlphaOfLength(7))); - final TransportRequest request = randomFrom(GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()), - InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())); + final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); + final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); - assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); - assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); } public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner() { final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); - final Authentication authentication = createMockAuthentication("realm1", "native", Map.of()); - final TransportRequest request = randomFrom(GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"), - InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe")); + final Authentication authentication = createMockAuthentication("joe","realm1", "native", Map.of()); + final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"); + final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe"); - assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); - assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); - assertFalse(clusterPermission.check("cluster:admin/something", request, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); + assertFalse(clusterPermission.check("cluster:admin/something", mock(TransportRequest.class), authentication)); } - public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsNotOwner() { + public void testAuthenticationWithUserDeniesAccessToApiKeyActionsWhenItIsNotOwner() { final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); - final Authentication authentication = createMockAuthentication("realm1", "native", Map.of()); - final TransportRequest request = randomFrom( + final Authentication authentication = createMockAuthentication("joe", "realm1", "native", Map.of()); + final TransportRequest getApiKeyRequest = randomFrom( GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), + new GetApiKeyRequest(randomAlphaOfLength(5), randomAlphaOfLength(7), null, null, randomBoolean())); + final TransportRequest invalidateApiKeyRequest = randomFrom( InvalidateApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), - InvalidateApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe")); + InvalidateApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), + new InvalidateApiKeyRequest(randomAlphaOfLength(5), randomAlphaOfLength(7), null, null, randomBoolean())); - assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", request, authentication)); - assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", request, authentication)); + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); + assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); } - private Authentication createMockAuthentication(String realmName, String realmType, Map metadata) { - final User user = new User("joe"); + private Authentication createMockAuthentication(String username, String realmName, String realmType, Map metadata) { + final User user = new User(username); final Authentication authentication = mock(Authentication.class); final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); when(authentication.getUser()).thenReturn(user); From 45b019265f6f031641ffc0db2aa6942507e43b33 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Thu, 22 Aug 2019 23:07:38 +1000 Subject: [PATCH 21/27] remove extra blank lines --- .../elasticsearch/xpack/security/authc/ApiKeyIntegTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index a2cf94ed3b30a..c18f85b7f155e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -589,8 +589,6 @@ public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOth assertThat(invalidateResponse.getInvalidatedApiKeys(), containsInAnyOrder(responses.get(0).getId())); assertThat(invalidateResponse.getPreviouslyInvalidatedApiKeys().size(), equalTo(0)); assertThat(invalidateResponse.getErrors().size(), equalTo(0)); - - } private void verifyGetResponse(int expectedNumberOfApiKeys, List responses, From dc512c23b76fb0e3c1f152323bbc886cd1bfa66a Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Fri, 23 Aug 2019 01:22:40 +1000 Subject: [PATCH 22/27] check owner flag in manage own cluster privilege, add tests --- .../ManageOwnApiKeyClusterPrivilege.java | 11 ++++++---- .../ManageOwnApiKeyClusterPrivilegeTests.java | 10 +++++---- .../security/authc/ApiKeyIntegTests.java | 22 ++++++++++++------- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index e571097fa7747..1daf08fb3672a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -53,12 +53,12 @@ protected boolean extendedCheck(String action, TransportRequest request, Authent } else if (request instanceof GetApiKeyRequest) { final GetApiKeyRequest getApiKeyRequest = (GetApiKeyRequest) request; return checkIfUserIsOwnerOfApiKeys(authentication, getApiKeyRequest.getApiKeyId(), getApiKeyRequest.getUserName(), - getApiKeyRequest.getRealmName()); + getApiKeyRequest.getRealmName(), getApiKeyRequest.ownedByAuthenticatedUser()); } else if (request instanceof InvalidateApiKeyRequest) { final InvalidateApiKeyRequest invalidateApiKeyRequest = (InvalidateApiKeyRequest) request; return checkIfUserIsOwnerOfApiKeys(authentication, invalidateApiKeyRequest.getId(), - invalidateApiKeyRequest.getUserName(), - invalidateApiKeyRequest.getRealmName()); + invalidateApiKeyRequest.getUserName(), invalidateApiKeyRequest.getRealmName(), + invalidateApiKeyRequest.ownedByAuthenticatedUser()); } return false; } @@ -68,7 +68,8 @@ protected boolean doImplies(ClusterPermission.ActionBasedPermissionCheck permiss return permissionCheck instanceof ManageOwnClusterPermissionCheck; } - private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, String apiKeyId, String username, String realmName) { + private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, String apiKeyId, String username, String realmName, + boolean ownedByAuthenticatedUser) { if (isCurrentAuthenticationUsingSameApiKeyIdFromRequest(authentication, apiKeyId)) { return true; } else { @@ -81,6 +82,8 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin if (authenticatedUserRealm.equals(API_KEY_REALM_TYPE)) { // API key cannot own any other API key so deny access return false; + } else if (ownedByAuthenticatedUser) { + return true; } else if (Strings.hasText(username) && Strings.hasText(realmName)) { return username.equals(authenticatedUserPrincipal) && realmName.equals(authenticatedUserRealm); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java index dbccc2daefc21..c3e017c9294f9 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -57,8 +57,10 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner() ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); final Authentication authentication = createMockAuthentication("joe","realm1", "native", Map.of()); - final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"); - final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe"); + final TransportRequest getApiKeyRequest = randomFrom(GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"), + GetApiKeyRequest.forOwnedApiKeys()); + final TransportRequest invalidateApiKeyRequest = randomFrom(InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe"), + GetApiKeyRequest.forOwnedApiKeys()); assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); @@ -73,11 +75,11 @@ public void testAuthenticationWithUserDeniesAccessToApiKeyActionsWhenItIsNotOwne final TransportRequest getApiKeyRequest = randomFrom( GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), - new GetApiKeyRequest(randomAlphaOfLength(5), randomAlphaOfLength(7), null, null, randomBoolean())); + new GetApiKeyRequest(randomAlphaOfLength(5), randomAlphaOfLength(7), null, null, false)); final TransportRequest invalidateApiKeyRequest = randomFrom( InvalidateApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), InvalidateApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), - new InvalidateApiKeyRequest(randomAlphaOfLength(5), randomAlphaOfLength(7), null, null, randomBoolean())); + new InvalidateApiKeyRequest(randomAlphaOfLength(5), randomAlphaOfLength(7), null, null, false)); assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); assertFalse(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java index c18f85b7f155e..abe9c02c19143 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java @@ -93,7 +93,9 @@ public void wipeSecurityIndex() throws InterruptedException { public String configRoles() { return super.configRoles() + "\n" + "manage_api_key_role:\n" + - " cluster: [\"manage_api_key\"]\n"; + " cluster: [\"manage_api_key\"]\n" + + "manage_own_api_key_role:\n" + + " cluster: [\"manage_own_api_key\"]\n"; } @Override @@ -101,13 +103,15 @@ public String configUsers() { final String usersPasswdHashed = new String( getFastStoredHashAlgoForTests().hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)); return super.configUsers() + - "user_with_manage_api_key_role:" + usersPasswdHashed + "\n"; + "user_with_manage_api_key_role:" + usersPasswdHashed + "\n" + + "user_with_manage_own_api_key_role:" + usersPasswdHashed + "\n"; } @Override public String configUsersRoles() { return super.configUsersRoles() + - "manage_api_key_role:user_with_manage_api_key_role\n"; + "manage_api_key_role:user_with_manage_api_key_role\n" + + "manage_own_api_key_role:user_with_manage_own_api_key_role\n"; } private void awaitApiKeysRemoverCompletion() throws InterruptedException { @@ -505,15 +509,16 @@ public void testGetApiKeysOwnedByCurrentAuthenticatedUser() throws InterruptedEx int noOfSuperuserApiKeys = randomIntBetween(3, 5); int noOfApiKeysForUserWithManageApiKeyRole = randomIntBetween(3, 5); List defaultUserCreatedKeys = createApiKeys(noOfSuperuserApiKeys, null); - List userWithManageApiKeyRoleApiKeys = createApiKeys("user_with_manage_api_key_role", + String userWithManageApiKeyRole = randomFrom("user_with_manage_api_key_role", "user_with_manage_own_api_key_role"); + List userWithManageApiKeyRoleApiKeys = createApiKeys(userWithManageApiKeyRole, noOfApiKeysForUserWithManageApiKeyRole, null); final Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken - .basicAuthHeaderValue("user_with_manage_api_key_role", SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); + .basicAuthHeaderValue(userWithManageApiKeyRole, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); PlainActionFuture listener = new PlainActionFuture<>(); client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.forOwnedApiKeys(), listener); GetApiKeyResponse response = listener.get(); - verifyGetResponse("user_with_manage_api_key_role", noOfApiKeysForUserWithManageApiKeyRole, userWithManageApiKeyRoleApiKeys, + verifyGetResponse(userWithManageApiKeyRole, noOfApiKeysForUserWithManageApiKeyRole, userWithManageApiKeyRoleApiKeys, response, userWithManageApiKeyRoleApiKeys.stream().map(o -> o.getId()).collect(Collectors.toSet()), null); } @@ -521,10 +526,11 @@ public void testInvalidateApiKeysOwnedByCurrentAuthenticatedUser() throws Interr int noOfSuperuserApiKeys = randomIntBetween(3, 5); int noOfApiKeysForUserWithManageApiKeyRole = randomIntBetween(3, 5); List defaultUserCreatedKeys = createApiKeys(noOfSuperuserApiKeys, null); - List userWithManageApiKeyRoleApiKeys = createApiKeys("user_with_manage_api_key_role", + String userWithManageApiKeyRole = randomFrom("user_with_manage_api_key_role", "user_with_manage_own_api_key_role"); + List userWithManageApiKeyRoleApiKeys = createApiKeys(userWithManageApiKeyRole, noOfApiKeysForUserWithManageApiKeyRole, null); final Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken - .basicAuthHeaderValue("user_with_manage_api_key_role", SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); + .basicAuthHeaderValue(userWithManageApiKeyRole, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); PlainActionFuture listener = new PlainActionFuture<>(); client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.forOwnedApiKeys(), listener); From d7400344ff3bc17c6231fa5adb312ceb37431bdd Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Fri, 23 Aug 2019 08:05:18 +1000 Subject: [PATCH 23/27] Update x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java Co-Authored-By: Albert Zaharovits --- .../authz/privilege/ManageOwnApiKeyClusterPrivilege.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index 1daf08fb3672a..d05b68a12b5de 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -79,7 +79,7 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin */ String authenticatedUserPrincipal = authentication.getUser().principal(); String authenticatedUserRealm = authentication.getAuthenticatedBy().getName(); - if (authenticatedUserRealm.equals(API_KEY_REALM_TYPE)) { + if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) { // API key cannot own any other API key so deny access return false; } else if (ownedByAuthenticatedUser) { From 7a9cfb7b497828adae04eddf10b83b09ef7c5356 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Fri, 23 Aug 2019 08:14:23 +1000 Subject: [PATCH 24/27] fix compilation error --- .../authz/privilege/ManageOwnApiKeyClusterPrivilege.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index d05b68a12b5de..f0444dffb0bb7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -79,7 +79,7 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin */ String authenticatedUserPrincipal = authentication.getUser().principal(); String authenticatedUserRealm = authentication.getAuthenticatedBy().getName(); - if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) { + if (authentication.getAuthenticatedBy().getType().equals(API_KEY_REALM_TYPE)) { // API key cannot own any other API key so deny access return false; } else if (ownedByAuthenticatedUser) { From 1a2a371044f8fe78a4e428b6e706bfff6c921a29 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Fri, 23 Aug 2019 12:25:35 +1000 Subject: [PATCH 25/27] address review comments - separate tests to test one thing - common code to extract realm name move to ApiKeyService - final and move variables to where it is being used - test API keys with no cluster privilege for an API key --- .../ManageOwnApiKeyClusterPrivilege.java | 8 ++++---- .../ManageOwnApiKeyClusterPrivilegeTests.java | 19 +++++++++++++++---- .../action/TransportGetApiKeyAction.java | 6 +----- .../TransportInvalidateApiKeyAction.java | 6 +----- .../xpack/security/authc/ApiKeyService.java | 15 +++++++++++++++ .../security/authc/ApiKeyIntegTests.java | 18 +++++------------- 6 files changed, 41 insertions(+), 31 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index f0444dffb0bb7..c307eb4bc1511 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -60,7 +60,7 @@ protected boolean extendedCheck(String action, TransportRequest request, Authent invalidateApiKeyRequest.getUserName(), invalidateApiKeyRequest.getRealmName(), invalidateApiKeyRequest.ownedByAuthenticatedUser()); } - return false; + throw new IllegalArgumentException("manage own api key privilege only supports API key requests"); } @Override @@ -77,14 +77,14 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin * 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 (authentication.getAuthenticatedBy().getType().equals(API_KEY_REALM_TYPE)) { // API key cannot own any other API key so deny access return false; } else if (ownedByAuthenticatedUser) { return true; } else if (Strings.hasText(username) && Strings.hasText(realmName)) { + final String authenticatedUserPrincipal = authentication.getUser().principal(); + final String authenticatedUserRealm = authentication.getAuthenticatedBy().getName(); return username.equals(authenticatedUserPrincipal) && realmName.equals(authenticatedUserRealm); } } @@ -94,7 +94,7 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authentication authentication, String apiKeyId) { if (authentication.getAuthenticatedBy().getType().equals(API_KEY_REALM_TYPE)) { // API key id from authentication must match the id from request - String authenticatedApiKeyId = (String) authentication.getMetadata().get(API_KEY_ID_KEY); + final String authenticatedApiKeyId = (String) authentication.getMetadata().get(API_KEY_ID_KEY); if (Strings.hasText(apiKeyId)) { return apiKeyId.equals(authenticatedApiKeyId); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java index c3e017c9294f9..c6d67b9e00b58 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -57,10 +57,21 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner() ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); final Authentication authentication = createMockAuthentication("joe","realm1", "native", Map.of()); - final TransportRequest getApiKeyRequest = randomFrom(GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"), - GetApiKeyRequest.forOwnedApiKeys()); - final TransportRequest invalidateApiKeyRequest = randomFrom(InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe"), - GetApiKeyRequest.forOwnedApiKeys()); + final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"); + final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe"); + + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); + assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); + assertFalse(clusterPermission.check("cluster:admin/something", mock(TransportRequest.class), authentication)); + } + + public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner_WithOwnerFlagOnly() { + final ClusterPermission clusterPermission = + ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); + + final Authentication authentication = createMockAuthentication("joe","realm1", "native", Map.of()); + final TransportRequest getApiKeyRequest = GetApiKeyRequest.forOwnedApiKeys(); + final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.forOwnedApiKeys(); assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/get", getApiKeyRequest, authentication)); assertTrue(clusterPermission.check("cluster:admin/xpack/security/api_key/invalidate", invalidateApiKeyRequest, authentication)); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java index 847cdce9b95ef..d62241a0f109f 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java @@ -50,11 +50,7 @@ protected void doExecute(Task task, GetApiKeyRequest request, ActionListener defaultUserCreatedKeys = createApiKeys(noOfSuperuserApiKeys, null); String userWithManageApiKeyRole = randomFrom("user_with_manage_api_key_role", "user_with_manage_own_api_key_role"); List userWithManageApiKeyRoleApiKeys = createApiKeys(userWithManageApiKeyRole, - noOfApiKeysForUserWithManageApiKeyRole, null); + noOfApiKeysForUserWithManageApiKeyRole, null, "monitor"); final Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken .basicAuthHeaderValue(userWithManageApiKeyRole, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); @@ -528,7 +528,7 @@ public void testInvalidateApiKeysOwnedByCurrentAuthenticatedUser() throws Interr List defaultUserCreatedKeys = createApiKeys(noOfSuperuserApiKeys, null); String userWithManageApiKeyRole = randomFrom("user_with_manage_api_key_role", "user_with_manage_own_api_key_role"); List userWithManageApiKeyRoleApiKeys = createApiKeys(userWithManageApiKeyRole, - noOfApiKeysForUserWithManageApiKeyRole, null); + noOfApiKeysForUserWithManageApiKeyRole, null, "monitor"); final Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken .basicAuthHeaderValue(userWithManageApiKeyRole, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); @@ -541,7 +541,7 @@ public void testInvalidateApiKeysOwnedByCurrentAuthenticatedUser() throws Interr public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformationButNotAnyOtherKeysCreatedBySameOwner() throws InterruptedException, ExecutionException { - List responses = createApiKeys(2, null); + List responses = createApiKeys(SecuritySettingsSource.TEST_SUPERUSER,2, null, (String[]) null); final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString( (responses.get(0).getId() + ":" + responses.get(0).getKey().toString()).getBytes(StandardCharsets.UTF_8)); Client client = client().filterWithHeader(Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue)); @@ -632,14 +632,10 @@ private List createApiKeys(int noOfApiKeys, TimeValue expi return createApiKeys(SecuritySettingsSource.TEST_SUPERUSER, noOfApiKeys, expiration, "monitor"); } - private List createApiKeys(String user, int noOfApiKeys, TimeValue expiration) { - return createApiKeys(user, noOfApiKeys, expiration, "monitor"); - } - - private List createApiKeys(String user, int noOfApiKeys, TimeValue expiration, String role) { + private List createApiKeys(String user, int noOfApiKeys, TimeValue expiration, String... clusterPrivileges) { List responses = new ArrayList<>(); for (int i = 0; i < noOfApiKeys; i++) { - final RoleDescriptor descriptor = new RoleDescriptor("role", new String[] { role }, null, null); + final RoleDescriptor descriptor = new RoleDescriptor("role", clusterPrivileges, null, null); Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken .basicAuthHeaderValue(user, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); final CreateApiKeyResponse response = new CreateApiKeyRequestBuilder(client) @@ -657,8 +653,4 @@ private void assertErrorMessage(final ElasticsearchSecurityException ese, String assertThat(ese.getMessage(), is("action [" + action + "] is unauthorized for API key id [" + apiKeyId + "] of user [" + userName + "]")); } - - private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName) { - assertThat(ese.getMessage(), is("action [" + action + "] is unauthorized for user [" + userName + "]")); - } } From 43cf4e3f89e3935cffccbf4ba333071194e5f13d Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad Date: Fri, 23 Aug 2019 12:39:43 +1000 Subject: [PATCH 26/27] static method instead of instance method --- .../xpack/security/action/TransportGetApiKeyAction.java | 2 +- .../xpack/security/action/TransportInvalidateApiKeyAction.java | 2 +- .../org/elasticsearch/xpack/security/authc/ApiKeyService.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java index d62241a0f109f..994cb90b5f2b6 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java @@ -50,7 +50,7 @@ protected void doExecute(Task task, GetApiKeyRequest request, ActionListener Date: Fri, 23 Aug 2019 16:15:38 +1000 Subject: [PATCH 27/27] add unsupported class name to exception --- .../authz/privilege/ManageOwnApiKeyClusterPrivilege.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index c307eb4bc1511..bea9b16ebfc1d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -60,7 +60,8 @@ protected boolean extendedCheck(String action, TransportRequest request, Authent invalidateApiKeyRequest.getUserName(), invalidateApiKeyRequest.getRealmName(), invalidateApiKeyRequest.ownedByAuthenticatedUser()); } - throw new IllegalArgumentException("manage own api key privilege only supports API key requests"); + throw new IllegalArgumentException( + "manage own api key privilege only supports API key requests (not " + request.getClass().getName() + ")"); } @Override