diff --git a/docs/plans/2026-07-31-apikey-acl-enforcement-design.md b/docs/plans/2026-07-31-apikey-acl-enforcement-design.md index 7c2e9c00..d883cb95 100644 --- a/docs/plans/2026-07-31-apikey-acl-enforcement-design.md +++ b/docs/plans/2026-07-31-apikey-acl-enforcement-design.md @@ -78,9 +78,9 @@ string; only what produces that string changes. | # | #520's blocker | Resolution under this design | |---|---|---| -| 1 | Raw-realm nodes have no representable `NodeScope` (`NodeHierarchyKind` has one member, `Equipment`) | **Add `NodeHierarchyKind.Raw`**, whose path is the RawPath segments `Folder/Driver/Device/TagGroup/Tag`. `NodeAclScopeKind` already carries `Namespace` and `FolderSegment` — `FolderSegment` becomes matchable for the first time, at raw-folder level. This is the one genuine **design** item, not wiring. | +| 1 | Raw-realm nodes have no representable `NodeScope` (`NodeHierarchyKind` has one member, `Equipment`) | **Add `NodeHierarchyKind.Raw`.** ⚠️ **This cell was wrong about the shape** — it described the path as the five-tuple `Folder/Driver/Device/TagGroup/Tag`, but folders and tag groups both self-nest, so the real path is `folder… / Driver / Device / group… / Tag` with unbounded depth on two axes. The fix was therefore not an enum member but a variable-depth `NodeScope` + segment-loop walk. `FolderSegment` was **not** repurposed as the raw-folder level; it is retired, and `RawFolder`/`RawDriver`/`RawDevice`/`RawTagGroup` were appended instead. Correctly identified as the one genuine **design** item. | | 2 | Session carries roles, not LDAP groups | **Dissolved by D1** — the principal comes from the key, not from a directory. | -| 3 | `PermissionTrieBuilder`'s `scopePaths` has no producer | Built node-side from the artifact's UNS hierarchy + raw tree. Note every existing evaluator unit test runs in the no-`scopePaths` "deterministic test mode", so **the production hierarchy path is currently untested** — it needs its own coverage, not just a producer. | +| 3 | `PermissionTrieBuilder`'s `scopePaths` has no producer | **Done in stage 3** — `AclScopePathResolver`, built node-side from the artifact's raw + UNS topology (no DB read, so a driver-only node can build it). The coverage note was right in substance but imprecise: six tests *did* pass a `scopePaths` dictionary, but every one hand-wrote it in the test body with exactly the segments the walker expected, so nothing pinned a producer — and there was none to pin. That gap is closed by an artifact → parse → build → walk test. | | 4 | No session lifecycle / `AuthGenerationId` | Stamp the generation from the artifact `RevisionHash` at session activation; a new deploy invalidates cached decisions. `UserAuthorizationState` already has the freshness predicates. | Plus, from the same audit: the node never parses ACLs out of the artifact, and `OtOpcUaNodeManager` @@ -99,11 +99,18 @@ mismatch). ## Smaller inconsistencies to settle en route - `NodeAclScopeKind.FolderSegment` is offered in the authoring dropdown but the trie walker has no - level to match it at — fixed by blocker #1's `Raw` kind. + level to match it at — **settled in stage 3, but not the way this line assumed.** It is a retired v2 + concept (it described the `SystemPlatform`/Galaxy hierarchy v3 deleted), so rather than being given a + level it is now explicitly unresolvable, with `RawFolder` as its v3 replacement. It survives in the + enum only because the artifact stores the ordinal, so removing it would renumber every later member + in every deployed artifact. The dropdown labels it retired, and the deploy gate names it. + ⚠️ The old claim that the walker could not match it was slightly overstated: `Build` routed it through + `Descend` like any other kind, so given a path it *would* have matched and been reported under + whatever level it happened to land on. It was unlabellable, not unplaceable. - `NodePermissions.AlarmRead` has no `OpcUaOperation` mapping — grantable but unreachable. - `NodeAcl.ScopeId` has no FK and no existence check, so scope ids dangle silently. -- `ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj` does not reference `Core` at all — the evaluator is not even - on the node manager's compile-time reference graph. +- ~~`ZB.MOM.WW.OtOpcUa.OpcUaServer.csproj` does not reference `Core` at all~~ — **stale**; stage 1 added + the reference. The line read as an open item long after it was closed. - `NodePermissions.HistoryUpdate` is deliberately in no bundle until HistoryUpdate exists. Leave it. ## Stages @@ -115,8 +122,55 @@ Each stage is one commit, builds clean, and leaves the tree shippable. 2. ✅ **Identity cutover.** `ApiKeyOpcUaUserAuthenticator` replaces `LdapOpcUaUserAuthenticator` on the OPC UA data plane; Anonymous resolves to the `anonymous` principal or is rejected per setting. AdminUI LDAP untouched. -3. **Raw scope.** *(not started — sequenced after stage 4 at the owner's request)* `NodeHierarchyKind.Raw` + RawPath scope paths; `scopePaths` producer built from the - artifact; coverage for the production hierarchy path that today has none. +3. **Raw scope.** ✅ **Done.** `NodeHierarchyKind.Raw`, a variable-depth scope model, the `scopePaths` + producer (`AclScopePathResolver`) fed from the artifact, a deploy-time gate for ACL scopes, and the + first coverage of the production hierarchy path. + + **It was substantially larger than this line originally implied, because the sketch had the shape of + the raw tree wrong.** The plan said the Raw path is the five-tuple + `Folder/Driver/Device/TagGroup/Tag`. It is not: `RawFolder.ParentRawFolderId` and + `TagGroup.ParentTagGroupId` are both self-referencing, so folders and tag groups each nest without + bound and a real RawPath is `folder… / Driver / Device / group… / Tag`. `NodeScope` was fixed-arity + (five nullable level fields) and `PermissionTrie.CollectMatches` was five hardcoded steps, so the + scope model could not address a real raw tree at all. `NodeScope` is now `{ClusterId, Kind, + IReadOnlyList Segments}` and the walk is a segment loop. Good news found en route: the trie + *structure* was already variable-depth (`NodeAclPath.Segments` is a list; `PermissionTrieNode.Children` + is a recursive dictionary), so only the walker and the scope record had to change. + + Four decisions worth keeping: + + - **The namespace level became the realm.** Level 1 of the walk gated on `NodeScope.NamespaceId`, but + v3 **has no `Namespace` entity** — the snapshot was retired when the address space went + dual-namespace, and the identifier appeared nowhere in production outside `NodeScope`. So level 1 + could only ever fail to match, which silently made *every* sub-cluster grant unreachable while + cluster grants kept working. It is now the realm token (`raw`/`uns`), derived from the scope's + `Kind` by `NodeAclRealms` rather than carried on it, so the walk and the path producer cannot + disagree about its spelling. `NodeAclScopeKind.Namespace` becomes meaningful again as "a whole + realm". + - **Provenance comes from the granting row's `ScopeKind`**, recorded on `TrieGrant` at build time, + not from the depth the grant sits at. Depth-derived provenance could not name a raw level at all + (the depth of "Device" varies with folder nesting) and misreported any path whose length disagreed + with its kind. + - **The trie comparer moved from `OrdinalIgnoreCase` to `Ordinal`.** Raw sibling-name uniqueness is + ordinal, so `Speed` and `speed` can both exist under one device and a grant on either would have + matched both — a silent widening. Case-sensitive is also the fail-closed direction. + - **The null-`scopePaths` "deterministic test mode" is deleted.** It planted unresolvable rows at a + direct child of the trie root, a position the walker cannot reach — so the grant looked placed and + could never match, and every test using that mode proved nothing about production placement. An + unresolvable row is now dropped with a diagnostic, and `scopePaths` is required. + + A scope resolves to a **list** of paths, not one: a raw tag projected into N equipments is reachable + at N+1 NodeIds and a write through any of them lands on the same driver reference, so a Tag-scoped + grant that covered only the raw node would be bypassable through a UNS one. The lookup is keyed on + `(ScopeKind, ScopeId)` rather than `ScopeId` alone, because `ScopeId` has no FK and no type + discriminator — ids are prefixed only by convention, and an id collision across entity types would + hand a grant the wrong node's paths. + + Also closed here: `NodeAcl` rows had **no deploy-time validation of any kind** and were not even + reachable from `DraftValidator` (`DraftSnapshot` carried no `NodeAcls` collection). They are now + gated for dangling ids, cross-cluster scopes, blank/contradictory `ScopeId`, the retired + `FolderSegment` kind, and undefined ordinals. That gate is what substitutes for the ACL scope picker + until stage 7 — `ScopeId` is still a free-text box. 4. **Artifact + lifecycle.** ✅ **Done.** `ConfigComposer` snapshots `ApiKeys` alongside the `NodeAcls` it already carried; `DeploymentArtifact.ParseDataPlaneAuthority` decodes both node-side (grants scoped to the node's own cluster, keys not — a key is a fleet-wide credential and the ACL rows are diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodeAclScopeKind.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodeAclScopeKind.cs index 63c73864..104e48f4 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodeAclScopeKind.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodeAclScopeKind.cs @@ -1,18 +1,62 @@ namespace ZB.MOM.WW.OtOpcUa.Configuration.Enums; /// ACL scope level. Per acl-design.md §"Scope Hierarchy". +/// +/// ⚠️ Append new members only — never insert. SQL Server stores this as a string +/// (nvarchar(16), so a member name must also stay ≤ 16 characters), but the deployment +/// artifact serialises it as the integer ordinal and the node-side decoder reads it back +/// with Enum.IsDefined. Inserting a member mid-enum silently re-maps every grant in every +/// already-deployed artifact to a different scope level. +/// public enum NodeAclScopeKind { + /// The whole cluster — the only kind that carries a null ScopeId. Cluster, - Namespace, - UnsArea, - UnsLine, - Equipment, + /// - /// A folder segment anchored below a namespace (used for FolderPath-scoped tags). - /// Distinguishes folder grants from UNS grants in the - /// AuthorizationDecision.Provenance audit trail and Admin UI diagnostics. + /// One whole OPC UA namespace (realm). ScopeId is the realm token raw or + /// uns; see NodeAclRealms, which is the single authority for that spelling. + /// + Namespace, + + /// A UNS area. ScopeId is a UnsArea.UnsAreaId. + UnsArea, + + /// A UNS line. ScopeId is a UnsLine.UnsLineId. + UnsLine, + + /// A UNS equipment. ScopeId is an Equipment.EquipmentId. + Equipment, + + /// + /// ⚠️ Retired v2 concept — do not author new rows with this kind. It described a folder + /// segment under the v2 SystemPlatform/Galaxy hierarchy, which v3 deleted along with the + /// WalkSystemPlatform walker. The v3 replacement is . The member + /// survives only because removing it would shift every later ordinal in already-deployed + /// artifacts; a row still carrying it resolves no scope path and is reported as an unresolved + /// scope by the deploy gate. /// FolderSegment, + + /// + /// A single tag. ScopeId is a Tag.TagId — i.e. a raw tag, which is the only + /// kind of tag v3 has. Because one raw tag can be projected into several equipments by + /// UnsTagReference rows, a Tag-scoped grant resolves to several trie paths: the raw + /// node plus every UNS projection of it. That is deliberate — a write routes to the same driver + /// reference through either NodeId, so a grant that covered only one of them would be bypassable + /// through the other. + /// Tag, + + /// A raw-tree folder. ScopeId is a RawFolder.RawFolderId. (v3; appended.) + RawFolder, + + /// A raw-tree driver instance. ScopeId is a DriverInstance.DriverInstanceId. (v3; appended.) + RawDriver, + + /// A raw-tree device. ScopeId is a Device.DeviceId. (v3; appended.) + RawDevice, + + /// A raw-tree tag group. ScopeId is a TagGroup.TagGroupId. (v3; appended.) + RawTagGroup, } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshot.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshot.cs index 4bed38ba..8618aa2b 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshot.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshot.cs @@ -62,6 +62,10 @@ public sealed class DraftSnapshot /// Gets the list of poll groups. public IReadOnlyList PollGroups { get; init; } = []; + /// ACL grants. Drive the scope-resolution rule: a scope that names nothing deployed is + /// silently dropped when the permission trie is built, so the grant has to be caught here instead. + public IReadOnlyList NodeAcls { get; init; } = []; + /// Prior Equipment rows (any generation, same cluster) for stability checks. public IReadOnlyList PriorEquipment { get; init; } = []; diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs index d10dfc82..3bfd0a5c 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshotFactory.cs @@ -52,6 +52,7 @@ public static class DraftSnapshotFactory ScriptedAlarms = await db.ScriptedAlarms.AsNoTracking().ToListAsync(ct), Scripts = await db.Scripts.AsNoTracking().ToListAsync(ct), PollGroups = await db.PollGroups.AsNoTracking().ToListAsync(ct), + NodeAcls = await db.NodeAcls.AsNoTracking().ToListAsync(ct), PriorEquipment = [], // intentional: no prior-generation table to diff against at this level ActiveReservations = await db.ExternalIdReservations .AsNoTracking() diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs index fde97dbd..3f44056f 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs @@ -1,4 +1,5 @@ using System.Text.RegularExpressions; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; @@ -42,9 +43,235 @@ public static class DraftValidator ValidateEquipReferenceResolution(draft, errors); ValidateCalculationTags(draft, errors); ValidateSqlConnectionStringNotPersisted(draft, errors); + ValidateNodeAclScopes(draft, errors); return errors; } + /// + /// #520 stage 3: every row's scope must name something the deployed address + /// space actually contains. + /// + /// + /// + /// ACL rows have had no validation anywhere until now, and the failure mode is the worst + /// shape a security control can have: AclScopePathResolver returns no path for a scope it + /// cannot resolve, the trie builder then drops the grant with a build-time diagnostic, and the + /// deploy still seals green. An operator who mistypes a ScopeId, or scopes a grant at an + /// entity someone later deleted, gets a row that reads as a live grant in the AdminUI and + /// authorizes nothing at runtime. Silently dropping a grant is fail-closed and therefore safe, + /// but it is not visible — which is why the check belongs at the deploy gate, where the + /// author is still standing in front of the mistake. + /// + /// + /// The realm tokens are duplicated here, deliberately. NodeAclRealms in + /// Core.Authorization is the authority for their spelling, but Core references + /// Configuration and not the reverse, so this project cannot reach it. The two namespace + /// URIs come from in Commons, which both sides do share; + /// only the two short tokens are literals. If a third realm is ever added, it must be added in + /// both places — the accepted cost of not inverting the project graph for one gate. + /// + /// + /// Existence is checked with the same ordinal, untrimmed comparison + /// AclScopePathResolver's dictionary lookups use, so a row this rule passes is a row that + /// resolver resolves. The realm check mirrors NodeAclRealms.TryCanonicalize instead + /// (trimmed, case-insensitive) for the same reason. + /// + /// + /// Containment is checked as well as existence. A grant in cluster A naming an entity that + /// belongs to cluster B is not the "names nothing" failure — it resolves to a real trie path, and + /// so it silently confers authority across a cluster boundary. It gets its own code because the + /// remedy is different: an unresolved id is a typo or a deletion, a cross-cluster id is a grant + /// authored against the wrong cluster. The lookups are deliberately fleet-wide, not + /// cluster-filtered — a per-cluster lookup would make a cross-cluster id present as merely + /// unresolved, which is the wrong diagnosis and would leave this check unable to ever fire. + /// + /// + /// Ownership is not a uniform property. RawFolder, DriverInstance and UnsArea + /// carry ClusterId; Device, TagGroup, Tag, UnsLine and + /// Equipment do not, and inherit it through their parent. Both chains are at most three + /// hops and neither can loop (TagGroup and RawFolder nest, but both reach the + /// cluster through a non-nesting link — DeviceId and ClusterId respectively), so + /// unlike the resolver's ancestry walks these need no depth guard. A broken chain means the + /// owning cluster is unknowable, which is the same state as unresolvable: it reports + /// NodeAclScopeUnresolved and never both codes. + /// + /// + private static void ValidateNodeAclScopes(DraftSnapshot draft, List errors) + { + if (draft.NodeAcls.Count == 0) return; + + // One map per ancestry link, built once for the whole pass. Their key sets double as the existence + // sets, so "does this id name a row of the kind's type" and "which cluster owns it" cannot disagree. + // GroupBy/First rather than ToDictionary: a duplicated id is another rule's business, and throwing + // here would take down every remaining check in the same pass. + var folderCluster = Link(draft.RawFolders, f => f.RawFolderId, f => f.ClusterId); + var driverCluster = Link(draft.DriverInstances, d => d.DriverInstanceId, d => d.ClusterId); + var deviceDriver = Link(draft.Devices, d => d.DeviceId, d => d.DriverInstanceId); + var groupDevice = Link(draft.TagGroups, g => g.TagGroupId, g => g.DeviceId); + var tagDevice = Link(draft.Tags, t => t.TagId, t => t.DeviceId); + var areaCluster = Link(draft.UnsAreas, a => a.UnsAreaId, a => a.ClusterId); + var lineArea = Link(draft.UnsLines, l => l.UnsLineId, l => l.UnsAreaId); + var equipLine = Link(draft.Equipment, e => e.EquipmentId, e => e.UnsLineId); + + static Dictionary Link( + IReadOnlyList rows, Func key, Func value) => + rows.GroupBy(key, StringComparer.Ordinal) + .ToDictionary(g => g.Key, g => value(g.First()), StringComparer.Ordinal); + + // Owning cluster, or null when a link in the chain is dangling. Direct-ClusterId kinds bottom out + // immediately; the rest hop through their parent. + string? ClusterOfDevice(string deviceId) => + deviceDriver.TryGetValue(deviceId, out var driverId) ? driverCluster.GetValueOrDefault(driverId) : null; + string? ClusterOfLine(string lineId) => + lineArea.TryGetValue(lineId, out var areaId) ? areaCluster.GetValueOrDefault(areaId) : null; + + foreach (var acl in draft.NodeAcls) + { + var id = acl.NodeAclId; + + // An ordinal outside the enum comes from a hand-edited row or a schema the code no longer + // matches. Every later check switches on the kind, so nothing below it would be meaningful. + if (!Enum.IsDefined(acl.ScopeKind)) + { + errors.Add(new("NodeAclScopeKindUndefined", + $"NodeAcl '{id}' has ScopeKind ordinal {(int)acl.ScopeKind}, which is not a defined " + + "NodeAclScopeKind; the row cannot be evaluated. Re-author it with a valid scope kind.", + id)); + continue; + } + + if (acl.ScopeKind == NodeAclScopeKind.Cluster) + { + // Cluster is the one kind with nothing to point at, so a ScopeId here means the author + // intended a narrower scope and picked the wrong kind — the grant is cluster-wide either + // way, i.e. broader than intended, which is the direction that must never pass silently. + if (!string.IsNullOrWhiteSpace(acl.ScopeId)) + errors.Add(new("NodeAclScopeIdUnexpected", + $"NodeAcl '{id}' is Cluster-scoped but carries ScopeId '{acl.ScopeId}'. A Cluster grant " + + "covers the whole cluster and ignores ScopeId; clear it, or change ScopeKind to the level " + + "the id names.", + id)); + continue; + } + + // Ahead of the missing-ScopeId check on purpose: the retirement is the actionable fact, and + // telling someone to supply a ScopeId for a kind that can never resolve sends them the wrong way. + if (acl.ScopeKind == NodeAclScopeKind.FolderSegment) + { + errors.Add(new("NodeAclScopeKindRetired", + $"NodeAcl '{id}' uses the retired v2 scope kind FolderSegment, which has no v3 entity behind " + + "it and can never resolve, whatever its ScopeId. Re-scope the grant to RawFolder (the v3 " + + "replacement) and point ScopeId at the RawFolderId that replaced the old folder segment.", + id)); + continue; + } + + if (string.IsNullOrWhiteSpace(acl.ScopeId)) + { + errors.Add(new("NodeAclScopeIdMissing", + $"NodeAcl '{id}' has ScopeKind {acl.ScopeKind} but no ScopeId. Only Cluster grants may omit " + + "it; every other kind resolves no scope path without one and the grant is dropped at build " + + "time. Set the ScopeId, or change ScopeKind to Cluster.", + id)); + continue; + } + + var scopeId = acl.ScopeId!; + + if (acl.ScopeKind == NodeAclScopeKind.Namespace) + { + if (CanonicalRealm(scopeId) is null) + errors.Add(new("NodeAclNamespaceInvalid", + $"NodeAcl '{id}' is Namespace-scoped but ScopeId '{scopeId}' names no OPC UA namespace. " + + $"Use '{RealmRaw}' or '{RealmUns}' (or the full URI '{V3NodeIds.RawNamespaceUri}' / " + + $"'{V3NodeIds.UnsNamespaceUri}').", + id)); + continue; + } + + // Each arm names the entity type the kind's ScopeId is drawn from, whether a row with that id + // exists at all, and which cluster owns it. Switch arms evaluate lazily, so only the matched + // kind's chain is walked. + var (entity, exists, owner) = acl.ScopeKind switch + { + NodeAclScopeKind.UnsArea => + ("UnsArea", areaCluster.ContainsKey(scopeId), areaCluster.GetValueOrDefault(scopeId)), + NodeAclScopeKind.UnsLine => + ("UnsLine", lineArea.ContainsKey(scopeId), ClusterOfLine(scopeId)), + NodeAclScopeKind.Equipment => + ("Equipment", equipLine.ContainsKey(scopeId), + equipLine.TryGetValue(scopeId, out var eqLine) ? ClusterOfLine(eqLine) : null), + NodeAclScopeKind.Tag => + ("Tag", tagDevice.ContainsKey(scopeId), + tagDevice.TryGetValue(scopeId, out var tagDev) ? ClusterOfDevice(tagDev) : null), + NodeAclScopeKind.RawFolder => + ("RawFolder", folderCluster.ContainsKey(scopeId), folderCluster.GetValueOrDefault(scopeId)), + NodeAclScopeKind.RawDriver => + ("DriverInstance", driverCluster.ContainsKey(scopeId), driverCluster.GetValueOrDefault(scopeId)), + NodeAclScopeKind.RawDevice => + ("Device", deviceDriver.ContainsKey(scopeId), ClusterOfDevice(scopeId)), + NodeAclScopeKind.RawTagGroup => + ("TagGroup", groupDevice.ContainsKey(scopeId), + groupDevice.TryGetValue(scopeId, out var grpDev) ? ClusterOfDevice(grpDev) : null), + + // Cluster / Namespace / FolderSegment all returned above; the undefined case did too. + _ => (string.Empty, true, acl.ClusterId), + }; + + if (!exists) + { + errors.Add(new("NodeAclScopeUnresolved", + $"NodeAcl '{id}' is scoped at {acl.ScopeKind} '{scopeId}', which is not a {entity} in this " + + "configuration. The grant would be dropped at build time and authorize nothing. Correct the " + + "ScopeId, or delete the grant if the entity was removed.", + id)); + continue; + } + + if (owner is null) + { + // The row exists but its parent chain is dangling, so it has no place in any cluster's tree + // and resolves to no path — the same outcome as a missing row, reported under the same code. + errors.Add(new("NodeAclScopeUnresolved", + $"NodeAcl '{id}' is scoped at {acl.ScopeKind} '{scopeId}', which exists but whose parent " + + $"chain is broken, so no cluster owns it and it resolves to no scope path. Repair the {entity}'s " + + "parent reference, or delete the grant.", + id)); + continue; + } + + if (!string.Equals(owner, acl.ClusterId, StringComparison.Ordinal)) + errors.Add(new("NodeAclScopeCrossCluster", + $"NodeAcl '{id}' is a grant in cluster '{acl.ClusterId}' but is scoped at {acl.ScopeKind} " + + $"'{scopeId}', which belongs to cluster '{owner}'. The id is real, so the grant would resolve " + + "and confer authority across a cluster boundary. Re-author the grant under cluster " + + $"'{owner}', or point ScopeId at the matching {entity} in '{acl.ClusterId}'.", + id)); + } + } + + // Realm tokens — see ValidateNodeAclScopes for why these are duplicated rather than referenced. + // Core.Authorization.NodeAclRealms is the authority; keep the two in step. + private const string RealmRaw = "raw"; + private const string RealmUns = "uns"; + + /// Mirror of NodeAclRealms.TryCanonicalize: accepts either short realm token or either + /// full namespace URI, trimmed and case-insensitive. Null when the id names no realm. + private static string? CanonicalRealm(string scopeId) + { + var trimmed = scopeId.Trim(); + + if (string.Equals(trimmed, RealmRaw, StringComparison.OrdinalIgnoreCase) + || string.Equals(trimmed, V3NodeIds.RawNamespaceUri, StringComparison.OrdinalIgnoreCase)) + return RealmRaw; + + if (string.Equals(trimmed, RealmUns, StringComparison.OrdinalIgnoreCase) + || string.Equals(trimmed, V3NodeIds.UnsNamespaceUri, StringComparison.OrdinalIgnoreCase)) + return RealmUns; + + return null; + } + /// /// The Sql driver's "a pasted literal connection string is never persisted" guarantee, enforced /// at the deploy gate (Gitea #498). diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/AclScopePathResolver.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/AclScopePathResolver.cs new file mode 100644 index 00000000..9b0d04f2 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/AclScopePathResolver.cs @@ -0,0 +1,288 @@ +using ZB.MOM.WW.OtOpcUa.Commons.Types; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; + +namespace ZB.MOM.WW.OtOpcUa.Core.Authorization; + +/// +/// Identifies a scope for path lookup. Keyed on the kind as well as the id. +/// +/// +/// The kind is part of the key because NodeAcl.ScopeId is a bare string with no foreign key +/// and no type discriminator of its own — ids are prefixed by convention (RF…, EQ…) +/// but nothing enforces it. Keying on the id alone would let a DeviceId that happened to +/// equal a UnsAreaId hand a grant the other entity's paths, which is a silent +/// wrong-node authorization rather than a visible error. +/// +/// The authored scope kind. +/// The authored scope id. +public readonly record struct AclScopeRef(NodeAclScopeKind Kind, string ScopeId); + +/// +/// Produces the scopePaths lookup consumes: for each +/// ACL scope, the trie path(s) at which a grant on it must be planted (#520 stage 3). +/// +/// +/// +/// Pure and input-shape agnostic, in the same spirit as : callers +/// build the ancestry maps from whatever they hold — EF entities on the authoring / deploy-gate +/// side, artifact JSON on the node side — so both sides produce identical paths without sharing a +/// database. That matters because a driver-only node has no ConfigDb at all (per-cluster mesh +/// Phase 4), so anything requiring a DB read node-side is disqualified outright. +/// +/// +/// Raw tag paths are delegated to rather than recomputed, so an ACL +/// path and the RawPath that forms the node's actual NodeId cannot drift apart. Container paths +/// (folder / driver / device / group) are composed here from the same chains in the same order, +/// which keeps every container path a true prefix of the tags beneath it — the property the trie +/// walk depends on for a container grant to cascade. +/// +/// +/// A broken or dangling link yields no path for that scope, never a throw: the grant is then +/// dropped by the builder with an +/// diagnostic. Fail-closed, and consistent with the resolver family's "a malformed artifact drops +/// one entry rather than faulting the whole decode" rule. +/// +/// +public sealed class AclScopePathResolver +{ + private readonly IReadOnlyDictionary _folders; + private readonly IReadOnlyDictionary _drivers; + private readonly IReadOnlyDictionary _devices; + private readonly IReadOnlyDictionary _groups; + private readonly IReadOnlyDictionary _tags; + private readonly IReadOnlyDictionary _areas; + private readonly IReadOnlyDictionary _lines; + private readonly IReadOnlyDictionary _equipment; + private readonly IReadOnlyList<(string EquipmentId, string TagId, string EffectiveName)> _references; + private readonly RawPathResolver _rawPaths; + + /// Initializes a new over the deployed topology. + /// RawFolderId → (ParentRawFolderId?, Name). + /// DriverInstanceId → (RawFolderId?, Name). + /// DeviceId → (DriverInstanceId, Name). + /// TagGroupId → (ParentTagGroupId?, DeviceId, Name). + /// TagId → (DeviceId, TagGroupId?, Name). + /// UnsAreaId → Name. + /// UnsLineId → (UnsAreaId, Name). + /// EquipmentId → (UnsLineId, Name). + /// The UNS projections: (EquipmentId, TagId, EffectiveName). + public AclScopePathResolver( + IReadOnlyDictionary folders, + IReadOnlyDictionary drivers, + IReadOnlyDictionary devices, + IReadOnlyDictionary groups, + IReadOnlyDictionary tags, + IReadOnlyDictionary areas, + IReadOnlyDictionary lines, + IReadOnlyDictionary equipment, + IReadOnlyList<(string EquipmentId, string TagId, string EffectiveName)> references) + { + ArgumentNullException.ThrowIfNull(folders); + ArgumentNullException.ThrowIfNull(drivers); + ArgumentNullException.ThrowIfNull(devices); + ArgumentNullException.ThrowIfNull(groups); + ArgumentNullException.ThrowIfNull(tags); + ArgumentNullException.ThrowIfNull(areas); + ArgumentNullException.ThrowIfNull(lines); + ArgumentNullException.ThrowIfNull(equipment); + ArgumentNullException.ThrowIfNull(references); + + _folders = folders; + _drivers = drivers; + _devices = devices; + _groups = groups; + _tags = tags; + _areas = areas; + _lines = lines; + _equipment = equipment; + _references = references; + + _rawPaths = new RawPathResolver( + folders, + drivers, + devices, + groups.ToDictionary(kv => kv.Key, kv => (kv.Value.ParentId, kv.Value.Name), StringComparer.Ordinal)); + } + + /// + /// Resolve the trie path(s) for every distinct scope named by . Scopes that + /// resolve to nothing are absent from the result, which is what makes the builder drop them. + /// + /// The ACL rows whose scopes need resolving. + /// The scopePaths lookup. + public IReadOnlyDictionary> Resolve(IReadOnlyList rows) + { + ArgumentNullException.ThrowIfNull(rows); + + var result = new Dictionary>(); + + foreach (var row in rows) + { + if (row.ScopeKind == NodeAclScopeKind.Cluster || string.IsNullOrEmpty(row.ScopeId)) continue; + + var key = new AclScopeRef(row.ScopeKind, row.ScopeId); + if (result.ContainsKey(key)) continue; + + var paths = ResolveOne(row.ScopeKind, row.ScopeId); + if (paths.Count > 0) result[key] = paths; + } + + return result; + } + + /// Resolve one scope to its trie path(s). Empty when the scope names nothing deployed. + /// The scope kind. + /// The scope id. + /// The resolved paths, possibly empty. + public IReadOnlyList ResolveOne(NodeAclScopeKind kind, string scopeId) + { + if (string.IsNullOrEmpty(scopeId)) return []; + + switch (kind) + { + case NodeAclScopeKind.Namespace: + var realm = NodeAclRealms.TryCanonicalize(scopeId); + return realm is null ? [] : [new NodeAclPath([realm])]; + + case NodeAclScopeKind.UnsArea: + return _areas.TryGetValue(scopeId, out var areaName) + ? [UnsPath([areaName])] + : []; + + case NodeAclScopeKind.UnsLine: + return TryUnsLineSegments(scopeId) is { } lineSegs ? [UnsPath(lineSegs)] : []; + + case NodeAclScopeKind.Equipment: + return TryEquipmentSegments(scopeId) is { } eqSegs ? [UnsPath(eqSegs)] : []; + + case NodeAclScopeKind.Tag: + return ResolveTag(scopeId); + + case NodeAclScopeKind.RawFolder: + return TryFolderChain(scopeId) is { } folderSegs ? [RawPath(folderSegs)] : []; + + case NodeAclScopeKind.RawDriver: + return TryDriverSegments(scopeId) is { } driverSegs ? [RawPath(driverSegs)] : []; + + case NodeAclScopeKind.RawDevice: + return TryDeviceSegments(scopeId) is { } deviceSegs ? [RawPath(deviceSegs)] : []; + + case NodeAclScopeKind.RawTagGroup: + return TryGroupSegments(scopeId) is { } groupSegs ? [RawPath(groupSegs)] : []; + + // Cluster never reaches here (the caller short-circuits it), and FolderSegment is the retired + // v2 kind with no v3 entity behind it — deliberately unresolvable, so an operator who still + // has such a row gets a diagnostic naming it rather than a grant that quietly does nothing. + case NodeAclScopeKind.Cluster: + case NodeAclScopeKind.FolderSegment: + default: + return []; + } + } + + private IReadOnlyList ResolveTag(string tagId) + { + var paths = new List(); + + if (_tags.TryGetValue(tagId, out var tag)) + { + var rawPath = _rawPaths.TryBuildTagPath(tag.DeviceId, tag.TagGroupId, tag.Name); + if (rawPath is not null) + paths.Add(RawPath(rawPath.Split(RawPaths.Separator))); + } + + // Every UNS projection of this raw tag. A tag referenced by three equipments is reachable at four + // NodeIds, and a write through any of them lands on the same driver reference — so a Tag-scoped + // grant has to cover all four or it is bypassable through whichever one it missed. + foreach (var reference in _references) + { + if (!string.Equals(reference.TagId, tagId, StringComparison.Ordinal)) continue; + if (TryEquipmentSegments(reference.EquipmentId) is not { } equipSegs) continue; + paths.Add(UnsPath([.. equipSegs, reference.EffectiveName])); + } + + return paths; + } + + private string[]? TryUnsLineSegments(string lineId) + { + if (!_lines.TryGetValue(lineId, out var line)) return null; + if (!_areas.TryGetValue(line.UnsAreaId, out var areaName)) return null; + return [areaName, line.Name]; + } + + private string[]? TryEquipmentSegments(string equipmentId) + { + if (!_equipment.TryGetValue(equipmentId, out var equip)) return null; + if (TryUnsLineSegments(equip.UnsLineId) is not { } lineSegs) return null; + return [.. lineSegs, equip.Name]; + } + + private string[]? TryFolderChain(string? leafFolderId) + { + if (leafFolderId is null) return []; + + var chain = new List(); + var current = leafFolderId; + var guard = 0; + + // Depth-bounded like RawPathResolver's own walks: this is a corrupt-parent-cycle guard, not a + // schema limit — raw folder nesting is unbounded by design. + while (current is not null) + { + if (++guard > MaxAncestryDepth) return null; + if (!_folders.TryGetValue(current, out var folder)) return null; + chain.Add(folder.Name); + current = folder.ParentId; + } + + chain.Reverse(); + return [.. chain]; + } + + private string[]? TryDriverSegments(string driverInstanceId) + { + if (!_drivers.TryGetValue(driverInstanceId, out var driver)) return null; + if (TryFolderChain(driver.RawFolderId) is not { } folderSegs) return null; + return [.. folderSegs, driver.Name]; + } + + private string[]? TryDeviceSegments(string deviceId) + { + if (!_devices.TryGetValue(deviceId, out var device)) return null; + if (TryDriverSegments(device.DriverInstanceId) is not { } driverSegs) return null; + return [.. driverSegs, device.Name]; + } + + private string[]? TryGroupSegments(string tagGroupId) + { + if (!_groups.TryGetValue(tagGroupId, out var group)) return null; + if (TryDeviceSegments(group.DeviceId) is not { } deviceSegs) return null; + + var chain = new List(); + string? current = tagGroupId; + var guard = 0; + + while (current is not null) + { + if (++guard > MaxAncestryDepth) return null; + if (!_groups.TryGetValue(current, out var node)) return null; + chain.Add(node.Name); + current = node.ParentId; + } + + chain.Reverse(); + return [.. deviceSegs, .. chain]; + } + + // No params-array overloads here: string[] binds to both params and IEnumerable, and the resulting + // ambiguity would be resolved silently by overload rules rather than by intent. + private static NodeAclPath RawPath(IEnumerable segments) => + new([NodeAclRealms.Raw, .. segments]); + + private static NodeAclPath UnsPath(IEnumerable segments) => + new([NodeAclRealms.Uns, .. segments]); + + private const int MaxAncestryDepth = 256; +} 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 2d199ea6..8ec90efe 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/DataPlaneAuthority.cs @@ -22,7 +22,8 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Authorization; public sealed class DataPlaneAuthoritySnapshot { /// The state of a node that has not yet applied an artifact. Authenticates nothing. - public static readonly DataPlaneAuthoritySnapshot Empty = Create(null, string.Empty, 0, [], []); + public static readonly DataPlaneAuthoritySnapshot Empty = + Create(null, string.Empty, 0, [], [], new Dictionary>()); private DataPlaneAuthoritySnapshot( string? clusterId, @@ -30,6 +31,7 @@ public sealed class DataPlaneAuthoritySnapshot long generationId, IReadOnlyList keys, IReadOnlyList grants, + IReadOnlyDictionary> scopePaths, IReadOnlyDictionary keysById, IReadOnlyDictionary> grantsByPrincipal) { @@ -38,6 +40,7 @@ public sealed class DataPlaneAuthoritySnapshot GenerationId = generationId; Keys = keys; Grants = grants; + ScopePaths = scopePaths; _keysById = keysById; _grantsByPrincipal = grantsByPrincipal; } @@ -66,6 +69,13 @@ public sealed class DataPlaneAuthoritySnapshot /// Every ACL grant scoped to this node's cluster. public IReadOnlyList Grants { get; } + /// + /// Where each grant's scope sits in the address space, resolved from the same artifact the grants + /// came from. Feeds ; a scope absent here has no + /// deployed node and its grant is dropped rather than planted somewhere unreachable. + /// + public IReadOnlyDictionary> ScopePaths { get; } + /// True once a real artifact has been applied. public bool HasArtifact => GenerationId > 0; @@ -101,17 +111,20 @@ public sealed class DataPlaneAuthoritySnapshot /// The node-local generation stamp. /// The API keys carried by the artifact. /// The ACL grants for this node's cluster. + /// Where each grant's scope sits in the address space. /// The built snapshot. public static DataPlaneAuthoritySnapshot Create( string? clusterId, string revisionHash, long generationId, IReadOnlyList keys, - IReadOnlyList grants) + IReadOnlyList grants, + IReadOnlyDictionary> scopePaths) { ArgumentNullException.ThrowIfNull(revisionHash); ArgumentNullException.ThrowIfNull(keys); ArgumentNullException.ThrowIfNull(grants); + ArgumentNullException.ThrowIfNull(scopePaths); // OrdinalIgnoreCase throughout: key ids are minted as lowercase hex, but a client hand-copying a // key out of the AdminUI can upper-case it, and the principal comparison must agree with the @@ -140,7 +153,8 @@ public sealed class DataPlaneAuthoritySnapshot kv => (IReadOnlyList)kv.Value, StringComparer.OrdinalIgnoreCase); - return new DataPlaneAuthoritySnapshot(clusterId, revisionHash, generationId, keys, grants, byId, frozen); + return new DataPlaneAuthoritySnapshot( + clusterId, revisionHash, generationId, keys, grants, scopePaths, byId, frozen); } } @@ -179,12 +193,14 @@ public interface IDataPlaneAuthoritySink /// SHA-256 of the artifact bytes. /// The API keys the artifact carries. /// The ACL grants for this node's cluster. + /// Where each grant's scope sits in the address space. /// The installed snapshot. DataPlaneAuthoritySnapshot Apply( string? clusterId, string revisionHash, IReadOnlyList keys, - IReadOnlyList grants); + IReadOnlyList grants, + IReadOnlyDictionary> scopePaths); } /// @@ -204,7 +220,8 @@ public sealed class DataPlaneAuthorityState : IDataPlaneAuthoritySource, IDataPl string? clusterId, string revisionHash, IReadOnlyList keys, - IReadOnlyList grants) + IReadOnlyList grants, + IReadOnlyDictionary> scopePaths) { ArgumentNullException.ThrowIfNull(revisionHash); @@ -227,7 +244,8 @@ public sealed class DataPlaneAuthorityState : IDataPlaneAuthoritySource, IDataPl ? previous.GenerationId : previous.GenerationId + 1; - var next = DataPlaneAuthoritySnapshot.Create(clusterId, revisionHash, generation, keys, grants); + var next = DataPlaneAuthoritySnapshot.Create( + clusterId, revisionHash, generation, keys, grants, scopePaths); Volatile.Write(ref _current, next); return next; } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/NodeScope.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/NodeScope.cs index 285b8835..752a77cb 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/NodeScope.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/NodeScope.cs @@ -1,47 +1,146 @@ +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; + namespace ZB.MOM.WW.OtOpcUa.Core.Authorization; /// -/// Address of a node in the 6-level scope hierarchy the Phase 6.2 evaluator walks. -/// Assembled by the dispatch layer from the node's namespace + UNS path + tag; passed -/// to which walks the matching trie path. +/// Address of a node in the ACL scope hierarchy the evaluator walks: a cluster, a realm, and the +/// node's path within that realm. Assembled by the dispatch layer from the node's NodeId; passed to +/// , which walks the matching trie path. /// /// -/// Per the Phase 6.2 Stream B plan the hierarchy is -/// Cluster → Namespace → UnsArea → UnsLine → Equipment → Tag for all -/// (Equipment-kind) namespaces. Galaxy is a standard Equipment-kind driver, so Galaxy -/// points are ordinary equipment tags that resolve through this same walk. -/// -/// Unset mid-path levels (e.g. a Cluster-scoped request with no UnsArea) leave -/// the corresponding id null. The evaluator walks as far as the scope goes + -/// stops at the first null. +/// +/// The hierarchy is Cluster → Realm → …node path…. The node path is variable length, +/// which is the whole reason this type is no longer a fixed set of level properties (#520 stage 3): +/// +/// +/// +/// UNS paths are always Area / Line / Equipment [/ EffectiveName] — fixed depth, +/// because equipment is a tree leaf. +/// +/// +/// Raw paths are folder… / Driver / Device / group… / Tag, and BOTH the folder +/// chain and the tag-group chain nest arbitrarily deep (RawFolder.ParentRawFolderId, +/// TagGroup.ParentTagGroupId). There is no schema depth cap — the 256 in +/// RawPathResolver is a corrupt-cycle guard, not a limit. A fixed-arity scope record +/// therefore cannot address a real raw tree at all; the previous six-property shape silently +/// capped it at five levels and mislabelled every one of them. +/// +/// +/// +/// The realm segment is NOT carried here. It is derived from by +/// , which the scope-path producer also uses — so the two +/// sides cannot disagree about its spelling. That matters more than it looks: the realm is a hard +/// gate at level 1 of the walk, so a mismatch would make every sub-cluster grant in the system +/// unreachable while cluster-scoped grants kept working, which reads as "ACLs work" right up until +/// someone tries to narrow one. +/// /// public sealed record NodeScope { /// Cluster the node belongs to. Required. public required string ClusterId { get; init; } - /// Namespace within the cluster. Null is not allowed for a request against a real node. - public string? NamespaceId { get; init; } - - /// UNS area (e.g. "warsaw-west") below the namespace. - public string? UnsAreaId { get; init; } - - /// UNS line below the area. - public string? UnsLineId { get; init; } - - /// Equipment row below the line. - public string? EquipmentId { get; init; } - - /// Target tag id when the scope addresses a specific tag; null for equipment-level scopes. - public string? TagId { get; init; } - - /// Which hierarchy applies. Currently only Equipment-kind (UNS) exists. + /// Which realm — and therefore which hierarchy shape — the node lives in. public required NodeHierarchyKind Kind { get; init; } + + /// + /// The node's path within its realm, root-first, excluding the realm segment itself. Empty + /// addresses the realm as a whole. Segment comparison is case-sensitive ordinal, matching + /// RawPaths.Comparer and OPC UA NodeId identity. + /// + public required IReadOnlyList Segments { get; init; } } -/// Selector for the scope-hierarchy shape. +/// Selector for the scope-hierarchy shape — one member per OPC UA namespace (realm). public enum NodeHierarchyKind { - /// Cluster → Namespace → UnsArea → UnsLine → Equipment → Tag — UNS / Equipment kind. - Equipment, + /// Area / Line / Equipment [/ EffectiveName] — the UNS realm. + Uns, + + /// folder… / Driver / Device / group… / Tag — the raw device-tree realm. (Appended, #520 stage 3.) + Raw, +} + +/// +/// The single authority for the realm segment that sits at level 1 of every permission trie, directly +/// under the cluster root. +/// +/// +/// +/// This exists because v3 has no Namespace entity. The snapshot was retired when the +/// address space went dual-namespace ("the two OPC UA namespaces are implicit"), which left the +/// trie's level-1 gate with no configured key source at all — the pre-stage-3 walker read a +/// NamespaceId that nothing in the system ever produced, so the level could only ever fail +/// to match. Deriving the segment from the realm restores the level as something real without +/// reintroducing an entity. +/// +/// +/// The short tokens, rather than the namespace URIs, are deliberate: they are what an operator +/// types into a row's ScopeId, +/// and they read clearly in a diagnostic. They cannot collide with a user-authored segment, +/// because only a realm token is ever placed at level 1 — real names start at level 2. +/// +/// +public static class NodeAclRealms +{ + /// Realm segment for the raw device tree (https://zb.com/otopcua/raw). + public const string Raw = "raw"; + + /// Realm segment for the UNS tree (https://zb.com/otopcua/uns). + public const string Uns = "uns"; + + /// The realm segment for a hierarchy kind. + /// The hierarchy kind. + /// The level-1 trie segment. + /// The kind is not a defined member. + public static string SegmentFor(NodeHierarchyKind kind) => kind switch + { + NodeHierarchyKind.Uns => Uns, + NodeHierarchyKind.Raw => Raw, + _ => throw new ArgumentOutOfRangeException(nameof(kind), kind, "Unhandled hierarchy kind."), + }; + + /// The realm segment for an address-space realm. + /// The address-space realm carried at the OPC UA sinks. + /// The level-1 trie segment. + /// The realm is not a defined member. + public static string SegmentFor(AddressSpaceRealm realm) => realm switch + { + AddressSpaceRealm.Uns => Uns, + AddressSpaceRealm.Raw => Raw, + _ => throw new ArgumentOutOfRangeException(nameof(realm), realm, "Unhandled address-space realm."), + }; + + /// + /// True when is exactly a canonical realm segment. Used by the trie + /// builder to reject a scope path that does not start at a realm — such a path addresses a node + /// the walk cannot reach, so the grant would look planted and never match. + /// + /// The candidate segment. + /// when it is a realm segment. + public static bool IsRealmSegment(string? segment) => + string.Equals(segment, Raw, StringComparison.Ordinal) + || string.Equals(segment, Uns, StringComparison.Ordinal); + + /// + /// Canonicalizes an operator-supplied realm token from a Namespace-scoped row's + /// ScopeId. Accepts the short token or the full namespace URI, in any case. + /// + /// The authored scope id. + /// The canonical realm segment, or when it names no realm. + public static string? TryCanonicalize(string? scopeId) + { + if (string.IsNullOrWhiteSpace(scopeId)) return null; + var trimmed = scopeId.Trim(); + + if (string.Equals(trimmed, Raw, StringComparison.OrdinalIgnoreCase) + || string.Equals(trimmed, V3NodeIds.RawNamespaceUri, StringComparison.OrdinalIgnoreCase)) + return Raw; + + if (string.Equals(trimmed, Uns, StringComparison.OrdinalIgnoreCase) + || string.Equals(trimmed, V3NodeIds.UnsNamespaceUri, StringComparison.OrdinalIgnoreCase)) + return Uns; + + return null; + } } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs index d12ff13b..2ad1666c 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrie.cs @@ -3,17 +3,16 @@ using ZB.MOM.WW.OtOpcUa.Configuration.Enums; namespace ZB.MOM.WW.OtOpcUa.Core.Authorization; /// -/// In-memory permission trie for one (ClusterId, GenerationId). Walk from the cluster -/// root down through namespace → UNS levels → tag, OR-ing the +/// In-memory permission trie for one (ClusterId, GenerationId). Walk from the cluster root +/// down through the realm and then each segment of the node's path, OR-ing the /// granted at each visited level for each of the session's -/// principals. The accumulated bitmask is compared to the permission required by the -/// requested . +/// principals. The accumulated bitmask is compared to the permission required by the requested +/// . /// /// -/// The walk is pure union (additive grants; no explicit Deny in v2.0): -/// encountering a grant at any level contributes its flags, never revokes them. A grant at -/// the Cluster root therefore cascades to every tag below it; a grant at a deep equipment -/// leaf is visible only on that equipment subtree. +/// The walk is pure union (additive grants; no explicit Deny in v2.0): encountering a grant at any +/// level contributes its flags, never revokes them. A grant at the cluster root therefore cascades to +/// every node below it; a grant at a deep leaf is visible only on that subtree. /// public sealed class PermissionTrie { @@ -27,9 +26,9 @@ public sealed class PermissionTrie public PermissionTrieNode Root { get; init; } = new(); /// - /// Walk the trie collecting grants that apply to for any of the - /// session's . Returns the matched-grant list; the caller - /// OR-s the flag bits to decide whether the requested permission is carried. + /// Walk the trie collecting grants that apply to for any of the session's + /// . Returns the matched-grant list; the caller OR-s the flag bits to + /// decide whether the requested permission is carried. /// /// The node scope to match permissions for. /// The principal names carried by the session (API-key principals, or the reserved "anonymous"). @@ -44,47 +43,42 @@ public sealed class PermissionTrie var matches = new List(); - // Level 0 — cluster-scoped grants. - CollectAtLevel(Root, NodeAclScopeKind.Cluster, groups, matches); + // Level 0 — cluster-scoped grants apply to every node in the cluster. + CollectAtLevel(Root, groups, matches); - // Level 1 — namespace. - if (scope.NamespaceId is null) return matches; - if (!Root.Children.TryGetValue(scope.NamespaceId, out var ns)) return matches; - CollectAtLevel(ns, NodeAclScopeKind.Namespace, groups, matches); + // Level 1 — the realm. Derived from the scope's kind rather than carried on it, so this can + // never disagree with the spelling the scope-path producer used. SegmentFor throws on an + // undefined kind, which is deliberate: a new realm that nobody taught this walk about must fail + // loudly rather than walk some other realm's subtree and return a confidently wrong verdict. + var realm = NodeAclRealms.SegmentFor(scope.Kind); + if (!Root.Children.TryGetValue(realm, out var node)) return matches; + CollectAtLevel(node, groups, matches); - // Below the namespace every kind walks the Equipment hierarchy — Galaxy is a standard - // Equipment-kind driver, so Galaxy points are ordinary equipment tags keyed by - // EquipmentId/path and resolve through this same walk. - WalkEquipment(ns, scope, groups, matches); + // Levels 2..n — the node's path within the realm, however deep it happens to be. There is no + // per-kind ladder here on purpose: the UNS path (Area/Line/Equipment) and a raw path + // (folder…/Driver/Device/group…/Tag) are both just ordered segments, and raw nesting is + // unbounded, so any fixed ladder would silently truncate deep raw trees. + foreach (var segment in scope.Segments) + { + if (segment is null || !node.Children.TryGetValue(segment, out var next)) return matches; + node = next; + CollectAtLevel(node, groups, matches); + } return matches; } - private static void WalkEquipment(PermissionTrieNode ns, NodeScope scope, HashSet groups, List matches) - { - if (scope.UnsAreaId is null) return; - if (!ns.Children.TryGetValue(scope.UnsAreaId, out var area)) return; - CollectAtLevel(area, NodeAclScopeKind.UnsArea, groups, matches); - - if (scope.UnsLineId is null) return; - if (!area.Children.TryGetValue(scope.UnsLineId, out var line)) return; - CollectAtLevel(line, NodeAclScopeKind.UnsLine, groups, matches); - - if (scope.EquipmentId is null) return; - if (!line.Children.TryGetValue(scope.EquipmentId, out var eq)) return; - CollectAtLevel(eq, NodeAclScopeKind.Equipment, groups, matches); - - if (scope.TagId is null) return; - if (!eq.Children.TryGetValue(scope.TagId, out var tag)) return; - CollectAtLevel(tag, NodeAclScopeKind.Tag, groups, matches); - } - - private static void CollectAtLevel(PermissionTrieNode node, NodeAclScopeKind level, HashSet groups, List matches) + private static void CollectAtLevel(PermissionTrieNode node, HashSet groups, List matches) { foreach (var grant in node.Grants) { + // Provenance is the granting ROW's own scope kind, recorded when the trie was built — not the + // structural depth the grant happens to sit at. Depth-derived provenance was wrong in two + // directions: it could not name a raw level at all (the depth of "Device" varies with folder + // nesting), and where a producer emitted a path whose length disagreed with the row's kind it + // reported a level the operator never authored. if (groups.Contains(grant.Principal)) - matches.Add(new MatchedGrant(grant.Principal, level, grant.PermissionFlags)); + matches.Add(new MatchedGrant(grant.Principal, grant.ScopeKind, grant.PermissionFlags)); } } } @@ -96,12 +90,21 @@ public sealed class PermissionTrieNode public List Grants { get; } = []; /// - /// Children keyed by the next level's id — namespace id under cluster; UnsAreaId or - /// folder-segment name under namespace; etc. Comparer is OrdinalIgnoreCase so the walk - /// tolerates case drift between the NodeAcl row and the requested scope. + /// Children keyed by the next level's segment — the realm token under the root, then entity names. /// - public Dictionary Children { get; } = new(StringComparer.OrdinalIgnoreCase); + /// + /// ⚠️ The comparer is case-sensitive ordinal, matching RawPaths.Comparer and OPC UA + /// NodeId identity. It was OrdinalIgnoreCase before #520 stage 3, which was a silent + /// widening: raw sibling names are unique under an ordinal comparer, so Speed and + /// speed can both exist under one device, and a grant on either would have matched both. + /// Case-sensitive matching is also the fail-closed direction — a case-drifted ACL row now grants + /// nothing instead of granting something adjacent. + /// + public Dictionary Children { get; } = new(StringComparer.Ordinal); } /// Projection of a row into the trie. -public sealed record TrieGrant(string Principal, NodePermissions PermissionFlags); +/// The principal the row grants to. +/// The row's authored scope kind, carried for decision provenance. +/// The permission bits the row grants. +public sealed record TrieGrant(string Principal, NodeAclScopeKind ScopeKind, NodePermissions PermissionFlags); diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrieBuilder.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrieBuilder.cs index 60783693..003974c4 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrieBuilder.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/PermissionTrieBuilder.cs @@ -4,107 +4,116 @@ using ZB.MOM.WW.OtOpcUa.Configuration.Enums; namespace ZB.MOM.WW.OtOpcUa.Core.Authorization; /// -/// Builds a from a set of rows anchored -/// in one generation. The trie is keyed on the rows' scope hierarchy — rows with -/// land at the trie root, rows with -/// land at a leaf, etc. +/// Builds a from a set of rows anchored in one +/// generation. Rows with land at the trie root; every other +/// row is placed at the trie node(s) its ScopeId resolves to. /// /// -/// Intended to be called by once per published -/// generation; the resulting trie is immutable for the life of the cache entry. Idempotent — -/// two builds from the same rows produce equal tries (grant lists may be in insertion order; -/// evaluators don't depend on order). -/// -/// The builder deliberately does not know about the node-row metadata the trie path -/// will be walked with. The caller assembles values from the live -/// config (UnsArea parent of UnsLine, etc.); this class only honors the ScopeId -/// each row carries. +/// +/// Intended to be called once per published generation; the resulting trie is immutable for the +/// life of the cache entry. Idempotent — two builds from the same rows produce equal tries. +/// +/// +/// ⚠️ There is no longer a "deterministic test mode". Before #520 stage 3 a null +/// scopePaths made sub-cluster rows fall back to a direct child of the trie +/// root keyed on ScopeId. That position is unreachable by construction — level 1 of the walk +/// is the realm — so the fallback produced grants that looked placed and could never match, and +/// every test that used it proved nothing about production placement. A row that cannot be +/// resolved is now dropped with a diagnostic, which is the fail-closed direction and is +/// honest about what happened. +/// /// public static class PermissionTrieBuilder { /// - /// Build a trie for one cluster/generation from the supplied rows. The caller is - /// responsible for pre-filtering rows to the target generation + cluster. + /// Build a trie for one cluster/generation from the supplied rows. The caller is responsible for + /// pre-filtering rows to the target generation + cluster. /// /// Cluster the trie is being built for; rows for other clusters are skipped. /// Config-generation the rows belong to; stamped on the returned trie. /// ACL rows for this cluster + generation. /// - /// Optional ScopeId → multi-level trie-path lookup. When supplied, sub-cluster rows - /// descend to their structurally-correct trie node. When null, sub-cluster rows fall back - /// to a direct child of the trie root keyed on ScopeId — deterministic-test mode. + /// (ScopeKind, ScopeId) → the trie path(s) that scope resolves to, as produced by + /// . A scope resolves to several paths when one entity is + /// reachable at several places in the address space — a raw tag projected into N equipments by + /// UnsTagReference rows has one raw path plus N UNS paths, and a grant on it must cover all + /// of them or it would be bypassable through the NodeId it missed. /// /// - /// Optional callback invoked when a sub-cluster row's ScopeId cannot be located - /// in . Production callers should wire a logger here so - /// orphaned grants surface — silently dropping them under the wrong trie level was a - /// production hazard. The callback fires only when - /// is non-null (a null lookup is the explicit deterministic-test fallback mode). + /// Optional callback invoked for every row that could not be placed. Production callers should wire + /// a logger here — a dropped grant is invisible otherwise. /// /// The built for the cluster + generation. public static PermissionTrie Build( string clusterId, long generationId, IReadOnlyList rows, - IReadOnlyDictionary? scopePaths = null, + IReadOnlyDictionary> scopePaths, Action? diagnostic = null) { ArgumentException.ThrowIfNullOrWhiteSpace(clusterId); ArgumentNullException.ThrowIfNull(rows); + ArgumentNullException.ThrowIfNull(scopePaths); var trie = new PermissionTrie { ClusterId = clusterId, GenerationId = generationId }; foreach (var row in rows) { if (!string.Equals(row.ClusterId, clusterId, StringComparison.OrdinalIgnoreCase)) continue; - var grant = new TrieGrant(row.Principal, row.PermissionFlags); - var node = row.ScopeKind switch + var grant = new TrieGrant(row.Principal, row.ScopeKind, row.PermissionFlags); + + if (row.ScopeKind == NodeAclScopeKind.Cluster) { - NodeAclScopeKind.Cluster => trie.Root, - _ => Descend(trie.Root, row, scopePaths, diagnostic), - }; + trie.Root.Grants.Add(grant); + continue; + } - if (node is not null) + // A sub-cluster row with no ScopeId names nothing. It used to vanish here with no signal at + // all — unlike an unresolvable ScopeId, which at least emitted a diagnostic. Both are + // authoring mistakes and both now say so. + if (string.IsNullOrEmpty(row.ScopeId)) + { + diagnostic?.Invoke(new PermissionTrieBuildDiagnostic( + row.NodeAclId, row.ScopeKind, ScopeId: null, + PermissionTrieBuildDiagnosticReason.MissingScopeId)); + continue; + } + + if (!scopePaths.TryGetValue(new AclScopeRef(row.ScopeKind, row.ScopeId), out var paths) + || paths.Count == 0) + { + diagnostic?.Invoke(new PermissionTrieBuildDiagnostic( + row.NodeAclId, row.ScopeKind, row.ScopeId, + PermissionTrieBuildDiagnosticReason.UnresolvedScope)); + continue; + } + + foreach (var path in paths) + { + // Guard the producer's output contract. Level 1 of the walk is always a realm token, so a + // path that is empty or does not start with one addresses a node the walker can never + // reach — the grant would look planted and could never match. That is the same silent + // failure the deleted root-child fallback used to produce; without this check it simply + // moves into the resolver, where nothing else cross-checks it. + if (path.Segments.Count == 0 || !NodeAclRealms.IsRealmSegment(path.Segments[0])) + { + diagnostic?.Invoke(new PermissionTrieBuildDiagnostic( + row.NodeAclId, row.ScopeKind, row.ScopeId, + PermissionTrieBuildDiagnosticReason.InvalidScopePath)); + continue; + } + + var node = trie.Root; + foreach (var segment in path.Segments) + node = EnsureChild(node, segment); node.Grants.Add(grant); + } } return trie; } - private static PermissionTrieNode? Descend( - PermissionTrieNode root, - NodeAcl row, - IReadOnlyDictionary? scopePaths, - Action? diagnostic) - { - if (string.IsNullOrEmpty(row.ScopeId)) return null; - - // For sub-cluster scopes the caller supplies a path lookup so we know the containing - // namespace / UnsArea / UnsLine ids. Without a path lookup we fall back to putting the - // row directly under the root using its ScopeId — works for deterministic tests, not - // for production where the hierarchy must be honored. If a scopePaths lookup IS - // provided but is missing the row's ScopeId, surface a diagnostic so the caller can - // log the orphan instead of silently dropping the grant under an unreachable node. - if (scopePaths is null || !scopePaths.TryGetValue(row.ScopeId, out var path)) - { - if (scopePaths is not null) - { - diagnostic?.Invoke(new PermissionTrieBuildDiagnostic( - NodeAclId: row.NodeAclId, - ScopeKind: row.ScopeKind, - ScopeId: row.ScopeId, - Reason: PermissionTrieBuildDiagnosticReason.MissingScopePath)); - } - return EnsureChild(root, row.ScopeId); - } - - var node = root; - foreach (var segment in path.Segments) - node = EnsureChild(node, segment); - return node; - } - private static PermissionTrieNode EnsureChild(PermissionTrieNode parent, string key) { if (!parent.Children.TryGetValue(key, out var child)) @@ -117,37 +126,41 @@ public static class PermissionTrieBuilder } /// -/// Ordered list of trie-path segments from root to the target node. Supplied to -/// so the builder knows where a -/// -scoped row sits in the hierarchy. +/// Ordered list of trie-path segments from the cluster root to the target node, including the +/// realm segment at index 0 (see ). /// -/// -/// Namespace id, then UnsAreaId / UnsLineId / EquipmentId / TagId as applicable. -/// +/// Realm token, then the entity names that lead to the scoped node. public sealed record NodeAclPath(IReadOnlyList Segments); /// -/// Diagnostic emitted by when a row could not be -/// placed at its structurally-correct trie node. Production callers should log these so -/// orphaned grants surface instead of being silently dropped under an unreachable node. +/// Diagnostic emitted by for a row that could not be placed. +/// Production callers should log these so dropped grants surface to an operator. /// /// The offending row's logical id. /// The row's . -/// The row's ScopeId that could not be located. +/// The row's ScopeId, or when it had none. /// Why the diagnostic fired. public sealed record PermissionTrieBuildDiagnostic( string NodeAclId, NodeAclScopeKind ScopeKind, - string ScopeId, + string? ScopeId, PermissionTrieBuildDiagnosticReason Reason); -/// Reasons can be emitted. +/// Reasons can be emitted. The grant is dropped in every case. public enum PermissionTrieBuildDiagnosticReason { /// - /// The row's ScopeId was not present in the supplied scopePaths lookup. - /// The grant is placed as a direct child of the trie root keyed on ScopeId — a - /// position the production trie walker cannot reach for multi-level scopes. + /// The row's ScopeId did not resolve to any node in the deployed address space — a dangling + /// id, a deleted entity, or a scope kind with no v3 meaning (). /// - MissingScopePath, + UnresolvedScope, + + /// The row is narrower than but carries no ScopeId. + MissingScopeId, + + /// + /// A resolved path was empty or did not begin with a realm segment, so it addresses a trie node + /// the walk cannot reach. This is a bug in the scope-path producer, not an authoring mistake. + /// + InvalidScopePath, } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/AclEdit.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/AclEdit.razor index 7061d22a..8eb24569 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/AclEdit.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/AclEdit.razor @@ -29,15 +29,28 @@ else if (!IsNew && _existing is null) else { @@ -54,7 +67,7 @@ else
+ placeholder="scadabridge-dcl" />
@@ -66,14 +79,34 @@ else - + + + + +
+
+ Free text, no picker — a typo saves a grant that resolves to nothing. + Cluster → leave blank · + Namespace → raw or + uns · + UnsArea → a UnsAreaId · + UnsLine → a UnsLineId · + Equipment → an EquipmentId · + Tag → a TagId (a raw tag; the grant covers the raw + node and every UNS projection of it) · + RawFolder → a RawFolderId · + RawDriver → a DriverInstanceId · + RawDevice → a DeviceId · + RawTagGroup → a TagGroupId · + FolderSegment → retired v2 kind, resolves nothing. +
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/ClusterAcls.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/ClusterAcls.razor index f7179c5f..4fc22d23 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/ClusterAcls.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Clusters/ClusterAcls.razor @@ -20,29 +20,42 @@ else {
ACL rows grant principals specific NodePermissions on a scope - (a folder, an equipment, a tag). Per-cluster role grants were dropped in favour of - fleet-wide LDAP-group → role mapping; ACLs here are the finer-grained per-node scope. + (the cluster, a namespace, an area, an equipment, a raw device, a tag). A principal is an API + key's principal, or the reserved anonymous principal that an + anonymous session authenticates as.
@_rows.Count ACL row@(_rows.Count == 1 ? "" : "s")
@if (_rows.Count == 0) { -
No ACL rows for this cluster — default permissions from the fleet-wide LDAP group mapping apply.
+
No ACL rows for this cluster — no principal holds WriteOperate or AlarmAck here, so writes and alarm acknowledges are refused. Reads, browses, subscriptions and history reads are unaffected.
} else { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ArtifactApiKeyStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ArtifactApiKeyStore.cs index 477bda4f..07b0b023 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ArtifactApiKeyStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ArtifactApiKeyStore.cs @@ -61,10 +61,20 @@ public sealed class ArtifactApiKeyStore : IApiKeyStore Task.FromResult(ClusterGrants(_authority.Current, principal)); /// - /// Narrows a principal's grants to the Cluster-scoped ones, matching ConfigDbApiKeyStore. - /// Only those confer an interim role (see ); stage 5 widens - /// both stores to the full grant set once scope is actually enforced. + /// Narrows a principal's grants to the Cluster-scoped ones. Only those confer an interim role + /// (see ); stage 5 widens both stores to the full grant set + /// once scope is actually enforced. /// + /// + /// Scope kind is the ONLY narrowing needed here — cluster confinement already happened. + /// DeploymentArtifact.ParseDataPlaneAuthority kept only the node's own cluster's ACL rows + /// (ClusterFilterMode.ScopeTo, matched ), + /// so this snapshot cannot contain another cluster's grant. ConfigDbApiKeyStore reads the + /// fleet-wide ConfigDb and therefore has to apply that confinement itself; the two stores agree on + /// the outcome, not on the mechanism. An earlier version of this comment claimed + /// ConfigDbApiKeyStore "filters the same way in SQL" — it did not filter on cluster at all, + /// which was the cross-cluster over-grant this remark now exists to keep straight. + /// private static IReadOnlyList ClusterGrants(DataPlaneAuthoritySnapshot snapshot, string principal) { var all = snapshot.GrantsFor(principal); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs index 0c55c575..0b3456ad 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/ConfigDbApiKeyStore.cs @@ -1,4 +1,5 @@ using Microsoft.EntityFrameworkCore; +using ZB.MOM.WW.OtOpcUa.Cluster; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; @@ -16,6 +17,13 @@ namespace ZB.MOM.WW.OtOpcUa.Host.OpcUa; /// Reading per request is what makes revocation immediate here, which is why this store is /// preferred wherever a ConfigDb exists — including on a fused admin+driver node. /// +/// +/// ⚠️ The ConfigDb holds the WHOLE fleet's ACL rows, not just this node's cluster's. That is +/// the difference from , whose snapshot was already narrowed to +/// the node's own cluster by DeploymentArtifact.ParseDataPlaneAuthority. So this store must +/// confine grants to itself — without that predicate a Cluster-scoped +/// grant authored in ANY cluster in the fleet conferred an interim server-wide role here. +/// /// public sealed class ConfigDbApiKeyStore : IApiKeyStore { @@ -23,7 +31,57 @@ public sealed class ConfigDbApiKeyStore : IApiKeyStore /// Initializes a new instance of the class. /// The config DB context factory. - public ConfigDbApiKeyStore(IDbContextFactory factory) => _factory = factory; + /// + /// This node's own ClusterId, from . on a + /// legacy / single-cluster node that carries no cluster-{ClusterId} role — see + /// for what that means for filtering. + /// + public ConfigDbApiKeyStore(IDbContextFactory factory, string? clusterId) + { + ArgumentNullException.ThrowIfNull(factory); + _factory = factory; + ClusterId = string.IsNullOrWhiteSpace(clusterId) ? null : clusterId; + } + + /// + /// The node's own ClusterId, or when it carries no cluster-scoped role. + /// + /// + /// A ClusterId applies NO cluster filter, mirroring the artifact path's + /// ClusterFilterMode.None — a single-cluster artifact is applied whole. Failing closed here + /// instead would silently strip every grant on an existing single-cluster deployment, which is a + /// worse failure than the one being fixed and would not be visible until a client was denied. + /// + public string? ClusterId { get; } + + /// + /// Resolves the node's ClusterId from its OWN configured roles, e.g. cluster-SITE-A → + /// SITE-A; when no role is cluster-scoped. + /// + /// + /// + /// This is the same derivation performs for + /// IClusterRoleInfo.ClusterId, reusing the same primitives — + /// first cluster-scoped role wins, as it does there. + /// + /// + /// ⚠️ Takes (pure configuration), NOT + /// IClusterRoleInfo resolved from the service provider. ClusterRoleInfo + /// depends on the ActorSystem; resolving it during host composition recurses into + /// ActorSystem construction and stack-overflows the node at boot while compiling clean (see + /// CLAUDE.md, per-cluster mesh Phase 6). The ClusterId is configuration, so read it from + /// configuration. + /// + /// + /// The node's bound cluster options. + /// The node's ClusterId, or when it carries no cluster-scoped role. + public static string? ResolveClusterId(AkkaClusterOptions options) + { + ArgumentNullException.ThrowIfNull(options); + + var clusterRole = options.Roles.FirstOrDefault(RoleParser.IsClusterRole); + return clusterRole is null ? null : RoleParser.ClusterIdFromRole(clusterRole); + } /// public async Task FindAsync(string apiKeyId, DateTime utcNow, CancellationToken ct) @@ -53,15 +111,33 @@ public sealed class ConfigDbApiKeyStore : IApiKeyStore return await LoadClusterGrantsAsync(db, principal, ct).ConfigureAwait(false); } - private static async Task> LoadClusterGrantsAsync( + private async Task> LoadClusterGrantsAsync( OtOpcUaConfigDbContext db, string principal, CancellationToken ct) { // Only cluster-scoped rows can confer an interim role (see ApiKeyInterimRoleMapper), so only // those are fetched. Stage 5 widens this to the full grant set for per-node evaluation. - return await db.NodeAcls + var rows = await db.NodeAcls .AsNoTracking() .Where(a => a.Principal == principal && a.ScopeKind == NodeAclScopeKind.Cluster) .ToListAsync(ct) .ConfigureAwait(false); + + if (ClusterId is null) return rows; + + // The ClusterId match is done HERE rather than as a SQL predicate, unlike the Enabled/expiry + // filters above, because it must be case-insensitive DETERMINISTICALLY. `a.ClusterId == ...` in + // SQL is decided by the database collation — case-insensitive on the usual SQL Server default, + // case-SENSITIVE on a CS collation or on the in-memory provider the tests use — and the two + // spellings genuinely differ: `Cluster:Roles` binds a role verbatim (cluster-SITE-A ⇒ SITE-A) + // while the OTOPCUA_ROLES path goes through RoleParser.Parse, which lower-cases it (⇒ site-a). + // A collation-dependent match would therefore strip every grant on a node configured the second + // way. OrdinalIgnoreCase is the SAME comparison DeploymentArtifact.ParseDataPlaneAuthority uses + // to scope the artifact's grants, so both stores confine identically. + var mine = new List(rows.Count); + foreach (var grant in rows) + { + if (string.Equals(grant.ClusterId, ClusterId, StringComparison.OrdinalIgnoreCase)) mine.Add(grant); + } + return mine; } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs index f098d719..0305c222 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs @@ -376,7 +376,15 @@ if (hasDriver) // A node WITH a ConfigDb reads keys straight from it, which is what makes revocation take effect // on the very next request rather than on the next deployment. Preferred wherever it is available, // including on a fused admin+driver node. - builder.Services.AddSingleton(); + // The ClusterId confines the grants this node honours to its OWN cluster — the ConfigDb holds + // the whole fleet's ACL rows, unlike the artifact a driver-only node reads, which + // ParseDataPlaneAuthority already scoped. Taken from AkkaClusterOptions (pure configuration), + // NOT from IClusterRoleInfo: that depends on the ActorSystem, and the same options-not-services + // rule is what keeps the redundancy-singleton registration below from recursing into ActorSystem + // construction. Null on a node with no cluster-{ClusterId} role ⇒ no filter (legacy behaviour). + builder.Services.AddSingleton(sp => new ConfigDbApiKeyStore( + sp.GetRequiredService>(), + ConfigDbApiKeyStore.ResolveClusterId(sp.GetRequiredService>().Value))); builder.Services.AddSingleton(); // Minting surface for the AdminUI. Same node condition as the store above: both need a ConfigDb. 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 a385b468..6e2d199c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs @@ -4,6 +4,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; using ZB.MOM.WW.OtOpcUa.OpcUaServer; namespace ZB.MOM.WW.OtOpcUa.Runtime.Drivers; @@ -46,13 +47,22 @@ public enum ClusterFilterMode /// lookup filters those, so that a revocation is visible rather than indistinguishable from a /// missing key. /// The NodeAcl rows scoped to . +/// +/// Where each grant's scope sits in the address space, resolved from the artifact's own topology. +/// This is why the node needs no ConfigDb to build a permission trie: the artifact already carries +/// every raw and UNS parent link, so the paths are derivable from the same bytes the grants came in. +/// A scope that resolves to nothing is absent here, and the trie builder drops its grant with a +/// diagnostic rather than planting it somewhere unreachable. +/// public sealed record DataPlaneAuthorityProjection( string? ClusterId, IReadOnlyList Keys, - IReadOnlyList Grants) + IReadOnlyList Grants, + IReadOnlyDictionary> ScopePaths) { /// The projection of an absent or unparseable artifact: no cluster, no keys, no grants. - public static readonly DataPlaneAuthorityProjection Empty = new(null, [], []); + public static readonly DataPlaneAuthorityProjection Empty = + new(null, [], [], new Dictionary>()); } /// Resolved scoping decision for a node against an artifact. @@ -610,7 +620,8 @@ public static class DeploymentArtifact grants.Add(acl); } - return new DataPlaneAuthorityProjection(scope.ClusterId, keys, grants); + return new DataPlaneAuthorityProjection( + scope.ClusterId, keys, grants, BuildAclScopePaths(root, grants)); } catch (JsonException) { @@ -618,6 +629,109 @@ public static class DeploymentArtifact } } + /// + /// Resolve every scope named by against the artifact's own raw + UNS + /// topology, using the same RawPathResolver the address space is built through — so an ACL + /// path and the RawPath that forms a node's NodeId cannot drift apart. + /// + private static IReadOnlyDictionary> BuildAclScopePaths( + JsonElement root, + IReadOnlyList grants) + { + if (grants.Count == 0) return new Dictionary>(); + + var folders = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "RawFolders")) + { + var id = ReadString(el, "RawFolderId"); + if (id is null) continue; + folders[id] = (ReadNullableString(el, "ParentRawFolderId"), ReadString(el, "Name") ?? id); + } + + var drivers = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "DriverInstances")) + { + var id = ReadString(el, "DriverInstanceId"); + if (id is null) continue; + drivers[id] = (ReadNullableString(el, "RawFolderId"), ReadString(el, "Name") ?? id); + } + + var devices = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "Devices")) + { + var id = ReadString(el, "DeviceId"); + var driverId = ReadString(el, "DriverInstanceId"); + if (id is null || driverId is null) continue; + devices[id] = (driverId, ReadString(el, "Name") ?? id); + } + + var groups = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "TagGroups")) + { + var id = ReadString(el, "TagGroupId"); + var deviceId = ReadString(el, "DeviceId"); + if (id is null || deviceId is null) continue; + groups[id] = (ReadNullableString(el, "ParentTagGroupId"), deviceId, ReadString(el, "Name") ?? id); + } + + var tags = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "Tags")) + { + var id = ReadString(el, "TagId"); + var deviceId = ReadString(el, "DeviceId"); + if (id is null || deviceId is null) continue; + tags[id] = (deviceId, ReadNullableString(el, "TagGroupId"), ReadString(el, "Name") ?? id); + } + + var areas = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "UnsAreas")) + { + var id = ReadString(el, "UnsAreaId"); + if (id is null) continue; + areas[id] = ReadString(el, "Name") ?? id; + } + + var lines = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "UnsLines")) + { + var id = ReadString(el, "UnsLineId"); + var areaId = ReadString(el, "UnsAreaId"); + if (id is null || areaId is null) continue; + lines[id] = (areaId, ReadString(el, "Name") ?? id); + } + + var equipment = new Dictionary(StringComparer.Ordinal); + foreach (var el in EnumerateArray(root, "Equipment")) + { + var id = ReadString(el, "EquipmentId"); + var lineId = ReadString(el, "UnsLineId"); + if (id is null || lineId is null) continue; + equipment[id] = (lineId, ReadString(el, "Name") ?? id); + } + + var references = new List<(string EquipmentId, string TagId, string EffectiveName)>(); + foreach (var el in EnumerateArray(root, "UnsTagReferences")) + { + var equipmentId = ReadString(el, "EquipmentId"); + var tagId = ReadString(el, "TagId"); + if (equipmentId is null || tagId is null) continue; + + // Effective name = the override when present, else the backing raw tag's own name. This is the + // same rule the address space materializes UNS variables under; a reference whose tag is + // missing from the artifact has no resolvable name and is skipped. + var effective = ReadNullableString(el, "DisplayNameOverride"); + if (string.IsNullOrWhiteSpace(effective)) + effective = tags.TryGetValue(tagId, out var t) ? t.Name : null; + if (effective is null) continue; + + references.Add((equipmentId, tagId, effective)); + } + + var resolver = new AclScopePathResolver( + folders, drivers, devices, groups, tags, areas, lines, equipment, references); + return resolver.Resolve(grants); + } + /// Reads one ApiKeys element; null when a field the credential check depends on is /// missing, so a half-written row can never authenticate. private static ApiKey? TryReadApiKey(JsonElement el) 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 8053d95a..1fbcdc9e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -9,6 +9,7 @@ using ZB.MOM.WW.OtOpcUa.Commons.Observability; using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Core.Authorization; using ZB.MOM.WW.OtOpcUa.OpcUaServer; using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; @@ -533,12 +534,29 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers // which is what lets a node recognise a re-applied revision without the hash riding the wire. var revision = Convert.ToHexStringLower(System.Security.Cryptography.SHA256.HashData(artifact)); var snapshot = _dataPlaneAuthority.Apply( - projection.ClusterId, revision, projection.Keys, projection.Grants); + projection.ClusterId, revision, projection.Keys, projection.Grants, projection.ScopePaths); + + // unresolvedScopes counts grants narrower than Cluster whose ScopeId named nothing in this + // artifact — a dangling id, or a scope kind with no v3 entity behind it. Those grants will be + // dropped when the trie is built, so surfacing the count here is the operator's only warning + // that a rule they authored is not going to do anything. + var unresolvedScopes = projection.Grants + .Where(g => g.ScopeKind != NodeAclScopeKind.Cluster && !string.IsNullOrEmpty(g.ScopeId)) + .Select(g => new AclScopeRef(g.ScopeKind, g.ScopeId!)) + .Distinct() + .Count(r => !projection.ScopePaths.ContainsKey(r)); _log.Info( - "OpcUaPublish: data-plane authority generation {Generation} installed (cluster={Cluster}, revision={Revision}, keys={Keys}, grants={Grants})", + "OpcUaPublish: data-plane authority generation {Generation} installed (cluster={Cluster}, revision={Revision}, keys={Keys}, grants={Grants}, resolvedScopes={ResolvedScopes})", snapshot.GenerationId, projection.ClusterId ?? "(single-cluster)", revision[..12], - projection.Keys.Count, projection.Grants.Count); + projection.Keys.Count, projection.Grants.Count, projection.ScopePaths.Count); + + if (unresolvedScopes > 0) + { + _log.Warning( + "OpcUaPublish: {Unresolved} ACL scope(s) in generation {Generation} name nothing in the deployed address space — grants on them will be DROPPED, not applied. Check the ScopeId on those rules", + unresolvedScopes, snapshot.GenerationId); + } } catch (Exception ex) { diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorNodeAclScopeTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorNodeAclScopeTests.cs new file mode 100644 index 00000000..5d6e3b66 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorNodeAclScopeTests.cs @@ -0,0 +1,419 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Configuration.Validation; + +namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests; + +/// +/// The #520 stage-3 deploy gate for scopes. An unresolvable scope is dropped by +/// the permission-trie builder and the deploy still seals green, so the row reads as a live grant in the +/// AdminUI while authorizing nothing — these pin that it fails the deploy instead. +/// +[Trait("Category", "Unit")] +public sealed class DraftValidatorNodeAclScopeTests +{ + /// The cluster every ACL row under test is authored in. + private const string Own = "c"; + + /// A second, fully-populated cluster in the same snapshot — the cross-cluster counterparty. + private const string Other = "other"; + + private const string OwnEquipId = "EQ-000000000000"; + private const string OtherEquipId = "EQ-0000000b0000"; + + private static NodeAcl Acl(NodeAclScopeKind kind, string? scopeId, string id = "acl-1") => new() + { + NodeAclId = id, + ClusterId = Own, + Principal = "svc-line3", + ScopeKind = kind, + ScopeId = scopeId, + PermissionFlags = NodePermissions.Read, + }; + + /// + /// A draft carrying one of each scoped entity in and a mirrored set in + /// . Two clusters, not one, so that a cross-cluster id is genuinely present in the + /// snapshot: if the rule's lookups were cluster-filtered such an id would present as merely + /// unresolved and the containment check could never fire. + /// + private static DraftSnapshot Draft(params NodeAcl[] acls) => new() + { + GenerationId = 1, + ClusterId = Own, + NodeAcls = acls, + RawFolders = + [ + new RawFolder { RawFolderId = "rf-1", ClusterId = Own, Name = "plant" }, + new RawFolder { RawFolderId = "rf-2", ClusterId = Other, Name = "plant2" }, + ], + DriverInstances = + [ + new DriverInstance + { + DriverInstanceId = "di-1", ClusterId = Own, Name = "modbus", + DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "rf-1", + }, + new DriverInstance + { + DriverInstanceId = "di-2", ClusterId = Other, Name = "modbus2", + DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "rf-2", + }, + ], + Devices = + [ + new Device { DeviceId = "dev-1", DriverInstanceId = "di-1", Name = "dev1", DeviceConfig = "{}" }, + new Device { DeviceId = "dev-2", DriverInstanceId = "di-2", Name = "dev2", DeviceConfig = "{}" }, + ], + TagGroups = + [ + new TagGroup { TagGroupId = "tg-1", DeviceId = "dev-1", Name = "group1" }, + new TagGroup { TagGroupId = "tg-2", DeviceId = "dev-2", Name = "group2" }, + ], + UnsAreas = + [ + new UnsArea { UnsAreaId = "area-1", ClusterId = Own, Name = "north" }, + new UnsArea { UnsAreaId = "area-2", ClusterId = Other, Name = "south" }, + ], + UnsLines = + [ + new UnsLine { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "line3" }, + new UnsLine { UnsLineId = "line-2", UnsAreaId = "area-2", Name = "line4" }, + ], + Equipment = + [ + new Equipment + { + EquipmentUuid = Guid.Parse("00000000-0000-0000-0000-0000000000aa"), + EquipmentId = OwnEquipId, + Name = "filler", UnsLineId = "line-1", MachineCode = "m", + }, + new Equipment + { + EquipmentUuid = Guid.Parse("0000000b-0000-0000-0000-000000000000"), + EquipmentId = OtherEquipId, + Name = "packer", UnsLineId = "line-2", MachineCode = "m", + }, + ], + Tags = + [ + new Tag + { + TagId = "tag-1", DeviceId = "dev-1", Name = "speed", + DataType = "Float", AccessLevel = TagAccessLevel.Read, TagConfig = "{}", + }, + new Tag + { + TagId = "tag-2", DeviceId = "dev-2", Name = "speed2", + DataType = "Float", AccessLevel = TagAccessLevel.Read, TagConfig = "{}", + }, + ], + }; + + /// + /// The fixture itself must trip no rule, or a test asserting on the whole error list is really + /// asserting on unrelated noise. This also pins both hardcoded EquipmentIds against the canonical + /// derivation, which a later edit to the fixture's UUIDs would otherwise silently break. + /// + [Fact] + public void The_fixture_with_no_acl_rows_is_clean() + { + DraftValidator.Validate(Draft()).ShouldBeEmpty(); + DraftValidator.DeriveEquipmentId(Guid.Parse("00000000-0000-0000-0000-0000000000aa")).ShouldBe(OwnEquipId); + DraftValidator.DeriveEquipmentId(Guid.Parse("0000000b-0000-0000-0000-000000000000")).ShouldBe(OtherEquipId); + } + + /// + /// The row shape the trie builder drops without a word: a narrower kind with nothing to narrow to. + /// + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void A_non_Cluster_kind_without_a_ScopeId_is_a_deploy_error(string? scopeId) + { + var errors = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Equipment, scopeId))); + + errors.ShouldContain(e => e.Code == "NodeAclScopeIdMissing" && e.Context == "acl-1"); + } + + /// + /// A Cluster grant ignores ScopeId, so a populated one means the author picked the wrong kind and the + /// grant is broader than intended — the direction that must never pass silently. + /// + [Fact] + public void A_Cluster_kind_carrying_a_ScopeId_is_a_deploy_error() + { + var errors = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Cluster, "EQ-000000000000"))); + + errors.ShouldContain(e => e.Code == "NodeAclScopeIdUnexpected" && e.Context == "acl-1"); + } + + [Fact] + public void A_Cluster_kind_with_no_ScopeId_passes() + { + DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Cluster, null))) + .ShouldNotContain(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)); + } + + /// + /// One case per scoped kind: a well-formed id that names no such entity must fail. Kept exhaustive + /// because the kind→collection mapping is hand-written, and a kind wired to the wrong collection + /// would still "work" for every id that happens to exist in both. + /// + [Theory] + [InlineData(NodeAclScopeKind.UnsArea)] + [InlineData(NodeAclScopeKind.UnsLine)] + [InlineData(NodeAclScopeKind.Equipment)] + [InlineData(NodeAclScopeKind.Tag)] + [InlineData(NodeAclScopeKind.RawFolder)] + [InlineData(NodeAclScopeKind.RawDriver)] + [InlineData(NodeAclScopeKind.RawDevice)] + [InlineData(NodeAclScopeKind.RawTagGroup)] + public void A_ScopeId_naming_no_such_entity_is_a_deploy_error(NodeAclScopeKind kind) + { + var errors = DraftValidator.Validate(Draft(Acl(kind, "does-not-exist"))); + + errors.ShouldContain(e => e.Code == "NodeAclScopeUnresolved" && e.Context == "acl-1"); + } + + /// + /// The counterpart to the theory above: each kind pointed at the id from its OWN collection must + /// pass. Without this the rule could reject everything and still look correct. + /// + [Theory] + [InlineData(NodeAclScopeKind.UnsArea, "area-1")] + [InlineData(NodeAclScopeKind.UnsLine, "line-1")] + [InlineData(NodeAclScopeKind.Equipment, OwnEquipId)] + [InlineData(NodeAclScopeKind.Tag, "tag-1")] + [InlineData(NodeAclScopeKind.RawFolder, "rf-1")] + [InlineData(NodeAclScopeKind.RawDriver, "di-1")] + [InlineData(NodeAclScopeKind.RawDevice, "dev-1")] + [InlineData(NodeAclScopeKind.RawTagGroup, "tg-1")] + public void A_ScopeId_naming_a_real_entity_of_its_own_kind_passes(NodeAclScopeKind kind, string scopeId) + { + DraftValidator.Validate(Draft(Acl(kind, scopeId))) + .ShouldNotContain(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)); + } + + /// + /// The containment check, one case per kind. Each id is real and resolves to a real trie path — it + /// just belongs to another cluster, so the grant would confer authority across a cluster boundary. + /// Three of these kinds carry ClusterId directly; the other five are only reachable by walking a + /// parent chain, which is why every kind is covered rather than a representative one. + /// + [Theory] + [InlineData(NodeAclScopeKind.UnsArea, "area-2")] + [InlineData(NodeAclScopeKind.UnsLine, "line-2")] + [InlineData(NodeAclScopeKind.Equipment, OtherEquipId)] + [InlineData(NodeAclScopeKind.Tag, "tag-2")] + [InlineData(NodeAclScopeKind.RawFolder, "rf-2")] + [InlineData(NodeAclScopeKind.RawDriver, "di-2")] + [InlineData(NodeAclScopeKind.RawDevice, "dev-2")] + [InlineData(NodeAclScopeKind.RawTagGroup, "tg-2")] + public void A_ScopeId_naming_an_entity_in_another_cluster_is_a_deploy_error( + NodeAclScopeKind kind, string scopeId) + { + var errors = DraftValidator.Validate(Draft(Acl(kind, scopeId))); + + errors.ShouldContain(e => e.Code == "NodeAclScopeCrossCluster" && e.Context == "acl-1"); + } + + /// + /// The trap this check exists to avoid: if the lookups were cluster-filtered, a cross-cluster id + /// would present as unresolved, the operator would be told to fix a typo in an id that is not a typo, + /// and the containment rule would never fire at all. The two codes must be mutually exclusive. + /// + [Theory] + [InlineData(NodeAclScopeKind.UnsArea, "area-2")] + [InlineData(NodeAclScopeKind.UnsLine, "line-2")] + [InlineData(NodeAclScopeKind.Equipment, OtherEquipId)] + [InlineData(NodeAclScopeKind.Tag, "tag-2")] + [InlineData(NodeAclScopeKind.RawFolder, "rf-2")] + [InlineData(NodeAclScopeKind.RawDriver, "di-2")] + [InlineData(NodeAclScopeKind.RawDevice, "dev-2")] + [InlineData(NodeAclScopeKind.RawTagGroup, "tg-2")] + public void A_cross_cluster_scope_is_never_also_reported_as_unresolved( + NodeAclScopeKind kind, string scopeId) + { + var errors = DraftValidator.Validate(Draft(Acl(kind, scopeId))); + + errors.ShouldNotContain(e => e.Code == "NodeAclScopeUnresolved"); + errors.Where(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)).ShouldHaveSingleItem(); + } + + /// The error must name both clusters — which one the grant is in and which one owns the + /// entity — because "re-author under the other cluster" and "repoint at the local entity" are different + /// fixes and the operator picks between them from this message alone. + [Fact] + public void The_cross_cluster_message_names_both_clusters() + { + var error = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.RawDevice, "dev-2"))) + .First(e => e.Code == "NodeAclScopeCrossCluster"); + + error.Message.ShouldContain($"'{Own}'"); + error.Message.ShouldContain($"'{Other}'"); + } + + /// + /// An entity whose parent link dangles belongs to no cluster, so containment is unknowable — and it + /// resolves to no scope path either, which is the unresolved case. It must report that, once, rather + /// than a cross-cluster error built on a null owner. + /// + [Fact] + public void A_device_whose_driver_is_missing_reports_unresolved_only() + { + var draft = new DraftSnapshot + { + GenerationId = 1, + ClusterId = Own, + NodeAcls = [Acl(NodeAclScopeKind.RawDevice, "dev-orphan")], + Devices = + [ + new Device + { + DeviceId = "dev-orphan", DriverInstanceId = "di-gone", + Name = "orphan", DeviceConfig = "{}", + }, + ], + }; + + var errors = DraftValidator.Validate(draft); + + errors.ShouldContain(e => e.Code == "NodeAclScopeUnresolved" && e.Context == "acl-1"); + errors.ShouldNotContain(e => e.Code == "NodeAclScopeCrossCluster"); + } + + /// The UNS counterpart, and a two-hop break rather than one: Equipment reaches its cluster + /// through UnsLine and only then UnsArea, so a missing line strands it just as a missing driver strands + /// a device. + [Fact] + public void An_equipment_whose_line_is_missing_reports_unresolved_only() + { + var uuid = Guid.Parse("00000000-0000-0000-0000-0000000000aa"); + var draft = new DraftSnapshot + { + GenerationId = 1, + ClusterId = Own, + NodeAcls = [Acl(NodeAclScopeKind.Equipment, OwnEquipId)], + Equipment = + [ + new Equipment + { + EquipmentUuid = uuid, EquipmentId = DraftValidator.DeriveEquipmentId(uuid), + Name = "filler", UnsLineId = "line-gone", MachineCode = "m", + }, + ], + }; + + var errors = DraftValidator.Validate(draft); + + errors.ShouldContain(e => e.Code == "NodeAclScopeUnresolved" && e.Context == "acl-1"); + errors.ShouldNotContain(e => e.Code == "NodeAclScopeCrossCluster"); + } + + /// + /// ScopeId is a bare string with no foreign key, so a real id of the wrong kind is exactly + /// the mistake that would otherwise hand a grant another entity's subtree. + /// + [Fact] + public void A_real_id_of_the_wrong_kind_is_still_unresolved() + { + var errors = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.UnsArea, "dev-1"))); + + errors.ShouldContain(e => e.Code == "NodeAclScopeUnresolved" && e.Context == "acl-1"); + } + + /// Both realm tokens and both full namespace URIs are accepted, in any case — matching + /// NodeAclRealms.TryCanonicalize, which is what actually resolves the row at build time. + [Theory] + [InlineData("raw")] + [InlineData("uns")] + [InlineData("RAW")] + [InlineData("Uns")] + [InlineData(V3NodeIds.RawNamespaceUri)] + [InlineData(V3NodeIds.UnsNamespaceUri)] + [InlineData(" raw ")] + public void A_Namespace_scope_naming_a_realm_passes(string scopeId) + { + DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Namespace, scopeId))) + .ShouldNotContain(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)); + } + + /// The server has exactly two namespaces; anything else names none of them. + [Theory] + [InlineData("ns=2")] + [InlineData("https://zb.com/otopcua/ns")] // the retired v2 single namespace + [InlineData("equipment")] + public void A_Namespace_scope_naming_no_realm_is_a_deploy_error(string scopeId) + { + var errors = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.Namespace, scopeId))); + + errors.ShouldContain(e => e.Code == "NodeAclNamespaceInvalid" && e.Context == "acl-1"); + } + + /// + /// FolderSegment survives in the enum only so later ordinals do not shift; it has no v3 entity and + /// can never resolve, so the error has to name the replacement rather than just say "unresolved". + /// + [Fact] + public void The_retired_FolderSegment_kind_is_a_deploy_error_naming_RawFolder() + { + var error = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.FolderSegment, "some-folder"))) + .Where(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)) + .ShouldHaveSingleItem(); + + error.Code.ShouldBe("NodeAclScopeKindRetired"); + error.Context.ShouldBe("acl-1"); + error.Message.ShouldContain("RawFolder"); + } + + /// + /// A FolderSegment row with no ScopeId is both retired and missing an id. The retirement is the + /// actionable fact — the kind can never resolve whatever its ScopeId — so telling the operator to + /// supply one would send them to fix the wrong thing. + /// + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void FolderSegment_retirement_wins_over_the_missing_ScopeId_error(string? scopeId) + { + var error = DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.FolderSegment, scopeId))) + .Where(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)) + .ShouldHaveSingleItem(); + + error.Code.ShouldBe("NodeAclScopeKindRetired"); + } + + /// + /// An ordinal outside the enum comes from a hand-edited row or a stale schema. Every other check + /// switches on the kind, so it must be rejected up front rather than falling into a default branch. + /// + [Fact] + public void An_undefined_ScopeKind_ordinal_is_a_deploy_error() + { + var errors = DraftValidator.Validate(Draft(Acl((NodeAclScopeKind)99, "anything"))); + + errors.ShouldContain(e => e.Code == "NodeAclScopeKindUndefined" && e.Context == "acl-1"); + } + + /// Every bad row is reported in one pass — the validator's stated contract, and what lets an + /// operator fix a batch of ACLs in one edit instead of one deploy attempt each. + [Fact] + public void All_offending_rows_are_reported_in_one_pass() + { + var errors = DraftValidator.Validate(Draft( + Acl(NodeAclScopeKind.Equipment, null, "acl-a"), + Acl(NodeAclScopeKind.Cluster, OwnEquipId, "acl-b"), + Acl(NodeAclScopeKind.RawDevice, "nope", "acl-c"), + Acl(NodeAclScopeKind.RawDevice, "dev-2", "acl-d"))); + + errors.Where(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)) + .Select(e => e.Context) + .ShouldBe(["acl-a", "acl-b", "acl-c", "acl-d"], ignoreOrder: true); + } +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/AclScopePathResolverTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/AclScopePathResolverTests.cs new file mode 100644 index 00000000..ac6d5b46 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/AclScopePathResolverTests.cs @@ -0,0 +1,270 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; + +namespace ZB.MOM.WW.OtOpcUa.Core.Tests.Authorization; + +/// +/// Coverage for — the producer that turns an ACL row's +/// ScopeId into the trie path(s) a grant on it must be planted at (#520 stage 3). +/// +/// +/// The fixture is deliberately a deeply nested raw tree — two nested folders and two nested +/// tag groups — because the shape that broke the previous design is exactly the one a shallow +/// fixture cannot express. A five-level ladder resolves the shallowest possible raw tree correctly +/// and silently truncates this one. +/// +public sealed class AclScopePathResolverTests +{ + // Plant/Line1 (nested folders) → ModbusDrv → dev1 → Fast/Inner (nested groups) → Speed + private static AclScopePathResolver Fixture() => new( + folders: new Dictionary + { + ["RF-plant"] = (null, "Plant"), + ["RF-line1"] = ("RF-plant", "Line1"), + }, + drivers: new Dictionary + { + ["DI-modbus"] = ("RF-line1", "ModbusDrv"), + ["DI-rootlevel"] = (null, "RootDrv"), + }, + devices: new Dictionary + { + ["DV-1"] = ("DI-modbus", "dev1"), + }, + groups: new Dictionary + { + ["TG-fast"] = (null, "DV-1", "Fast"), + ["TG-inner"] = ("TG-fast", "DV-1", "Inner"), + }, + tags: new Dictionary + { + ["TAG-speed"] = ("DV-1", "TG-inner", "Speed"), + ["TAG-bare"] = ("DV-1", null, "Bare"), + }, + areas: new Dictionary { ["UA-w"] = "West" }, + lines: new Dictionary { ["UL-1"] = ("UA-w", "L1") }, + equipment: new Dictionary + { + ["EQ-mixer"] = ("UL-1", "Mixer"), + ["EQ-filler"] = ("UL-1", "Filler"), + }, + references: + [ + ("EQ-mixer", "TAG-speed", "MixerSpeed"), + ("EQ-filler", "TAG-speed", "FillerSpeed"), + ]); + + private static IReadOnlyList Single(NodeAclScopeKind kind, string scopeId) => + Fixture().ResolveOne(kind, scopeId).ShouldHaveSingleItem().Segments; + + [Fact] + public void A_nested_raw_folder_resolves_through_its_whole_ancestor_chain() + { + Single(NodeAclScopeKind.RawFolder, "RF-line1").ShouldBe(["raw", "Plant", "Line1"]); + } + + [Fact] + public void A_raw_device_resolves_through_folders_and_its_driver() + { + Single(NodeAclScopeKind.RawDevice, "DV-1").ShouldBe(["raw", "Plant", "Line1", "ModbusDrv", "dev1"]); + } + + [Fact] + public void A_nested_tag_group_resolves_through_its_own_ancestor_chain_too() + { + // Both nesting axes at once — the folder chain AND the group chain. This is the case a + // fixed-depth scope model cannot represent at all: it needs seven segments below the realm. + Single(NodeAclScopeKind.RawTagGroup, "TG-inner") + .ShouldBe(["raw", "Plant", "Line1", "ModbusDrv", "dev1", "Fast", "Inner"]); + } + + [Fact] + public void Every_container_path_is_a_true_prefix_of_the_tag_beneath_it() + { + // The invariant the trie walk depends on: a grant on a container only cascades to the tags below + // it if the container's path is a genuine prefix of theirs. If the container paths were composed + // in a different order from the tag path — the obvious way to get this wrong, since the tag path + // comes from RawPathResolver and the container paths are composed here — a container grant would + // land on a sibling branch and silently protect nothing. + var resolver = Fixture(); + var tag = resolver.ResolveOne(NodeAclScopeKind.Tag, "TAG-speed") + .First(p => p.Segments[0] == NodeAclRealms.Raw).Segments; + + foreach (var (kind, id) in new[] + { + (NodeAclScopeKind.RawFolder, "RF-plant"), + (NodeAclScopeKind.RawFolder, "RF-line1"), + (NodeAclScopeKind.RawDriver, "DI-modbus"), + (NodeAclScopeKind.RawDevice, "DV-1"), + (NodeAclScopeKind.RawTagGroup, "TG-fast"), + (NodeAclScopeKind.RawTagGroup, "TG-inner"), + }) + { + var container = resolver.ResolveOne(kind, id).ShouldHaveSingleItem().Segments; + container.Count.ShouldBeLessThan(tag.Count, $"{kind} {id}"); + tag.Take(container.Count).ShouldBe(container, $"{kind} {id} is not a prefix of the tag path"); + } + } + + [Fact] + public void A_tag_resolves_to_its_raw_node_AND_every_UNS_projection_of_it() + { + // A raw tag referenced by two equipments is reachable at three NodeIds, and a write through any + // of them lands on the same driver reference. A grant covering only the raw node would be + // bypassable through either UNS NodeId — so all three must be planted. + var paths = Fixture().ResolveOne(NodeAclScopeKind.Tag, "TAG-speed") + .Select(p => p.Segments) + .ToList(); + + paths.Count.ShouldBe(3); + paths.ShouldContain(s => s.SequenceEqual( + new[] { "raw", "Plant", "Line1", "ModbusDrv", "dev1", "Fast", "Inner", "Speed" })); + paths.ShouldContain(s => s.SequenceEqual(new[] { "uns", "West", "L1", "Mixer", "MixerSpeed" })); + paths.ShouldContain(s => s.SequenceEqual(new[] { "uns", "West", "L1", "Filler", "FillerSpeed" })); + } + + [Fact] + public void A_tag_with_no_UNS_reference_resolves_to_its_raw_node_only() + { + // The positive control for the test above: it proves the three-path result came from the two + // references rather than from the resolver always emitting extra paths. + Single(NodeAclScopeKind.Tag, "TAG-bare") + .ShouldBe(["raw", "Plant", "Line1", "ModbusDrv", "dev1", "Bare"]); + } + + [Fact] + public void A_driver_at_the_cluster_root_has_no_folder_segments() + { + Single(NodeAclScopeKind.RawDriver, "DI-rootlevel").ShouldBe(["raw", "RootDrv"]); + } + + [Theory] + [InlineData(NodeAclScopeKind.UnsArea, "UA-w", new[] { "uns", "West" })] + [InlineData(NodeAclScopeKind.UnsLine, "UL-1", new[] { "uns", "West", "L1" })] + [InlineData(NodeAclScopeKind.Equipment, "EQ-mixer", new[] { "uns", "West", "L1", "Mixer" })] + public void UNS_scopes_resolve_to_their_name_path(NodeAclScopeKind kind, string id, string[] expected) + { + Single(kind, id).ShouldBe(expected); + } + + [Theory] + [InlineData("raw", "raw")] + [InlineData("RAW", "raw")] + [InlineData("uns", "uns")] + [InlineData("https://zb.com/otopcua/raw", "raw")] + [InlineData("https://zb.com/otopcua/uns", "uns")] + public void A_namespace_scope_accepts_the_token_or_the_full_uri(string scopeId, string expected) + { + Single(NodeAclScopeKind.Namespace, scopeId).ShouldBe([expected]); + } + + [Fact] + public void A_namespace_scope_naming_no_realm_resolves_to_nothing() + { + Fixture().ResolveOne(NodeAclScopeKind.Namespace, "https://zb.com/otopcua/ns").ShouldBeEmpty(); + } + + [Theory] + [InlineData(NodeAclScopeKind.RawFolder, "RF-nope")] + [InlineData(NodeAclScopeKind.RawDriver, "DI-nope")] + [InlineData(NodeAclScopeKind.RawDevice, "DV-nope")] + [InlineData(NodeAclScopeKind.RawTagGroup, "TG-nope")] + [InlineData(NodeAclScopeKind.Tag, "TAG-nope")] + [InlineData(NodeAclScopeKind.UnsArea, "UA-nope")] + [InlineData(NodeAclScopeKind.UnsLine, "UL-nope")] + [InlineData(NodeAclScopeKind.Equipment, "EQ-nope")] + public void A_dangling_scope_id_resolves_to_nothing(NodeAclScopeKind kind, string scopeId) + { + // Resolving to nothing is what makes the builder DROP the grant with a diagnostic. The dangerous + // alternative — inventing a path — would plant an operator's grant on some other node. + Fixture().ResolveOne(kind, scopeId).ShouldBeEmpty(); + } + + [Fact] + public void The_retired_FolderSegment_kind_resolves_to_nothing() + { + // It described a v2 hierarchy that v3 deleted. Deliberately unresolvable so a surviving row + // produces a diagnostic naming it, rather than a grant that quietly does nothing. + Fixture().ResolveOne(NodeAclScopeKind.FolderSegment, "RF-line1").ShouldBeEmpty(); + } + + [Fact] + public void A_corrupt_parent_cycle_terminates_instead_of_hanging() + { + // Folder nesting is unbounded by design, so the walk cannot be bounded by a schema limit — only + // by a cycle guard. Without one a corrupt artifact would hang the deploy-apply thread rather than + // dropping one grant. + var resolver = new AclScopePathResolver( + folders: new Dictionary + { + ["A"] = ("B", "A"), + ["B"] = ("A", "B"), + }, + drivers: new Dictionary(), + devices: new Dictionary(), + groups: new Dictionary(), + tags: new Dictionary(), + areas: new Dictionary(), + lines: new Dictionary(), + equipment: new Dictionary(), + references: []); + + resolver.ResolveOne(NodeAclScopeKind.RawFolder, "A").ShouldBeEmpty(); + } + + [Fact] + public void Resolve_keys_on_the_kind_as_well_as_the_id() + { + // ScopeId is a bare string with no FK and no type discriminator; ids are prefixed only by + // convention. Keying the lookup on the id alone would let one entity's id collide with another's + // and hand a grant the wrong node's paths — a silent wrong-node authorization. + var resolver = new AclScopePathResolver( + folders: new Dictionary { ["dup"] = (null, "AsFolder") }, + drivers: new Dictionary(), + devices: new Dictionary(), + groups: new Dictionary(), + tags: new Dictionary(), + areas: new Dictionary { ["dup"] = "AsArea" }, + lines: new Dictionary(), + equipment: new Dictionary(), + references: []); + + var resolved = resolver.Resolve( + [ + Acl("acl-1", NodeAclScopeKind.RawFolder, "dup"), + Acl("acl-2", NodeAclScopeKind.UnsArea, "dup"), + ]); + + resolved[new AclScopeRef(NodeAclScopeKind.RawFolder, "dup")] + .ShouldHaveSingleItem().Segments.ShouldBe(["raw", "AsFolder"]); + resolved[new AclScopeRef(NodeAclScopeKind.UnsArea, "dup")] + .ShouldHaveSingleItem().Segments.ShouldBe(["uns", "AsArea"]); + } + + [Fact] + public void Resolve_omits_cluster_scoped_and_unresolvable_rows() + { + var resolved = Fixture().Resolve( + [ + Acl("acl-cluster", NodeAclScopeKind.Cluster, null), + Acl("acl-dangling", NodeAclScopeKind.Equipment, "EQ-nope"), + Acl("acl-good", NodeAclScopeKind.Equipment, "EQ-mixer"), + ]); + + resolved.Count.ShouldBe(1); + resolved.ShouldContainKey(new AclScopeRef(NodeAclScopeKind.Equipment, "EQ-mixer")); + } + + private static NodeAcl Acl(string id, NodeAclScopeKind kind, string? scopeId) => new() + { + NodeAclId = id, + ClusterId = "c1", + Principal = "p", + ScopeKind = kind, + ScopeId = scopeId, + PermissionFlags = NodePermissions.Read, + }; +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/DataPlaneAuthorityStateTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/DataPlaneAuthorityStateTests.cs index a8a9513a..90695ce7 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/DataPlaneAuthorityStateTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/DataPlaneAuthorityStateTests.cs @@ -14,6 +14,14 @@ public sealed class DataPlaneAuthorityStateTests { private static readonly DateTime Now = new(2026, 7, 31, 12, 0, 0, DateTimeKind.Utc); + /// + /// These tests are about key lookup and the generation counter, not about scope resolution, so + /// they install no scope paths. Resolution is covered by AclScopePathResolverTests and the + /// artifact-to-walk path by DeploymentArtifactScopePathTests. + /// + private static readonly IReadOnlyDictionary> NoScopes = + new Dictionary>(); + [Fact] public void Before_any_artifact_the_node_authenticates_nothing() { @@ -30,8 +38,8 @@ public sealed class DataPlaneAuthorityStateTests { var sut = new DataPlaneAuthorityState(); - sut.Apply(null, "rev-1", [], []).GenerationId.ShouldBe(1); - sut.Apply(null, "rev-2", [], []).GenerationId.ShouldBe(2); + sut.Apply(null, "rev-1", [], [], NoScopes).GenerationId.ShouldBe(1); + sut.Apply(null, "rev-2", [], [], NoScopes).GenerationId.ShouldBe(2); } [Fact] @@ -41,8 +49,8 @@ public sealed class DataPlaneAuthorityStateTests // generation there would invalidate every cached authorization decision for no reason. var sut = new DataPlaneAuthorityState(); - sut.Apply(null, "rev-1", [], []).GenerationId.ShouldBe(1); - sut.Apply(null, "rev-1", [], []).GenerationId.ShouldBe(1); + sut.Apply(null, "rev-1", [], [], NoScopes).GenerationId.ShouldBe(1); + sut.Apply(null, "rev-1", [], [], NoScopes).GenerationId.ShouldBe(1); } [Fact] @@ -56,7 +64,7 @@ public sealed class DataPlaneAuthorityStateTests var sut = new DataPlaneAuthorityState(); var generations = new[] { "ffff", "cccc", "aaaa", "0001" } - .Select(rev => sut.Apply(null, rev, [], []).GenerationId) + .Select(rev => sut.Apply(null, rev, [], [], NoScopes).GenerationId) .ToArray(); generations.ShouldBe([1, 2, 3, 4]); @@ -66,7 +74,7 @@ public sealed class DataPlaneAuthorityStateTests public void A_usable_key_is_found_by_id() { var sut = new DataPlaneAuthorityState(); - sut.Apply("cluster-a", "rev-1", [Key("abc", "p")], []); + sut.Apply("cluster-a", "rev-1", [Key("abc", "p")], [], NoScopes); sut.Current.FindUsableKey("abc", Now)!.Principal.ShouldBe("p"); } @@ -77,7 +85,7 @@ public sealed class DataPlaneAuthorityStateTests // Key ids are minted lowercase hex, but an operator copying one out of the AdminUI can upper-case // it. A case-sensitive miss would read to them as "the key was revoked". var sut = new DataPlaneAuthorityState(); - sut.Apply(null, "rev-1", [Key("abc", "p")], []); + sut.Apply(null, "rev-1", [Key("abc", "p")], [], NoScopes); sut.Current.FindUsableKey("ABC", Now).ShouldNotBeNull(); } @@ -88,7 +96,7 @@ public sealed class DataPlaneAuthorityStateTests var revoked = Key("abc", "p"); revoked.Enabled = false; var sut = new DataPlaneAuthorityState(); - sut.Apply(null, "rev-1", [revoked], []); + sut.Apply(null, "rev-1", [revoked], [], NoScopes); sut.Current.FindUsableKey("abc", Now).ShouldBeNull(); } @@ -101,7 +109,7 @@ public sealed class DataPlaneAuthorityStateTests var expiring = Key("abc", "p"); expiring.ExpiresUtc = Now; var sut = new DataPlaneAuthorityState(); - sut.Apply(null, "rev-1", [expiring], []); + sut.Apply(null, "rev-1", [expiring], [], NoScopes); sut.Current.FindUsableKey("abc", Now).ShouldBeNull("expiry is inclusive — at the instant it expires it is gone"); sut.Current.FindUsableKey("abc", Now.AddSeconds(-1)).ShouldNotBeNull(); @@ -115,7 +123,7 @@ public sealed class DataPlaneAuthorityStateTests [ Acl("acl-1", "alice", NodePermissions.Read), Acl("acl-2", "bob", NodePermissions.WriteOperate), - ]); + ], NoScopes); sut.Current.GrantsFor("alice").ShouldHaveSingleItem().NodeAclId.ShouldBe("acl-1"); sut.Current.GrantsFor("nobody").ShouldBeEmpty(); @@ -129,7 +137,7 @@ public sealed class DataPlaneAuthorityStateTests [ Acl("acl-1", "alice", NodePermissions.Read), Acl("acl-2", "alice", NodePermissions.WriteOperate), - ]); + ], NoScopes); sut.Current.GrantsFor("alice").Count.ShouldBe(2); } @@ -140,8 +148,8 @@ public sealed class DataPlaneAuthorityStateTests // Merging would make revocation impossible: a key dropped from the artifact would live on in the // node's memory forever. var sut = new DataPlaneAuthorityState(); - sut.Apply(null, "rev-1", [Key("old", "p")], []); - sut.Apply(null, "rev-2", [Key("new", "p")], []); + sut.Apply(null, "rev-1", [Key("old", "p")], [], NoScopes); + sut.Apply(null, "rev-2", [Key("new", "p")], [], NoScopes); sut.Current.FindUsableKey("old", Now).ShouldBeNull(); sut.Current.FindUsableKey("new", Now).ShouldNotBeNull(); @@ -154,7 +162,7 @@ public sealed class DataPlaneAuthorityStateTests // artifact — where refusing to build the snapshot would leave the node authenticating nothing. var sut = new DataPlaneAuthorityState(); - Should.NotThrow(() => sut.Apply(null, "rev-1", [Key("dup", "a"), Key("dup", "b")], [])); + Should.NotThrow(() => sut.Apply(null, "rev-1", [Key("dup", "a"), Key("dup", "b")], [], NoScopes)); sut.Current.FindUsableKey("dup", Now).ShouldNotBeNull(); } @@ -164,7 +172,7 @@ public sealed class DataPlaneAuthorityStateTests // The node manager's only source of cluster identity; NodeScope.ClusterId is required and the // evaluator fails closed when it disagrees with the session's. var sut = new DataPlaneAuthorityState(); - sut.Apply("cluster-a", "rev-1", [], []); + sut.Apply("cluster-a", "rev-1", [], [], NoScopes); sut.Current.ClusterId.ShouldBe("cluster-a"); } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieBuilderTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieBuilderTests.cs index 86848b9a..e78d555e 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieBuilderTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieBuilderTests.cs @@ -7,17 +7,19 @@ using ZB.MOM.WW.OtOpcUa.Core.Authorization; namespace ZB.MOM.WW.OtOpcUa.Core.Tests.Authorization; /// -/// Core-011 regression coverage for 's -/// Descend helper: +/// Core-011 regression coverage for 's row placement: /// -/// With a scopePaths lookup the row must land at the correct multi-level -/// trie node — a deep grant must be visible -/// ONLY when the requested scope walks the same namespace/area/line chain. -/// Without a scopePaths entry the row falls back to a direct child of -/// the namespace root keyed on the row's ScopeId. The builder must surface -/// this fallback (warning callback) so callers know a grant was placed where the -/// walker can't reach it for production hierarchies — silently dropping the grant -/// is the Core-011 production hazard. +/// A row whose scope resolves must land at the correct multi-level trie node — a deep +/// grant is visible ONLY when the requested scope +/// walks the same realm/area/line chain. +/// A row whose scope does NOT resolve must be dropped with a diagnostic. Before +/// #520 stage 3 it was planted as a direct child of the trie root keyed on ScopeId, +/// a position level 1 of the walk (the realm) can never reach — so the grant looked placed +/// and could never match. Silently misplacing the grant is the Core-011 production +/// hazard. +/// A scope resolving to several paths (a raw tag projected into N equipments) must plant +/// the grant at all of them, or the grant is bypassable through the NodeId it +/// missed. /// /// [Trait("Category", "Unit")] @@ -35,78 +37,150 @@ public sealed class PermissionTrieBuilderTests PermissionFlags = flags, }; - private static NodeScope EquipmentTag(string cluster, string ns, string area, string line, string equip, string tag) => - new() + /// One scope resolving to one path. + private static IReadOnlyDictionary> ScopePaths( + params (NodeAclScopeKind Kind, string ScopeId, string[] Segments)[] entries) => + entries.ToDictionary( + e => new AclScopeRef(e.Kind, e.ScopeId), + e => (IReadOnlyList)new[] { new NodeAclPath(e.Segments) }); + + /// A UNS-realm trie path: uns / …segments…. + private static string[] UnsPath(params string[] segments) => [NodeAclRealms.Uns, .. segments]; + + /// A raw-realm trie path: raw / …segments…. + private static string[] RawPath(params string[] segments) => [NodeAclRealms.Raw, .. segments]; + + [Fact] + public void Build_Path_Not_Starting_At_A_Realm_Is_Rejected_Not_Planted() + { + // Level 1 of the walk is always a realm token, so each of these addresses a node CollectMatches + // can never reach. Without this guard the grant would be planted, look present in the trie, and + // silently never match — the same failure mode as the deleted root-child fallback, just relocated + // into the scope-path producer where nothing else cross-checks it. + string[][] malformed = + [ + [], // empty path + ["area-1", "line-1"], // producer forgot the realm segment + ["Raw", "area-1"], // realm token with the wrong case + ["https://zb.com/otopcua/raw", "area-1"], // URI where the short token belongs + ]; + + foreach (var segments in malformed) { - ClusterId = cluster, - NamespaceId = ns, - UnsAreaId = area, - UnsLineId = line, - EquipmentId = equip, - TagId = tag, - Kind = NodeHierarchyKind.Equipment, + var diagnostics = new List(); + var rows = new[] { Row("cn=a", NodeAclScopeKind.UnsLine, "line-1", NodePermissions.Read) }; + var paths = new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.UnsLine, "line-1")] = [new NodeAclPath(segments)], + }; + + var label = segments.Length == 0 ? "(empty)" : string.Join('/', segments); + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths, diagnostics.Add); + + diagnostics.ShouldHaveSingleItem(label).Reason + .ShouldBe(PermissionTrieBuildDiagnosticReason.InvalidScopePath, label); + trie.Root.Children.ShouldBeEmpty(label); + } + } + + [Fact] + public void Build_Plants_The_Valid_Paths_Of_A_Scope_Even_When_A_Sibling_Path_Is_Malformed() + { + // The positive control for the theory above: a guard that rejected every path would satisfy it. + // A multi-path scope must keep the reachable placements and drop only the broken one. + var diagnostics = new List(); + var rows = new[] { Row("cn=a", NodeAclScopeKind.Tag, "tag-1", NodePermissions.Read) }; + var paths = new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.Tag, "tag-1")] = + [ + new NodeAclPath(RawPath("Drv", "dev1", "Speed")), + new NodeAclPath(["area-1", "line-1"]), + ], }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths, diagnostics.Add); + + diagnostics.ShouldHaveSingleItem().Reason + .ShouldBe(PermissionTrieBuildDiagnosticReason.InvalidScopePath); + trie.CollectMatches( + new NodeScope + { + ClusterId = "c1", + Kind = NodeHierarchyKind.Raw, + Segments = ["Drv", "dev1", "Speed"], + }, + ["cn=a"]).ShouldHaveSingleItem(); + } + + /// A node in the UNS realm addressed by its path below the realm segment. + private static NodeScope UnsScope(string cluster, params string[] segments) => + new() { ClusterId = cluster, Kind = NodeHierarchyKind.Uns, Segments = segments }; + + /// A node in the raw realm addressed by its path below the realm segment. + private static NodeScope RawScope(string cluster, params string[] segments) => + new() { ClusterId = cluster, Kind = NodeHierarchyKind.Raw, Segments = segments }; + /// Verifies that Build with ScopePaths places UnsLine row at correct multi-level node. [Fact] public void Build_With_ScopePaths_Places_UnsLine_Row_At_Correct_Multi_Level_Node() { - // Scope path mirrors the production hierarchy: namespace → area → line. - var paths = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - ["line-42"] = new(new[] { "ns", "area-1", "line-42" }), - }; + // Scope path mirrors the production hierarchy: realm → area → line. + var paths = ScopePaths( + (NodeAclScopeKind.UnsLine, "line-42", UnsPath("area-1", "line-42"))); var rows = new[] { Row("cn=ops", NodeAclScopeKind.UnsLine, "line-42", NodePermissions.Read) }; var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); // Walk through the same chain — the grant must be reachable. var matchOnLine = trie.CollectMatches( - EquipmentTag("c1", "ns", "area-1", "line-42", "eq-A", "tag-A"), + UnsScope("c1", "area-1", "line-42", "eq-A", "tag-A"), ["cn=ops"]); matchOnLine.Count.ShouldBe(1, "row must land at the correct multi-level trie node"); // A different line under the same area must not pick up the grant. var matchOnOtherLine = trie.CollectMatches( - EquipmentTag("c1", "ns", "area-1", "line-99", "eq-A", "tag-A"), + UnsScope("c1", "area-1", "line-99", "eq-A", "tag-A"), ["cn=ops"]); matchOnOtherLine.ShouldBeEmpty( "grant anchored at line-42 must not leak to sibling line-99 under the same area"); } - /// Verifies that Build without ScopePaths falls back to root child for deterministic tests. + /// + /// Replaces Build_Without_ScopePaths_Falls_Back_To_Root_Child_For_Tests: the null-scopePaths + /// "deterministic test mode" is deleted. An unresolvable sub-cluster row is now dropped entirely — + /// it must NOT appear as a root child keyed on its ScopeId, because level 1 of the walk is + /// the realm and nothing can ever reach such a node. + /// [Fact] - public void Build_Without_ScopePaths_Falls_Back_To_Root_Child_For_Tests() + public void Build_Unresolvable_Row_Is_Dropped_Not_Planted_At_Root_Child() { - // Fallback path — deterministic tests pass without a scope-path lookup. The row - // is placed as a direct child of the trie root keyed by ScopeId. var rows = new[] { Row("cn=ops", NodeAclScopeKind.UnsLine, "line-42", NodePermissions.Read) }; - var trie = PermissionTrieBuilder.Build("c1", 1, rows); + var trie = PermissionTrieBuilder.Build( + "c1", 1, rows, new Dictionary>()); - // Root has one child — "line-42". - trie.Root.Children.ShouldContainKey("line-42"); - var node = trie.Root.Children["line-42"]; - node.Grants.Count.ShouldBe(1); + trie.Root.Children.ShouldNotContainKey("line-42", + "the retired fallback planted the row here, where the realm-first walk can never reach it"); + trie.Root.Children.ShouldBeEmpty(); + trie.Root.Grants.ShouldBeEmpty("an unresolvable row must not become a cluster-wide grant either"); } /// - /// Core-011 regression: when a sub-cluster row's ScopeId is not in the supplied - /// scopePaths, the fallback diagnostic callback must fire so the caller can - /// surface a warning. Silently dropping the grant under the wrong trie level is the - /// production hazard the finding flagged. + /// Core-011 regression: when a sub-cluster row's scope is not in the supplied scopePaths, + /// the diagnostic callback must fire so the caller can surface a warning. Silently dropping the + /// grant is the production hazard the finding flagged. /// [Fact] public void Build_Missing_ScopePath_Entry_Invokes_Diagnostic_Callback() { - var paths = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - ["line-known"] = new(new[] { "ns", "area-1", "line-known" }), - }; + var paths = ScopePaths( + (NodeAclScopeKind.UnsLine, "line-known", UnsPath("area-1", "line-known"))); // Row references a line that is NOT in the path lookup. + var orphan = Row("cn=ops", NodeAclScopeKind.UnsLine, "line-orphan", NodePermissions.Read); var rows = new[] { - Row("cn=ops", NodeAclScopeKind.UnsLine, "line-orphan", NodePermissions.Read), + orphan, Row("cn=ops", NodeAclScopeKind.UnsLine, "line-known", NodePermissions.Read), }; var diagnostics = new List(); @@ -114,45 +188,165 @@ public sealed class PermissionTrieBuilderTests var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths, diagnostics.Add); diagnostics.Count.ShouldBe(1, "exactly one row had no matching scope-path entry"); + diagnostics[0].NodeAclId.ShouldBe(orphan.NodeAclId); diagnostics[0].ScopeId.ShouldBe("line-orphan"); diagnostics[0].ScopeKind.ShouldBe(NodeAclScopeKind.UnsLine); - diagnostics[0].Reason.ShouldBe(PermissionTrieBuildDiagnosticReason.MissingScopePath); + diagnostics[0].Reason.ShouldBe(PermissionTrieBuildDiagnosticReason.UnresolvedScope); + + trie.CollectMatches(UnsScope("c1", "area-1", "line-known", "eq", "tag"), ["cn=ops"]) + .Count.ShouldBe(1, "the resolvable row is still placed"); } - /// Verifies that no diagnostic is emitted when all sub-cluster rows have ScopePaths. + /// + /// The scope-path lookup is keyed on (Kind, ScopeId), not the bare id: an entry filed under + /// one kind must not resolve a row of another kind that happens to share the id. ScopeId is + /// an unconstrained string, so id collisions across entity types are possible and resolving one + /// through the other is a silent wrong-node authorization. + /// [Fact] - public void Build_No_Diagnostic_When_All_Sub_Cluster_Rows_Have_ScopePaths() + public void Build_ScopePath_Lookup_Is_Keyed_On_Kind_As_Well_As_Id() { - var paths = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - ["line-A"] = new(new[] { "ns", "area-1", "line-A" }), - ["line-B"] = new(new[] { "ns", "area-1", "line-B" }), - }; + var paths = ScopePaths( + (NodeAclScopeKind.UnsLine, "shared-id", UnsPath("area-1", "line-1"))); + var rows = new[] { Row("cn=ops", NodeAclScopeKind.Equipment, "shared-id", NodePermissions.Read) }; + var diagnostics = new List(); + + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths, diagnostics.Add); + + diagnostics.Count.ShouldBe(1); + diagnostics[0].Reason.ShouldBe(PermissionTrieBuildDiagnosticReason.UnresolvedScope); + trie.CollectMatches(UnsScope("c1", "area-1", "line-1", "eq", "tag"), ["cn=ops"]) + .ShouldBeEmpty("an Equipment row must not borrow a UnsLine entry's path"); + } + + /// + /// Verifies that a diagnostic fires for the unresolvable row and for that row only. The negative + /// half on its own ("no diagnostics were emitted") passes for a Build that never emits any, + /// so the positive control lives in the same build rather than in a separate test. + /// + [Fact] + public void Build_Diagnostic_Fires_For_The_Unresolvable_Row_Only() + { + var paths = ScopePaths( + (NodeAclScopeKind.UnsLine, "line-A", UnsPath("area-1", "line-A")), + (NodeAclScopeKind.UnsLine, "line-B", UnsPath("area-1", "line-B"))); + var orphan = Row("cn=ops", NodeAclScopeKind.UnsLine, "line-orphan", NodePermissions.Read); var rows = new[] { - Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Read), // cluster-level — no descent + Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Read), // cluster-level — never descends Row("cn=ops", NodeAclScopeKind.UnsLine, "line-A", NodePermissions.Read), Row("cn=ops", NodeAclScopeKind.UnsLine, "line-B", NodePermissions.Read), + orphan, // positive control }; var diagnostics = new List(); - PermissionTrieBuilder.Build("c1", 1, rows, paths, diagnostics.Add); + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths, diagnostics.Add); - diagnostics.ShouldBeEmpty("no rows are missing a scope-path entry"); + diagnostics.Count.ShouldBe(1, "only the orphan row is unresolvable"); + diagnostics[0].NodeAclId.ShouldBe(orphan.NodeAclId, + "the cluster row and the two resolvable line rows must not emit a diagnostic"); + + // …and the rows that did resolve are genuinely placed, so "one diagnostic" is not being reached + // by dropping everything. + trie.CollectMatches(UnsScope("c1", "area-1", "line-A", "eq", "tag"), ["cn=ops"]) + .Count.ShouldBe(2, "the cluster grant plus the line-A grant"); + trie.CollectMatches(UnsScope("c1", "area-1", "line-B", "eq", "tag"), ["cn=ops"]) + .Count.ShouldBe(2, "the cluster grant plus the line-B grant"); } - /// Verifies that diagnostic callback is optional when ScopePaths is null. - [Fact] - public void Build_Diagnostic_Callback_Optional_When_ScopePaths_Null() + /// + /// Replaces Build_Diagnostic_Callback_Optional_When_ScopePaths_Null — scopePaths is + /// required now, so the mode that test described no longer exists. What still needs pinning is the + /// other authoring mistake: a sub-cluster row carrying no ScopeId names nothing, and used to + /// vanish with no signal at all. + /// + [Theory] + [InlineData(null)] + [InlineData("")] + public void Build_Sub_Cluster_Row_Without_ScopeId_Emits_MissingScopeId(string? scopeId) { - // No diagnostics callback should fire when scopePaths itself is null — that's the - // "deterministic-test fallback" mode, not a production drop. - var rows = new[] { Row("cn=ops", NodeAclScopeKind.UnsLine, "line-42", NodePermissions.Read) }; + var row = Row("cn=ops", NodeAclScopeKind.UnsLine, scopeId, NodePermissions.Read); var diagnostics = new List(); - PermissionTrieBuilder.Build("c1", 1, rows, scopePaths: null, diagnostic: diagnostics.Add); + var trie = PermissionTrieBuilder.Build( + "c1", 1, [row], new Dictionary>(), diagnostics.Add); - diagnostics.ShouldBeEmpty( - "scopePaths=null is the explicit test-fallback mode and must not emit per-row warnings"); + diagnostics.Count.ShouldBe(1); + diagnostics[0].NodeAclId.ShouldBe(row.NodeAclId); + diagnostics[0].ScopeId.ShouldBeNull("the row had no scope id to report"); + diagnostics[0].Reason.ShouldBe(PermissionTrieBuildDiagnosticReason.MissingScopeId); + trie.Root.Grants.ShouldBeEmpty("a scope-less sub-cluster row must not fall through to the cluster root"); + } + + /// + /// A row never consults scopePaths and never emits a + /// diagnostic, even though it carries a null ScopeId — that is the one kind for which a null + /// id is correct. + /// + [Fact] + public void Build_Cluster_Row_With_Null_ScopeId_Is_Placed_Without_Diagnostic() + { + var rows = new[] { Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Read) }; + var diagnostics = new List(); + + var trie = PermissionTrieBuilder.Build( + "c1", 1, rows, new Dictionary>(), diagnostics.Add); + + diagnostics.ShouldBeEmpty(); + trie.Root.Grants.Count.ShouldBe(1); + } + + /// + /// A scope that resolves to several paths — a raw tag plus its UNS projections — plants the grant + /// at every one of them. A write routes to the same driver reference through either NodeId, so a + /// grant covering only one path would be bypassable through the other. + /// + [Fact] + public void Build_Multi_Path_Scope_Plants_The_Grant_At_Every_Path() + { + var paths = new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.Tag, "tag-1")] = new[] + { + new NodeAclPath(RawPath("Plant", "Modbus", "dev1", "Speed")), + new NodeAclPath(UnsPath("area-1", "line-1", "eq-A", "Speed")), + new NodeAclPath(UnsPath("area-1", "line-1", "eq-B", "MotorSpeed")), + }, + }; + var rows = new[] { Row("cn=ops", NodeAclScopeKind.Tag, "tag-1", NodePermissions.WriteOperate) }; + + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + trie.CollectMatches(RawScope("c1", "Plant", "Modbus", "dev1", "Speed"), ["cn=ops"]) + .Count.ShouldBe(1, "the raw NodeId"); + trie.CollectMatches(UnsScope("c1", "area-1", "line-1", "eq-A", "Speed"), ["cn=ops"]) + .Count.ShouldBe(1, "the first UNS projection"); + trie.CollectMatches(UnsScope("c1", "area-1", "line-1", "eq-B", "MotorSpeed"), ["cn=ops"]) + .Count.ShouldBe(1, "the second UNS projection, under its own effective name"); + trie.CollectMatches(UnsScope("c1", "area-1", "line-1", "eq-B", "Speed"), ["cn=ops"]) + .ShouldBeEmpty("eq-B projects the tag as MotorSpeed; the eq-A name must not match there"); + } + + /// + /// An empty path list for a resolved key is treated the same as an absent key — dropped with an + /// diagnostic rather than + /// silently planting the grant at the cluster root, which is where a zero-segment walk would leave + /// it. + /// + [Fact] + public void Build_Empty_Path_List_Is_Treated_As_Unresolved() + { + var paths = new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.Equipment, "eq-A")] = Array.Empty(), + }; + var rows = new[] { Row("cn=ops", NodeAclScopeKind.Equipment, "eq-A", NodePermissions.Read) }; + var diagnostics = new List(); + + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths, diagnostics.Add); + + diagnostics.Count.ShouldBe(1); + diagnostics[0].Reason.ShouldBe(PermissionTrieBuildDiagnosticReason.UnresolvedScope); + trie.Root.Grants.ShouldBeEmpty("an empty path list must not become a cluster-wide grant"); } } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieTests.cs index fcf4ec74..b1dddbf2 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/PermissionTrieTests.cs @@ -6,9 +6,18 @@ using ZB.MOM.WW.OtOpcUa.Core.Authorization; namespace ZB.MOM.WW.OtOpcUa.Core.Tests.Authorization; +/// +/// Coverage for — the walk itself. Post-#520-stage-3 the +/// walk is cluster → realm → …variable-length node path…, so every non-cluster test here +/// supplies a real scopePaths entry whose path starts with a realm segment and drives a +/// chain through it. +/// [Trait("Category", "Unit")] public sealed class PermissionTrieTests { + private static readonly IReadOnlyDictionary> NoScopePaths = + new Dictionary>(); + private static NodeAcl Row(string group, NodeAclScopeKind scope, string? scopeId, NodePermissions flags, string clusterId = "c1") => new() { @@ -21,27 +30,36 @@ public sealed class PermissionTrieTests PermissionFlags = flags, }; - private static NodeScope EquipmentTag(string cluster, string ns, string area, string line, string equip, string tag) => - new() - { - ClusterId = cluster, - NamespaceId = ns, - UnsAreaId = area, - UnsLineId = line, - EquipmentId = equip, - TagId = tag, - Kind = NodeHierarchyKind.Equipment, - }; + /// One scope resolving to one path. + private static IReadOnlyDictionary> ScopePaths( + params (NodeAclScopeKind Kind, string ScopeId, string[] Segments)[] entries) => + entries.ToDictionary( + e => new AclScopeRef(e.Kind, e.ScopeId), + e => (IReadOnlyList)new[] { new NodeAclPath(e.Segments) }); + + /// A UNS-realm trie path: uns / …segments…. + private static string[] UnsPath(params string[] segments) => [NodeAclRealms.Uns, .. segments]; + + /// A raw-realm trie path: raw / …segments…. + private static string[] RawPath(params string[] segments) => [NodeAclRealms.Raw, .. segments]; + + /// A node in the UNS realm addressed by its path below the realm segment. + private static NodeScope UnsScope(string cluster, params string[] segments) => + new() { ClusterId = cluster, Kind = NodeHierarchyKind.Uns, Segments = segments }; + + /// A node in the raw realm addressed by its path below the realm segment. + private static NodeScope RawScope(string cluster, params string[] segments) => + new() { ClusterId = cluster, Kind = NodeHierarchyKind.Raw, Segments = segments }; /// Verifies cluster-level grant cascades to every tag. [Fact] public void ClusterLevelGrant_Cascades_ToEveryTag() { var rows = new[] { Row("cn=ops", NodeAclScopeKind.Cluster, scopeId: null, NodePermissions.Read) }; - var trie = PermissionTrieBuilder.Build("c1", 1, rows); + var trie = PermissionTrieBuilder.Build("c1", 1, rows, NoScopePaths); var matches = trie.CollectMatches( - EquipmentTag("c1", "ns", "area1", "line1", "eq1", "tag1"), + UnsScope("c1", "area1", "line1", "eq1", "tag1"), ["cn=ops"]); matches.Count.ShouldBe(1); @@ -53,38 +71,44 @@ public sealed class PermissionTrieTests [Fact] public void EquipmentScope_DoesNotLeak_ToSibling() { - var paths = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - ["eq-A"] = new(new[] { "ns", "area1", "line1", "eq-A" }), - }; + var paths = ScopePaths( + (NodeAclScopeKind.Equipment, "eq-A", UnsPath("area1", "line1", "eq-A"))); var rows = new[] { Row("cn=ops", NodeAclScopeKind.Equipment, "eq-A", NodePermissions.Read) }; var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); - var matchA = trie.CollectMatches(EquipmentTag("c1", "ns", "area1", "line1", "eq-A", "tag1"), ["cn=ops"]); - var matchB = trie.CollectMatches(EquipmentTag("c1", "ns", "area1", "line1", "eq-B", "tag1"), ["cn=ops"]); + var matchA = trie.CollectMatches(UnsScope("c1", "area1", "line1", "eq-A", "tag1"), ["cn=ops"]); + var matchB = trie.CollectMatches(UnsScope("c1", "area1", "line1", "eq-B", "tag1"), ["cn=ops"]); matchA.Count.ShouldBe(1); matchB.ShouldBeEmpty("grant at eq-A must not apply to sibling eq-B"); } - /// Verifies multiple groups union their permission flags. + /// + /// Verifies that every principal-matching grant is collected once, with its own flags left + /// untouched. deliberately does NOT combine the flag + /// bits — the OR lives in TriePermissionEvaluator.Authorize and is pinned there by + /// TriePermissionEvaluatorTests.Union_Across_Levels_Ors_Flags_From_Every_Match. (Renamed + /// from MultiGroup_Union_OrsPermissionFlags, which OR-ed the flags in the test body and so + /// asserted nothing about production behaviour.) + /// [Fact] - public void MultiGroup_Union_OrsPermissionFlags() + public void MultiGroup_Matches_Are_Collected_PerGrant_Without_Combining() { var rows = new[] { Row("cn=readers", NodeAclScopeKind.Cluster, null, NodePermissions.Read), Row("cn=writers", NodeAclScopeKind.Cluster, null, NodePermissions.WriteOperate), }; - var trie = PermissionTrieBuilder.Build("c1", 1, rows); + var trie = PermissionTrieBuilder.Build("c1", 1, rows, NoScopePaths); var matches = trie.CollectMatches( - EquipmentTag("c1", "ns", "area1", "line1", "eq1", "tag1"), + UnsScope("c1", "area1", "line1", "eq1", "tag1"), ["cn=readers", "cn=writers"]); matches.Count.ShouldBe(2); - var combined = matches.Aggregate(NodePermissions.None, (acc, m) => acc | m.PermissionFlags); - combined.ShouldBe(NodePermissions.Read | NodePermissions.WriteOperate); + matches.Select(m => m.Principal).ShouldBe(["cn=readers", "cn=writers"], ignoreOrder: true); + matches.Single(m => m.Principal == "cn=readers").PermissionFlags.ShouldBe(NodePermissions.Read); + matches.Single(m => m.Principal == "cn=writers").PermissionFlags.ShouldBe(NodePermissions.WriteOperate); } /// Verifies no matching group returns empty. @@ -92,57 +116,52 @@ public sealed class PermissionTrieTests public void NoMatchingGroup_ReturnsEmpty() { var rows = new[] { Row("cn=different", NodeAclScopeKind.Cluster, null, NodePermissions.Read) }; - var trie = PermissionTrieBuilder.Build("c1", 1, rows); + var trie = PermissionTrieBuilder.Build("c1", 1, rows, NoScopePaths); var matches = trie.CollectMatches( - EquipmentTag("c1", "ns", "area1", "line1", "eq1", "tag1"), + UnsScope("c1", "area1", "line1", "eq1", "tag1"), ["cn=ops"]); matches.ShouldBeEmpty(); } /// - /// Galaxy is now a standard Equipment-kind driver: Galaxy points are ordinary equipment - /// tags, so a grant on a Galaxy equipment resolves via the Equipment walk and must not leak - /// to a sibling Galaxy equipment. + /// Galaxy is a standard Equipment-kind driver: Galaxy points are ordinary equipment tags, so a + /// grant on a Galaxy equipment resolves via the UNS walk and must not leak to a sibling Galaxy + /// equipment. /// [Fact] public void Galaxy_EquipmentScope_Grant_DoesNotLeak_To_Sibling_Equipment() { - var paths = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - ["gal-eq-A"] = new(new[] { "ns-gal", "area1", "line1", "gal-eq-A" }), - }; + var paths = ScopePaths( + (NodeAclScopeKind.Equipment, "gal-eq-A", UnsPath("area1", "line1", "gal-eq-A"))); var rows = new[] { Row("cn=ops", NodeAclScopeKind.Equipment, "gal-eq-A", NodePermissions.Read) }; var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); - var matchA = trie.CollectMatches(EquipmentTag("c1", "ns-gal", "area1", "line1", "gal-eq-A", "tag1"), ["cn=ops"]); - var matchB = trie.CollectMatches(EquipmentTag("c1", "ns-gal", "area1", "line1", "gal-eq-B", "tag1"), ["cn=ops"]); + var matchA = trie.CollectMatches(UnsScope("c1", "area1", "line1", "gal-eq-A", "tag1"), ["cn=ops"]); + var matchB = trie.CollectMatches(UnsScope("c1", "area1", "line1", "gal-eq-B", "tag1"), ["cn=ops"]); matchA.Count.ShouldBe(1); matchB.ShouldBeEmpty("grant at gal-eq-A must not apply to sibling gal-eq-B"); } /// - /// A grant on a Galaxy equipment tag is reported with its true Equipment-walk structural - /// scope (Equipment), not a Galaxy-specific scope — Galaxy now flows through the same - /// Equipment permission walk as every other equipment tag. + /// A grant on a Galaxy equipment tag is reported with the authored row's Equipment scope kind — + /// Galaxy flows through the same UNS permission walk as every other equipment tag. /// [Fact] public void Galaxy_EquipmentScope_Grant_Reports_Equipment_Scope() { - var paths = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - ["gal-eq"] = new(new[] { "ns-gal", "area1", "line1", "gal-eq" }), - }; + var paths = ScopePaths( + (NodeAclScopeKind.Equipment, "gal-eq", UnsPath("area1", "line1", "gal-eq"))); var rows = new[] { Row("cn=ops", NodeAclScopeKind.Equipment, "gal-eq", NodePermissions.Read) }; var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); - var matches = trie.CollectMatches(EquipmentTag("c1", "ns-gal", "area1", "line1", "gal-eq", "tag1"), ["cn=ops"]); + var matches = trie.CollectMatches(UnsScope("c1", "area1", "line1", "gal-eq", "tag1"), ["cn=ops"]); matches.Count.ShouldBe(1); matches[0].Scope.ShouldBe(NodeAclScopeKind.Equipment, - "Galaxy equipment grants resolve via the Equipment walk and report Equipment scope"); + "Galaxy equipment grants resolve via the UNS walk and report the row's Equipment scope"); } /// Verifies cross-cluster grant does not leak. @@ -150,16 +169,21 @@ public sealed class PermissionTrieTests public void CrossCluster_Grant_DoesNotLeak() { var rows = new[] { Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Read, clusterId: "c-other") }; - var trie = PermissionTrieBuilder.Build("c1", 1, rows); + var trie = PermissionTrieBuilder.Build("c1", 1, rows, NoScopePaths); var matches = trie.CollectMatches( - EquipmentTag("c1", "ns", "area1", "line1", "eq1", "tag1"), + UnsScope("c1", "area1", "line1", "eq1", "tag1"), ["cn=ops"]); matches.ShouldBeEmpty("rows for cluster c-other must not land in c1's trie"); } - /// Verifies build is idempotent. + /// + /// Verifies build is idempotent, against an independent expectation rather than by + /// comparing one build's output to the other's. The previous form asserted + /// trie1.Count.ShouldBe(trie2.Count), which a Build that dropped every row passed as + /// 0 == 0; the absolute counts below go red the moment placement regresses. + /// [Fact] public void Build_IsIdempotent() { @@ -167,13 +191,187 @@ public sealed class PermissionTrieTests { Row("cn=a", NodeAclScopeKind.Cluster, null, NodePermissions.Read), Row("cn=b", NodeAclScopeKind.Cluster, null, NodePermissions.WriteOperate), + Row("cn=a", NodeAclScopeKind.Equipment, "eq-A", NodePermissions.WriteOperate), }; + var paths = ScopePaths( + (NodeAclScopeKind.Equipment, "eq-A", UnsPath("area1", "line1", "eq-A"))); + var scope = UnsScope("c1", "area1", "line1", "eq-A", "tag1"); - var trie1 = PermissionTrieBuilder.Build("c1", 1, rows); - var trie2 = PermissionTrieBuilder.Build("c1", 1, rows); + var trie1 = PermissionTrieBuilder.Build("c1", 1, rows, paths); + var trie2 = PermissionTrieBuilder.Build("c1", 1, rows, paths); - trie1.Root.Grants.Count.ShouldBe(trie2.Root.Grants.Count); - trie1.ClusterId.ShouldBe(trie2.ClusterId); - trie1.GenerationId.ShouldBe(trie2.GenerationId); + foreach (var trie in new[] { trie1, trie2 }) + { + trie.ClusterId.ShouldBe("c1"); + trie.GenerationId.ShouldBe(1); + trie.Root.Grants.Count.ShouldBe(2, "both cluster rows land at the root"); + trie.CollectMatches(scope, ["cn=a"]).Count.ShouldBe(2, "one cluster grant + the eq-A grant"); + trie.CollectMatches(scope, ["cn=b"]).Count.ShouldBe(1, "cn=b holds only the cluster grant"); + } + } + + /// + /// A grant planted at a raw container matches a tag nested arbitrarily deep beneath it — two + /// nested folders and two nested tag groups here — and does not match a sibling device. + /// The sibling leg is the negative control: without it a walk that ignored + /// entirely would still pass the positive leg. + /// + [Fact] + public void RawDevice_Grant_Matches_Deeply_Nested_Tag_And_Not_A_Sibling_Device() + { + var paths = ScopePaths( + (NodeAclScopeKind.RawDevice, "dev-1", RawPath("Plant", "CellA", "Modbus", "dev1"))); + var rows = new[] { Row("cn=ops", NodeAclScopeKind.RawDevice, "dev-1", NodePermissions.Read) }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + // raw / Plant / CellA / Modbus / dev1 / Motors / Drive / Speed — 7 segments below the realm. + var beneath = trie.CollectMatches( + RawScope("c1", "Plant", "CellA", "Modbus", "dev1", "Motors", "Drive", "Speed"), + ["cn=ops"]); + + var sibling = trie.CollectMatches( + RawScope("c1", "Plant", "CellA", "Modbus", "dev2", "Motors", "Drive", "Speed"), + ["cn=ops"]); + + beneath.Count.ShouldBe(1, "a container grant cascades to every node beneath it, at any depth"); + beneath[0].Scope.ShouldBe(NodeAclScopeKind.RawDevice); + sibling.ShouldBeEmpty("grant at dev1 must not reach sibling device dev2"); + } + + /// + /// Regression guard for the StringComparer.Ordinal children comparer (#520 stage 3): a + /// grant authored on Speed must not match a scope for speed. Raw sibling names are + /// unique only under an ordinal comparer, so both spellings can exist under one device and the + /// previous OrdinalIgnoreCase map silently granted one through the other. The positive leg + /// is asserted alongside so the negative cannot pass by the walk simply never matching. + /// + [Fact] + public void Segment_Matching_Is_Case_Sensitive() + { + var paths = ScopePaths( + (NodeAclScopeKind.Tag, "tag-speed", RawPath("Plant", "Modbus", "dev1", "Speed"))); + var rows = new[] { Row("cn=ops", NodeAclScopeKind.Tag, "tag-speed", NodePermissions.Read) }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + var exact = trie.CollectMatches(RawScope("c1", "Plant", "Modbus", "dev1", "Speed"), ["cn=ops"]); + var caseDrifted = trie.CollectMatches(RawScope("c1", "Plant", "Modbus", "dev1", "speed"), ["cn=ops"]); + + exact.Count.ShouldBe(1, "the exact spelling must match, or the negative leg proves nothing"); + caseDrifted.ShouldBeEmpty( + "segment matching is case-sensitive ordinal — 'Speed' and 'speed' are distinct raw siblings"); + } + + /// + /// Regression guard for the case-sensitive comparer on the container levels too: a grant + /// under device Dev1 must not cascade into a scope walking dev1. + /// + [Fact] + public void Container_Segment_Matching_Is_Case_Sensitive() + { + var paths = ScopePaths( + (NodeAclScopeKind.RawDevice, "dev-1", RawPath("Plant", "Modbus", "Dev1"))); + var rows = new[] { Row("cn=ops", NodeAclScopeKind.RawDevice, "dev-1", NodePermissions.Read) }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + trie.CollectMatches(RawScope("c1", "Plant", "Modbus", "Dev1", "Speed"), ["cn=ops"]) + .Count.ShouldBe(1); + trie.CollectMatches(RawScope("c1", "Plant", "Modbus", "dev1", "Speed"), ["cn=ops"]) + .ShouldBeEmpty("a case-drifted container segment must not cascade"); + } + + /// + /// The realm is a hard gate at level 1: a grant planted in the raw realm must not match a scope in + /// the UNS realm, even when the segments below the realm are identical. This pins that the walk + /// derives its level-1 key from via + /// rather than skipping the level. + /// + [Fact] + public void Grant_In_One_Realm_DoesNot_Match_A_Scope_In_The_Other() + { + var paths = ScopePaths( + (NodeAclScopeKind.RawDevice, "dev-1", RawPath("Plant", "Modbus", "dev1"))); + var rows = new[] { Row("cn=ops", NodeAclScopeKind.RawDevice, "dev-1", NodePermissions.Read) }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + trie.CollectMatches(RawScope("c1", "Plant", "Modbus", "dev1", "Speed"), ["cn=ops"]) + .Count.ShouldBe(1, "the raw scope must match, or the cross-realm leg proves nothing"); + trie.CollectMatches(UnsScope("c1", "Plant", "Modbus", "dev1", "Speed"), ["cn=ops"]) + .ShouldBeEmpty("a raw-realm grant must not be reachable from a UNS scope"); + } + + /// + /// A row planted at the realm segment itself grants every + /// node in that realm and nothing in the other one. + /// + [Fact] + public void NamespaceScope_Grant_Covers_Its_Realm_Only() + { + var paths = ScopePaths( + (NodeAclScopeKind.Namespace, NodeAclRealms.Raw, [NodeAclRealms.Raw])); + var rows = new[] { Row("cn=ops", NodeAclScopeKind.Namespace, NodeAclRealms.Raw, NodePermissions.Read) }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + var inRaw = trie.CollectMatches(RawScope("c1", "Plant", "Modbus", "dev1", "Speed"), ["cn=ops"]); + var inUns = trie.CollectMatches(UnsScope("c1", "area1", "line1", "eq1", "tag1"), ["cn=ops"]); + + inRaw.Count.ShouldBe(1); + inRaw[0].Scope.ShouldBe(NodeAclScopeKind.Namespace); + inUns.ShouldBeEmpty("a raw-namespace grant must not cover the UNS realm"); + } + + /// + /// Provenance is the granting row's , not one derived from the + /// structural depth the grant sits at. Two rows of different kinds planted at the same trie node + /// must report their own kinds — under any depth-derived scheme they would report the same one. + /// + [Fact] + public void MatchedGrant_Reports_The_Rows_Own_ScopeKind_Not_A_Depth_Derived_One() + { + var samePath = RawPath("Plant", "Modbus", "dev1"); + var paths = new Dictionary> + { + [new AclScopeRef(NodeAclScopeKind.RawDevice, "dev-1")] = new[] { new NodeAclPath(samePath) }, + [new AclScopeRef(NodeAclScopeKind.RawTagGroup, "grp-1")] = new[] { new NodeAclPath(samePath) }, + }; + var rows = new[] + { + Row("cn=ops", NodeAclScopeKind.RawDevice, "dev-1", NodePermissions.Read), + Row("cn=ops", NodeAclScopeKind.RawTagGroup, "grp-1", NodePermissions.WriteOperate), + }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + var matches = trie.CollectMatches(RawScope("c1", "Plant", "Modbus", "dev1", "Speed"), ["cn=ops"]); + + matches.Count.ShouldBe(2); + matches.Select(m => m.Scope).ShouldBe( + [NodeAclScopeKind.RawDevice, NodeAclScopeKind.RawTagGroup], + ignoreOrder: true, + "each grant carries the scope kind its own row was authored with"); + } + + /// + /// A scope addressing the realm itself (no segments) collects the realm-level and cluster-level + /// grants and nothing planted below it. + /// + [Fact] + public void RealmScope_With_No_Segments_Collects_Only_Cluster_And_Realm_Grants() + { + var paths = ScopePaths( + (NodeAclScopeKind.Namespace, NodeAclRealms.Raw, [NodeAclRealms.Raw]), + (NodeAclScopeKind.RawDevice, "dev-1", RawPath("Plant", "Modbus", "dev1"))); + var rows = new[] + { + Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Browse), + Row("cn=ops", NodeAclScopeKind.Namespace, NodeAclRealms.Raw, NodePermissions.Read), + Row("cn=ops", NodeAclScopeKind.RawDevice, "dev-1", NodePermissions.WriteOperate), + }; + var trie = PermissionTrieBuilder.Build("c1", 1, rows, paths); + + var matches = trie.CollectMatches(RawScope("c1"), ["cn=ops"]); + + matches.Select(m => m.Scope).ShouldBe( + [NodeAclScopeKind.Cluster, NodeAclScopeKind.Namespace], + ignoreOrder: true, + "the device grant sits below the realm and must not be collected at the realm"); } } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/TriePermissionEvaluatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/TriePermissionEvaluatorTests.cs index 76b6a2d9..f52481ba 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/TriePermissionEvaluatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/TriePermissionEvaluatorTests.cs @@ -13,6 +13,9 @@ public sealed class TriePermissionEvaluatorTests private static readonly DateTime Now = new(2026, 4, 19, 12, 0, 0, DateTimeKind.Utc); private readonly FakeTimeProvider _time = new(); + private static readonly IReadOnlyDictionary> NoScopePaths = + new Dictionary>(); + private sealed class FakeTimeProvider : TimeProvider { /// Gets or sets the current UTC time. @@ -46,22 +49,42 @@ public sealed class TriePermissionEvaluatorTests MembershipVersion = 1, }; + /// One scope resolving to one path. + private static IReadOnlyDictionary> ScopePaths( + params (NodeAclScopeKind Kind, string ScopeId, string[] Segments)[] entries) => + entries.ToDictionary( + e => new AclScopeRef(e.Kind, e.ScopeId), + e => (IReadOnlyList)new[] { new NodeAclPath(e.Segments) }); + + /// A UNS-realm trie path: uns / …segments…. + private static string[] UnsPath(params string[] segments) => [NodeAclRealms.Uns, .. segments]; + + /// A raw-realm trie path: raw / …segments…. + private static string[] RawPath(params string[] segments) => [NodeAclRealms.Raw, .. segments]; + + /// The default UNS node the flag/staleness/generation tests evaluate against. private static NodeScope Scope(string cluster = "c1") => new() { ClusterId = cluster, - NamespaceId = "ns", - UnsAreaId = "area", - UnsLineId = "line", - EquipmentId = "eq", - TagId = "tag", - Kind = NodeHierarchyKind.Equipment, + Kind = NodeHierarchyKind.Uns, + Segments = ["area", "line", "eq", "tag"], }; - private TriePermissionEvaluator MakeEvaluator(NodeAcl[] rows) + /// A node in the UNS realm addressed by its path below the realm segment. + private static NodeScope UnsScope(string cluster, params string[] segments) => + new() { ClusterId = cluster, Kind = NodeHierarchyKind.Uns, Segments = segments }; + + /// A node in the raw realm addressed by its path below the realm segment. + private static NodeScope RawScope(string cluster, params string[] segments) => + new() { ClusterId = cluster, Kind = NodeHierarchyKind.Raw, Segments = segments }; + + private TriePermissionEvaluator MakeEvaluator( + NodeAcl[] rows, + IReadOnlyDictionary>? scopePaths = null) { var cache = new PermissionTrieCache(); - cache.Install(PermissionTrieBuilder.Build("c1", 1, rows)); + cache.Install(PermissionTrieBuilder.Build("c1", 1, rows, scopePaths ?? NoScopePaths)); return new TriePermissionEvaluator(cache, _time); } @@ -160,8 +183,8 @@ public sealed class TriePermissionEvaluatorTests // The evaluator must still honour the session's bound generation 1, not generation 2. var gen1Row = Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Read); var cache = new PermissionTrieCache(); - cache.Install(PermissionTrieBuilder.Build("c1", 1, [gen1Row])); - cache.Install(PermissionTrieBuilder.Build("c1", 2, [])); // gen 2 — grant revoked, now current + cache.Install(PermissionTrieBuilder.Build("c1", 1, [gen1Row], NoScopePaths)); + cache.Install(PermissionTrieBuilder.Build("c1", 2, [], NoScopePaths)); // gen 2 — grant revoked, now current var evaluator = new TriePermissionEvaluator(cache, _time); var decision = evaluator.Authorize( @@ -180,7 +203,7 @@ public sealed class TriePermissionEvaluatorTests // The evaluator must fail closed rather than silently using the current generation. var gen5Row = Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Read); var cache = new PermissionTrieCache(); - cache.Install(PermissionTrieBuilder.Build("c1", 5, [gen5Row])); + cache.Install(PermissionTrieBuilder.Build("c1", 5, [gen5Row], NoScopePaths)); var evaluator = new TriePermissionEvaluator(cache, _time); // Session bound to generation 1, which was never installed / has been pruned. @@ -234,4 +257,144 @@ public sealed class TriePermissionEvaluatorTests TriePermissionEvaluator.MapOperationToPermission(op); } } + + /// + /// End-to-end coverage of the sub-cluster walk: an Equipment-scoped grant authorizes a node + /// beneath that equipment, and denies the same operation on a sibling equipment. Every other test + /// in this class uses a cluster-scoped grant, which the walk collects at level 0 before it ever + /// descends — so without this pair nothing here exercises at all. + /// + [Fact] + public void SubCluster_Grant_Allows_Beneath_Its_Equipment_And_Denies_A_Sibling() + { + var evaluator = MakeEvaluator( + [Row("cn=ops", NodeAclScopeKind.Equipment, "eq-A", NodePermissions.WriteOperate)], + ScopePaths((NodeAclScopeKind.Equipment, "eq-A", UnsPath("area-1", "line-1", "eq-A")))); + + var onEquipment = evaluator.Authorize( + Session(["cn=ops"]), OpcUaOperation.WriteOperate, + UnsScope("c1", "area-1", "line-1", "eq-A", "Setpoint")); + + var onSibling = evaluator.Authorize( + Session(["cn=ops"]), OpcUaOperation.WriteOperate, + UnsScope("c1", "area-1", "line-1", "eq-B", "Setpoint")); + + onEquipment.IsAllowed.ShouldBeTrue("the grant sits on the equipment the scope walks through"); + onEquipment.Provenance.Count.ShouldBe(1); + onEquipment.Provenance[0].Scope.ShouldBe(NodeAclScopeKind.Equipment); + onSibling.IsAllowed.ShouldBeFalse("a grant on eq-A must not authorize eq-B"); + } + + /// + /// The same walk through the raw realm, over a path deep enough to have nested folders and nested + /// tag groups: a grant on the device authorizes a tag six levels below it, and not the sibling + /// device's tag. + /// + [Fact] + public void SubCluster_Grant_Walks_A_Deep_Raw_Path() + { + var evaluator = MakeEvaluator( + [Row("cn=ops", NodeAclScopeKind.RawDevice, "dev-1", NodePermissions.Read)], + ScopePaths((NodeAclScopeKind.RawDevice, "dev-1", RawPath("Plant", "CellA", "Modbus", "dev1")))); + + var beneath = evaluator.Authorize( + Session(["cn=ops"]), OpcUaOperation.Read, + RawScope("c1", "Plant", "CellA", "Modbus", "dev1", "Motors", "Drive", "Speed")); + + var sibling = evaluator.Authorize( + Session(["cn=ops"]), OpcUaOperation.Read, + RawScope("c1", "Plant", "CellA", "Modbus", "dev2", "Motors", "Drive", "Speed")); + + beneath.IsAllowed.ShouldBeTrue(); + beneath.Provenance[0].Scope.ShouldBe(NodeAclScopeKind.RawDevice, + "provenance names the row's own kind, not one derived from the depth it was planted at"); + sibling.IsAllowed.ShouldBeFalse("the grant is anchored at dev1"); + } + + /// + /// The permission OR lives here, in Authorize: flags from grants matched at different trie + /// levels combine into one bitmask. Both legs are asserted with the same session and scope, so + /// neither "use the first match" nor "use the last match" can pass. + /// + [Fact] + public void Union_Across_Levels_Ors_Flags_From_Every_Match() + { + var evaluator = MakeEvaluator( + [ + Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Read), + Row("cn=ops", NodeAclScopeKind.Equipment, "eq-A", NodePermissions.WriteOperate), + ], + ScopePaths((NodeAclScopeKind.Equipment, "eq-A", UnsPath("area-1", "line-1", "eq-A")))); + + var scope = UnsScope("c1", "area-1", "line-1", "eq-A", "Setpoint"); + + evaluator.Authorize(Session(["cn=ops"]), OpcUaOperation.Read, scope) + .IsAllowed.ShouldBeTrue("Read comes from the cluster-level grant"); + evaluator.Authorize(Session(["cn=ops"]), OpcUaOperation.WriteOperate, scope) + .IsAllowed.ShouldBeTrue("WriteOperate comes from the equipment-level grant"); + } + + /// + /// The union also spans principals: a session carrying two principals gets the OR of both their + /// grants, and neither principal alone carries both permissions. + /// + [Fact] + public void Union_Across_Principals_Ors_Flags() + { + NodeAcl[] rows = + [ + Row("cn=readers", NodeAclScopeKind.Cluster, null, NodePermissions.Read), + Row("cn=writers", NodeAclScopeKind.Cluster, null, NodePermissions.WriteOperate), + ]; + var evaluator = MakeEvaluator(rows); + + evaluator.Authorize(Session(["cn=readers", "cn=writers"]), OpcUaOperation.WriteOperate, Scope()) + .IsAllowed.ShouldBeTrue(); + evaluator.Authorize(Session(["cn=readers", "cn=writers"]), OpcUaOperation.Read, Scope()) + .IsAllowed.ShouldBeTrue(); + evaluator.Authorize(Session(["cn=readers"]), OpcUaOperation.WriteOperate, Scope()) + .IsAllowed.ShouldBeFalse("cn=readers alone carries no write bit"); + } + + /// + /// A grant resolved into the raw realm must not authorize the UNS node of the same name. The realm + /// is level 1 of the walk and is derived from , so a scope in the wrong + /// realm never reaches the grant. + /// + [Fact] + public void Grant_In_The_Raw_Realm_DoesNot_Authorize_A_Uns_Scope() + { + var evaluator = MakeEvaluator( + [Row("cn=ops", NodeAclScopeKind.RawDevice, "dev-1", NodePermissions.Read)], + ScopePaths((NodeAclScopeKind.RawDevice, "dev-1", RawPath("Plant", "Modbus", "dev1")))); + + evaluator.Authorize(Session(["cn=ops"]), OpcUaOperation.Read, + RawScope("c1", "Plant", "Modbus", "dev1", "Speed")) + .IsAllowed.ShouldBeTrue("the raw scope must be allowed, or the cross-realm leg proves nothing"); + + evaluator.Authorize(Session(["cn=ops"]), OpcUaOperation.Read, + UnsScope("c1", "Plant", "Modbus", "dev1", "Speed")) + .IsAllowed.ShouldBeFalse("a raw-realm grant must not be reachable from a UNS scope"); + } + + /// + /// A case-drifted scope segment must not reach a grant — the trie's children compare with + /// , matching OPC UA NodeId identity, so Speed and + /// speed are distinct nodes and a grant on one denies the other. + /// + [Fact] + public void Case_Drifted_Scope_Segment_Is_NotGranted() + { + var evaluator = MakeEvaluator( + [Row("cn=ops", NodeAclScopeKind.Tag, "tag-speed", NodePermissions.Read)], + ScopePaths((NodeAclScopeKind.Tag, "tag-speed", RawPath("Plant", "Modbus", "dev1", "Speed")))); + + evaluator.Authorize(Session(["cn=ops"]), OpcUaOperation.Read, + RawScope("c1", "Plant", "Modbus", "dev1", "Speed")) + .IsAllowed.ShouldBeTrue(); + + evaluator.Authorize(Session(["cn=ops"]), OpcUaOperation.Read, + RawScope("c1", "Plant", "Modbus", "dev1", "speed")) + .IsAllowed.ShouldBeFalse("segment matching is case-sensitive ordinal"); + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Authorization/PageAuthorizationGuardTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Authorization/PageAuthorizationGuardTests.cs index 4a21b837..b818fe63 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Authorization/PageAuthorizationGuardTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Authorization/PageAuthorizationGuardTests.cs @@ -44,8 +44,11 @@ public class PageAuthorizationGuardTests [typeof(Scripts)] = AdminUiPolicies.ConfigEditor, [typeof(ScriptEdit)] = AdminUiPolicies.ConfigEditor, - // ── FleetAdmin (1) ── + // ── FleetAdmin (2) ── [typeof(RoleGrants)] = AdminUiPolicies.FleetAdmin, + // Minting a key hands out a data-plane credential, and the secret is shown once at creation — + // the same authority level as editing role grants. + [typeof(ApiKeys)] = AdminUiPolicies.FleetAdmin, // ── Secrets (1): host wrapper for the ZB.MOM.WW.Secrets.Ui RCL page (lmxopcua#483) — // carries the RCL page's own policy, registered additively by AddAdminUI via diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ArtifactApiKeyStoreTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ArtifactApiKeyStoreTests.cs index cec25672..478b8c38 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ArtifactApiKeyStoreTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ArtifactApiKeyStoreTests.cs @@ -15,6 +15,12 @@ public sealed class ArtifactApiKeyStoreTests { private static readonly DateTime Now = new(2026, 7, 31, 12, 0, 0, DateTimeKind.Utc); + /// + /// No test here exercises a narrower-than-Cluster scope, so the resolved scope paths are empty. + /// + private static readonly IReadOnlyDictionary> NoScopePaths = + new Dictionary>(); + [Fact] public async Task A_node_that_has_not_been_deployed_to_authenticates_nothing() { @@ -29,7 +35,7 @@ public sealed class ArtifactApiKeyStoreTests var state = new DataPlaneAuthorityState(); state.Apply("cluster-a", "rev-1", [Key("abc", "scadabridge-dcl")], - [Acl("acl-1", "scadabridge-dcl", NodeAclScopeKind.Cluster, NodePermissions.WriteOperate)]); + [Acl("acl-1", "scadabridge-dcl", NodeAclScopeKind.Cluster, NodePermissions.WriteOperate)], NoScopePaths); var found = await new ArtifactApiKeyStore(state).FindAsync("abc", Now, default); @@ -41,16 +47,18 @@ public sealed class ArtifactApiKeyStoreTests [Fact] public async Task Only_cluster_scoped_grants_are_returned() { - // Matches ConfigDbApiKeyStore, which filters the same way in SQL. Narrower scopes are not yet - // enforceable (stage 5), and ApiKeyInterimRoleMapper deliberately under-grants rather than - // promoting a scoped grant's flags to server-wide authority. + // Narrower scopes are not yet enforceable (stage 5), and ApiKeyInterimRoleMapper deliberately + // under-grants rather than promoting a scoped grant's flags to server-wide authority. Scope kind + // is the only narrowing this store performs — cluster confinement already happened in + // ParseDataPlaneAuthority, which is where it differs from ConfigDbApiKeyStore (that one filters + // on cluster itself; see ConfigDbApiKeyStoreTests). var state = new DataPlaneAuthorityState(); state.Apply("cluster-a", "rev-1", [Key("abc", "p")], [ Acl("acl-cluster", "p", NodeAclScopeKind.Cluster, NodePermissions.Read), Acl("acl-equipment", "p", NodeAclScopeKind.Equipment, NodePermissions.WriteOperate), - ]); + ], NoScopePaths); var found = await new ArtifactApiKeyStore(state).FindAsync("abc", Now, default); @@ -65,12 +73,12 @@ public sealed class ArtifactApiKeyStoreTests var state = new DataPlaneAuthorityState(); var sut = new ArtifactApiKeyStore(state); - state.Apply("cluster-a", "rev-1", [Key("abc", "p")], []); + state.Apply("cluster-a", "rev-1", [Key("abc", "p")], [], NoScopePaths); (await sut.FindAsync("abc", Now, default)).ShouldNotBeNull(); var revoked = Key("abc", "p"); revoked.Enabled = false; - state.Apply("cluster-a", "rev-2", [revoked], []); + state.Apply("cluster-a", "rev-2", [revoked], [], NoScopePaths); (await sut.FindAsync("abc", Now, default)).ShouldBeNull(); } @@ -81,7 +89,8 @@ public sealed class ArtifactApiKeyStoreTests // The anonymous principal has grants but obviously no credential, so FindGrantsAsync is its path. var state = new DataPlaneAuthorityState(); state.Apply("cluster-a", "rev-1", [], - [Acl("acl-anon", OpcUaPrincipals.Anonymous, NodeAclScopeKind.Cluster, NodePermissions.Read)]); + [Acl("acl-anon", OpcUaPrincipals.Anonymous, NodeAclScopeKind.Cluster, NodePermissions.Read)], + NoScopePaths); var grants = await new ArtifactApiKeyStore(state).FindGrantsAsync(OpcUaPrincipals.Anonymous, default); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ConfigDbApiKeyStoreTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ConfigDbApiKeyStoreTests.cs new file mode 100644 index 00000000..0990e3d7 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/OpcUa/ConfigDbApiKeyStoreTests.cs @@ -0,0 +1,227 @@ +using Microsoft.EntityFrameworkCore; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Cluster; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.Host.OpcUa; + +namespace ZB.MOM.WW.OtOpcUa.Host.Tests.OpcUa; + +/// +/// The key source for a node WITH a ConfigDb (#520 stage 2) — an admin-role node, including a fused +/// admin+driver one. +/// +/// +/// +/// Why cluster confinement is tested here and not on ArtifactApiKeyStore. The two +/// stores read different things. The artifact store's snapshot was already narrowed to the node's +/// own cluster by DeploymentArtifact.ParseDataPlaneAuthority +/// (ClusterFilterMode.ScopeTo), so it cannot see another cluster's rows at all. This store +/// queries the fleet-wide ConfigDb, so the confinement is its own job — and until it was added, a +/// Cluster-scoped grant authored in ANY cluster conferred an interim server-wide role on a fused +/// central node. +/// +/// +/// The in-memory EF provider is the convention in the ControlPlane/Runtime test projects; nothing +/// here touches the central SQL Server. Note it compares strings with LINQ-to-Objects ordinal +/// semantics, NOT SQL Server's case-insensitive collation — which is exactly why the ClusterId +/// match is done explicitly with in the store +/// rather than left to the provider. pins it. +/// +/// +public sealed class ConfigDbApiKeyStoreTests +{ + private const string ThisCluster = "SITE-A"; + private const string OtherCluster = "SITE-B"; + private static readonly DateTime Now = new(2026, 7, 31, 12, 0, 0, DateTimeKind.Utc); + + /// + /// THE REGRESSION TEST. A Cluster-scoped grant authored in another cluster must not reach this + /// node — it would collapse into a server-wide WriteOperate role via + /// ApiKeyInterimRoleMapper, i.e. write authority granted in SITE-B taking effect in SITE-A. + /// + [Fact] + public async Task A_cluster_scoped_grant_from_another_cluster_confers_nothing() + { + var factory = NewInMemoryDbFactory(); + Seed(factory, + keys: [Key("abc", "scadabridge-dcl")], + acls: [Acl("acl-other", OtherCluster, "scadabridge-dcl", NodeAclScopeKind.Cluster, NodePermissions.WriteOperate)]); + + var found = await new ConfigDbApiKeyStore(factory, ThisCluster).FindAsync("abc", Now, default); + + // The KEY still resolves — keys are fleet-wide credentials (ApiKey carries no ClusterId); it is + // the grants that are cluster-bound. Asserting a null lookup here would pass for the wrong reason. + found.ShouldNotBeNull(); + found!.Grants.ShouldBeEmpty(); + } + + /// + /// The positive control for the test above: without it, a "fix" that returned no grants at all + /// would pass. + /// + [Fact] + public async Task A_cluster_scoped_grant_in_this_cluster_still_confers_its_permissions() + { + var factory = NewInMemoryDbFactory(); + Seed(factory, + keys: [Key("abc", "scadabridge-dcl")], + acls: + [ + Acl("acl-mine", ThisCluster, "scadabridge-dcl", NodeAclScopeKind.Cluster, NodePermissions.WriteOperate), + Acl("acl-other", OtherCluster, "scadabridge-dcl", NodeAclScopeKind.Cluster, NodePermissions.WriteOperate), + ]); + + var found = await new ConfigDbApiKeyStore(factory, ThisCluster).FindAsync("abc", Now, default); + + found!.Grants.ShouldHaveSingleItem().NodeAclId.ShouldBe("acl-mine"); + } + + /// + /// A legacy / single-cluster node carries no cluster-{ClusterId} role, so its ClusterId is + /// null and NO filter applies — mirroring the artifact path's ClusterFilterMode.None. + /// Failing closed here would silently strip every grant on an existing deployment. + /// + [Fact] + public async Task A_node_with_no_cluster_scoped_role_sees_every_cluster_scoped_grant() + { + var factory = NewInMemoryDbFactory(); + Seed(factory, + keys: [Key("abc", "p")], + acls: + [ + Acl("acl-a", ThisCluster, "p", NodeAclScopeKind.Cluster, NodePermissions.Read), + Acl("acl-b", OtherCluster, "p", NodeAclScopeKind.Cluster, NodePermissions.WriteOperate), + ]); + + var found = await new ConfigDbApiKeyStore(factory, clusterId: null).FindAsync("abc", Now, default); + + found!.Grants.Select(g => g.NodeAclId).ShouldBe(["acl-a", "acl-b"], ignoreOrder: true); + } + + /// + /// The node's ClusterId comes from its role suffix, and that arrives in either the operator's + /// spelling (Cluster:Roles binds verbatim) or lower-cased (OTOPCUA_ROLES goes through + /// RoleParser.Parse, which calls ToLowerInvariant). The ACL row's ClusterId is + /// authored independently. A case-sensitive match would therefore strip every grant on a node + /// configured the second way — so the match is OrdinalIgnoreCase, the same comparison + /// DeploymentArtifact.ParseDataPlaneAuthority uses. + /// + [Fact] + public async Task A_ClusterId_matches_case_insensitively() + { + var factory = NewInMemoryDbFactory(); + Seed(factory, + keys: [Key("abc", "p")], + acls: [Acl("acl-mine", "SITE-A", "p", NodeAclScopeKind.Cluster, NodePermissions.Read)]); + + var found = await new ConfigDbApiKeyStore(factory, "site-a").FindAsync("abc", Now, default); + + found!.Grants.ShouldHaveSingleItem().NodeAclId.ShouldBe("acl-mine"); + } + + /// + /// FindGrantsAsync is the anonymous principal's path — no credential, so it never goes + /// through FindAsync. It must be confined to the node's cluster too. + /// + [Fact] + public async Task Anonymous_grants_are_cluster_confined_on_their_own_path() + { + var factory = NewInMemoryDbFactory(); + Seed(factory, + keys: [], + acls: + [ + Acl("acl-anon-mine", ThisCluster, OpcUaPrincipals.Anonymous, NodeAclScopeKind.Cluster, NodePermissions.Read), + Acl("acl-anon-other", OtherCluster, OpcUaPrincipals.Anonymous, NodeAclScopeKind.Cluster, NodePermissions.WriteOperate), + ]); + + var grants = await new ConfigDbApiKeyStore(factory, ThisCluster) + .FindGrantsAsync(OpcUaPrincipals.Anonymous, default); + + grants.ShouldHaveSingleItem().NodeAclId.ShouldBe("acl-anon-mine"); + } + + /// + /// Narrower-than-Cluster scopes are still excluded (stage 5 widens this) — the cluster filter is + /// an ADDITIONAL narrowing, not a replacement for the scope-kind one. + /// + [Fact] + public async Task A_narrower_scope_in_this_cluster_is_still_excluded() + { + var factory = NewInMemoryDbFactory(); + Seed(factory, + keys: [Key("abc", "p")], + acls: + [ + Acl("acl-cluster", ThisCluster, "p", NodeAclScopeKind.Cluster, NodePermissions.Read), + Acl("acl-equipment", ThisCluster, "p", NodeAclScopeKind.Equipment, NodePermissions.WriteOperate), + ]); + + var found = await new ConfigDbApiKeyStore(factory, ThisCluster).FindAsync("abc", Now, default); + + found!.Grants.ShouldHaveSingleItem().NodeAclId.ShouldBe("acl-cluster"); + } + + /// + /// The ClusterId derivation itself: the same one ClusterRoleInfo performs for + /// IClusterRoleInfo.ClusterId, off the node's OWN configured roles. + /// + [Fact] + public void ResolveClusterId_reads_the_nodes_own_cluster_scoped_role() + { + ConfigDbApiKeyStore.ResolveClusterId(new AkkaClusterOptions { Roles = ["driver", "cluster-SITE-A"] }) + .ShouldBe("SITE-A"); + ConfigDbApiKeyStore.ResolveClusterId(new AkkaClusterOptions { Roles = ["admin", "driver"] }) + .ShouldBeNull(); + ConfigDbApiKeyStore.ResolveClusterId(new AkkaClusterOptions()) + .ShouldBeNull(); + } + + private static void Seed( + IDbContextFactory factory, + IReadOnlyList keys, + IReadOnlyList acls) + { + using var db = factory.CreateDbContext(); + db.ApiKeys.AddRange(keys); + db.NodeAcls.AddRange(acls); + db.SaveChanges(); + } + + private static ApiKey Key(string id, string principal) => new() + { + ApiKeyRowId = Guid.NewGuid(), + ApiKeyId = id, + Name = id, + SecretHash = "hash", + Principal = principal, + Enabled = true, + CreatedUtc = Now.AddDays(-1), + }; + + private static NodeAcl Acl( + string id, string clusterId, string principal, NodeAclScopeKind scope, NodePermissions flags) => new() + { + NodeAclRowId = Guid.NewGuid(), + NodeAclId = id, + ClusterId = clusterId, + Principal = principal, + ScopeKind = scope, + PermissionFlags = flags, + }; + + private static IDbContextFactory NewInMemoryDbFactory() => + new InMemoryConfigDbFactory(Guid.NewGuid().ToString("N")); + + private sealed class InMemoryConfigDbFactory(string dbName) : IDbContextFactory + { + /// Creates a context over this test's own in-memory database. + /// A new . + public OtOpcUaConfigDbContext CreateDbContext() => + new(new DbContextOptionsBuilder().UseInMemoryDatabase(dbName).Options); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/ZB.MOM.WW.OtOpcUa.Host.Tests.csproj b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/ZB.MOM.WW.OtOpcUa.Host.Tests.csproj index 63747137..7798ef15 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/ZB.MOM.WW.OtOpcUa.Host.Tests.csproj +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.Tests/ZB.MOM.WW.OtOpcUa.Host.Tests.csproj @@ -10,6 +10,9 @@ + + all 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 new file mode 100644 index 00000000..f661c122 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactScopePathTests.cs @@ -0,0 +1,183 @@ +using System.Text; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// End-to-end coverage of the ACL scope path as it actually runs on a node (#520 stage 3): +/// artifact bytes → → scope paths → +/// → . +/// +/// +/// +/// This is the coverage the design doc called out as missing, and it was missing in a specific +/// way worth remembering: the pre-stage-3 tests all hand-built the scopePaths dictionary in +/// the test body with exactly the segments the walker expected, so the walker was pinned against +/// the test's idea of the format rather than against any producer. Nothing proved a real artifact +/// could yield a matching grant, and in fact nothing could have — there was no producer at all. +/// +/// +/// So the artifact JSON here is deliberately hand-written rather than composed: it is standing in +/// for the wire format, and the parser must agree with it. The composer→parser agreement +/// is pinned separately, in ControlPlane.Tests/DataPlaneAuthorityArtifactTests. +/// +/// +public sealed class DeploymentArtifactScopePathTests +{ + private const string NodeId = "node-a:4840"; + + // Plant/Line1 → ModbusDrv → dev1 → Fast → Speed, projected into equipment West/L1/Mixer as "MixerSpeed". + private const string Artifact = """ + { + "RawFolders": [ + {"RawFolderId":"RF-plant","ClusterId":"c1","ParentRawFolderId":null,"Name":"Plant"}, + {"RawFolderId":"RF-line1","ClusterId":"c1","ParentRawFolderId":"RF-plant","Name":"Line1"} + ], + "DriverInstances": [ + {"DriverInstanceId":"DI-modbus","ClusterId":"c1","RawFolderId":"RF-line1","Name":"ModbusDrv"} + ], + "Devices": [{"DeviceId":"DV-1","DriverInstanceId":"DI-modbus","Name":"dev1"}], + "TagGroups": [{"TagGroupId":"TG-fast","DeviceId":"DV-1","ParentTagGroupId":null,"Name":"Fast"}], + "Tags": [{"TagId":"TAG-speed","DeviceId":"DV-1","TagGroupId":"TG-fast","Name":"Speed"}], + "UnsAreas": [{"UnsAreaId":"UA-w","ClusterId":"c1","Name":"West"}], + "UnsLines": [{"UnsLineId":"UL-1","UnsAreaId":"UA-w","Name":"L1"}], + "Equipment": [{"EquipmentId":"EQ-mixer","UnsLineId":"UL-1","Name":"Mixer"}], + "UnsTagReferences": [ + {"UnsTagReferenceId":"REF-1","EquipmentId":"EQ-mixer","TagId":"TAG-speed","DisplayNameOverride":"MixerSpeed"} + ], + "NodeAcls": [ + {"NodeAclId":"acl-dev","ClusterId":"c1","Principal":"p","ScopeKind":9,"ScopeId":"DV-1","PermissionFlags":2}, + {"NodeAclId":"acl-eq","ClusterId":"c1","Principal":"p","ScopeKind":4,"ScopeId":"EQ-mixer","PermissionFlags":16}, + {"NodeAclId":"acl-dangling","ClusterId":"c1","Principal":"p","ScopeKind":4,"ScopeId":"EQ-gone","PermissionFlags":16} + ] + } + """; + + private static (PermissionTrie Trie, List Diagnostics) BuildFromArtifact() + { + var projection = DeploymentArtifact.ParseDataPlaneAuthority(Encoding.UTF8.GetBytes(Artifact), NodeId); + var diagnostics = new List(); + var trie = PermissionTrieBuilder.Build("c1", 1, projection.Grants, projection.ScopePaths, diagnostics.Add); + return (trie, diagnostics); + } + + [Fact] + public void A_raw_device_grant_from_the_artifact_matches_a_tag_beneath_that_device() + { + var (trie, _) = BuildFromArtifact(); + + var matches = trie.CollectMatches( + new NodeScope + { + ClusterId = "c1", + Kind = NodeHierarchyKind.Raw, + Segments = ["Plant", "Line1", "ModbusDrv", "dev1", "Fast", "Speed"], + }, + ["p"]); + + var match = matches.ShouldHaveSingleItem(); + match.PermissionFlags.ShouldBe(NodePermissions.Read); + // Provenance is the authored kind, not the depth the grant happens to sit at. + match.Scope.ShouldBe(NodeAclScopeKind.RawDevice); + } + + [Fact] + public void A_tag_under_a_DIFFERENT_device_does_not_match_that_grant() + { + // The negative control. Without it the test above passes for any trie that returns every grant it + // holds regardless of path — which is exactly what a broken walk would do. + var (trie, _) = BuildFromArtifact(); + + trie.CollectMatches( + new NodeScope + { + ClusterId = "c1", + Kind = NodeHierarchyKind.Raw, + Segments = ["Plant", "Line1", "ModbusDrv", "otherdev", "Fast", "Speed"], + }, + ["p"]).ShouldBeEmpty(); + } + + [Fact] + public void The_same_grant_set_matches_through_the_UNS_realm_too() + { + // The equipment grant is authored once and must be reachable at the UNS NodeId, which is a + // completely different path from the raw one for the same underlying value. + var (trie, _) = BuildFromArtifact(); + + var matches = trie.CollectMatches( + new NodeScope + { + ClusterId = "c1", + Kind = NodeHierarchyKind.Uns, + Segments = ["West", "L1", "Mixer", "MixerSpeed"], + }, + ["p"]); + + matches.ShouldHaveSingleItem().PermissionFlags.ShouldBe(NodePermissions.WriteOperate); + } + + [Fact] + public void A_raw_scope_does_not_pick_up_a_UNS_grant_at_the_same_depth() + { + // Realm isolation. "West/L1/Mixer" and a three-segment raw path are the same depth, so without the + // realm gate at level 1 a UNS grant would match a raw node whose folder/driver/device happened to + // be named the same. The two trees are authored independently, so that collision is not exotic. + var (trie, _) = BuildFromArtifact(); + + trie.CollectMatches( + new NodeScope + { + ClusterId = "c1", + Kind = NodeHierarchyKind.Raw, + Segments = ["West", "L1", "Mixer", "MixerSpeed"], + }, + ["p"]).ShouldBeEmpty(); + } + + [Fact] + public void A_dangling_scope_is_dropped_with_a_diagnostic_rather_than_planted() + { + var (_, diagnostics) = BuildFromArtifact(); + + var diagnostic = diagnostics.ShouldHaveSingleItem(); + diagnostic.NodeAclId.ShouldBe("acl-dangling"); + diagnostic.Reason.ShouldBe(PermissionTrieBuildDiagnosticReason.UnresolvedScope); + } + + [Fact] + public void Case_drift_in_a_path_segment_grants_nothing() + { + // Raw sibling names are unique under an ordinal comparer, so "Speed" and "speed" can both exist + // under one device. The trie compared OrdinalIgnoreCase before stage 3, which would have let a + // grant on either match both. This goes red if the comparer is reverted. + var (trie, _) = BuildFromArtifact(); + + trie.CollectMatches( + new NodeScope + { + ClusterId = "c1", + Kind = NodeHierarchyKind.Raw, + Segments = ["plant", "Line1", "ModbusDrv", "dev1", "Fast", "Speed"], + }, + ["p"]).ShouldBeEmpty(); + } + + [Fact] + public void An_artifact_with_no_topology_resolves_no_scopes() + { + var projection = DeploymentArtifact.ParseDataPlaneAuthority( + Encoding.UTF8.GetBytes(""" + {"NodeAcls":[{"NodeAclId":"a","ClusterId":"c1","Principal":"p","ScopeKind":4,"ScopeId":"EQ-x","PermissionFlags":1}]} + """), + NodeId); + + projection.Grants.ShouldHaveSingleItem(); + projection.ScopePaths.ShouldBeEmpty(); + } +}