From 167a48bffada2191cecc86a3797d5483a66b94f6 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 00:54:08 -0400 Subject: [PATCH] fix(alarms,acl): one condition event per Part 9 method call; gate Server-object model-change/audit events (#587, #588) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #587 — a client Acknowledge/Confirm/AddComment/Shelve/Enable/Disable emitted no condition event: the SDK's ReportStateChange is gated on AutoReportStateChanges (never set), and the engine re-projection found the node already in the new state and was suppressed by the delta-gate. WireMethodTransitionReporting now wraps each method's SDK handler and, on Good, reports exactly once via ReportConditionEvent, under Lock so the routed re-projection cannot race in ahead of the SDK applying the state. Independent of the SDK's imprecise AreEventsMonitored bookkeeping. The shelve methods' state is applied by the wrapper (the SDK never applies it). Native conditions' Confirmed/Shelving are method-owned (NativeAlarmProjector's constants would re-report every ack and undo every shelve). MaxTimeShelved set to int.MaxValue ms at materialise — Create() left it 0 and every TimedShelve failed BadShelvingTimeOutOfRange. Wrong "SDK auto-fires (E2)" comments fixed. #588 — with ACL on, GeneralModelChange and AuditWriteUpdate events no longer go to Server.ReportEvent (ungated to every i=2253 item): ReportToServerObject delivers them per item, a model change narrowed to the Affected nodes the session may Browse (dropped when none), an audit write only with Read on its source plus the SDK audit/encryption guard. Flag off: unchanged. #586 review: materialise raises only the new condition's monitored unit (was a full O(N) reconcile per condition ⇒ O(N²) deploys under Lock). Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 29 +- docs/AlarmTracking.md | 52 ++- docs/ScriptedAlarms.md | 2 +- docs/security.md | 35 +- .../OtOpcUaNodeManager.cs | 388 ++++++++++++++++-- .../AclEventSubscriptionEnforcementTests.cs | 9 +- .../AclServerObjectModelChangeTests.cs | 201 +++++++++ .../ConditionMethodEventTests.cs | 306 ++++++++++++++ .../ServerEventRig.cs | 373 +++++++++++++++++ .../NodeManagerModelChangeFilterTests.cs | 125 ++++++ 10 files changed, 1454 insertions(+), 66 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclServerObjectModelChangeTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionMethodEventTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerModelChangeFilterTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 5d271da7..a195b856 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -938,11 +938,20 @@ Two things to know before relying on any of this: the SDK's audit guard (`AuditEventState` only while `Server.Auditing` over SignAndEncrypt/https), because conditions report their Part 9 method audit events through themselves. `ReconcileAllEventsMonitoring` must keep `AreEventsMonitored` raised on every condition while any - `i=2253` item exists, because `ConditionState` gates its own state-change and audit reports on - `EventsMonitored()`. The protected `SubscribeToEvents` override that no-ops all-events items is what + `i=2253` item exists, because `ConditionState` gates its method **audit** reports on + `EventsMonitored()` (method-driven state-change events no longer depend on it — #587, below). The + protected `SubscribeToEvents` override that no-ops all-events items is what stops the SDK's `AddRootNotifier` (`unsubscribe: true` bug) taking that unit back. Both review - findings (review of `75e559f6`). ⚠️ Residual: audit/model-change events the manager reports straight - to `Server.ReportEvent` still go to the core Server object, ungated. + findings (review of `75e559f6`). ⚠️ The materialise path raises only the **new** condition's unit + (`RaiseAllEventsMonitoring`); a full reconcile there made a deploy of N alarms O(N²) under `Lock` with + ScadaBridge connected — keep the full reconcile to remove/rebuild/(un)subscribe. + **Model-change and audit events are gated too (Gitea #588).** The manager's own server-wide events + (`GeneralModelChangeEvent`, the rejected-write `AuditWriteUpdateEvent`) go through + `ReportToServerObject`: flag **off** ⇒ `Server.ReportEvent` as before; flag **on** ⇒ delivered per item to + `_allEventsItems` (same audience), a model change narrowed to the `Affected` nodes the session may + **Browse** (dropped when none — the builders' Messages name the node, so a narrowed copy gets a neutral + one), an audit write only with `Read` on its source node plus the SDK's audit/encryption guard. Any other + event type fails closed. Auditing is off by default (never configured in this repo). - **Anonymous is a normal grantable principal**, not a bypass. `OpcUa:AllowAnonymous` (default `true`, preserving today's behaviour) controls *admission*; when admitted the session authenticates as the reserved principal `anonymous` and gets roles from `NodeAcl` rows naming it, arriving at the gates as @@ -1092,6 +1101,18 @@ See `docs/ScriptEditor.md` for the full guide. Scripts may use the `{{equip}}` t Inbound operator acknowledge/shelve for scripted alarms is fully implemented. Two surfaces converge on the `alarm-commands` DPS topic: (1) OPC UA Part 9 condition methods (Acknowledge / Confirm / AddComment / OneShotShelve / TimedShelve / Unshelve) wired in `OtOpcUaNodeManager`, gated on the `AlarmAck` LDAP role via `RoleCarryingUserIdentity`; (2) AdminUI `/alerts` per-row buttons routed through the `AdminOperationsActor` singleton (gated by `DriverOperator` policy). `ScriptedAlarmHostActor` dispatches from the topic to the engine. Client.CLI supports `ack`, `confirm`, `shelve` commands. See `docs/ScriptedAlarms.md` §"Inbound operator ack/shelve" and `docs/AlarmTracking.md`. +⚠️ **One condition event per accepted method call is OURS, not the SDK's (Gitea #587).** The SDK's own +report is gated on `AutoReportStateChanges` (left `false`), so before #587 a client Acknowledge emitted **no** +condition event — the old "SDK auto-fires its own event (E2)" comments were wrong. `WireMethodTransitionReporting` +wraps every method's SDK handler (Ack/Confirm/AddComment/Enable/Disable/OneShotShelve/TimedShelve/Unshelve) and, +on Good, reports once via `ReportConditionEvent` — **under `Lock`**, so the engine's/driver's re-projection +(routed by a fire-and-forget Tell *before* the SDK applies state) waits and then hits the delta-gate with no +delta. Things to keep: the SDK's shelve handlers never apply shelving, so the wrapper does; a **native** +condition's Confirmed and Shelving are method-owned (`WriteAlarmCondition` neither projects nor compares them — +`NativeAlarmProjector`'s constants would otherwise re-report every ack and undo every shelve); `MaxTimeShelved` +is set to `int.MaxValue` ms at materialise because `Create()` left it 0 and every TimedShelve failed. Pinned +over the wire by `ConditionMethodEventTests` (exactly-once after a sentinel, folder + `i=2253`). + ## Historian / HistoryRead **Backend: HistorianGateway (sole historian backend).** As of the gateway-integration cutover, the diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index 281551cb..abcb1948 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -371,11 +371,53 @@ hosted service. `OnTimedUnshelve` (the SDK's automatic unshelve timer) bypasses the operator gate — it is system-initiated. -`WriteAlarmCondition` fires the Part 9 condition event only when the -incoming state differs from the node's current live state (delta-gate), -preventing the double-emit that would otherwise occur when the SDK -auto-applies the acked state and the engine re-projection fires a -duplicate event immediately after. +### One condition event per method call (Gitea #587) + +Every accepted Part 9 method call — Acknowledge, Confirm, AddComment, +OneShotShelve, TimedShelve, Unshelve, Enable, Disable, on a native or a +scripted condition — reports **exactly one** condition event carrying the +new state, to folder subscribers and (through the ACL) Server-object +subscribers alike. A refused call (bad status, e.g. a denied gate, an +already-acknowledged condition, Enable/Disable on a native condition) +reports nothing. + +- **The SDK does not report it.** `ConditionState.ReportStateChange` is + gated on `AutoReportStateChanges`, which this repo leaves `false` — so + before #587 a client Acknowledge produced no condition event at all (an + older comment claimed "the SDK auto-fires its own event"; it never did). + Instead the node manager wraps each method's SDK handler + (`WireMethodTransitionReporting`) and, once the handler returns Good, + reports the condition itself. That report does not depend on the SDK's + `AreEventsMonitored` bookkeeping, which is imprecise for conditions + materialised under an already-subscribed folder. +- **The SDK's shelve methods never apply shelving state** (they only call + `OnShelve`), so the wrapper applies it — with the same + `(shelved, oneShot, 0)` mapping the projection uses — before reporting. +- **The re-projection does not duplicate it.** The engine's (or driver's) + answer to the routed command reaches `WriteAlarmCondition`'s delta-gate, + which compares against the node's current state — already the new state — + and suppresses. The whole method call runs under the node-manager lock, so + a fast re-projection cannot slip in between the SDK applying the state and + the report. +- **Native conditions: Confirmed and Shelving are method-owned.** + `NativeAlarmProjector` models neither (it always says `Confirmed = true`, + `Unshelved`), while the SDK's Acknowledge sets `Confirmed = false`. So for a + native condition `WriteAlarmCondition` neither projects nor compares those + two fields — otherwise the driver's echo of an ack would report a second + event, and the next driver transition would undo (and report) a local + shelve. A native condition starts Confirmed; Confirm and the shelve methods + are still not routed to the driver (local only, as before), but they now + stick. Scripted conditions are unchanged: the engine models both fields. +- **TimedShelve works.** `Create()` left `MaxTimeShelved` at 0, and the SDK + rejects any shelving time above it, so every client TimedShelve failed with + `BadShelvingTimeOutOfRange`. It is now `int.MaxValue` ms (≈24.8 days — the + longest the SDK's own shelving timer can hold). +- Method **audit** events (`AuditConditionAcknowledgeEventType`, …) are still + the SDK's, delivered only while the server is auditing over an encrypted + channel. + +Engine-driven transitions are unaffected: `WriteAlarmCondition` still fires +whenever the incoming state differs from the node's current state. ### AdminUI path diff --git a/docs/ScriptedAlarms.md b/docs/ScriptedAlarms.md index 132baeb3..02123aee 100644 --- a/docs/ScriptedAlarms.md +++ b/docs/ScriptedAlarms.md @@ -358,7 +358,7 @@ On allow, the handler publishes a `Commons.OpcUa.AlarmCommand` (containing comma ### Delta-gate de-duplication -`WriteAlarmCondition` fires a Part 9 condition event only when the incoming state differs from the node's current live state. This suppresses the double-emit that would otherwise occur when the OPC UA SDK auto-applies the acked state on the node and the engine's re-projection then attempts to fire a duplicate event. +`WriteAlarmCondition` fires a Part 9 condition event only when the incoming state differs from the node's current live state. For an OPC UA method call (Acknowledge, Confirm, AddComment, the shelve methods, Enable/Disable) the SDK applies the new state to the node, and the node manager then reports **the one** condition event for it from the method path (Gitea #587 — the SDK reports none of its own, since `AutoReportStateChanges` stays false; the shelve methods' state is applied by the node manager, because the SDK's never applies it). The engine's later re-projection of the same transition finds the node already in that state and is suppressed, so each accepted call yields exactly one event. See `docs/AlarmTracking.md` §"One condition event per method call". ### AdminUI path diff --git a/docs/security.md b/docs/security.md index ae72af8f..3883f281 100644 --- a/docs/security.md +++ b/docs/security.md @@ -564,18 +564,37 @@ Behaviours worth knowing before enabling it: condition reports through itself (`AuditConditionAcknowledgeEventType`, Comment, Enable, Shelving) reach a Server-object item only while the server is auditing, over SignAndEncrypt or https-binary, which is exactly the SDK's own guard. And while any Server-object item exists, every condition counts as - **monitored** (`ConditionState.EventsMonitored()`), so the SDK still reports its method-driven state - changes on a server whose only subscribers are Server-object items. Both were caught in review: the + **monitored** (`ConditionState.EventsMonitored()`), which gates the SDK's method **audit** events on a + server whose only subscribers are Server-object items. Both were caught in review: the first cut queued `i=8944` to every Server-object item with auditing off, and left conditions unmonitored. The second also needed the SDK's `AddRootNotifier` neutralised: its "subscribe existing all-events items with `unsubscribe: true`" bug would otherwise take the unit back from every - condition rebuilt under a live item. + condition rebuilt under a live item. (Method-driven **state-change** events no longer depend on this + bookkeeping at all — the node manager reports them itself, Gitea #587; see `docs/AlarmTracking.md`. + The SDK's counter stays imprecise in one known way — a folder subscriber's unit never reaches a + condition materialised under that folder later, yet its unsubscribe still lowers it — which can now + only cost an audit event, and only while auditing is on.) + - Materialising a condition raises only **that** condition's monitored unit; the full reconcile (every + condition) runs on removal, rebuild and Server-object subscribe/delete. A deploy of N alarms under a + live Server-object item is therefore N bookkeeping steps, not N²/2 under the node-manager lock. - Residual, not closed by #586: **audit and model-change events** (`AuditWriteUpdateEvent`, - `GeneralModelChangeEvent`, …) are still reported straight to the Server object and reach every - Server-object item ungated. They are delivered only when auditing / model-change reporting is on (and - audit events only over an encrypted channel), and a node manager has no seam on the core Server - object's fan-out to filter them. + **Model-change and audit events on the Server object are gated too** (Gitea #588). The node manager's + own server-wide events — `GeneralModelChangeEvent` (NodeAdded / NodeDeleted / DataTypeChanged) and the + `AuditWriteUpdateEvent` of a rejected device write — used to go straight to `Server.ReportEvent`, where + the core Server object queued them to every Server-object item ungated: a session with no grant learned + the RawPath / UNS path of every node added, deleted or retyped, and (with auditing on) the written and + prior values of a rejected write. With the flag **on**, the node manager now delivers them itself, item by + item, to the Server-object items it already holds for #586 (the same audience — only Server-object items + ever received them): + - A **model-change** event keeps only the changes whose `Affected` node the session may **Browse** + (ancestors of a granted node included, as for Browse itself). Every event the server raises today carries + one change, so in practice it is delivered whole or not at all; a multi-change event would be delivered + as a copy holding only the visible changes, with a neutral `Message` (the original names the nodes). + - An **audit write** event needs `Read` on its source node — it carries the node's prior value and the + attempted one — on top of the SDK's own guard (server auditing on, SignAndEncrypt or https-binary). + Auditing stays **off by default**: this repo never sets `ServerConfiguration.AuditingEnabled`. + - Any other server-wide event type is withheld (fail closed); the node manager raises none today. + - With the flag **off**, these events go to `Server.ReportEvent` exactly as before. - **An ancestor of a granted node is browsable**, or no narrow grant would be reachable: a client arrives at a deep node by browsing its ancestors, and the trie otherwise inherits downward only. Note what this necessarily discloses — the *names* of the folders on the path to a granted node are visible diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 423c5629..a37b9dbd 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -825,11 +825,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // the incoming snapshot onto it), then decide fire-vs-suppress by comparing the incoming // snapshot to that current state. We gate against the NODE's state, NOT a "last written" // cache, because an inbound client ack the SDK applied (OnAcknowledge returned Good → - // SDK mutated AckedState + auto-fired its own event) NEVER passed through this method, so a - // last-written cache would be stale and wrongly report a delta. By the time the engine - // re-projects that ack here, the node already holds the acked state → no delta → suppress. + // SDK mutated AckedState, then WireMethodTransitionReporting reported the one event for it — + // Gitea #587) NEVER passed through this method, so a last-written cache would be stale and + // wrongly report a delta. By the time the engine re-projects that ack here, the node already + // holds the acked state → no delta → suppress. + // Gitea #587: a NATIVE condition's Confirmed and Shelving belong to the Part 9 methods, not + // to this projection — see WireMethodTransitionReporting. + var methodOwned = _nativeAlarmNodeIds.Contains(key); var current = ReadConditionDelta(condition); - var incoming = ToConditionDelta(state, condition); + var incoming = ToConditionDelta(state, condition, methodOwned); bool fire = ShouldFireConditionEvent(current, incoming); // EnabledState / AckedState / ActiveState are mandatory children — always present after @@ -841,11 +845,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 condition.SetEnableState(SystemContext, state.Enabled); condition.SetActiveState(SystemContext, state.Active); condition.SetAcknowledgedState(SystemContext, state.Acknowledged); - if (condition.ConfirmedState is not null) + if (condition.ConfirmedState is not null && !methodOwned) { condition.SetConfirmedState(SystemContext, state.Confirmed); } - if (condition.ShelvingState is not null) + if (condition.ShelvingState is not null && !methodOwned) { // SetShelvingState(shelved, oneShot, shelvingTime): map our 3-way kind onto the SDK's // (shelved, oneShot) flag pair. Timed shelving's expiry is owned by the engine, not the @@ -881,8 +885,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // change (the delta-gate decided above, against the node's pre-projection state). A // genuine engine-driven transition (alarm goes active/clear, severity bucket shifts, an // engine-side ack, etc.) differs from the node's current state → fire. The re-projection - // of a client ack the SDK already applied equals the node's current state → no delta → - // suppress, so we don't double-emit (E2 from the SDK + E3 from here). ReportConditionEvent + // of a client method call already applied (and already reported, by the method path — + // WireMethodTransitionReporting) equals the node's current state → no delta → suppress, so + // the transition is reported exactly once. ReportConditionEvent // stamps a fresh EventId, ClearChangeMasks, and ReportEvent — all still under this lock. if (fire) { @@ -970,16 +975,17 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// materialise) — no branch creation here. /// /// - /// Double-emit note (resolved by delta-gate). An inbound client Acknowledge/Confirm - /// goes through the SDK's own handler, which (after T18's gate returns Good) applies the acked - /// state to the node and auto-fires its own condition event (E2) — directly on the node, - /// BYPASSING . The engine then re-projects that same logical - /// transition through , which would otherwise fire a second - /// event (E3). 's delta-gate suppresses E3: it compares the - /// incoming snapshot against the NODE's CURRENT state, and because the SDK has ALREADY - /// pre-applied the inbound-ack state, the re-projection is a no-delta no-op (no fire). Genuine - /// engine-driven transitions still differ from the node's current state, so they fire here as - /// before. + /// Method-driven transitions (Gitea #587). An inbound client Acknowledge / Confirm / + /// AddComment / shelve / Enable / Disable goes through the SDK's own handler, which (after T18's + /// gate returns Good) applies the new state to the node, BYPASSING . + /// The SDK reports no event for it — its ReportStateChange is gated on + /// AutoReportStateChanges, which stays false — so the method path calls this method itself, + /// once, after the handler returns (). The engine then + /// re-projects that same transition through , whose delta-gate + /// compares against the NODE's CURRENT state, finds no change, and suppresses a second event. (An + /// earlier comment here claimed the SDK "auto-fires its own event (E2)"; it never did, so a client + /// ack produced no event at all.) Genuine engine-driven transitions still differ from the node's + /// current state, so they fire here as before. /// /// /// The materialised condition whose new state has already been projected; must be non-null. @@ -1046,11 +1052,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// ScriptedAlarmHostActor's inbound AlarmCommand handler — meaning /// CommentAdded ALWAYS originates from a client calling the condition's /// AddComment method. On that path T18's OnAddComment delegate returns - /// ServiceResult.Good, so the OPC UA SDK itself applies the comment to the node - /// and auto-fires the Part 9 comment event (E2) directly to subscribers — BEFORE the - /// engine re-projects via . When that re-projection - /// arrives here, the delta-gate sees no change in any compared field (the snapshot carries - /// no comments list) and correctly suppresses a second event (E3). Force-firing for + /// ServiceResult.Good, the OPC UA SDK applies the comment to the node, and the method + /// path reports the one Part 9 comment event (, + /// Gitea #587) — BEFORE the engine re-projects via . When that + /// re-projection arrives here, the delta-gate sees no change in any compared field (the snapshot + /// carries no comments list) and correctly suppresses a second event. Force-firing for /// CommentAdded would double-emit. There is no engine-internal or script-driven /// comment path, so suppression never drops a needed event. /// @@ -1111,16 +1117,24 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// the bucket the node holds (the projection calls SetSeverity(MapSeverity(...))), and an /// optional Confirmed/Shelving that the node can't actually hold (missing child) is folded to the /// node's read-back default so it never spuriously registers as a delta. - private static AlarmConditionDelta ToConditionDelta(AlarmConditionSnapshot state, AlarmConditionState condition) => new( + /// True for a native condition, whose Confirmed and Shelving the projection does + /// not apply (see ); they are folded to the node's current value so the + /// snapshot's constants can never register as a delta. + private static AlarmConditionDelta ToConditionDelta( + AlarmConditionSnapshot state, AlarmConditionState condition, bool methodOwned = false) => new( Active: state.Active, Acknowledged: state.Acknowledged, // If the node has no ConfirmedState child, the projection is a no-op there; mirror the node's // read-back default (true) so a snapshot Confirmed value can't create a phantom delta. - Confirmed: condition.ConfirmedState is not null ? state.Confirmed : true, + Confirmed: condition.ConfirmedState is null ? true + : methodOwned ? condition.ConfirmedState.Id?.Value ?? true + : state.Confirmed, Enabled: state.Enabled, // Likewise for shelving: without a ShelvingState child the projection can't apply, so fold to the // node's read-back default (Unshelved). - Shelving: condition.ShelvingState is not null ? state.Shelving : AlarmShelvingKind.Unshelved, + Shelving: condition.ShelvingState is null ? AlarmShelvingKind.Unshelved + : methodOwned ? ReadShelvingKind(condition) + : state.Shelving, MappedSeverity: (ushort)MapSeverity(state.Severity), Message: state.Message ?? string.Empty, // If the node has no Quality child, WriteAlarmCondition's projection is a no-op there; fold to the @@ -1201,6 +1215,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 UnwireAlarmNotifiers(conditionKey); existing.Parent?.RemoveChild(existing); PredefinedNodes?.Remove(existing.NodeId); + // The materialise tail raises only the new instance's unit (no full reconcile), so give the + // dropped instance's back here rather than leave it tracked until the next full reconcile. + if (_allEventsMonitoredConditions.Remove(existing)) + existing.SetAreEventsMonitored(SystemContext, false, includeChildren: true); } // H6a: re-materialising the same id as the OTHER kind (native↔scripted) must reflect the new @@ -1276,7 +1294,16 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 alarm.SetEnableState(SystemContext, true); alarm.SetActiveState(SystemContext, false); alarm.SetAcknowledgedState(SystemContext, true); + // A native condition's Confirmed is method-owned (Gitea #587 — see WireMethodTransitionReporting), so + // no projection will ever set it: start it confirmed, the value the driver projection used to supply. + if (isNative && alarm.ConfirmedState is not null) alarm.SetConfirmedState(SystemContext, true); alarm.SetSeverity(SystemContext, MapSeverity(severity)); + // Gitea #587 (found by its TimedShelve test): Create() leaves MaxTimeShelved at 0, and the SDK's + // TimedShelve handler rejects any ShelvingTime above it — so every client TimedShelve failed with + // BadShelvingTimeOutOfRange. The bound becomes the longest shelve the SDK's own shelving timer can + // represent (int.MaxValue ms ≈ 24.8 days, the same ceiling SetShelvingState already saturated a 0 to). + if (alarm.MaxTimeShelved is { } maxTimeShelved && maxTimeShelved.Value <= 0) + maxTimeShelved.Value = int.MaxValue; alarm.Retain.Value = false; // inactive + acked ⇒ nothing to retain yet alarm.Message.Value = new LocalizedText(displayName); if (alarm.ConditionName is not null) alarm.ConditionName.Value = displayName; @@ -1286,10 +1313,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // routing; these delegates are the veto/permission seam the SDK invokes BEFORE applying the // state change. Each gates on the caller's AlarmAck role (fails closed) and, when allowed, // routes a mapped AlarmCommand to the engine via AlarmCommandRouter, then returns Good so the - // SDK applies its node state + auto-fires its own event (E2). - // T20: the engine re-projects that same logical transition through WriteAlarmCondition; its - // delta-gate (compares against the node's current state, which the SDK already pre-applied) - // sees no change and suppresses the would-be second event (E3) — so no double-emit. + // SDK applies its node state. The SDK does NOT report an event for it (its ReportStateChange is + // gated on AutoReportStateChanges, which stays false) — the ONE condition event per transition + // is ours, raised after the SDK handler returns by WireMethodTransitionReporting (Gitea #587). + // The engine's later re-projection of the same transition reaches WriteAlarmCondition's + // delta-gate, which finds the node already in that state and suppresses a second event. // H6c — a NATIVE (driver-fed) condition's Acknowledge belongs to the backing driver, NOT the // scripted engine, so branch on native-ness (via the lock-guarded accessor) and route a native // ack to NativeAlarmAckRouter instead of the scripted AlarmCommandRouter. Confirm/AddComment/ @@ -1348,6 +1376,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 return HandleAlarmCommand(context, condition, enabling ? "Enable" : "Disable", comment: null, unshelveAt: null); }; + // Gitea #587: exactly one condition event per accepted method call, reported by us once the SDK's + // own handler has applied the new state. + WireMethodTransitionReporting(alarm, conditionKey); + // Gitea #586: the condition itself delivers its events to Server-object (all-events) // subscriptions, item by item through the ACL. A condition is never an event notifier, so no // MonitoredNode2 ever claims this single-slot delegate — every event the condition reports, ours @@ -1363,7 +1395,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 AddPredefinedNode(SystemContext, alarm); _alarmConditions[conditionKey] = alarm; - ReconcileAllEventsMonitoring(); + // Only the new condition can be missing its unit here, so raise just that one — a full reconcile + // walks every condition, which made a deploy of N alarms O(N²) under Lock whenever any Server-object + // item was live (always, in production). Removal, rebuild and (un)subscribe keep the full reconcile. + RaiseAllEventsMonitoring(alarm); // H6a: record native (driver-fed) conditions so a later task can route their inbound // Acknowledge to the driver rather than the scripted engine. if (isNative) _nativeAlarmNodeIds.Add(conditionKey); @@ -1487,6 +1522,112 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 _alarmNotifierWiring.Remove(alarmKey); } + /// + /// Wraps each Part 9 method the SDK wired on (Acknowledge, Confirm, AddComment, + /// Enable, Disable, OneShotShelve, TimedShelve, Unshelve) so an accepted call reports exactly one + /// condition event carrying the post-call state (Gitea #587). + /// + /// The freshly created condition; its method children carry the SDK's handlers. + /// Its key in _alarmConditions. + /// + /// + /// Why not AutoReportStateChanges. The SDK's own report from a method handler + /// (ConditionState.ReportStateChange) is gated on AutoReportStateChanges — which this + /// repo had never set, so a client Acknowledge produced no condition event at all, and the engine's + /// re-projection found the node already acked and suppressed its own — and then on + /// EventsMonitored(), the AreEventsMonitored counter. That counter is imprecise in the + /// SDK: a folder subscriber's unit never reaches a condition materialised under the folder later, yet + /// the folder's unsubscribe still lowers it, which can take away the Server-object unit while an + /// i=2253 item lives (#586 review). And the SDK's shelve handlers report before any + /// shelving state is applied (they only call OnShelve), so their event would carry the old + /// state and the engine's re-projection would then report a second one. Reporting ourselves, after + /// the handler returns, depends on none of that: walks the notifier + /// graph unconditionally, and Server-object delivery goes through + /// and the ACL like every other condition event. + /// + /// + /// Shelving is applied here. The SDK's shelve methods never change ShelvingState + /// themselves, so an accepted shelve call applies it — with the same (shelved, oneShot, 0) + /// mapping projects — before reporting. The engine's re-projection of + /// the shelve then finds no delta. Every other method's state the SDK has already applied. + /// + /// + /// Exactly once. The whole call runs under Lock. The method handlers hand the command + /// to the engine (a fire-and-forget Tell) before the SDK applies the state, so without the lock a fast + /// re-projection could take Lock first, see a delta, and report, followed by this report — two + /// events. Holding it makes the re-projection wait until the node holds the new state. + /// + /// + /// Native conditions: Confirmed and Shelving are the methods'. A native condition's state is + /// re-projected from each driver transition by NativeAlarmProjector, which models neither + /// confirmation nor shelving — it always says Confirmed = true, Unshelved. The SDK's + /// Acknowledge sets Confirmed = false (the condition supports Confirm), so the driver's own + /// echo of that ack would otherwise flip it back and report a second event for one ack, and a + /// local shelve would be undone (and reported) by the next driver transition. So for a native + /// condition neither projects nor compares those two fields: + /// they change only through these methods, each reported once here. A scripted condition's engine + /// models both, so its projection still owns them. + /// + /// + /// A rejected call (bad , e.g. acknowledging an acknowledged condition, a + /// denied gate, or Enable/Disable on a native condition) reports nothing: there was no transition. + /// + /// + private void WireMethodTransitionReporting(AlarmConditionState alarm, RealmKey conditionKey) + { + alarm.AutoReportStateChanges = false; // the SDK's own report stays off; ours replaces it (see remarks) + + WrapComment(alarm.Acknowledge); + WrapComment(alarm.Confirm); + WrapComment(alarm.AddComment); + WrapGeneric(alarm.Enable, apply: null); + WrapGeneric(alarm.Disable, apply: null); + + if (alarm.ShelvingState is { } shelving) + { + WrapGeneric(shelving.OneShotShelve, () => alarm.SetShelvingState(SystemContext, shelved: true, oneShot: true, shelvingTime: 0)); + WrapGeneric(shelving.Unshelve, () => alarm.SetShelvingState(SystemContext, shelved: false, oneShot: false, shelvingTime: 0)); + if (shelving.TimedShelve is { OnCall: { } timed } timedShelve) + { + timedShelve.OnCall = (context, method, objectId, shelvingTime) => ReportAfterMethod( + () => timed(context, method, objectId, shelvingTime), + () => alarm.SetShelvingState(SystemContext, shelved: true, oneShot: false, shelvingTime: 0)); + } + } + + void WrapComment(AddCommentMethodState? method) + { + if (method?.OnCall is not { } sdk) return; + method.OnCall = (context, m, objectId, eventId, comment) => + ReportAfterMethod(() => sdk(context, m, objectId, eventId, comment), apply: null); + } + + void WrapGeneric(MethodState? method, Action? apply) + { + if (method?.OnCallMethod is not { } sdk) return; + method.OnCallMethod = (context, m, inputs, outputs) => + ReportAfterMethod(() => sdk(context, m, inputs, outputs), apply); + } + + ServiceResult ReportAfterMethod(Func sdkHandler, Action? apply) + { + lock (Lock) + { + var result = sdkHandler(); + // A rebuild may have discarded this instance mid-call; its replacement is not what was called. + if (ServiceResult.IsGood(result) + && _alarmConditions.TryGetValue(conditionKey, out var live) + && ReferenceEquals(live, alarm)) + { + apply?.Invoke(); + ReportConditionEvent(alarm, DateTime.UtcNow); + } + + return result; + } + } + } + /// H6a — true if the condition materialised at is a NATIVE /// (driver-fed) alarm rather than a scripted one. A later task uses this to route a native condition's /// inbound Acknowledge to the driver instead of the scripted engine. @@ -1513,8 +1654,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// The Part 9 operation name (e.g. Acknowledge, TimedShelve). /// The call's comment text, or null when none was supplied. /// For TimedShelve, the computed UTC expiry; otherwise null. - /// ServiceResult.Good when allowed (the SDK then applies state + auto-fires its - /// event); BadUserAccessDenied when the gate vetoes (no route, no state mutation). + /// ServiceResult.Good when allowed (the SDK then applies state, and the method path + /// reports the one condition event — Gitea #587);BadUserAccessDenied when the gate vetoes (no route, no state mutation). private ServiceResult HandleAlarmCommand( ISystemContext context, ConditionState condition, string operation, LocalizedText? comment, DateTime? unshelveAt) { @@ -1534,7 +1675,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Non-blocking by contract (host wires a fire-and-forget mediator.Tell); safe to call under Lock. AlarmCommandRouter?.Invoke(cmd); - // Good ⇒ the SDK applies the node-state change + auto-fires its own condition event. + // Good ⇒ the SDK applies the node-state change; WireMethodTransitionReporting then reports the one + // condition event (the SDK reports none of its own — Gitea #587). return ServiceResult.Good; } @@ -1552,7 +1694,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// The condition the Acknowledge targets; its NodeId identifier is the /// folder-scoped condition node id the driver-bound router resolves back to a driver ref. /// The acknowledge comment text, or null when none was supplied. - /// ServiceResult.Good when allowed (the SDK then applies state + auto-fires its event); + /// ServiceResult.Good when allowed (the SDK then applies state; the method path reports the event); /// BadUserAccessDenied when the gate vetoes (no route, no state mutation). private ServiceResult HandleNativeAlarmAck(ISystemContext context, ConditionState condition, LocalizedText? comment) { @@ -1571,7 +1713,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 Comment: comment?.Text, OperatorUser: AlarmOperatorName(identity!))); - // Good ⇒ the SDK applies the node-state change + auto-fires its own condition event. + // Good ⇒ the SDK applies the node-state change; WireMethodTransitionReporting then reports the one + // condition event (the SDK reports none of its own — Gitea #587). return ServiceResult.Good; } @@ -2054,7 +2197,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { try { - Server.ReportEvent(SystemContext, auditEvent); + ReportToServerObject(auditEvent); } catch (Exception ex) { @@ -2417,7 +2560,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { try { - Server.ReportEvent(SystemContext, BuildNodeShapeChangedEvent(variable)); + ReportToServerObject(BuildNodeShapeChangedEvent(variable)); } catch (Exception ex) { @@ -2461,7 +2604,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Lock across it risks a lock-order inversion (mirrors ReportNodeShapeChangedEvent). try { - Server.ReportEvent(SystemContext, e); + ReportToServerObject(e); } catch (Exception ex) { @@ -2647,7 +2790,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { try { - Server.ReportEvent(SystemContext, e); + ReportToServerObject(e); } catch (Exception ex) { @@ -3484,9 +3627,13 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// SubscribeToEvents, which also calls SetAreEventsMonitored(true, includeChildren) /// on the notifier — a counter that propagates to every condition beneath or wired to it. Not /// attaching the items lost that bookkeeping, and a condition reads it: ConditionState - /// reports its own method-driven state changes (and their audit events) only while - /// EventsMonitored() is true. A server whose only subscribers are Server-object items — the - /// common machine-consumer shape — would otherwise report none of them. + /// reports its method audit events only while EventsMonitored() is true. A server whose only + /// subscribers are Server-object items — the common machine-consumer shape — would otherwise report + /// none of them. (The method-driven state-change events no longer depend on this: the SDK + /// reports none, and reports them unconditionally — + /// Gitea #587. That also takes the SDK counter's known imprecision — a folder subscriber's unit never + /// reaches a condition materialised under the folder later, yet its unsubscribe still lowers it — off + /// the state-change path; it can now cost at most an audit event while auditing is on.) /// /// /// Held per condition, exactly one unit each, rather than per root notifier per item as the @@ -3507,6 +3654,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 : new HashSet( _alarmConditions.Values, System.Collections.Generic.ReferenceEqualityComparer.Instance); + AllEventsMonitoringConditionVisits += desired.Count + _allEventsMonitoredConditions.Count; + foreach (var condition in _allEventsMonitoredConditions.Where(c => !desired.Contains(c)).ToList()) { condition.SetAreEventsMonitored(SystemContext, false, includeChildren: true); @@ -3521,6 +3670,32 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } } + /// + /// The incremental form of for one newly materialised + /// condition: raises its unit when a Server-object item exists, touching no other condition. Call under + /// Lock. + /// + /// The condition just added to _alarmConditions. + /// + /// Equivalent to the full reconcile on the materialise path, because materialising adds one condition + /// and removes none (a same-key kind-swap gives the dropped instance's unit back itself). Kept separate + /// so a deploy of N alarms costs N set operations rather than N²/2 (review of Gitea #586). + /// + private void RaiseAllEventsMonitoring(AlarmConditionState condition) + { + AllEventsMonitoringConditionVisits++; + if (!_allEventsItems.IsEmpty && _allEventsMonitoredConditions.Add(condition)) + condition.SetAreEventsMonitored(SystemContext, true, includeChildren: true); + } + + /// + /// How many conditions the Server-object monitored bookkeeping has visited since start — every + /// condition a full walks, plus one per incremental + /// . Test observability: bounds the bookkeeping cost of a deploy. + /// + /// Mutated only under Lock. + internal long AllEventsMonitoringConditionVisits { get; private set; } + /// /// /// @@ -3533,7 +3708,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// /// /// Only condition events bubble through this manager's notifiers; the audit and model-change - /// events it raises go to Server.ReportEvent directly and are untouched by this. + /// events it raises never reach this, and go through instead + /// (Gitea #588). /// /// protected override void OnReportEvent(ISystemContext context, NodeState node, IFilterTarget e) @@ -3608,6 +3784,130 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 || endpoint?.TransportProfileUri == Profiles.HttpsBinaryTransport; } + /// + /// Reports a server-wide event this manager raises — a GeneralModelChangeEvent or an audit event + /// — to the Server object's (i=2253) subscribers, through the data-plane ACL when it is enforced + /// (Gitea #588). + /// + /// The built event. + /// + /// + /// These events are not condition events, so they never pass a notifier of ours: they went straight + /// to Server.ReportEvent, where the core node manager's Server object queues them to every + /// all-events item with no ACL decision. A model-change event names the RawPath / UNS path of the + /// node it is about, so a session holding no grant learned the names of nodes it could not browse; + /// an audit write event would carry the written value to it too. + /// + /// + /// With enforcement on, this manager delivers them itself, item by item, to the all-events items it + /// already holds for #586 — the same audience Server.ReportEvent reached, since only Server-object + /// items receive what is reported at the Server object. Each item receives only what its session may + /// see (); everything else the SDK's Server object did — its audit + /// guard, its role-permission check — is kept. With enforcement off the event goes to + /// Server.ReportEvent exactly as before. + /// + /// + /// Called outside Lock, like the Server.ReportEvent call it replaces; the decision + /// takes no node-manager lock. + /// + /// + private void ReportToServerObject(BaseEventState e) + { + if (!AclEnforced) + { + Server.ReportEvent(SystemContext, e); + return; + } + + // ServerInternalData.ReportEvent's own guard: no audit event leaves the server while it is not auditing. + if (e is AuditEventState && !Server.Auditing) return; + + foreach (var item in _allEventsItems.Keys) + { + if (ServerObjectEventFor(item, e) is not { } visible) continue; + if (ServiceResult.IsBad(ValidateEventRolePermissions(item, visible))) continue; + item.QueueEvent(visible); + } + } + + /// + /// What of a server-wide event 's session may receive with enforcement on: the + /// event itself, a copy narrowed to what it may see, or nothing (Gitea #588). + /// + /// A Server-object event item. + /// The event. + /// The event to queue, or to withhold it. + /// + /// + /// + /// A model-change event keeps only the changes whose Affected node the session may + /// Browse () — knowing a node exists is exactly what + /// Browse grants, ancestors of a granted node included. + /// + /// + /// An audit event needs the SDK's own delivery guard () + /// and Read on its source node: an AuditWriteUpdateEvent carries the node's + /// prior value and the attempted one, which is what Read protects. + /// + /// Anything else is withheld: this manager reports no other server-wide event type, so an + /// unrecognised one fails closed rather than slipping through ungated. + /// + /// + private BaseEventState? ServerObjectEventFor(IEventMonitoredItem item, BaseEventState e) + { + var identity = item.EffectiveIdentity as RoleCarryingUserIdentity; + return e switch + { + GeneralModelChangeEventState modelChange => FilterModelChangeFor(identity, modelChange), + AuditEventState audit => IsAuditDeliverableTo(item) + && audit.SourceNode?.Value is { IsNullNodeId: false } source + && Permits(identity, OpcUaOperation.Read, source) + ? audit + : null, + _ => null, + }; + } + + /// + /// A model-change event narrowed to the changes whose affected node may + /// browse: the event itself when every change is visible, when none is, otherwise + /// a copy carrying only the visible ones (Gitea #588). + /// + /// The session's identity. + /// The model-change event. + /// The event to deliver, or to withhold it. + /// + /// Every builder in this manager emits a single change, so in practice the event is delivered whole or + /// not at all; the copy keeps a future multi-change event from leaking the hidden names, or from hiding + /// the visible ones. The copy keeps the original's EventId, time and source, with a neutral + /// Message (the builders' messages name the affected node). A change with no + /// Affected node names nothing and is kept. + /// + internal GeneralModelChangeEventState? FilterModelChangeFor( + RoleCarryingUserIdentity? identity, GeneralModelChangeEventState e) + { + var changes = e.Changes?.Value; + if (changes is not { Length: > 0 }) return e; + + var visible = changes + .Where(c => c.Affected is null || c.Affected.IsNullNodeId || Permits(identity, OpcUaOperation.Browse, c.Affected)) + .ToArray(); + if (visible.Length == changes.Length) return e; + if (visible.Length == 0) return null; + + // The original Message names the affected node(s), so the copy carries a neutral one. + var copy = new GeneralModelChangeEventState(null); + copy.Initialize( + SystemContext, source: null, (EventSeverity)e.Severity.Value, new LocalizedText($"{visible.Length} node(s) changed")); + copy.EventId.Value = e.EventId.Value; + copy.Time.Value = e.Time.Value; + copy.ReceiveTime.Value = e.ReceiveTime.Value; + copy.SetChildValue(SystemContext, BrowseNames.SourceNode, e.SourceNode?.Value ?? ObjectIds.Server, false); + copy.SetChildValue(SystemContext, BrowseNames.SourceName, e.SourceName?.Value ?? "Server", false); + copy.SetChildValue(SystemContext, BrowseNames.Changes, visible, false); + return copy; + } + /// /// The nodes whose grant admits a condition to a session: the condition itself, every root-notifier /// folder its events bubble up through, and every equipment folder wired as its extra notifier. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs index 5c47420b..0104c75d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs @@ -650,11 +650,12 @@ public sealed class AclEventSubscriptionEnforcementTests /// including a condition rebuilt by a redeploy while the item was live. /// /// - /// ConditionState.EventsMonitored() gates whether the SDK reports a condition's method-driven - /// state changes at all. The SDK base raised it by attaching the item to every root notifier; #586 + /// ConditionState.EventsMonitored() gates whether the SDK reports a condition's method audit + /// events. The SDK base raised it by attaching the item to every root notifier; #586 /// attaches to nothing, so the first cut left it false on a server whose only subscribers are - /// Server-object items — the machine-consumer default. Asserted server-side because the consequence on - /// the wire depends on a separate, still-open reporting gap. + /// Server-object items — the machine-consumer default. Asserted server-side: method-driven state-change + /// events no longer depend on it (Gitea #587, ConditionMethodEventTests), and audit events only + /// leave the server while it is auditing. /// [Fact] public async Task A_server_object_item_alone_marks_conditions_monitored_until_it_is_deleted() diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclServerObjectModelChangeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclServerObjectModelChangeTests.cs new file mode 100644 index 00000000..27d22a48 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclServerObjectModelChangeTests.cs @@ -0,0 +1,201 @@ +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using static ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests.ServerEventRig; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// Gitea #588 over the wire — with Security:Acl:Enabled on, the server-wide events this node manager +/// raises (GeneralModelChangeEvents, AuditWriteUpdateEvents) reach a Server-object +/// (i=2253) item only when its session may see the node the event names. +/// +/// +/// +/// They used to go straight to Server.ReportEvent and the core node manager's Server object, which +/// queues to every all-events item with no ACL decision — so a session with no grant learned, from a +/// model-change event, the RawPath of every node added, deleted or retyped under it. +/// +/// +/// The withheld session is granted only, so the absence of the raw +/// event on its item is paired with a positive control on the same item: the sentinel event about +/// is reported after the withheld one, through the same path, and must +/// arrive. +/// +/// +public sealed class AclServerObjectModelChangeTests +{ + private const string RawVariable = "Plant/Modbus/dev1/Speed"; + + private const NodePermissions See = NodePermissions.Browse | NodePermissions.Read | NodePermissions.Subscribe; + + /// + /// A NodeAdded / NodeDeleted / DataTypeChanged model-change event naming a raw node reaches the + /// cluster-granted session and not the session granted only an equipment folder — which still receives + /// the one naming its own folder. + /// + [Theory] + [InlineData("NodeAdded")] + [InlineData("NodeDeleted")] + [InlineData("DataTypeChanged")] + public async Task A_model_change_naming_a_node_the_session_cannot_browse_is_withheld(string verb) + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync($"AclModelChange{verb}", encrypted: false, ct); + try + { + rig.Enforce(true, [Key()], ClusterGrant(KeyPrincipal, "acl-key", See), EquipmentGrant(EqA, See)); + rig.EnsureEquipment(); + rig.Sink.EnsureFolder(RawDeviceFolder, parentNodeId: null, displayName: "dev1", realm: AddressSpaceRealm.Raw); + rig.Sink.EnsureVariable(RawVariable, RawDeviceFolder, "Speed", "Int32", writable: false, AddressSpaceRealm.Raw); + + var (granted, grantedEvents, grantedItem, grantedSubscription) = await ServerObjectAsync(rig, anonymous: false, ct); + var (narrow, narrowEvents, narrowItem, narrowSubscription) = await ServerObjectAsync(rig, anonymous: true, ct); + + switch (verb) + { + case "NodeAdded": rig.Sink.RaiseNodesAddedModelChange(RawDeviceFolder, AddressSpaceRealm.Raw); break; + case "NodeDeleted": rig.Sink.RemoveVariableNode(RawVariable, AddressSpaceRealm.Raw).ShouldBeTrue(); break; + default: + rig.Sink.UpdateTagAttributes(RawVariable, false, null, "Double", false, null, AddressSpaceRealm.Raw).ShouldBeTrue(); + break; + } + + rig.Sink.RaiseNodesAddedModelChange(EqA, AddressSpaceRealm.Uns); + + await RecordedEvents.WaitUntilAsync(() => + grantedEvents.Of(grantedItem).Count(e => NamesEqA(e)) == 1 + && narrowEvents.Of(narrowItem).Any(NamesEqA)); + + var rawNames = verb == "NodeAdded" ? RawDeviceFolder : RawVariable; + grantedEvents.Of(grantedItem).Count(e => Names(e, rawNames)).ShouldBe(1, "the granted control receives the raw change"); + narrowEvents.Of(narrowItem).Where(e => Names(e, rawNames)).ShouldBeEmpty( + "a session that cannot browse the node must not learn its name from a model-change event"); + narrowEvents.Of(narrowItem).Count.ShouldBe(1, "only the change to its own equipment folder"); + + await narrowSubscription.DeleteAsync(true, ct); + await grantedSubscription.DeleteAsync(true, ct); + narrow.Dispose(); + granted.Dispose(); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// With enforcement off nothing changes: a session holding no grant at all still receives the raw + /// model-change event through the Server object, exactly once. + /// + /// Without this the test above also passes on a build that withholds every model change from everyone. + [Fact] + public async Task With_acl_enforcement_off_every_server_object_item_receives_the_model_change() + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync("AclModelChangeOff", encrypted: false, ct); + try + { + rig.Enforce(false, [Key()], ClusterGrant(KeyPrincipal, "acl-key", See)); + rig.Sink.EnsureFolder(RawDeviceFolder, parentNodeId: null, displayName: "dev1", realm: AddressSpaceRealm.Raw); + + var (session, events, item, subscription) = await ServerObjectAsync(rig, anonymous: true, ct); + + rig.Sink.RaiseNodesAddedModelChange(RawDeviceFolder, AddressSpaceRealm.Raw); + await RecordedEvents.WaitUntilAsync(() => events.Of(item).Any(e => Names(e, RawDeviceFolder))); + await Task.Delay(TimeSpan.FromMilliseconds(300), ct); + events.Of(item).Count(e => Names(e, RawDeviceFolder)).ShouldBe(1); + + await subscription.DeleteAsync(true, ct); + session.Dispose(); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// With auditing on over an encrypted channel, a rejected device write's AuditWriteUpdateEvent — + /// which carries the written value and the node's prior one — reaches a session granted Read on + /// the node and not one granted only an equipment folder. + /// + /// + /// Auditing is off by default (ServerConfiguration.AuditingEnabled, which this repo never sets), + /// and audit events only ever leave over SignAndEncrypt — so both sessions are encrypted here, or the + /// withheld one would be denied by the channel rather than by the ACL and the test would prove nothing. + /// + [Fact] + public async Task An_audit_write_event_reaches_only_sessions_that_may_read_the_node() + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync("AclAuditWrite", encrypted: true, ct); + try + { + rig.Enforce( + true, [Key()], + ClusterGrant(KeyPrincipal, "acl-key", See | NodePermissions.WriteOperate), EquipmentGrant(EqA, See)); + rig.SetAuditing(true); + rig.EnsureEquipment(); + rig.Sink.EnsureFolder(RawDeviceFolder, parentNodeId: null, displayName: "dev1", realm: AddressSpaceRealm.Raw); + rig.Sink.EnsureVariable(RawVariable, RawDeviceFolder, "Speed", "Int32", writable: true, AddressSpaceRealm.Raw); + rig.Nm.NodeWriteGateway = new RejectingGateway(); + + var (granted, grantedEvents, grantedItem, grantedSubscription) = + await ServerObjectAsync(rig, anonymous: false, ct, encrypted: true); + var (narrow, narrowEvents, narrowItem, narrowSubscription) = + await ServerObjectAsync(rig, anonymous: true, ct, encrypted: true); + + var variable = new NodeId(RawVariable, (ushort)granted.NamespaceUris.GetIndex(V3NodeIds.RawNamespaceUri)); + var write = await granted.WriteAsync( + null, + [new WriteValue { NodeId = variable, AttributeId = Attributes.Value, Value = new DataValue(new Variant(42)) }], + ct); + StatusCode.IsGood(write.Results[0]).ShouldBeTrue(); + + await RecordedEvents.WaitUntilAsync(() => + grantedEvents.HasType(grantedItem, ObjectTypeIds.AuditWriteUpdateEventType)); + + // Reported after the audit event through the same delivery, so it queues behind it on the item. + rig.Sink.RaiseNodesAddedModelChange(EqA, AddressSpaceRealm.Uns); + await RecordedEvents.WaitUntilAsync(() => narrowEvents.Of(narrowItem).Any(NamesEqA)); + + narrowEvents.HasType(narrowItem, ObjectTypeIds.AuditWriteUpdateEventType).ShouldBeFalse( + "a session that may not read the node must not receive its written values through an audit event"); + + await narrowSubscription.DeleteAsync(true, ct); + await grantedSubscription.DeleteAsync(true, ct); + narrow.Dispose(); + granted.Dispose(); + } + finally { await rig.DisposeAsync(); } + } + + // ───────────────────────────── helpers ───────────────────────────── + + private static async Task<(ISession Session, RecordedEvents Events, MonitoredItem Item, Subscription Subscription)> + ServerObjectAsync(ServerEventRig rig, bool anonymous, CancellationToken ct, bool encrypted = false) + { + var session = await rig.ConnectAsync(anonymous, ct, encrypted); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var item = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + item.Status.Error.ShouldBeNull(); + return (session, events, item, subscription); + } + + private static bool NamesEqA(VariantCollection e) => Names(e, EqA); + + /// Whether a model-change event's Changes name the node whose identifier is . + private static bool Names(VariantCollection e, string id) => + e[RecordedEvents.ChangesField].Value is ExtensionObject[] changes + && changes.Any(c => c.Body is ModelChangeStructureDataType { Affected: { } affected } + && affected.Identifier as string == id); + + /// Every device write fails, so the node manager reverts and audits it. + private sealed class RejectingGateway : IOpcUaNodeWriteGateway + { + /// + public Task WriteAsync(string nodeId, object? value, AddressSpaceRealm realm, CancellationToken ct) => + Task.FromResult(new NodeWriteOutcome(false, "rejected by the test device")); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionMethodEventTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionMethodEventTests.cs new file mode 100644 index 00000000..4b4ba023 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionMethodEventTests.cs @@ -0,0 +1,306 @@ +using Opc.Ua; +using Opc.Ua.Client; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using static ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests.ServerEventRig; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// Gitea #587 over the wire — every accepted Part 9 method call (Acknowledge, Confirm, AddComment, +/// OneShotShelve, TimedShelve, Unshelve, Enable, Disable) on a native or scripted condition reaches a +/// folder event item and a Server-object (i=2253) item as exactly one condition event +/// carrying the new state. +/// +/// +/// +/// Before #587 a client Acknowledge produced no condition event at all: the SDK's own report is gated +/// on AutoReportStateChanges (never set here), and the engine's re-projection found the node +/// already acknowledged and suppressed its own. The code comments claiming "the SDK auto-fires its own +/// event" were wrong — and a fix that only enabled the SDK's report would still double-report a shelve +/// (the SDK reports before any shelving state is applied, then the re-projection reports the change). +/// +/// +/// Each case runs the engine's (or, for a native alarm, the driver's) answer to the routed command as +/// a re-projection from a pool thread, so "exactly one" covers the duplicate the re-projection could +/// add, not only the method path. The count is read after a later sentinel transition arrives on +/// the same item, so a late duplicate would already be in the queue ahead of it. +/// +/// +public sealed class ConditionMethodEventTests +{ + private const NodePermissions All = + NodePermissions.Browse | NodePermissions.Read | NodePermissions.Subscribe | NodePermissions.AlarmRead + | NodePermissions.AlarmAcknowledge | NodePermissions.AlarmConfirm | NodePermissions.AlarmShelve + | NodePermissions.MethodCall; + + /// + /// One accepted method call ⇒ one condition event, to the folder item and to the Server-object item, + /// with the new state — while a session holding no grant receives none of it through the Server object. + /// + [Theory] + [InlineData("Acknowledge", false, true)] + [InlineData("Confirm", false, true)] + [InlineData("AddComment", false, true)] + [InlineData("OneShotShelve", false, true)] + [InlineData("TimedShelve", false, true)] + [InlineData("Unshelve", false, true)] + [InlineData("Disable", false, true)] + [InlineData("Enable", false, true)] + [InlineData("Acknowledge", true, true)] + [InlineData("Confirm", true, true)] + [InlineData("AddComment", true, true)] + [InlineData("OneShotShelve", true, true)] + [InlineData("TimedShelve", true, true)] + [InlineData("Unshelve", true, true)] + // Flag off: the server-wide AlarmAck role admits the call; delivery is ungated. + [InlineData("Acknowledge", false, false)] + [InlineData("Acknowledge", true, false)] + [InlineData("OneShotShelve", true, false)] + public async Task An_accepted_method_call_reports_exactly_one_condition_event(string method, bool native, bool aclOn) + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync($"CondMethod{method}{native}{aclOn}", encrypted: false, ct); + try + { + rig.Enforce(aclOn, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All)); + rig.MaterialiseNative(); + rig.MaterialiseScripted(); + + var current = PreState(method, native); + WireSimulatedOwners(rig, native, () => current, s => current = s); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var folderItem = RecordedEvents.AddEventItem(subscription, UnsNodeId(session, EqA)); + var serverItem = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + folderItem.Status.Error.ShouldBeNull(); + + using var ungranted = await rig.ConnectAsync(anonymous: true, ct); + var ungrantedEvents = new RecordedEvents(); + var ungrantedSubscription = await RecordedEvents.SubscribeAsync(ungranted, ungrantedEvents, ct); + var ungrantedItem = RecordedEvents.AddEventItem(ungrantedSubscription, ObjectIds.Server); + await ungrantedSubscription.ApplyChangesAsync(ct); + + var condition = ConditionNodeId(session, native); + + // Every transition the test drives also moves the simulated owner's state, so its re-projections + // carry the node's current message. + void ProjectAs(string message) + { + current = current with { Message = message }; + rig.Project(native, current); + } + + // A native condition's Confirmed and Shelving are reached only through the methods. + ProjectAs("setup"); + await RecordedEvents.WaitUntilAsync(() => events.Has(folderItem, "setup")); + if (native && method == "Confirm") + await CallAsync(session, condition, "Acknowledge", events.LatestEventId(folderItem, "setup"), ct); + if (native && method == "Unshelve") + await CallAsync(session, condition, "OneShotShelve", events.LatestEventId(folderItem, "setup"), ct); + await rig.DrainProjectionsAsync(); + + ProjectAs("base"); + await RecordedEvents.WaitUntilAsync(() => events.Has(folderItem, "base") && events.Has(serverItem, "base")); + + await CallAsync(session, condition, method, events.LatestEventId(folderItem, "base"), ct); + await rig.DrainProjectionsAsync(); + + ProjectAs("sentinel"); + await RecordedEvents.WaitUntilAsync(() => + events.Has(folderItem, "sentinel") && events.Has(serverItem, "sentinel") + && (aclOn || ungrantedEvents.Has(ungrantedItem, "sentinel"))); + + foreach (var item in new[] { folderItem, serverItem }) + { + var baseEvents = events.ConditionEvents(item, "base"); + baseEvents.Count.ShouldBe(2, $"the base transition plus exactly one {method} event ({item.StartNodeId})"); + AssertNewState(method, baseEvents[1]); + } + + if (aclOn) + { + ungrantedEvents.Of(ungrantedItem).ShouldBeEmpty( + "a session with no grant receives no condition event through the Server object"); + } + else + { + ungrantedEvents.ConditionEvents(ungrantedItem, "base").Count.ShouldBe(2, "flag off: ungated, still once"); + } + + await ungrantedSubscription.DeleteAsync(true, ct); + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// A refused call reports nothing: Disable on a native condition is BadNotSupported, so no + /// transition happened and no condition event may claim one. + /// + [Fact] + public async Task A_refused_method_call_reports_no_condition_event() + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync("CondMethodRefused", encrypted: false, ct); + try + { + rig.Enforce(true, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All)); + rig.MaterialiseNative(); + var current = PreState("Disable", native: true); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var folderItem = RecordedEvents.AddEventItem(subscription, UnsNodeId(session, EqA)); + var serverItem = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Project(native: true, current = current with { Message = "base" }); + await RecordedEvents.WaitUntilAsync(() => events.Has(serverItem, "base")); + + var refused = await Should.ThrowAsync( + CallAsync(session, ConditionNodeId(session, native: true), "Disable", null, ct)); + refused.StatusCode.ShouldBe(StatusCodes.BadNotSupported); + + rig.Project(native: true, current with { Message = "sentinel" }); + await RecordedEvents.WaitUntilAsync(() => events.Has(folderItem, "sentinel") && events.Has(serverItem, "sentinel")); + events.ConditionEvents(folderItem, "base").Count.ShouldBe(1); + events.ConditionEvents(serverItem, "base").Count.ShouldBe(1); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// Materialising N conditions while a Server-object item is live costs N monitored-bookkeeping visits, + /// not N²/2 — and every one of them still ends up marked monitored (#586 review, performance note). + /// + [Fact] + public async Task Materialising_conditions_under_a_live_server_object_item_visits_each_once() + { + const int n = 40; + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync("CondMonitoredCost", encrypted: false, ct); + try + { + rig.EnsureEquipment(); + using var session = await rig.ConnectAsync(anonymous: true, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + var before = rig.Nm.AllEventsMonitoringConditionVisits; + for (var i = 0; i < n; i++) + rig.Sink.MaterialiseAlarmCondition($"bulk-{i}", EqA, $"bulk {i}", "AlarmCondition", 500, AddressSpaceRealm.Uns); + + (rig.Nm.AllEventsMonitoringConditionVisits - before).ShouldBe(n); + for (var i = 0; i < n; i++) + rig.Nm.ConditionEventsMonitored($"bulk-{i}", AddressSpaceRealm.Uns).ShouldBeTrue(); + + await subscription.DeleteAsync(true, ct); + for (var i = 0; i < n; i++) + rig.Nm.ConditionEventsMonitored($"bulk-{i}", AddressSpaceRealm.Uns).ShouldBeFalse( + "the last item's delete still gives every unit back through the full reconcile"); + } + finally { await rig.DisposeAsync(); } + } + + // ───────────────────────────── helpers ───────────────────────────── + + /// The condition state a case starts from — one in which its method is accepted. + private static AlarmConditionSnapshot PreState(string method, bool native) + { + var s = new AlarmConditionSnapshot( + Active: true, Acknowledged: false, Confirmed: native, Enabled: true, + Shelving: AlarmShelvingKind.Unshelved, Severity: 700, Message: "pre"); + if (native) return s; // the driver projection's constants: Confirmed, Unshelved + + return method switch + { + "Confirm" => s with { Acknowledged = true, Confirmed = false }, + "Unshelve" => s with { Shelving = AlarmShelvingKind.OneShot }, + "Enable" => s with { Enabled = false }, + _ => s, + }; + } + + /// + /// Stands in for the scripted engine and the native driver: each routed command comes back as a + /// re-projection of the state it produces, from a pool thread. + /// + private static void WireSimulatedOwners( + ServerEventRig rig, bool native, Func current, Action set) + { + rig.Nm.NativeAlarmAckRouter = _ => + { + // NativeAlarmProjector's answer to the driver's ack echo: Acked, with its Confirmed/Unshelved constants. + var next = current() with { Acknowledged = true, Confirmed = true, Shelving = AlarmShelvingKind.Unshelved }; + set(next); + rig.ProjectLater(native: true, next); + }; + rig.Nm.AlarmCommandRouter = cmd => + { + if (native || cmd.AlarmId != ScriptedAlarmId) return; // the engine owns only scripted alarms + var s = current(); + var next = cmd.Operation switch + { + "Acknowledge" => s with { Acknowledged = true }, + "Confirm" => s with { Confirmed = true }, + "OneShotShelve" => s with { Shelving = AlarmShelvingKind.OneShot }, + "TimedShelve" => s with { Shelving = AlarmShelvingKind.Timed }, + "Unshelve" => s with { Shelving = AlarmShelvingKind.Unshelved }, + "Enable" => s with { Enabled = true }, + "Disable" => s with { Enabled = false }, + _ => s, // AddComment: no field of the snapshot changes + }; + set(next); + rig.ProjectLater(native: false, next); + }; + } + + private static void AssertNewState(string method, VariantCollection e) + { + switch (method) + { + case "Acknowledge": e[RecordedEvents.AckedField].Value.ShouldBe(true); break; + case "Confirm": e[RecordedEvents.ConfirmedField].Value.ShouldBe(true); break; + case "AddComment": (e[RecordedEvents.CommentField].Value as LocalizedText)?.Text.ShouldBe("note"); break; + case "OneShotShelve": e[RecordedEvents.ShelvingField].Value.ShouldBe(ObjectIds.ShelvedStateMachineType_OneShotShelved); break; + case "TimedShelve": e[RecordedEvents.ShelvingField].Value.ShouldBe(ObjectIds.ShelvedStateMachineType_TimedShelved); break; + case "Unshelve": e[RecordedEvents.ShelvingField].Value.ShouldBe(ObjectIds.ShelvedStateMachineType_Unshelved); break; + case "Disable": e[RecordedEvents.EnabledField].Value.ShouldBe(false); break; + case "Enable": e[RecordedEvents.EnabledField].Value.ShouldBe(true); break; + default: throw new ArgumentOutOfRangeException(nameof(method), method, null); + } + } + + /// + /// Calls one Part 9 method on the condition by its type's method id (the SDK resolves it against the + /// condition's method declarations — the shelve methods included); throws on a bad result. + /// + private static async Task CallAsync(ISession session, NodeId condition, string method, byte[]? eventId, CancellationToken ct) + { + var (methodId, args) = method switch + { + "Acknowledge" => (MethodIds.AcknowledgeableConditionType_Acknowledge, new object[] { eventId!, new LocalizedText("ack") }), + "Confirm" => (MethodIds.AcknowledgeableConditionType_Confirm, [eventId!, new LocalizedText("confirm")]), + "AddComment" => (MethodIds.ConditionType_AddComment, [eventId!, new LocalizedText("note")]), + "Enable" => (MethodIds.ConditionType_Enable, []), + "Disable" => (MethodIds.ConditionType_Disable, []), + "OneShotShelve" => (MethodIds.AlarmConditionType_ShelvingState_OneShotShelve, []), + "TimedShelve" => (MethodIds.AlarmConditionType_ShelvingState_TimedShelve, [60_000.0]), + "Unshelve" => (MethodIds.AlarmConditionType_ShelvingState_Unshelve, []), + _ => throw new ArgumentOutOfRangeException(nameof(method), method, null), + }; + await session.CallAsync(condition, methodId, ct, args); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs new file mode 100644 index 00000000..389f2b34 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs @@ -0,0 +1,373 @@ +using System.Net; +using System.Net.Sockets; +using System.Reflection; +using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Opc.Ua.Client; +using Opc.Ua.Configuration; +using Shouldly; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.Host.OpcUa; +using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; + +/// +/// A booted server for the Server-object event tests (Gitea #587 / #588): one native condition on a raw +/// device wired to two equipment notifiers, one scripted condition under , an API-key +/// principal and anonymous, optionally an encrypted endpoint and server auditing. +/// +internal sealed class ServerEventRig : IAsyncDisposable +{ + public const string Cluster = "SITE-A"; + public const string RawDeviceFolder = "Plant/Modbus/dev1"; + public const string RawAlarmPath = "Plant/Modbus/dev1/temp_hi"; + public const string ScriptedAlarmId = "scripted-eqa-hi"; + public const string EqA = "EqA"; + public const string EqB = "EqB"; + public const string KeyPrincipal = "event-probe"; + public const string KeyId = "k-server-events"; + + private readonly string _pkiRoot; + private readonly List _pending = []; + private OpcUaApplicationHost _host = null!; + private OtOpcUaSdkServer _server = null!; + private int _port; + + private ServerEventRig(string pkiRoot) => _pkiRoot = pkiRoot; + + public OtOpcUaNodeManager Nm => _server.NodeManager!; + + public SdkAddressSpaceSink Sink { get; private set; } = null!; + + public static async Task StartAsync(string name, bool encrypted, CancellationToken ct) + { + var rig = new ServerEventRig(Path.Combine(Path.GetTempPath(), $"otopcua-{name}-{Guid.NewGuid():N}")); + rig._port = AllocateFreePort(); + rig._server = new OtOpcUaSdkServer(); + rig._host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = $"OtOpcUa.{name}", + ApplicationUri = $"urn:OtOpcUa.{name}.{Guid.NewGuid():N}", + OpcUaPort = rig._port, + PublicHostname = "127.0.0.1", + PkiStoreRoot = rig._pkiRoot, + EnabledSecurityProfiles = encrypted + ? [OpcUaSecurityProfile.None, OpcUaSecurityProfile.Basic256Sha256SignAndEncrypt] + : [OpcUaSecurityProfile.None], + AutoAcceptUntrustedClientCertificates = true, + }, + NullLogger.Instance, + new KeyPresentingAuthenticator()); + + await rig._host.StartAsync(rig._server, ct); + rig.Sink = new SdkAddressSpaceSink(rig.Nm); + return rig; + } + + /// Wires the authorizer over the given keys and grants, enforcing or not. + public DataPlaneAuthorityState Enforce(bool enabled, ApiKey[] keys, params NodeAcl[] grants) + { + var state = new DataPlaneAuthorityState(); + state.Apply(Cluster, "rev-1", keys, grants, ScopePaths()); + Nm.DataPlaneAuthority = state; + Nm.Authorizer = new DataPlaneAuthorizer( + state, new TriePermissionEvaluator(state.Tries), enabled, NullLogger.Instance, TimeProvider.System); + return state; + } + + public static ApiKey Key() => new() + { + ApiKeyId = KeyId, + Name = KeyId, + SecretHash = "hash", + Principal = KeyPrincipal, + Enabled = true, + CreatedUtc = DateTime.UtcNow.AddDays(-1), + }; + + public static NodeAcl ClusterGrant(string principal, string id, NodePermissions permissions) => new() + { + NodeAclId = id, + ClusterId = Cluster, + Principal = principal, + ScopeKind = NodeAclScopeKind.Cluster, + PermissionFlags = permissions, + }; + + public static NodeAcl EquipmentGrant(string equipment, NodePermissions permissions) => new() + { + NodeAclId = $"acl-{equipment}", + ClusterId = Cluster, + Principal = OpcUaPrincipals.Anonymous, + ScopeKind = NodeAclScopeKind.Equipment, + ScopeId = $"eq-{equipment}", + PermissionFlags = permissions, + }; + + private static IReadOnlyDictionary> ScopePaths() => + new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.Equipment, $"eq-{EqA}")] = [new NodeAclPath([NodeAclRealms.Uns, EqA])], + [new AclScopeRef(NodeAclScopeKind.Equipment, $"eq-{EqB}")] = [new NodeAclPath([NodeAclRealms.Uns, EqB])], + }; + + /// The native condition, wired as an event source of both equipment folders. + public void MaterialiseNative() + { + Sink.EnsureFolder(RawDeviceFolder, parentNodeId: null, displayName: "dev1", realm: AddressSpaceRealm.Raw); + Sink.MaterialiseAlarmCondition( + RawAlarmPath, RawDeviceFolder, "temp_hi", "OffNormalAlarm", 700, AddressSpaceRealm.Raw, isNative: true); + EnsureEquipment(); + Sink.WireAlarmNotifiers(RawAlarmPath, AddressSpaceRealm.Raw, [EqA, EqB], AddressSpaceRealm.Uns); + } + + /// The scripted condition under . + public void MaterialiseScripted() + { + EnsureEquipment(); + Sink.MaterialiseAlarmCondition( + ScriptedAlarmId, EqA, "hi", "AlarmCondition", 500, AddressSpaceRealm.Uns, isNative: false); + } + + public void EnsureEquipment() + { + Sink.EnsureFolder(EqA, parentNodeId: null, displayName: "A", realm: AddressSpaceRealm.Uns); + Sink.EnsureFolder(EqB, parentNodeId: null, displayName: "B", realm: AddressSpaceRealm.Uns); + } + + /// Projects onto a condition, as the engine or a driver would. + public void Project(bool native, AlarmConditionSnapshot state) => + Sink.WriteAlarmCondition( + native ? RawAlarmPath : ScriptedAlarmId, + state, + DateTime.UtcNow, + native ? AddressSpaceRealm.Raw : AddressSpaceRealm.Uns); + + /// + /// Re-projects from a pool thread, the way the engine's or the driver's answer + /// to a routed method call arrives — it contends for the node manager's lock like the real one does. + /// + public void ProjectLater(bool native, AlarmConditionSnapshot state) + { + lock (_pending) _pending.Add(Task.Run(() => Project(native, state))); + } + + /// Waits for every re-projection to have run. + public async Task DrainProjectionsAsync() + { + Task[] pending; + lock (_pending) pending = [.. _pending]; + await Task.WhenAll(pending); + } + + /// The condition's NodeId in the session's namespaces. + public static NodeId ConditionNodeId(ISession session, bool native) => native + ? new NodeId(RawAlarmPath, (ushort)session.NamespaceUris.GetIndex(V3NodeIds.RawNamespaceUri)) + : new NodeId(ScriptedAlarmId, (ushort)session.NamespaceUris.GetIndex(V3NodeIds.UnsNamespaceUri)); + + public static NodeId UnsNodeId(ISession session, string id) + { + var ns = (ushort)session.NamespaceUris.GetIndex(V3NodeIds.UnsNamespaceUri); + ns.ShouldBeGreaterThan((ushort)0); + return new NodeId(id, ns); + } + + /// Switches the server's Auditing flag (no configuration seam reaches it in this repo). + public void SetAuditing(bool on) + { + var server = typeof(Opc.Ua.Server.CustomNodeManager2) + .GetProperty("Server", BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public)! + .GetValue(Nm)!; + server.GetType() + .GetProperty("Auditing", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)! + .GetSetMethod(nonPublic: true)! + .Invoke(server, [on]); + } + + public async Task ConnectAsync(bool anonymous, CancellationToken ct, bool encrypted = false) + { + var pki = TestClientSecurity.AllocatePkiRoot(); + var security = TestClientSecurity.Build(pki); + security.ApplicationCertificate.SubjectName = "CN=OtOpcUa.ServerEventClient"; + var appConfig = new ApplicationConfiguration + { + ApplicationName = "OtOpcUa.ServerEventClient", + ApplicationUri = $"urn:OtOpcUa.ServerEventClient.{Guid.NewGuid():N}", + ApplicationType = ApplicationType.Client, + SecurityConfiguration = security, + ClientConfiguration = new ClientConfiguration { DefaultSessionTimeout = 60_000 }, + }; + await appConfig.ValidateAsync(ApplicationType.Client, ct); + appConfig.CertificateValidator.CertificateValidation += (_, e) => e.Accept = true; + + if (encrypted) + { +#pragma warning disable CS0618 // the telemetry-context overload is not needed for a test client + var app = new ApplicationInstance { ApplicationName = appConfig.ApplicationName, ApplicationType = ApplicationType.Client }; +#pragma warning restore CS0618 + app.ApplicationConfiguration = appConfig; + await app.CheckApplicationInstanceCertificatesAsync(silent: true, lifeTimeInMonths: null, ct); + } + + var endpoint = await CoreClientUtils.SelectEndpointAsync( + appConfig, $"opc.tcp://127.0.0.1:{_port}/OtOpcUa", encrypted, DefaultTelemetry.Create(_ => { }), ct); + if (encrypted) endpoint!.SecurityMode.ShouldBe(MessageSecurityMode.SignAndEncrypt); + + var identity = anonymous + ? new UserIdentity(new AnonymousIdentityToken()) + : new UserIdentity(new UserNameIdentityToken + { + UserName = "apikey", + DecryptedPassword = Encoding.UTF8.GetBytes($"otua_{KeyId}_secret"), + }); + + return await new DefaultSessionFactory(DefaultTelemetry.Create(_ => { })).CreateAsync( + appConfig, + new ConfiguredEndpoint(null, endpoint, EndpointConfiguration.Create(appConfig)), + updateBeforeConnect: false, + checkDomain: false, + sessionName: "ServerEventRig", + sessionTimeout: 60_000, + identity: identity, + preferredLocales: null, + ct: ct); + } + + public async ValueTask DisposeAsync() + { + await _host.DisposeAsync(); + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch (IOException) { /* best effort */ } + } + } + + private static int AllocateFreePort() + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// + /// Admits apikey as carrying the two server-wide roles (used only + /// while enforcement is off); anonymous falls through to the anonymous principal. + /// + private sealed class KeyPresentingAuthenticator : IOpcUaUserAuthenticator + { + /// + public Task AuthenticateUserNameAsync(string username, string password, CancellationToken ct) => + Task.FromResult( + string.Equals(username, "apikey", StringComparison.Ordinal) + && password.StartsWith($"otua_{KeyId}_", StringComparison.Ordinal) + ? OpcUaUserAuthResult.AllowKey( + KeyPrincipal, [OpcUaDataPlaneRoles.AlarmAck, OpcUaDataPlaneRoles.WriteOperate], KeyId) + : OpcUaUserAuthResult.Deny("Invalid API key.")); + } +} + +/// Records every event each monitored item received, in order, with the fields the tests select. +internal sealed class RecordedEvents +{ + // The select clauses, in order — see AddEventItem. + public const int EventIdField = 0; + public const int MessageField = 1; + public const int EventTypeField = 2; + public const int AckedField = 3; + public const int ConfirmedField = 4; + public const int EnabledField = 5; + public const int ShelvingField = 6; + public const int CommentField = 7; + public const int ChangesField = 8; + public const int SourceNodeField = 9; + + private readonly object _gate = new(); + private readonly List<(uint Handle, VariantCollection Fields)> _events = []; + + public void OnEvents(Subscription subscription, EventNotificationList notification, IList stringTable) + { + lock (_gate) + { + foreach (var e in notification.Events) _events.Add((e.ClientHandle, e.EventFields)); + } + } + + /// Every event received, in arrival order. + public IReadOnlyList Of(MonitoredItem item) + { + lock (_gate) return _events.Where(e => e.Handle == item.ClientHandle).Select(e => e.Fields).ToList(); + } + + /// The condition events (they carry AckedState/Id; audit and refresh events do not). + public IReadOnlyList ConditionEvents(MonitoredItem item, string message) => + Of(item).Where(f => f[AckedField].Value is bool && Message(f) == message).ToList(); + + public bool Has(MonitoredItem item, string message) => Of(item).Any(f => Message(f) == message); + + public bool HasType(MonitoredItem item, NodeId type) => Of(item).Any(f => f[EventTypeField].Value as NodeId == type); + + public byte[] LatestEventId(MonitoredItem item, string message) => + (byte[])Of(item).Last(f => Message(f) == message && f[AckedField].Value is bool)[EventIdField].Value; + + public static string? Message(VariantCollection f) => (f[MessageField].Value as LocalizedText)?.Text; + + public static async Task SubscribeAsync(ISession session, RecordedEvents events, CancellationToken ct) + { + var subscription = new Subscription(session.DefaultSubscription) { PublishingInterval = 100 }; + subscription.FastEventCallback = events.OnEvents; + session.AddSubscription(subscription); + await subscription.CreateAsync(ct); + return subscription; + } + + public static MonitoredItem AddEventItem(Subscription subscription, NodeId source) + { + var filter = new EventFilter(); + filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.EventId); + filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.Message); + filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.EventType); + filter.SelectClauses.Add(new SimpleAttributeOperand( + ObjectTypeIds.AcknowledgeableConditionType, [BrowseNames.AckedState, BrowseNames.Id])); + filter.SelectClauses.Add(new SimpleAttributeOperand( + ObjectTypeIds.AcknowledgeableConditionType, [BrowseNames.ConfirmedState, BrowseNames.Id])); + filter.SelectClauses.Add(new SimpleAttributeOperand( + ObjectTypeIds.ConditionType, [BrowseNames.EnabledState, BrowseNames.Id])); + filter.SelectClauses.Add(new SimpleAttributeOperand( + ObjectTypeIds.AlarmConditionType, [BrowseNames.ShelvingState, BrowseNames.CurrentState, BrowseNames.Id])); + filter.AddSelectClause(ObjectTypeIds.ConditionType, BrowseNames.Comment); + filter.AddSelectClause(ObjectTypeIds.GeneralModelChangeEventType, BrowseNames.Changes); + filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.SourceNode); + + var item = new MonitoredItem(subscription.DefaultItem) + { + StartNodeId = source, + AttributeId = Attributes.EventNotifier, + Filter = filter, + QueueSize = 1000, + SamplingInterval = 0, + }; + subscription.AddItem(item); + return item; + } + + public static async Task WaitUntilAsync(Func condition) + { + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(10); + while (DateTime.UtcNow < deadline) + { + if (condition()) return; + await Task.Delay(50); + } + condition().ShouldBeTrue("condition not met within timeout"); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerModelChangeFilterTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerModelChangeFilterTests.cs new file mode 100644 index 00000000..7c0f235e --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerModelChangeFilterTests.cs @@ -0,0 +1,125 @@ +using Microsoft.Extensions.Logging.Abstractions; +using Opc.Ua; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; + +namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; + +/// +/// Gitea #588 — narrows a model-change event to the +/// changes a session may browse. Every builder in the manager emits one change, so the over-the-wire tests +/// only ever see "whole or nothing"; this pins the multi-change copy at its seam. +/// +public sealed class NodeManagerModelChangeFilterTests : IDisposable +{ + private const string Cluster = "SITE-A"; + private const string Principal = "scadabridge"; + private const string RawFolder = "Plant/Modbus/dev1"; + + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + private readonly string _pkiRoot = Path.Combine(Path.GetTempPath(), $"otopcua-mcfilter-{Guid.NewGuid():N}"); + + /// + /// A session granted one equipment folder receives a copy carrying only that folder's change — same + /// EventId, and a Message that no longer names the hidden node — while a fully granted session receives + /// the original and an ungranted one nothing. + /// + [Fact] + public async Task A_multi_change_event_is_narrowed_to_the_changes_the_session_may_browse() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var sink = new SdkAddressSpaceSink(nm); + sink.EnsureFolder("EqA", null, "A", AddressSpaceRealm.Uns); + sink.EnsureFolder(RawFolder, null, "dev1", AddressSpaceRealm.Raw); + + var e = nm.BuildNodesAddedModelChange("EqA", AddressSpaceRealm.Uns); + var eqA = e.Changes.Value[0]; + var raw = nm.BuildNodesAddedModelChange(RawFolder, AddressSpaceRealm.Raw).Changes.Value[0]; + e.Changes.Value = [eqA, raw]; + e.Message.Value = new LocalizedText($"Nodes added under {eqA.Affected} and {raw.Affected}"); + + nm.Authorizer = Authorizer( + new NodeAcl + { + NodeAclId = "acl-eqa", ClusterId = Cluster, Principal = Principal, + ScopeKind = NodeAclScopeKind.Equipment, ScopeId = "eq-EqA", + PermissionFlags = NodePermissions.Browse, + }, + new NodeAcl + { + NodeAclId = "acl-all", ClusterId = Cluster, Principal = "everything", + ScopeKind = NodeAclScopeKind.Cluster, PermissionFlags = NodePermissions.Browse, + }); + + var narrowed = nm.FilterModelChangeFor(Identity(Principal), e).ShouldNotBeNull(); + narrowed.ShouldNotBeSameAs(e); + narrowed.Changes.Value.Select(c => c.Affected).ShouldBe([eqA.Affected]); + narrowed.EventId.Value.ShouldBe(e.EventId.Value); + narrowed.Message.Value.Text.ShouldNotContain(RawFolder); + + nm.FilterModelChangeFor(Identity("everything"), e).ShouldBeSameAs(e); + nm.FilterModelChangeFor(Identity("nobody"), e).ShouldBeNull(); + } + finally { await host.DisposeAsync(); } + } + + private static DataPlaneAuthorizer Authorizer(params NodeAcl[] grants) + { + var state = new DataPlaneAuthorityState(); + state.Apply(Cluster, "rev-1", [], grants, + new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.Equipment, "eq-EqA")] = [new NodeAclPath([NodeAclRealms.Uns, "EqA"])], + }); + return new DataPlaneAuthorizer(state, new TriePermissionEvaluator(state.Tries), true, NullLogger.Instance); + } + + private static RoleCarryingUserIdentity Identity(string principal) => + new(new UserNameIdentityToken { UserName = "apikey" }, [], principal, apiKeyId: null); + + private async Task<(OpcUaApplicationHost Host, OtOpcUaSdkServer Server)> BootAsync() + { + var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.ModelChangeFilterTest", + ApplicationUri = $"urn:OtOpcUa.ModelChangeFilterTest:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + NullLogger.Instance); + + var server = new OtOpcUaSdkServer(); + await host.StartAsync(server, Ct); + return (host, server); + } + + private static int AllocateFreePort() + { + using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); + listener.Start(); + var port = ((System.Net.IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + return port; + } + + /// Cleans up the PKI root directory. + public void Dispose() + { + if (Directory.Exists(_pkiRoot)) + { + try { Directory.Delete(_pkiRoot, recursive: true); } + catch (IOException) { /* best effort */ } + } + } +}