Phase 6.2 Stream C — Browse gating on DriverNodeManager
Closes task #120 (partial — strict point-check; ancestor-visibility implication is a follow-up). Before this commit DriverNodeManager exposed every materialized node to every browsing session regardless of the user's ACL. Read + Write + HistoryRead were already gated through AuthorizationGate in Phase 6.2 Stream C core; Browse was the one surface where the session could still enumerate nodes it had no permission to touch, discovering structure even when reads failed with BadUserAccessDenied. Implementation - New `Browse` override on DriverNodeManager that calls base.Browse first (lets the stack populate the reference list normally), then post-filters the IList<ReferenceDescription> so denied nodes are removed silently. OPC UA convention: Browse filtering is invisible to the client; no BadUserAccessDenied surfaces. - Extracted the filter loop into the static internal `FilterBrowseReferences(references, userIdentity, gate, scopeResolver)` so the policy is unit-testable without standing up the full OPC UA server stack. - Non-string NodeId identifiers (stack-synthesized standard-type references with numeric identifiers) bypass the gate — only driver- materialized nodes key into the authz trie. - When AuthorizationGate or NodeScopeResolver is null, the filter is a no-op — preserves the pre-Phase-6.2 dispatch path for integration tests that construct DriverNodeManager without authz. Tests — 6 new in BrowseGatingTests.cs (gate-null no-op, empty-list no-op, denied-removed, allowed-passes-through, numeric-id bypass, lax-mode null-identity keeps references). Server.Tests 257 → 263. Known follow-up (tracked implicitly under #120 re-scope): - Ancestor-visibility implication (acl-design.md §Browse line 111): a user with Read at `Line/Tag` should be able to Browse `Line` even without an explicit Browse grant. Current filter does a strict point-check. Proper fix needs TriePermissionEvaluator to expose a "subtree-has-any-grant" query. - TranslateBrowsePathsToNodeIds not yet filtered (same extension pattern; small follow-up). Docs: v2-release-readiness.md Phase 6.2 Stream C hardening list marks the Browse bullet struck-through with "Partial" close-out note. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
159
tests/ZB.MOM.WW.OtOpcUa.Server.Tests/BrowseGatingTests.cs
Normal file
159
tests/ZB.MOM.WW.OtOpcUa.Server.Tests/BrowseGatingTests.cs
Normal file
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// Unit tests for <see cref="DriverNodeManager.FilterBrowseReferences"/> — 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.
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class BrowseGatingTests
|
||||
{
|
||||
[Fact]
|
||||
public void Gate_null_leaves_references_untouched()
|
||||
{
|
||||
var refs = new List<ReferenceDescription>
|
||||
{
|
||||
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<ReferenceDescription>();
|
||||
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<ReferenceDescription>
|
||||
{
|
||||
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<ReferenceDescription>
|
||||
{
|
||||
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<ReferenceDescription>
|
||||
{
|
||||
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<ReferenceDescription> { 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<string> groups)
|
||||
{
|
||||
DisplayName = name;
|
||||
LdapGroups = groups;
|
||||
}
|
||||
public new string DisplayName { get; }
|
||||
public IReadOnlyList<string> LdapGroups { get; }
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user