From ec6f4813bc6fa3231b92181f8d4303a4b71519a4 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:51:55 +0200 Subject: [PATCH 01/22] Add failing tests for CUSTOM target rejection in device push notifications --- .../test/notification/NotificationTest.groovy | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 31db45e2a0..25b87503af 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -36,12 +36,14 @@ import org.openremote.manager.security.ManagerIdentityService import org.openremote.manager.setup.SetupService import org.openremote.manager.web.ManagerWebService import org.openremote.model.asset.impl.ConsoleAsset +import org.openremote.model.asset.impl.ThingAsset import org.openremote.model.attribute.AttributeRef import org.openremote.model.console.ConsoleProvider import org.openremote.model.console.ConsoleRegistration import org.openremote.model.console.ConsoleResource import org.openremote.model.notification.* import org.openremote.model.security.User +import org.openremote.model.util.UniqueIdentifierGenerator import org.openremote.setup.integration.KeycloakTestSetup import org.openremote.setup.integration.ManagerTestSetup import org.openremote.test.ManagerContainerTrait @@ -666,6 +668,101 @@ class NotificationTest extends Specification implements ManagerContainerTrait { notificationService.notificationHandlerMap.put(pushNotificationHandler.getTypeName(), pushNotificationHandler) } + def "#source device push with mode #pushTargetType and targets #targetNames is allowed: #allowed"() { + given: "a real notification route and target mapper with only delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def notificationService = container.getService(NotificationService.class) + def assetStorageService = container.getService(AssetStorageService.class) + def pushHandler = container.getService(PushNotificationHandler.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Custom push target ${UniqueIdentifierGenerator.generateId()}" + List deliveredTargetIds = new CopyOnWriteArrayList<>() + List createdAssetIds = [] + def storedNotifications = { + notificationService.getNotifications(null, [PushNotificationMessage.TYPE], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + PushNotificationHandler capturedPushHandler = Spy(pushHandler) + capturedPushHandler.isValid() >> true + capturedPushHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargetIds.add(target.id) + } + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, capturedPushHandler) + + and: "push consoles inside the rule scope, outside its subtree and in another realm" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def createConsole = { String name, String consoleRealm, String parentId -> + def console = assetStorageService.merge(new ConsoleAsset(name).setRealm(consoleRealm).setParentId(parentId) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + console + } + def allowedConsole = createConsole("${notificationName} allowed", realm, scopedAsset.id) + createConsole("${notificationName} unrelated", realm, null) + createConsole("${notificationName} foreign", keycloakSetup.realmCity.name, null) + def targets = [ + valid: new Notification.Target(Notification.TargetType.ASSET, allowedConsole.id), + custom: new Notification.Target(Notification.TargetType.CUSTOM, "unsupported-device-target") + ] + def notification = new Notification(notificationName, + new PushNotificationMessage().setTitle(notificationName).setTargetType(pushTargetType), + targetNames.collect { targets[it] }, null, null) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + + when: "the service receives the same source headers as the rules facade" + // Use request/reply on the real queue so all processing has completed before checking absence + // of delivery and persistence. No FCM calls are made by the capture above. + def result = notificationService.sendNotification(notification, source, sourceId) + + then: "unsupported targets reject the entire request before any delivery or stored record" + def persisted = storedNotifications() + if (allowed) { + verifyAll { + result + deliveredTargetIds == [allowedConsole.id] + persisted.size() == 1 + persisted[0].target == Notification.TargetType.ASSET + persisted[0].targetId == allowedConsole.id + persisted[0].realm == realm + persisted[0].source == source + persisted[0].sourceId == sourceId + persisted[0].error == null + } + } else { + verifyAll { + !result + deliveredTargetIds.isEmpty() + persisted.isEmpty() + } + } + + cleanup: + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, pushHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + + where: + [source, pushTargetType, targetNames, allowed] << [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [null, PushNotificationMessage.TargetType.DEVICE].collectMany { pushTargetType -> + [ + [["valid"], true], + [["custom"], false], + [["valid", "custom"], false], + [["custom", "valid"], false] + ].collect { row -> [source, pushTargetType] + row } + } + } + } + def "Check email notification functionality"() { List sentEmails = new CopyOnWriteArrayList<>() From 5594b7623972d754b2620597e09a00d585628d91 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:59:27 +0200 Subject: [PATCH 02/22] Reject CUSTOM targets in device push notifications before target expansion --- .../manager/notification/PushNotificationHandler.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index a6281e349b..f6b198a1ce 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -213,6 +213,11 @@ public List getTargets( } if (targets != null) { + if (targets.stream().anyMatch(target -> target.getType() == Notification.TargetType.CUSTOM)) { + throw new NotificationProcessingException( + NotificationProcessingException.Reason.ERROR_TARGET_MAPPING, + "CUSTOM targets are not supported for device push notifications"); + } // Filter out console targets String[] assetTargets = From 8b8ab1c06410fb1650773f037370709c797c4b11 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:09:45 +0200 Subject: [PATCH 03/22] Add failing regression test for global rules notification submission --- .../test/notification/NotificationTest.groovy | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 25b87503af..387cb4b111 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -31,6 +31,8 @@ import org.openremote.manager.notification.EmailNotificationHandler import org.openremote.manager.notification.LocalizedNotificationHandler import org.openremote.manager.notification.NotificationService import org.openremote.manager.notification.PushNotificationHandler +import org.openremote.manager.rules.RulesEngineId +import org.openremote.manager.rules.facade.NotificationsFacade import org.openremote.manager.rules.geofence.ORConsoleGeofenceAssetAdapter import org.openremote.manager.security.ManagerIdentityService import org.openremote.manager.setup.SetupService @@ -763,6 +765,78 @@ class NotificationTest extends Specification implements ManagerContainerTrait { } } + def "Global rules facade sends push notifications and stores them in each recipient realm"() { + given: "a global notifications facade using the real notification route" + def conditions = new PollingConditions(timeout: 10, delay: 0.2) + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def notificationService = container.getService(NotificationService.class) + def assetStorageService = container.getService(AssetStorageService.class) + def pushHandler = container.getService(PushNotificationHandler.class) + def facade = new NotificationsFacade(new RulesEngineId(), notificationService) + def notificationName = "Global rules push ${UniqueIdentifierGenerator.generateId()}" + List deliveredTargetIds = new CopyOnWriteArrayList<>() + List createdAssetIds = [] + def storedNotifications = { + notificationService.getNotifications(null, [PushNotificationMessage.TYPE], null, null, + null, null, null, Notification.Source.GLOBAL_RULESET, null, false, null, null, null).findAll { + it.name == notificationName + } + } + + and: "only push delivery is intercepted, so no FCM calls are made" + PushNotificationHandler capturedPushHandler = Spy(pushHandler) + capturedPushHandler.isValid() >> true + capturedPushHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, source, sourceId, target, message -> + deliveredTargetIds.add(target.id) + } + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, capturedPushHandler) + + and: "the notification targets a console in each of two realms" + def consoles = [keycloakSetup.realmBuilding.name, keycloakSetup.realmCity.name].collect { realm -> + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} ${realm}").setRealm(realm) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + console + } + def expectedRealms = consoles.collectEntries { [(it.id): it.realm] } + def notification = new Notification(notificationName, + new PushNotificationMessage().setTitle(notificationName).setTargetType(PushNotificationMessage.TargetType.DEVICE), + consoles.collect { + new Notification.Target(Notification.TargetType.ASSET, it.id) + }, null, null) + + when: "the global rule submits the notification through its facade" + facade.send(notification) + + then: "submission succeeds without a realm or asset source ID" + noExceptionThrown() + + and: "both recipients receive a notification stored under their own realm" + conditions.eventually { + assert deliveredTargetIds.size() == expectedRealms.size() + assert deliveredTargetIds.toSet() == expectedRealms.keySet() + def persisted = storedNotifications() + assert persisted.size() == expectedRealms.size() + assert persisted.collectEntries { [(it.targetId): it.realm] } == expectedRealms + assert persisted.every { + it.target == Notification.TargetType.ASSET && + it.source == Notification.Source.GLOBAL_RULESET && + it.sourceId == "" && it.error == null + } + } + + cleanup: + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, pushHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + } + def "Check email notification functionality"() { List sentEmails = new CopyOnWriteArrayList<>() From 9f6d5dbce8da89a74f27084e6eeff12c2d276e5b Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:12:45 +0200 Subject: [PATCH 04/22] Fix global rules notification submission with an empty source ID --- .../openremote/manager/rules/facade/NotificationsFacade.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manager/src/main/java/org/openremote/manager/rules/facade/NotificationsFacade.java b/manager/src/main/java/org/openremote/manager/rules/facade/NotificationsFacade.java index 4a61eed6b0..edd246902e 100644 --- a/manager/src/main/java/org/openremote/manager/rules/facade/NotificationsFacade.java +++ b/manager/src/main/java/org/openremote/manager/rules/facade/NotificationsFacade.java @@ -43,7 +43,7 @@ public NotificationsFacade( public void send(Notification notification) { Notification.Source source; - String sourceId = null; + String sourceId = ""; if (rulesEngineId.getScope() == GlobalRuleset.class) { source = GLOBAL_RULESET; From da643c59a6f34090d43193e3eef46598abb23485 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:23:37 +0200 Subject: [PATCH 05/22] Add failing tests for restricted recipients in asset-scoped rule notifications --- .../test/notification/NotificationTest.groovy | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 387cb4b111..543e4109ca 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -24,6 +24,8 @@ import jakarta.mail.Message import jakarta.mail.internet.InternetAddress import jakarta.ws.rs.BadRequestException import jakarta.ws.rs.WebApplicationException +import org.apache.camel.builder.NotifyBuilder +import org.openremote.container.message.MessageBrokerService import org.openremote.manager.asset.AssetProcessingService import org.openremote.manager.asset.AssetStorageService import org.openremote.manager.asset.console.ConsoleResourceImpl @@ -37,6 +39,7 @@ import org.openremote.manager.rules.geofence.ORConsoleGeofenceAssetAdapter import org.openremote.manager.security.ManagerIdentityService import org.openremote.manager.setup.SetupService import org.openremote.manager.web.ManagerWebService +import org.openremote.model.asset.UserAssetLink import org.openremote.model.asset.impl.ConsoleAsset import org.openremote.model.asset.impl.ThingAsset import org.openremote.model.attribute.AttributeRef @@ -837,6 +840,154 @@ class NotificationTest extends Specification implements ManagerContainerTrait { } } + def "Asset rules #messageType notification to users #targetNames is allowed: #allowed"() { + given: "an asset-scoped facade with real identity, asset and notification services" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def identityProvider = container.getService(ManagerIdentityService.class).getIdentityProvider() + def assetStorageService = container.getService(AssetStorageService.class) + def notificationService = container.getService(NotificationService.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Asset rules users ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List createdUsers = [] + List deliveredTargetIds = new CopyOnWriteArrayList<>() + List expectedTargetIds = [] + def storedNotifications = { + notificationService.getNotifications(null, [messageType], null, null, + null, null, null, Notification.Source.ASSET_RULESET, null, false, null, null, null).findAll { + it.name == notificationName + } + } + + and: "only delivery is intercepted, leaving access checks and target mapping intact" + def push = messageType == PushNotificationMessage.TYPE + def handler = container.getService(push ? PushNotificationHandler : EmailNotificationHandler) + def capturedHandler = Spy(handler) + capturedHandler.isValid() >> true + capturedHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, source, sourceId, target, message -> + deliveredTargetIds.add(target.id) + } + notificationService.notificationHandlerMap.put(messageType, capturedHandler) + + and: "the scoped asset has an ancestor, a descendant and an unrelated asset in the same realm" + def createAsset = { String name, String parentId -> + def asset = assetStorageService.merge(new ThingAsset("${notificationName} ${name}").setRealm(realm).setParentId(parentId)) + createdAssetIds.add(asset.id) + asset + } + def ancestor = createAsset("ancestor", null) + def scopedAsset = createAsset("scope", ancestor.id) + def child = createAsset("child", scopedAsset.id) + def unrelated = createAsset("unrelated", null) + def facade = new NotificationsFacade(new RulesEngineId(realm, scopedAsset.id), notificationService) + def scopeLinks = [restrictedScope: scopedAsset.id, restrictedChild: child.id, + restrictedAncestor: ancestor.id, restrictedUnrelated: unrelated.id] + + and: "recipients have the specified restricted role and direct asset links" + def targets = targetNames.collect { targetName -> + def userRealm = targetName == "foreign" ? keycloakSetup.realmCity.name : realm + def restricted = targetName.startsWith("restricted") + def username = "scoped-notif-${UniqueIdentifierGenerator.generateId()}" + def user = identityProvider.createUpdateUser(userRealm, + new User().setUsername(username).setEmail("${username}@example.com").setEnabled(true), null, false) + createdUsers.add([realm: userRealm, id: user.id]) + identityProvider.updateUserClientRoles(userRealm, user.id, KEYCLOAK_CLIENT_ID, READ_ASSETS_ROLE, READ_NOTIFICATIONS_ROLE) + if (restricted) { + identityProvider.updateUserRealmRoles(userRealm, user.id, + identityProvider.addUserRealmRoles(userRealm, user.id, RESTRICTED_USER_REALM_ROLE)) + } + assert identityProvider.getUserRealmRoles(userRealm, user.id).contains(RESTRICTED_USER_REALM_ROLE) == restricted + if (scopeLinks[targetName] != null) { + assetStorageService.storeUserAssetLinks([new UserAssetLink(userRealm, user.id, scopeLinks[targetName])]) + } + + if (push) { + // A linked console enables delivery; it does not grant access to the scoped asset itself. + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} ${targetName}") + .setRealm(userRealm).setParentId(userRealm == realm ? scopedAsset.id : null) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + assetStorageService.storeUserAssetLinks([new UserAssetLink(userRealm, user.id, console.id)]) + expectedTargetIds.add(console.id) + } else { + expectedTargetIds.add(user.id) + } + new Notification.Target(Notification.TargetType.USER, user.id) + } + def notificationMessage = push + ? new PushNotificationMessage().setTitle(notificationName).setTargetType(PushNotificationMessage.TargetType.DEVICE) + : new EmailNotificationMessage().setSubject(notificationName).setText("Asset rule notification") + def notification = new Notification(notificationName, notificationMessage, targets, null, null) + + and: "route completion is observed before asserting the absence of side effects" + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == Notification.Source.ASSET_RULESET && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == scopedAsset.id + } + .whenDone(1).create() + + when: "the asset rule sends directly to these users" + facade.send(notification) + + then: "the entire request is accepted or rejected before delivery and persistence" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + if (allowed) { + verifyAll { + deliveredTargetIds.size() == expectedTargetIds.size() + deliveredTargetIds.toSet() == expectedTargetIds.toSet() + persisted.size() == expectedTargetIds.size() + persisted.collect { it.targetId }.toSet() == expectedTargetIds.toSet() + persisted.every { + it.target == (push ? Notification.TargetType.ASSET : Notification.TargetType.USER) && + it.realm == realm && it.source == Notification.Source.ASSET_RULESET && + it.sourceId == scopedAsset.id && it.error == null + } + } + } else { + verifyAll { + deliveredTargetIds.isEmpty() + persisted.isEmpty() + } + } + + cleanup: + processed?.destroy() + notificationService.notificationHandlerMap.put(messageType, handler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUsers.each { identityProvider.deleteUser(it.realm, it.id) } + + where: + [messageType, targetNames, allowed] << [PushNotificationMessage.TYPE, EmailNotificationMessage.TYPE].collectMany { messageType -> + [ + [["unrestricted"], true], + [["restrictedScope"], true], + [["unrestricted", "restrictedScope"], true], + [["restrictedUnlinked"], false], + [["restrictedChild"], false], + [["restrictedAncestor"], false], + [["restrictedUnrelated"], false], + [["foreign"], false], + [["unrestricted", "restrictedUnlinked"], false], + [["restrictedUnlinked", "unrestricted"], false], + [["restrictedScope", "restrictedChild"], false], + [["restrictedChild", "restrictedScope"], false], + [["unrestricted", "foreign"], false], + [["foreign", "unrestricted"], false] + ].collect { row -> [messageType] + row } + } + } + def "Check email notification functionality"() { List sentEmails = new CopyOnWriteArrayList<>() From 6964bf8c62054726111a621a1759cb88f19873ef Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:56:19 +0200 Subject: [PATCH 06/22] Require scope links for restricted recipients of asset rule notifications --- .../manager/notification/NotificationService.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/manager/src/main/java/org/openremote/manager/notification/NotificationService.java b/manager/src/main/java/org/openremote/manager/notification/NotificationService.java index 91e94af372..c4258055a7 100644 --- a/manager/src/main/java/org/openremote/manager/notification/NotificationService.java +++ b/manager/src/main/java/org/openremote/manager/notification/NotificationService.java @@ -849,6 +849,20 @@ protected void checkAccess( throw new NotificationProcessingException( INSUFFICIENT_ACCESS, "Targets must all be in the same realm as the requester"); } + if (source == ASSET_RULESET && target.getType() == Notification.TargetType.USER) { + boolean restrictedRecipient = + Arrays.asList( + identityService + .getIdentityProvider() + .getUserRealmRoles(realm, target.getId())) + .contains(Constants.RESTRICTED_USER_REALM_ROLE); + if (restrictedRecipient + && !assetStorageService.isUserAssets(target.getId(), List.of(assetId))) { + throw new NotificationProcessingException( + INSUFFICIENT_ACCESS, + "Restricted user targets must be directly linked to the rule's scoped asset"); + } + } break; case ASSET: From 880cf89efe94130f9aa4a461ff980d70a80ad2d4 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:24:11 +0200 Subject: [PATCH 07/22] Add failing tests for scoped topic push rejection and document breaking change --- .../test/notification/NotificationTest.groovy | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 543e4109ca..6f32bf2661 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -768,6 +768,127 @@ class NotificationTest extends Specification implements ManagerContainerTrait { } } + def "#source push with mode #pushTargetType and #targetSelection targets is allowed: #allowed"() { + given: "the real notification route and push mapper with delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def notificationService = container.getService(NotificationService.class) + def assetStorageService = container.getService(AssetStorageService.class) + def pushHandler = container.getService(PushNotificationHandler.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Scoped push audience ${UniqueIdentifierGenerator.generateId()}" + List deliveredTargets = new CopyOnWriteArrayList<>() + List createdAssetIds = [] + def storedNotifications = { + notificationService.getNotifications(null, [PushNotificationMessage.TYPE], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + PushNotificationHandler capturedPushHandler = Spy(pushHandler) + capturedPushHandler.isValid() >> true + // Intercept before FCM token lookup: a later delivery failure must not mask missing authorization. + capturedPushHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargets.add([type: target.type, id: target.id]) + } + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, capturedPushHandler) + + and: "a console that is a valid target for both realm and asset rules" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} console") + .setRealm(realm).setParentId(scopedAsset.id) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + def targets = [ + absent: null, + empty: [], + inScope: [new Notification.Target(Notification.TargetType.ASSET, console.id)] + ][targetSelection] + def audience = pushTargetType == PushNotificationMessage.TargetType.TOPIC ? "maintenance" : + pushTargetType == PushNotificationMessage.TargetType.CONDITION ? "'maintenance' in topics && 'staff' in topics" : null + def notification = new Notification(notificationName, + new PushNotificationMessage().setTitle(notificationName).setTargetType(pushTargetType).setTarget(audience), + targets, null, null) + + and: "the appropriate rules facade and a completion observer for its notification route" + def engineId = new RulesEngineId() + def sourceId = "" + if (source == Notification.Source.REALM_RULESET) { + engineId = new RulesEngineId(realm) + sourceId = realm + } else if (source == Notification.Source.ASSET_RULESET) { + engineId = new RulesEngineId(realm, scopedAsset.id) + sourceId = scopedAsset.id + } + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the notification is submitted by its source" + if (source == Notification.Source.INTERNAL) { + notificationService.sendNotificationAsync(notification, source, sourceId) + } else { + facade.send(notification) + } + + then: "scoped topic and condition requests are rejected before persistence or delivery" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + if (allowed) { + def expectedType = audience == null ? Notification.TargetType.ASSET : Notification.TargetType.CUSTOM + def expectedId = audience == null ? console.id : "${pushTargetType}: ${audience}".toString() + verifyAll { + deliveredTargets == [[type: expectedType, id: expectedId]] + persisted.size() == 1 + persisted[0].target == expectedType + persisted[0].targetId == expectedId + persisted[0].realm == (audience == null ? realm : MASTER_REALM) + persisted[0].source == source + persisted[0].sourceId == sourceId + persisted[0].error == null + } + } else { + verifyAll { + deliveredTargets.isEmpty() + persisted.isEmpty() + } + } + + cleanup: + processed?.destroy() + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, pushHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + + where: + [source, pushTargetType, targetSelection, allowed] << ( + [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [PushNotificationMessage.TargetType.TOPIC, PushNotificationMessage.TargetType.CONDITION].collectMany { pushTargetType -> + ["absent", "empty", "inScope"].collect { targetSelection -> + [source, pushTargetType, targetSelection, false] + } + } + [null, PushNotificationMessage.TargetType.DEVICE].collect { pushTargetType -> + [source, pushTargetType, "inScope", true] + } + } + [Notification.Source.GLOBAL_RULESET, Notification.Source.INTERNAL].collectMany { source -> + [PushNotificationMessage.TargetType.TOPIC, PushNotificationMessage.TargetType.CONDITION].collect { pushTargetType -> + [source, pushTargetType, "absent", true] + } + }) + } + def "Global rules facade sends push notifications and stores them in each recipient realm"() { given: "a global notifications facade using the real notification route" def conditions = new PollingConditions(timeout: 10, delay: 0.2) From b99b2366b99acef52ba59b33c83d0142fe0f9ec6 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:31:05 +0200 Subject: [PATCH 08/22] Reject topic and condition push notifications from realm- and asset-scoped rules --- .../manager/notification/PushNotificationHandler.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index f6b198a1ce..df5058274e 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -200,11 +200,17 @@ public List getTargets( List targets, AbstractNotificationMessage message) { - // Check if message is going to a topic if so then filter consoles subscribed to that topic PushNotificationMessage pushMessage = (PushNotificationMessage) message; List mappedTargets = new ArrayList<>(); if (pushMessage.getTargetType() == TOPIC || pushMessage.getTargetType() == CONDITION) { + // Firebase resolves these audiences, so their realm and asset scope cannot be verified. + if (source == Notification.Source.REALM_RULESET + || source == Notification.Source.ASSET_RULESET) { + throw new NotificationProcessingException( + NotificationProcessingException.Reason.INSUFFICIENT_ACCESS, + "Realm- and asset-scoped rules cannot send TOPIC or CONDITION push notifications"); + } mappedTargets.add( new Notification.Target( Notification.TargetType.CUSTOM, From ed095fd6efcafb3dde9e8fafcbb9b86d90b6e76e Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:47:16 +0200 Subject: [PATCH 09/22] Add failing tests for missing user targets in rule notifications --- .../test/notification/NotificationTest.groovy | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 6f32bf2661..cb06e5aa4e 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -1109,6 +1109,124 @@ class NotificationTest extends Specification implements ManagerContainerTrait { } } + def "#source #messageType notification with user targets #targetNames is allowed: #allowed"() { + given: "real rule notification processing with only delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def identityProvider = container.getService(ManagerIdentityService.class).getIdentityProvider() + def assetStorageService = container.getService(AssetStorageService.class) + def notificationService = container.getService(NotificationService.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Missing notification user ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List createdUserIds = [] + List deliveredTargetIds = new CopyOnWriteArrayList<>() + def storedNotifications = { + notificationService.getNotifications(null, [messageType], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + def push = messageType == PushNotificationMessage.TYPE + def handler = container.getService(push ? PushNotificationHandler : EmailNotificationHandler) + def capturedHandler = Spy(handler) + capturedHandler.isValid() >> true + capturedHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargetIds.add(target.id) + } + notificationService.notificationHandlerMap.put(messageType, capturedHandler) + + and: "a valid same-realm recipient and a user ID that does not exist" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def username = "missing-notif-${UniqueIdentifierGenerator.generateId()}" + def user = identityProvider.createUpdateUser(realm, + new User().setUsername(username).setEmail("${username}@example.com").setEnabled(true), null, false) + createdUserIds.add(user.id) + identityProvider.updateUserClientRoles(realm, user.id, KEYCLOAK_CLIENT_ID, READ_ASSETS_ROLE, READ_NOTIFICATIONS_ROLE) + assert !identityProvider.getUserRealmRoles(realm, user.id).contains(RESTRICTED_USER_REALM_ROLE) + def missingUserId = UUID.randomUUID().toString() + assert identityProvider.getUser(missingUserId) == null + def expectedTargetId = user.id + if (push) { + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} console") + .setRealm(realm).setParentId(scopedAsset.id) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + assetStorageService.storeUserAssetLinks([new UserAssetLink(realm, user.id, console.id)]) + expectedTargetId = console.id + } + def userIds = [valid: user.id, missing: missingUserId] + def notificationMessage = push + ? new PushNotificationMessage().setTitle(notificationName).setTargetType(PushNotificationMessage.TargetType.DEVICE) + : new EmailNotificationMessage().setSubject(notificationName).setText("Missing recipient test") + def notification = new Notification(notificationName, notificationMessage, + targetNames.collect { + new Notification.Target(Notification.TargetType.USER, userIds[it]) + }, null, null) + + and: "a facade for the requested scope and an observer that waits for processing to finish" + def engineId = source == Notification.Source.ASSET_RULESET ? new RulesEngineId(realm, scopedAsset.id) : new RulesEngineId(realm) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the rule submits the user targets" + facade.send(notification) + + then: "a missing user rejects the whole request before any delivery or stored record" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + if (allowed) { + verifyAll { + deliveredTargetIds == [expectedTargetId] + persisted.size() == 1 + persisted[0].target == (push ? Notification.TargetType.ASSET : Notification.TargetType.USER) + persisted[0].targetId == expectedTargetId + persisted[0].realm == realm + persisted[0].source == source + persisted[0].sourceId == sourceId + persisted[0].error == null + } + } else { + verifyAll { + deliveredTargetIds.isEmpty() + persisted.isEmpty() + } + } + + cleanup: + processed?.destroy() + notificationService.notificationHandlerMap.put(messageType, handler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUserIds.each { identityProvider.deleteUser(realm, it) } + + where: + [source, messageType, targetNames, allowed] << [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [PushNotificationMessage.TYPE, EmailNotificationMessage.TYPE].collectMany { messageType -> + [ + [["valid"], true], + [["missing"], false], + [["valid", "missing"], false], + [["missing", "valid"], false] + ].collect { row -> [source, messageType] + row } + } + } + } + def "Check email notification functionality"() { List sentEmails = new CopyOnWriteArrayList<>() From a3c466c03772f9bef661631af0b7ef6d08a8dde2 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:00:17 +0200 Subject: [PATCH 10/22] Reject missing user targets before processing scoped notifications --- .../notification/NotificationService.java | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/NotificationService.java b/manager/src/main/java/org/openremote/manager/notification/NotificationService.java index c4258055a7..88314525dd 100644 --- a/manager/src/main/java/org/openremote/manager/notification/NotificationService.java +++ b/manager/src/main/java/org/openremote/manager/notification/NotificationService.java @@ -825,21 +825,24 @@ protected void checkAccess( boolean realmMatch; if (target.getType() == Notification.TargetType.USER) { + User[] targetUsers = + identityService + .getIdentityProvider() + .queryUsers( + // Exclude service accounts and system accounts + new UserQuery() + .ids(target.getId()) + .serviceUsers(false) + .attributes( + new UserQuery.AttributeValuePredicate( + true, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE)))); + if (targetUsers.length == 0) { + throw new NotificationProcessingException( + INSUFFICIENT_ACCESS, + "User targets must exist and must not be service or system accounts"); + } realmMatch = - Arrays.stream( - identityService - .getIdentityProvider() - .queryUsers( - // Exclude service accounts and system accounts - new UserQuery() - .ids(target.getId()) - .serviceUsers(false) - .attributes( - new UserQuery.AttributeValuePredicate( - true, - new StringPredicate( - User.SYSTEM_ACCOUNT_ATTRIBUTE))))) - .allMatch(user -> realm.equals(user.getRealm())); + Arrays.stream(targetUsers).allMatch(user -> realm.equals(user.getRealm())); } else { // Can only send to the same realm as the requester realm realmMatch = realm.equals(target.getId()); From 053102c62a9dd90c56ac78c1e2c56893b63d8f7e Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:11:33 +0200 Subject: [PATCH 11/22] Add tests for realm filtering of expanded push recipients --- .../test/notification/NotificationTest.groovy | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index cb06e5aa4e..a2bcdb0067 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -1227,6 +1227,135 @@ class NotificationTest extends Specification implements ManagerContainerTrait { } } + def "#source push to #targetType with linked consoles #consoleNames and restricted user #restricted keeps #expectedConsoleNames"() { + given: "real rule notification processing with only push delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def identityProvider = container.getService(ManagerIdentityService.class).getIdentityProvider() + def assetStorageService = container.getService(AssetStorageService.class) + def notificationService = container.getService(NotificationService.class) + def pushHandler = container.getService(PushNotificationHandler.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Rule push expansion ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List createdUserIds = [] + List deliveredTargetIds = new CopyOnWriteArrayList<>() + def storedNotifications = { + notificationService.getNotifications(null, [PushNotificationMessage.TYPE], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + PushNotificationHandler capturedPushHandler = Spy(pushHandler) + capturedPushHandler.isValid() >> true + capturedPushHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargetIds.add(target.id) + } + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, capturedPushHandler) + + and: "push consoles exist inside the scope, outside its subtree and in another realm" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def createConsole = { String name, String consoleRealm, String parentId -> + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} ${name}") + .setRealm(consoleRealm).setParentId(parentId) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + console + } + def consoles = [scope: createConsole("scope", realm, scopedAsset.id), + outside: createConsole("outside", realm, null), + foreign: createConsole("foreign", keycloakSetup.realmCity.name, null)] + + and: "an authorized same-realm user is linked to the selected consoles" + def username = "push-expansion-${UniqueIdentifierGenerator.generateId()}" + def user = identityProvider.createUpdateUser(realm, + new User().setUsername(username).setEmail("${username}@example.com").setEnabled(true), null, false) + createdUserIds.add(user.id) + identityProvider.updateUserClientRoles(realm, user.id, KEYCLOAK_CLIENT_ID, READ_ASSETS_ROLE, READ_NOTIFICATIONS_ROLE) + if (restricted) { + identityProvider.updateUserRealmRoles(realm, user.id, + identityProvider.addUserRealmRoles(realm, user.id, RESTRICTED_USER_REALM_ROLE)) + assetStorageService.storeUserAssetLinks([new UserAssetLink(realm, user.id, scopedAsset.id)]) + } + assert identityProvider.getUserRealmRoles(realm, user.id).contains(RESTRICTED_USER_REALM_ROLE) == restricted + // Deliberately seed cross-realm links through the storage service: REST creation rejects them, + // but notification expansion must also respect the realm when such links exist in stored data. + assetStorageService.storeUserAssetLinks(consoleNames.collect { + new UserAssetLink(realm, user.id, consoles[it].id) + }) + def expectedTargetIds = expectedConsoleNames.collect { consoles[it].id } + def targets = targetType == Notification.TargetType.USER + ? [new Notification.Target(Notification.TargetType.USER, user.id)] + : consoleNames.collect { + new Notification.Target(Notification.TargetType.ASSET, consoles[it].id) + } + def notification = new Notification(notificationName, + new PushNotificationMessage().setTitle(notificationName).setTargetType(PushNotificationMessage.TargetType.DEVICE), + targets, null, null) + + and: "a facade for the requested scope and an observer that waits for processing to finish" + def engineId = source == Notification.Source.ASSET_RULESET ? new RulesEngineId(realm, scopedAsset.id) : new RulesEngineId(realm) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the rule sends to the user or explicitly to their consoles" + facade.send(notification) + + then: "only the expected consoles receive a notification and have a stored record" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + verifyAll { + deliveredTargetIds.size() == expectedTargetIds.size() + deliveredTargetIds.toSet() == expectedTargetIds.toSet() + persisted.size() == expectedTargetIds.size() + persisted.collect { it.targetId }.toSet() == expectedTargetIds.toSet() + persisted.every { + it.target == Notification.TargetType.ASSET && it.realm == realm && + it.source == source && it.sourceId == sourceId && it.error == null + } + } + + cleanup: + processed?.destroy() + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, pushHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUserIds.each { identityProvider.deleteUser(realm, it) } + + where: + [source, restricted, targetType, consoleNames, expectedConsoleNames] << ( + [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [false, true].collectMany { restricted -> + [ + [["scope"], ["scope"]], + [["outside"], ["outside"]], + [["foreign"], []], + [["scope", "foreign"], ["scope"]], + [["outside", "foreign"], ["outside"]] + ].collect { row -> [source, restricted, Notification.TargetType.USER] + row } + } + } + [ + // A user link permits delivery through a USER target; it does not widen direct ASSET scope. + [Notification.Source.ASSET_RULESET, false, Notification.TargetType.ASSET, ["outside"], []], + [Notification.Source.ASSET_RULESET, false, Notification.TargetType.ASSET, ["scope", "outside"], []], + [Notification.Source.ASSET_RULESET, false, Notification.TargetType.ASSET, ["outside", "scope"], []] + ]) + } + def "Check email notification functionality"() { List sentEmails = new CopyOnWriteArrayList<>() From 509de2d5819078d0b4cf8973e0c7b0321b515fe9 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:22:46 +0200 Subject: [PATCH 12/22] Filter expanded push recipients to the rule's realm --- .../notification/PushNotificationHandler.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index df5058274e..cdb97840d8 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -225,6 +225,21 @@ public List getTargets( "CUSTOM targets are not supported for device push notifications"); } + String rulesRealm = + switch (source) { + case REALM_RULESET -> sourceId; + case ASSET_RULESET -> { + Asset scopedAsset = assetStorageService.find(sourceId, false); + if (scopedAsset == null) { + throw new NotificationProcessingException( + NotificationProcessingException.Reason.INSUFFICIENT_ACCESS, + "Rule's scoped asset does not exist"); + } + yield scopedAsset.getRealm(); + } + default -> null; + }; + // Filter out console targets String[] assetTargets = targets.stream() @@ -295,6 +310,12 @@ public List getTargets( .toArray(String[]::new)); } + if (rulesRealm != null) { + // Delivery consoles must share the rule's realm, but need not be in its asset + // subtree. + assetQuery.realm(new RealmPredicate(rulesRealm)); + } + List consoleAssetIds = assetStorageService.findAll(assetQuery).stream() .map(Asset::getId) From 1b393233fc2308af7bdf0c161116255559fd2a71 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:34:06 +0200 Subject: [PATCH 13/22] Add failing tests for realm filtering of expanded email recipients --- .../test/notification/NotificationTest.groovy | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index a2bcdb0067..2b15611661 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -1356,6 +1356,127 @@ class NotificationTest extends Specification implements ManagerContainerTrait { ]) } + def "#source email to an asset with linked users #userNames and asset email #hasAssetEmail keeps users #expectedUserNames"() { + given: "real rule notification processing with only email delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def identityProvider = container.getService(ManagerIdentityService.class).getIdentityProvider() + def assetStorageService = container.getService(AssetStorageService.class) + def notificationService = container.getService(NotificationService.class) + def emailHandler = container.getService(EmailNotificationHandler.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Rule email expansion ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List createdUsers = [] + List deliveredRecipients = new CopyOnWriteArrayList<>() + def storedNotifications = { + notificationService.getNotifications(null, [EmailNotificationMessage.TYPE], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + EmailNotificationHandler capturedEmailHandler = Spy(emailHandler) + capturedEmailHandler.isValid() >> true + capturedEmailHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredRecipients.add([id: target.id, type: target.type, + address: (target.data as EmailNotificationMessage.Recipient)?.address]) + } + notificationService.notificationHandlerMap.put(EmailNotificationMessage.TYPE, capturedEmailHandler) + + and: "the target is a descendant asset with an optional email address of its own" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def assetEmail = "asset-${UUID.randomUUID()}@example.com" + def targetAsset = new ThingAsset("${notificationName} target").setRealm(realm).setParentId(scopedAsset.id) + if (hasAssetEmail) { + targetAsset.setEmail(assetEmail) + } + targetAsset = assetStorageService.merge(targetAsset) + createdAssetIds.add(targetAsset.id) + + and: "unrestricted users from the selected realms are linked to the target asset" + def users = userNames.collectEntries { userName -> + def userRealm = userName == "foreign" ? keycloakSetup.realmCity.name : realm + def username = "email-expansion-${UniqueIdentifierGenerator.generateId()}" + def user = identityProvider.createUpdateUser(userRealm, + new User().setUsername(username).setEmail("${username}@example.com").setEnabled(true), null, false) + createdUsers.add([realm: userRealm, id: user.id]) + identityProvider.updateUserClientRoles(userRealm, user.id, KEYCLOAK_CLIENT_ID, READ_ASSETS_ROLE, READ_NOTIFICATIONS_ROLE) + assert !identityProvider.getUserRealmRoles(userRealm, user.id).contains(RESTRICTED_USER_REALM_ROLE) + // Seed cross-realm links through storage to exercise filtering of existing data, independently + // of the REST checks that prevent creating such links. + assetStorageService.storeUserAssetLinks([new UserAssetLink(userRealm, user.id, targetAsset.id)]) + [(userName): user] + } + def expectedRecipients = expectedUserNames.collectEntries { userName -> + def user = users[userName] + [(user.id): [type: Notification.TargetType.USER, address: user.email]] + } + if (hasAssetEmail) { + expectedRecipients[targetAsset.id] = [type: Notification.TargetType.ASSET, address: assetEmail] + } + def notification = new Notification(notificationName, + new EmailNotificationMessage().setSubject(notificationName).setText("Asset recipient expansion"), + [new Notification.Target(Notification.TargetType.ASSET, targetAsset.id)], null, null) + + and: "a facade for the requested scope and an observer that waits for processing to finish" + def engineId = source == Notification.Source.ASSET_RULESET ? new RulesEngineId(realm, scopedAsset.id) : new RulesEngineId(realm) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the rule sends an email notification to the asset" + facade.send(notification) + + then: "foreign users are filtered while valid users and the asset email retain delivery and stored records" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + verifyAll { + deliveredRecipients.size() == expectedRecipients.size() + deliveredRecipients.collectEntries { + [(it.id): [type: it.type, address: it.address]] + } == expectedRecipients + persisted.size() == expectedRecipients.size() + persisted.collectEntries { + [(it.targetId): it.target] + } == expectedRecipients.collectEntries { id, recipient -> + [(id): recipient.type] + } + persisted.every { + it.realm == realm && it.source == source && it.sourceId == sourceId && it.error == null + } + } + + cleanup: + processed?.destroy() + notificationService.notificationHandlerMap.put(EmailNotificationMessage.TYPE, emailHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUsers.each { identityProvider.deleteUser(it.realm, it.id) } + + where: + [source, hasAssetEmail, userNames, expectedUserNames] << [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [false, true].collectMany { hasAssetEmail -> + [ + [[], []], + [["valid"], ["valid"]], + [["foreign"], []], + [["valid", "foreign"], ["valid"]] + ].collect { row -> [source, hasAssetEmail] + row } + } + } + } + def "Check email notification functionality"() { List sentEmails = new CopyOnWriteArrayList<>() From c30d39b0ff4895c4b46ad5f5a6888dfdc2a11626 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:50:50 +0200 Subject: [PATCH 14/22] Filter expanded email recipients by realm and share scope resolution --- .../EmailNotificationHandler.java | 7 +++ .../notification/NotificationScope.java | 46 +++++++++++++++++++ .../notification/PushNotificationHandler.java | 14 +----- 3 files changed, 54 insertions(+), 13 deletions(-) create mode 100644 manager/src/main/java/org/openremote/manager/notification/NotificationScope.java diff --git a/manager/src/main/java/org/openremote/manager/notification/EmailNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/EmailNotificationHandler.java index 048f9f26f7..40b8942b22 100644 --- a/manager/src/main/java/org/openremote/manager/notification/EmailNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/EmailNotificationHandler.java @@ -225,6 +225,8 @@ public List getTargets( List mappedTargets = new ArrayList<>(); if (targets != null) { + String rulesRealm = + NotificationScope.resolveRulesRealm(source, sourceId, assetStorageService); targets.forEach( target -> { @@ -267,6 +269,11 @@ public List getTargets( // Filter users that don't have disabled email notifications attribute if (userQuery != null) { + if (rulesRealm != null) { + // Stored asset links must not expand the audience beyond the rule's realm. + userQuery.realm(new RealmPredicate(rulesRealm)); + } + // Exclude service accounts, system accounts and accounts with disabled email // notifications userQuery diff --git a/manager/src/main/java/org/openremote/manager/notification/NotificationScope.java b/manager/src/main/java/org/openremote/manager/notification/NotificationScope.java new file mode 100644 index 0000000000..05e2cf5b21 --- /dev/null +++ b/manager/src/main/java/org/openremote/manager/notification/NotificationScope.java @@ -0,0 +1,46 @@ +/* + * 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.notification; + +import org.openremote.manager.asset.AssetStorageService; +import org.openremote.model.asset.Asset; +import org.openremote.model.notification.Notification; + +final class NotificationScope { + + private NotificationScope() {} + + /** Resolves the realm for realm- and asset-scoped rules, or {@code null} for other sources. */ + static String resolveRulesRealm( + Notification.Source source, String sourceId, AssetStorageService assetStorageService) { + return switch (source) { + case REALM_RULESET -> sourceId; + case ASSET_RULESET -> { + Asset scopedAsset = assetStorageService.find(sourceId, false); + if (scopedAsset == null) { + throw new NotificationProcessingException( + NotificationProcessingException.Reason.INSUFFICIENT_ACCESS, + "Rule's scoped asset does not exist"); + } + yield scopedAsset.getRealm(); + } + default -> null; + }; + } +} diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index cdb97840d8..5affdfca38 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -226,19 +226,7 @@ public List getTargets( } String rulesRealm = - switch (source) { - case REALM_RULESET -> sourceId; - case ASSET_RULESET -> { - Asset scopedAsset = assetStorageService.find(sourceId, false); - if (scopedAsset == null) { - throw new NotificationProcessingException( - NotificationProcessingException.Reason.INSUFFICIENT_ACCESS, - "Rule's scoped asset does not exist"); - } - yield scopedAsset.getRealm(); - } - default -> null; - }; + NotificationScope.resolveRulesRealm(source, sourceId, assetStorageService); // Filter out console targets String[] assetTargets = From be81c425448c3ffde50267ffc97df7f0c6861ce7 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:02:44 +0200 Subject: [PATCH 15/22] Add failing tests for restricted users in asset notification expansion --- .../test/notification/NotificationTest.groovy | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 2b15611661..ff24c2f7c5 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -1477,6 +1477,154 @@ class NotificationTest extends Specification implements ManagerContainerTrait { } } + def "#source #messageType expansion from a descendant asset to #userNames with asset email #hasAssetEmail keeps #expectedUserNames"() { + given: "real rule notification processing with only delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def identityProvider = container.getService(ManagerIdentityService.class).getIdentityProvider() + def assetStorageService = container.getService(AssetStorageService.class) + def notificationService = container.getService(NotificationService.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Restricted asset recipients ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List createdUserIds = [] + List deliveredTargets = new CopyOnWriteArrayList<>() + def storedNotifications = { + notificationService.getNotifications(null, [messageType], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + def push = messageType == PushNotificationMessage.TYPE + def handler = container.getService(push ? PushNotificationHandler : EmailNotificationHandler) + def capturedHandler = Spy(handler) + capturedHandler.isValid() >> true + capturedHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargets.add(target) + } + notificationService.notificationHandlerMap.put(messageType, capturedHandler) + + and: "the notification targets a descendant rather than the engine's scoped asset" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def targetAsset = new ThingAsset("${notificationName} target").setRealm(realm).setParentId(scopedAsset.id) + if (hasAssetEmail) { + targetAsset.setEmail("asset-${UUID.randomUUID()}@example.com") + } + targetAsset = assetStorageService.merge(targetAsset) + createdAssetIds.add(targetAsset.id) + + and: "all recipients are linked to the target, but only some restricted users are linked to the scope" + def userTargets = userNames.collectEntries { userName -> + def restricted = userName.startsWith("restricted") + def username = "asset-recipient-${UniqueIdentifierGenerator.generateId()}" + def user = identityProvider.createUpdateUser(realm, + new User().setUsername(username).setEmail("${username}@example.com").setEnabled(true), null, false) + createdUserIds.add(user.id) + identityProvider.updateUserClientRoles(realm, user.id, KEYCLOAK_CLIENT_ID, READ_ASSETS_ROLE, READ_NOTIFICATIONS_ROLE) + if (restricted) { + identityProvider.updateUserRealmRoles(realm, user.id, + identityProvider.addUserRealmRoles(realm, user.id, RESTRICTED_USER_REALM_ROLE)) + } + assert identityProvider.getUserRealmRoles(realm, user.id).contains(RESTRICTED_USER_REALM_ROLE) == restricted + assetStorageService.storeUserAssetLinks([new UserAssetLink(realm, user.id, targetAsset.id)]) + if (userName == "restrictedScope") { + assetStorageService.storeUserAssetLinks([new UserAssetLink(realm, user.id, scopedAsset.id)]) + } + assert assetStorageService.isUserAssets(user.id, [scopedAsset.id]) == (userName == "restrictedScope") + + def deliveryTarget = new Notification.Target(Notification.TargetType.USER, user.id) + if (push) { + // Every console is in scope; authorization must follow the recipient's own scope link. + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} ${userName}") + .setRealm(realm).setParentId(targetAsset.id) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + assetStorageService.storeUserAssetLinks([new UserAssetLink(realm, user.id, console.id)]) + deliveryTarget = new Notification.Target(Notification.TargetType.ASSET, console.id) + } + [(userName): deliveryTarget] + } + def expectedTargets = expectedUserNames.collectEntries { userName -> + def target = userTargets[userName] + [(target.id): target.type] + } + if (hasAssetEmail) { + expectedTargets[targetAsset.id] = Notification.TargetType.ASSET + } + def notificationMessage = push + ? new PushNotificationMessage().setTitle(notificationName).setTargetType(PushNotificationMessage.TargetType.DEVICE) + : new EmailNotificationMessage().setSubject(notificationName).setText("Restricted asset recipients") + def notification = new Notification(notificationName, notificationMessage, + [new Notification.Target(Notification.TargetType.ASSET, targetAsset.id)], null, null) + + and: "a facade for the requested scope and an observer that waits for processing to finish" + def engineId = source == Notification.Source.ASSET_RULESET ? new RulesEngineId(realm, scopedAsset.id) : new RulesEngineId(realm) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the rule sends to the descendant asset and the handler expands its linked users" + facade.send(notification) + + then: "only eligible recipients receive notifications and have stored records" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + verifyAll { + deliveredTargets.size() == expectedTargets.size() + deliveredTargets.collectEntries { [(it.id): it.type] } == expectedTargets + persisted.size() == expectedTargets.size() + persisted.collectEntries { [(it.targetId): it.target] } == expectedTargets + persisted.every { + it.realm == realm && it.source == source && it.sourceId == sourceId && it.error == null + } + } + + cleanup: + processed?.destroy() + notificationService.notificationHandlerMap.put(messageType, handler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUserIds.each { identityProvider.deleteUser(realm, it) } + + where: + [source, messageType, hasAssetEmail, userNames, expectedUserNames] << ( + [Notification.Source.ASSET_RULESET, Notification.Source.REALM_RULESET].collectMany { source -> + [PushNotificationMessage.TYPE, EmailNotificationMessage.TYPE].collectMany { messageType -> + [ + [["unrestricted"], ["unrestricted"]], + [["restrictedScope"], ["restrictedScope"]], + [["restrictedChild"], []], + [["unrestricted", "restrictedChild"], ["unrestricted"]], + [["restrictedScope", "restrictedChild"], ["restrictedScope"]] + ].collect { row -> + [source, messageType, false, row[0], source == Notification.Source.ASSET_RULESET ? row[1] : row[0]] + } + } + } + [ + [Notification.Source.ASSET_RULESET, EmailNotificationMessage.TYPE, true, ["restrictedChild"], []], + [ + Notification.Source.ASSET_RULESET, + EmailNotificationMessage.TYPE, + true, + ["restrictedScope", "restrictedChild"], + ["restrictedScope"] + ] + ]) + } + def "Check email notification functionality"() { List sentEmails = new CopyOnWriteArrayList<>() From ddf7346c184959bf30b92d7ff7a3ed4e4d43ee81 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:18:57 +0200 Subject: [PATCH 16/22] Filter restricted recipients during asset notification expansion --- .../EmailNotificationHandler.java | 11 ++++++++++ .../notification/NotificationScope.java | 21 +++++++++++++++++++ .../notification/NotificationService.java | 14 ++++++------- .../notification/PushNotificationHandler.java | 10 +++++++++ 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/EmailNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/EmailNotificationHandler.java index 40b8942b22..63ed4f89f4 100644 --- a/manager/src/main/java/org/openremote/manager/notification/EmailNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/EmailNotificationHandler.java @@ -291,6 +291,17 @@ true, new StringPredicate(User.SYSTEM_ACCOUNT_ATTRIBUTE)), .filter( user -> !user.isSystemAccount() && !TextUtil.isNullOrEmpty(user.getEmail())) + // Filter derived recipients without removing the asset's own email target. + .filter( + user -> + source != Notification.Source.ASSET_RULESET + || targetType != Notification.TargetType.ASSET + || NotificationScope.isUserInAssetScope( + user.getRealm(), + user.getId(), + sourceId, + managerIdentityService.getIdentityProvider(), + assetStorageService)) .map( user -> { Notification.Target emailTarget = diff --git a/manager/src/main/java/org/openremote/manager/notification/NotificationScope.java b/manager/src/main/java/org/openremote/manager/notification/NotificationScope.java index 05e2cf5b21..69b79912dc 100644 --- a/manager/src/main/java/org/openremote/manager/notification/NotificationScope.java +++ b/manager/src/main/java/org/openremote/manager/notification/NotificationScope.java @@ -18,7 +18,11 @@ */ package org.openremote.manager.notification; +import java.util.Arrays; +import java.util.List; import org.openremote.manager.asset.AssetStorageService; +import org.openremote.manager.security.ManagerIdentityProvider; +import org.openremote.model.Constants; import org.openremote.model.asset.Asset; import org.openremote.model.notification.Notification; @@ -26,6 +30,23 @@ final class NotificationScope { private NotificationScope() {} + /** + * Restricted recipients must be directly linked to the scoped asset; unrestricted recipients need + * no asset link. The realm must be the recipient's realm. Callers are responsible for enforcing + * realm authorization. + */ + static boolean isUserInAssetScope( + String realm, + String userId, + String scopedAssetId, + ManagerIdentityProvider identityProvider, + AssetStorageService assetStorageService) { + boolean restrictedRecipient = + Arrays.asList(identityProvider.getUserRealmRoles(realm, userId)) + .contains(Constants.RESTRICTED_USER_REALM_ROLE); + return !restrictedRecipient || assetStorageService.isUserAssets(userId, List.of(scopedAssetId)); + } + /** Resolves the realm for realm- and asset-scoped rules, or {@code null} for other sources. */ static String resolveRulesRealm( Notification.Source source, String sourceId, AssetStorageService assetStorageService) { diff --git a/manager/src/main/java/org/openremote/manager/notification/NotificationService.java b/manager/src/main/java/org/openremote/manager/notification/NotificationService.java index 88314525dd..18d1f3acb1 100644 --- a/manager/src/main/java/org/openremote/manager/notification/NotificationService.java +++ b/manager/src/main/java/org/openremote/manager/notification/NotificationService.java @@ -853,14 +853,12 @@ protected void checkAccess( INSUFFICIENT_ACCESS, "Targets must all be in the same realm as the requester"); } if (source == ASSET_RULESET && target.getType() == Notification.TargetType.USER) { - boolean restrictedRecipient = - Arrays.asList( - identityService - .getIdentityProvider() - .getUserRealmRoles(realm, target.getId())) - .contains(Constants.RESTRICTED_USER_REALM_ROLE); - if (restrictedRecipient - && !assetStorageService.isUserAssets(target.getId(), List.of(assetId))) { + if (!NotificationScope.isUserInAssetScope( + realm, + target.getId(), + assetId, + identityService.getIdentityProvider(), + assetStorageService)) { throw new NotificationProcessingException( INSUFFICIENT_ACCESS, "Restricted user targets must be directly linked to the rule's scoped asset"); diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index 5affdfca38..813e86c72e 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -293,6 +293,16 @@ public List getTargets( // Any users linked to this asset and then their linked console assets assetQuery.userIds( assetStorageService.findUserAssetLinks(null, null, targetId).stream() + // Check the recipient's scope link before expanding to their consoles. + .filter( + ual -> + source != Notification.Source.ASSET_RULESET + || NotificationScope.isUserInAssetScope( + ual.getId().getRealm(), + ual.getId().getUserId(), + sourceId, + managerIdentityService.getIdentityProvider(), + assetStorageService)) .map(ual -> ual.getId().getUserId()) .collect(Collectors.toSet()) .toArray(String[]::new)); From b38d13f97489cf34d3c88fa5d4d2029345be2e53 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:38:25 +0200 Subject: [PATCH 17/22] Add failing tests for user realm filtering in asset push expansion --- .../test/notification/NotificationTest.groovy | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index ff24c2f7c5..422c93f306 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -1477,6 +1477,122 @@ class NotificationTest extends Specification implements ManagerContainerTrait { } } + def "#source push to an asset with linked users #userNames keeps consoles for #expectedUserNames"() { + given: "real rule notification processing with only push delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def identityProvider = container.getService(ManagerIdentityService.class).getIdentityProvider() + def assetStorageService = container.getService(AssetStorageService.class) + def notificationService = container.getService(NotificationService.class) + def pushHandler = container.getService(PushNotificationHandler.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Asset push user realms ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List createdUsers = [] + List deliveredTargets = new CopyOnWriteArrayList<>() + def storedNotifications = { + notificationService.getNotifications(null, [PushNotificationMessage.TYPE], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + PushNotificationHandler capturedPushHandler = Spy(pushHandler) + capturedPushHandler.isValid() >> true + capturedPushHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargets.add(target) + } + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, capturedPushHandler) + + and: "the target is a descendant asset in the rule's realm" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def targetAsset = assetStorageService.merge(new ThingAsset("${notificationName} target") + .setRealm(realm).setParentId(scopedAsset.id)) + createdAssetIds.add(targetAsset.id) + + and: "users from the selected realms are linked to the target and each has an in-scope console" + def consoles = userNames.collectEntries { userName -> + def userRealm = userName == "foreign" ? keycloakSetup.realmCity.name : realm + def username = "asset-push-realm-${UniqueIdentifierGenerator.generateId()}" + def user = identityProvider.createUpdateUser(userRealm, + new User().setUsername(username).setEmail("${username}@example.com").setEnabled(true), null, false) + createdUsers.add([realm: userRealm, id: user.id]) + identityProvider.updateUserClientRoles(userRealm, user.id, KEYCLOAK_CLIENT_ID, READ_ASSETS_ROLE, READ_NOTIFICATIONS_ROLE) + assert !identityProvider.getUserRealmRoles(userRealm, user.id).contains(RESTRICTED_USER_REALM_ROLE) + + // Every console is in the rule's realm and subtree: only the intermediate user's realm + // distinguishes eligible recipients from foreign users. + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} ${userName}") + .setRealm(realm).setParentId(targetAsset.id) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + // Seed cross-realm links through storage to exercise existing inconsistent data, + // independently of the REST checks that prevent creating these links. + assetStorageService.storeUserAssetLinks([ + new UserAssetLink(userRealm, user.id, targetAsset.id), + new UserAssetLink(userRealm, user.id, console.id) + ]) + [(userName): console] + } + def expectedTargets = expectedUserNames.collectEntries { userName -> + [(consoles[userName].id): Notification.TargetType.ASSET] + } + def notification = new Notification(notificationName, + new PushNotificationMessage().setTitle(notificationName).setTargetType(PushNotificationMessage.TargetType.DEVICE), + [new Notification.Target(Notification.TargetType.ASSET, targetAsset.id)], null, null) + + and: "a facade for the requested scope and an observer that waits for processing to finish" + def engineId = source == Notification.Source.ASSET_RULESET ? new RulesEngineId(realm, scopedAsset.id) : new RulesEngineId(realm) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the rule sends to the asset and its linked users are expanded into console targets" + facade.send(notification) + + then: "only consoles reached through same-realm users receive notifications and have stored records" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + verifyAll { + deliveredTargets.size() == expectedTargets.size() + deliveredTargets.collectEntries { [(it.id): it.type] } == expectedTargets + persisted.size() == expectedTargets.size() + persisted.collectEntries { [(it.targetId): it.target] } == expectedTargets + persisted.every { + it.realm == realm && it.source == source && it.sourceId == sourceId && it.error == null + } + } + + cleanup: + processed?.destroy() + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, pushHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUsers.each { identityProvider.deleteUser(it.realm, it.id) } + + where: + [source, userNames, expectedUserNames] << [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [ + [["valid"], ["valid"]], + [["foreign"], []], + [["valid", "foreign"], ["valid"]], + [["foreign", "valid"], ["valid"]] + ].collect { row -> [source] + row } + } + } + def "#source #messageType expansion from a descendant asset to #userNames with asset email #hasAssetEmail keeps #expectedUserNames"() { given: "real rule notification processing with only delivery intercepted" def container = startContainer(defaultConfig(), defaultServices()) From 38f7361d70db3eadca376019d0242910bea62843 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 16:56:50 +0200 Subject: [PATCH 18/22] Filter asset-expanded push recipients by user realm --- .../notification/PushNotificationHandler.java | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index 813e86c72e..668052856b 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -289,23 +289,38 @@ public List getTargets( case USER -> // Any console assets linked to the target user assetQuery.userIds(targetId); - case ASSET -> + case ASSET -> { + if (rulesRealm == null) { // Any users linked to this asset and then their linked console assets assetQuery.userIds( assetStorageService.findUserAssetLinks(null, null, targetId).stream() - // Check the recipient's scope link before expanding to their consoles. + .map(ual -> ual.getId().getUserId()) + .distinct() + .toArray(String[]::new)); + } else { + // Resolve users in the rule's realm before expanding their consoles. + assetQuery.userIds( + Arrays.stream( + managerIdentityService + .getIdentityProvider() + .queryUsers( + new UserQuery() + .assets(targetId) + .realm(new RealmPredicate(rulesRealm)))) .filter( - ual -> + user -> source != Notification.Source.ASSET_RULESET || NotificationScope.isUserInAssetScope( - ual.getId().getRealm(), - ual.getId().getUserId(), + user.getRealm(), + user.getId(), sourceId, managerIdentityService.getIdentityProvider(), assetStorageService)) - .map(ual -> ual.getId().getUserId()) - .collect(Collectors.toSet()) + .map(User::getId) + .distinct() .toArray(String[]::new)); + } + } } if (rulesRealm != null) { From 120a147bc2bb9cccab4391a67cc33a5480b7b380 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 17:05:41 +0200 Subject: [PATCH 19/22] Add failing tests for service and system accounts in asset push expansion --- .../test/notification/NotificationTest.groovy | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 422c93f306..d484b45c46 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -1593,6 +1593,125 @@ class NotificationTest extends Specification implements ManagerContainerTrait { } } + def "#source push expansion from an asset to #accountTypes keeps accounts #expectedAccountTypes"() { + given: "real rule notification processing with only push delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def identityProvider = container.getService(ManagerIdentityService.class).getIdentityProvider() + def assetStorageService = container.getService(AssetStorageService.class) + def notificationService = container.getService(NotificationService.class) + def pushHandler = container.getService(PushNotificationHandler.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Asset push account types ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List createdUserIds = [] + List deliveredTargets = new CopyOnWriteArrayList<>() + def storedNotifications = { + notificationService.getNotifications(null, [PushNotificationMessage.TYPE], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + PushNotificationHandler capturedPushHandler = Spy(pushHandler) + capturedPushHandler.isValid() >> true + capturedPushHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargets.add(target) + } + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, capturedPushHandler) + + and: "the notification targets a descendant asset in the rule's realm" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def targetAsset = assetStorageService.merge(new ThingAsset("${notificationName} target") + .setRealm(realm).setParentId(scopedAsset.id)) + createdAssetIds.add(targetAsset.id) + + and: "the selected account types are linked to the asset and each has its own in-scope console" + def consoles = accountTypes.collectEntries { accountType -> + def username = "asset-push-account-${UniqueIdentifierGenerator.generateId()}" + def user = identityProvider.createUpdateUser(realm, + new User().setUsername(username).setEmail("${username}@example.com").setEnabled(true) + .setServiceAccount(accountType == "service").setSystemAccount(accountType == "system"), null, false) + createdUserIds.add(user.id) + identityProvider.updateUserClientRoles(realm, user.id, KEYCLOAK_CLIENT_ID, READ_ASSETS_ROLE, READ_NOTIFICATIONS_ROLE) + def storedUser = identityProvider.getUser(user.id) + assert storedUser.realm == realm + assert storedUser.isServiceAccount() == (accountType == "service") + assert storedUser.isSystemAccount() == (accountType == "system") + assert !identityProvider.getUserRealmRoles(realm, user.id).contains(RESTRICTED_USER_REALM_ROLE) + + // Realm and asset scope are valid for every console; account eligibility must be checked + // before expanding the users into these delivery targets. + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} ${accountType}") + .setRealm(realm).setParentId(targetAsset.id) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + assetStorageService.storeUserAssetLinks([ + new UserAssetLink(realm, user.id, targetAsset.id), + new UserAssetLink(realm, user.id, console.id) + ]) + [(accountType): console] + } + def expectedTargets = expectedAccountTypes.collectEntries { accountType -> + [(consoles[accountType].id): Notification.TargetType.ASSET] + } + def notification = new Notification(notificationName, + new PushNotificationMessage().setTitle(notificationName).setTargetType(PushNotificationMessage.TargetType.DEVICE), + [new Notification.Target(Notification.TargetType.ASSET, targetAsset.id)], null, null) + + and: "a facade for the requested scope and an observer that waits for processing to finish" + def engineId = source == Notification.Source.ASSET_RULESET ? new RulesEngineId(realm, scopedAsset.id) : new RulesEngineId(realm) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the rule sends to the asset and the handler expands its linked accounts" + facade.send(notification) + + then: "only regular users' consoles receive notifications and have stored records" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + verifyAll { + deliveredTargets.size() == expectedTargets.size() + deliveredTargets.collectEntries { [(it.id): it.type] } == expectedTargets + persisted.size() == expectedTargets.size() + persisted.collectEntries { [(it.targetId): it.target] } == expectedTargets + persisted.every { + it.realm == realm && it.source == source && it.sourceId == sourceId && it.error == null + } + } + + cleanup: + processed?.destroy() + notificationService.notificationHandlerMap.put(PushNotificationMessage.TYPE, pushHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUserIds.each { identityProvider.deleteUser(realm, it) } + + where: + [source, accountTypes, expectedAccountTypes] << [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [ + [["regular"], ["regular"]], + [["service"], []], + [["system"], []], + [["regular", "service"], ["regular"]], + [["regular", "system"], ["regular"]] + ].collect { row -> [source] + row } + } + } + def "#source #messageType expansion from a descendant asset to #userNames with asset email #hasAssetEmail keeps #expectedUserNames"() { given: "real rule notification processing with only delivery intercepted" def container = startContainer(defaultConfig(), defaultServices()) From 6dc22032205d05dc87abb6ee3087fe33b4cec544 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 17:15:55 +0200 Subject: [PATCH 20/22] Exclude service and system accounts from scoped asset push expansion --- .../manager/notification/PushNotificationHandler.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java index 668052856b..5414ed1891 100644 --- a/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java +++ b/manager/src/main/java/org/openremote/manager/notification/PushNotificationHandler.java @@ -298,7 +298,8 @@ public List getTargets( .distinct() .toArray(String[]::new)); } else { - // Resolve users in the rule's realm before expanding their consoles. + // Resolve regular, non-system users in the rule's realm before expanding their + // consoles. assetQuery.userIds( Arrays.stream( managerIdentityService @@ -306,7 +307,13 @@ public List getTargets( .queryUsers( new UserQuery() .assets(targetId) - .realm(new RealmPredicate(rulesRealm)))) + .realm(new RealmPredicate(rulesRealm)) + .serviceUsers(false) + .attributes( + new UserQuery.AttributeValuePredicate( + true, + new StringPredicate( + User.SYSTEM_ACCOUNT_ATTRIBUTE))))) .filter( user -> source != Notification.Source.ASSET_RULESET From dc1ebdbcbf66cf2fc8dd47d531e49f19c8fd4d22 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 17:28:44 +0200 Subject: [PATCH 21/22] Add scoped rule tests for localized push notifications --- .../test/notification/NotificationTest.groovy | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index d484b45c46..07ec36e3fa 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -1972,6 +1972,121 @@ class NotificationTest extends Specification implements ManagerContainerTrait { notificationService.notificationHandlerMap.put(emailNotificationHandler.getTypeName(), emailNotificationHandler) } + def "#source localized push with #pushTargetType in nl and default #defaultLanguage is allowed: #allowed"() { + given: "real scoped notification processing and localization with only push delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def notificationService = container.getService(NotificationService.class) + def assetStorageService = container.getService(AssetStorageService.class) + def localizedHandler = container.getService(LocalizedNotificationHandler.class) + def pushHandler = container.getService(PushNotificationHandler.class) + def realm = keycloakSetup.realmBuilding.name + def notificationName = "Localized rule push ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List deliveredTargets = new CopyOnWriteArrayList<>() + def storedNotifications = { + notificationService.getNotifications(null, [LocalizedNotificationMessage.TYPE], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + PushNotificationHandler capturedPushHandler = Spy(pushHandler) + capturedPushHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargets.add([id: id, source: notificationSource, sourceId: notificationSourceId, + target: target.type, targetId: target.id, title: (message as PushNotificationMessage).title, + pushTargetType: (message as PushNotificationMessage).targetType]) + } + def previousLocalizedHandler = notificationService.notificationHandlerMap.put(LocalizedNotificationMessage.TYPE, localizedHandler) + // The localized handler has its own handler map; keep its validation, mapping and dispatch real. + def previousPushHandler = localizedHandler.notificationHandlerMap.put(PushNotificationMessage.TYPE, capturedPushHandler) + + and: "an in-scope console without user links so dispatch uses the notification's default language" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} console") + .setRealm(realm).setParentId(scopedAsset.id) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + + and: "a valid device message followed by the tested message in another locale" + def audience = pushTargetType == PushNotificationMessage.TargetType.TOPIC ? "maintenance" : + pushTargetType == PushNotificationMessage.TargetType.CONDITION ? "'maintenance' in topics && 'staff' in topics" : null + Map messages = [ + en: new PushNotificationMessage().setTitle("${notificationName} en").setTargetType(PushNotificationMessage.TargetType.DEVICE), + nl: new PushNotificationMessage().setTitle("${notificationName} nl").setTargetType(pushTargetType).setTarget(audience) + ] + def localizedMessage = new LocalizedNotificationMessage(defaultLanguage, messages) + assert localizedHandler.isMessageValid(localizedMessage) + def notification = new Notification(notificationName, localizedMessage, + [new Notification.Target(Notification.TargetType.ASSET, console.id)], null, null) + + and: "a scoped rules facade and an observer that waits for notification processing to finish" + def engineId = source == Notification.Source.ASSET_RULESET ? new RulesEngineId(realm, scopedAsset.id) : new RulesEngineId(realm) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the rule sends the localized notification" + facade.send(notification) + + then: "device messages are localized, while any forbidden locale rejects all delivery and persistence" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + if (allowed) { + assert persisted.size() == 1 + verifyAll { + deliveredTargets == [ + [id: persisted[0].id, source: source, sourceId: sourceId, + target: Notification.TargetType.ASSET, targetId: console.id, + title: "${notificationName} ${defaultLanguage}".toString(), pushTargetType: PushNotificationMessage.TargetType.DEVICE] + ] + persisted[0].target == Notification.TargetType.ASSET + persisted[0].targetId == console.id + persisted[0].realm == realm + persisted[0].source == source + persisted[0].sourceId == sourceId + persisted[0].type == LocalizedNotificationMessage.TYPE + persisted[0].error == null + } + } else { + verifyAll { + deliveredTargets.isEmpty() + persisted.isEmpty() + } + } + + cleanup: + processed?.destroy() + localizedHandler.notificationHandlerMap.put(PushNotificationMessage.TYPE, previousPushHandler) + notificationService.notificationHandlerMap.put(LocalizedNotificationMessage.TYPE, previousLocalizedHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + + where: + [source, pushTargetType, defaultLanguage] << [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [ + PushNotificationMessage.TargetType.DEVICE, + PushNotificationMessage.TargetType.TOPIC, + PushNotificationMessage.TargetType.CONDITION + ].collectMany { pushTargetType -> + ["en", "nl"].collect { defaultLanguage -> [source, pushTargetType, defaultLanguage] } + } + } + allowed = pushTargetType == PushNotificationMessage.TargetType.DEVICE + } + def "Check localized notification functionality"() { List localizedNotificationIds = new CopyOnWriteArrayList<>() From 154395e89325dc96aa54ac0829e69662c8aa876c Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Thu, 22 Oct 2026 17:48:29 +0200 Subject: [PATCH 22/22] Add localized notification recipient scoping tests --- .../test/notification/NotificationTest.groovy | 181 ++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy index 07ec36e3fa..6a824f9ceb 100644 --- a/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy +++ b/test/src/test/groovy/org/openremote/test/notification/NotificationTest.groovy @@ -2087,6 +2087,187 @@ class NotificationTest extends Specification implements ManagerContainerTrait { allowed = pushTargetType == PushNotificationMessage.TargetType.DEVICE } + def "#source localized #messageType expansion to #userNames keeps #expectedUserNames in their preferred language"() { + given: "real rule processing, recipient expansion and localization with only delivery intercepted" + def container = startContainer(defaultConfig(), defaultServices()) + def keycloakSetup = container.getService(SetupService.class).getTaskOfType(KeycloakTestSetup.class) + def identityProvider = container.getService(ManagerIdentityService.class).getIdentityProvider() + def assetStorageService = container.getService(AssetStorageService.class) + def notificationService = container.getService(NotificationService.class) + def localizedHandler = container.getService(LocalizedNotificationHandler.class) + def push = messageType == PushNotificationMessage.TYPE + def handler = container.getService(push ? PushNotificationHandler : EmailNotificationHandler) + def realm = keycloakSetup.realmBuilding.name + def foreignRealm = keycloakSetup.realmCity.name + def notificationName = "Localized rule recipients ${UniqueIdentifierGenerator.generateId()}" + List createdAssetIds = [] + List createdUsers = [] + List deliveredTargets = new CopyOnWriteArrayList<>() + def storedNotifications = { + notificationService.getNotifications(null, [LocalizedNotificationMessage.TYPE], null, null, + null, null, null, source, null, false, null, null, null).findAll { + it.name == notificationName + } + } + def capturedHandler = Spy(handler) + capturedHandler.sendMessage(_ as Long, _ as Notification.Source, _ as String, + _ as Notification.Target, _ as AbstractNotificationMessage) >> { id, notificationSource, notificationSourceId, target, message -> + deliveredTargets.add([id: id, source: notificationSource, sourceId: notificationSourceId, + target: target.type, targetId: target.id, messageType: message.type, + content: push ? (message as PushNotificationMessage).title : (message as EmailNotificationMessage).subject, + address: push ? null : (target.data as EmailNotificationMessage.Recipient).address]) + } + def previousLocalizedHandler = notificationService.notificationHandlerMap.put(LocalizedNotificationMessage.TYPE, localizedHandler) + def previousHandler = localizedHandler.notificationHandlerMap.put(messageType, capturedHandler) + + and: "a descendant asset with no email address of its own" + def scopedAsset = assetStorageService.merge(new ThingAsset(notificationName).setRealm(realm)) + createdAssetIds.add(scopedAsset.id) + def targetAsset = assetStorageService.merge(new ThingAsset("${notificationName} target") + .setRealm(realm).setParentId(scopedAsset.id)) + createdAssetIds.add(targetAsset.id) + def createConsole = { String name, String consoleRealm, String parentId -> + def console = assetStorageService.merge(new ConsoleAsset("${notificationName} ${name}") + .setRealm(consoleRealm).setParentId(parentId) + .setConsoleProvider(PushNotificationMessage.TYPE, + new ConsoleProvider("fcm", true, true, true, true, false, + [token: "test-${UniqueIdentifierGenerator.generateId()}"]))) + createdAssetIds.add(console.id) + console + } + + and: "users with different eligibility and language preferences are linked to the target" + def recipients = userNames.collectEntries { userName -> + def userRealm = userName == "foreign" ? foreignRealm : realm + def restricted = userName.startsWith("restricted") + def language = userName == "restrictedScope" ? "en" : "nl" + def username = "localized-recipient-${UUID.randomUUID()}" + def email = "${username}@example.com".toString() + def user = identityProvider.createUpdateUser(userRealm, + new User().setUsername(username).setEmail(email).setEnabled(true) + .setServiceAccount(userName == "service").setSystemAccount(userName == "system") + .setAttribute(User.LOCALE_ATTRIBUTE, language), null, false) + createdUsers.add([realm: userRealm, id: user.id]) + if (userName == "service") { + // Keycloak creates service users without the supplied email; give it a usable address so + // account exclusion, rather than a missing address, determines eligibility. + user = identityProvider.createUpdateUser(userRealm, user.setEmail(email), null, true) + } + identityProvider.updateUserClientRoles(userRealm, user.id, KEYCLOAK_CLIENT_ID, READ_ASSETS_ROLE, READ_NOTIFICATIONS_ROLE) + if (restricted) { + identityProvider.updateUserRealmRoles(userRealm, user.id, + identityProvider.addUserRealmRoles(userRealm, user.id, RESTRICTED_USER_REALM_ROLE)) + } + def storedUser = identityProvider.getUser(user.id) + assert storedUser.realm == userRealm + assert storedUser.email == email + assert storedUser.isServiceAccount() == (userName == "service") + assert storedUser.isSystemAccount() == (userName == "system") + assert storedUser.attributeMap[User.LOCALE_ATTRIBUTE] == [language] + assert identityProvider.getUserRealmRoles(userRealm, user.id).contains(RESTRICTED_USER_REALM_ROLE) == restricted + + // Storage permits seeding cross-realm links, independently of REST link-creation checks. + assetStorageService.storeUserAssetLinks([new UserAssetLink(userRealm, user.id, targetAsset.id)]) + if (userName == "restrictedScope") { + assetStorageService.storeUserAssetLinks([new UserAssetLink(userRealm, user.id, scopedAsset.id)]) + } + assert assetStorageService.isUserAssets(user.id, [scopedAsset.id]) == (userName == "restrictedScope") + + def deliveryTarget = new Notification.Target(Notification.TargetType.USER, user.id) + if (push) { + def console = createConsole(userName, realm, targetAsset.id) + assetStorageService.storeUserAssetLinks([new UserAssetLink(userRealm, user.id, console.id)]) + deliveryTarget = new Notification.Target(Notification.TargetType.ASSET, console.id) + if (userName == "regular") { + // An eligible user's foreign console must also stay excluded after locale merging. + def foreignConsole = createConsole("foreign console", foreignRealm, null) + assetStorageService.storeUserAssetLinks([new UserAssetLink(userRealm, user.id, foreignConsole.id)]) + } + } + [(userName): [targetId: deliveryTarget.id, target: deliveryTarget.type, + content: "${notificationName} ${language}".toString(), address: push ? null : email]] + } + def expectedTargets = expectedUserNames.collectEntries { userName -> + def recipient = recipients[userName] + [(recipient.targetId): [target: recipient.target, content: recipient.content, address: recipient.address]] + } + + and: "both languages use the same channel, with English as the fallback" + Map messages = ["en", "nl"].collectEntries { language -> + def message = push + ? new PushNotificationMessage().setTitle("${notificationName} ${language}").setTargetType(PushNotificationMessage.TargetType.DEVICE) + : new EmailNotificationMessage().setSubject("${notificationName} ${language}").setText("Localized recipients ${language}") + [(language): message] + } + def localizedMessage = new LocalizedNotificationMessage("en", messages) + assert localizedHandler.isMessageValid(localizedMessage) + def notification = new Notification(notificationName, localizedMessage, + [new Notification.Target(Notification.TargetType.ASSET, targetAsset.id)], null, null) + def engineId = source == Notification.Source.ASSET_RULESET ? new RulesEngineId(realm, scopedAsset.id) : new RulesEngineId(realm) + def sourceId = source == Notification.Source.ASSET_RULESET ? scopedAsset.id : realm + def facade = new NotificationsFacade(engineId, notificationService) + def processed = new NotifyBuilder(container.getService(MessageBrokerService.class).context) + .fromRoute("NotificationQueue") + .filter { exchange -> + exchange.in.getHeader(Notification.HEADER_SOURCE) == source && + exchange.in.getHeader(Notification.HEADER_SOURCE_ID) == sourceId + } + .whenDone(1).create() + + when: "the rule sends the localized notification to the asset" + facade.send(notification) + + then: "eligible recipients receive their preferred language once, and filtered recipients leave no records" + noExceptionThrown() + assert processed.matches(10, TimeUnit.SECONDS) + def persisted = storedNotifications() + verifyAll { + deliveredTargets.size() == expectedTargets.size() + deliveredTargets.collectEntries { + [(it.targetId): [target: it.target, content: it.content, address: it.address]] + } == expectedTargets + deliveredTargets.every { + it.source == source && it.sourceId == sourceId && it.messageType == messageType + } + persisted.size() == expectedTargets.size() + persisted.collectEntries { + [(it.targetId): it.target] + } == expectedTargets.collectEntries { id, recipient -> + [(id): recipient.target] + } + deliveredTargets.collect { it.id }.toSet() == persisted.collect { it.id }.toSet() + persisted.every { + it.realm == realm && it.source == source && it.sourceId == sourceId && + it.type == LocalizedNotificationMessage.TYPE && it.error == null + } + } + + cleanup: + processed?.destroy() + localizedHandler.notificationHandlerMap.put(messageType, previousHandler) + notificationService.notificationHandlerMap.put(LocalizedNotificationMessage.TYPE, previousLocalizedHandler) + storedNotifications().each { notificationService.removeNotification(it.id) } + if (!createdAssetIds.isEmpty()) { + assert assetStorageService.delete(createdAssetIds) + } + createdUsers.each { identityProvider.deleteUser(it.realm, it.id) } + + where: + [source, messageType, userNames, expectedUserNames] << [Notification.Source.REALM_RULESET, Notification.Source.ASSET_RULESET].collectMany { source -> + [PushNotificationMessage.TYPE, EmailNotificationMessage.TYPE].collectMany { messageType -> + [ + [["regular", "restrictedScope"], ["regular", "restrictedScope"]], + [["restrictedChild"], source == Notification.Source.ASSET_RULESET ? [] : ["restrictedChild"]], + [ + ["regular", "restrictedScope", "restrictedChild", "foreign", "service", "system"], + source == Notification.Source.ASSET_RULESET ? ["regular", "restrictedScope"] : ["regular", "restrictedScope", "restrictedChild"] + ], + [["foreign", "service", "system"], []] + ].collect { row -> [source, messageType] + row } + } + } + } + def "Check localized notification functionality"() { List localizedNotificationIds = new CopyOnWriteArrayList<>()