diff --git a/docs/security.md b/docs/security.md index 9345706c9..8928634d3 100644 --- a/docs/security.md +++ b/docs/security.md @@ -571,7 +571,11 @@ and is **never** split into a scope: `OtOpcUaNodeManager` binds each LIVE script path through the CURRENT authority generation's `EquipmentScopePaths` (decoded from the artifact by the same `AclScopePathResolver` call that places an `Equipment` grant). So a grant on the owning equipment or any ancestor (line, area, UNS namespace, cluster) authorizes its scripted alarms; a sibling equipment's grant or a -grant on one tag beneath the equipment does not. Fail-closed: an unbound condition (owner folder missing), a +grant on one tag beneath the equipment does not. The same generation also carries each scripted alarm's +deployed owner (`ScriptedAlarmOwners`, from the artifact's `ScriptedAlarms` rows), and the live binding must +**agree** with it: the publish actor installs a generation before applying its address space, and a failed apply +keeps the old one, so a moved alarm is denied under its old and new owner's grants alike until the address space +re-binds it. Fail-closed: an unbound condition (owner folder missing), a removed/replaced instance, or an owner the generation cannot place is denied for **every** grant, `Cluster` included — there is no realm-root or id-split fallback. The key, grant and generation are re-read on every call (uncached). NodeIds and `AlarmCommand.AlarmId` are unchanged. With the flag **off** none of this is consulted. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs index b3800a7b1..0dfabc31f 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs @@ -35,7 +35,8 @@ public sealed class DataPlaneAuthoritySnapshot IReadOnlyList droppedGrants, IReadOnlyDictionary keysById, IReadOnlyDictionary> grantsByPrincipal, - IReadOnlyDictionary equipmentScopePaths) + IReadOnlyDictionary equipmentScopePaths, + IReadOnlyDictionary scriptedAlarmOwners) { ClusterId = clusterId; RevisionHash = revisionHash; @@ -47,6 +48,7 @@ public sealed class DataPlaneAuthoritySnapshot _keysById = keysById; _grantsByPrincipal = grantsByPrincipal; EquipmentScopePaths = equipmentScopePaths; + ScriptedAlarmOwners = scriptedAlarmOwners; } private readonly IReadOnlyDictionary _keysById; @@ -77,6 +79,31 @@ public sealed class DataPlaneAuthoritySnapshot /// public IReadOnlyDictionary EquipmentScopePaths { get; } + /// + /// ScriptedAlarmId → owning EquipmentId as THIS generation's artifact deploys it (Gitea #797 review). + /// Keyed Ordinal — the id is the condition's NodeId identifier, and NodeIds compare ordinally. + /// + /// + /// The node manager's owner binding describes the address space as last materialised; the publish actor + /// installs a new generation BEFORE it applies that generation's address space, and an apply that throws + /// after the install keeps the old address space. A scripted alarm moved from equipment A to B would + /// otherwise stay commandable with A's grant under B's generation for that window — or until the next + /// successful deploy. The authorizer therefore requires the live binding and this map to AGREE on the + /// owner; any disagreement or absence denies until the address space catches up. + /// + public IReadOnlyDictionary ScriptedAlarmOwners { get; } + + /// The owning EquipmentId this generation deploys under. + /// The scripted alarm's id (its condition NodeId identifier). + /// The owner, when known. + /// when this generation deploys the alarm. + public bool TryGetScriptedAlarmOwner(string scriptedAlarmId, out string equipmentId) + { + equipmentId = string.Empty; + return !string.IsNullOrEmpty(scriptedAlarmId) + && ScriptedAlarmOwners.TryGetValue(scriptedAlarmId, out equipmentId!); + } + /// /// The owning equipment's path WITHIN the UNS realm (the realm segment stripped), for a scripted /// condition bound to . False when this generation knows no such @@ -183,6 +210,8 @@ public sealed class DataPlaneAuthoritySnapshot /// Grants the trie build discarded, or empty when no trie was built. /// Every equipment's UNS ACL path (see ); /// null ⇒ none, which denies every scripted-condition command under enforcement. + /// ScriptedAlarmId → EquipmentId as this generation deploys them + /// (see ); null ⇒ none. /// The built snapshot. public static DataPlaneAuthoritySnapshot Create( string? clusterId, @@ -192,7 +221,8 @@ public sealed class DataPlaneAuthoritySnapshot IReadOnlyList grants, IReadOnlyDictionary> scopePaths, IReadOnlyList? droppedGrants = null, - IReadOnlyDictionary? equipmentScopePaths = null) + IReadOnlyDictionary? equipmentScopePaths = null, + IReadOnlyDictionary? scriptedAlarmOwners = null) { ArgumentNullException.ThrowIfNull(revisionHash); ArgumentNullException.ThrowIfNull(keys); @@ -239,9 +269,19 @@ public sealed class DataPlaneAuthoritySnapshot } } + var owners = new Dictionary(StringComparer.Ordinal); + if (scriptedAlarmOwners is not null) + { + foreach (var (alarmId, equipmentId) in scriptedAlarmOwners) + { + if (string.IsNullOrEmpty(alarmId) || string.IsNullOrWhiteSpace(equipmentId)) continue; + owners[alarmId] = equipmentId; + } + } + return new DataPlaneAuthoritySnapshot( clusterId, revisionHash, generationId, keys, grants, scopePaths, droppedGrants ?? [], byId, frozen, - equipment); + equipment, owners); } } @@ -309,6 +349,8 @@ public interface IDataPlaneAuthoritySink /// Where each grant's scope sits in the address space. /// Every equipment's UNS ACL path, resolved from the same artifact — /// the owner lookup for scripted alarm conditions (Gitea #797). Null ⇒ none. + /// ScriptedAlarmId → EquipmentId as the same artifact deploys + /// them (Gitea #797 review). Null ⇒ none, which denies every scripted-condition command under enforcement. /// The installed snapshot. DataPlaneAuthoritySnapshot Apply( string? clusterId, @@ -316,7 +358,8 @@ public interface IDataPlaneAuthoritySink IReadOnlyList keys, IReadOnlyList grants, IReadOnlyDictionary> scopePaths, - IReadOnlyDictionary? equipmentScopePaths = null); + IReadOnlyDictionary? equipmentScopePaths = null, + IReadOnlyDictionary? scriptedAlarmOwners = null); } /// @@ -365,7 +408,8 @@ public sealed class DataPlaneAuthorityState : IDataPlaneAuthoritySource, IDataPl IReadOnlyList keys, IReadOnlyList grants, IReadOnlyDictionary> scopePaths, - IReadOnlyDictionary? equipmentScopePaths = null) + IReadOnlyDictionary? equipmentScopePaths = null, + IReadOnlyDictionary? scriptedAlarmOwners = null) { ArgumentNullException.ThrowIfNull(revisionHash); @@ -398,7 +442,8 @@ public sealed class DataPlaneAuthorityState : IDataPlaneAuthoritySource, IDataPl var dropped = BuildAndInstallTrie(clusterId, generation, grants, scopePaths); published = DataPlaneAuthoritySnapshot.Create( - clusterId, revisionHash, generation, keys, grants, scopePaths, dropped, equipmentScopePaths); + clusterId, revisionHash, generation, keys, grants, scopePaths, dropped, equipmentScopePaths, + scriptedAlarmOwners); Volatile.Write(ref _current, published); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs index 53110529e..1e64076d2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs @@ -306,6 +306,18 @@ public sealed class DataPlaneAuthorizer "the scripted condition has no live owner binding (unknown, removed or replaced)"); } + // The live binding describes the address space as last materialised; the generation is installed BEFORE + // its address space is applied, and a failed apply keeps the old one. Require both to name the SAME owner + // for this exact alarm id, so a moved alarm is denied — under its old AND its new owner's grant — until + // the address space catches up, rather than staying commandable with the old owner's grant. + if (!conditionNodeId.TryGetValue(out string alarmId) + || !snapshot.TryGetScriptedAlarmOwner(alarmId, out var deployedOwner) + || !string.Equals(deployedOwner, ownerEquipmentId, StringComparison.OrdinalIgnoreCase)) + { + return Deny(operation, conditionNodeId, identity.Principal, + $"the live owner binding ({ownerEquipmentId}) does not match generation {snapshot.GenerationId}'s deployed owner of this scripted alarm"); + } + if (!snapshot.TryGetEquipmentUnsSegments(ownerEquipmentId, out var segments)) { return Deny(operation, conditionNodeId, identity.Principal, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs index 68c8da45b..42cfe1373 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs @@ -72,6 +72,16 @@ public sealed record DataPlaneAuthorityProjection( public IReadOnlyDictionary EquipmentScopePaths { get; init; } = new Dictionary(StringComparer.OrdinalIgnoreCase); + /// + /// ScriptedAlarmId → EquipmentId from the artifact's ScriptedAlarms rows — the same rows the + /// applier's composition materialises scripted conditions from. Installed with the generation so the + /// authorizer can require the address space's live owner binding to agree with what THIS generation + /// deploys (Gitea #797 review: a moved alarm must not stay commandable with its old owner's grant while + /// the address space lags the generation). + /// + public IReadOnlyDictionary ScriptedAlarmOwners { get; init; } = + new Dictionary(StringComparer.Ordinal); + /// The projection of an absent or unparseable artifact: no cluster, no keys, no grants. public static readonly DataPlaneAuthorityProjection Empty = new(null, [], [], new Dictionary>()); @@ -918,9 +928,21 @@ internal sealed class ParsedDeploymentArtifact : IDisposable } var (scopePaths, equipmentScopePaths) = BuildAclScopePaths(root, grants); + // Every scripted alarm's owner, from the rows the address space is composed from. Read regardless of + // the grant count: it is a topology fact, cheap, and an absent map only ever denies. + var scriptedAlarmOwners = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "ScriptedAlarms")) + { + var alarmId = ReadString(el, "ScriptedAlarmId"); + var equipmentId = ReadString(el, "EquipmentId"); + if (string.IsNullOrEmpty(alarmId) || string.IsNullOrWhiteSpace(equipmentId)) continue; + scriptedAlarmOwners[alarmId] = equipmentId; + } + return new DataPlaneAuthorityProjection(scope.ClusterId, keys, grants, scopePaths) { EquipmentScopePaths = equipmentScopePaths, + ScriptedAlarmOwners = scriptedAlarmOwners, }; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs index 6315c9c53..196136a6f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -1426,7 +1426,7 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers var revision = Convert.ToHexStringLower(System.Security.Cryptography.SHA256.HashData(artifactBytes)); var snapshot = _dataPlaneAuthority.Apply( projection.ClusterId, revision, projection.Keys, projection.Grants, projection.ScopePaths, - projection.EquipmentScopePaths); + projection.EquipmentScopePaths, projection.ScriptedAlarmOwners); _log.Info( "OpcUaPublish: data-plane authority generation {Generation} installed (cluster={Cluster}, revision={Revision}, keys={Keys}, grants={Grants}, resolvedScopes={ResolvedScopes})", diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ScriptedAlarmOwnerScopeWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ScriptedAlarmOwnerScopeWireTests.cs index 8db2b2542..e54b3cc4b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ScriptedAlarmOwnerScopeWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ScriptedAlarmOwnerScopeWireTests.cs @@ -244,7 +244,9 @@ public sealed class ScriptedAlarmOwnerScopeWireTests oldGrant.StatusCode.ShouldBe(StatusCodes.BadUserAccessDenied); routed.Count.ShouldBe(1); - Reapply(state, "rev-fan", [ServerEventRig.Key()], OwnerGrant(Fan), Observer()); + // The old grant was refused above with the generation STILL deploying the alarm under Pump (binding ≠ + // generation); now the deploy that moved it lands, granting the new owner. + Reapply(state, "rev-fan", Deployed(alarmOwner: Fan), [ServerEventRig.Key()], OwnerGrant(Fan), Observer()); await ConditionMethodEventTests.CallAsync( session, Uns(session, Alarm), "Acknowledge", events.LatestEventId(item, "recreated"), ct); @@ -306,7 +308,16 @@ public sealed class ScriptedAlarmOwnerScopeWireTests } private static void Reapply(DataPlaneAuthorityState state, string revision, ApiKey[] keys, params NodeAcl[] grants) => - state.Apply(ServerEventRig.Cluster, revision, keys, grants, Resolver().Resolve(grants), OwnerPaths()); + Reapply(state, revision, Deployed(), keys, grants); + + private static void Reapply( + DataPlaneAuthorityState state, string revision, IReadOnlyDictionary deployedOwners, + ApiKey[] keys, params NodeAcl[] grants) => + state.Apply(ServerEventRig.Cluster, revision, keys, grants, Resolver().Resolve(grants), OwnerPaths(), deployedOwners); + + /// The generation's deployed scripted-alarm owners; moves the Pump alarm. + private static Dictionary Deployed(string alarmOwner = Pump) => + new() { [Alarm] = alarmOwner, [FanAlarm] = Fan }; /// The key's ONLY alarm authority: an Equipment grant on . private static NodeAcl OwnerGrant(string equipmentId) => new() diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/Sdk2RepeatedAlarmAuthorityWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/Sdk2RepeatedAlarmAuthorityWireTests.cs index 430eec1b1..9b3bfd7dd 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/Sdk2RepeatedAlarmAuthorityWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/Sdk2RepeatedAlarmAuthorityWireTests.cs @@ -95,7 +95,7 @@ public sealed class Sdk2RepeatedAlarmAuthorityWireTests if (change == "grant") grants = []; if (change == "key") { var disabled = Key(); disabled.Enabled = false; keys = [disabled]; } if (change == "expiry") clock.Advance(TimeSpan.FromMinutes(20)); - else state.Apply(ServerEventRig.Cluster, "revoked", keys, grants, scopes, EquipmentOwnerPaths()); + else state.Apply(ServerEventRig.Cluster, "revoked", keys, grants, scopes, EquipmentOwnerPaths(), ScriptedAlarmOwners()); var error = await Should.ThrowAsync(() => session.CallAsync(condition.NodeId, methodId, ct, Variant.From(currentId), Variant.From(new LocalizedText("must not be recorded")))); @@ -107,7 +107,7 @@ public sealed class Sdk2RepeatedAlarmAuthorityWireTests confirmNode.Value.ShouldBe(beforeConfirm); // A subsequent authorized same-item sentinel drains any queued method event before the assertion. clock.Reset(); - state.Apply(ServerEventRig.Cluster, "restored", [key], [grant], scopes, EquipmentOwnerPaths()); + state.Apply(ServerEventRig.Cluster, "restored", [key], [grant], scopes, EquipmentOwnerPaths(), ScriptedAlarmOwners()); rig.Project(native, new AlarmConditionSnapshot(true, true, true, true, AlarmShelvingKind.Unshelved, 500, "sentinel")); await RecordedEvents.WaitUntilAsync(() => events.Has(item, "sentinel")); events.Of(item).Count.ShouldBe(beforeEvents + 1, "the denied retry emits nothing before the sentinel"); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs index f9e74ed3b..93405a1b0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ServerEventRig.cs @@ -126,7 +126,7 @@ internal sealed class ServerEventRig : IAsyncDisposable IReadOnlyDictionary equipmentOwnerPaths, params NodeAcl[] grants) { var state = new DataPlaneAuthorityState(); - state.Apply(Cluster, "rev-1", keys, grants, scopePaths, equipmentOwnerPaths); + state.Apply(Cluster, "rev-1", keys, grants, scopePaths, equipmentOwnerPaths, ScriptedAlarmOwners()); Nm.DataPlaneAuthority = state; Nm.Authorizer = new DataPlaneAuthorizer( state, new TriePermissionEvaluator(state.Tries), enabled, NullLogger.Instance, TimeProvider.System); @@ -181,6 +181,10 @@ internal sealed class ServerEventRig : IAsyncDisposable [EqB] = new NodeAclPath([NodeAclRealms.Uns, EqB]), }; + /// The rig's deployed scripted-alarm owners: its one scripted alarm belongs to . + public static IReadOnlyDictionary ScriptedAlarmOwners() => + new Dictionary { [ScriptedAlarmId] = EqA }; + /// The native condition, wired as an event source of both equipment folders. public void MaterialiseNative() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerScriptedAlarmOwnerScopeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerScriptedAlarmOwnerScopeTests.cs index f8ad12dac..14fd89e02 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerScriptedAlarmOwnerScopeTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerScriptedAlarmOwnerScopeTests.cs @@ -234,7 +234,7 @@ public sealed class NodeManagerScriptedAlarmOwnerScopeTests : IDisposable // The generation no longer places the owner (equipment deleted from the artifact while the // node is still served): the grant's own scope stays resolvable, the owner does not. state.Apply(Cluster, "rev-2", [key], [grant], Resolver().Resolve([grant]), - new Dictionary()); + new Dictionary(), Deployed()); break; } @@ -284,12 +284,12 @@ public sealed class NodeManagerScriptedAlarmOwnerScopeTests : IDisposable Denied(recreated.OnAcknowledge!(ctx, recreated, EventId(), new LocalizedText("old grant"))); routed.Count.ShouldBe(1); - Reapply(state, "rev-fan", [Key()], EquipmentGrant(Fan, AlarmOps)); + Reapply(state, "rev-fan", Deployed(alarmOwner: Fan), [Key()], EquipmentGrant(Fan, AlarmOps)); recreated.OnAcknowledge!(ctx, recreated, EventId(), new LocalizedText("new grant")).ShouldBe(ServiceResult.Good); routed.Count.ShouldBe(2); // The replaced instance stays dead under every grant, the broadest included. - Reapply(state, "rev-cluster", [Key()], ClusterGrant(AlarmOps)); + Reapply(state, "rev-cluster", Deployed(alarmOwner: Fan), [Key()], ClusterGrant(AlarmOps)); Denied(original.OnAcknowledge!(ctx, original, EventId(), new LocalizedText("stale"))); Denied(original.OnAddComment!(ctx, original, EventId(), new LocalizedText("stale"))); routed.Count.ShouldBe(2); @@ -433,6 +433,130 @@ public sealed class NodeManagerScriptedAlarmOwnerScopeTests : IDisposable finally { await host.DisposeAsync(); } } + /// + /// #797 review, window (a): the publish actor installs a generation BEFORE applying its address space. A + /// generation that moves the alarm from Pump to Fan, while the address space still binds it to Pump, must + /// deny BOTH owners' grants (and a Cluster grant) — never leave it commandable with the old owner's grant — + /// until the rebuild re-materialises it under Fan, after which only Fan's grant allows. + /// + [Fact] + public async Task A_generation_that_moves_the_alarm_denies_both_owners_until_the_address_space_rebinds() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var routed = Route(nm); + Topology(nm); + var (state, _) = Wire(nm, new FakeClock(Start), [Key()], EquipmentGrant(Pump, AlarmOps)); + var ctx = Context(server, KeyIdentity()); + var condition = nm.TryGetAlarmCondition(Alarm)!; + condition.OnAcknowledge!(ctx, condition, EventId(), new LocalizedText("before move")).ShouldBe(ServiceResult.Good); + + // The moving generation lands; the address space has not been applied yet (binding still Pump). + var moved = Deployed(alarmOwner: Fan); + Reapply(state, "rev-move-pump", moved, [Key()], EquipmentGrant(Pump, AlarmOps)); + nm.ScriptedAlarmOwnerOf(Alarm).ShouldBe(Pump); + Denied(condition.OnAcknowledge!(ctx, condition, EventId(), new LocalizedText("old owner grant"))); + Reapply(state, "rev-move-fan", moved, [Key()], EquipmentGrant(Fan, AlarmOps)); + Denied(condition.OnAcknowledge!(ctx, condition, EventId(), new LocalizedText("new owner grant"))); + Reapply(state, "rev-move-cluster", moved, [Key()], ClusterGrant(AlarmOps)); + Denied(condition.OnAcknowledge!(ctx, condition, EventId(), new LocalizedText("cluster grant"))); + routed.Count.ShouldBe(1); + + // The apply catches up: a ChangedAlarms rebuild re-materialises the alarm under Fan. + RebuildUnder(nm, alarmOwner: Fan); + var rebound = nm.TryGetAlarmCondition(Alarm)!; + nm.ScriptedAlarmOwnerOf(Alarm).ShouldBe(Fan); + Reapply(state, "rev-applied-fan", moved, [Key()], EquipmentGrant(Fan, AlarmOps)); + rebound.OnAcknowledge!(ctx, rebound, EventId(), new LocalizedText("new owner")).ShouldBe(ServiceResult.Good); + Reapply(state, "rev-applied-pump", moved, [Key()], EquipmentGrant(Pump, AlarmOps)); + Denied(rebound.OnAcknowledge!(ctx, rebound, EventId(), new LocalizedText("old owner after rebind"))); + routed.Select(c => c.AlarmId).ShouldBe([Alarm, Alarm]); + } + finally { await host.DisposeAsync(); } + } + + /// + /// #797 review, window (b): the apply THROWS after the moving generation was installed, so the old address + /// space (and its Pump binding) stays in force indefinitely. The alarm stays denied under every grant for as + /// long as that lasts — the clock moving and further generations that still move it change nothing — and a + /// later successful rebuild under Fan restores exactly Fan's authority. + /// + /// + /// The failed apply is represented by its end state — a new generation installed over the previous address + /// space with nothing re-materialised — which is what OpcUaPublishActor's outer catch leaves. + /// + [Fact] + public async Task A_failed_apply_after_the_moving_generation_keeps_the_alarm_denied_until_a_successful_rebuild() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var routed = Route(nm); + Topology(nm); + var clock = new FakeClock(Start); + var (state, _) = Wire(nm, clock, [Key()], EquipmentGrant(Pump, AlarmOps), EquipmentGrant(Fan, AlarmOps)); + var ctx = Context(server, KeyIdentity()); + var stale = nm.TryGetAlarmCondition(Alarm)!; + + Reapply(state, "rev-move", Deployed(alarmOwner: Fan), [Key()], + EquipmentGrant(Pump, AlarmOps), EquipmentGrant(Fan, AlarmOps)); + // (apply threw here — nothing below touches the address space) + Denied(stale.OnAcknowledge!(ctx, stale, EventId(), new LocalizedText("1"))); + clock.Advance(TimeSpan.FromHours(6)); + Denied(stale.OnAddComment!(ctx, stale, EventId(), new LocalizedText("2"))); + Reapply(state, "rev-move-again", Deployed(alarmOwner: Fan), [Key()], ClusterGrant(AlarmOps)); + Denied(stale.OnAcknowledge!(ctx, stale, EventId(), new LocalizedText("3"))); + nm.ScriptedAlarmOwnerOf(Alarm).ShouldBe(Pump, "the stale binding is still there; the generation check is what denies"); + routed.ShouldBeEmpty(); + + RebuildUnder(nm, alarmOwner: Fan); + var rebound = nm.TryGetAlarmCondition(Alarm)!; + Reapply(state, "rev-fan", Deployed(alarmOwner: Fan), [Key()], EquipmentGrant(Fan, AlarmOps)); + rebound.OnAcknowledge!(ctx, rebound, EventId(), new LocalizedText("after rebuild")).ShouldBe(ServiceResult.Good); + Denied(stale.OnAcknowledge!(ctx, stale, EventId(), new LocalizedText("stale instance"))); + routed.Select(c => c.AlarmId).ShouldBe([Alarm]); + } + finally { await host.DisposeAsync(); } + } + + /// An alarm id the generation does not deploy at all is denied even though the address space binds it. + [Fact] + public async Task A_bound_alarm_the_generation_does_not_deploy_is_denied() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var routed = Route(nm); + Topology(nm); + var (state, _) = Wire(nm, new FakeClock(Start), [Key()], ClusterGrant(AlarmOps)); + var ctx = Context(server, KeyIdentity()); + var fan = nm.TryGetAlarmCondition(FanAlarm)!; + fan.OnAcknowledge!(ctx, fan, EventId(), new LocalizedText("deployed")).ShouldBe(ServiceResult.Good); + + Reapply(state, "rev-without-fan-alarm", new Dictionary { [Alarm] = Pump }, [Key()], ClusterGrant(AlarmOps)); + Denied(fan.OnAcknowledge!(ctx, fan, EventId(), new LocalizedText("deleted by this generation"))); + routed.Count.ShouldBe(1); + } + finally { await host.DisposeAsync(); } + } + + /// What a successful ChangedAlarms rebuild does to the node manager: wipe, re-ensure, re-materialise. + private static void RebuildUnder(OtOpcUaNodeManager nm, string alarmOwner) + { + nm.RebuildAddressSpace(); + nm.EnsureFolder(AreaId, parentNodeId: null, displayName: "Area1", realm: AddressSpaceRealm.Uns); + nm.EnsureFolder(LineId, parentNodeId: AreaId, displayName: "Line1", realm: AddressSpaceRealm.Uns); + nm.EnsureFolder(Pump, parentNodeId: LineId, displayName: "Pump", realm: AddressSpaceRealm.Uns); + nm.EnsureFolder(Fan, parentNodeId: LineId, displayName: "Fan", realm: AddressSpaceRealm.Uns); + nm.MaterialiseAlarmCondition(Alarm, alarmOwner, "hi", "AlarmCondition", 500, AddressSpaceRealm.Uns); + nm.MaterialiseAlarmCondition(FanAlarm, Fan, "fan hi", "AlarmCondition", 500, AddressSpaceRealm.Uns); + nm.CompleteAddressSpaceApply(); + } + // ───────────────────────────── what must not change ───────────────────────────── /// @@ -471,7 +595,7 @@ public sealed class NodeManagerScriptedAlarmOwnerScopeTests : IDisposable { [new AclScopeRef(NodeAclScopeKind.RawDevice, "dev1")] = [new NodeAclPath([NodeAclRealms.Raw, "Plant", "Modbus", "dev1"])], }; - state.Apply(Cluster, "rev-raw", [Key()], [rawDevice], scopes, OwnerPaths()); + state.Apply(Cluster, "rev-raw", [Key()], [rawDevice], scopes, OwnerPaths(), Deployed()); condition.OnAcknowledge!(ctx, condition, EventId(), new LocalizedText("raw grant")).ShouldBe(ServiceResult.Good); condition.OnAddComment!(ctx, condition, EventId(), new LocalizedText("raw note")).ShouldBe(ServiceResult.Good); @@ -578,7 +702,7 @@ public sealed class NodeManagerScriptedAlarmOwnerScopeTests : IDisposable IReadOnlyDictionary areas, NodeAcl[] grants) { var state = new DataPlaneAuthorityState(); - state.Apply(Cluster, "rev-1", keys, grants, Resolver(areas).Resolve(grants), OwnerPaths()); + state.Apply(Cluster, "rev-1", keys, grants, Resolver(areas).Resolve(grants), OwnerPaths(), Deployed()); var authorizer = new DataPlaneAuthorizer( state, new TriePermissionEvaluator(state.Tries), enabled, NullLogger.Instance, clock); nm.DataPlaneAuthority = state; @@ -588,7 +712,23 @@ public sealed class NodeManagerScriptedAlarmOwnerScopeTests : IDisposable /// A new deployment of the same topology carrying and . private static void Reapply(DataPlaneAuthorityState state, string revision, ApiKey[] keys, params NodeAcl[] grants) => - state.Apply(Cluster, revision, keys, grants, Resolver().Resolve(grants), OwnerPaths()); + Reapply(state, revision, Deployed(), keys, grants); + + private static void Reapply( + DataPlaneAuthorityState state, string revision, IReadOnlyDictionary deployedOwners, + ApiKey[] keys, params NodeAcl[] grants) => + state.Apply(Cluster, revision, keys, grants, Resolver().Resolve(grants), OwnerPaths(), deployedOwners); + + /// + /// The generation's deployed scripted-alarm owners (the artifact's ScriptedAlarms rows); + /// moves the Pump alarm. + /// + private static Dictionary Deployed(string alarmOwner = Pump) => new() + { + [Alarm] = alarmOwner, + [FanAlarm] = Fan, + ["scripted-orphan"] = "EQ-not-materialised", + }; private static NodeAcl EquipmentGrant(string equipmentId, NodePermissions flags) => new() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs index b834dad78..b92cfeada 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs @@ -225,4 +225,33 @@ public sealed class DeploymentArtifactScopePathTests projection.EquipmentScopePaths.ShouldBeEmpty(); } + + /// + /// #797 review — the generation also carries each scripted alarm's deployed owner, decoded from the same + /// ScriptedAlarms rows the address space is composed from, keyed ordinally by the alarm id. + /// + [Fact] + public void The_projection_carries_each_scripted_alarms_deployed_owner() + { + var projection = DeploymentArtifact.ParseDataPlaneAuthority( + Encoding.UTF8.GetBytes(""" + { + "ScriptedAlarms": [ + {"ScriptedAlarmId":"scripted-eqa-hi","EquipmentId":"EQ-mixer","Name":"hi"}, + {"ScriptedAlarmId":"scripted-no-owner","Name":"x"} + ], + "NodeAcls":[{"NodeAclId":"a","ClusterId":"c1","Principal":"p","ScopeKind":0,"PermissionFlags":1}] + } + """), + NodeId); + + projection.ScriptedAlarmOwners.ShouldBe(new Dictionary { ["scripted-eqa-hi"] = "EQ-mixer" }); + projection.ScriptedAlarmOwners.ContainsKey("SCRIPTED-EQA-HI").ShouldBeFalse("alarm ids are NodeId identifiers: ordinal"); + + var snapshot = new DataPlaneAuthorityState().Apply("c1", "rev-1", projection.Keys, projection.Grants, + projection.ScopePaths, projection.EquipmentScopePaths, projection.ScriptedAlarmOwners); + snapshot.TryGetScriptedAlarmOwner("scripted-eqa-hi", out var owner).ShouldBeTrue(); + owner.ShouldBe("EQ-mixer"); + snapshot.TryGetScriptedAlarmOwner("scripted-no-owner", out _).ShouldBeFalse(); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorDeferredWriteTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorDeferredWriteTests.cs index 3c56ac2b3..a975116c0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorDeferredWriteTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorDeferredWriteTests.cs @@ -665,10 +665,12 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase string? clusterId, string revision, IReadOnlyList keys, IReadOnlyList grants, IReadOnlyDictionary> scopePaths, - IReadOnlyDictionary? equipmentScopePaths = null) + IReadOnlyDictionary? equipmentScopePaths = null, + IReadOnlyDictionary? scriptedAlarmOwners = null) { Interlocked.Increment(ref _applies); - return new DataPlaneAuthorityState().Apply(clusterId, revision, keys, grants, scopePaths, equipmentScopePaths); + return new DataPlaneAuthorityState().Apply( + clusterId, revision, keys, grants, scopePaths, equipmentScopePaths, scriptedAlarmOwners); } }