From f8d5b0fdbb5b701be75d1d8cc8b2a076305d4f92 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 19 Apr 2026 11:02:17 -0400 Subject: [PATCH] =?UTF-8?q?Phase=206.2=20Stream=20C=20follow-up=20?= =?UTF-8?q?=E2=80=94=20wire=20AuthorizationGate=20into=20DriverNodeManager?= =?UTF-8?q?=20Read=20/=20Write=20/=20HistoryRead=20dispatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the Phase 6.2 security gap the v2 release-readiness dashboard flagged: the evaluator + trie + gate shipped as code in PRs #84-88 but no dispatch path called them. This PR threads the gate end-to-end from OpcUaApplicationHost → OtOpcUaServer → DriverNodeManager and calls it on every Read / Write / 4 HistoryRead paths. Server.Security additions: - NodeScopeResolver — maps driver fullRef → Core.Authorization NodeScope. Phase 1 shape: populates ClusterId + TagId; leaves NamespaceId / UnsArea / UnsLine / Equipment null. The cluster-level ACL cascade covers this configuration (decision #129 additive grants). Finer-grained scope resolution (joining against the live Configuration DB for UnsArea / UnsLine path) lands as Stream C.12 follow-up. - WriteAuthzPolicy.ToOpcUaOperation — maps SecurityClassification → the OpcUaOperation the gate evaluator consults (Operate/SecuredWrite → WriteOperate; Tune → WriteTune; Configure/VerifiedWrite → WriteConfigure). DriverNodeManager wiring: - Ctor gains optional AuthorizationGate + NodeScopeResolver; both null means the pre-Phase-6.2 dispatch runs unchanged (backwards-compat for every integration test that constructs DriverNodeManager directly). - OnReadValue: ahead of the invoker call, builds NodeScope + calls gate.IsAllowed(identity, Read, scope). Denied reads return BadUserAccessDenied without hitting the driver. - OnWriteValue: preserves the existing WriteAuthzPolicy check (classification vs session roles) + adds an additive gate check using WriteAuthzPolicy.ToOpcUaOperation(classification) to pick the right WriteOperate/Tune/Configure surface. Lax mode falls through for identities without LDAP groups. - Four HistoryRead paths (Raw / Processed / AtTime / Events): gate check runs per-node before the invoker. Events path tolerates fullRef=null (event-history queries can target a notifier / driver-root; those are cluster-wide reads that need a different scope shape — deferred). - New WriteAccessDenied helper surfaces BadUserAccessDenied in the OpcHistoryReadResult slot + errors list, matching the shape of the existing WriteUnsupported / WriteInternalError helpers. OtOpcUaServer + OpcUaApplicationHost: gate + resolver thread through as optional constructor parameters (same pattern as DriverResiliencePipelineBuilder in Phase 6.1). Null defaults keep the existing 3 OpcUaApplicationHost integration tests constructing without them unchanged. Tests (5 new in NodeScopeResolverTests): - Resolve populates ClusterId + TagId + Equipment Kind. - Resolve leaves finer path null per Phase 1 shape (doc'd as follow-up). - Empty fullReference throws. - Empty clusterId throws at ctor. - Resolver is stateless across calls. The existing 9 AuthorizationGate tests (shipped in PR #86) continue to cover the gate's allow/deny semantics under strict + lax mode. Full solution dotnet test: 1164 passing (was 1159, +5). Pre-existing Client.CLI Subscribe flake unchanged. Existing OpcUaApplicationHost + HealthEndpointsHost + driver integration tests continue to pass because the gate defaults to null → no enforcement, and the lax-mode fallback returns true for identities without LDAP groups (the anonymous test path). Production deployments flip the gate on by constructing it via OpcUaApplicationHost's new authzGate parameter + setting `Authorization:StrictMode = true` once ACL data is populated. Flipping the switch post-seed turns the evaluator + trie from scaffolded code into actual enforcement. This closes release blocker #1 listed in docs/v2/v2-release-readiness.md. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../OpcUa/DriverNodeManager.cs | 93 ++++++++++++++++++- .../OpcUa/OpcUaApplicationHost.cs | 11 ++- .../OpcUa/OtOpcUaServer.cs | 11 ++- .../Security/NodeScopeResolver.cs | 47 ++++++++++ .../Security/WriteAuthzPolicy.cs | 18 ++++ .../NodeScopeResolverTests.cs | 64 +++++++++++++ 6 files changed, 239 insertions(+), 5 deletions(-) create mode 100644 src/ZB.MOM.WW.OtOpcUa.Server/Security/NodeScopeResolver.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Server.Tests/NodeScopeResolverTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs index 8aa2d32..7dd64ec 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.Logging; using Opc.Ua; using Opc.Ua.Server; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; using ZB.MOM.WW.OtOpcUa.Core.Resilience; using ZB.MOM.WW.OtOpcUa.Server.Security; using DriverWriteRequest = ZB.MOM.WW.OtOpcUa.Core.Abstractions.WriteRequest; @@ -59,14 +60,24 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder // returns a child builder per Folder call and the caller threads nesting through those references. private FolderState _currentFolder = null!; + // Phase 6.2 Stream C follow-up — optional gate + scope resolver. When both are null + // the old pre-Phase-6.2 dispatch path runs unchanged (backwards compat for every + // integration test that constructs DriverNodeManager without the gate). When wired, + // OnReadValue / OnWriteValue / HistoryRead all consult the gate before the invoker call. + private readonly AuthorizationGate? _authzGate; + private readonly NodeScopeResolver? _scopeResolver; + public DriverNodeManager(IServerInternal server, ApplicationConfiguration configuration, - IDriver driver, CapabilityInvoker invoker, ILogger logger) + IDriver driver, CapabilityInvoker invoker, ILogger logger, + AuthorizationGate? authzGate = null, NodeScopeResolver? scopeResolver = null) : base(server, configuration, namespaceUris: $"urn:OtOpcUa:{driver.DriverInstanceId}") { _driver = driver; _readable = driver as IReadable; _writable = driver as IWritable; _invoker = invoker; + _authzGate = authzGate; + _scopeResolver = scopeResolver; _logger = logger; } @@ -197,6 +208,20 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder try { var fullRef = node.NodeId.Identifier as string ?? ""; + + // Phase 6.2 Stream C — authorization gate. Runs ahead of the invoker so a denied + // read never hits the driver. Returns true in lax mode when identity lacks LDAP + // groups; strict mode denies those cases. See AuthorizationGate remarks. + if (_authzGate is not null && _scopeResolver is not null) + { + var scope = _scopeResolver.Resolve(fullRef); + if (!_authzGate.IsAllowed(context.UserIdentity, OpcUaOperation.Read, scope)) + { + statusCode = StatusCodes.BadUserAccessDenied; + return ServiceResult.Good; + } + } + var result = _invoker.ExecuteAsync( DriverCapability.Read, _driver.DriverInstanceId, @@ -390,6 +415,23 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder fullRef, classification, string.Join(",", roles)); return new ServiceResult(StatusCodes.BadUserAccessDenied); } + + // Phase 6.2 Stream C — additive gate check. The classification/role check above + // is the pre-Phase-6.2 baseline; the gate adds per-tag ACL enforcement on top. In + // lax mode (default during rollout) the gate falls through when the identity + // lacks LDAP groups, so existing integration tests keep passing. + if (_authzGate is not null && _scopeResolver is not null) + { + var scope = _scopeResolver.Resolve(fullRef!); + var writeOp = WriteAuthzPolicy.ToOpcUaOperation(classification); + if (!_authzGate.IsAllowed(context.UserIdentity, writeOp, scope)) + { + _logger.LogInformation( + "Write denied by ACL gate for {FullRef}: operation={Op} classification={Classification}", + fullRef, writeOp, classification); + return new ServiceResult(StatusCodes.BadUserAccessDenied); + } + } } try @@ -482,6 +524,16 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder continue; } + if (_authzGate is not null && _scopeResolver is not null) + { + var historyScope = _scopeResolver.Resolve(fullRef); + if (!_authzGate.IsAllowed(context.UserIdentity, OpcUaOperation.HistoryRead, historyScope)) + { + WriteAccessDenied(results, errors, i); + continue; + } + } + try { var driverResult = _invoker.ExecuteAsync( @@ -546,6 +598,16 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder continue; } + if (_authzGate is not null && _scopeResolver is not null) + { + var historyScope = _scopeResolver.Resolve(fullRef); + if (!_authzGate.IsAllowed(context.UserIdentity, OpcUaOperation.HistoryRead, historyScope)) + { + WriteAccessDenied(results, errors, i); + continue; + } + } + try { var driverResult = _invoker.ExecuteAsync( @@ -603,6 +665,16 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder continue; } + if (_authzGate is not null && _scopeResolver is not null) + { + var historyScope = _scopeResolver.Resolve(fullRef); + if (!_authzGate.IsAllowed(context.UserIdentity, OpcUaOperation.HistoryRead, historyScope)) + { + WriteAccessDenied(results, errors, i); + continue; + } + } + try { var driverResult = _invoker.ExecuteAsync( @@ -660,6 +732,19 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder // "all sources in the driver's namespace" per the IHistoryProvider contract. var fullRef = ResolveFullRef(handle); + // fullRef is null for event-history queries that target a notifier (driver root). + // Those are cluster-wide reads + need a different scope shape; skip the gate here + // and let the driver-level authz handle them. Non-null path gets per-node gating. + if (fullRef is not null && _authzGate is not null && _scopeResolver is not null) + { + var historyScope = _scopeResolver.Resolve(fullRef); + if (!_authzGate.IsAllowed(context.UserIdentity, OpcUaOperation.HistoryRead, historyScope)) + { + WriteAccessDenied(results, errors, i); + continue; + } + } + try { var driverResult = _invoker.ExecuteAsync( @@ -721,6 +806,12 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder errors[i] = StatusCodes.BadInternalError; } + private static void WriteAccessDenied(IList results, IList errors, int i) + { + results[i] = new OpcHistoryReadResult { StatusCode = StatusCodes.BadUserAccessDenied }; + errors[i] = StatusCodes.BadUserAccessDenied; + } + private static void WriteNodeIdUnknown(IList results, IList errors, int i) { WriteNodeIdUnknown(results, errors, i); diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs index b692bb7..f962f80 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OpcUaApplicationHost.cs @@ -23,6 +23,8 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable private readonly DriverHost _driverHost; private readonly IUserAuthenticator _authenticator; private readonly DriverResiliencePipelineBuilder _pipelineBuilder; + private readonly AuthorizationGate? _authzGate; + private readonly NodeScopeResolver? _scopeResolver; private readonly ILoggerFactory _loggerFactory; private readonly ILogger _logger; private ApplicationInstance? _application; @@ -32,12 +34,16 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable public OpcUaApplicationHost(OpcUaServerOptions options, DriverHost driverHost, IUserAuthenticator authenticator, ILoggerFactory loggerFactory, ILogger logger, - DriverResiliencePipelineBuilder? pipelineBuilder = null) + DriverResiliencePipelineBuilder? pipelineBuilder = null, + AuthorizationGate? authzGate = null, + NodeScopeResolver? scopeResolver = null) { _options = options; _driverHost = driverHost; _authenticator = authenticator; _pipelineBuilder = pipelineBuilder ?? new DriverResiliencePipelineBuilder(); + _authzGate = authzGate; + _scopeResolver = scopeResolver; _loggerFactory = loggerFactory; _logger = logger; } @@ -64,7 +70,8 @@ public sealed class OpcUaApplicationHost : IAsyncDisposable throw new InvalidOperationException( $"OPC UA application certificate could not be validated or created in {_options.PkiStoreRoot}"); - _server = new OtOpcUaServer(_driverHost, _authenticator, _pipelineBuilder, _loggerFactory); + _server = new OtOpcUaServer(_driverHost, _authenticator, _pipelineBuilder, _loggerFactory, + authzGate: _authzGate, scopeResolver: _scopeResolver); await _application.Start(_server).ConfigureAwait(false); _logger.LogInformation("OPC UA server started — endpoint={Endpoint} driverCount={Count}", diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs index bc52665..f0cbc04 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/OtOpcUaServer.cs @@ -21,6 +21,8 @@ public sealed class OtOpcUaServer : StandardServer private readonly DriverHost _driverHost; private readonly IUserAuthenticator _authenticator; private readonly DriverResiliencePipelineBuilder _pipelineBuilder; + private readonly AuthorizationGate? _authzGate; + private readonly NodeScopeResolver? _scopeResolver; private readonly ILoggerFactory _loggerFactory; private readonly List _driverNodeManagers = new(); @@ -28,11 +30,15 @@ public sealed class OtOpcUaServer : StandardServer DriverHost driverHost, IUserAuthenticator authenticator, DriverResiliencePipelineBuilder pipelineBuilder, - ILoggerFactory loggerFactory) + ILoggerFactory loggerFactory, + AuthorizationGate? authzGate = null, + NodeScopeResolver? scopeResolver = null) { _driverHost = driverHost; _authenticator = authenticator; _pipelineBuilder = pipelineBuilder; + _authzGate = authzGate; + _scopeResolver = scopeResolver; _loggerFactory = loggerFactory; } @@ -58,7 +64,8 @@ public sealed class OtOpcUaServer : StandardServer // DriverInstance row in a follow-up PR; for now every driver gets Tier A defaults. var options = new DriverResilienceOptions { Tier = DriverTier.A }; var invoker = new CapabilityInvoker(_pipelineBuilder, driver.DriverInstanceId, () => options, driver.DriverType); - var manager = new DriverNodeManager(server, configuration, driver, invoker, logger); + var manager = new DriverNodeManager(server, configuration, driver, invoker, logger, + authzGate: _authzGate, scopeResolver: _scopeResolver); _driverNodeManagers.Add(manager); } diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/Security/NodeScopeResolver.cs b/src/ZB.MOM.WW.OtOpcUa.Server/Security/NodeScopeResolver.cs new file mode 100644 index 0000000..812f716 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Server/Security/NodeScopeResolver.cs @@ -0,0 +1,47 @@ +using ZB.MOM.WW.OtOpcUa.Core.Authorization; + +namespace ZB.MOM.WW.OtOpcUa.Server.Security; + +/// +/// Maps a driver-side full reference (e.g. "TestMachine_001/Oven/SetPoint") to the +/// the Phase 6.2 evaluator walks. Today a simplified resolver that +/// returns a cluster-scoped + tag-only scope — the deeper UnsArea / UnsLine / Equipment +/// path lookup from the live Configuration DB is a Stream C.12 follow-up. +/// +/// +/// The flat cluster-level scope is sufficient for v2 GA because Phase 6.2 ACL grants +/// at the Cluster scope cascade to every tag below (decision #129 — additive grants). The +/// finer hierarchy only matters when operators want per-area or per-equipment grants; +/// those still work for Cluster-level grants, and landing the finer resolution in a +/// follow-up doesn't regress the base security model. +/// +/// Thread-safety: the resolver is stateless once constructed. Callers may cache a +/// single instance per DriverNodeManager without locks. +/// +public sealed class NodeScopeResolver +{ + private readonly string _clusterId; + + public NodeScopeResolver(string clusterId) + { + ArgumentException.ThrowIfNullOrWhiteSpace(clusterId); + _clusterId = clusterId; + } + + /// + /// Resolve a node scope for the given driver-side . + /// Phase 1 shape: returns ClusterId + TagId = fullReference only; + /// NamespaceId / UnsArea / UnsLine / Equipment stay null. A future resolver will + /// join against the Configuration DB to populate the full path. + /// + public NodeScope Resolve(string fullReference) + { + ArgumentException.ThrowIfNullOrWhiteSpace(fullReference); + return new NodeScope + { + ClusterId = _clusterId, + TagId = fullReference, + Kind = NodeHierarchyKind.Equipment, + }; + } +} diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/Security/WriteAuthzPolicy.cs b/src/ZB.MOM.WW.OtOpcUa.Server/Security/WriteAuthzPolicy.cs index f7447c9..4a46ce6 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/Security/WriteAuthzPolicy.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/Security/WriteAuthzPolicy.cs @@ -67,4 +67,22 @@ public static class WriteAuthzPolicy SecurityClassification.ViewOnly => null, // IsAllowed short-circuits _ => null, }; + + /// + /// Maps a driver-reported to the + /// the Phase 6.2 evaluator consults + /// for the matching bit. + /// FreeAccess + ViewOnly fall back to WriteOperate — the evaluator never sees them + /// because short-circuits first. + /// + public static Core.Abstractions.OpcUaOperation ToOpcUaOperation(SecurityClassification classification) => + classification switch + { + SecurityClassification.Operate => Core.Abstractions.OpcUaOperation.WriteOperate, + SecurityClassification.SecuredWrite => Core.Abstractions.OpcUaOperation.WriteOperate, + SecurityClassification.Tune => Core.Abstractions.OpcUaOperation.WriteTune, + SecurityClassification.VerifiedWrite => Core.Abstractions.OpcUaOperation.WriteConfigure, + SecurityClassification.Configure => Core.Abstractions.OpcUaOperation.WriteConfigure, + _ => Core.Abstractions.OpcUaOperation.WriteOperate, + }; } diff --git a/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/NodeScopeResolverTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/NodeScopeResolverTests.cs new file mode 100644 index 0000000..b5bbbec --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Server.Tests/NodeScopeResolverTests.cs @@ -0,0 +1,64 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Authorization; +using ZB.MOM.WW.OtOpcUa.Server.Security; + +namespace ZB.MOM.WW.OtOpcUa.Server.Tests; + +[Trait("Category", "Unit")] +public sealed class NodeScopeResolverTests +{ + [Fact] + public void Resolve_PopulatesClusterAndTag() + { + var resolver = new NodeScopeResolver("c-warsaw"); + + var scope = resolver.Resolve("TestMachine_001/Oven/SetPoint"); + + scope.ClusterId.ShouldBe("c-warsaw"); + scope.TagId.ShouldBe("TestMachine_001/Oven/SetPoint"); + scope.Kind.ShouldBe(NodeHierarchyKind.Equipment); + } + + [Fact] + public void Resolve_Leaves_UnsPath_Null_For_Phase1() + { + var resolver = new NodeScopeResolver("c-1"); + + var scope = resolver.Resolve("tag-1"); + + // Phase 1 flat scope — finer resolution tracked as Stream C.12 follow-up. + scope.NamespaceId.ShouldBeNull(); + scope.UnsAreaId.ShouldBeNull(); + scope.UnsLineId.ShouldBeNull(); + scope.EquipmentId.ShouldBeNull(); + } + + [Fact] + public void Resolve_Throws_OnEmptyFullReference() + { + var resolver = new NodeScopeResolver("c-1"); + + Should.Throw(() => resolver.Resolve("")); + Should.Throw(() => resolver.Resolve(" ")); + } + + [Fact] + public void Ctor_Throws_OnEmptyClusterId() + { + Should.Throw(() => new NodeScopeResolver("")); + } + + [Fact] + public void Resolver_IsStateless_AcrossCalls() + { + var resolver = new NodeScopeResolver("c"); + var s1 = resolver.Resolve("tag-a"); + var s2 = resolver.Resolve("tag-b"); + + s1.TagId.ShouldBe("tag-a"); + s2.TagId.ShouldBe("tag-b"); + s1.ClusterId.ShouldBe("c"); + s2.ClusterId.ShouldBe("c"); + } +}