Phase 6.2 Stream C — CreateMonitoredItems per-item gating
Closes task #121 (partial — creation-time gate; decision #153 per-item revocation stamp is a follow-up). Before this commit a session could subscribe to any node via CreateMonitoredItems, even nodes where Read was denied — the subscription would surface BadUserAccessDenied on each data-change read, but the client saw a successful CreateMonitoredItems response and held the subscription open, wasting resources and leaking the address-space shape through the item metadata. New override on DriverNodeManager.CreateMonitoredItems: - Pre-iterates itemsToCreate, gates each through AuthorizationGate with OpcUaOperation.CreateMonitoredItems at the target node's scope. - For denied slots: sets errors[i] = new ServiceResult( StatusCodes.BadUserAccessDenied). The OPC Foundation base stack honours pre-populated non-success errors and skips item creation for those slots — the subscription never holds a handle to a denied node. - Preserves prior errors (e.g. BadNodeIdUnknown) — first diagnosis wins. - Non-string-identifier references (stack-synthesized numeric ids) bypass the gate. Extracted the pure filter logic into GateMonitoredItemCreateRequests(items, errors, identity, gate, scopeResolver) — static internal, unit-testable without the OPC UA server stack. Tests — 6 new in MonitoredItemGatingTests.cs (gate-null no-op, denied-gets-BadUserAccessDenied, allowed-passes, mixed-batch-denies- per-item, pre-populated-error-preserved, numeric-id-bypass). Server.Tests 263 → 269. Known follow-ups: - Per-item (AuthGenerationId, MembershipVersion) stamp (decision #153) for detecting revocation mid-subscription — needs subscription-layer plumbing. - TransferSubscriptions not yet wired (same pattern, smaller scope). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
@@ -35,7 +35,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.~~ **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).
|
||||
- ~~CreateMonitoredItems + TransferSubscriptions gating with per-item `(AuthGenerationId, MembershipVersion)` stamp so revoked grants surface `BadUserAccessDenied` within one publish cycle (decision #153).~~ **Partial, 2026-04-24.** `DriverNodeManager.CreateMonitoredItems` override pre-gates each request and pre-populates `BadUserAccessDenied` into the errors slot for denied items (the base stack honours pre-set errors and skips those items). Decision #153's per-item `(AuthGenerationId, MembershipVersion)` stamp for detecting mid-subscription revocation is still to ship — needs subscription-layer plumbing. TransferSubscriptions not yet wired (same pattern).
|
||||
- Alarm Acknowledge / Confirm / Shelve gating.
|
||||
- Call (method invocation) gating.
|
||||
- Finer-grained scope resolution — current `NodeScopeResolver` returns a flat cluster-level scope. Joining against the live Configuration DB to populate UnsArea / UnsLine / Equipment path is tracked as Stream C.12.
|
||||
|
||||
@@ -312,6 +312,76 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
|
||||
FilterBrowseReferences(references, context.UserIdentity, _authzGate, _scopeResolver);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Phase 6.2 Stream C — Subscribe/MonitoredItems gating. Pre-populates
|
||||
/// <paramref name="errors"/> slots with <see cref="StatusCodes.BadUserAccessDenied"/>
|
||||
/// for any monitored-item request whose target node the session can't
|
||||
/// <see cref="OpcUaOperation.CreateMonitoredItems"/> on, then delegates to the base
|
||||
/// implementation. The OPC Foundation stack honours pre-populated non-success error
|
||||
/// slots and skips creation for those items.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Decision #153 per-item ACL stamping (so a revoked grant on a running
|
||||
/// subscription surfaces <c>BadUserAccessDenied</c> on the next publish cycle
|
||||
/// rather than continuing to stream data) is a follow-up — it needs the
|
||||
/// subscription layer to plumb <c>(AuthGenerationId, MembershipVersion)</c>
|
||||
/// through per monitored item + re-evaluate on every publish. The current
|
||||
/// filter catches creation-time denials, which is the common case.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
public override void CreateMonitoredItems(
|
||||
OperationContext context,
|
||||
uint subscriptionId,
|
||||
double publishingInterval,
|
||||
TimestampsToReturn timestampsToReturn,
|
||||
IList<MonitoredItemCreateRequest> itemsToCreate,
|
||||
IList<ServiceResult> errors,
|
||||
IList<MonitoringFilterResult> filterResults,
|
||||
IList<IMonitoredItem> monitoredItems,
|
||||
ref long globalIdCounter)
|
||||
{
|
||||
GateMonitoredItemCreateRequests(
|
||||
itemsToCreate, errors, context.UserIdentity, _authzGate, _scopeResolver);
|
||||
|
||||
base.CreateMonitoredItems(
|
||||
context, subscriptionId, publishingInterval, timestampsToReturn,
|
||||
itemsToCreate, errors, filterResults, monitoredItems, ref globalIdCounter);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Pure-function gate for a batch of <see cref="MonitoredItemCreateRequest"/>.
|
||||
/// Sets <paramref name="errors"/>[i] to <see cref="StatusCodes.BadUserAccessDenied"/>
|
||||
/// for every slot whose target node's scope the session isn't allowed to
|
||||
/// <see cref="OpcUaOperation.CreateMonitoredItems"/> on. No-op when
|
||||
/// <paramref name="gate"/> or <paramref name="scopeResolver"/> is null (matches the
|
||||
/// pre-Phase-6.2 no-authz dispatch). Extracted for unit-testability without the
|
||||
/// full OPC UA server stack.
|
||||
/// </summary>
|
||||
internal static void GateMonitoredItemCreateRequests(
|
||||
IList<MonitoredItemCreateRequest> itemsToCreate,
|
||||
IList<ServiceResult> errors,
|
||||
IUserIdentity? userIdentity,
|
||||
AuthorizationGate? gate,
|
||||
NodeScopeResolver? scopeResolver)
|
||||
{
|
||||
if (gate is null || scopeResolver is null) return;
|
||||
if (itemsToCreate.Count == 0) return;
|
||||
|
||||
for (var i = 0; i < itemsToCreate.Count; i++)
|
||||
{
|
||||
// Only slots the caller has't already flagged — preserve earlier per-item
|
||||
// errors (e.g. BadNodeIdUnknown the stack might have filled in).
|
||||
if (errors[i] is not null && ServiceResult.IsBad(errors[i])) continue;
|
||||
|
||||
if (itemsToCreate[i].ItemToMonitor.NodeId.Identifier is not string fullRef) continue;
|
||||
|
||||
var scope = scopeResolver.Resolve(fullRef);
|
||||
if (!gate.IsAllowed(userIdentity, OpcUaOperation.CreateMonitoredItems, scope))
|
||||
errors[i] = new ServiceResult(StatusCodes.BadUserAccessDenied);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Pure-function filter over a <see cref="ReferenceDescription"/> list. Extracted so
|
||||
/// the Browse-gate policy is unit-testable without standing up the OPC UA server
|
||||
|
||||
146
tests/ZB.MOM.WW.OtOpcUa.Server.Tests/MonitoredItemGatingTests.cs
Normal file
146
tests/ZB.MOM.WW.OtOpcUa.Server.Tests/MonitoredItemGatingTests.cs
Normal file
@@ -0,0 +1,146 @@
|
||||
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.GateMonitoredItemCreateRequests"/> —
|
||||
/// Phase 6.2 Stream C per-item subscription gating. Pre-populates the errors array
|
||||
/// with <see cref="StatusCodes.BadUserAccessDenied"/> for denied items; base stack
|
||||
/// honours the pre-set error and skips the item.
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class MonitoredItemGatingTests
|
||||
{
|
||||
[Fact]
|
||||
public void Gate_null_leaves_errors_untouched()
|
||||
{
|
||||
var items = new List<MonitoredItemCreateRequest> { NewRequest("c1/area/line/eq/tag1") };
|
||||
var errors = new List<ServiceResult> { (ServiceResult)null! };
|
||||
|
||||
DriverNodeManager.GateMonitoredItemCreateRequests(items, errors, new UserIdentity(), gate: null, scopeResolver: null);
|
||||
|
||||
errors[0].ShouldBeNull();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Denied_item_gets_BadUserAccessDenied()
|
||||
{
|
||||
var items = new List<MonitoredItemCreateRequest> { NewRequest("c1/area/line/eq/tag1") };
|
||||
var errors = new List<ServiceResult> { (ServiceResult)null! };
|
||||
var gate = MakeGate(strict: true, rows: []); // no grants → deny
|
||||
|
||||
DriverNodeManager.GateMonitoredItemCreateRequests(items, errors, NewIdentity("alice"), gate, new NodeScopeResolver("c1"));
|
||||
|
||||
ServiceResult.IsBad(errors[0]).ShouldBeTrue();
|
||||
errors[0].StatusCode.ShouldBe((StatusCode)StatusCodes.BadUserAccessDenied);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Allowed_item_is_not_touched()
|
||||
{
|
||||
var items = new List<MonitoredItemCreateRequest> { NewRequest("c1/area/line/eq/tag1") };
|
||||
var errors = new List<ServiceResult> { (ServiceResult)null! };
|
||||
var gate = MakeGate(strict: true, rows: [Row("grp-ops", NodePermissions.Subscribe)]);
|
||||
|
||||
DriverNodeManager.GateMonitoredItemCreateRequests(items, errors, NewIdentity("alice", "grp-ops"), gate, new NodeScopeResolver("c1"));
|
||||
|
||||
errors[0].ShouldBeNull();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Mixed_batch_denies_per_item()
|
||||
{
|
||||
var items = new List<MonitoredItemCreateRequest>
|
||||
{
|
||||
NewRequest("c1/area/line/eq/tagA"),
|
||||
NewRequest("c1/area/line/eq/tagB"),
|
||||
};
|
||||
var errors = new List<ServiceResult> { (ServiceResult)null!, (ServiceResult)null! };
|
||||
// Grant Browse not CreateMonitoredItems → still denied for this op
|
||||
var gate = MakeGate(strict: true, rows: [Row("grp-ops", NodePermissions.Browse)]);
|
||||
|
||||
DriverNodeManager.GateMonitoredItemCreateRequests(items, errors, NewIdentity("alice", "grp-ops"), gate, new NodeScopeResolver("c1"));
|
||||
|
||||
ServiceResult.IsBad(errors[0]).ShouldBeTrue();
|
||||
ServiceResult.IsBad(errors[1]).ShouldBeTrue();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Pre_populated_error_is_preserved()
|
||||
{
|
||||
// Base stack may have already flagged an item (e.g. BadNodeIdUnknown). The gate
|
||||
// must not overwrite that with a generic BadUserAccessDenied — the first diagnosis
|
||||
// wins.
|
||||
var items = new List<MonitoredItemCreateRequest> { NewRequest("c1/area/line/eq/tag1") };
|
||||
var errors = new List<ServiceResult> { new(StatusCodes.BadNodeIdUnknown) };
|
||||
var gate = MakeGate(strict: true, rows: []);
|
||||
|
||||
DriverNodeManager.GateMonitoredItemCreateRequests(items, errors, NewIdentity("alice"), gate, new NodeScopeResolver("c1"));
|
||||
|
||||
errors[0].StatusCode.ShouldBe((StatusCode)StatusCodes.BadNodeIdUnknown);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Non_string_identifier_bypasses_the_gate()
|
||||
{
|
||||
// Numeric-id references (standard-type nodes) aren't keyed into the authz trie.
|
||||
var items = new List<MonitoredItemCreateRequest>
|
||||
{
|
||||
new() { ItemToMonitor = new ReadValueId { NodeId = new NodeId(62u) } },
|
||||
};
|
||||
var errors = new List<ServiceResult> { (ServiceResult)null! };
|
||||
var gate = MakeGate(strict: true, rows: []);
|
||||
|
||||
DriverNodeManager.GateMonitoredItemCreateRequests(items, errors, NewIdentity("alice"), gate, new NodeScopeResolver("c1"));
|
||||
|
||||
errors[0].ShouldBeNull("numeric-id references bypass the gate");
|
||||
}
|
||||
|
||||
// ---- helpers -----------------------------------------------------------
|
||||
|
||||
private static MonitoredItemCreateRequest NewRequest(string fullRef) => new()
|
||||
{
|
||||
ItemToMonitor = new ReadValueId { NodeId = new NodeId(fullRef, 2) },
|
||||
};
|
||||
|
||||
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