From 998b2e4f7903fe8011257b48216acd073bfe8ca7 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 10:34:09 +0200 Subject: [PATCH 01/13] Add failing tests for realm filtering in asset-scoped user queries --- .../test/rules/UsersFacadeTest.groovy | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy diff --git a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy new file mode 100644 index 0000000000..8a12dfaa8c --- /dev/null +++ b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy @@ -0,0 +1,111 @@ +/* + * Copyright 2026, OpenRemote Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package org.openremote.test.rules + +import org.openremote.manager.asset.AssetStorageService +import org.openremote.manager.notification.NotificationService +import org.openremote.manager.rules.RulesEngineId +import org.openremote.manager.rules.facade.UsersFacade +import org.openremote.manager.security.ManagerIdentityService +import org.openremote.manager.setup.SetupService +import org.openremote.model.asset.UserAssetLink +import org.openremote.model.asset.impl.ThingAsset +import org.openremote.model.query.UserQuery +import org.openremote.model.security.User +import org.openremote.setup.integration.KeycloakTestSetup +import org.openremote.test.ManagerContainerTrait +import spock.lang.Specification + +class UsersFacadeTest extends Specification implements ManagerContainerTrait { + + def "Asset scope querying users linked to #targetName with #linkedUserNames returns #expectedUserNames"() { + given: "real identity and asset storage services" + def container = startContainer(defaultConfig(), defaultServices()) + def assetStorageService = container.getService(AssetStorageService.class) + def identityService = container.getService(ManagerIdentityService.class) + def identityProvider = identityService.getIdentityProvider() + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def realm = keycloakSetup.realmBuilding.name + def foreignRealm = keycloakSetup.realmCity.name + List createdAssetIds = [] + List createdUsers = [] + + and: "a scope root, its descendant and an unrelated asset in the same realm" + def createAsset = { String name, String parentId -> + def asset = assetStorageService.merge(new ThingAsset("Users facade ${name}") + .setRealm(realm).setParentId(parentId)) + createdAssetIds.add(asset.id) + asset + } + def scopedAsset = createAsset("scope", null) + def childAsset = createAsset("child", scopedAsset.id) + def unrelatedAsset = createAsset("unrelated", null) + def targetAsset = targetName == "root" ? scopedAsset : childAsset + + and: "regular users in the rule's realm and another realm" + def users = ["sameRealm", "foreignRealm", "outsideSubtree"].collectEntries { name -> + def userRealm = name == "foreignRealm" ? foreignRealm : realm + def user = identityProvider.createUpdateUser(userRealm, + new User().setUsername("users-facade-${UUID.randomUUID()}").setEnabled(true), null, false) + createdUsers.add(user) + def storedUser = identityProvider.getUser(user.id) + assert storedUser.realm == userRealm + assert !storedUser.isServiceAccount() + assert !storedUser.isSystemAccount() + [(name): storedUser] + } + + and: "stored links include any foreign user selected by this case" + // Storage permits cross-realm links independently of REST link-creation checks. + linkedUserNames.each { name -> + def user = users[name] + assetStorageService.storeUserAssetLinks([new UserAssetLink(user.realm, user.id, targetAsset.id)]) + } + def outsideUser = users.outsideSubtree + assetStorageService.storeUserAssetLinks([new UserAssetLink(outsideUser.realm, outsideUser.id, unrelatedAsset.id)]) + assert identityProvider.queryUsers(new UserQuery().assets(targetAsset.id))*.id.toSet() == + linkedUserNames.collect { users[it].id }.toSet() + assert identityProvider.queryUsers(new UserQuery().assets(unrelatedAsset.id))*.id == [outsideUser.id] + + def facade = new UsersFacade(new RulesEngineId(realm, scopedAsset.id), assetStorageService, + container.getService(NotificationService.class), identityService) + def query = new UserQuery().ids(users.values().collect { it.id } as String[]) + + when: "the asset-scoped facade queries all three users" + def result = facade.getResults(query).toList() + + then: "only users in the rule's realm linked within its subtree are returned" + result.toSet() == expectedUserNames.collect { users[it].id }.toSet() + + cleanup: + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUsers.each { identityProvider.deleteUser(it.realm, it.id) } + + where: + targetName | linkedUserNames | expectedUserNames + "root" | ["sameRealm"] | ["sameRealm"] + "root" | ["foreignRealm"] | [] + "root" | ["sameRealm", "foreignRealm"] | ["sameRealm"] + "child" | ["sameRealm"] | ["sameRealm"] + "child" | ["foreignRealm"] | [] + "child" | ["sameRealm", "foreignRealm"] | ["sameRealm"] + } +} From b89bf7a862be67241f6dd14c572a386e861787eb Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 10:40:32 +0200 Subject: [PATCH 02/13] Enforce realm filtering in asset-scoped user queries --- .../org/openremote/manager/rules/facade/UsersFacade.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java index a5b217cbca..0582872e64 100644 --- a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java +++ b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java @@ -71,7 +71,13 @@ public Stream 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() From d6b72e4021c5e7b2920128c39e797dcd6385bd90 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 10:48:32 +0200 Subject: [PATCH 03/13] Add failing tests for service-account exclusion with username filters --- .../test/rules/UsersFacadeTest.groovy | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy index 8a12dfaa8c..fa4b4ccacb 100644 --- a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy +++ b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy @@ -27,6 +27,7 @@ import org.openremote.manager.setup.SetupService import org.openremote.model.asset.UserAssetLink import org.openremote.model.asset.impl.ThingAsset import org.openremote.model.query.UserQuery +import org.openremote.model.query.filter.StringPredicate import org.openremote.model.security.User import org.openremote.setup.integration.KeycloakTestSetup import org.openremote.test.ManagerContainerTrait @@ -108,4 +109,76 @@ class UsersFacadeTest extends Specification implements ManagerContainerTrait { "child" | ["foreignRealm"] | [] "child" | ["sameRealm", "foreignRealm"] | ["sameRealm"] } + + def "#scope scope querying usernames #queryUserNames returns only regular users #expectedUserNames"() { + given: "real identity and asset storage services" + def container = startContainer(defaultConfig(), defaultServices()) + def assetStorageService = container.getService(AssetStorageService.class) + def identityService = container.getService(ManagerIdentityService.class) + def identityProvider = identityService.getIdentityProvider() + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def realm = keycloakSetup.realmBuilding.name + List createdUsers = [] + def scopedAsset = assetStorageService.merge(new ThingAsset("Users facade username filtering").setRealm(realm)) + + and: "two regular users and a real service account, all within the rule's scope" + def users = ["regular", "unmatched", "service"].collectEntries { name -> + def user = identityProvider.createUpdateUser(realm, + new User().setUsername("users-facade-${UUID.randomUUID()}").setEnabled(true) + .setServiceAccount(name == "service"), null, false) + createdUsers.add(user) + def storedUser = identityProvider.getUser(user.id) + assert storedUser.realm == realm + assert storedUser.isServiceAccount() == (name == "service") + assert !storedUser.isSystemAccount() + assetStorageService.storeUserAssetLinks([new UserAssetLink(realm, user.id, scopedAsset.id)]) + [(name): storedUser] + } + assert identityProvider.queryUsers(new UserQuery().assets(scopedAsset.id))*.id.toSet() == + users.values().collect { it.id }.toSet() + + and: "a username query which independently matches the intended accounts" + def createQuery = { + def query = new UserQuery().ids(users.values().collect { it.id } as String[]) + if (!queryUserNames.isEmpty()) { + query.usernames(queryUserNames.collect { name -> + def user = users[name] + // User.getUsername() omits Keycloak's prefix; username predicates match the stored name. + def storedUsername = user.isServiceAccount() ? User.SERVICE_ACCOUNT_PREFIX + user.username : user.username + new StringPredicate(storedUsername) + } as StringPredicate[]) + } + query + } + def matchingNames = queryUserNames.isEmpty() ? users.keySet() : queryUserNames + assert identityProvider.queryUsers(createQuery())*.id.toSet() == + matchingNames.collect { users[it].id }.toSet() + + def engineId = scope == "global" ? new RulesEngineId() : + scope == "realm" ? new RulesEngineId(realm) : new RulesEngineId(realm, scopedAsset.id) + def facade = new UsersFacade(engineId, assetStorageService, + container.getService(NotificationService.class), identityService) + + when: "the facade applies its account exclusions to the query" + def result = facade.getResults(createQuery()).toList() + + then: "service accounts stay excluded and the username filter is respected" + result.toSet() == expectedUserNames.collect { users[it].id }.toSet() + + cleanup: + if (scopedAsset != null) { + assert assetStorageService.delete([scopedAsset.id]) + } + createdUsers.each { identityProvider.deleteUser(it.realm, it.id) } + + where: + [scope, queryUserNames, expectedUserNames] << ["global", "realm", "asset"].collectMany { scope -> + [ + [[], ["regular", "unmatched"]], + [["regular"], ["regular"]], + [["service"], []], + [["regular", "service"], ["regular"]] + ].collect { row -> [scope] + row } + } + } } From 3a4386de10006ae4d6a0380051c77edf9368d6a2 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:00:23 +0200 Subject: [PATCH 04/13] Combine service-account filtering with username predicates using AND --- .../security/ManagerIdentityProvider.java | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/security/ManagerIdentityProvider.java b/manager/src/main/java/org/openremote/manager/security/ManagerIdentityProvider.java index 6cfa228fd7..49705385b0 100644 --- a/manager/src/main/java/org/openremote/manager/security/ManagerIdentityProvider.java +++ b/manager/src/main/java/org/openremote/manager/security/ManagerIdentityProvider.java @@ -113,22 +113,6 @@ static User[] getUsersFromDb(PersistenceService persistenceService, UserQuery qu List 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"); From 9f0955636ab1148f2a832fbb712af22397e496a9 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:04:45 +0200 Subject: [PATCH 05/13] Add failing tests for preserving user attribute filters in rules facade --- .../test/rules/UsersFacadeTest.groovy | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy index fa4b4ccacb..15a36eb28d 100644 --- a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy +++ b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy @@ -181,4 +181,87 @@ class UsersFacadeTest extends Specification implements ManagerContainerTrait { ].collect { row -> [scope] + row } } } + + def "#scope scope querying attributes #queryAttributes returns only regular users #expectedUserNames"() { + given: "real identity and asset storage services" + def container = startContainer(defaultConfig(), defaultServices()) + def assetStorageService = container.getService(AssetStorageService.class) + def identityService = container.getService(ManagerIdentityService.class) + def identityProvider = identityService.getIdentityProvider() + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def realm = keycloakSetup.realmBuilding.name + List createdUsers = [] + def scopedAsset = assetStorageService.merge(new ThingAsset("Users facade attribute filtering").setRealm(realm)) + + and: "regular users with different attributes and a system account matching both filters" + def userAttributes = [ + matching: [department: "maintenance", location: "north"], + otherDepartment: [department: "operations", location: "north"], + otherLocation: [department: "maintenance", location: "south"], + system: [department: "maintenance", location: "north"] + ] + def users = userAttributes.collectEntries { name, attributes -> + def newUser = new User().setUsername("users-facade-${UUID.randomUUID()}").setEnabled(true) + .setSystemAccount(name == "system") + attributes.each { key, value -> newUser.setAttribute(key, value) } + def user = identityProvider.createUpdateUser(realm, newUser, null, false) + createdUsers.add(user) + def storedUser = identityProvider.getUser(user.id) + assert storedUser.realm == realm + assert !storedUser.isServiceAccount() + assert storedUser.isSystemAccount() == (name == "system") + attributes.each { key, value -> assert storedUser.attributeMap[key] == [value] } + assetStorageService.storeUserAssetLinks([new UserAssetLink(realm, user.id, scopedAsset.id)]) + [(name): storedUser] + } + assert identityProvider.queryUsers(new UserQuery().assets(scopedAsset.id))*.id.toSet() == + users.values().collect { it.id }.toSet() + + and: "an attribute query which independently matches the expected regular and system accounts" + def createQuery = { + def query = new UserQuery().ids(users.values().collect { it.id } as String[]) + if (!queryAttributes.isEmpty()) { + query.attributes(queryAttributes.collect { key, value -> + new UserQuery.AttributeValuePredicate(false, new StringPredicate(key), new StringPredicate(value)) + } as UserQuery.AttributeValuePredicate[]) + } + query + } + assert identityProvider.queryUsers(createQuery())*.id.toSet() == + rawUserNames.collect { users[it].id }.toSet() + + def engineId = scope == "global" ? new RulesEngineId() : + scope == "realm" ? new RulesEngineId(realm) : new RulesEngineId(realm, scopedAsset.id) + def facade = new UsersFacade(engineId, assetStorageService, + container.getService(NotificationService.class), identityService) + + when: "the facade applies its system-account exclusion to the attribute query" + def result = facade.getResults(createQuery()).toList() + + then: "caller filters and system-account exclusion both apply" + verifyAll { + !result.contains(users.system.id) + result.toSet() == expectedUserNames.collect { users[it].id }.toSet() + } + + cleanup: + if (scopedAsset != null) { + assert assetStorageService.delete([scopedAsset.id]) + } + createdUsers.each { identityProvider.deleteUser(it.realm, it.id) } + + where: + [scope, queryAttributes, rawUserNames, expectedUserNames] << ["global", "realm", "asset"].collectMany { scope -> + [ + [ + [:], + ["matching", "otherDepartment", "otherLocation", "system"], + ["matching", "otherDepartment", "otherLocation"] + ], + [[department: "maintenance"], ["matching", "otherLocation", "system"], ["matching", "otherLocation"]], + [[department: "maintenance", location: "north"], ["matching", "system"], ["matching"]], + [[department: "nonexistent"], [], []] + ].collect { row -> [scope] + row } + } + } } From 28a4f3d5169d32244d1af27bcb3f9f20ba6a2e78 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:11:03 +0200 Subject: [PATCH 06/13] Preserve user attribute filters when excluding system accounts --- .../openremote/manager/rules/facade/UsersFacade.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java index 0582872e64..1194397a00 100644 --- a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java +++ b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java @@ -93,10 +93,15 @@ public Stream getResults(UserQuery userQuery) { } } - // Prevent system users being retrieved - userQuery.attributes( + // Preserve caller filters and require system-account exclusion as an additional AND condition. + UserQuery.AttributeValuePredicate[] attributes = + userQuery.attributes == null + ? new UserQuery.AttributeValuePredicate[1] + : Arrays.copyOf(userQuery.attributes, userQuery.attributes.length + 1); + attributes[attributes.length - 1] = new UserQuery.AttributeValuePredicate( - true, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE), null)); + true, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE), null); + userQuery.attributes(attributes); // Prevent service users being retrieved userQuery.serviceUsers(false); From 655ba5837d32d3f402e614141203e49d4069c1f2 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:18:48 +0200 Subject: [PATCH 07/13] Add scope-boundary regression tests for users facade --- .../test/rules/UsersFacadeTest.groovy | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy index 15a36eb28d..376ac78546 100644 --- a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy +++ b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy @@ -27,6 +27,8 @@ import org.openremote.manager.setup.SetupService import org.openremote.model.asset.UserAssetLink import org.openremote.model.asset.impl.ThingAsset import org.openremote.model.query.UserQuery +import org.openremote.model.query.filter.PathPredicate +import org.openremote.model.query.filter.RealmPredicate import org.openremote.model.query.filter.StringPredicate import org.openremote.model.security.User import org.openremote.setup.integration.KeycloakTestSetup @@ -264,4 +266,103 @@ class UsersFacadeTest extends Specification implements ManagerContainerTrait { ].collect { row -> [scope] + row } } } + + def "#scope scope with realm #realmFilter, assets #assetNames and path #pathNames returns #expectedUserNames"() { + given: "real identity and asset storage services" + def container = startContainer(defaultConfig(), defaultServices()) + def assetStorageService = container.getService(AssetStorageService.class) + def identityService = container.getService(ManagerIdentityService.class) + def identityProvider = identityService.getIdentityProvider() + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def realm = keycloakSetup.realmBuilding.name + def foreignRealm = keycloakSetup.realmCity.name + List createdAssetIds = [] + List createdUsers = [] + + and: "a scoped subtree, a sibling outside it, and an asset in another realm" + def createAsset = { String name, String assetRealm, String parentId -> + def asset = assetStorageService.merge(new ThingAsset("Users facade boundaries ${name}") + .setRealm(assetRealm).setParentId(parentId)) + createdAssetIds.add(asset.id) + asset + } + def assets = [:] + assets.ancestor = createAsset("ancestor", realm, null) + assets.scoped = createAsset("scoped", realm, assets.ancestor.id) + assets.child = createAsset("child", realm, assets.scoped.id) + assets.grandchild = createAsset("grandchild", realm, assets.child.id) + assets.outside = createAsset("outside", realm, assets.ancestor.id) + assets.foreign = createAsset("foreign", foreignRealm, null) + assert assetStorageService.find(assets.grandchild.id, true).path.contains(assets.child.id) + assert !assetStorageService.find(assets.outside.id, true).path.contains(assets.scoped.id) + + and: "one regular user linked to each target asset" + def users = ["scoped", "child", "grandchild", "outside", "foreign"].collectEntries { name -> + def asset = assets[name] + def user = identityProvider.createUpdateUser(asset.realm, + new User().setUsername("users-facade-${UUID.randomUUID()}").setEnabled(true), null, false) + createdUsers.add(user) + def storedUser = identityProvider.getUser(user.id) + assert storedUser.realm == asset.realm + assert !storedUser.isServiceAccount() + assert !storedUser.isSystemAccount() + assetStorageService.storeUserAssetLinks([new UserAssetLink(asset.realm, user.id, asset.id)]) + assert identityProvider.queryUsers(new UserQuery().assets(asset.id))*.id == [user.id] + [(name): storedUser] + } + + and: "a caller query with optional realm, asset and path restrictions" + def createQuery = { + def query = new UserQuery().ids(users.values().collect { it.id } as String[]) + if (realmFilter != null) { + query.realm(new RealmPredicate(foreignRealm)) + } + if (assetNames != null) { + query.assets(assetNames.collect { assets[it].id } as String[]) + } + if (pathNames != null) { + query.assetPath(new PathPredicate(pathNames.collect { assets[it].id } as String[])) + } + query + } + // Every supplied query matches actual users before the facade enforces its scope. + assert identityProvider.queryUsers(createQuery()).length> 0 + def engineId = scope == "global" ? new RulesEngineId() : + scope == "realm" ? new RulesEngineId(realm) : new RulesEngineId(realm, assets.scoped.id) + def facade = new UsersFacade(engineId, assetStorageService, + container.getService(NotificationService.class), identityService) + + when: "the rule queries users through its facade" + def result = facade.getResults(createQuery()).toList() + + then: "the engine scope is enforced while valid narrower filters are retained" + result.toSet() == expectedUserNames.collect { users[it].id }.toSet() + + cleanup: + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUsers.each { identityProvider.deleteUser(it.realm, it.id) } + + where: + scope | realmFilter | assetNames | pathNames | expectedUserNames + "global" | null | null | null | ["scoped", "child", "grandchild", "outside", "foreign"] + "global" | "foreign" | null | null | ["foreign"] + "realm" | null | null | null | ["scoped", "child", "grandchild", "outside"] + "realm" | "foreign" | null | null | ["scoped", "child", "grandchild", "outside"] + "realm" | null | ["foreign"] | null | [] + "asset" | null | null | null | ["scoped", "child", "grandchild"] + "asset" | "foreign" | null | null | ["scoped", "child", "grandchild"] + "asset" | null | ["scoped"] | null | ["scoped"] + "asset" | null | ["child"] | null | ["child"] + "asset" | null | ["outside"] | null | [] + "asset" | null | ["foreign"] | null | [] + "asset" | null | ["child", "outside", "foreign"] | null | ["child"] + "asset" | null | null | ["ancestor"] | ["scoped", "child", "grandchild"] + "asset" | null | null | ["outside"] | [] + "asset" | null | null | ["foreign"] | [] + "asset" | null | null | ["child"] | ["child", "grandchild"] + "asset" | null | null | ["grandchild"] | ["grandchild"] + "asset" | null | ["grandchild", "outside"] | ["child"] | ["grandchild"] + } } From f6449fb1fb47a0ee56c0f59a7ab6b618c4f1e204 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:37:19 +0200 Subject: [PATCH 08/13] Make users facade system-account exclusion idempotent --- manager/build.gradle | 2 +- .../manager/rules/facade/UsersFacade.java | 26 ++-- .../rules/facade/UsersFacadeQueryTest.groovy | 119 ++++++++++++++++++ 3 files changed, 139 insertions(+), 8 deletions(-) create mode 100644 manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy diff --git a/manager/build.gradle b/manager/build.gradle index 4c3d6d3d23..5283f00f8e 100644 --- a/manager/build.gradle +++ b/manager/build.gradle @@ -79,7 +79,7 @@ dependencies { sourceSets { test { - groovy.srcDirs = ['test/src/groovy'] + groovy.srcDirs = ['test/src/groovy', 'src/test/groovy'] } } diff --git a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java index 1194397a00..a5f7a45d60 100644 --- a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java +++ b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java @@ -93,15 +93,27 @@ public Stream getResults(UserQuery userQuery) { } } - // Preserve caller filters and require system-account exclusion as an additional AND condition. - UserQuery.AttributeValuePredicate[] attributes = - userQuery.attributes == null - ? new UserQuery.AttributeValuePredicate[1] - : Arrays.copyOf(userQuery.attributes, userQuery.attributes.length + 1); - attributes[attributes.length - 1] = + // Queries are reused across rule firings: preserve caller filters and add the full exclusion + // only once, rather than accumulating another correlated subquery on each execution. + UserQuery.AttributeValuePredicate systemAccountExclusion = new UserQuery.AttributeValuePredicate( true, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE), null); - userQuery.attributes(attributes); + 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); diff --git a/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy b/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy new file mode 100644 index 0000000000..584c15bf42 --- /dev/null +++ b/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy @@ -0,0 +1,119 @@ +/* + * Copyright 2026, OpenRemote Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package org.openremote.manager.rules.facade + +import org.openremote.manager.asset.AssetStorageService +import org.openremote.manager.notification.NotificationService +import org.openremote.manager.rules.RulesEngineId +import org.openremote.manager.security.ManagerIdentityProvider +import org.openremote.manager.security.ManagerIdentityService +import org.openremote.model.query.UserQuery +import org.openremote.model.query.filter.StringPredicate +import org.openremote.model.security.User +import spock.lang.Specification + +class UsersFacadeQueryTest extends Specification { + + def "Reusing a #scope query with #filterCase keeps system-account exclusion bounded"() { + given: "the query object retained across rule firings" + def callerFilters = createFilters() as UserQuery.AttributeValuePredicate[] + def query = new UserQuery().attributes(callerFilters) + def expectedFilters = snapshot(callerFilters) + if (!alreadyExcluded) { + expectedFilters.add(exclusion().toString()) + } + List> submittedFilters = [] + def provider = Stub(ManagerIdentityProvider) { + queryUsers(_ as UserQuery) >> { UserQuery submitted -> + // Snapshot at execution time; retaining the mutable query would hide earlier growth. + submittedFilters.add(snapshot(submitted.attributes)) + new User[0] + } + } + def identityService = Stub(ManagerIdentityService) { + getIdentityProvider() >> provider + } + def engineId = scope == "global" ? new RulesEngineId() : + scope == "realm" ? new RulesEngineId("building") : new RulesEngineId("building", "scope-asset") + def facade = new UsersFacade(engineId, Stub(AssetStorageService), Stub(NotificationService), identityService) + + when: "the same query is executed repeatedly" + 100.times { facade.getResults(query).toList() } + def filterCounts = submittedFilters.collect { it.size() }.unique() + + then: "every execution preserves the caller filters and has just one full system-account exclusion" + submittedFilters.size() == 100 + filterCounts == [expectedFilters.size()] + submittedFilters.each { assert it == expectedFilters } + + where: + [scope, filterCase, createFilters, alreadyExcluded] << ["global", "realm", "asset"].collectMany { scope -> + [ + [ + "no attributes", + { -> null }, + false + ], + [ + "empty attributes", + { -> [] }, + false + ], + [ + "a caller filter", + { -> [department()] }, + false + ], + [ + "an existing full exclusion", + { -> [department(), exclusion()] }, + true + ], + [ + "a value-specific exclusion", + { -> [department(), exclusion("true")] }, + false + ], + [ + "a positive system filter", + { + -> [ + department(), + new UserQuery.AttributeValuePredicate(false, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE)) + ] + }, + false + ] + ].collect { row -> [scope] + row } + } + } + + private static UserQuery.AttributeValuePredicate department() { + new UserQuery.AttributeValuePredicate(false, new StringPredicate("department"), new StringPredicate("maintenance")) + } + + private static UserQuery.AttributeValuePredicate exclusion(String value = null) { + new UserQuery.AttributeValuePredicate(true, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE), + value == null ? null : new StringPredicate(value)) + } + + private static List snapshot(UserQuery.AttributeValuePredicate[] attributes) { + attributes == null ? [] : attributes.collect { it.toString() } + } +} From ac966b15dcef0b8bf60fb5cee6636676ebff21be Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:47:30 +0200 Subject: [PATCH 09/13] Make users facade asset-path scoping idempotent --- .../manager/rules/facade/UsersFacade.java | 3 +- .../rules/facade/UsersFacadeQueryTest.groovy | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java index a5f7a45d60..e26cfb9264 100644 --- a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java +++ b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java @@ -86,7 +86,8 @@ public Stream 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)) { + // Reused queries must retain caller restrictions without accumulating the scope asset. List path = new ArrayList<>(Arrays.asList(userQuery.pathPredicate.path)); path.add(assetId); userQuery.pathPredicate.path = path.toArray(new String[0]); diff --git a/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy b/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy index 584c15bf42..607751ba6a 100644 --- a/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy +++ b/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy @@ -24,6 +24,7 @@ import org.openremote.manager.rules.RulesEngineId import org.openremote.manager.security.ManagerIdentityProvider import org.openremote.manager.security.ManagerIdentityService import org.openremote.model.query.UserQuery +import org.openremote.model.query.filter.PathPredicate import org.openremote.model.query.filter.StringPredicate import org.openremote.model.security.User import spock.lang.Specification @@ -104,6 +105,43 @@ class UsersFacadeQueryTest extends Specification { } } + def "Reusing an asset query with #pathCase keeps the scope path bounded"() { + given: "an asset-scoped rule retaining its user query across firings" + def query = new UserQuery().assetPath(callerPath) + List> submittedPaths = [] + def provider = Stub(ManagerIdentityProvider) { + queryUsers(_ as UserQuery) >> { UserQuery submitted -> + submittedPaths.add(submitted.pathPredicate.path.toList()) + new User[0] + } + } + def identityService = Stub(ManagerIdentityService) { + getIdentityProvider() >> provider + } + def facade = new UsersFacade(new RulesEngineId("building", "scope-asset"), + Stub(AssetStorageService), Stub(NotificationService), identityService) + + when: "the same query is executed repeatedly" + 100.times { facade.getResults(query).toList() } + def pathLengths = submittedPaths.collect { it.size() }.unique() + + then: "every execution retains the caller's path and includes the scope asset only once" + submittedPaths.size() == 100 + pathLengths == [expectedPath.size()] + submittedPaths.each { assert it == expectedPath } + + where: + pathCase | callerPath | expectedPath + "no predicate" | null | ["scope-asset"] + "null path values" | new PathPredicate() | ["scope-asset"] + "empty path values" | new PathPredicate([] as String[]) | ["scope-asset"] + "a descendant filter" | new PathPredicate("child") | ["child", "scope-asset"] + "an outside filter" | new PathPredicate("outside") | ["outside", "scope-asset"] + "the scope already present" | new PathPredicate("scope-asset") | ["scope-asset"] + "the scope at the start" | new PathPredicate("scope-asset", "child") | ["scope-asset", "child"] + "the scope at the end" | new PathPredicate("child", "scope-asset") | ["child", "scope-asset"] + } + private static UserQuery.AttributeValuePredicate department() { new UserQuery.AttributeValuePredicate(false, new StringPredicate("department"), new StringPredicate("maintenance")) } From 384c5a3243198876758651c304ec5fed38055d37 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:00:52 +0200 Subject: [PATCH 10/13] Add failing tests for users facade query isolation --- .../rules/facade/UsersFacadeQueryTest.groovy | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy b/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy index 607751ba6a..b05bc24aad 100644 --- a/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy +++ b/manager/src/test/groovy/org/openremote/manager/rules/facade/UsersFacadeQueryTest.groovy @@ -23,8 +23,10 @@ import org.openremote.manager.notification.NotificationService import org.openremote.manager.rules.RulesEngineId import org.openremote.manager.security.ManagerIdentityProvider import org.openremote.manager.security.ManagerIdentityService +import org.openremote.model.query.AssetQuery import org.openremote.model.query.UserQuery import org.openremote.model.query.filter.PathPredicate +import org.openremote.model.query.filter.RealmPredicate import org.openremote.model.query.filter.StringPredicate import org.openremote.model.security.User import spock.lang.Specification @@ -142,6 +144,155 @@ class UsersFacadeQueryTest extends Specification { "the scope at the end" | new PathPredicate("child", "scope-asset") | ["child", "scope-asset"] } + def "#scope scope leaves the caller query unchanged with populated filters: #populated"() { + given: "a query owned by the caller" + def query = populated ? populatedQuery() : new UserQuery() + def original = queryState(query) + def engineId = engineFor(scope) + List submittedStates = [] + def provider = Stub(ManagerIdentityProvider) { + queryUsers(_ as UserQuery) >> { UserQuery submitted -> + submittedStates.add(queryState(submitted)) + // The real identity query normalizes username matching in place. + submitted.usernames?.each { it.caseSensitive = false } + new User[0] + } + } + + when: + facadeFor(engineId, provider).getResults(query).toList() + + then: "scope checks apply to the execution without changing caller filters or nested predicates" + verifyAll { + queryState(query) == original + submittedStates == [scopedState(original, engineId)] + } + + where: + [scope, populated] << ["global", "realm", "asset"].collectMany { scope -> + [false, true].collect { populated -> [scope, populated] } + } + } + + def "Reusing a caller query across engines does not carry scope restrictions forward with populated filters: #populated"() { + given: "one query used by two asset engines, a realm engine and a global engine" + def query = populated ? populatedQuery() : new UserQuery() + def original = queryState(query) + def engines = [ + new RulesEngineId("realm-a", "asset-a"), + new RulesEngineId("realm-b", "asset-b"), + new RulesEngineId("realm-c"), + new RulesEngineId() + ] + List submittedStates = [] + def provider = Stub(ManagerIdentityProvider) { + queryUsers(_ as UserQuery) >> { UserQuery submitted -> + submittedStates.add(queryState(submitted)) + new User[0] + } + } + + when: + engines.each { facadeFor(it, provider).getResults(query).toList() } + + then: "each engine applies only its own scope to the original caller query" + verifyAll { + queryState(query) == original + submittedStates == engines.collect { scopedState(original, it) } + } + + where: + populated << [false, true] + } + + def "Later caller edits are isolated from earlier executions in #scope scope"() { + given: "a query with nested filters and an identity provider retaining submitted queries" + def query = populatedQuery() + def engineId = engineFor(scope) + List submittedQueries = [] + def provider = Stub(ManagerIdentityProvider) { + queryUsers(_ as UserQuery) >> { UserQuery submitted -> + submittedQueries.add(submitted) + new User[0] + } + } + def facade = facadeFor(engineId, provider) + facade.getResults(query).toList() + def firstExecution = queryState(submittedQueries[0]) + + when: "the caller updates nested filters and executes the same query again" + query.realmPredicate.name = "next-realm" + query.assets[0] = "next-asset" + query.pathPredicate.path[0] = "next-path" + query.ids[0] = "next-user" + query.usernames[0].value = "NextOperator" + query.attributes[0].name.value = "team" + query.attributes[0].value.value = "operations" + query.attributes[1].negated = false + query.clientRoles[0].value = "write:assets" + query.realmRoles[0].value = "next-role" + query.select.basic = false + query.orderBy.descending = false + query.limit = 10 + query.offset = 3 + def updatedInput = queryState(query) + facade.getResults(query).toList() + + then: "the first execution retains its filters, and the next execution uses the edited input" + verifyAll { + submittedQueries.size() == 2 + queryState(submittedQueries[0]) == firstExecution + queryState(submittedQueries[1]) == scopedState(updatedInput, engineId) + queryState(query) == updatedInput + } + + where: + scope << ["global", "realm", "asset"] + } + + private UsersFacade facadeFor(RulesEngineId engineId, ManagerIdentityProvider provider) { + def identityService = Stub(ManagerIdentityService) { + getIdentityProvider() >> provider + } + new UsersFacade(engineId, Stub(AssetStorageService), Stub(NotificationService), identityService) + } + + private static RulesEngineId engineFor(String scope) { + scope == "global" ? new RulesEngineId() : + scope == "realm" ? new RulesEngineId("building") : new RulesEngineId("building", "scope-asset") + } + + private static UserQuery populatedQuery() { + new UserQuery().realm(new RealmPredicate("caller-realm")) + .assets("caller-asset").assetPath(new PathPredicate("caller-path")).ids("caller-user") + .usernames(new StringPredicate(AssetQuery.Match.BEGIN, true, "Operator").negate(true)) + .attributes(department(), new UserQuery.AttributeValuePredicate(true, new StringPredicate("blocked"))) + .clientRoles(new StringPredicate("read:assets").negate(true)) + .realmRoles(new StringPredicate("restricted-user")) + .serviceUsers(true).limit(7).offset(2) + .select(new UserQuery.Select().basic(true)) + .orderBy(new UserQuery.OrderBy(UserQuery.OrderBy.Property.USERNAME, true)) + } + + private static Map queryState(UserQuery query) { + [realm: query.realmPredicate?.name, assets: query.assets?.toList(), path: query.pathPredicate?.path?.toList(), + ids: query.ids?.toList(), usernames: query.usernames?.collect { it.toString() }, + attributes: query.attributes == null ? null : snapshot(query.attributes), + clientRoles: query.clientRoles?.collect { + it.toString() + }, realmRoles: query.realmRoles?.collect { + it.toString() + }, + serviceUsers: query.serviceUsers, limit: query.limit, offset: query.offset, + select: query.select?.toString(), orderBy: query.orderBy?.toString()] + } + + private static Map scopedState(Map original, RulesEngineId engineId) { + original + [realm: engineId.realm.orElse(original.realm), + path: engineId.assetId.present ? (original.path ?: []) + [engineId.assetId.get()] : original.path, + attributes: (original.attributes ?: []) + [exclusion().toString()], serviceUsers: false] + } + private static UserQuery.AttributeValuePredicate department() { new UserQuery.AttributeValuePredicate(false, new StringPredicate("department"), new StringPredicate("maintenance")) } From 37fe4fb34e5b957ef8c36f2a8e3b76a61538b3db Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:07:15 +0200 Subject: [PATCH 11/13] Copy user queries before applying rules facade restrictions --- .../manager/rules/facade/UsersFacade.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java index e26cfb9264..608c1f0966 100644 --- a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java +++ b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java @@ -35,6 +35,7 @@ import org.openremote.model.rules.Ruleset; import org.openremote.model.rules.Users; import org.openremote.model.security.User; +import org.openremote.model.util.ValueUtil; /** * Restricts rule RHS access to the scope of the engine (a rule in asset scope can not access users @@ -60,6 +61,13 @@ public UsersFacade( @Override public Stream 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. + userQuery = ValueUtil.clone(userQuery); + if (userQuery == null) { + throw new IllegalArgumentException("Unable to copy user query"); + } + // Do security checks to ensure correct scoping // No restriction for global rulesets if (RealmRuleset.class.isAssignableFrom(rulesEngineId.getScope())) { @@ -87,15 +95,15 @@ public Stream getResults(UserQuery userQuery) { if (userQuery.pathPredicate == null || userQuery.pathPredicate.path == null) { userQuery.pathPredicate = new PathPredicate(assetId); } else if (!Arrays.asList(userQuery.pathPredicate.path).contains(assetId)) { - // Reused queries must retain caller restrictions without accumulating the scope asset. + // Retain caller restrictions without duplicating an already supplied scope asset. List path = new ArrayList<>(Arrays.asList(userQuery.pathPredicate.path)); path.add(assetId); userQuery.pathPredicate.path = path.toArray(new String[0]); } } - // Queries are reused across rule firings: preserve caller filters and add the full exclusion - // only once, rather than accumulating another correlated subquery on each execution. + // 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); From 9685a0786a74e9e28720834bf9d04fa0dac87178 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:26:40 +0200 Subject: [PATCH 12/13] Add failing tests for empty asset filters in users facade --- .../openremote/test/rules/UsersFacadeTest.groovy | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy index 376ac78546..d5dd86c68b 100644 --- a/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy +++ b/test/src/test/groovy/org/openremote/test/rules/UsersFacadeTest.groovy @@ -325,8 +325,14 @@ class UsersFacadeTest extends Specification implements ManagerContainerTrait { } query } - // Every supplied query matches actual users before the facade enforces its scope. - assert identityProvider.queryUsers(createQuery()).length> 0 + // An explicit empty asset selection must match nobody, unlike an absent asset filter. + // All other cases match seeded users before the facade enforces its scope. + def directResults = identityProvider.queryUsers(createQuery()) + if (assetNames != null && assetNames.isEmpty()) { + assert directResults.length == 0 + } else { + assert directResults.length> 0 + } def engineId = scope == "global" ? new RulesEngineId() : scope == "realm" ? new RulesEngineId(realm) : new RulesEngineId(realm, assets.scoped.id) def facade = new UsersFacade(engineId, assetStorageService, @@ -347,11 +353,14 @@ class UsersFacadeTest extends Specification implements ManagerContainerTrait { where: scope | realmFilter | assetNames | pathNames | expectedUserNames "global" | null | null | null | ["scoped", "child", "grandchild", "outside", "foreign"] + "global" | null | [] | null | [] "global" | "foreign" | null | null | ["foreign"] "realm" | null | null | null | ["scoped", "child", "grandchild", "outside"] + "realm" | null | [] | null | [] "realm" | "foreign" | null | null | ["scoped", "child", "grandchild", "outside"] "realm" | null | ["foreign"] | null | [] "asset" | null | null | null | ["scoped", "child", "grandchild"] + "asset" | null | [] | null | [] "asset" | "foreign" | null | null | ["scoped", "child", "grandchild"] "asset" | null | ["scoped"] | null | ["scoped"] "asset" | null | ["child"] | null | ["child"] From eafea01c9fc821e312428c2d30ad0441a042bca1 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:32:26 +0200 Subject: [PATCH 13/13] Preserve empty asset filters when copying user queries --- .../manager/rules/facade/UsersFacade.java | 3 +- .../org/openremote/model/query/UserQuery.java | 53 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java index 608c1f0966..ff8bf536c4 100644 --- a/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java +++ b/manager/src/main/java/org/openremote/manager/rules/facade/UsersFacade.java @@ -35,7 +35,6 @@ import org.openremote.model.rules.Ruleset; import org.openremote.model.rules.Users; import org.openremote.model.security.User; -import org.openremote.model.util.ValueUtil; /** * Restricts rule RHS access to the scope of the engine (a rule in asset scope can not access users @@ -63,10 +62,10 @@ public UsersFacade( public Stream 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. - userQuery = ValueUtil.clone(userQuery); 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 diff --git a/model/src/main/java/org/openremote/model/query/UserQuery.java b/model/src/main/java/org/openremote/model/query/UserQuery.java index b96d94495a..055b274b07 100644 --- a/model/src/main/java/org/openremote/model/query/UserQuery.java +++ b/model/src/main/java/org/openremote/model/query/UserQuery.java @@ -205,6 +205,59 @@ public String toString() { public UserQuery() {} + /** + * Copies all query options, including nested mutable predicates. Empty arrays are preserved: an + * empty asset selection matches no users, whereas a null selection imposes no asset filter. + */ + public UserQuery(UserQuery source) { + realmPredicate = + source.realmPredicate == null ? null : new RealmPredicate(source.realmPredicate.name); + assets = source.assets == null ? null : source.assets.clone(); + pathPredicate = + source.pathPredicate == null + ? null + : new PathPredicate( + source.pathPredicate.path == null ? null : source.pathPredicate.path.clone()); + ids = source.ids == null ? null : source.ids.clone(); + select = source.select == null ? null : new Select().basic(source.select.basic); + usernames = copyPredicates(source.usernames); + attributes = + source.attributes == null + ? null + : Arrays.stream(source.attributes) + .map( + attribute -> + attribute == null + ? null + : new AttributeValuePredicate( + attribute.negated, + copyPredicate(attribute.name), + copyPredicate(attribute.value))) + .toArray(AttributeValuePredicate[]::new); + clientRoles = copyPredicates(source.clientRoles); + realmRoles = copyPredicates(source.realmRoles); + serviceUsers = source.serviceUsers; + limit = source.limit; + offset = source.offset; + orderBy = + source.orderBy == null + ? null + : new OrderBy(source.orderBy.property, source.orderBy.descending); + } + + private static StringPredicate[] copyPredicates(StringPredicate[] predicates) { + return predicates == null + ? null + : Arrays.stream(predicates).map(UserQuery::copyPredicate).toArray(StringPredicate[]::new); + } + + private static StringPredicate copyPredicate(StringPredicate predicate) { + return predicate == null + ? null + : new StringPredicate(predicate.match, predicate.caseSensitive, predicate.value) + .negate(predicate.negate); + } + public UserQuery realm(RealmPredicate realmPredicate) { this.realmPredicate = realmPredicate; return this;