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 */ } + } + } +}