diff --git a/docs/v2/v2-release-readiness.md b/docs/v2/v2-release-readiness.md index c950d5e..93588cb 100644 --- a/docs/v2/v2-release-readiness.md +++ b/docs/v2/v2-release-readiness.md @@ -34,7 +34,7 @@ All code-path release blockers are closed. The remaining items are live-hardware Remaining Stream C surfaces (hardening, not release-blocking): -- Browse + TranslateBrowsePathsToNodeIds gating with ancestor-visibility logic per `acl-design.md` §Browse. +- ~~Browse + TranslateBrowsePathsToNodeIds gating with ancestor-visibility logic per `acl-design.md` §Browse.~~ **Partial, 2026-04-24.** `DriverNodeManager.Browse` override post-filters the `ReferenceDescription` list via a new `FilterBrowseReferences` helper — denied nodes disappear silently per OPC UA convention. Ancestor-visibility implication (Read-grant at `Line/Tag` implying Browse on `Line`) still to ship; needs a subtree-has-any-grant query on the trie evaluator. `TranslateBrowsePathsToNodeIds` surface not yet wired. - CreateMonitoredItems + TransferSubscriptions gating with per-item `(AuthGenerationId, MembershipVersion)` stamp so revoked grants surface `BadUserAccessDenied` within one publish cycle (decision #153). - Alarm Acknowledge / Confirm / Shelve gating. - Call (method invocation) gating. diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs index b99ad75..2271e4d 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs @@ -276,6 +276,73 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder return ServiceResult.Good; } + /// + /// Phase 6.2 Stream C — Browse gating. Post-filters the reference list the base + /// produced so nodes the session isn't allowed to + /// see disappear from the browse result silently (OPC UA convention: deny = omit, + /// not an error). + /// + /// + /// + /// Each surviving reference is a ; we map its + /// back to the driver-side fullRef the + /// node manager uses as its identifier, resolve a via + /// , and ask + /// whether is allowed for that scope. + /// + /// + /// References with non-string NodeId identifiers (e.g. stack-synthesized numeric + /// standard-type references) bypass the gate — only driver-materialized nodes + /// key into _variablesByFullRef and carry an authz policy. + /// + /// + /// Ancestor-visibility implication (a user with Read at Line/Tag should + /// be able to browse Line even without an explicit Browse grant there) is + /// a follow-up that needs the TriePermissionEvaluator to expose a + /// "subtree-has-any-grant" query. For now this filter does a strict point check; + /// admins grant Browse at the right levels in practice. + /// + /// + public override void Browse( + OperationContext context, + ref ContinuationPoint continuationPoint, + IList references) + { + base.Browse(context, ref continuationPoint, references); + FilterBrowseReferences(references, context.UserIdentity, _authzGate, _scopeResolver); + } + + /// + /// Pure-function filter over a list. Extracted so + /// the Browse-gate policy is unit-testable without standing up the OPC UA server + /// stack. When either the gate or the resolver is null, the list is left + /// untouched — matches the pre-Phase-6.2 no-authz path. + /// + /// + /// References whose isn't a string (stack-synthesized + /// standard-type references, numeric identifiers, etc.) bypass the gate — only + /// driver-materialized nodes key into the authz trie. + /// + internal static void FilterBrowseReferences( + IList references, + IUserIdentity? userIdentity, + AuthorizationGate? gate, + NodeScopeResolver? scopeResolver) + { + if (gate is null || scopeResolver is null) return; + if (references.Count == 0) return; + + // Remove by index from the back so indices stay valid as we shrink the list. + for (var i = references.Count - 1; i >= 0; i--) + { + if (references[i].NodeId.Identifier is not string fullRef) continue; + + var scope = scopeResolver.Resolve(fullRef); + if (!gate.IsAllowed(userIdentity, OpcUaOperation.Browse, scope)) + references.RemoveAt(i); + } + } + /// /// Picks the the dispatch layer routes through based on the /// node's Phase 7 source kind (ADR-002). Extracted as a pure function for unit test diff --git a/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/BrowseGatingTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/BrowseGatingTests.cs new file mode 100644 index 0000000..22ca6bc --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/BrowseGatingTests.cs @@ -0,0 +1,159 @@ +using Opc.Ua; +using Shouldly; +using Xunit; +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.Server.OpcUa; +using ZB.MOM.WW.OtOpcUa.Server.Security; + +namespace ZB.MOM.WW.OtOpcUa.Server.Tests; + +/// +/// Unit tests for — Phase 6.2 +/// Stream C Browse gating. Verifies that references to nodes the session isn't +/// allowed to browse are removed silently, while allowed references pass through. +/// +[Trait("Category", "Unit")] +public sealed class BrowseGatingTests +{ + [Fact] + public void Gate_null_leaves_references_untouched() + { + var refs = new List + { + NewRef("c1/area/line/eq/tag1"), + NewRef("c1/area/line/eq/tag2"), + }; + + DriverNodeManager.FilterBrowseReferences(refs, new UserIdentity(), gate: null, scopeResolver: null); + + refs.Count.ShouldBe(2); + } + + [Fact] + public void Empty_reference_list_is_a_no_op() + { + var refs = new List(); + var gate = MakeGate(strict: true, rows: []); + var resolver = new NodeScopeResolver("c1"); + + DriverNodeManager.FilterBrowseReferences(refs, new UserIdentity(), gate, resolver); + + refs.Count.ShouldBe(0); + } + + [Fact] + public void Denied_references_are_removed() + { + var refs = new List + { + NewRef("c1/area/line/eq/tag1"), + NewRef("c1/area/line/eq/tag2"), + }; + + // Strict mode with no ACL rows → everyone is denied. + var gate = MakeGate(strict: true, rows: []); + var resolver = new NodeScopeResolver("c1"); + + DriverNodeManager.FilterBrowseReferences(refs, NewIdentity("alice", "grp-ops"), gate, resolver); + + refs.Count.ShouldBe(0); + } + + [Fact] + public void Allowed_references_remain() + { + var refs = new List + { + NewRef("c1/area/line/eq/tag1"), + NewRef("c1/area/line/eq/tag2"), + }; + + var gate = MakeGate(strict: true, rows: + [ + Row("grp-ops", NodePermissions.Browse), + ]); + var resolver = new NodeScopeResolver("c1"); + + DriverNodeManager.FilterBrowseReferences(refs, NewIdentity("alice", "grp-ops"), gate, resolver); + + refs.Count.ShouldBe(2); + } + + [Fact] + public void Non_string_identifiers_bypass_the_gate() + { + // A numeric-identifier reference (stack-synthesized standard type) must not be + // filtered — only driver-materialized (string-id) nodes are subject to the authz trie. + var refs = new List + { + new() { NodeId = new NodeId(62u) }, // VariableTypeIds.BaseVariableType or similar + NewRef("c1/area/line/eq/tag1"), + }; + + // Strict + no grants → would deny everything, but the numeric ref bypasses. + var gate = MakeGate(strict: true, rows: []); + var resolver = new NodeScopeResolver("c1"); + + DriverNodeManager.FilterBrowseReferences(refs, NewIdentity("alice", "grp-ops"), gate, resolver); + + refs.Count.ShouldBe(1); + refs[0].NodeId.Identifier.ShouldBe(62u); + } + + [Fact] + public void Lax_mode_null_identity_keeps_references() + { + var refs = new List { NewRef("c1/area/line/eq/tag1") }; + var gate = MakeGate(strict: false, rows: []); + var resolver = new NodeScopeResolver("c1"); + + DriverNodeManager.FilterBrowseReferences(refs, userIdentity: null, gate, resolver); + + refs.Count.ShouldBe(1, "lax mode keeps the pre-Phase-6.2 behaviour — everything visible"); + } + + // ---- helpers ----------------------------------------------------------- + + private static ReferenceDescription NewRef(string fullRef) => new() + { + NodeId = new NodeId(fullRef, 2), + BrowseName = new QualifiedName("browse"), + DisplayName = new LocalizedText("display"), + }; + + private static NodeAcl Row(string group, NodePermissions flags) => new() + { + NodeAclRowId = Guid.NewGuid(), + NodeAclId = Guid.NewGuid().ToString(), + GenerationId = 1, + ClusterId = "c1", + LdapGroup = group, + ScopeKind = NodeAclScopeKind.Cluster, + ScopeId = null, + PermissionFlags = flags, + }; + + private static AuthorizationGate MakeGate(bool strict, NodeAcl[] rows) + { + var cache = new PermissionTrieCache(); + cache.Install(PermissionTrieBuilder.Build("c1", 1, rows)); + var evaluator = new TriePermissionEvaluator(cache); + return new AuthorizationGate(evaluator, strictMode: strict); + } + + private static IUserIdentity NewIdentity(string name, params string[] groups) => new FakeIdentity(name, groups); + + private sealed class FakeIdentity : UserIdentity, ILdapGroupsBearer + { + public FakeIdentity(string name, IReadOnlyList groups) + { + DisplayName = name; + LdapGroups = groups; + } + public new string DisplayName { get; } + public IReadOnlyList LdapGroups { get; } + } +}