From e39a125069ff585e39661ad57733c857df236fac Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 10 Nov 2026 23:44:11 -0500 Subject: [PATCH] =?UTF-8?q?fix(acl):=20close=20the=20#808=20review=20findi?= =?UTF-8?q?ngs=20=E2=80=94=20reference=20variables,=20owner=20folder=20cha?= =?UTF-8?q?in,=20NUL=20keys,=20unknown=20ids=20(#808)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of af5c9744: 1. UNS reference variables (s=Area/Line/Equip/Eff) of a moved or renamed equipment stayed readable/writable under the OLD names' grants: they now resolve as ReferenceVariable and are authorized at their split path only while it equals their live equipment folder's placement (live chain == generation chain) + one name; otherwise denied (Read and Write). 2. TryPlaceScriptedCondition now also requires the owner folder's live chain to match the generation (AuthorizeScriptedCondition gains ownerFolderChain), so a moved equipment's alarm/children/commands follow its folder. 3. Identifiers containing NUL are denied before the decision cache, keeping client ids disjoint from the NUL-prefixed resolved-scope keys (a client could otherwise poison a real folder's cached verdict for its key shard). 4. An id no live node holds is Unknown: placed at [\0unknown] so only Cluster / Namespace-uns grants decide it (a removed opaque id can no longer split onto a same-named narrow grant; narrow grantees get no NodeDeleted for removed UNS folders/vtags/conditions — documented); 4-segment reference-shaped ids keep their split. 5. A wire test drives placements from DeploymentArtifact.ParseDataPlaneAuthority. Baseline updates: #797 owner tests (unit + wire) and Sdk2RepeatedAlarmAuthority now install folder scopes; Sdk2HistoryAuthority's UNS alias used the RAW path as its UNS id (never minted in production) and now carries EqA/HistoryAlias. Each new guard was falsified (reverting it fails its test). --- .claude/rules/data-plane-auth.md | 26 +- docs/security.md | 11 +- .../OtOpcUaNodeManager.cs | 80 ++++-- .../Security/DataPlaneAuthorizer.cs | 96 +++++++- .../Security/UnsAclSubject.cs | 48 +++- .../ScriptedAlarmOwnerScopeWireTests.cs | 19 +- .../Sdk2HistoryAuthorityWireTests.cs | 11 +- .../Sdk2RepeatedAlarmAuthorityWireTests.cs | 4 +- .../UnsFolderScopeWireTests.cs | 232 +++++++++++++++++- .../NodeManagerModelChangeFilterTests.cs | 69 ++++++ ...NodeManagerScriptedAlarmOwnerScopeTests.cs | 25 +- 11 files changed, 557 insertions(+), 64 deletions(-) diff --git a/.claude/rules/data-plane-auth.md b/.claude/rules/data-plane-auth.md index eeb34fecd..feb04ccf3 100644 --- a/.claude/rules/data-plane-auth.md +++ b/.claude/rules/data-plane-auth.md @@ -112,14 +112,24 @@ Two things to know before relying on any of this: denied under old AND new line until the rebuild). Owned nodes (vtags, conditions, children) get `owner path + DataPlaneAuthorizer.OwnedNodeSegment` — only equipment-or-ancestor grants confer them, and a Tag grant's Browse transit can't enumerate them. Conditions share `TryPlaceScriptedCondition` with the #797 command - gate. Reference variables (`s=Area/Line/Equip/Name`) keep the split. ⚠️ The folder-owned rule keys on "first id - segment == nearest slash-free live UNS folder ancestor's id", which can only narrow (an Area named exactly like - its equipment's id loses that variable's Tag-grant match). ⚠️ `ConditionVisibilityScopes` returns the condition - ALONE for a scripted condition, so a moved alarm can't reach its old owner via the Server object; direct - per-folder event items still ride the SDK fan-out of the folder it hangs under. Unplaceable ⇒ denied for every - grant incl. Cluster (a behaviour change for Cluster keys only in the stale window / corrupt topology). Tests: - `UnsFolderScopeWireTests` (opaque ids); rig tests (`ServerEventRig.FolderScopes()`, `EqA` ⇒ `[uns, EqA]`) now - install folder scopes explicitly — without them every UNS folder denies. + gate, which ALSO requires the owner folder's live chain to match (review: else the new line could command a moved + equipment's alarm while its folder is denied). Reference variables (`s=Area/Line/Equip/Name`) keep their split + path (so Tag grants match) but only while it equals their live equipment folder's placement + one name — a + moved/renamed equipment's old-name variables are denied (Read AND Write) under old and new names until + re-pointed. An id NO live node holds (removed / probe / NodeDeleted `Affected`) is `Unknown`: placed at + `[\0unknown]`, so only Cluster / Namespace-uns grants decide it and a removed opaque id can never split onto a + same-named narrow grant — except a 4-segment (reference-shaped) id, which keeps its split. ⇒ narrow grantees do + NOT receive NodeDeleted for removed UNS folders / vtags / conditions (by decision; they re-browse). Identifiers + containing NUL are denied before the cache (resolved-scope keys are NUL-prefixed — a client could otherwise + poison a real folder's cached verdict). ⚠️ The folder-owned rule keys on "first id segment == nearest slash-free + live UNS folder ancestor's id", which can only narrow (an Area named exactly like its equipment's id loses that + variable's Tag-grant match). ⚠️ `ConditionVisibilityScopes` returns the condition ALONE for a scripted condition, + so a moved alarm can't reach its old owner via the Server object; direct per-folder event items still ride the + SDK fan-out of the folder it hangs under (follow-up). A UNS node with NO live hierarchy-folder ancestor (rig-only + shape) keeps the plain split. Unplaceable ⇒ denied for every grant incl. Cluster (a behaviour change for Cluster + keys only in the stale window / corrupt topology). Tests: `UnsFolderScopeWireTests` (opaque ids, incl. one fed by + `DeploymentArtifact.ParseDataPlaneAuthority`); rig tests (`ServerEventRig.FolderScopes()`, `EqA` ⇒ `[uns, EqA]`) + install folder scopes explicitly — without them every UNS folder and scripted condition denies. - **Revocation reaches live subscriptions** (stage 6): every other operation re-asks per request, but a subscription is authorized once and then streams — and streaming is how machine consumers actually read this server. `DataPlaneAuthorityState.Changed` fires `RevalidateMonitoredItems`. ⚠️ A denied diff --git a/docs/security.md b/docs/security.md index 71f1a6b90..be8288f35 100644 --- a/docs/security.md +++ b/docs/security.md @@ -592,8 +592,15 @@ deploys** (a moved equipment is denied under its old and new line's grants until virtual tag / its sub-folder at its equipment's path plus a reserved segment; a scripted condition and its children at the owning equipment's path plus that reserved segment, through exactly the #797 owner rule. The reserved segment means only grants on the equipment or an ancestor confer them — a `Tag` grant beneath the equipment never does, and -its Browse transit does not expose the equipment's alarms or virtual tags. UNS reference variables -(`s=Area/Line/Equipment/Name`) are unchanged: their id IS their path. Server-object (`i=2253`) delivery of a +its Browse transit does not expose the equipment's alarms or virtual tags. A scripted condition additionally requires +its owner folder's live ancestry to match the generation's (the folder rule). UNS reference variables +(`s=Area/Line/Equipment/Name`) keep their id as their path — but only while it equals their live equipment folder's +placement plus one name, so a moved or renamed equipment's variables (still on the old-name id) are denied for Read +and Write under old and new names until the address space re-points them. An id that no live node holds (a removed +folder / virtual tag / condition, a probe, a NodeDeleted model change) is decided only by a `Cluster` or `uns` +`Namespace` grant — so narrow grantees do not receive NodeDeleted for removed opaque-id nodes, and a removed id can +never match an unrelated narrow grant that shares its name; a four-segment (reference-shaped) id keeps its split. +Identifiers containing NUL are refused. Server-object (`i=2253`) delivery of a **scripted** condition is now decided on the condition alone (its folders add nothing in steady state and would leak a moved alarm to its old owner). So a key holding only an `Equipment` grant can navigate to its equipment, read its alarm children, receive its scripted alarm events through the Server object and Acknowledge / Confirm / diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 85b202de7..4b13d3931 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -2304,6 +2304,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // authority. Never a fallback to splitting the opaque id. var native = false; string? owner = null; + List? ownerChain = null; lock (Lock) { var key = MapKey(condition.NodeId); @@ -2311,12 +2312,11 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 { native = true; } - else if (_scriptedAlarmOwners.TryGetValue(key, out var binding) - && ReferenceEquals(binding.Alarm, condition) - && _alarmConditions.TryGetValue(key, out var live) - && ReferenceEquals(live, condition)) + else { - owner = binding.EquipmentId; + owner = BoundScriptedOwner(key, condition); + // Gitea #808 review: the owner's folder must also sit where the generation deploys it. + ownerChain = BoundScriptedOwnerChain(key, condition); } } @@ -2324,7 +2324,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 ? _authorizer!.Authorize( identity, AlarmOperationFor(operation), condition.NodeId, RealmOf(condition.NodeId)) : _authorizer!.AuthorizeScriptedCondition( - identity, AlarmOperationFor(operation), condition.NodeId, owner); + identity, AlarmOperationFor(operation), condition.NodeId, owner, ownerChain); } // The activation-time AlarmAck role is honoured only while the key that earned it is @@ -4224,7 +4224,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// /// /// UNS reference variables — s=Area/Line/Equipment/EffectiveName, NAMES: the id IS the - /// ACL path (), so a Tag grant keeps matching it. + /// ACL path, so a Tag grant keeps matching it — but only while it equals its live equipment folder's + /// placement + one name (, #808 review). + /// Anything no live node holds (removed, never deployed, probed): + /// — decided only by a realm-wide grant. /// Hierarchy folders and what they own — Area / Line / Equipment folders under their opaque /// entity ids (s=UA-…/s=UL-…/s=EQ-…), and an equipment's virtual tags and their /// sub-folders (s=EQ-…/Name): , with the live id ancestry. @@ -4263,21 +4266,18 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 if (node is null) { // Not a folder or variable: a scripted condition (s=) or one of its instance-scoped children - // (s=/, #629). Anything else is an unknown id and keeps the split. + // (s=/, #629). Anything else is held by NO live node — removed, never deployed, or a + // probe — and is Unknown: only a realm-wide grant decides it (review of #808: splitting a removed opaque + // id let it match a narrow grant on an unrelated entity of the same name). var conditionId = slash < 0 ? identifier : identifier[..slash]; var conditionKey = MapKey(AddressSpaceRealm.Uns, conditionId); - if (!_alarmConditions.TryGetValue(conditionKey, out var condition)) return UnsAclSubject.Path; + if (!_alarmConditions.TryGetValue(conditionKey, out var condition)) return UnsAclSubject.Unknown; lock (Lock) { if (_nativeAlarmNodeIds.Contains(conditionKey)) return UnsAclSubject.Path; - var owner = _scriptedAlarmOwners.TryGetValue(conditionKey, out var binding) - && ReferenceEquals(binding.Alarm, condition) - && _alarmConditions.TryGetValue(conditionKey, out var live) - && ReferenceEquals(live, condition) - ? binding.EquipmentId - : null; - return UnsAclSubject.ScriptedCondition(conditionId, owner); + return UnsAclSubject.ScriptedCondition( + conditionId, BoundScriptedOwner(conditionKey, condition), BoundScriptedOwnerChain(conditionKey, condition)); } } @@ -4292,11 +4292,30 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 break; } - if (owning is null || !firstSegment.SequenceEqual(owning.NodeId.IdentifierAsString)) return UnsAclSubject.Path; + // No live hierarchy folder above it: a shape only test rigs mint (a UNS node hung straight under the root). + if (owning is null) return UnsAclSubject.Path; - // Its live ancestry, root-first: every live hierarchy folder up to the first non-UNS-folder (the root). + var chain = LiveHierarchyChain(owning); + + // A node whose first id segment is NOT its equipment's id is a UNS reference variable + // (s=Area/Line/Equipment/EffectiveName): authorized at its split path, but only while that path is where the + // current generation places the equipment it hangs under (review of #808 — a moved or renamed equipment's + // variables keep their OLD-name ids until re-pointed, and the old names' grants must not keep them). + return firstSegment.SequenceEqual(owning.NodeId.IdentifierAsString) + ? UnsAclSubject.Folder(chain, isFolderItself: ReferenceEquals(node, owning)) + : UnsAclSubject.ReferenceVariable(chain); + } + + /// + /// The live ancestry of , root-first and ending at it: every live hierarchy folder (a + /// slash-free UNS folder id) up to the first node that is not one (the shared root). + /// + /// A live hierarchy folder. + /// The id chain. + private List LiveHierarchyChain(NodeState folder) + { var chain = new List(3); - current = owning; + NodeState? current = folder; for (var depth = 0; current is not null && depth < MaxUnsAncestry; depth++, current = ParentOf(current)) { if (LiveUnsFolderId(current) is not { } id || id.Contains('/')) break; @@ -4304,9 +4323,30 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 } chain.Reverse(); - return UnsAclSubject.Folder(chain, isFolderItself: ReferenceEquals(node, owning)); + return chain; } + /// The EquipmentId the exact live is bound to, or null. Call under Lock. + private string? BoundScriptedOwner(RealmKey key, ConditionState condition) => + _scriptedAlarmOwners.TryGetValue(key, out var binding) + && ReferenceEquals(binding.Alarm, condition) + && _alarmConditions.TryGetValue(key, out var live) + && ReferenceEquals(live, condition) + ? binding.EquipmentId + : null; + + /// + /// The live folder-id ancestry of the equipment folder a BOUND scripted condition hangs under (the #808 folder + /// rule applied to the #797 owner), or null when unbound or no longer under that folder. Call under Lock. + /// + private List? BoundScriptedOwnerChain(RealmKey key, ConditionState condition) => + BoundScriptedOwner(key, condition) is { } owner + && ParentOf(condition) is FolderState parent + && LiveUnsFolderId(parent) is { } parentId + && string.Equals(parentId, owner, StringComparison.Ordinal) + ? LiveHierarchyChain(parent) + : null; + /// The node's parent in the instance tree, or null for a type/root node. /// The node. /// Its parent. 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 04a81049d..36ec94171 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs @@ -147,6 +147,17 @@ public sealed class DataPlaneAuthorizer /// internal const string OwnedNodeSegment = "\u0000owned"; + /// + /// The single segment, directly under the UNS realm, that an identifier no live node holds is placed at + /// (Gitea #808 review): only a realm-wide grant reaches it, and — unlike the realm root itself — it carries no + /// Browse transit, so a narrow grantee is never handed a removed opaque id (a NodeDeleted model change, a + /// probe) and a removed id can never match a narrow grant on an unrelated entity that shares its name. + /// + internal const string UnknownNodeSegment = "\u0000unknown"; + + /// The depth of a UNS reference-variable id: Area/Line/Equipment/EffectiveName. + private const int ReferenceVariableSegments = 4; + /// /// Authorizes on through the decision cache — /// the entry point for Read, Browse and subscription creation, which ask the same question many @@ -299,12 +310,15 @@ public sealed class DataPlaneAuthorizer /// The condition the method targets (for the denial log only — never parsed). /// The EquipmentId the live condition is bound to, or null when the /// node manager holds no binding for this exact instance. + /// The LIVE folder-id ancestry of the equipment folder the condition hangs under + /// (root-first, ending at ), or null when unbound (Gitea #808 review). /// null to proceed; otherwise the veto to return to the SDK. public ServiceResult? AuthorizeScriptedCondition( RoleCarryingUserIdentity? identity, OpcUaOperation operation, NodeId conditionNodeId, - string? ownerEquipmentId) + string? ownerEquipmentId, + IReadOnlyList? ownerFolderChain) { if (identity is null) return Deny(operation, conditionNodeId, principal: null, "the session carries no role-carrying identity"); @@ -325,8 +339,11 @@ public sealed class DataPlaneAuthorizer return DenyRevokedKey(operation, conditionNodeId, identity.Principal, keyId, snapshot.GenerationId, cached: false); conditionNodeId.TryGetValue(out string alarmId); - if (!TryPlaceScriptedCondition(snapshot, alarmId, ownerEquipmentId, out var segments, out var reason)) + if (!TryPlaceScriptedCondition( + snapshot, alarmId, ownerEquipmentId, ownerFolderChain, out var segments, out var reason)) + { return Deny(operation, conditionNodeId, identity.Principal, reason); + } var scope = new NodeScope { @@ -348,6 +365,7 @@ public sealed class DataPlaneAuthorizer /// The generation in force. /// The condition's ScriptedAlarmId. /// The live owner binding, or null when unbound. + /// The live id ancestry of the owner's folder, or null when unbound. /// The owning equipment's path below the UNS realm root. /// Why placement failed (the caller denies). /// when the condition is placed. @@ -355,6 +373,7 @@ public sealed class DataPlaneAuthorizer DataPlaneAuthoritySnapshot snapshot, string? alarmId, string? ownerEquipmentId, + IReadOnlyList? ownerFolderChain, out IReadOnlyList segments, out string reason) { @@ -383,21 +402,41 @@ public sealed class DataPlaneAuthorizer return false; } + // The owner's FOLDER must also sit where this generation deploys it (the folder rule, #808 review): an + // equipment moved to another line keeps its id, so the owner checks above pass while its folder — and this + // alarm under it — still hangs under the old line. Without this the new line's grant could read and command + // the alarm while the folder above it is denied. + if (ownerFolderChain is null + || !snapshot.TryGetUnsFolderSegments(ownerEquipmentId, ownerFolderChain, out var folderSegments) + || !folderSegments.SequenceEqual(segments, StringComparer.Ordinal)) + { + segments = []; + reason = $"owner equipment {ownerEquipmentId}'s folder is not placed as generation {snapshot.GenerationId} deploys it (moved, or not yet rebuilt)"; + return false; + } + reason = string.Empty; return true; } /// /// Turns a non- UNS subject into the segments it is authorized at in - /// (Gitea #808). Every gap fails — the caller denies; there is no fallback to - /// splitting the identifier, to the realm root, or to the cluster. + /// (Gitea #808). For a folder, a node a folder owns, a reference variable and a + /// scripted condition every gap fails — the caller denies, with no fallback to splitting the identifier, to + /// the realm root or to the cluster. The one deliberate exception is + /// (no live node holds the id): it is placed at directly under the realm, so + /// only a realm-wide (Cluster / Namespace uns) grant decides it — never a narrow grant that + /// happens to share a name with a removed opaque id — except a reference-variable-shaped id (exactly four + /// segments), which keeps its name-path split. /// + /// The node's s= identifier (split only for the shapes that are paths). /// The generation in force. /// The live facts the node manager reported. /// The resolved path below the UNS realm root. /// Why placement failed. /// when the subject is placed. private static bool TryPlaceUnsSubject( + string identifier, DataPlaneAuthoritySnapshot snapshot, in UnsAclSubject subject, out IReadOnlyList segments, @@ -407,6 +446,36 @@ public sealed class DataPlaneAuthorizer bool owned; switch (subject.Kind) { + case UnsAclSubjectKind.Unknown: + { + var split = identifier.Split('/', StringSplitOptions.RemoveEmptyEntries); + segments = split.Length == ReferenceVariableSegments ? split : [UnknownNodeSegment]; + reason = string.Empty; + return true; + } + + case UnsAclSubjectKind.ReferenceVariable: + { + // A reference variable's id IS its name path, so a Tag grant keeps matching it — but only while that + // path is where THIS generation puts its equipment (plus one effective-name segment). An equipment + // moved to another line, or an Area/Line/Equipment renamed, leaves the variable's id on the OLD names + // until the address space re-points it; the old names' grants must not keep reading / writing it. + var split = identifier.Split('/', StringSplitOptions.RemoveEmptyEntries); + if (subject.FolderChain is not { Count: > 0 } equipmentChain + || !snapshot.TryGetUnsFolderSegments(equipmentChain[^1], equipmentChain, out var equipment) + || split.Length != equipment.Count + 1 + || !split.Take(equipment.Count).SequenceEqual(equipment, StringComparer.Ordinal)) + { + segments = []; + reason = $"the UNS reference variable's path does not match its equipment {subject.FolderChain?.LastOrDefault() ?? "(none)"}'s placement in generation {snapshot.GenerationId} (moved, renamed, or not yet re-pointed)"; + return false; + } + + segments = split; + reason = string.Empty; + return true; + } + case UnsAclSubjectKind.Folder: if (subject.FolderChain is not { Count: > 0 } chain || !snapshot.TryGetUnsFolderSegments(chain[^1], chain, out owner)) @@ -420,7 +489,8 @@ public sealed class DataPlaneAuthorizer break; case UnsAclSubjectKind.ScriptedCondition: - if (!TryPlaceScriptedCondition(snapshot, subject.AlarmId, subject.OwnerEquipmentId, out owner, out reason)) + if (!TryPlaceScriptedCondition( + snapshot, subject.AlarmId, subject.OwnerEquipmentId, subject.FolderChain, out owner, out reason)) { segments = []; return false; @@ -444,7 +514,10 @@ public sealed class DataPlaneAuthorizer /// The decision-cache key for a node authorized at a RESOLVED scope rather than at its split identifier. It /// names the scope, not the node, because the cached grant walk depends only on the scope — and keying on the /// node would let a verdict outlive a re-bind or re-parent the resolver sees but the generation does not. The - /// leading NUL keeps it disjoint from every real s= identifier. + /// leading NUL keeps it disjoint from every s= identifier — which Evaluate ENFORCES by denying any + /// identifier that contains a NUL before the cache is consulted (review of #808: otherwise a client reading + /// the nonexistent s="\0Area1\0Line1\0Pump" would memoize a deny under the real folder's key for its + /// whole key shard). /// /// The resolved path below the UNS realm root. /// The cache key. @@ -515,6 +588,15 @@ public sealed class DataPlaneAuthorizer "the node id carries no string identifier, so it addresses no ACL scope", cached); } + // No node this server mints carries a NUL, and the resolved-scope cache keys are NUL-prefixed: refusing NUL + // here keeps the two keyspaces disjoint, so a client-chosen identifier can never collide with (and poison) + // a resolved scope's memoized verdict. Denied uncached, before the lookup. + if (identifier.Contains('\u0000')) + { + return Deny(operation, nodeId, identity.Principal, + "the node id carries a NUL, which no node in this address space does", cached); + } + var sessionKey = identity.ApiKeyId ?? identity.Principal; // Gitea #808: a UNS folder minted under an opaque entity id, a node it owns, and a scripted condition (or @@ -528,7 +610,7 @@ public sealed class DataPlaneAuthorizer var subject = resolve(identifier); if (subject.Kind != UnsAclSubjectKind.Path) { - if (!TryPlaceUnsSubject(snapshot, subject, out var placed, out var reason)) + if (!TryPlaceUnsSubject(identifier, snapshot, subject, out var placed, out var reason)) return Deny(operation, nodeId, identity.Principal, reason, cached); resolvedSegments = placed; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/UnsAclSubject.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/UnsAclSubject.cs index 20319a020..f5c6c6238 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/UnsAclSubject.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/UnsAclSubject.cs @@ -3,9 +3,27 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; /// How a UNS-namespace node is to be placed in the ACL scope tree (Gitea #808). public enum UnsAclSubjectKind { - /// The node's s= identifier IS its ACL path (a UNS reference variable, or an unknown id). + /// + /// The node's s= identifier IS its ACL path and nothing live vouches for it — a UNS node with no live + /// hierarchy-folder ancestor (only test rigs mint those). Split as before. + /// Path, + /// + /// No live node holds the identifier (removed, never deployed, or a client probing). Only a realm-wide grant + /// may decide it — except a reference-variable-shaped id (exactly four segments), which keeps its split so a + /// removed reference variable's NodeDeleted still reaches the grantees of that name path. + /// + Unknown, + + /// + /// A UNS reference variable (s=Area/Line/Equipment/EffectiveName) hanging under a live equipment folder: + /// authorized at its split path, but only while that path equals the equipment's placement in the current + /// generation plus one effective-name segment (a moved or renamed equipment's variables are denied until the + /// address space re-points them). + /// + ReferenceVariable, + /// /// A UNS Area / Line / Equipment folder minted under its opaque entity id, or a node owned by one /// (a virtual tag EQ-…/Name or its sub-folder): placed at the folder's path in the current generation. @@ -42,12 +60,18 @@ public readonly record struct UnsAclSubject /// The identifier is the ACL path — the pre-#808 behaviour. public static readonly UnsAclSubject Path = new() { Kind = UnsAclSubjectKind.Path }; + /// No live node holds the identifier. + public static readonly UnsAclSubject Unknown = new() { Kind = UnsAclSubjectKind.Unknown }; + /// Which placement rule applies. public UnsAclSubjectKind Kind { get; init; } /// - /// For : the owning folder's LIVE id ancestry, root-first, ending at - /// the owning folder itself (e.g. [UA-…, UL-…, EQ-…]). + /// The owning folder's LIVE id ancestry, root-first, ending at the owning folder itself + /// (e.g. [UA-…, UL-…, EQ-…]): for and + /// the folder at or above the node; for + /// the folder the bound condition hangs under (null when + /// unbound). /// public IReadOnlyList? FolderChain { get; init; } @@ -76,7 +100,21 @@ public readonly record struct UnsAclSubject /// A scripted condition (or one of its children) and its live owner binding. /// The condition's ScriptedAlarmId. /// The live owner binding, or null when unbound. + /// The live id ancestry of the folder the condition hangs under, or null. /// The subject. - public static UnsAclSubject ScriptedCondition(string alarmId, string? ownerEquipmentId) => - new() { Kind = UnsAclSubjectKind.ScriptedCondition, AlarmId = alarmId, OwnerEquipmentId = ownerEquipmentId }; + public static UnsAclSubject ScriptedCondition( + string alarmId, string? ownerEquipmentId, IReadOnlyList? ownerFolderChain) => + new() + { + Kind = UnsAclSubjectKind.ScriptedCondition, + AlarmId = alarmId, + OwnerEquipmentId = ownerEquipmentId, + FolderChain = ownerFolderChain, + }; + + /// A UNS reference variable under a live equipment folder. + /// Root-first live ancestry ending at the variable's equipment folder. + /// The subject. + public static UnsAclSubject ReferenceVariable(IReadOnlyList equipmentChain) => + new() { Kind = UnsAclSubjectKind.ReferenceVariable, FolderChain = equipmentChain }; } 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 1b17c323d..35534fb7c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ScriptedAlarmOwnerScopeWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ScriptedAlarmOwnerScopeWireTests.cs @@ -298,6 +298,23 @@ public sealed class ScriptedAlarmOwnerScopeWireTests new Dictionary { [Pump] = (LineId, "Pump"), [Fan] = (LineId, "Fan") }, []); + /// + /// The topology's UNS folder placements (Gitea #808), from the same resolver: a scripted condition is placed + /// only while its owner's live folder ancestry matches the generation's. + /// + private static Dictionary FolderScopes() + { + var resolver = Resolver(); + NodeAclPath One(NodeAclScopeKind kind, string id) => resolver.ResolveOne(kind, id).ShouldHaveSingleItem(); + return new Dictionary + { + [AreaId] = new(One(NodeAclScopeKind.UnsArea, AreaId), [AreaId]), + [LineId] = new(One(NodeAclScopeKind.UnsLine, LineId), [AreaId, LineId]), + [Pump] = new(One(NodeAclScopeKind.Equipment, Pump), [AreaId, LineId, Pump]), + [Fan] = new(One(NodeAclScopeKind.Equipment, Fan), [AreaId, LineId, Fan]), + }; + } + private static Dictionary OwnerPaths() { var resolver = Resolver(); @@ -324,7 +341,7 @@ public sealed class ScriptedAlarmOwnerScopeWireTests 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); + state.Apply(ServerEventRig.Cluster, revision, keys, grants, Resolver().Resolve(grants), OwnerPaths(), deployedOwners, FolderScopes()); /// The generation's deployed scripted-alarm owners; moves the Pump alarm. private static Dictionary Deployed(string alarmOwner = Pump) => diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/Sdk2HistoryAuthorityWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/Sdk2HistoryAuthorityWireTests.cs index d50080820..8375fbb6e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/Sdk2HistoryAuthorityWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/Sdk2HistoryAuthorityWireTests.cs @@ -18,6 +18,11 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests; public sealed class Sdk2HistoryAuthorityWireTests { private const string ValuePath = RawDeviceFolder + "/History"; + + // Gitea #808 review: a UNS variable is authorized at its placed path, and a reference-variable id that is not + // its equipment's path + a name is denied (a moved/renamed equipment's stale ids). The alias used to reuse the + // RAW path as its UNS id — a shape production never mints — so it now carries a UNS-shaped id under EqA. + private const string AliasPath = EqA + "/HistoryAlias"; private static readonly DateTime Epoch = new(2026, 10, 1, 0, 0, 0, DateTimeKind.Utc); public static TheoryData Cases => new( @@ -79,20 +84,20 @@ public sealed class Sdk2HistoryAuthorityWireTests private static void Apply(DataPlaneAuthorityState state, ApiKey key, NodeAcl[] grants) => state.Apply(ServerEventRig.Cluster, Guid.NewGuid().ToString("N"), [key], grants, - new Dictionary>()); + new Dictionary>(), unsFolderScopes: FolderScopes()); private static void Materialise(ServerEventRig rig) { rig.MaterialiseNative(); rig.Sink.EnsureVariable(ValuePath, RawDeviceFolder, "History", "Double", false, AddressSpaceRealm.Raw, "same-backend-tag"); - rig.Sink.EnsureVariable(ValuePath, EqA, "HistoryAlias", "Double", false, AddressSpaceRealm.Uns, "same-backend-tag"); + rig.Sink.EnsureVariable(AliasPath, EqA, "HistoryAlias", "Double", false, AddressSpaceRealm.Uns, "same-backend-tag"); } private static async Task PageAsync(ISession session, string kind, bool alias, ByteString point, CancellationToken ct) { var realm = alias ? V3NodeIds.UnsNamespaceUri : V3NodeIds.RawNamespaceUri; - var path = kind == "events" ? alias ? EqA : RawDeviceFolder : ValuePath; + var path = kind == "events" ? alias ? EqA : RawDeviceFolder : alias ? AliasPath : ValuePath; var node = new NodeId(path, (ushort)session.NamespaceUris.GetIndex(realm)); HistoryReadDetails details = kind == "events" ? new ReadEventDetails { 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 9b3bfd7dd..4f512cf39 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(), ScriptedAlarmOwners()); + else state.Apply(ServerEventRig.Cluster, "revoked", keys, grants, scopes, EquipmentOwnerPaths(), ScriptedAlarmOwners(), FolderScopes()); 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(), ScriptedAlarmOwners()); + state.Apply(ServerEventRig.Cluster, "restored", [key], [grant], scopes, EquipmentOwnerPaths(), ScriptedAlarmOwners(), FolderScopes()); 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/UnsFolderScopeWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/UnsFolderScopeWireTests.cs index 17534adc4..0b004a00d 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/UnsFolderScopeWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/UnsFolderScopeWireTests.cs @@ -212,34 +212,223 @@ public sealed class UnsFolderScopeWireTests /// /// An EQUIPMENT moved to another line by the generation while its folder still hangs under the old line: the live - /// folder ancestry disagrees with the generation's, so the folder and everything it owns are denied under the - /// new line's grant AND the old one's. After the rebuild re-parents it, the new line's grant reads it and the old - /// line's does not. + /// folder ancestry disagrees with the generation's, so the folder, everything it owns, its scripted alarm (read + /// AND Acknowledge) and its UNS reference variable (still on its OLD-name id — read AND write) are denied under + /// the new line's grant AND the old one's. After the rebuild re-parents it (and re-ids the reference variable), + /// the new line's grant reaches all of it and the old line's reaches none of it. /// [Theory] [InlineData(Line1)] [InlineData(Line2)] - public async Task A_moved_equipment_folder_is_denied_until_the_rebuild_reparents_it(string grantedLine) + public async Task A_moved_equipment_and_everything_under_it_is_denied_until_the_rebuild_reparents_it(string grantedLine) { var ct = TestContext.Current.CancellationToken; await using var rig = await ServerEventRig.StartAsync($"Uns808Folder{grantedLine[^1..]}", encrypted: false, ct); rig.KeyRoles = []; Topology(rig, pumpLine: Line1); Install(rig, pumpLine: Line2, Deployed(), LineGrant(grantedLine)); + var routed = Route(rig); using var session = await rig.ConnectAsync(anonymous: false, ct); + var oldFlow = FlowId(Line1); (await ReadAsync(session, Uns(session, Pump), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.BadUserAccessDenied); (await ReadAsync(session, Uns(session, PumpSpeed), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.BadUserAccessDenied); - // Positive control: the sibling, placed identically by both, follows the grant. + (await ReadAsync(session, Uns(session, oldFlow), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.BadUserAccessDenied); + (await WriteAsync(session, Uns(session, oldFlow), 1.5, ct)).ShouldBe(StatusCodes.BadUserAccessDenied); + (await ReadAsync(session, Uns(session, Alarm), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.BadUserAccessDenied); + // The condition must carry a real EventId first, or the SDK refuses the call before any gate runs. + Project(rig, Alarm, new AlarmConditionSnapshot(true, false, false, true, AlarmShelvingKind.Unshelved, 700, "base")); + (await Should.ThrowAsync(ConditionMethodEventTests.CallAsync( + session, Uns(session, Alarm), "Acknowledge", rig.Nm.TryGetAlarmCondition(Alarm)!.EventId!.Value.ToArray(), ct))) + .StatusCode.ShouldBe(StatusCodes.BadUserAccessDenied); + routed.ShouldBeEmpty(); + // Positive control: the sibling, placed identically by both generations, follows the grant. (await ReadAsync(session, Uns(session, Fan), Attributes.DisplayName, ct)) .ShouldBe(grantedLine == Line1 ? StatusCodes.Good : StatusCodes.BadUserAccessDenied); rig.Sink.RebuildAddressSpace(); Topology(rig, pumpLine: Line2); - var expected = grantedLine == Line2 ? StatusCodes.Good : StatusCodes.BadUserAccessDenied; + var allowed = grantedLine == Line2; + var expected = allowed ? StatusCodes.Good : StatusCodes.BadUserAccessDenied; + var newFlow = FlowId(Line2); (await ReadAsync(session, Uns(session, Pump), Attributes.DisplayName, ct)).ShouldBe(expected); (await ReadAsync(session, Uns(session, PumpSpeed), Attributes.DisplayName, ct)).ShouldBe(expected); + (await ReadAsync(session, Uns(session, newFlow), Attributes.DisplayName, ct)).ShouldBe(expected); + (await ReadAsync(session, Uns(session, Alarm), Attributes.DisplayName, ct)).ShouldBe(expected); + var write = await WriteAsync(session, Uns(session, newFlow), 1.5, ct); + if (allowed) write.ShouldNotBe(StatusCodes.BadUserAccessDenied, "the new line's grant reaches the re-ided variable"); + else write.ShouldBe(StatusCodes.BadUserAccessDenied); + + Project(rig, Alarm, new AlarmConditionSnapshot(true, false, false, true, AlarmShelvingKind.Unshelved, 700, "rebuilt")); + var ack = ConditionMethodEventTests.CallAsync( + session, Uns(session, Alarm), "Acknowledge", rig.Nm.TryGetAlarmCondition(Alarm)!.EventId!.Value.ToArray(), ct); + if (allowed) await ack; + else (await Should.ThrowAsync(ack)).StatusCode.ShouldBe(StatusCodes.BadUserAccessDenied); + routed.Count.ShouldBe(allowed ? 1 : 0); + + await session.CloseAsync(ct); + } + + /// + /// An Area RENAMED by the generation before the address space re-ids its reference variables: the folders keep + /// their opaque ids (still placed — the Equipment grant reads the folder and its virtual tag), but the reference + /// variable's id still spells the OLD name, so it is denied until it is re-pointed. + /// + [Fact] + public async Task A_reference_variable_on_a_stale_name_path_is_denied_while_its_folder_is_placed() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Uns808Rename", encrypted: false, ct); + rig.KeyRoles = []; + Topology(rig, pumpLine: Line1); + // The Tag grant's path still spells the OLD Area name (Install plants it at Area1/Line1/Pump/Flow) — standing in + // for any grant that names the old path, e.g. another Area that has since been given the name "Area1". + Install(rig, pumpLine: Line1, Deployed(), areaName: "AreaRenamed", EquipmentGrant(Pump, Operator), TagGrant()); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + (await ReadAsync(session, Uns(session, Pump), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.Good); + (await ReadAsync(session, Uns(session, PumpSpeed), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.Good); + (await ReadAsync(session, Uns(session, PumpFlow), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.BadUserAccessDenied); + + await session.CloseAsync(ct); + } + + /// + /// Review of #808 (cache poisoning): the resolved-scope cache keys are NUL-prefixed, so a client that first + /// Reads the nonexistent s="\0Area1\0Line1\0Pump" — the exact cache key of the real Pump folder — must + /// not memoize a deny the real folder then inherits. A NUL-bearing id is refused before the cache. + /// + [Fact] + public async Task A_nul_bearing_identifier_cannot_poison_a_resolved_scopes_cached_verdict() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Uns808Nul", encrypted: false, ct); + rig.KeyRoles = []; + Topology(rig, pumpLine: Line1); + Install(rig, pumpLine: Line1, Deployed(), EquipmentGrant(Pump, Operator)); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + (await ReadAsync(session, Uns(session, "\u0000Area1\u0000Line1\u0000Pump"), Attributes.DisplayName, ct)) + .ShouldBe(StatusCodes.BadUserAccessDenied); + (await ReadAsync(session, Uns(session, Pump), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.Good); + (await ReadAsync(session, Uns(session, Pump), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.Good); + + await session.CloseAsync(ct); + } + + /// + /// Review of #808: an identifier NO live node holds (a removed folder, virtual tag or scripted alarm — what a + /// NodeDeleted model change names — or a probe) is decided only by a realm-wide grant. It no longer splits, so a + /// removed opaque id never matches a narrow grant on an unrelated entity of the same NAME (an Area named like a + /// removed alarm id). A reference-variable-shaped id (four segments) keeps its name-path split. + /// + [Fact] + public async Task An_unknown_uns_identifier_is_decided_only_by_a_realm_wide_grant() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Uns808Unknown", encrypted: false, ct); + rig.KeyRoles = []; + Topology(rig, pumpLine: Line1); + Install(rig, pumpLine: Line1, Deployed(), + KeyGrant("area", NodeAclScopeKind.UnsArea, AreaId, NodePermissions.ReadOnly), + new NodeAcl + { + NodeAclId = "anon-cluster", ClusterId = ServerEventRig.Cluster, Principal = OpcUaPrincipals.Anonymous, + ScopeKind = NodeAclScopeKind.Cluster, PermissionFlags = NodePermissions.ReadOnly, + }); + + using var narrow = await rig.ConnectAsync(anonymous: false, ct); + using var wide = await rig.ConnectAsync(anonymous: true, ct); + + // "Area1" is the granted Area's NAME — a removed scripted alarm with that id used to split onto it. + foreach (var gone in new[] { "Area1", "EQ-00000000dead", "Area1/Line1", "scripted-removed/EnabledState" }) + { + (await ReadAsync(narrow, Uns(narrow, gone), Attributes.DisplayName, ct)) + .ShouldBe(StatusCodes.BadUserAccessDenied, gone); + (await ReadAsync(wide, Uns(wide, gone), Attributes.DisplayName, ct)) + .ShouldBe(StatusCodes.BadNodeIdUnknown, $"{gone}: a realm-wide grant still decides it"); + } + + // A four-segment (reference-variable-shaped) id keeps its name-path split under the Area grant. + (await ReadAsync(narrow, Uns(narrow, "Area1/Line1/Pump/Gone"), Attributes.DisplayName, ct)) + .ShouldBe(StatusCodes.BadNodeIdUnknown); + // Positive control: the live Area folder itself. + (await ReadAsync(narrow, Uns(narrow, AreaId), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.Good); + + await narrow.CloseAsync(ct); + await wide.CloseAsync(ct); + } + + /// + /// The placements through the PRODUCTION producer: a deployment artifact's UNS rows, scripted alarms and grant → + /// DeploymentArtifact.ParseDataPlaneAuthority (its folder scopes, owner paths, deployed owners and grant + /// paths) → install → wire decisions, with no hand-built scope anywhere. + /// + [Fact] + public async Task Placements_decoded_from_a_deployment_artifact_authorize_a_narrow_grant_over_the_wire() + { + var ct = TestContext.Current.CancellationToken; + await using var rig = await ServerEventRig.StartAsync("Uns808Artifact", encrypted: false, ct); + rig.KeyRoles = []; + Topology(rig, pumpLine: Line1); + var routed = Route(rig); + + var artifact = $$""" + { + "UnsAreas": [{"UnsAreaId":"{{AreaId}}","ClusterId":"{{ServerEventRig.Cluster}}","Name":"Area1"}], + "UnsLines": [ + {"UnsLineId":"{{Line1}}","UnsAreaId":"{{AreaId}}","Name":"Line1"}, + {"UnsLineId":"{{Line2}}","UnsAreaId":"{{AreaId}}","Name":"Line2"} + ], + "Equipment": [ + {"EquipmentId":"{{Pump}}","UnsLineId":"{{Line1}}","Name":"Pump"}, + {"EquipmentId":"{{Fan}}","UnsLineId":"{{Line1}}","Name":"Fan"} + ], + "ScriptedAlarms": [ + {"ScriptedAlarmId":"{{Alarm}}","EquipmentId":"{{Pump}}","Name":"hi"}, + {"ScriptedAlarmId":"{{PumpLo}}","EquipmentId":"{{Pump}}","Name":"lo"}, + {"ScriptedAlarmId":"{{FanAlarm}}","EquipmentId":"{{Fan}}","Name":"fan hi"} + ], + "NodeAcls": [ + {"NodeAclId":"acl-pump","ClusterId":"{{ServerEventRig.Cluster}}","Principal":"{{ServerEventRig.KeyPrincipal}}", + "ScopeKind":{{(int)NodeAclScopeKind.Equipment}},"ScopeId":"{{Pump}}","PermissionFlags":{{(int)Operator}}} + ] + } + """; + var projection = ZB.MOM.WW.OtOpcUa.Runtime.Drivers.DeploymentArtifact.ParseDataPlaneAuthority( + System.Text.Encoding.UTF8.GetBytes(artifact), "node-a:4840"); + projection.UnsFolderScopes.Keys.Order(StringComparer.Ordinal).ShouldBe([Pump, Fan, AreaId, Line1, Line2]); + + var state = new DataPlaneAuthorityState(); + state.Apply(ServerEventRig.Cluster, "rev-artifact", [ServerEventRig.Key()], projection.Grants, + projection.ScopePaths, projection.EquipmentScopePaths, projection.ScriptedAlarmOwners, projection.UnsFolderScopes); + rig.Nm.DataPlaneAuthority = state; + rig.Nm.Authorizer = new DataPlaneAuthorizer( + state, new TriePermissionEvaluator(state.Tries), true, NullLogger.Instance, TimeProvider.System); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + (await BrowseAsync(session, Uns(session, Line1), ct)).Ids.ShouldBe([Pump]); + (await ReadAsync(session, Uns(session, Pump), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.Good); + (await ReadAsync(session, Uns(session, PumpSpeed), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.Good); + (await ReadAsync(session, Uns(session, PumpFlow), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.Good); + (await ReadAsync(session, Uns(session, ChildId(rig, Alarm, c => c.EnabledState!.Id!)), Attributes.Value, ct)) + .ShouldBe(StatusCodes.Good); + (await ReadAsync(session, Uns(session, Fan), Attributes.DisplayName, ct)).ShouldBe(StatusCodes.BadUserAccessDenied); + + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var server = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + var pre = new AlarmConditionSnapshot(true, false, false, true, AlarmShelvingKind.Unshelved, 700, "base"); + Project(rig, FanAlarm, pre with { Message = "fan-base" }); + Project(rig, Alarm, pre); + await RecordedEvents.WaitUntilAsync(() => events.Has(server, "base")); + events.Has(server, "fan-base").ShouldBeFalse(); + + await ConditionMethodEventTests.CallAsync( + session, Uns(session, Alarm), "Acknowledge", events.LatestEventId(server, "base"), ct); + routed.Select(c => (c.AlarmId, c.Operation)).ShouldBe([(Alarm, "Acknowledge")]); await session.CloseAsync(ct); } @@ -260,19 +449,22 @@ public sealed class UnsFolderScopeWireTests rig.Sink.EnsureFolder(Fan, parentNodeId: Line1, displayName: "Fan", realm: AddressSpaceRealm.Uns); rig.Sink.EnsureVariable(PumpSpeed, Pump, "Speed", "Double", writable: false, AddressSpaceRealm.Uns); rig.Sink.EnsureVariable(FanSpeed, Fan, "Speed", "Double", writable: false, AddressSpaceRealm.Uns); - rig.Sink.EnsureVariable(PumpFlow, Pump, "Flow", "Double", writable: false, AddressSpaceRealm.Uns); + rig.Sink.EnsureVariable(FlowId(pumpLine), Pump, "Flow", "Double", writable: true, AddressSpaceRealm.Uns); rig.Sink.MaterialiseAlarmCondition(Alarm, Pump, "hi", "AlarmCondition", 500, AddressSpaceRealm.Uns); rig.Sink.MaterialiseAlarmCondition(PumpLo, Pump, "lo", "AlarmCondition", 300, AddressSpaceRealm.Uns); rig.Sink.MaterialiseAlarmCondition(FanAlarm, Fan, "fan hi", "AlarmCondition", 500, AddressSpaceRealm.Uns); } - private static AclScopePathResolver Resolver(string pumpLine) => new( + /// The live reference variable's id when Pump hangs under (its NAME path). + private static string FlowId(string pumpLine) => $"Area1/{(pumpLine == Line1 ? "Line1" : "Line2")}/Pump/Flow"; + + private static AclScopePathResolver Resolver(string pumpLine, string areaName) => new( new Dictionary(), new Dictionary(), new Dictionary(), new Dictionary(), new Dictionary(), - new Dictionary { [AreaId] = "Area1" }, + new Dictionary { [AreaId] = areaName }, new Dictionary { [Line1] = (AreaId, "Line1"), [Line2] = (AreaId, "Line2") }, new Dictionary { [Pump] = (pumpLine, "Pump"), [Fan] = (Line1, "Fan") }, []); @@ -282,9 +474,16 @@ public sealed class UnsFolderScopeWireTests /// scopes and grant paths all from ONE resolver, as DeploymentArtifact.ParseDataPlaneAuthority does. /// private static void Install( - ServerEventRig rig, string pumpLine, IReadOnlyDictionary deployedOwners, params NodeAcl[] grants) + ServerEventRig rig, string pumpLine, IReadOnlyDictionary deployedOwners, params NodeAcl[] grants) => + Install(rig, pumpLine, deployedOwners, "Area1", grants); + + /// As above, with the generation naming the Area (a rename the address space + /// has not caught up with when it differs from the live Area1). + private static void Install( + ServerEventRig rig, string pumpLine, IReadOnlyDictionary deployedOwners, string areaName, + params NodeAcl[] grants) { - var resolver = Resolver(pumpLine); + var resolver = Resolver(pumpLine, areaName); var scopePaths = new Dictionary>(resolver.Resolve(grants)) { // A Tag grant's UNS projection (the resolver would derive it from raw rows this rig does not model). @@ -322,7 +521,7 @@ public sealed class UnsFolderScopeWireTests KeyGrant($"eq-{equipmentId}", NodeAclScopeKind.Equipment, equipmentId, flags); private static NodeAcl LineGrant(string lineId) => - KeyGrant($"line-{lineId}", NodeAclScopeKind.UnsLine, lineId, NodePermissions.ReadOnly); + KeyGrant($"line-{lineId}", NodeAclScopeKind.UnsLine, lineId, Operator | NodePermissions.WriteOperate); private static NodeAcl TagGrant() => KeyGrant("tag-flow", NodeAclScopeKind.Tag, FlowTagId, Operator); @@ -370,6 +569,15 @@ public sealed class UnsFolderScopeWireTests return response.Results.ToList().ShouldHaveSingleItem().StatusCode; } + private static async Task WriteAsync(ISession session, NodeId node, double value, CancellationToken ct) + { + var response = await session.WriteAsync(null, new WriteValue[] + { + new() { NodeId = node, AttributeId = Attributes.Value, Value = new DataValue(SdkTestValues.FromClr(value)) }, + }, ct); + return response.Results.ToList().ShouldHaveSingleItem(); + } + private static async Task<(StatusCode Status, List Ids)> BrowseAsync( ISession session, NodeId parent, CancellationToken ct) { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerModelChangeFilterTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerModelChangeFilterTests.cs index dce66a0a1..69ca4ff7f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerModelChangeFilterTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerModelChangeFilterTests.cs @@ -72,6 +72,75 @@ public sealed class NodeManagerModelChangeFilterTests : IDisposable finally { await host.DisposeAsync(); } } + /// + /// Gitea #808 review — NodeDeleted delivery on a production-shaped (opaque-id) tree. A NodesAdded change for a + /// LIVE equipment folder reaches the narrow Equipment grantee (it is placed); a NodeDeleted change names a node + /// no live map holds any more, so only a realm-wide grantee receives it — the removed opaque id (a virtual tag, + /// a scripted alarm whose id equals the granted Area's NAME) is never split onto an unrelated narrow grant. + /// + [Fact] + public async Task A_removed_opaque_uns_node_is_announced_only_to_realm_wide_grantees() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + var sink = new SdkAddressSpaceSink(nm); + sink.EnsureFolder("UA-1", null, "Area1", AddressSpaceRealm.Uns); + sink.EnsureFolder("UL-1", "UA-1", "Line1", AddressSpaceRealm.Uns); + sink.EnsureFolder("EQ-1", "UL-1", "Pump", AddressSpaceRealm.Uns); + + var state = new DataPlaneAuthorityState(); + NodeAcl[] grants = + [ + new NodeAcl + { + NodeAclId = "acl-eq", ClusterId = Cluster, Principal = Principal, + ScopeKind = NodeAclScopeKind.Equipment, ScopeId = "EQ-1", PermissionFlags = NodePermissions.Browse, + }, + new NodeAcl + { + NodeAclId = "acl-area", ClusterId = Cluster, Principal = "area-key", + ScopeKind = NodeAclScopeKind.UnsArea, ScopeId = "UA-1", PermissionFlags = NodePermissions.Browse, + }, + new NodeAcl + { + NodeAclId = "acl-all", ClusterId = Cluster, Principal = "everything", + ScopeKind = NodeAclScopeKind.Cluster, PermissionFlags = NodePermissions.Browse, + }, + ]; + var area = new NodeAclPath([NodeAclRealms.Uns, "Area1"]); + var line = new NodeAclPath([NodeAclRealms.Uns, "Area1", "Line1"]); + var pump = new NodeAclPath([NodeAclRealms.Uns, "Area1", "Line1", "Pump"]); + state.Apply(Cluster, "rev-1", [], grants, + new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.Equipment, "EQ-1")] = [pump], + [new AclScopeRef(NodeAclScopeKind.UnsArea, "UA-1")] = [area], + }, + new Dictionary { ["EQ-1"] = pump }, + unsFolderScopes: new Dictionary + { + ["UA-1"] = new(area, ["UA-1"]), + ["UL-1"] = new(line, ["UA-1", "UL-1"]), + ["EQ-1"] = new(pump, ["UA-1", "UL-1", "EQ-1"]), + }); + nm.Authorizer = new DataPlaneAuthorizer(state, new TriePermissionEvaluator(state.Tries), true, NullLogger.Instance); + + var added = nm.BuildNodesAddedModelChange("EQ-1", AddressSpaceRealm.Uns); + nm.FilterModelChangeFor(Identity(Principal), added).ShouldBeSameAs(added, "the live folder is placed"); + + foreach (var gone in new[] { "EQ-1/Speed", "Area1", "scripted-gone" }) + { + var deleted = nm.BuildNodesRemovedModelChange(gone, AddressSpaceRealm.Uns); + nm.FilterModelChangeFor(Identity(Principal), deleted).ShouldBeNull(gone); + nm.FilterModelChangeFor(Identity("area-key"), deleted).ShouldBeNull($"{gone}: no name-collision match"); + nm.FilterModelChangeFor(Identity("everything"), deleted).ShouldBeSameAs(deleted, gone); + } + } + finally { await host.DisposeAsync(); } + } + private static DataPlaneAuthorizer Authorizer(params NodeAcl[] grants) { var state = new DataPlaneAuthorityState(); 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 14fd89e02..50f5e907d 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(), Deployed()); + new Dictionary(), Deployed(), FolderScopes()); break; } @@ -595,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(), Deployed()); + state.Apply(Cluster, "rev-raw", [Key()], [rawDevice], scopes, OwnerPaths(), Deployed(), FolderScopes()); condition.OnAcknowledge!(ctx, condition, EventId(), new LocalizedText("raw grant")).ShouldBe(ServiceResult.Good); condition.OnAddComment!(ctx, condition, EventId(), new LocalizedText("raw note")).ShouldBe(ServiceResult.Good); @@ -684,6 +684,23 @@ public sealed class NodeManagerScriptedAlarmOwnerScopeTests : IDisposable }; } + /// + /// Gitea #808 review: the scripted-condition placement also requires the owner's LIVE folder ancestry to match + /// the generation's, so every install here carries the topology's folder scopes (same resolver as the owners). + /// + private static Dictionary FolderScopes() + { + var resolver = Resolver(); + NodeAclPath One(NodeAclScopeKind kind, string id) => resolver.ResolveOne(kind, id).ShouldHaveSingleItem(); + return new Dictionary + { + [AreaId] = new(One(NodeAclScopeKind.UnsArea, AreaId), [AreaId]), + [LineId] = new(One(NodeAclScopeKind.UnsLine, LineId), [AreaId, LineId]), + [Pump] = new(One(NodeAclScopeKind.Equipment, Pump), [AreaId, LineId, Pump]), + [Fan] = new(One(NodeAclScopeKind.Equipment, Fan), [AreaId, LineId, Fan]), + }; + } + private static (DataPlaneAuthorityState State, DataPlaneAuthorizer Authorizer) Wire( OtOpcUaNodeManager nm, TimeProvider clock, ApiKey[] keys, params NodeAcl[] grants) => Wire(nm, clock, keys, enabled: true, Areas(), grants); @@ -702,7 +719,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(), Deployed()); + state.Apply(Cluster, "rev-1", keys, grants, Resolver(areas).Resolve(grants), OwnerPaths(), Deployed(), FolderScopes()); var authorizer = new DataPlaneAuthorizer( state, new TriePermissionEvaluator(state.Tries), enabled, NullLogger.Instance, clock); nm.DataPlaneAuthority = state; @@ -717,7 +734,7 @@ public sealed class NodeManagerScriptedAlarmOwnerScopeTests : IDisposable 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); + state.Apply(Cluster, revision, keys, grants, Resolver().Resolve(grants), OwnerPaths(), deployedOwners, FolderScopes()); /// /// The generation's deployed scripted-alarm owners (the artifact's ScriptedAlarms rows);