dohertj2/lmxopcua
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(acl): require the live scripted-alarm owner to match the generation's deployed owner (#797)

Browse Source 
Review of 3d1b86b4: OpcUaPublishActor installs a new authority generation
before applying its address space, and an apply that throws keeps the old one.
A scripted alarm moved from equipment A to B therefore stayed commandable with
A's grant under B's generation (briefly before the rebuild, or indefinitely
after a failed apply).

The generation now carries ScriptedAlarmOwners (ScriptedAlarmId -> EquipmentId,
decoded by ParseDataPlaneAuthority from the artifact's ScriptedAlarms rows, the
same rows the composition materialises from). AuthorizeScriptedCondition denies
unless the node manager's live binding names the same owner as the generation:
a moved alarm is denied under the old owner's, the new owner's and a Cluster
grant until the address space re-binds it; an alarm the generation no longer
deploys is denied. Install/apply order unchanged.

Tests: window (a) generation moved / binding stale => both owners and Cluster
denied, then rebuild under the new owner => new owner allowed, old denied;
window (b) apply failed after install => denied across clock and further
generations, then a successful rebuild restores the new owner only; undeployed
alarm denied; artifact decode of the owner map.
This commit is contained in:
Joseph Doherty
2026-11-10 21:42:03 -05:00
parent 3d1b86b451
commit 4620e542e8
11 changed files with 290 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerScriptedAlarmOwnerScopeTests.cs
+146 -6
View File
@@ -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<string, NodeAclPath>());
new Dictionary<string, NodeAclPath>(), 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(); }
}
/// <summary>
/// #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.
/// </summary>
[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(); }
}
/// <summary>
/// #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.
/// </summary>
/// <remarks>
/// 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 <c>OpcUaPublishActor</c>'s outer catch leaves.
/// </remarks>
[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(); }
}
/// <summary>An alarm id the generation does not deploy at all is denied even though the address space binds it.</summary>
[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<string, string> { [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(); }
}
/// <summary>What a successful ChangedAlarms rebuild does to the node manager: wipe, re-ensure, re-materialise.</summary>
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 ─────────────────────────────
/// <summary>
@@ -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<string, string> 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
/// <summary>A new deployment of the same topology carrying <paramref name="keys"/> and <paramref name="grants"/>.</summary>
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<string, string> deployedOwners,
ApiKey[] keys, params NodeAcl[] grants) =>
state.Apply(Cluster, revision, keys, grants, Resolver().Resolve(grants), OwnerPaths(), deployedOwners);
/// <summary>
/// The generation's deployed scripted-alarm owners (the artifact's <c>ScriptedAlarms</c> rows);
/// <paramref name="alarmOwner"/> moves the Pump alarm.
/// </summary>
private static Dictionary<string, string> Deployed(string alarmOwner = Pump) => new()
{
[Alarm] = alarmOwner,
[FanAlarm] = Fan,
["scripted-orphan"] = "EQ-not-materialised",
};
private static NodeAcl EquipmentGrant(string equipmentId, NodePermissions flags) => new()
{