Skip to content

Commit

Permalink
more tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakelandis
jakelandis committed Apr 5, 2024
1 parent 7470e6d commit 947ef38
Show file tree
Hide file tree
Showing 13 changed files with 154 additions and 122 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ public User requireUser() {
@Nullable
public User getUser() {
Authentication authentication = getAuthentication();
if(authentication != null ) {
if (authentication != null) {
if (authentication.isCrossClusterAccess()) {
authentication = getAuthenticationFromCrossClusterAccessMetadata(authentication);
authentication = getAuthenticationFromCrossClusterAccessMetadata(authentication);
}
}
return authentication == null ? null : authentication.getEffectiveSubject().getUser();
Expand Down
8 changes: 6 additions & 2 deletions ...sticsearch/xpack/core/security/action/apikey/CrossClusterApiKeyRoleDescriptorBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@

public class CrossClusterApiKeyRoleDescriptorBuilder {

public static final String[] CCS_CLUSTER_PRIVILEGE_NAMES = { "cross_cluster_search", "monitor_enrich",
"cluster:data/read/esql/open_exchange", "cluster:data/read/esql/exchange", "cluster:admin/xpack/security/user/has_privileges" }; //TODO: don't add this here ... just a hack for now
public static final String[] CCS_CLUSTER_PRIVILEGE_NAMES = {
"cross_cluster_search",
"monitor_enrich",
"cluster:data/read/esql/open_exchange",
"cluster:data/read/esql/exchange",
"cluster:admin/xpack/security/user/has_privileges" }; // TODO: don't add this here ... just a hack for now
public static final String[] CCR_CLUSTER_PRIVILEGE_NAMES = { "cross_cluster_replication" };
public static final String[] CCS_AND_CCR_CLUSTER_PRIVILEGE_NAMES = Stream.concat(
Arrays.stream(CCS_CLUSTER_PRIVILEGE_NAMES),
Expand Down
16 changes: 8 additions & 8 deletions .../core/src/main/java/org/elasticsearch/xpack/core/security/action/role/PutRoleRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ public void addRemoteIndex(
);
}

//TODO: who calls this ?
// public void addRemoteCluster(final String[] privileges, final String[] remoteClusters) {
// remoteClusterPrivileges.add(new RoleDescriptor.RemoteClusterPrivileges(remoteClusters, privileges));
// }
// TODO: who calls this ?
// public void addRemoteCluster(final String[] privileges, final String[] remoteClusters) {
// remoteClusterPrivileges.add(new RoleDescriptor.RemoteClusterPrivileges(remoteClusters, privileges));
// }

public void addIndex(
String[] indices,
Expand Down Expand Up @@ -198,10 +198,10 @@ public boolean hasRemoteIndicesPrivileges() {
return false == remoteIndicesPrivileges.isEmpty();
}

//TODO: who calls this ?
//public boolean hasRemoteClusterPrivileges() {
// return false == remoteClusterPrivileges.isEmpty();
// }
// TODO: who calls this ?
// public boolean hasRemoteClusterPrivileges() {
// return false == remoteClusterPrivileges.isEmpty();
// }

public List<RoleDescriptor.ApplicationResourcePrivileges> applicationPrivileges() {
return Collections.unmodifiableList(applicationPrivileges);
Expand Down
34 changes: 16 additions & 18 deletions ...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,6 @@ public boolean hasClusterPrivileges() {
return clusterPrivileges.length != 0;
}


public boolean hasApplicationPrivileges() {
return applicationPrivileges.length != 0;
}
Expand All @@ -273,10 +272,10 @@ public boolean hasRunAs() {
}

public boolean hasUnsupportedPrivileges() {
// //this is enforced elsewhere
// assert Arrays.stream(this.getRemoteClusterPermissions().privilegeNames("*"))
// .anyMatch(s -> s.equalsIgnoreCase("monitor_enrich") == false) : "monitor_enrich is the only value allowed";
//TODO: make this validation configurable
// //this is enforced elsewhere
// assert Arrays.stream(this.getRemoteClusterPermissions().privilegeNames("*"))
// .anyMatch(s -> s.equalsIgnoreCase("monitor_enrich") == false) : "monitor_enrich is the only value allowed";
// TODO: make this validation configurable

return hasConfigurableClusterPrivileges()
|| hasApplicationPrivileges()
Expand Down Expand Up @@ -714,8 +713,7 @@ private static RemoteIndicesPrivileges parseRemoteIndex(String roleName, XConten
return new RemoteIndicesPrivileges(parsed.indicesPrivileges(), parsed.remoteClusters());
}

private static RemoteClusterPermissions parseRemoteCluster(final String roleName, final XContentParser parser)
throws IOException {
private static RemoteClusterPermissions parseRemoteCluster(final String roleName, final XContentParser parser) throws IOException {
if (parser.currentToken() != XContentParser.Token.START_ARRAY) {
throw new ElasticsearchParseException(
"failed to parse remote_cluster for role [{}]. expected field [{}] value " + "to be an array, but found [{}] instead",
Expand All @@ -724,7 +722,7 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName
parser.currentToken()
);
}
RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions();
RemoteClusterPermissions remoteClusterPermissions = new RemoteClusterPermissions();
String[] privileges = null;
String[] clusters = null;
while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
Expand All @@ -735,14 +733,14 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName
currentFieldName = parser.currentName();
} else if (Fields.PRIVILEGES.match(currentFieldName, parser.getDeprecationHandler())) {
privileges = readStringArray(roleName, parser, false);
//TODO: re-enable this validation
// if (privileges.length != 1 || "monitor_enrich".equals(privileges[0].trim()) == false) {
// throw new ElasticsearchParseException(
// "failed to parse remote_cluster for role [{}]. " + "[monitor_enrich] is the only value allowed for [{}]",
// roleName,
// currentFieldName
// );
// }
// TODO: re-enable this validation
// if (privileges.length != 1 || "monitor_enrich".equals(privileges[0].trim()) == false) {
// throw new ElasticsearchParseException(
// "failed to parse remote_cluster for role [{}]. " + "[monitor_enrich] is the only value allowed for [{}]",
// roleName,
// currentFieldName
// );
// }
} else if (Fields.CLUSTERS.match(currentFieldName, parser.getDeprecationHandler())) {
clusters = readStringArray(roleName, parser, false);
} else {
Expand All @@ -753,12 +751,12 @@ private static RemoteClusterPermissions parseRemoteCluster(final String roleName
);
}
}
if(privileges != null && clusters == null) {
if (privileges != null && clusters == null) {
throw new ElasticsearchParseException(
"failed to parse remote_cluster for role [{}]. [clusters] must be defined when [privileges] are defined ",
roleName
);
} else if( privileges == null && clusters != null) {
} else if (privileges == null && clusters != null) {
throw new ElasticsearchParseException(
"failed to parse remote_cluster for role [{}]. [privileges] must be defined when [clusters] are defined ",
roleName
Expand Down
5 changes: 3 additions & 2 deletions .../org/elasticsearch/xpack/core/security/authz/permission/RemoteClusterPermissionGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
}
* </code>
*/
public class RemoteClusterPermissionGroup implements Writeable,ToXContentObject {
public class RemoteClusterPermissionGroup implements Writeable, ToXContentObject {

private final String[] clusterPrivileges;
private final String[] remoteClusterAliases;
Expand All @@ -37,8 +37,9 @@ public RemoteClusterPermissionGroup(StreamInput in) throws IOException {
remoteClusterAliases = in.readStringArray();
remoteClusterAliasMatcher = StringMatcher.of(remoteClusterAliases);
}

public RemoteClusterPermissionGroup(String[] clusterPrivileges, String[] remoteClusterAliases) {
//TODO: throw an exception if using unsupported cluster privilges
// TODO: throw an exception if using unsupported cluster privilges
this(clusterPrivileges, remoteClusterAliases, StringMatcher.of(remoteClusterAliases));
}

Expand Down
22 changes: 12 additions & 10 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 @@ -50,39 +50,41 @@
*/
public class RemoteClusterPermissions implements Writeable, ToXContentObject {

private final List<RemoteClusterPermissionGroup> remoteClusterPermissionGroups ;
private final List<RemoteClusterPermissionGroup> remoteClusterPermissionGroups;

public static final RemoteClusterPermissions NONE = new RemoteClusterPermissions();

public RemoteClusterPermissions(StreamInput in) throws IOException {
remoteClusterPermissionGroups = in.readCollectionAsList(RemoteClusterPermissionGroup::new);
}
public RemoteClusterPermissions(){

public RemoteClusterPermissions() {
remoteClusterPermissionGroups = new ArrayList<>();
}

public RemoteClusterPermissions addGroup(RemoteClusterPermissionGroup remoteClusterPermissionGroup) {
Objects.requireNonNull(remoteClusterPermissionGroup, "remoteClusterPermissionGroup must not be null");
if(this == NONE) {
if (this == NONE) {
throw new IllegalArgumentException("Cannot add a group to the `NONE` instance");
}
remoteClusterPermissionGroups.add(remoteClusterPermissionGroup);
return this;
}

public String[] privilegeNames(final String remoteClusterAlias) {
return
remoteClusterPermissionGroups.stream()
.filter(group -> group.hasPrivileges(remoteClusterAlias))
.flatMap(groups -> Arrays.stream(groups.clusterPrivileges())).distinct().sorted().toArray(String[]::new);
return remoteClusterPermissionGroups.stream()
.filter(group -> group.hasPrivileges(remoteClusterAlias))
.flatMap(groups -> Arrays.stream(groups.clusterPrivileges()))
.distinct()
.sorted()
.toArray(String[]::new);
}

public boolean hasPrivileges(final String remoteClusterAlias) {
return remoteClusterPermissionGroups.stream()
.anyMatch(remoteIndicesGroup -> remoteIndicesGroup.hasPrivileges(remoteClusterAlias));
return remoteClusterPermissionGroups.stream().anyMatch(remoteIndicesGroup -> remoteIndicesGroup.hasPrivileges(remoteClusterAlias));
}

public boolean hasPrivileges(){
public boolean hasPrivileges() {
return remoteClusterPermissionGroups.isEmpty() == false;
}

Expand Down
29 changes: 14 additions & 15 deletions ...lugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/Role.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,24 +275,23 @@ public Builder addRemoteIndicesGroup(
return this;
}

public Builder addRemoteClusterPermissions(RemoteClusterPermissions remoteClusterPermissions ){
public Builder addRemoteClusterPermissions(RemoteClusterPermissions remoteClusterPermissions) {
Objects.requireNonNull(remoteClusterPermissions, "remoteClusterPermissions must not be null");
assert this.remoteClusterPermissions == null
: "addRemoteClusterPermissions should only be called once";
assert this.remoteClusterPermissions == null : "addRemoteClusterPermissions should only be called once";
if (remoteClusterPermissions.hasPrivileges()) {
Set<String> namedClusterPrivileges = ClusterPrivilegeResolver.names();
for (RemoteClusterPermissionGroup group : remoteClusterPermissions.groups()) {
for (String namedPrivilege : group.clusterPrivileges()) {
//TODO: re-enable this validation (and test)
// if ("monitor_enrich".equals(namedPrivilege) == false) {
// //this should be enforced upstream while defining the role, so the check here too is just in case...
// throw new IllegalArgumentException("Only [monitor_enrich] is supported as a remote cluster privilege");
// }
// // this can never happen, but if we ever expand the list of remote cluster privileges then we want to ensure that
// // only named cluster privileges are supported
// if (namedClusterPrivileges.contains(namedPrivilege) == false) {
// throw new IllegalArgumentException("Unknown cluster privilege [" + namedPrivilege + "]");
// }
// TODO: re-enable this validation (and test)
// if ("monitor_enrich".equals(namedPrivilege) == false) {
// //this should be enforced upstream while defining the role, so the check here too is just in case...
// throw new IllegalArgumentException("Only [monitor_enrich] is supported as a remote cluster privilege");
// }
// // this can never happen, but if we ever expand the list of remote cluster privileges then we want to ensure that
// // only named cluster privileges are supported
// if (namedClusterPrivileges.contains(namedPrivilege) == false) {
// throw new IllegalArgumentException("Unknown cluster privilege [" + namedPrivilege + "]");
// }
}
}
}
Expand Down Expand Up @@ -445,9 +444,9 @@ static SimpleRole buildFromRoleDescriptor(
}

RemoteClusterPermissions remoteClusterPermissions = roleDescriptor.getRemoteClusterPermissions();
for(RemoteClusterPermissionGroup group : remoteClusterPermissions.groups()){
for (RemoteClusterPermissionGroup group : remoteClusterPermissions.groups()) {
final String[] clusterAliases = group.remoteClusterAliases();
//note: this validation only occurs from reserved roles, see the builder for additional general validation
// note: this validation only occurs from reserved roles, see the builder for additional general validation
assert Arrays.equals(new String[] { "*" }, clusterAliases)
: "reserved role should not define remote cluster privileges for specific clusters";
}
Expand Down
2 changes: 1 addition & 1 deletion ...core/src/main/java/org/elasticsearch/xpack/core/security/authz/permission/SimpleRole.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public RoleDescriptorsIntersection getRoleDescriptorsIntersectionForRemoteCluste
&& remoteClusterPermissions.hasPrivileges(remoteClusterAlias) == false) {
return RoleDescriptorsIntersection.EMPTY;
}
//TODO: test the case where only remote cluster permissions are present, but no indices permissions are defined (and vice versa)
// TODO: test the case where only remote cluster permissions are present, but no indices permissions are defined (and vice versa)
final List<RoleDescriptor.IndicesPrivileges> indicesPrivileges = new ArrayList<>();
for (RemoteIndicesPermission.RemoteIndicesGroup remoteIndicesGroup : remoteIndicesPermission.remoteIndicesGroups()) {
for (IndicesPermission.Group indicesGroup : remoteIndicesGroup.indicesPermissionGroups()) {
Expand Down
9 changes: 6 additions & 3 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/SystemUser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,12 @@ public class SystemUser extends InternalUser {

private static final RoleDescriptor REMOTE_ACCESS_ROLE_DESCRIPTOR = new RoleDescriptor(
ROLE_NAME + "_cross_cluster_access",
//TODO: don't add these esql here... just a hack for now
new String[] { "cross_cluster_search", "cross_cluster_replication", "cluster:data/read/esql/open_exchange"
,"cluster:data/read/esql/exchange" },
// TODO: don't add these esql here... just a hack for now
new String[] {
"cross_cluster_search",
"cross_cluster_replication",
"cluster:data/read/esql/open_exchange",
"cluster:data/read/esql/exchange" },
// Needed for CCR background jobs (with system user)
new RoleDescriptor.IndicesPrivileges[] {
RoleDescriptor.IndicesPrivileges.builder()
Expand Down
1 change: 0 additions & 1 deletion ...ql/compute/src/main/java/org/elasticsearch/compute/operator/exchange/ExchangeService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ public final class ExchangeService extends AbstractLifecycleComponent {
private static final String OPEN_EXCHANGE_ACTION_NAME = "internal:data/read/esql/open_exchange";
private static final String OPEN_EXCHANGE_ACTION_NAME_FOR_CCS = "cluster:internal:data/read/esql/open_exchange";


/**
* The time interval for an exchange sink handler to be considered inactive and subsequently
* removed from the exchange service if no sinks are attached (i.e., no computation uses that sink handler).
Expand Down
Loading

0 comments on commit 947ef38

Please sign in to comment.