From 633aebe749fe00c34dd6da1de235b20678a967c9 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 31 Oct 2024 17:07:36 -0500 Subject: [PATCH] update test --- .../permission/RemoteClusterPermissions.java | 43 ++++++++---------- ...usterApiKeyRoleDescriptorBuilderTests.java | 45 ++++++++++++++++--- .../xpack/security/authc/ApiKeyService.java | 2 - 3 files changed, 57 insertions(+), 33 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java index a29dfadda3fb7..807af32185815 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java @@ -123,16 +123,7 @@ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion out RemoteClusterPermissions copyForOutBoundVersion = new RemoteClusterPermissions(); - // TODO: centralize this and put in a poor mans cache - // find all the privileges that are allowed for the remote cluster version - Set allowedPermissionsPerVersion = allowedRemoteClusterPermissions.entrySet() - .stream() - .filter((entry) -> entry.getKey().onOrBefore(outboundVersion)) - .map(Map.Entry::getValue) - .flatMap(Set::stream) - .map(s -> s.toLowerCase(Locale.ROOT)) - .collect(Collectors.toSet()); - + Set allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(outboundVersion); for (RemoteClusterPermissionGroup group : remoteClusterPermissionGroups) { String[] privileges = group.clusterPrivileges(); List privilegesMutated = new ArrayList<>(privileges.length); @@ -159,14 +150,12 @@ public RemoteClusterPermissions removeUnsupportedPrivileges(TransportVersion out } } } else { - if (logger.isDebugEnabled()) { - logger.debug( - "Removed all remote cluster permissions for remote cluster [{}]. " - + "Due to the remote cluster version, only the following permissions are allowed: {}", - group.remoteClusterAliases(), - allowedPermissionsPerVersion - ); - } + logger.debug( + "Removed all remote cluster permissions for remote cluster [{}]. " + + "Due to the remote cluster version, only the following permissions are allowed: {}", + group.remoteClusterAliases(), + allowedPermissionsPerVersion + ); } } return copyForOutBoundVersion; @@ -188,13 +177,7 @@ public String[] privilegeNames(final String remoteClusterAlias, TransportVersion .collect(Collectors.toSet()); // find all the privileges that are allowed for the remote cluster version - Set allowedPermissionsPerVersion = allowedRemoteClusterPermissions.entrySet() - .stream() - .filter((entry) -> entry.getKey().onOrBefore(remoteClusterVersion)) - .map(Map.Entry::getValue) - .flatMap(Set::stream) - .map(s -> s.toLowerCase(Locale.ROOT)) - .collect(Collectors.toSet()); + Set allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(remoteClusterVersion); // intersect the two sets to get the allowed privileges for the remote cluster version Set allowedPrivileges = new HashSet<>(groupPrivileges); @@ -270,6 +253,16 @@ public List groups() { return Collections.unmodifiableList(remoteClusterPermissionGroups); } + private Set getAllowedPermissionsPerVersion(TransportVersion outboundVersion) { + return allowedRemoteClusterPermissions.entrySet() + .stream() + .filter((entry) -> entry.getKey().onOrBefore(outboundVersion)) + .map(Map.Entry::getValue) + .flatMap(Set::stream) + .map(s -> s.toLowerCase(Locale.ROOT)) + .collect(Collectors.toSet()); + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { for (RemoteClusterPermissionGroup remoteClusterPermissionGroup : remoteClusterPermissionGroups) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java index e4864291793fb..f0db44070d2c4 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java @@ -10,11 +10,16 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.core.Strings; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.transport.TransportRequest; import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParserConfiguration; import org.elasticsearch.xcontent.json.JsonXContent; +import org.elasticsearch.xpack.core.security.authc.AuthenticationTestHelper; 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.RemoteClusterPermissions; +import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilege; +import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; import java.io.IOException; import java.util.List; @@ -27,6 +32,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.mock; public class CrossClusterApiKeyRoleDescriptorBuilderTests extends ESTestCase { @@ -355,13 +361,40 @@ public void testEmptyAccessIsNotAllowed() throws IOException { assertThat(e2.getMessage(), containsString("doesn't support values of type: VALUE_NULL")); } - // TODO: create automaton and test that the permissions are supported instead of checking the names directly - @AwaitsFix(bugUrl = "http://example.com/do/not/merge/me") public void testAPIKeyAllowsAllRemoteClusterPrivilegesForCCS() { - // if users can add remote cluster permissions to a role, then the APIKey should also allow that for that permission - // the inverse however, is not guaranteed. cross_cluster_search exists largely for internal use and is not exposed to the users role - // TODO: create automaton and test that the permissions are supported instead of checking the names directly. - assertTrue(Set.of(CCS_CLUSTER_PRIVILEGE_NAMES).containsAll(RemoteClusterPermissions.getSupportedRemoteClusterPermissions())); + // test to help ensure that at least 1 action that is allowed by the remote cluster permissions are supported by CCS + List actionsToTest = List.of("cluster:monitor/xpack/enrich/esql/resolve_policy", "cluster:monitor/stats/remote"); + // if you add new remote cluster permissions, please define an action we can test to help ensure it is supported by RCS 2.0 + assertThat(actionsToTest.size(), equalTo(RemoteClusterPermissions.getSupportedRemoteClusterPermissions().size())); + + for (String privilege : RemoteClusterPermissions.getSupportedRemoteClusterPermissions()) { + boolean actionPassesRemoteClusterPermissionCheck = false; + ClusterPrivilege clusterPrivilege = ClusterPrivilegeResolver.resolve(privilege); + // each remote cluster privilege has an action to test + for (String action : actionsToTest) { + if (clusterPrivilege.buildPermission(ClusterPermission.builder()) + .build() + .check(action, mock(TransportRequest.class), AuthenticationTestHelper.builder().build())) { + actionPassesRemoteClusterPermissionCheck = true; + break; + } + } + assertTrue(actionPassesRemoteClusterPermissionCheck); + } + // test that the actions pass the privilege check for CCS + for (String privilege : Set.of(CCS_CLUSTER_PRIVILEGE_NAMES)) { + boolean actionPassesRemoteCCSCheck = false; + ClusterPrivilege clusterPrivilege = ClusterPrivilegeResolver.resolve(privilege); + for (String action : actionsToTest) { + if (clusterPrivilege.buildPermission(ClusterPermission.builder()) + .build() + .check(action, mock(TransportRequest.class), AuthenticationTestHelper.builder().build())) { + actionPassesRemoteCCSCheck = true; + break; + } + } + assertTrue(actionPassesRemoteCCSCheck); + } } private static void assertRoleDescriptor( diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 4da5cb4b45f6b..03558e72fdca3 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -831,8 +831,6 @@ static Set maybeRemoveRemotePrivileges( + ". Remote cluster privileges are not supported by all nodes in the cluster." ); } - // TODO: support the additional cases where we are trying to send to somethign like 8.16 that understands remote cluster, - // but does not support the new privilege } return result; }