From fe92ff4c343371d24a8af8fb5ebb550b8b0f42b2 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 00:05:12 -0400 Subject: [PATCH] fix(acl): keep the SDK's audit guard and EventsMonitored bookkeeping on Server-object delivery (#586) Review of 75e559f6: - OnConditionReportEvent now mirrors MonitoredNode2's AuditEventState guard (only while Server.Auditing, over SignAndEncrypt / https-uabinary). Conditions report their Part 9 method audit events through themselves, so the first cut queued AuditConditionAcknowledge (i=8944) etc. to every i=2253 item with auditing off -- the removed folder->Server forward used to drop them. - ReconcileAllEventsMonitoring keeps one AreEventsMonitored unit on every materialised condition while any i=2253 item exists (called from SubscribeToAllEvents, materialise, removal, rebuild), so ConditionState.EventsMonitored() is true on a server whose only subscribers are Server-object items. A protected SubscribeToEvents override no-ops all-events items, which stops the SDK's AddRootNotifier (unsubscribe: true bug) taking that unit back from conditions rebuilt under a live item. Flag-off over-the-wire coverage: Acknowledge/AddComment with and without a folder co-subscriber (no audit event, transitions once each), EventsMonitored on/after-rebuild/off, rebuild survival with one copy per event, native + scripted conditions once each, refresh once per condition with RefreshStart/End; plus an ACL-on native-vs-scripted notifier-folder decision. Falsified: removing the audit guard reds both audit cases; removing the reconcile reds the monitored test. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- CLAUDE.md | 11 +- docs/security.md | 10 + .../OtOpcUaNodeManager.cs | 137 +++++++- .../AclEventSubscriptionEnforcementTests.cs | 308 ++++++++++++++++++ 4 files changed, 460 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 36bafab2..5d271da7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -934,8 +934,15 @@ Two things to know before relying on any of this: ungated path, and it is the only one that served an item created before the alarm was deployed (the SDK's `AddRootNotifier` subscribes existing all-events items with `unsubscribe: true`). Revalidation is **implicit** — every event re-asks the authorizer, so a deploy or expiry lands on the next event with no - pass. `ConditionRefresh` on an `i=2253` item is filtered the same way. ⚠️ Residual: audit/model-change - events still go straight to the core Server object, ungated. + pass. `ConditionRefresh` on an `i=2253` item is filtered the same way. The dispatcher **must** keep + 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 + 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. - **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 diff --git a/docs/security.md b/docs/security.md index 70e76ca8..ae72af8f 100644 --- a/docs/security.md +++ b/docs/security.md @@ -560,6 +560,16 @@ Behaviours worth knowing before enabling it: check on (once per condition, not once per notifier it is wired to). Without that, a refresh would replay every condition's current state to any session. - Inert with the flag off: every Server-object item receives every condition, as before. + - The replacement delivery keeps the two behaviours of the SDK path it replaced. **Audit events** a + 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 + 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. Residual, not closed by #586: **audit and model-change events** (`AuditWriteUpdateEvent`, `GeneralModelChangeEvent`, …) are still reported straight to the Server object and reach every diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 4c27d2a5..423c5629 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -350,6 +350,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 _liveItems.Clear(); _eventItems.Clear(); _allEventsItems.Clear(); + lock (Lock) _allEventsMonitoredConditions.Clear(); } base.Dispose(disposing); @@ -1362,6 +1363,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 AddPredefinedNode(SystemContext, alarm); _alarmConditions[conditionKey] = alarm; + ReconcileAllEventsMonitoring(); // 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); @@ -2743,6 +2745,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // Phase C: drop the event-history source registrations alongside the notifier folders // they map; re-materialised alarms re-register them (with the HistoryRead bit) on re-promotion. _eventNotifierSources.Clear(); + ReconcileAllEventsMonitoring(); } } @@ -2832,6 +2835,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 condition.Parent?.RemoveChild(condition); PredefinedNodes?.Remove(condition.NodeId); _nativeAlarmNodeIds.Remove(key); + ReconcileAllEventsMonitoring(); e = BuildNodesRemovedModelChange(alarmNodeId, realm); } ReportModelChangeOutsideLock(e, alarmNodeId); @@ -2881,6 +2885,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } _nativeAlarmNodeIds.Remove(id); } + ReconcileAllEventsMonitoring(); // v3 Batch 4 WP4: a native alarm condition lives at its raw tag (Raw realm) and SURVIVES this // UNS-scoped subtree removal, but it may hold an in-scope equipment folder as an extra @@ -3428,11 +3433,94 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 public override ServiceResult SubscribeToAllEvents( OperationContext context, uint subscriptionId, IEventMonitoredItem monitoredItem, bool unsubscribe) { - if (unsubscribe) _allEventsItems.TryRemove(monitoredItem, out _); - else _allEventsItems[monitoredItem] = 0; + lock (Lock) + { + if (unsubscribe) _allEventsItems.TryRemove(monitoredItem, out _); + else _allEventsItems[monitoredItem] = 0; + ReconcileAllEventsMonitoring(); + } + return ServiceResult.Good; } + /// + /// + /// + /// A Server-object (all-events) item is never attached to a notifier of ours (Gitea #586), so a + /// subscribe or unsubscribe of one against a notifier is a no-op here. The only SDK caller that + /// still reaches this with such an item is AddRootNotifier, which — an SDK bug — "subscribes" + /// every existing all-events item to a new root notifier with unsubscribe: true. Passed to the + /// base, that lowers on the new notifier, its children and + /// its forward notifiers — taking back, from every condition materialised under a live item, the + /// unit just gave it (and, before #586, a folder + /// subscriber's unit too). Caught by the rebuild leg of the monitored-bookkeeping test. + /// + /// + /// Direct notifier items, including the #562 revalidation pass's attach and detach, go to the base + /// unchanged. + /// + /// + protected override ServiceResult SubscribeToEvents( + ServerSystemContext context, NodeState source, IEventMonitoredItem monitoredItem, bool unsubscribe) => + monitoredItem.MonitoringAllEvents + ? ServiceResult.Good + : base.SubscribeToEvents(context, source, monitoredItem, unsubscribe); + + /// + /// The conditions currently carrying this manager's one "a Server-object item is listening" unit of + /// (Gitea #586 review). + /// + /// Mutated only by , under Lock. + private readonly HashSet _allEventsMonitoredConditions = + new(System.Collections.Generic.ReferenceEqualityComparer.Instance); + + /// + /// Keeps raised on every materialised condition while at + /// least one Server-object item exists, and lowered once none does. Call under Lock. + /// + /// + /// + /// The SDK base attached an all-events item to every root notifier through its own + /// 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. + /// + /// + /// Held per condition, exactly one unit each, rather than per root notifier per item as the + /// base did. The counter is shared with the folder subscribers' own units, so it must balance + /// exactly; a notifier-level unit would also reach a condition materialised under an already- + /// raised folder never, or be taken back from one it never reached. This reconciles the desired + /// set (every condition, while any item exists) against what it has raised, so materialise, + /// remove, rebuild and subscribe all converge through one place. + /// + /// + private void ReconcileAllEventsMonitoring() + { + // The protected SubscribeToEvents override is what stops the SDK taking these units back. + lock (Lock) + { + var desired = _allEventsItems.IsEmpty + ? new HashSet(System.Collections.Generic.ReferenceEqualityComparer.Instance) + : new HashSet( + _alarmConditions.Values, System.Collections.Generic.ReferenceEqualityComparer.Instance); + + foreach (var condition in _allEventsMonitoredConditions.Where(c => !desired.Contains(c)).ToList()) + { + condition.SetAreEventsMonitored(SystemContext, false, includeChildren: true); + _allEventsMonitoredConditions.Remove(condition); + } + + foreach (var condition in desired) + { + if (_allEventsMonitoredConditions.Add(condition)) + condition.SetAreEventsMonitored(SystemContext, true, includeChildren: true); + } + } + } + /// /// /// @@ -3484,6 +3572,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 foreach (var item in _allEventsItems.Keys) { + if (e is AuditEventState && !IsAuditDeliverableTo(item)) continue; if (scopes is not null && !IsConditionVisibleTo(item, scopes)) continue; if (ServiceResult.IsBad(ValidateEventRolePermissions(item, e))) continue; if (reportingSession is not null @@ -3497,6 +3586,28 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } } + /// + /// The SDK's audit-event guard from MonitoredNode2.OnReportEvent, reproduced exactly: an audit + /// event reaches an item only while the server is auditing, and only over a SignAndEncrypt or + /// https-binary endpoint. + /// + /// The Server-object event item. + /// when the audit event may be queued to the item. + /// + /// A condition reports its Part 9 method audit events (AuditConditionAcknowledgeEventType, + /// Comment, Enable, Shelving…) through its own ReportEvent, so they pass through + /// . Before #586 the Server-object copy took the root-notifier + /// forward, where ServerInternalData.ReportEvent dropped every audit event unless auditing was + /// on; without this guard they reached every Server-object item regardless (caught in review, flag off). + /// + private bool IsAuditDeliverableTo(IEventMonitoredItem item) + { + if (!Server.Auditing) return false; + var endpoint = item.Session?.EndpointDescription; + return endpoint?.SecurityMode == MessageSecurityMode.SignAndEncrypt + || endpoint?.TransportProfileUri == Profiles.HttpsBinaryTransport; + } + /// /// 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. @@ -3571,8 +3682,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// client connecting after a transition gets the current alarm picture — and the SDK base replays /// every condition under every root notifier to it after only its own role-permission check. With /// enforcement on, those items are refreshed here instead, through the same - /// decision live delivery uses (Gitea #586), and each condition - /// is replayed once rather than once per notifier it is wired to. + /// decision live delivery uses (Gitea #586); an event that names no + /// condition is withheld. With it off, the base runs unchanged (its per-notifier repeats of one + /// condition share an EventId, which the item's queue already collapses to one copy). /// /// /// Items on one of this manager's own notifiers keep the base behaviour: a stood-down item is off @@ -3633,6 +3745,23 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 _ => null, }; + /// + /// Whether the condition at believes it is being listened to + /// (ConditionState.EventsMonitored()) — the gate on the SDK reporting its method-driven state + /// changes. Test observability for the Gitea #586 review. + /// + /// The condition's NodeId identifier. + /// The condition's realm. + /// when the condition exists and reports itself monitored. + internal bool ConditionEventsMonitored(string conditionNodeId, AddressSpaceRealm realm) + { + lock (Lock) + { + return _alarmConditions.TryGetValue(MapKey(realm, conditionNodeId), out var condition) + && condition.EventsMonitored(); + } + } + /// /// How many event items are attached to the event fan-out of nodes this manager owns. Test /// observability for Gitea #562: an item resurrected onto a notifier after its client deleted it is 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 3989d45f..5c47420b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/AclEventSubscriptionEnforcementTests.cs @@ -42,6 +42,7 @@ public sealed class AclEventSubscriptionEnforcementTests private const string Cluster = "SITE-A"; private const string RawDeviceFolder = "Plant/Modbus/dev1"; private const string RawAlarmPath = "Plant/Modbus/dev1/temp_hi"; + private const string ScriptedAlarmId = "scripted-eqa-hi"; private const string EqA = "EqA"; private const string EqB = "EqB"; private const string KeyPrincipal = "expiry-probe"; @@ -572,8 +573,283 @@ public sealed class AclEventSubscriptionEnforcementTests finally { await rig.DisposeAsync(); } } + // ──────── Server-object delivery with the flag OFF — #586 review: no regression vs. the SDK path ──────── + + /// + /// Part 9 method calls (Acknowledge, AddComment) on a condition put no audit event in front of a + /// Server-object item while the server is not auditing — with and without a folder co-subscriber — + /// and the item still receives the condition's own transitions exactly once. + /// + /// + /// + /// A condition reports its method audit events (AuditConditionAcknowledgeEventType, + /// AuditConditionCommentEventType) through its own ReportEvent, which since #586 is + /// where Server-object delivery happens. The SDK path it replaced dropped them unless auditing was + /// on over an encrypted channel; the replacement must too. Found in review: the first cut queued + /// i=8944 to every Server-object item. + /// + /// + /// Both shapes matter. With a folder co-subscriber the condition counts as monitored through the + /// folder; with none, only the Server-object item makes it so — the shape the second review + /// finding was about, where nothing at all was reported before. + /// + /// + /// The positive control is the next transition: the method calls queue synchronously, so an audit + /// event would be ahead of t2 in the same item's queue. + /// + /// + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Method_audit_events_do_not_reach_a_server_object_item_while_not_auditing(bool folderCoSubscriber) + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync($"AclServerAudit{folderCoSubscriber}", ct); + try + { + rig.Materialise(); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var events = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, events, ct); + var serverItem = AddEventItem(subscription, ObjectIds.Server); + var folderItem = folderCoSubscriber ? AddEventItem(subscription, rig.UnsNodeId(session, EqA)) : null; + await subscription.ApplyChangesAsync(ct); + + rig.Transition("t1", active: true); + await WaitUntilAsync(() => events.Has(serverItem, "t1")); + var eventId = events.EventIdOf(serverItem, "t1"); + + var condition = rig.RawConditionNodeId(session); + await CallIgnoringStatusAsync( + session, condition, MethodIds.AcknowledgeableConditionType_Acknowledge, ct, eventId, new LocalizedText("ack")); + await CallIgnoringStatusAsync( + session, condition, MethodIds.ConditionType_AddComment, ct, eventId, new LocalizedText("note")); + + rig.Transition("t2", active: false); + await WaitUntilAsync(() => events.Has(serverItem, "t2")); + await Task.Delay(TimeSpan.FromMilliseconds(300), ct); + + events.HasType(serverItem, ObjectTypeIds.AuditConditionAcknowledgeEventType).ShouldBeFalse(); + events.HasType(serverItem, ObjectTypeIds.AuditConditionCommentEventType).ShouldBeFalse(); + events.Count(serverItem, "t1").ShouldBe(1); + events.Count(serverItem, "t2").ShouldBe(1); + if (folderItem is not null) + { + events.HasType(folderItem, ObjectTypeIds.AuditConditionAcknowledgeEventType).ShouldBeFalse(); + events.Count(folderItem, "t2").ShouldBe(1); + } + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// A Server-object item alone marks every condition as monitored, and deleting it un-marks them — + /// 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 + /// 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. + /// + [Fact] + public async Task A_server_object_item_alone_marks_conditions_monitored_until_it_is_deleted() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclServerMonitored", ct); + try + { + rig.Materialise(); + rig.NativeConditionEventsMonitored.ShouldBeFalse("baseline: nobody is listening yet"); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var events = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, events, ct); + AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.NativeConditionEventsMonitored.ShouldBeTrue(); + + rig.Rebuild(); + rig.Materialise(); + rig.NativeConditionEventsMonitored.ShouldBeTrue("a condition rebuilt under a live item is monitored too"); + + await subscription.DeleteAsync(true, ct); + rig.NativeConditionEventsMonitored.ShouldBeFalse("the last item's delete must give the unit back"); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// A Server-object item outlives a full address-space rebuild and receives exactly one copy of each + /// transition on the rebuilt condition. + /// + /// + /// Nothing is attached to the discarded folders, so there is no fan-out to repair; this pins that the + /// rebuilt condition's delivery is wired, and that the rebuild did not leave a second path behind. + /// + [Fact] + public async Task A_server_object_item_survives_a_rebuild_with_one_copy_per_event() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclServerRebuild", ct); + try + { + rig.Materialise(); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var events = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, events, ct); + var item = AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Transition("t1", active: true); + await WaitUntilAsync(() => events.Has(item, "t1")); + + rig.Rebuild(); + rig.Materialise(); + + rig.Transition("t2", active: false); + await WaitUntilAsync(() => events.Has(item, "t2")); + await Task.Delay(TimeSpan.FromMilliseconds(300), ct); + events.Count(item, "t1").ShouldBe(1); + events.Count(item, "t2").ShouldBe(1); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// With several conditions — a native one wired to two equipment folders and a scripted one under a + /// UNS equipment folder — a Server-object item receives each condition's transition exactly once. + /// + [Fact] + public async Task A_server_object_item_receives_native_and_scripted_conditions_once_each() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclServerMulti", ct); + try + { + rig.Materialise(); + rig.MaterialiseScripted(); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var events = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, events, ct); + var item = AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Transition("n1", active: true); + rig.TransitionScripted("s1", active: true); + await WaitUntilAsync(() => events.Has(item, "n1") && events.Has(item, "s1")); + await Task.Delay(TimeSpan.FromMilliseconds(300), ct); + events.Count(item, "n1").ShouldBe(1); + events.Count(item, "s1").ShouldBe(1); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// With enforcement on, the decision follows the notifier a condition reports through: an equipment + /// grant on admits the native condition wired to it and withholds the scripted + /// condition under ; a cluster-granted session receives both (the positive control). + /// + /// + /// The scripted condition's NodeId is its alarm id, not a path under its equipment, so it is admitted + /// only through its parent notifier folder — the branch of the check this pins. + /// + [Fact] + public async Task Server_object_delivery_follows_the_notifier_folders_for_native_and_scripted_conditions() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclServerScripted", ct); + try + { + rig.Enforce([UnexpiringKey()], EquipmentGrant(EqB), ClusterGrant(KeyPrincipal, "acl-key")); + rig.Materialise(); + rig.MaterialiseScripted(); + + using var control = await rig.ConnectAsync(anonymous: false, ct); + using var session = await rig.ConnectAsync(anonymous: true, ct); + + var controlEvents = new EventCollector(); + var controlSubscription = await CreateSubscriptionAsync(control, controlEvents, ct); + var controlItem = AddEventItem(controlSubscription, ObjectIds.Server); + await controlSubscription.ApplyChangesAsync(ct); + + var events = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, events, ct); + var item = AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Transition("n1", active: true); + rig.TransitionScripted("s1", active: true); + await WaitUntilAsync(() => + controlEvents.Has(controlItem, "n1") && controlEvents.Has(controlItem, "s1") && events.Has(item, "n1")); + await Task.Delay(TimeSpan.FromMilliseconds(500), ct); + events.Has(item, "s1").ShouldBeFalse("EqB's grant does not reach the scripted condition under EqA"); + + await subscription.DeleteAsync(true, ct); + await controlSubscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// With the flag off, a ConditionRefresh on a Server-object item replays each condition exactly + /// once — not once per notifier it is wired to — bracketed by RefreshStart and RefreshEnd. + /// + [Fact] + public async Task A_condition_refresh_on_the_server_object_replays_each_condition_once() + { + var ct = TestContext.Current.CancellationToken; + var rig = await Rig.StartAsync("AclServerRefreshOff", ct); + try + { + rig.Materialise(); + rig.MaterialiseScripted(); + rig.Transition("native-before", active: true); + rig.TransitionScripted("scripted-before", active: true); + + using var session = await rig.ConnectAsync(anonymous: true, ct); + var events = new EventCollector(); + var subscription = await CreateSubscriptionAsync(session, events, ct); + var item = AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + await RefreshAsync(session, subscription, ct); + + await WaitUntilAsync(() => events.HasType(item, ObjectTypeIds.RefreshEndEventType)); + await Task.Delay(TimeSpan.FromMilliseconds(300), ct); + events.HasType(item, ObjectTypeIds.RefreshStartEventType).ShouldBeTrue(); + events.Count(item, "native-before").ShouldBe(1, "the native condition is wired to three notifiers"); + events.Count(item, "scripted-before").ShouldBe(1); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + // ───────────────────────────── helpers ───────────────────────────── + /// + /// Calls a condition method, ignoring its outcome: the test asserts what the call puts on the event + /// stream, which the SDK reports whether or not the call itself is accepted. + /// + private static async Task CallIgnoringStatusAsync( + ISession session, NodeId objectId, NodeId methodId, CancellationToken ct, params object[] args) + { + try { await session.CallAsync(objectId, methodId, ct, args); } + catch (ServiceResultException) { /* the outcome is not under test */ } + } + private static ApiKey UnexpiringKey(DateTime? expiresUtc = null) => new() { ApiKeyId = KeyId, @@ -660,6 +936,7 @@ public sealed class AclEventSubscriptionEnforcementTests { private readonly object _gate = new(); private readonly List<(uint Handle, string Message)> _seen = []; + private readonly Dictionary<(uint Handle, string Message), byte[]> _eventIds = []; private readonly HashSet<(uint Handle, NodeId Type)> _types = []; public void OnEvents(Subscription subscription, EventNotificationList notification, IList stringTable) @@ -672,6 +949,7 @@ public sealed class AclEventSubscriptionEnforcementTests && e.EventFields[MessageFieldIndex].Value is LocalizedText { Text: { } text }) { _seen.Add((e.ClientHandle, text)); + if (e.EventFields[0].Value is byte[] eventId) _eventIds[(e.ClientHandle, text)] = eventId; } if (e.EventFields.Count > EventTypeFieldIndex @@ -694,6 +972,11 @@ public sealed class AclEventSubscriptionEnforcementTests { lock (_gate) return _types.Contains((item.ClientHandle, type)); } + + public byte[] EventIdOf(MonitoredItem item, string message) + { + lock (_gate) return _eventIds[(item.ClientHandle, message)]; + } } /// A booted server with ACL enforcement, one native alarm wired to two equipment notifiers. @@ -777,6 +1060,31 @@ public sealed class AclEventSubscriptionEnforcementTests _sink.WireAlarmNotifiers(RawAlarmPath, AddressSpaceRealm.Raw, [EqA, EqB], AddressSpaceRealm.Uns); } + /// A scripted condition under equipment folder (UNS realm, no extra notifiers). + public void MaterialiseScripted() => + _sink.MaterialiseAlarmCondition( + ScriptedAlarmId, EqA, "hi", "AlarmCondition", 500, AddressSpaceRealm.Uns, isNative: false); + + /// Fires one transition on the scripted condition, carrying . + public void TransitionScripted(string message, bool active) => + _sink.WriteAlarmCondition( + ScriptedAlarmId, + new AlarmConditionSnapshot( + Active: active, Acknowledged: !active, Confirmed: true, Enabled: true, + Shelving: AlarmShelvingKind.Unshelved, Severity: 500, Message: message), + DateTime.UtcNow, + AddressSpaceRealm.Uns); + + /// A full address-space rebuild, as a redeploy that is not surgical performs. + public void Rebuild() => _sink.RebuildAddressSpace(); + + /// Whether the native condition reports itself monitored (ConditionState.EventsMonitored()). + public bool NativeConditionEventsMonitored => Nm.ConditionEventsMonitored(RawAlarmPath, AddressSpaceRealm.Raw); + + /// The native condition's NodeId in the session's raw namespace. + public NodeId RawConditionNodeId(ISession session) => + new(RawAlarmPath, (ushort)session.NamespaceUris.GetIndex(V3NodeIds.RawNamespaceUri)); + /// Fires one genuine condition transition, carrying . public void Transition(string message, bool active) => _sink.WriteAlarmCondition(