From c236263e8dde484fd4b9b0401576ac7850a3eb58 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 15 Jun 2026 14:09:35 -0400 Subject: [PATCH] fix(authz): give HistoryUpdate its own NodePermissions bit (was aliased to HistoryRead) [H2] --- .../Enums/NodePermissions.cs | 5 +++ .../Authorization/TriePermissionEvaluator.cs | 2 +- .../TriePermissionEvaluatorTests.cs | 32 +++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodePermissions.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodePermissions.cs index 44a30628..87c1937d 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodePermissions.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Enums/NodePermissions.cs @@ -29,6 +29,11 @@ public enum NodePermissions : int // OPC UA Part 4 §5.11 MethodCall = 1 << 11, + // OPC UA HistoryUpdate (annotation / insert / delete) — separate from HistoryRead so a + // read-only grant cannot authorize historian writes. Not included in any composite bundle + // until the HistoryUpdate service surface is implemented. + HistoryUpdate = 1 << 12, + // Bundles (one-click grants in Admin UI) ReadOnly = Browse | Read | Subscribe | HistoryRead | AlarmRead, Operator = ReadOnly | WriteOperate | AlarmAcknowledge | AlarmConfirm, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/TriePermissionEvaluator.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/TriePermissionEvaluator.cs index a92608cc..a5881c3a 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/TriePermissionEvaluator.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Authorization/TriePermissionEvaluator.cs @@ -83,7 +83,7 @@ public sealed class TriePermissionEvaluator : IPermissionEvaluator OpcUaOperation.WriteTune => NodePermissions.WriteTune, OpcUaOperation.WriteConfigure => NodePermissions.WriteConfigure, OpcUaOperation.HistoryRead => NodePermissions.HistoryRead, - OpcUaOperation.HistoryUpdate => NodePermissions.HistoryRead, // HistoryUpdate bit not yet in NodePermissions; TODO Stream C follow-up + OpcUaOperation.HistoryUpdate => NodePermissions.HistoryUpdate, OpcUaOperation.CreateMonitoredItems => NodePermissions.Subscribe, OpcUaOperation.TransferSubscriptions=> NodePermissions.Subscribe, OpcUaOperation.Call => NodePermissions.MethodCall, diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/TriePermissionEvaluatorTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/TriePermissionEvaluatorTests.cs index 5309997c..4d8b5c4e 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/TriePermissionEvaluatorTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Authorization/TriePermissionEvaluatorTests.cs @@ -191,6 +191,38 @@ public sealed class TriePermissionEvaluatorTests "a session bound to a generation absent from the cache must fail closed"); } + /// Verifies that a HistoryRead-only grant does NOT authorize HistoryUpdate. + [Fact] + public void HistoryRead_grant_does_not_authorize_HistoryUpdate() + { + // Before the fix HistoryUpdate was mapped to the HistoryRead bit, so a read-only grant + // would wrongly authorise a write operation. + var evaluator = MakeEvaluator([Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.HistoryRead)]); + + var decision = evaluator.Authorize(Session(["cn=ops"]), OpcUaOperation.HistoryUpdate, Scope()); + + decision.IsAllowed.ShouldBeFalse("HistoryRead grant must NOT imply HistoryUpdate"); + } + + /// Verifies that a HistoryUpdate grant authorizes HistoryUpdate. + [Fact] + public void HistoryUpdate_grant_authorizes_HistoryUpdate() + { + var evaluator = MakeEvaluator([Row("cn=ops", NodeAclScopeKind.Cluster, null, NodePermissions.HistoryUpdate)]); + + var decision = evaluator.Authorize(Session(["cn=ops"]), OpcUaOperation.HistoryUpdate, Scope()); + + decision.IsAllowed.ShouldBeTrue("HistoryUpdate grant must authorize HistoryUpdate operation"); + } + + /// Verifies that the HistoryUpdate bit is 1<<12 and does not collide with MethodCall. + [Fact] + public void HistoryUpdate_bit_value_and_no_collision() + { + ((int)NodePermissions.HistoryUpdate).ShouldBe(1 << 12); + (NodePermissions.HistoryUpdate & NodePermissions.MethodCall).ShouldBe(NodePermissions.None); + } + /// Verifies that the operation-to-permission mapping is total. [Fact] public void OperationToPermission_Mapping_IsTotal()