Skip to content

Commit

Permalink
update test
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakelandis
jakelandis committed Oct 31, 2024
1 parent 2cde2f8 commit 633aebe
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 33 deletions.
43 changes: 18 additions & 25 deletions ...java/org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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<String> allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(outboundVersion);
for (RemoteClusterPermissionGroup group : remoteClusterPermissionGroups) {
String[] privileges = group.clusterPrivileges();
List<String> privilegesMutated = new ArrayList<>(privileges.length);
Expand All @@ -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;
Expand All @@ -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<String> 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<String> allowedPermissionsPerVersion = getAllowedPermissionsPerVersion(remoteClusterVersion);

// intersect the two sets to get the allowed privileges for the remote cluster version
Set<String> allowedPrivileges = new HashSet<>(groupPrivileges);
Expand Down Expand Up @@ -270,6 +253,16 @@ public List<RemoteClusterPermissionGroup> groups() {
return Collections.unmodifiableList(remoteClusterPermissionGroups);
}

private Set<String> 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) {
Expand Down
45 changes: 39 additions & 6 deletions ...earch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand Down Expand Up @@ -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<String> 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(
Expand Down
2 changes: 0 additions & 2 deletions ...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,6 @@ static Set<RoleDescriptor> 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;
}
Expand Down

0 comments on commit 633aebe

Please sign in to comment.