diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index ecbdc79c..ed397b0a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -2877,9 +2877,15 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 IEventMonitoredItem monitoredItem, bool unsubscribe) { + // ⚠️ sourceId is the MANAGER HANDLE, not a NodeState — the same `object`-typed trap as + // ContinuationPoint.NodeToBrowse. Casting it to NodeState compiles, always yields null, and + // silently disables this gate; IsHandleInNamespace is the SDK's own resolver and cannot be + // wrong about it. + // // Only creation is gated. Refusing an UNSUBSCRIBE would strand the item on a node the session can // no longer reach — the client could never detach it, and the server would keep delivering. - if (AclEnforced && !unsubscribe && sourceId is NodeState source + if (AclEnforced && !unsubscribe + && IsHandleInNamespace(sourceId) is { } source && !Permits(IdentityOf(context), OpcUaOperation.CreateMonitoredItems, source.NodeId)) { return StatusCodes.BadUserAccessDenied; diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs index 9c4256b2..595c677f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs @@ -229,6 +229,45 @@ public sealed class NodeManagerStage6EnforcementTests : IDisposable } + /// + /// Event subscriptions are gated, and the gate resolves its source through the SDK's own + /// IsHandleInNamespace rather than a cast. + /// + /// + /// + /// ⚠️ This exists because the first version of that gate read sourceId as NodeState. + /// sourceId is the manager handle, so the cast compiled, always yielded null, and + /// silently disabled the gate — the same object-typed trap as + /// ContinuationPoint.NodeToBrowse. Nothing about the code looked wrong. + /// + /// + /// The monitored item is deliberately : a denial must be reached without + /// the gate touching the item at all. That also makes the test a discriminator rather than a + /// restatement — against the broken cast the gate falls through to the base implementation, + /// which does dereference the item, so the test fails loudly instead of passing. + /// + /// + [Fact] + public async Task Event_subscriptions_are_gated_on_the_notifier() + { + var (host, server) = await BootAsync(); + try + { + var nm = server.NodeManager!; + nm.Authorizer = Authorizer(enabled: true, GrantOn(NodePermissions.Read)); + var nodeId = Variable(nm, "Plant/Modbus/dev1/Speed"); + + var context = new OperationContext( + new RequestHeader(), secureChannelContext: null, RequestType.CreateMonitoredItems, Identity()); + + var result = nm.SubscribeToEvents( + context, nm.GetManagerHandle(nodeId), subscriptionId: 1, monitoredItem: null!, unsubscribe: false); + + result.StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); + } + finally { await host.DisposeAsync(); } + } + // ───────────────────────────── helpers ───────────────────────────── private static NodeId Variable(OtOpcUaNodeManager nm, string rawPath)