diff --git a/docs/Configuration.md b/docs/Configuration.md index f9208c28..eff4ab94 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -104,7 +104,11 @@ The checked-in `appsettings*.json` files are deliberately thin: they carry only | `Port` | int | `4053` | Cluster transport port. **Duplicated as `ClusterNode.AkkaPort` in the Config DB** — see the note below. | | `PublicHostname` | string | `127.0.0.1` | Hostname advertised in cluster gossip; must be reachable by peers. **Duplicated as `ClusterNode.Host`** — see the note below. | | `SeedNodes` | string[] | `[]` | Seed nodes for bootstrapping. **Ordered:** a node that appears in its own list must be entry 0 (only `seed-nodes[0]` can form a new cluster), or the host refuses to start — see [Redundancy.md § Bootstrap: self-first seed ordering](Redundancy.md#bootstrap-self-first-seed-ordering). **Per-pair since Phase 6:** each application `Cluster`'s pair is its own Akka mesh, so a node's `SeedNodes` lists only itself (first) and its own pair partner (second) — never a node from another cluster's pair. | -| `Roles` | string[] | `[]` | Cluster roles for this node. When empty, falls back to `OTOPCUA_ROLES`. Allowed values: `admin`, `driver`, `dev`, plus a per-cluster `cluster-{ClusterId}` role (e.g. `cluster-MAIN`, `cluster-SITE-A`) — see [Per-cluster mesh split (Phase 6)](#per-cluster-mesh-split-phase-6-cluster-clusterid-roles--splittopologytransportvalidator) below. | +| `Roles` | string[] | `[]` | Cluster roles for this node. When empty, falls back to `OTOPCUA_ROLES` (see the note below). Allowed values: `admin`, `driver`, `dev`, plus a per-cluster `cluster-{ClusterId}` role (e.g. `cluster-MAIN`, `cluster-SITE-A`) — see [Per-cluster mesh split (Phase 6)](#per-cluster-mesh-split-phase-6-cluster-clusterid-roles--splittopologytransportvalidator) below. | + +> **`Cluster:Roles` and `OTOPCUA_ROLES` are two knobs for one thing, and only one of them is the Akka role list.** `OTOPCUA_ROLES` drives all of `Program.cs`'s conditional wiring (which subsystems mount); `Cluster:Roles` becomes the node's **gossiped Akka member roles**. Since Gitea #532 `AddOtOpcUaCluster` applies the documented fallback — an empty `Cluster:Roles` takes the env var — so setting only `OTOPCUA_ROLES` now works. Before that fix it did not, silently: the node joined the mesh carrying **no roles at all**, which reads as healthy locally and as *"no driver-role nodes are Up"* at central, and every deployment to it timed out ([`deployments/wonder-app-vd03-makino-z-34184.md`](deployments/wonder-app-vd03-makino-z-34184.md)). +> +> Setting `Cluster:Roles` explicitly is still the recommendation, because the two sources normalize differently: `Cluster:Roles` binds **verbatim** while the env path is **lower-cased** by `RoleParser.Parse`. Akka compares role names ordinally, so a redundant pair configured through *different* sources with a mixed-case `cluster-{ClusterId}` (e.g. `cluster-SITE-A` vs `cluster-site-a`) would gossip two different role names and elect two singletons. Configure both nodes of a pair the same way. > The full redundancy model (ServiceLevel tiers, split-brain, peer discovery) is in [`Redundancy.md`](Redundancy.md). The OPC UA peer-URI advertising lives in the `OpcUa:PeerApplicationUris` key above. @@ -329,7 +333,7 @@ All names are read in this repo's source via `Environment.GetEnvironmentVariable | Variable | Read by | Effect / default | |---|---|---| -| `OTOPCUA_ROLES` | `src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs` (`RoleParser.Parse`) | Comma-separated cluster roles for the node (`admin`, `driver`, `dev`). Drives the conditional wiring and the per-role appsettings overlay. Used when `Cluster:Roles` is empty. | +| `OTOPCUA_ROLES` | `src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs` (`RoleParser.Parse`) and `AddOtOpcUaCluster`'s `AkkaClusterOptions` post-configure (`RoleParser.ResolveEffectiveRoles`) | Comma-separated cluster roles for the node (`admin`, `driver`, `dev`, `cluster-{ClusterId}`). Drives the conditional wiring and the per-role appsettings overlay, and — when `Cluster:Roles` is empty — the node's Akka member roles too. See the note under `Cluster:Roles` for why the two sources are not interchangeable. | | `OTOPCUA_CONFIG_CONNECTION` | `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/DesignTimeDbContextFactory.cs` (design-time / `dotnet ef` only) | Read at **design time** by `DesignTimeDbContextFactory.cs` for `dotnet ef` migrations. At **runtime** the server resolves the connection string from `ConnectionStrings:ConfigDb` (env form: `ConnectionStrings__ConfigDb`) via `configuration.GetConnectionString("ConfigDb")` in `ServiceCollectionExtensions.cs` — `OTOPCUA_CONFIG_CONNECTION` appears there only as a hint in an error message, not via `GetEnvironmentVariable`. No credential is embedded in source. | | `ASPNETCORE_ENVIRONMENT` | ASP.NET host builder (framework) | Selects `appsettings.{Environment}.json` (e.g. `Development`). | diff --git a/docs/deployments/wonder-app-vd03-makino-z-34184.md b/docs/deployments/wonder-app-vd03-makino-z-34184.md index 51cf687b..00580cf8 100644 --- a/docs/deployments/wonder-app-vd03-makino-z-34184.md +++ b/docs/deployments/wonder-app-vd03-makino-z-34184.md @@ -56,6 +56,19 @@ now joins **UP with roles ADMIN+DRIVER** and deployments seal. Prior env backed > Product follow-up: either wire `OTOPCUA_ROLES` → `AkkaClusterOptions.Roles`, or bake `Cluster:Roles` > into the deploy template, so a node redeploy doesn't regress to role-less. +> **RESOLVED 2026-08-01 (Gitea #532).** The first option shipped: `AddOtOpcUaCluster` post-configures +> `AkkaClusterOptions`, so an empty `Cluster:Roles` now falls back to `OTOPCUA_ROLES` through +> `RoleParser.ResolveEffectiveRoles`. A node in this box's original configuration would join **UP with +> roles ADMIN+DRIVER** without the registry edit above, so a redeploy that drops the hand-added +> `Cluster__Roles__0/1` no longer regresses to role-less. The `Cluster__Roles` entries are still worth +> keeping — the two sources normalize case differently (see `docs/Configuration.md` → `Cluster:Roles`), +> and an explicit list is the one that survives someone changing the env var for a different reason. +> +> Worth recording for the next incident of this shape: this was **documented behaviour that did not +> exist**. `AkkaClusterOptions.Roles`' XML doc and `SplitTopologyTransportValidator` both described the +> fallback, so anyone reading the code to diagnose this box would have concluded the roles were already +> being applied and looked elsewhere. + ## Live verification (OtOpcUa CLI client → server) `connect` + `browse` confirm the equipment + both tags are served at the node IDs above. 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/docs/security.md b/docs/security.md index c9b26f35..8f308e01 100644 --- a/docs/security.md +++ b/docs/security.md @@ -337,16 +337,21 @@ until the tracking issue below is closed. **The four things blocking a wire-up** (all verified, none of them merely "call the evaluator"): -1. **Raw-realm nodes have no representable scope.** `NodeHierarchyKind` has exactly one member, - `Equipment`. Raw NodeIds are `Folder→Driver→Device→TagGroup→Tag` RawPaths, which `NodeScope` cannot - express — yet the write gate is attached to raw tags. Roughly half the writable address space has - no scope to evaluate. This is a design decision, not wiring. +1. ~~**Raw-realm nodes have no representable scope.**~~ **Closed by #520 stage 3.** `NodeScope` is now + `(ClusterId, Kind, Segments)` with a variable-length path, and `NodeHierarchyKind` carries both + `Uns` and `Raw`. The previous fixed-arity shape could not address a real raw tree at all — folder + chains and tag-group chains both nest without bound — so it silently capped at five levels and + mislabelled every one of them. Four `Raw*` scope kinds were appended for authoring. 2. ~~**The identity carries roles, not groups.**~~ **Dissolved by #520 stage 2.** The column is now `NodeAcl.Principal` and the principal comes from the presented API key (or the reserved `anonymous`), not from a directory — so there is no group list to lose. -3. **`PermissionTrieBuilder`'s `scopePaths` argument has no producer.** Without it every sub-cluster - grant lands in the builder's fallback bucket — the hazard its own comments warn about. It would have - to be derived from the artifact's UNS relations. +3. ~~**`PermissionTrieBuilder`'s `scopePaths` argument has no producer.**~~ **Closed by #520 stage 3.** + `AclScopePathResolver` is that producer, built on both sides of the split — EF entities at the deploy + gate, artifact JSON on the node — and `scopePaths` is now a required argument with no fallback: an + unresolvable scope is **dropped with a diagnostic** rather than planted at a trie position the walk + can never reach. A scope resolves to *several* paths when one entity is reachable at several NodeIds + (a raw tag projected into N equipments has one raw path plus N UNS paths), because a grant that + covered only some of them would be bypassable through the one it missed. 4. **No session lifecycle exists.** `UserAuthorizationState` has the freshness/staleness predicates but nothing in production constructs one per session. **Half-closed by stage 4:** the *generation* now exists and advances — `DataPlaneAuthorityState` stamps a monotonic `GenerationId` each time an @@ -354,9 +359,23 @@ until the tracking issue below is closed. the stamping is stage 5's job, because until an enforcement seam reads it, a per-session state object would be constructed and never consulted. -Two smaller inconsistencies inside the subsystem itself: `NodeAclScopeKind.FolderSegment` is offered in -the authoring dropdown but the trie walker has no level to match it at, and `NodePermissions.AlarmRead` -has no `OpcUaOperation` mapping, so it is grantable but unreachable. +`NodeAclScopeKind.FolderSegment` is the retired v2 kind with no v3 entity behind it. It is still offered +in the authoring dropdown (labelled retired), but it can never resolve, and both ends now say so +specifically rather than reporting it as a missing id: the deploy gate rejects it with +`NodeAclScopeKindRetired`, and the trie builder drops it with a `RetiredScopeKind` diagnostic (Gitea +#533 — it used to arrive as `UnresolvedScope`, which sent an operator holding a legacy row hunting for a +typo in an id that was never the problem). One inconsistency remains: `NodePermissions.AlarmRead` has no +`OpcUaOperation` mapping, so it is grantable but unreachable. + +**String identity in this subsystem** (Gitea #533): **identifiers** — `ScopeId`, `ClusterId`, +`Principal` — compare `OrdinalIgnoreCase`, because they are opaque handles whose spelling genuinely +varies by source (a `ClusterId` arrives lower-cased through `OTOPCUA_ROLES` and verbatim through +`Cluster:Roles`; a `ScopeId` is operator free-text until the stage-7 picker lands). **Path segments** — +every level of a scope path, and therefore `PermissionTrieNode.Children` — compare `Ordinal`, because +they are browse names and `Speed` / `speed` are two different nodes that can coexist under one device. +The rule is stated in full on `AclScopeRef`. It matters most where the deploy gate and the node are fed +from *different* snapshots, which is exactly the split `AclScopePathResolver` exists to bridge: case +drift there would silently drop every grant on the affected scope. ### Hierarchy (design) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/AkkaClusterOptions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/AkkaClusterOptions.cs index 99aad37c..d1dc6cb5 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/AkkaClusterOptions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/AkkaClusterOptions.cs @@ -48,10 +48,26 @@ public sealed class AkkaClusterOptions public string[] SeedNodes { get; set; } = Array.Empty(); /// - /// Cluster roles for this node. When empty the role list comes from - /// OTOPCUA_ROLES via . Allowed values: - /// admin, driver, dev. + /// Cluster roles for this node. Allowed values: admin, driver, dev, plus one + /// per-cluster cluster-{ClusterId} role (per-cluster mesh, Phase 6). /// + /// + /// + /// When this is empty the list falls back to the OTOPCUA_ROLES environment variable, + /// applied by AddOtOpcUaCluster through + /// . That fallback was + /// described here — and relied upon in the field — for a year before it was implemented; a + /// node configured only through the env var joined the cluster carrying no roles at all. + /// See Gitea #532 and docs/deployments/wonder-app-vd03-makino-z-34184.md. + /// + /// + /// The two sources normalize differently: values bound from Cluster:Roles are taken + /// verbatim, while the environment path is lower-cased by . + /// Akka compares gossiped role names ordinally, so configure both nodes of a redundant + /// pair through the same source — a pair split across the two with a mixed-case + /// cluster-{ClusterId} would gossip two different role names and elect two singletons. + /// + /// public string[] Roles { get; set; } = Array.Empty(); /// diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ClusterRoleInfo.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ClusterRoleInfo.cs index d1f660b6..49949497 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ClusterRoleInfo.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ClusterRoleInfo.cs @@ -39,22 +39,26 @@ public sealed class ClusterRoleInfo : IClusterRoleInfo, IDisposable // sharing a bind IP) stay distinct. Production hosts have unique DNS names so the port // suffix is harmless redundancy. _localNode = CommonsNodeId.Parse($"{options.Value.PublicHostname}:{options.Value.Port}"); - _localRoles = new HashSet(options.Value.Roles, StringComparer.Ordinal); + // OrdinalIgnoreCase (Gitea #532): Cluster:Roles binds verbatim, so a node authored with 'Driver' + // used to answer HasRole("driver") false while gossiping a role Akka would still match against + // its own ordinal comparison — the node was a driver to the cluster and not to itself. + _localRoles = new HashSet(options.Value.Roles, StringComparer.OrdinalIgnoreCase); // Derived from the node's OWN configured roles (not live Cluster.State) so this identity is // available at host-build time, before the cluster forms. Iterate the ORIGINAL array — not // _localRoles — because first-wins requires configuration order, which HashSet does not - // guarantee. - var clusterRoles = options.Value.Roles.Where(RoleParser.IsClusterRole).ToArray(); - if (clusterRoles.Length > 0) + // guarantee. The OTOPCUA_ROLES fallback is already applied to the bound options by + // AddOtOpcUaCluster's PostConfigure, so nothing is re-resolved here. + var clusterRoles = RoleParser.ClusterRolesIn(options.Value.Roles); + if (clusterRoles.Count > 0) { _clusterRole = clusterRoles[0]; _clusterId = RoleParser.ClusterIdFromRole(_clusterRole); - if (clusterRoles.Length > 1) + if (clusterRoles.Count > 1) { _logger.LogWarning( "Node configured with {Count} cluster-scoped roles ({Roles}); using the first ({ClusterRole})", - clusterRoles.Length, string.Join(", ", clusterRoles), _clusterRole); + clusterRoles.Count, string.Join(", ", clusterRoles), _clusterRole); } } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ConfigSourceOptionsValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ConfigSourceOptionsValidator.cs index 520044ef..707a2a4e 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ConfigSourceOptionsValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ConfigSourceOptionsValidator.cs @@ -93,11 +93,15 @@ public sealed class ConfigSourceOptionsValidator : IValidateOptions() ?? Array.Empty(); - var isDriver = Array.IndexOf(roles, "driver") >= 0; - var isAdmin = Array.IndexOf(roles, "admin") >= 0; + // connection). Roles come from configuration, not IClusterRoleInfo — see the ctor remark — and + // through RoleParser.ResolveEffectiveRoles so the OTOPCUA_ROLES fallback applies (Gitea #532). + // Reading Cluster:Roles alone exempted the exact node this rule exists for: an env-only + // driver-only node has no ConfigDb (Program.cs gates it on the same env var) yet saw an empty + // role list here and sailed through on Direct, to read its configuration from nothing. + var roles = RoleParser.ResolveEffectiveRoles( + _configuration.GetSection("Cluster:Roles").Get()); + var isDriver = Array.IndexOf(roles, RoleParser.Driver) >= 0; + var isAdmin = Array.IndexOf(roles, RoleParser.Admin) >= 0; if (isDriver && !isAdmin && isDirect) { diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/RoleParser.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/RoleParser.cs index 1421543a..789a0084 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/RoleParser.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/RoleParser.cs @@ -23,15 +23,110 @@ public static class RoleParser /// Returns true when is a cluster-scoped role — starts with /// and carries a non-empty ClusterId suffix. + /// + /// The prefix test is case-insensitive (Gitea #532). It was ordinal, which meant a + /// Cluster:Roles entry spelled Cluster-SITE-A — that path binds verbatim, unlike + /// the path, which lower-cases — classified as an ordinary role. The node + /// then resolved a ClusterId while still gossiping a cluster-scoped role, + /// so it looked split-topology to its peers and un-scoped to itself. + /// pre-normalized for exactly this reason; + /// ClusterRoleInfo did not, and got the null. + /// /// The role name to test. public static bool IsClusterRole(string role) => - role.StartsWith(ClusterRolePrefix, StringComparison.Ordinal) && role.Length > ClusterRolePrefix.Length; + role is not null + && role.StartsWith(ClusterRolePrefix, StringComparison.OrdinalIgnoreCase) + && role.Length > ClusterRolePrefix.Length; /// Extracts the ClusterId from a cluster-scoped role, e.g. cluster-SITE-A → SITE-A. /// Assumes is true for . + /// + /// The suffix keeps the operator's own spelling. It is NOT case-normalized here because the two + /// role sources disagree by construction — Cluster:Roles binds verbatim + /// (cluster-SITE-A ⇒ SITE-A) while lower-cases the + /// OTOPCUA_ROLES path (⇒ site-a) — and normalizing one side would silently rename + /// the Akka role of a running deployment (docker-dev's rig configures cluster-MAIN / + /// cluster-SITE-A verbatim today). Every consumer of a ClusterId therefore compares it + /// case-insensitively; see the identity rule on AclScopeRef (Gitea #533). + /// /// The cluster-scoped role name. public static string ClusterIdFromRole(string role) => role[ClusterRolePrefix.Length..]; + /// + /// The cluster-scoped roles in , in the order given. More than one is a + /// misconfiguration the caller is expected to report — a node belongs to exactly one mesh. + /// + /// The node's roles. + /// The cluster-scoped roles, original spelling preserved. + public static IReadOnlyList ClusterRolesIn(IEnumerable? roles) => + roles is null ? Array.Empty() : roles.Where(IsClusterRole).ToArray(); + + /// The first cluster-scoped role in , or null when there is none. + /// The node's roles. + /// The cluster role, e.g. cluster-SITE-A. + public static string? ClusterRoleFromRoles(IEnumerable? roles) => + roles?.FirstOrDefault(IsClusterRole); + + /// + /// The ClusterId this node belongs to, derived from its own roles; null when it carries no + /// cluster-scoped role (a legacy / single-mesh node). + /// + /// + /// The single home for a derivation that was open-coded at four call sites before Gitea #532 + /// (ClusterRoleInfo, the ControlPlane singleton scope, SplitTopologyTransportValidator + /// and ConfigDbApiKeyStore) — each with its own view of whether the classification was + /// case-sensitive, which is how the mixed-case gap above survived in three of the four. + /// + /// The node's roles. + /// The ClusterId, or . + public static string? ClusterIdFromRoles(IEnumerable? roles) + { + var role = ClusterRoleFromRoles(roles); + return role is null ? null : ClusterIdFromRole(role); + } + + /// + /// Resolves a node's effective Akka roles: Cluster:Roles when it is non-empty, + /// otherwise the OTOPCUA_ROLES environment variable through . + /// + /// + /// + /// ⚠️ This fallback was documented for a year before it existed (Gitea #532). + /// AkkaClusterOptions.Roles' own XML doc and + /// both described it; only the validator implemented it, privately. A node configured solely + /// through OTOPCUA_ROLES — which drives every bit of Program.cs's conditional + /// wiring, so it reads as the authoritative knob — joined the cluster carrying no Akka + /// roles at all. It ran, served, and logged nothing wrong, while central reported "no + /// driver-role nodes are Up" and every deployment to it timed out. That is not hypothetical: + /// see docs/deployments/wonder-app-vd03-makino-z-34184.md, where the field fix was to + /// hand-add Cluster__Roles__0/1 to a Windows service's registry environment. + /// + /// + /// Configured wins outright — a stray OTOPCUA_ROLES can never widen an explicit + /// Cluster:Roles. The two sources normalize differently on purpose (see + /// ), so a redundant pair should be configured through the + /// same one. + /// + /// + /// The bound Cluster:Roles value. + /// The raw OTOPCUA_ROLES value. + /// The effective roles; empty when neither source names any. + /// The environment value names an unknown role. + public static string[] ResolveEffectiveRoles(IReadOnlyList? configuredRoles, string? envRoles) + { + if (configuredRoles is { Count: > 0 }) return [.. configuredRoles]; + return Parse(envRoles); + } + + /// Reads the effective roles for the current process, resolving the environment itself. + /// The bound Cluster:Roles value. + /// The effective roles. + public static string[] ResolveEffectiveRoles(IReadOnlyList? configuredRoles) => + ResolveEffectiveRoles(configuredRoles, Environment.GetEnvironmentVariable(RolesEnvVar)); + + /// The environment variable the role list falls back to when Cluster:Roles is empty. + public const string RolesEnvVar = "OTOPCUA_ROLES"; + /// Parses a comma-separated string of role names into a validated array. /// The raw role string to parse. /// The distinct, lower-cased, validated role names. diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ServiceCollectionExtensions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ServiceCollectionExtensions.cs index 1dad8d14..a8d09b08 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ServiceCollectionExtensions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ServiceCollectionExtensions.cs @@ -34,6 +34,22 @@ public static class ServiceCollectionExtensions services.AddValidatedOptions( configuration, AkkaClusterOptions.SectionName); + // Gitea #532: apply the OTOPCUA_ROLES fallback the Roles property has always DOCUMENTED, in the + // one place every consumer reads through. Before this, a node configured only through the env + // var — the knob that drives all of Program.cs's role wiring — bound an EMPTY Roles array and so + // joined the cluster role-less: invisible to RoleLeader/MembersWithRole, ineligible for every + // role-scoped singleton, and un-deployable ("no driver-role nodes are Up") with nothing in its + // own logs to say so. Doing it here rather than at each call site means BuildClusterOptions + // (Akka's own member roles), ClusterRoleInfo, the ControlPlane singleton scope and + // ConfigDbApiKeyStore's ClusterId all see the same answer. + // + // PostConfigure, not a bind trick: it runs after the section binds and before ValidateOnStart, + // so the validators below see the effective list. An unknown role in the env var throws out of + // RoleParser.Parse — the same throw Program.cs already takes on the same value, so an invalid + // env var cannot reach here on the production path in the first place. + services.PostConfigure( + options => options.Roles = RoleParser.ResolveEffectiveRoles(options.Roles)); + // Per-cluster mesh Phase 2: which transport carries central→node commands. Validated at // startup for the same reason the cluster options are — a contact point that cannot resolve // produces silence, not an error, and silence is indistinguishable from "nothing to do". diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/SplitTopologyTransportValidator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/SplitTopologyTransportValidator.cs index 8d400ef8..e9a80a95 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/SplitTopologyTransportValidator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/SplitTopologyTransportValidator.cs @@ -61,17 +61,17 @@ public sealed class SplitTopologyTransportValidator : IValidateOptions() ?? Array.Empty(); - var effective = configured.Length > 0 - ? configured - : RoleParser.Parse(Environment.GetEnvironmentVariable("OTOPCUA_ROLES")); + var effective = RoleParser.ResolveEffectiveRoles(configured); var normalized = Array.ConvertAll(effective, r => r.Trim().ToLowerInvariant()); diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/TelemetryOptions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/TelemetryOptions.cs index b9fc6d9a..d155d1cb 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/TelemetryOptions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/TelemetryOptions.cs @@ -118,8 +118,12 @@ public sealed class TelemetryOptionsValidator : IValidateOptions() ?? Array.Empty(); - var isDriver = Array.IndexOf(roles, "driver") >= 0; + // Through ResolveEffectiveRoles so the OTOPCUA_ROLES fallback applies (Gitea #532) — a node + // configured only through the env var is still a driver node and must satisfy the Grpc-mode + // requirements below. + var roles = RoleParser.ResolveEffectiveRoles( + _configuration.GetSection("Cluster:Roles").Get()); + var isDriver = Array.IndexOf(roles, RoleParser.Driver) >= 0; if (isGrpc) { 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..20e6b671 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,241 @@ 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 case-insensitive, 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. Both this rule and the resolver were + /// ordinal until Gitea #533; the cross-cluster comparison below was ordinal too, which was the + /// worse half — it could reject a deploy as cross-cluster over a ClusterId the runtime + /// would have matched, and the two spellings genuinely occur (see + /// RoleParser.ClusterIdFromRole). The rule now is: identifiers compare + /// OrdinalIgnoreCase, path segments compare Ordinal; it is stated in full on + /// AclScopeRef. + /// + /// + /// 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.OrdinalIgnoreCase) + .ToDictionary(g => g.Key, g => value(g.First()), StringComparer.OrdinalIgnoreCase); + + // 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.OrdinalIgnoreCase)) + 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..0c8914fc --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/AclScopePathResolver.cs @@ -0,0 +1,351 @@ +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 string-identity rule for the whole authorization path (Gitea #533), stated here because +/// this type is where it was first violated: +/// +/// +/// +/// Identifiers compare OrdinalIgnoreCase — ScopeId, ClusterId, +/// Principal. They are opaque handles, not addresses; two spellings of one id always +/// mean the same entity, and the sources genuinely differ in case (a ClusterId arrives +/// lower-cased through OTOPCUA_ROLES and verbatim through Cluster:Roles — see +/// RoleParser.ClusterIdFromRole — and a ScopeId is operator free-text in the ACL +/// editor until the stage-7 picker lands). +/// +/// +/// Path segments compare Ordinal — every level of , +/// and therefore PermissionTrieNode.Children. They are OPC UA browse names, where +/// Speed and speed are two different nodes that can coexist under one device +/// (RawPaths.Comparer is ordinal, so raw sibling-name uniqueness is too). +/// +/// +/// +/// Before #533 this record struct used the compiler's default equality, i.e. ordinal, while the +/// trie builder filtered ClusterId and hashed principals OrdinalIgnoreCase — three +/// rules in one path. It worked only because the resolver and the builder were handed the same +/// NodeAcl instances, so the ScopeId strings were literally the same object. Feed +/// the two sides from different snapshots — which is the entire reason this resolver is +/// input-shape agnostic, EF on the gate and artifact JSON on the node — and any case drift drops +/// every grant on that scope. Fail-closed and diagnosed, but a silent loss of authority. +/// +/// +/// The authored scope kind. +/// The authored scope id. +public readonly record struct AclScopeRef(NodeAclScopeKind Kind, string ScopeId) +{ + /// Equality over the kind plus a case-insensitive id — see the rule on the type. + /// The other reference. + /// when both name the same scope. + public bool Equals(AclScopeRef other) => + Kind == other.Kind && string.Equals(ScopeId, other.ScopeId, StringComparison.OrdinalIgnoreCase); + + /// + public override int GetHashCode() => + HashCode.Combine(Kind, StringComparer.OrdinalIgnoreCase.GetHashCode(ScopeId ?? string.Empty)); +} + +/// +/// 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); + + // Every map is re-keyed OrdinalIgnoreCase rather than used as handed in. Ids are identifiers, so + // they follow the identifier half of the rule on AclScopeRef (Gitea #533) — and doing it HERE, + // rather than documenting a comparer contract for callers, is what makes the rule impossible to + // get wrong from the outside: the two producers (EF entities at the deploy gate, artifact JSON on + // the node) build these dictionaries independently and neither can now diverge from the other. + _folders = ById(folders); + _drivers = ById(drivers); + _devices = ById(devices); + _groups = ById(groups); + _tags = ById(tags); + _areas = ById(areas); + _lines = ById(lines); + _equipment = ById(equipment); + _references = references; + + // The projected group map goes in with the same comparer, so a raw ACL path and the RawPath that + // forms the node's NodeId still agree segment-for-segment — RawPathResolver emits each entity's + // OWN name, so a case-insensitive id lookup widens which chains resolve and never what they spell. + _rawPaths = new RawPathResolver( + _folders, + _drivers, + _devices, + ById(groups.ToDictionary(kv => kv.Key, kv => (kv.Value.ParentId, kv.Value.Name)))); + } + + /// + /// 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. The trie builder now + // short-circuits FolderSegment too, so it reports RetiredScopeKind rather than the + // indistinguishable-from-a-typo UnresolvedScope (Gitea #533); this arm stays as the answer + // for a direct ResolveOne caller. + 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.OrdinalIgnoreCase)) 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]; + } + + /// Re-keys an id map under the identifier comparer — see the rule on . + private static Dictionary ById(IReadOnlyDictionary source) + { + var copy = new Dictionary(source.Count, StringComparer.OrdinalIgnoreCase); + // Last-wins on a case-collision, matching the GroupBy/First tolerance the deploy gate applies to + // duplicate ids: a duplicate is another rule's business, and throwing here would fault the whole + // decode over one malformed row. + foreach (var (key, value) in source) copy[key] = value; + return copy; + } + + // 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..77b6726e 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,128 @@ 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) + // Ahead of the ScopeId checks, mirroring the deploy gate's ordering (DraftValidator's + // NodeAclScopeKindRetired): the retirement is the actionable fact, and a row carrying this + // kind can never resolve whatever its ScopeId, so reporting it as "unresolved" would send an + // operator hunting for a typo in an id that was never the problem. + if (row.ScopeKind == NodeAclScopeKind.FolderSegment) + { + diagnostic?.Invoke(new PermissionTrieBuildDiagnostic( + row.NodeAclId, row.ScopeKind, row.ScopeId, + PermissionTrieBuildDiagnosticReason.RetiredScopeKind)); + continue; + } + + // 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 +138,51 @@ 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, or an entity someone deleted after the grant was authored. The remedy is to correct the id + /// or delete the grant. /// - MissingScopePath, + UnresolvedScope, + + /// The row is narrower than but carries no ScopeId. + MissingScopeId, + + /// + /// The row's is retired and has no v3 entity behind it + /// (), so it can never resolve whatever its + /// ScopeId. Split out of by Gitea #533: the two have + /// different remedies — re-scope the grant to versus + /// fix an id — and conflating them told an operator with a legacy row to go looking for a typo. + /// + RetiredScopeKind, + + /// + /// 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/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs index f05b5164..87a630be 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/IAbCipTagRuntime.cs @@ -122,4 +122,19 @@ public sealed record AbCipTagCreateParams( bool AllowPacking = true, int ConnectionSize = 4002, int ElementCount = 1, - bool IsArray = false); + bool IsArray = false) +{ + /// + /// The value for libplctag's gateway attribute: host:port, or bare host when + /// no usable port was configured (libplctag then applies its own 44818 default). + /// + /// + /// ⚠️ used to be carried here and never consumed (Gitea #534, found in + /// the AbLegacy driver, which had the identical defect). The runtime passed the bare host to + /// libplctag, which silently substitutes 44818 — so an authored port did not merely fail to apply, + /// it redirected the connection to whatever answers on the default port, which on a plant network + /// is another PLC. This driver's own tests never caught it because its fixture sits on 44818, where + /// the bug is invisible by construction. + /// + public string LibplctagGateway => Port > 0 ? $"{Gateway}:{Port}" : Gateway; +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs index 9584fa06..11ef9b26 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/LibplctagTagRuntime.cs @@ -19,7 +19,9 @@ internal sealed class LibplctagTagRuntime : IAbCipTagRuntime { _tag = new Tag { - Gateway = p.Gateway, + // host:port — see AbLegacyTagCreateParams/AbCipTagCreateParams.LibplctagGateway. + // Passing p.Gateway alone drops the authored port onto libplctag's 44818 default (#534). + Gateway = p.LibplctagGateway, Path = p.CipPath, PlcType = MapPlcType(p.LibplctagPlcAttribute), Protocol = Protocol.ab_eip, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/IAbLegacyTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/IAbLegacyTagRuntime.cs index 6bc3dfa7..9f4872d6 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/IAbLegacyTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/IAbLegacyTagRuntime.cs @@ -79,4 +79,19 @@ public sealed record AbLegacyTagCreateParams( string LibplctagPlcAttribute, string TagName, TimeSpan Timeout, - int ElementCount = 1); + int ElementCount = 1) +{ + /// + /// The value for libplctag's gateway attribute: host:port, or bare host when + /// no usable port was configured (libplctag then applies its own 44818 default). + /// + /// + /// ⚠️ used to be carried here and never consumed (Gitea #534). The runtime + /// passed the bare host to libplctag, which silently substitutes 44818 — so an authored port was + /// not merely ignored, it redirected the connection to whatever answers on the default port. On the + /// test rig that was a different simulator of a different PLC family, which is how the AbLegacy + /// integration suite failed for years while looking like a fixture problem. On a plant network it + /// would be another PLC. + /// + public string LibplctagGateway => Port > 0 ? $"{Gateway}:{Port}" : Gateway; +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs index 72ce1c57..fb71ce32 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/LibplctagLegacyTagRuntime.cs @@ -18,7 +18,9 @@ internal sealed class LibplctagLegacyTagRuntime : IAbLegacyTagRuntime { _tag = new Tag { - Gateway = p.Gateway, + // host:port — see AbLegacyTagCreateParams/AbCipTagCreateParams.LibplctagGateway. + // Passing p.Gateway alone drops the authored port onto libplctag's 44818 default (#534). + Gateway = p.LibplctagGateway, Path = p.CipPath, PlcType = MapPlcType(p.LibplctagPlcAttribute), Protocol = Protocol.ab_eip, // PCCC-over-EIP; libplctag routes via the PlcType-specific PCCC layer 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.ControlPlane/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs index 81a2f973..08cf21e9 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.ControlPlane/ServiceCollectionExtensions.cs @@ -231,7 +231,7 @@ public static class ServiceCollectionExtensions // on the fused central node). AkkaClusterOptions is pure configuration, available before the // cluster forms — the same source ClusterRoleInfo.ClusterRole itself derives from — so the // scope is identical. First cluster role wins, in configuration order (matching ClusterRoleInfo). - var clusterRole = clusterOptions.Roles.FirstOrDefault(RoleParser.IsClusterRole); + var clusterRole = RoleParser.ClusterRoleFromRoles(clusterOptions.Roles); return new ClusterSingletonOptions { Role = clusterRole ?? RoleParser.Driver }; } } 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..519f8525 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,60 @@ 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 — literally so as of Gitea #532, which collapsed four + /// open-coded copies into . The copies had drifted: + /// three classified the cluster role case-sensitively and one did not, and only one applied + /// the OTOPCUA_ROLES fallback. On a node that hit either gap this method returned + /// , and a null ClusterId applies no filter at all — so the + /// cross-cluster over-grant this store exists to prevent failed open. + /// + /// + /// ⚠️ 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); + + return RoleParser.ClusterIdFromRoles(options.Roles); + } /// public async Task FindAsync(string apiKeyId, DateTime utcNow, CancellationToken ct) @@ -53,15 +114,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.Cluster.Tests/AkkaClusterOptionsRoleFallbackTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/AkkaClusterOptionsRoleFallbackTests.cs new file mode 100644 index 00000000..90ecdb3c --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/AkkaClusterOptionsRoleFallbackTests.cs @@ -0,0 +1,97 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests; + +/// +/// Gitea #532 — AkkaClusterOptions.Roles must actually apply the OTOPCUA_ROLES fallback +/// its own XML doc has always described. +/// +/// +/// +/// These go through AddOtOpcUaCluster + a real service provider rather than calling +/// RoleParser.ResolveEffectiveRoles directly, because the defect was never in the parsing — +/// it was that nothing called it on the way to the bound options. A unit test of the +/// helper alone would have passed against the broken tree. +/// +/// +/// The consequence being pinned: BuildClusterOptions feeds Roles straight to Akka as +/// the node's gossiped member roles. Empty there means a node that joins, comes Up, serves OPC UA +/// — and is invisible to RoleLeader, to MembersWithRole("driver"), to every +/// role-scoped singleton, and to ConfigPublishCoordinator's expected-ack set. The field +/// symptom was "no driver-role nodes are Up" with nothing wrong in the node's own logs +/// (docs/deployments/wonder-app-vd03-makino-z-34184.md). +/// +/// +[Collection(RoleEnvironmentCollection.Name)] +public class AkkaClusterOptionsRoleFallbackTests +{ + private static AkkaClusterOptions Bind(params (string Key, string Value)[] settings) + { + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(settings.ToDictionary(s => s.Key, s => (string?)s.Value)) + .Build(); + + var services = new ServiceCollection(); + services.AddLogging(); + services.AddOtOpcUaCluster(configuration); + + return services.BuildServiceProvider().GetRequiredService>().Value; + } + + [Fact] + public void Roles_fall_back_to_the_environment_when_Cluster_Roles_is_empty() + { + using (new EnvVarScope(RoleParser.RolesEnvVar, "admin,driver")) + { + Bind().Roles.ShouldBe(new[] { "admin", "driver" }); + } + } + + [Fact] + public void Akka_member_roles_carry_the_fallback() + { + // The end that matters. This is the value handed to Akka.Cluster.Hosting, so an empty array here + // is a role-less member — the exact production shape #532 describes. + using (new EnvVarScope(RoleParser.RolesEnvVar, "driver")) + { + ServiceCollectionExtensions.BuildClusterOptions(Bind()).Roles.ShouldBe(new[] { "driver" }); + } + } + + [Fact] + public void Configured_roles_win_over_the_environment() + { + // Negative control for the fallback: without this, a PostConfigure that unconditionally + // overwrote Roles from the environment would pass the test above and silently ignore every + // deployment's explicit Cluster:Roles. + using (new EnvVarScope(RoleParser.RolesEnvVar, "admin,driver,cluster-SITE-B")) + { + Bind(("Cluster:Roles:0", "driver"), ("Cluster:Roles:1", "cluster-SITE-A")).Roles + .ShouldBe(new[] { "driver", "cluster-SITE-A" }); + } + } + + [Fact] + public void No_roles_from_either_source_stays_empty() + { + using (new EnvVarScope(RoleParser.RolesEnvVar, null)) + { + Bind().Roles.ShouldBeEmpty(); + } + } + + [Fact] + public void The_cluster_role_from_the_environment_reaches_the_ClusterId_derivation() + { + // The over-grant fix in ConfigDbApiKeyStore reads its ClusterId off these options, and a null + // ClusterId applies no cluster filter at all — so an env-only node failed OPEN before #532. + using (new EnvVarScope(RoleParser.RolesEnvVar, "driver,cluster-SITE-A")) + { + RoleParser.ClusterIdFromRoles(Bind().Roles).ShouldBe("site-a"); + } + } +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/ConfigSourceOptionsValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/ConfigSourceOptionsValidatorTests.cs index 419f522d..27b97d2a 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/ConfigSourceOptionsValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/ConfigSourceOptionsValidatorTests.cs @@ -10,6 +10,7 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests; /// applies a deployment, and the deploy times out naming a node that is otherwise healthy. These /// tests make that misconfiguration a loud host-start failure instead. ///
+[Collection(RoleEnvironmentCollection.Name)] public class ConfigSourceOptionsValidatorTests { private static ValidateOptionsResult Validate(ConfigSourceOptions o, params string[] roles) diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/EnvVarScope.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/EnvVarScope.cs new file mode 100644 index 00000000..2c626175 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/EnvVarScope.cs @@ -0,0 +1,44 @@ +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests; + +/// Sets an environment variable for the scope's lifetime and restores its prior value on dispose. +public sealed class EnvVarScope : IDisposable +{ + private readonly string _name; + private readonly string? _previous; + + /// Sets to until disposed. + /// The environment variable name. + /// The value to set, or null to unset. + public EnvVarScope(string name, string? value) + { + _name = name; + _previous = Environment.GetEnvironmentVariable(name); + Environment.SetEnvironmentVariable(name, value); + } + + /// + public void Dispose() => Environment.SetEnvironmentVariable(_name, _previous); +} + +/// +/// Serializes every test class that reads or writes the OTOPCUA_ROLES process environment. +/// +/// +/// ⚠️ Load-bearing since Gitea #532. Environment variables are process-global while xUnit runs +/// test classes in an assembly concurrently, so a class that sets OTOPCUA_ROLES is +/// visible to every class running beside it. That was harmless while +/// SplitTopologyTransportValidator was the only code applying the fallback — its own tests both +/// set and read the variable. It stopped being harmless when ConfigSourceOptionsValidator and +/// TelemetryOptionsValidator started resolving effective roles the same way: their cases that +/// pass NO roles would intermittently observe another class's driver,cluster-SITE-A and change +/// verdict. Any test touching that variable, directly or through +/// RoleParser.ResolveEffectiveRoles, belongs in this collection. +/// +[CollectionDefinition(Name, DisableParallelization = true)] +public sealed class RoleEnvironmentCollection +{ + /// The collection name to put on [Collection]. + public const string Name = "role-environment"; +} diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/RoleParserTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/RoleParserTests.cs index ceac246b..1c7804f7 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/RoleParserTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/RoleParserTests.cs @@ -89,6 +89,12 @@ public sealed class RoleParserTests [InlineData("driver", false)] [InlineData("admin", false)] [InlineData("clusterx", false)] + // Gitea #532: the prefix test is case-insensitive. Cluster:Roles binds verbatim, so an operator's + // 'Cluster-SITE-A' used to classify as an ordinary role — the node then gossiped a cluster-scoped + // role while resolving a null ClusterId for itself. + [InlineData("Cluster-SITE-A", true)] + [InlineData("CLUSTER-main", true)] + [InlineData("Cluster-", false)] public void IsClusterRole_classifies_correctly(string role, bool expected) { RoleParser.IsClusterRole(role).ShouldBe(expected); @@ -101,6 +107,65 @@ public sealed class RoleParserTests RoleParser.ClusterIdFromRole("cluster-SITE-A").ShouldBe("SITE-A"); } + /// The suffix keeps the operator's spelling — consumers compare ClusterIds case-insensitively. + [Fact] + public void ClusterIdFromRole_preserves_the_authored_case() + { + // Normalizing here would silently rename the Akka role of a running deployment (the docker-dev + // rig configures cluster-MAIN / cluster-SITE-A verbatim), so the case is carried, not folded. + RoleParser.ClusterIdFromRole("Cluster-SITE-A").ShouldBe("SITE-A"); + } + + /// Verifies the ClusterId derivation picks the first cluster role, in configuration order. + [Fact] + public void ClusterIdFromRoles_takes_the_first_cluster_role() + { + RoleParser.ClusterIdFromRoles(["driver", "cluster-SITE-A", "cluster-SITE-B"]).ShouldBe("SITE-A"); + RoleParser.ClusterRoleFromRoles(["driver", "cluster-SITE-A"]).ShouldBe("cluster-SITE-A"); + RoleParser.ClusterRolesIn(["driver", "cluster-SITE-A", "cluster-SITE-B"]).Count.ShouldBe(2); + } + + /// Verifies a node with no cluster-scoped role derives a null ClusterId rather than throwing. + [Fact] + public void ClusterIdFromRoles_is_null_for_a_legacy_node() + { + RoleParser.ClusterIdFromRoles(["admin", "driver"]).ShouldBeNull(); + RoleParser.ClusterIdFromRoles([]).ShouldBeNull(); + RoleParser.ClusterIdFromRoles(null).ShouldBeNull(); + } + + /// Verifies the mixed-case derivation gap #532 named: classification folds case, the id does not. + [Fact] + public void ClusterIdFromRoles_resolves_a_mixed_case_cluster_role() + { + // Before #532 this returned null, and a null ClusterId applies NO cluster filter — so + // ConfigDbApiKeyStore's cross-cluster guard failed open on exactly this configuration. + RoleParser.ClusterIdFromRoles(["driver", "Cluster-SITE-A"]).ShouldBe("SITE-A"); + } + + /// Verifies Cluster:Roles wins outright over the environment fallback. + [Fact] + public void ResolveEffectiveRoles_prefers_the_configured_list() + { + RoleParser.ResolveEffectiveRoles(["admin"], "driver,cluster-SITE-A").ShouldBe(new[] { "admin" }); + } + + /// Verifies an empty or absent Cluster:Roles falls back to OTOPCUA_ROLES. + [Fact] + public void ResolveEffectiveRoles_falls_back_to_the_environment() + { + RoleParser.ResolveEffectiveRoles([], "driver,cluster-SITE-A") + .ShouldBe(new[] { "driver", "cluster-site-a" }); + RoleParser.ResolveEffectiveRoles(null, "admin").ShouldBe(new[] { "admin" }); + } + + /// Verifies neither source naming a role yields an empty list rather than a null or a throw. + [Fact] + public void ResolveEffectiveRoles_with_neither_source_is_empty() + { + RoleParser.ResolveEffectiveRoles(null, null).ShouldBeEmpty(); + } + /// Verifies the three well-known role constants match the historical literal values. [Fact] public void Well_known_role_constants() diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs index b873db2e..6d5b018c 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SplitTopologyTransportValidatorTests.cs @@ -12,6 +12,7 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests; /// that misconfiguration a loud host-start failure instead. A node with no cluster-role is exempt /// (legacy / single-mesh / test), and the validator is a no-op for it. /// +[Collection(RoleEnvironmentCollection.Name)] public class SplitTopologyTransportValidatorTests { private static ValidateOptionsResult Validate( @@ -248,20 +249,4 @@ public class SplitTopologyTransportValidatorTests telemetryDialMode: null, "cluster-SITE-A").Succeeded.ShouldBeTrue(); } - - /// Sets an environment variable for the scope's lifetime and restores its prior value on dispose. - private sealed class EnvVarScope : IDisposable - { - private readonly string _name; - private readonly string? _previous; - - public EnvVarScope(string name, string? value) - { - _name = name; - _previous = Environment.GetEnvironmentVariable(name); - Environment.SetEnvironmentVariable(name, value); - } - - public void Dispose() => Environment.SetEnvironmentVariable(_name, _previous); - } } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/TelemetryOptionsValidatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/TelemetryOptionsValidatorTests.cs index c55bd59a..d4bb9e14 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/TelemetryOptionsValidatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/TelemetryOptionsValidatorTests.cs @@ -10,6 +10,7 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster.Tests; /// telemetry, and whoever is watching sees nothing with no stack trace to point at. These tests /// make that misconfiguration a loud host-start failure instead. /// +[Collection(RoleEnvironmentCollection.Name)] public class TelemetryOptionsValidatorTests { private static ValidateOptionsResult Validate(TelemetryOptions o, params string[] roles) @@ -112,6 +113,7 @@ public class TelemetryOptionsValidatorTests /// /// Mirrors for the central dial-side options. /// +[Collection(RoleEnvironmentCollection.Name)] public class TelemetryDialOptionsValidatorTests { private static ValidateOptionsResult Validate(TelemetryDialOptions o) => 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..6934a57d --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorNodeAclScopeTests.cs @@ -0,0 +1,459 @@ +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(); + } + + /// + /// Gitea #533 — the gate's own identifier comparisons follow the same rule as the resolver's: + /// identifiers compare OrdinalIgnoreCase. Both were ordinal, which put the gate and the + /// runtime into disagreement in the direction that costs the most: the deploy gate would reject a + /// whole configuration as cross-cluster over a ClusterId case difference that + /// PermissionTrieBuilder — which has always matched case-insensitively — would have accepted. + /// + [Fact] + public void A_case_drifted_ClusterId_is_not_reported_as_cross_cluster() + { + var acl = Acl(NodeAclScopeKind.RawDevice, "dev-1"); + acl.ClusterId = Own.ToUpperInvariant(); + + var errors = DraftValidator.Validate(Draft(acl)); + + errors.ShouldNotContain(e => e.Code == "NodeAclScopeCrossCluster"); + errors.ShouldNotContain(e => e.Code == "NodeAclScopeUnresolved"); + } + + /// + /// The same rule applied to the ScopeId lookups, so a row this gate passes is a row + /// AclScopePathResolver resolves — the property the gate's whole value rests on. A gate + /// stricter than the resolver blocks deploys that would have worked; a gate looser than it lets + /// through the silently-dropped grants it exists to catch. + /// + [Fact] + public void A_case_drifted_ScopeId_resolves_at_the_gate_too() + { + DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.RawDevice, "DEV-1"))) + .ShouldNotContain(e => e.Code.StartsWith("NodeAcl", StringComparison.Ordinal)); + } + + /// Negative control: relaxing case must not stop a genuinely foreign cluster from being caught. + [Fact] + public void A_genuinely_different_ClusterId_is_still_cross_cluster() + { + DraftValidator.Validate(Draft(Acl(NodeAclScopeKind.RawDevice, "dev-2"))) + .ShouldContain(e => e.Code == "NodeAclScopeCrossCluster"); + } + + /// 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..e3bdcb82 --- /dev/null +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/AclScopePathResolverTests.cs @@ -0,0 +1,330 @@ +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")); + } + + [Theory] + [InlineData(NodeAclScopeKind.RawFolder, "rf-line1")] + [InlineData(NodeAclScopeKind.RawDevice, "dv-1")] + [InlineData(NodeAclScopeKind.Equipment, "eq-MIXER")] + [InlineData(NodeAclScopeKind.Tag, "TAG-SPEED")] + public void A_case_drifted_scope_id_still_resolves(NodeAclScopeKind kind, string scopeId) + { + // Gitea #533 — the identifier half of the rule. ScopeId is an opaque handle, and it is operator + // free-text in the ACL editor until the stage-7 picker lands, so two spellings always mean one + // entity. Ordinal lookups made a case slip drop EVERY grant on that scope: fail-closed and + // diagnosed, but indistinguishable from a genuine typo and invisible to the author. + Fixture().ResolveOne(kind, scopeId).ShouldNotBeEmpty(); + } + + [Fact] + public void A_case_drifted_scope_id_resolves_to_the_SAME_paths_as_the_authored_one() + { + // Stronger than "resolves to something": the paths must be identical, because the segments are + // entity NAMES and must keep their own spelling. A lookup that folded case into the emitted path + // would produce an ACL path that no longer matches the RawPath forming the node's NodeId. + var drifted = Fixture().ResolveOne(NodeAclScopeKind.RawDevice, "dv-1").ShouldHaveSingleItem(); + var authored = Fixture().ResolveOne(NodeAclScopeKind.RawDevice, "DV-1").ShouldHaveSingleItem(); + + drifted.Segments.ShouldBe(authored.Segments); + drifted.Segments.ShouldBe(["raw", "Plant", "Line1", "ModbusDrv", "dev1"]); + } + + [Fact] + public void A_case_drifted_scope_ref_hits_the_same_dictionary_entry() + { + // The key type's own equality, which is what the trie builder looks the resolver's output up by. + // Resolver and builder can be fed from different snapshots (EF at the deploy gate, artifact JSON + // on the node) — the whole reason this resolver is input-shape agnostic — so their agreement + // cannot rest on the two sides holding the same string instances. + var resolved = Fixture().Resolve([Acl("acl-1", NodeAclScopeKind.Equipment, "EQ-mixer")]); + + resolved.ShouldContainKey(new AclScopeRef(NodeAclScopeKind.Equipment, "eq-MIXER")); + new AclScopeRef(NodeAclScopeKind.Equipment, "EQ-mixer") + .ShouldBe(new AclScopeRef(NodeAclScopeKind.Equipment, "eq-mixer")); + } + + [Fact] + public void Case_insensitive_ids_do_NOT_merge_different_scope_kinds() + { + // Negative control for the widening: relaxing the id comparison must not relax the kind, or the + // wrong-node authorization the composite key exists to prevent comes back through the case door. + new AclScopeRef(NodeAclScopeKind.RawFolder, "dup") + .ShouldNotBe(new AclScopeRef(NodeAclScopeKind.UnsArea, "DUP")); + } + + [Fact] + public void A_case_drifted_tag_id_still_reaches_every_UNS_projection() + { + // The reference scan is a linear compare rather than a dictionary lookup, so it needed the rule + // applied separately — and this is the path where missing it is worst: a Tag grant that resolved + // its raw path but none of its UNS paths would leave the tag writable through the UNS NodeId it + // silently skipped. + Fixture().ResolveOne(NodeAclScopeKind.Tag, "tag-speed").Count.ShouldBe(3); + } + + 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..cd41c6be 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,225 @@ 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"); + } + + /// + /// Gitea #533 — the retired gets its own reason. It + /// used to arrive as , which told + /// an operator holding a legacy v2 row to go hunting for a typo in a ScopeId that was never + /// the problem — the kind itself has no v3 entity behind it and can never resolve. The two have + /// different remedies, so they need different reasons. + /// + [Fact] + public void Build_Retired_FolderSegment_Row_Emits_RetiredScopeKind() + { + var row = Row("cn=ops", NodeAclScopeKind.FolderSegment, "seg-1", NodePermissions.Read); + var diagnostics = new List(); + + var trie = PermissionTrieBuilder.Build( + "c1", 1, [row], new Dictionary>(), diagnostics.Add); + + diagnostics.Count.ShouldBe(1); + diagnostics[0].Reason.ShouldBe(PermissionTrieBuildDiagnosticReason.RetiredScopeKind); + diagnostics[0].ScopeId.ShouldBe("seg-1", "the id is still reported, so the operator can find the row"); + trie.Root.Grants.ShouldBeEmpty("a retired-kind row must not fall through to the cluster root"); + } + + /// + /// A genuinely dangling id must keep reporting . + /// Without this, moving FolderSegment to its own reason could be "achieved" by relabelling + /// every drop, and the split would carry no information. + /// + [Fact] + public void Build_A_Live_Kind_With_A_Dangling_Id_Still_Emits_UnresolvedScope() + { + var row = Row("cn=ops", NodeAclScopeKind.RawFolder, "RF-gone", NodePermissions.Read); + var diagnostics = new List(); + + PermissionTrieBuilder.Build( + "c1", 1, [row], new Dictionary>(), diagnostics.Add); + + diagnostics.ShouldHaveSingleItem().Reason + .ShouldBe(PermissionTrieBuildDiagnosticReason.UnresolvedScope); + } + + /// + /// Gitea #533 — the identifier rule reaches the builder's own ClusterId filter, which has + /// always been OrdinalIgnoreCase. Pinned because the two spellings genuinely occur: a + /// ClusterId arrives lower-cased through OTOPCUA_ROLES and verbatim through + /// Cluster:Roles. + /// + [Fact] + public void Build_Matches_The_Cluster_Case_Insensitively() + { + var rows = new[] { Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.Read) }; + + PermissionTrieBuilder.Build( + "C1", 1, rows, new Dictionary>()) + .Root.Grants.ShouldHaveSingleItem(); + + PermissionTrieBuilder.Build( + "c2", 1, rows, new Dictionary>()) + .Root.Grants.ShouldBeEmpty("a DIFFERENT cluster must still be excluded"); } } 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/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipGatewayCompositionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipGatewayCompositionTests.cs new file mode 100644 index 00000000..017e60b0 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipGatewayCompositionTests.cs @@ -0,0 +1,44 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Gitea #534 — the authored EtherNet/IP port must reach libplctag's gateway attribute. +/// +/// +/// This driver carried the identical defect to AbLegacy, where it was found: +/// was on the record and consumed by nobody, so the runtime +/// handed libplctag the bare host and libplctag substituted 44818. It survived here precisely because +/// it is invisible here — every AbCip fixture profile binds 44818, so no test this driver has could +/// distinguish "uses the authored port" from "always uses 44818". Worth remembering as a shape: a +/// fixture that only ever exercises the default value cannot falsify a rule about non-default values, +/// however many tests point at it. +/// +public sealed class AbCipGatewayCompositionTests +{ + private static AbCipTagCreateParams Params(string host, int port) => + new(Gateway: host, Port: port, CipPath: "1,0", LibplctagPlcAttribute: "controllogix", + TagName: "TestDINT", Timeout: TimeSpan.FromSeconds(5)); + + [Fact] + public void The_authored_port_is_appended_to_the_gateway() + { + Params("10.100.0.35", 44900).LibplctagGateway.ShouldBe("10.100.0.35:44900"); + } + + [Fact] + public void The_default_port_is_still_written_explicitly() + { + Params("plc1.plant.local", 44818).LibplctagGateway.ShouldBe("plc1.plant.local:44818"); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void A_missing_port_falls_back_to_the_bare_host(int port) + { + Params("10.100.0.35", port).LibplctagGateway.ShouldBe("10.100.0.35"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadSmokeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadSmokeTests.cs index 313892ce..9bbfd8f4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadSmokeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyReadSmokeTests.cs @@ -41,13 +41,13 @@ public sealed class AbLegacyReadSmokeTests(AbLegacyServerFixture sim) [MemberData(nameof(Profiles))] public async Task Driver_reads_seeded_N_file_from_ab_server_PCCC(AbLegacyServerProfile profile) { - if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + if (sim.SkipReasonFor(profile) is { } skip) Assert.Skip(skip); // PCCC semantics allow an empty cip-path (real SLC/PLC-5 hardware takes nothing // after the `/`), but libplctag's ab_server requires a non-empty path at the // CIP unconnected-send layer before the PCCC dispatcher runs. Default `1,0` // against the Docker fixture; set AB_LEGACY_CIP_PATH= (empty) against real HW. - var deviceUri = $"ab://{sim.Host}:{sim.Port}/{sim.CipPath}"; + var deviceUri = $"ab://{sim.Host}:{sim.PortFor(profile)}/{sim.CipPath}"; await using var drv = new AbLegacyDriver(new AbLegacyDriverOptions { Devices = [new AbLegacyDeviceOptions(deviceUri, profile.Family)], @@ -75,23 +75,25 @@ public sealed class AbLegacyReadSmokeTests(AbLegacyServerFixture sim) [AbLegacyFact] public async Task Slc500_write_then_read_round_trip_on_N7_scratch_register() { - if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + // Addresses the SLC500 container by its OWN port, so it no longer matters which other + // families happen to be running. The old AB_LEGACY_COMPOSE_PROFILE guard existed because every + // family shared :44818 and this test could otherwise write SLC500 semantics at whichever + // container held the port; it survives below only for real-hardware runs, where + // AB_LEGACY_ENDPOINT pins one device and the family IS a property of that device. + if (sim.SkipReasonFor(KnownProfiles.Slc500) is { } skip) Assert.Skip(skip); - // Skip when the running compose profile isn't SLC500 — ab_server's `--plc=` - // flag selects exactly one family per process, so a write against a plc5-mode - // container with SLC500 semantics always fails at the wire. var only = Environment.GetEnvironmentVariable("AB_LEGACY_COMPOSE_PROFILE"); - if (!string.IsNullOrEmpty(only) && + if (sim.EndpointPinned && !string.IsNullOrEmpty(only) && !string.Equals(only, "slc500", StringComparison.OrdinalIgnoreCase)) { - Assert.Skip($"Test targets the SLC500 compose profile; AB_LEGACY_COMPOSE_PROFILE='{only}'."); + Assert.Skip($"Pinned endpoint is not SLC500; AB_LEGACY_COMPOSE_PROFILE='{only}'."); } // PCCC semantics allow an empty cip-path (real SLC/PLC-5 hardware takes nothing // after the `/`), but libplctag's ab_server requires a non-empty path at the // CIP unconnected-send layer before the PCCC dispatcher runs. Default `1,0` // against the Docker fixture; set AB_LEGACY_CIP_PATH= (empty) against real HW. - var deviceUri = $"ab://{sim.Host}:{sim.Port}/{sim.CipPath}"; + var deviceUri = $"ab://{sim.Host}:{sim.PortFor(KnownProfiles.Slc500)}/{sim.CipPath}"; await using var drv = new AbLegacyDriver(new AbLegacyDriverOptions { Devices = [new AbLegacyDeviceOptions(deviceUri, AbLegacyPlcFamily.Slc500)], diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyServerFixture.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyServerFixture.cs index 7faa0ca8..f0a5a347 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyServerFixture.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/AbLegacyServerFixture.cs @@ -17,9 +17,11 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests; /// /// Env-var overrides: /// -/// AB_LEGACY_ENDPOINT — host:port of the PCCC-mode simulator. -/// Defaults to 10.100.0.35:44818 — the shared Docker host (EtherNet/IP port; ab_server's PCCC -/// emulation exposes PCCC-over-CIP on the same port as CIP itself). +/// AB_LEGACY_ENDPOINT — host:port of a single PCCC-mode device, pinning +/// every family to it (real-hardware mode, where the family is a property of the device +/// rather than of the port). Unset — the default — each family is reached on its own port on the +/// shared Docker host: 10.100.0.35:44819 slc500, :44820 micrologix, +/// :44821 plc5. /// AB_LEGACY_CIP_PATH — routing path appended to the ab://host:port/ /// URI. Defaults to 1,0 (port-1/slot-0 backplane), required by ab_server /// which rejects unconnected Send_RR_Data with an empty path at the CIP layer @@ -27,8 +29,10 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests; /// use an empty path — set AB_LEGACY_CIP_PATH= (empty) when pointing at /// real hardware. /// -/// Distinct from AB_SERVER_ENDPOINT used by the AB CIP fixture so both -/// can point at different containers simultaneously during a combined test run. +/// Distinct from AB_SERVER_ENDPOINT used by the AB CIP fixture so both can point at different +/// containers simultaneously during a combined test run — which was the stated intent from the start +/// but was not true in practice until 2026-08-01, because every ab_server across both fixtures +/// defaulted to :44818. /// public sealed class AbLegacyServerFixture : IAsyncLifetime { @@ -56,15 +60,58 @@ public sealed class AbLegacyServerFixture : IAsyncLifetime /// Gets the server host address. public string Host { get; } = "10.100.0.35"; - /// Gets the server TCP port. + /// + /// The port every family shares when AB_LEGACY_ENDPOINT pins one, or + /// . Prefer — against the Docker fixture each + /// family listens on its own port and this value addresses none of them. + /// public int Port { get; } = DefaultPort; + /// True when AB_LEGACY_ENDPOINT pinned a single endpoint (real-hardware mode). + public bool EndpointPinned { get; } + + /// + /// The port to reach 's simulator on: the family's own port against the + /// Docker fixture, or the pinned endpoint's port when AB_LEGACY_ENDPOINT is set. + /// + /// The family profile under test. + /// The TCP port. + public int PortFor(AbLegacyServerProfile profile) + { + ArgumentNullException.ThrowIfNull(profile); + return EndpointPinned ? Port : profile.Port; + } + + /// + /// Why 's simulator cannot be reached, or null when it can. + /// + /// + /// ⚠️ Per-family, not per-suite (2026-08-01). Every family used to share :44818, so + /// this could only ever answer "is some ab_server up" — and the AB CIP fixture's ControlLogix + /// container answers there too. The probe therefore reported reachable, the suite declined to + /// skip, and SLC500-mode PCCC ran against a ControlLogix and failed at the wire. A reachability + /// check that cannot tell one simulator from another is not a skip gate; it is a coin flip that + /// reports its result as a test failure. + /// + /// The family profile under test. + /// A skip reason, or . + public string? SkipReasonFor(AbLegacyServerProfile profile) + { + ArgumentNullException.ThrowIfNull(profile); + var port = PortFor(profile); + if (TcpProbe(Host, port)) return null; + + return EndpointPinned + ? $"AB Legacy PCCC endpoint at {Host}:{port} ({EndpointEnvVar}) not reachable within 2 s." + : $"AB Legacy {profile.ComposeProfile} simulator at {Host}:{port} not reachable within 2 s. " + + $"Start it (docker compose -f Docker/docker-compose.yml --profile {profile.ComposeProfile} " + + $"up -d), attach real hardware, or override {EndpointEnvVar}."; + } + /// CIP routing path portion of the device URI (after the / separator). /// May be empty when targeting real hardware; non-empty against ab_server. public string CipPath { get; } = DefaultCipPath; - /// Gets the skip reason if the server is unavailable, or null if available. - public string? SkipReason { get; } /// Initializes the AB Legacy server fixture by reading environment overrides. public AbLegacyServerFixture() @@ -74,6 +121,7 @@ public sealed class AbLegacyServerFixture : IAsyncLifetime var parts = raw.Split(':', 2); Host = parts[0]; if (parts.Length == 2 && int.TryParse(parts[1], out var p)) Port = p; + EndpointPinned = true; } // Empty override is intentional (real hardware); treat `null` as "not set, use @@ -81,7 +129,6 @@ public sealed class AbLegacyServerFixture : IAsyncLifetime var cipOverride = Environment.GetEnvironmentVariable(CipPathEnvVar); if (cipOverride is not null) CipPath = cipOverride; - SkipReason = ResolveSkipReason(Host, Port); } /// Initializes the fixture asynchronously. @@ -97,29 +144,19 @@ public sealed class AbLegacyServerFixture : IAsyncLifetime /// exists. /// public static bool IsServerAvailable() - { - var (host, port) = ResolveEndpoint(); - return ResolveSkipReason(host, port) is null; - } - - private static string? ResolveSkipReason(string host, int port) - { - if (!TcpProbe(host, port)) - { - return $"AB Legacy PCCC endpoint at {host}:{port} not reachable within 2 s. " + - $"Start the Docker container (docker compose -f Docker/docker-compose.yml " + - $"--profile slc500 up -d), attach real hardware, or override {EndpointEnvVar}."; - } - return null; - } - - private static (string Host, int Port) ResolveEndpoint() { var raw = Environment.GetEnvironmentVariable(EndpointEnvVar); - if (raw is null) return ("10.100.0.35", DefaultPort); - var parts = raw.Split(':', 2); - var port = parts.Length == 2 && int.TryParse(parts[1], out var p) ? p : DefaultPort; - return (parts[0], port); + if (raw is { Length: > 0 }) + { + var parts = raw.Split(':', 2); + var pinned = parts.Length == 2 && int.TryParse(parts[1], out var p) ? p : DefaultPort; + return TcpProbe(parts[0], pinned); + } + + // ANY family's simulator being up runs the class; each test then skips itself on its own port + // via SkipReasonFor. Gating the whole class on one family would skip the other two whenever it + // happened to be down, which reads as coverage that ran. + return KnownProfiles.All.Any(profile => TcpProbe("10.100.0.35", profile.Port)); } private static bool TcpProbe(string host, int port) @@ -140,27 +177,42 @@ public sealed class AbLegacyServerFixture : IAsyncLifetime /// for which --plc mode + tags each family seeds; this record just ties a /// family enum to its compose-profile name + operator-facing notes. /// +/// The PCCC family. +/// The compose profile name that starts this family's container. +/// +/// The family's own host port. Each family gets one so all three run together, and alongside the +/// AB CIP fixture on :44818. Overridden wholesale by AB_LEGACY_ENDPOINT (real hardware, +/// where there is one device and the family is a property of it, not of the port). +/// +/// Operator-facing description of what the family's container seeds. public sealed record AbLegacyServerProfile( AbLegacyPlcFamily Family, string ComposeProfile, + int Port, string Notes); /// Canonical profiles covering every PCCC family the driver supports. +/// +/// The Port values must match Docker/docker-compose.yml's host-side mappings. +/// public static class KnownProfiles { public static readonly AbLegacyServerProfile Slc500 = new( Family: AbLegacyPlcFamily.Slc500, ComposeProfile: "slc500", + Port: 44819, Notes: "SLC 500 / 5/05 family. ab_server SLC500 mode covers N/F/B/L files."); public static readonly AbLegacyServerProfile MicroLogix = new( Family: AbLegacyPlcFamily.MicroLogix, ComposeProfile: "micrologix", + Port: 44820, Notes: "MicroLogix 1000 / 1100 / 1400. Shares N/F/B file-type coverage with SLC500; ST (ASCII strings) included."); public static readonly AbLegacyServerProfile Plc5 = new( Family: AbLegacyPlcFamily.Plc5, ComposeProfile: "plc5", + Port: 44821, Notes: "PLC-5 family. ab_server PLC/5 mode covers N/F/B; per-family quirks on ST / timer file layouts unit-tested only."); /// Gets all known profiles. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/README.md b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/README.md index 2245bd23..84edcf90 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/README.md +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/README.md @@ -13,7 +13,7 @@ Desktop and nothing else. | File | Purpose | |---|---| -| [`docker-compose.yml`](docker-compose.yml) | Three per-family services (`slc500` / `micrologix` / `plc5`); all bind `:44818` | +| [`docker-compose.yml`](docker-compose.yml) | Three per-family services — `slc500` on `:44819`, `micrologix` on `:44820`, `plc5` on `:44821`. Run them together; they no longer collide with each other or with the AB CIP fixture on `:44818`. | ## Run @@ -38,12 +38,12 @@ docker compose -f tests\...\Docker\docker-compose.yml --profile slc500 down First run builds the `otopcua-ab-server:libplctag-release` image (~3-5 min — clones libplctag + compiles `ab_server`). If the AB CIP fixture already built the image locally, docker reuses the cached layers + this -runs in seconds. Only one family binds `:44818` at a time; to switch +runs in seconds. (Until 2026-08-01 every family bound `:44818`, so only one could run and the theory's other two cases always failed; each now has its own port.) To switch families stop the current service + start another. ## Endpoint -- Default: `localhost:44818` (EtherNet/IP standard) +- Default: `10.100.0.35:44819` / `:44820` / `:44821` per family (the shared Docker host) - Override with `AB_LEGACY_ENDPOINT=host:port` to point at a real SLC / MicroLogix / PLC-5 PLC on its native port. @@ -51,9 +51,9 @@ families stop the current service + start another. | Var | Default | Purpose | |---|---|---| -| `AB_LEGACY_ENDPOINT` | `localhost:44818` | `host:port` of the PCCC endpoint. | +| `AB_LEGACY_ENDPOINT` | *unset* — per-family ports on `10.100.0.35` | `host:port` of a single PCCC device, pinning **every** family to it (real-hardware mode). | | `AB_LEGACY_CIP_PATH` | `1,0` | CIP routing path portion of the `ab://host:port/` URI. ab_server rejects empty paths at the CIP unconnected-send layer; real SLC/MicroLogix/PLC-5 hardware accepts empty (no backplane). Set to empty (`AB_LEGACY_CIP_PATH=`) when pointing at real hardware. | -| `AB_LEGACY_COMPOSE_PROFILE` | *unset* | When set (e.g. `slc500`), the parametric theory filters to that profile. Only one compose container binds `:44818` at a time; set this to the profile currently up so the suite doesn't try to hit e.g. the Slc500 family against the PLC-5 container. Leave unset for real-hardware runs (all 3 families parameterize). | +| `AB_LEGACY_COMPOSE_PROFILE` | *unset* | Only consulted when `AB_LEGACY_ENDPOINT` pins an endpoint: names which family that one device is, so the SLC500-only write test skips against other hardware. Against the Docker fixture it is unnecessary — each family has its own port, and each test skips itself if its own port is down. | ## Run the integration tests diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/docker-compose.yml b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/docker-compose.yml index f0d34323..97e4df73 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/docker-compose.yml +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.IntegrationTests/Docker/docker-compose.yml @@ -5,15 +5,20 @@ # so `docker compose build` from here produces the same image if it # doesn't already exist; if it does, docker's cache reuses the layer. # -# One service per PCCC family. All bind :44818 on the host; run one at a -# time. PCCC tag format differs from CIP: `[]` without a -# type suffix since the type is implicit in the file letter (N = INT, -# F = REAL, B = bit-packed, L = DINT). +# One service per PCCC family, each on its OWN host port — 44819 slc500, +# 44820 micrologix, 44821 plc5 — so all three run together, and alongside the +# AB CIP fixture, which owns :44818. They all bound :44818 until 2026-08-01, +# which made the suite structurally un-greenable: the theory parameterises over +# all three families, but only one container could ever exist, so at least two +# cases failed on every run. Worse, the AB CIP ControlLogix container answers +# on :44818 too, so the fixture's TCP reachability probe saw "up", declined to +# skip, and ran SLC500-mode PCCC against a ControlLogix. # -# Usage: -# docker compose --profile slc500 up -# docker compose --profile micrologix up -# docker compose --profile plc5 up +# ab_server still listens on 44818 INSIDE the container (--port=44818); only the +# host mapping differs. Keep these in step with AbLegacyServerProfile.Port. +# +# Usage (all three at once — that is the point): +# docker compose --profile slc500 --profile micrologix --profile plc5 up -d services: slc500: profiles: ["slc500"] @@ -24,7 +29,7 @@ services: container_name: otopcua-ab-server-slc500 restart: "no" ports: - - "44818:44818" + - "44819:44818" command: [ "ab_server", "--plc=SLC500", @@ -44,7 +49,7 @@ services: container_name: otopcua-ab-server-micrologix restart: "no" ports: - - "44818:44818" + - "44820:44818" command: [ "ab_server", "--plc=Micrologix", @@ -63,7 +68,7 @@ services: container_name: otopcua-ab-server-plc5 restart: "no" ports: - - "44818:44818" + - "44821:44818" command: [ "ab_server", "--plc=PLC/5", diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyGatewayCompositionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyGatewayCompositionTests.cs new file mode 100644 index 00000000..062f5360 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests/AbLegacyGatewayCompositionTests.cs @@ -0,0 +1,54 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests; + +/// +/// Gitea #534 — the authored EtherNet/IP port must reach libplctag's gateway attribute. +/// +/// +/// +/// was carried on the record and consumed by nobody: +/// the runtime set Tag.Gateway = p.Gateway, the bare host. libplctag does not fail on a +/// portless gateway — it substitutes 44818. So the authored port was not ignored in the harmless +/// sense; the connection was silently *redirected* to whatever answers on the default port. On the +/// test rig that was a different simulator running a different PLC family, which is why the +/// AbLegacy integration suite failed in a way that looked like a missing fixture. On a plant +/// network it is another PLC, and the driver would read someone else's tags and report them Good. +/// +/// +/// The real guard is the integration suite, which now addresses each PCCC family on its own +/// non-default port (44819/44820/44821) and therefore cannot pass unless the port is honoured. +/// These unit tests pin the composition itself so the rule survives on a machine with no fixtures. +/// +/// +public sealed class AbLegacyGatewayCompositionTests +{ + private static AbLegacyTagCreateParams Params(string host, int port) => + new(Gateway: host, Port: port, CipPath: "1,0", LibplctagPlcAttribute: "slc500", + TagName: "N7:0", Timeout: TimeSpan.FromSeconds(5)); + + [Fact] + public void The_authored_port_is_appended_to_the_gateway() + { + Params("10.100.0.35", 44819).LibplctagGateway.ShouldBe("10.100.0.35:44819"); + } + + [Fact] + public void The_default_port_is_still_written_explicitly() + { + // Not an optimisation opportunity: emitting the port unconditionally is what makes the value + // independent of libplctag's own default, so a change to that default cannot move our traffic. + Params("plc1.plant.local", 44818).LibplctagGateway.ShouldBe("plc1.plant.local:44818"); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void A_missing_port_falls_back_to_the_bare_host(int port) + { + // An unset port must not produce "host:0", which libplctag would take literally. + Params("10.100.0.35", port).LibplctagGateway.ShouldBe("10.100.0.35"); + } +} 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.IntegrationTests/DriverProbeHandshakeE2eTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs index ceee9a94..25de396a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs @@ -178,11 +178,26 @@ public sealed class DriverProbeHandshakeE2eTests // ControlLogix instead returns ErrorNotFound, which the probe classifies as // reachable). Whether the `@raw_cpu_type` system-tag fallback is valid on a real // ControlLogix is a hardware-gated follow-up (AbCipDriverOptions.cs flags it deferred). + // ⚠️ The config must use the v3 `RawTags` shape — `[{RawPath, TagConfig, WriteIdempotent, + // DeviceName}]`, where the Logix tag path lives inside the TagConfig blob as `tagPath`. This + // test passed the retired v2 `Tags: [{DeviceHostAddress, TagPath}]` shape until 2026-08-01, so + // AbCipDriverOptions.RawTags bound EMPTY, the probe fell through to its `@raw_cpu_type` system-tag + // fallback, and ab_server answered ErrorBadParam. Which is to say it spent that whole time + // exercising the one path the comment above says it does not exercise — and reporting it as a + // failure of the fixture rather than of the config shape. var result = await new AbCipDriverProbe().ProbeAsync( $"{{\"Devices\":[{{\"HostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}]," + - $"\"Tags\":[{{\"DeviceHostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\",\"TagPath\":\"TestDINT\"}}]}}", + $"\"RawTags\":[{{\"RawPath\":\"Plant/AbCip/dev1/TestDINT\"," + + $"\"TagConfig\":\"{{\\\"tagPath\\\":\\\"TestDINT\\\",\\\"dataType\\\":\\\"DInt\\\"}}\"," + + $"\"WriteIdempotent\":false,\"DeviceName\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}]}}", Timeout, Ct); result.Ok.ShouldBeTrue($"Probe message: {result.Message}"); - result.Message!.ShouldContain("CIP session OK"); + + // EXACT, not ShouldContain. The probe's two green messages are "CIP session OK" (the tag was + // read) and "CIP session OK (controller reachable; probe tag not found)" (the fallback), and + // `ShouldContain` matches both — so it would have gone green again the moment the config shape + // was merely well-formed enough to reach the controller, whether or not the authored tag was + // ever resolved. Reading the authored tag is the entire point of this test. + result.Message.ShouldBe("CIP session OK"); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeRegistrationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeRegistrationTests.cs index 61d4a148..085f6d31 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeRegistrationTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeRegistrationTests.cs @@ -14,29 +14,20 @@ namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; /// admin path — not only the driver path — or every Test Connect button returns /// "No probe registered for driver type X". /// +/// +/// ⚠️ The expected set is derived from , not hand-written. +/// It used to be a literal array kept "in sync" by comment, and it drifted the moment `Sql` +/// (9b5a9687) and `Mqtt` (f79d13e2) shipped: the count assertion below went red +/// reporting 12 where 10 expected — a true statement that named neither driver and read like +/// the probes were wrong rather than the list. Worse is the direction it could NOT catch: a new +/// driver type registered with no probe passed silently, because a hand-written list grows +/// only when someone remembers to grow it, and this test exists precisely for the case where nobody +/// did. Deriving from the constants makes the guard bidirectional and self-maintaining. +/// public sealed class DriverProbeRegistrationTests { - // The canonical "all drivers" set — one entry per AdminUI typed driver page's DriverTypeKey. - // Keep in sync with the IDriverProbe registrations in - // src/Server/.../Host/Drivers/DriverFactoryBootstrap.AddOtOpcUaDriverProbes (the routed - // *DriverPage.razor flow was retired in v3 Batch 2 — the driver-type keys now live in the /raw - // modals, but the probe set is the authority for what the AdminUI can Test-connect). - private static readonly string[] AdminUiDriverTypeKeys = - [ - "Modbus", - "AbCip", - "AbLegacy", - "S7", - "TwinCat", // page key; probe reports "TwinCAT" — must resolve case-insensitively - "Focas", // page key; probe reports "FOCAS" — must resolve case-insensitively - "OpcUaClient", - "GalaxyMxGateway", - "Calculation", // v3 Batch 2 pseudo-driver; CalculationProbe registered in DriverFactoryBootstrap - "MTConnect", // MTConnectProbe — read-only Agent driver; probe issues a bare GET /probe - ]; - [Fact] - public void AddOtOpcUaDriverProbes_registers_a_probe_for_every_AdminUI_driver_type() + public void AddOtOpcUaDriverProbes_registers_a_probe_for_every_driver_type() { var services = new ServiceCollection(); services.AddOtOpcUaDriverProbes(); @@ -48,8 +39,50 @@ public sealed class DriverProbeRegistrationTests // (case-insensitive) and would throw on a duplicate key, crashing the singleton. var byType = probes.ToDictionary(p => p.DriverType, StringComparer.OrdinalIgnoreCase); - foreach (var key in AdminUiDriverTypeKeys) - byType.ContainsKey(key).ShouldBeTrue($"No IDriverProbe registered for AdminUI driver type '{key}'."); + var missing = DriverTypeNames.All.Where(t => !byType.ContainsKey(t)).ToList(); + missing.ShouldBeEmpty( + "Driver types with no IDriverProbe — Test Connect returns 'No probe registered for driver " + + $"type X' for each: {string.Join(", ", missing)}"); + } + + [Fact] + public void No_probe_is_registered_for_a_driver_type_that_does_not_exist() + { + // The other direction. A probe whose DriverType matches no DriverTypeNames constant can never + // be reached — AdminOperationsActor looks probes up by the authored DriverType — so it is a + // typo or a leftover from a renamed driver, and it reads as coverage while providing none. + var services = new ServiceCollection(); + services.AddOtOpcUaDriverProbes(); + + using var sp = services.BuildServiceProvider(); + + var orphans = sp.GetServices() + .Select(p => p.DriverType) + .Where(t => !DriverTypeNames.All.Contains(t, StringComparer.OrdinalIgnoreCase)) + .ToList(); + + orphans.ShouldBeEmpty($"Probes for unknown driver types: {string.Join(", ", orphans)}"); + } + + /// + /// Two driver types are spelled differently by the constant and by the AdminUI's own key + /// (TwinCAT/TwinCat, FOCAS/Focas), so the lookup must fold case. The + /// derived tests above cannot cover this — they compare the constants against themselves. + /// + [Theory] + [InlineData("TwinCat", DriverTypeNames.TwinCAT)] + [InlineData("Focas", DriverTypeNames.FOCAS)] + [InlineData("modbus", DriverTypeNames.Modbus)] + public void A_probe_resolves_by_a_case_drifted_driver_type_key(string authoredKey, string canonical) + { + var services = new ServiceCollection(); + services.AddOtOpcUaDriverProbes(); + + using var sp = services.BuildServiceProvider(); + var byType = sp.GetServices().ToDictionary(p => p.DriverType, StringComparer.OrdinalIgnoreCase); + + byType.ShouldContainKey(authoredKey); + byType[authoredKey].DriverType.ShouldBe(canonical); } [Fact] @@ -67,6 +100,6 @@ public sealed class DriverProbeRegistrationTests var distinctTypes = probes.Select(p => p.DriverType).Distinct(StringComparer.OrdinalIgnoreCase).Count(); probes.Count.ShouldBe(distinctTypes, "Duplicate IDriverProbe registrations — TryAddEnumerable should de-dup."); - distinctTypes.ShouldBe(AdminUiDriverTypeKeys.Length); + distinctTypes.ShouldBe(DriverTypeNames.All.Count); } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/TwoNodeClusterHarness.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/TwoNodeClusterHarness.cs index 9e2558b5..1d68ce50 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/TwoNodeClusterHarness.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/TwoNodeClusterHarness.cs @@ -423,6 +423,21 @@ public sealed class TwoNodeClusterHarness : IAsyncDisposable ab.WithOtOpcUaClusterBootstrap(sp); ab.WithOtOpcUaControlPlaneSingletons(); ab.WithOtOpcUaRuntimeActors(); + + // Production fidelity, per-cluster mesh Phase 6 (`a2c5d57f`). RedundancyStateActor moved OUT + // of WithOtOpcUaControlPlaneSingletons and into its own cluster-role-scoped registration, + // which Program.cs makes from its DRIVER branch — a driver-only site pair has no admin node + // to elect its Primary. The harness models a fused admin,driver node but was never updated, + // so from that commit until now NO node here spawned the singleton: nothing published + // `redundancy-state`, every node sat at "role unknown" forever, and PrimaryGateFailoverTests + // polled for 45s and timed out. + // + // This is the second time this exact shape has bitten the harness — see the AddOtOpcUaRuntime + // note above, where calling only the Akka-spawn half left IDriverHealthPublisher on its Null + // default. When Program.cs grows an actor registration, this list has to grow with it; the + // harness's whole value is that it is wired the way a real node is. + ab.WithOtOpcUaClusterRedundancySingleton( + sp.GetRequiredService>().Value); }); builder.Services.AddOtOpcUaAuth(builder.Configuration); 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(); + } +}