Fix user query scoping and filtering in rules facades

manager/build.gradle

@@ -79,7 +79,7 @@ dependencies {
 
 sourceSets {
   test {
-    groovy.srcDirs = ['test/src/groovy']
+    groovy.srcDirs = ['test/src/groovy', 'src/test/groovy']
   }
 }
 

manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java

@@ -60,6 +60,13 @@ public UsersFacade(
 
   @Override
   public Stream<String> getResults(UserQuery userQuery) {
+    // Rule targets can reuse the caller's query. Isolate nested predicates as well, since the
+    // facade and identity provider both modify filters during execution.
+    if (userQuery == null) {
+      throw new IllegalArgumentException("Unable to copy user query");
+    }
+    userQuery = new UserQuery(userQuery);
+
     // Do security checks to ensure correct scoping
     // No restriction for global rulesets
     if (RealmRuleset.class.isAssignableFrom(rulesEngineId.getScope())) {
@@ -71,7 +78,13 @@ public Stream<String> getResults(UserQuery userQuery) {
                   .orElseThrow(
                       () -> new IllegalArgumentException("Realm ID missing: " + rulesEngineId)));
     } else if (AssetRuleset.class.isAssignableFrom(rulesEngineId.getScope())) {
-      userQuery.realmPredicate = null;
+      // Asset links must not expose users from outside the rule's realm.
+      userQuery.realmPredicate =
+          new RealmPredicate(
+              rulesEngineId
+                  .getRealm()
+                  .orElseThrow(
+                      () -> new IllegalArgumentException("Realm ID missing: " + rulesEngineId)));
       String assetId =
           rulesEngineId
               .getAssetId()
@@ -80,17 +93,35 @@ public Stream<String> getResults(UserQuery userQuery) {
 
       if (userQuery.pathPredicate == null || userQuery.pathPredicate.path == null) {
         userQuery.pathPredicate = new PathPredicate(assetId);
-      } else {
+      } else if (!Arrays.asList(userQuery.pathPredicate.path).contains(assetId)) {
+        // Retain caller restrictions without duplicating an already supplied scope asset.
         List<String> path = new ArrayList<>(Arrays.asList(userQuery.pathPredicate.path));
         path.add(assetId);
         userQuery.pathPredicate.path = path.toArray(new String[0]);
       }
     }
 
-    // Prevent system users being retrieved
-    userQuery.attributes(
+    // Preserve caller filters and avoid duplicating a full exclusion already supplied by the
+    // caller.
+    UserQuery.AttributeValuePredicate systemAccountExclusion =
         new UserQuery.AttributeValuePredicate(
-            true, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE), null));
+            true, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE), null);
+    boolean excludesSystemAccounts =
+        userQuery.attributes != null
+            && Arrays.stream(userQuery.attributes)
+                .anyMatch(
+                    attribute ->
+                        attribute.negated
+                            && attribute.value == null
+                            && systemAccountExclusion.name.equals(attribute.name));
+    if (!excludesSystemAccounts) {
+      UserQuery.AttributeValuePredicate[] attributes =
+          userQuery.attributes == null
+              ? new UserQuery.AttributeValuePredicate[1]
+              : Arrays.copyOf(userQuery.attributes, userQuery.attributes.length + 1);
+      attributes[attributes.length - 1] = systemAccountExclusion;
+      userQuery.attributes(attributes);
+    }
 
     // Prevent service users being retrieved
     userQuery.serviceUsers(false);

manager/src/main/java/org/openremote/manager/security/ManagerIdentityProvider.java

@@ -113,22 +113,6 @@ static User[] getUsersFromDb(PersistenceService persistenceService, UserQuery qu
     List<Object> parameters = new ArrayList<>();
     final UserQuery userQuery = query != null ? query : new UserQuery();
 
-    // Limit to regular/service users
-    if (userQuery.serviceUsers != null) {
-      // Not ideal mutating the query but seems excessive cloning it
-      if (userQuery.usernames == null) {
-        userQuery.usernames = new StringPredicate[1];
-        userQuery.usernames(
-            new StringPredicate(AssetQuery.Match.BEGIN, User.SERVICE_ACCOUNT_PREFIX)
-                .negate(!userQuery.serviceUsers));
-      } else {
-        userQuery.usernames = Arrays.copyOf(userQuery.usernames, userQuery.usernames.length + 1);
-        userQuery.usernames[userQuery.usernames.length - 1] =
-            new StringPredicate(AssetQuery.Match.BEGIN, User.SERVICE_ACCOUNT_PREFIX)
-                .negate(!userQuery.serviceUsers);
-      }
-    }
-
     // BUILD SELECT
     sb.append(
         "SELECT u.*, (SELECT C.SECRET FROM PUBLIC.CLIENT C WHERE C.ID = SERVICE_ACCOUNT_CLIENT_LINK) as secret, r.name as realm");
@@ -163,6 +147,15 @@ static User[] getUsersFromDb(PersistenceService persistenceService, UserQuery qu
       sb.append(" AND u.id IN (?").append(parameters.size() + 1).append(")");
       parameters.add(Arrays.asList(userQuery.ids));
     }
+    if (userQuery.serviceUsers != null) {
+      // Account type must constrain the username alternatives, not add another OR match.
+      StringPredicate serviceUserPredicate =
+          new StringPredicate(AssetQuery.Match.BEGIN, false, User.SERVICE_ACCOUNT_PREFIX)
+              .negate(!userQuery.serviceUsers);
+      sb.append(" AND upper(u.username)");
+      sb.append(toSQLParameter(serviceUserPredicate, parameters.size() + 1, false));
+      parameters.add(serviceUserPredicate.prepareValue());
+    }
     if (userQuery.usernames != null && userQuery.usernames.length > 0) {
       sb.append(" and (FALSE");