Skip to content

Commit 588b7e9

Browse files
Copilotromanett
Copilot
and
romanett
authored
[Server] Implement event-driven RolePermissionsValidation caching in MonitoredNode2 (#3583)
* Initial plan * Implement event-driven RolePermissionsValidation with permission caching in MonitoredNode2 Co-authored-by: romanett <[email protected]> * Add DefaultPermissionsChanged event to IConfigurationNodeManager to handle namespace-level default permission changes Co-authored-by: romanett <[email protected]> * Add test & make validation functional * cover case when DefaultRolePermissions are aded after the node exists * cleanup --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: romanett <[email protected]> Co-authored-by: Roman Ettlinger <[email protected]>
1 parent 0a4c87c commit 588b7e9

File tree

6 files changed

+691
-28
lines changed

6 files changed

+691
-28
lines changed

‎Libraries/Opc.Ua.Server/Configuration/ConfigurationNodeManager.cs‎

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,34 @@ protected override NodeState AddBehaviourToPredefinedNode(
251251
return base.AddBehaviourToPredefinedNode(context, predefinedNode);
252252
}
253253

254+
/// <summary>
255+
/// Frees any unmanaged resources.
256+
/// </summary>
257+
protected override void Dispose(bool disposing)
258+
{
259+
if (disposing)
260+
{
261+
if (FindPredefinedNode<NamespacesState>(ObjectIds.Server_Namespaces)
262+
is NamespacesState serverNamespacesNode)
263+
{
264+
serverNamespacesNode.StateChanged -= ServerNamespacesChanged;
265+
}
266+
267+
foreach (NodeState node in PredefinedNodes.Values)
268+
{
269+
if (node is NamespaceMetadataState metadataState)
270+
{
271+
metadataState.StateChanged -= OnNamespaceChildrenChanged;
272+
metadataState.DefaultRolePermissions?.StateChanged -= OnNamespaceDefaultPermissionsChanged;
273+
274+
metadataState.DefaultUserRolePermissions?.StateChanged -= OnNamespaceDefaultPermissionsChanged;
275+
}
276+
}
277+
}
278+
279+
base.Dispose(disposing);
280+
}
281+
254282
///<inheritdoc/>
255283
public void CreateServerConfiguration(
256284
ServerSystemContext systemContext,
@@ -314,6 +342,17 @@ .. configuration.ServerConfiguration.SupportedPrivateKeyFormats
314342
is NamespacesState serverNamespacesNode)
315343
{
316344
serverNamespacesNode.StateChanged += ServerNamespacesChanged;
345+
346+
IList<BaseInstanceState> children = [];
347+
serverNamespacesNode.GetChildren(systemContext, children);
348+
349+
foreach (BaseInstanceState child in children)
350+
{
351+
if (child is NamespaceMetadataState metadataState)
352+
{
353+
SubscribeToNamespaceDefaultPermissions(metadataState);
354+
}
355+
}
317356
}
318357
}
319358

@@ -403,13 +442,19 @@ public NamespaceMetadataState CreateNamespaceMetadataState(string namespaceUri)
403442
namespaceMetadataState.DisplayName = LocalizedText.From(namespaceUri);
404443
namespaceMetadataState.SymbolicName = namespaceUri;
405444
namespaceMetadataState.NamespaceUri.Value = namespaceUri;
445+
namespaceMetadataState.AddDefaultRolePermissions(SystemContext);
446+
namespaceMetadataState.AddDefaultUserRolePermissions(SystemContext);
406447

407448
// add node as child of ServerNamespaces and in predefined nodes
408449
serverNamespacesNode.AddChild(namespaceMetadataState);
409-
serverNamespacesNode.ClearChangeMasks(Server.DefaultSystemContext, true);
450+
serverNamespacesNode.ClearChangeMasks(SystemContext, true);
410451
AddPredefinedNode(SystemContext, namespaceMetadataState);
411452
}
412453

454+
// Subscribe to the default permission properties so that any future changes
455+
// trigger a DefaultPermissionsChanged notification to allow caches to be invalidated.
456+
SubscribeToNamespaceDefaultPermissions(namespaceMetadataState);
457+
413458
return namespaceMetadataState;
414459
}
415460

@@ -1263,6 +1308,20 @@ private void ServerNamespacesChanged(
12631308
m_namespaceMetadataStates.Clear();
12641309
m_namespaceMetadataStatesByIndex.Clear();
12651310
}
1311+
1312+
if (node is NamespacesState serverNamespacesNode)
1313+
{
1314+
IList<BaseInstanceState> children = [];
1315+
serverNamespacesNode.GetChildren(context, children);
1316+
1317+
foreach (BaseInstanceState child in children)
1318+
{
1319+
if (child is NamespaceMetadataState metadataState)
1320+
{
1321+
SubscribeToNamespaceDefaultPermissions(metadataState);
1322+
}
1323+
}
1324+
}
12661325
}
12671326
catch
12681327
{
@@ -1271,6 +1330,64 @@ private void ServerNamespacesChanged(
12711330
}
12721331
}
12731332

1333+
/// <summary>
1334+
/// Subscribes to the <c>StateChanged</c> events of the <c>DefaultRolePermissions</c>
1335+
/// and <c>DefaultUserRolePermissions</c> child nodes of a <see cref="NamespaceMetadataState"/>
1336+
/// to detect changes that require permission cache invalidation.
1337+
/// </summary>
1338+
private void SubscribeToNamespaceDefaultPermissions(NamespaceMetadataState namespaceMetadataState)
1339+
{
1340+
if (namespaceMetadataState.DefaultRolePermissions != null)
1341+
{
1342+
// unsubscribe first to avoid duplicate subscriptions if called multiple times
1343+
namespaceMetadataState.DefaultRolePermissions.StateChanged -= OnNamespaceDefaultPermissionsChanged;
1344+
namespaceMetadataState.DefaultRolePermissions.StateChanged += OnNamespaceDefaultPermissionsChanged;
1345+
}
1346+
1347+
if (namespaceMetadataState.DefaultUserRolePermissions != null)
1348+
{
1349+
namespaceMetadataState.DefaultUserRolePermissions.StateChanged -= OnNamespaceDefaultPermissionsChanged;
1350+
namespaceMetadataState.DefaultUserRolePermissions.StateChanged += OnNamespaceDefaultPermissionsChanged;
1351+
}
1352+
1353+
namespaceMetadataState.StateChanged -= OnNamespaceChildrenChanged;
1354+
namespaceMetadataState.StateChanged += OnNamespaceChildrenChanged;
1355+
}
1356+
1357+
/// <summary>
1358+
/// Handles children change on NamespaceMetadataState and resubscribes to the default permissions nodes
1359+
/// to ensure we are notified of changes on those nodes even if they are recreated.
1360+
/// </summary>
1361+
private void OnNamespaceChildrenChanged(
1362+
ISystemContext context,
1363+
NodeState node,
1364+
NodeStateChangeMasks changes)
1365+
{
1366+
if ((changes & NodeStateChangeMasks.Children) != 0 &&
1367+
node is NamespaceMetadataState namespaceMetadataState)
1368+
{
1369+
SubscribeToNamespaceDefaultPermissions(namespaceMetadataState);
1370+
}
1371+
}
1372+
1373+
/// <summary>
1374+
/// Handles value changes on <c>DefaultRolePermissions</c> or <c>DefaultUserRolePermissions</c>
1375+
/// and raises the <see cref="DefaultPermissionsChanged"/> event.
1376+
/// </summary>
1377+
private void OnNamespaceDefaultPermissionsChanged(
1378+
ISystemContext context,
1379+
NodeState node,
1380+
NodeStateChangeMasks changes)
1381+
{
1382+
if ((changes & NodeStateChangeMasks.Value) != 0)
1383+
{
1384+
DefaultPermissionsChanged?.Invoke(this, EventArgs.Empty);
1385+
}
1386+
}
1387+
1388+
/// <inheritdoc/>
1389+
public event EventHandler DefaultPermissionsChanged;
1390+
12741391
private class UpdateCertificateData
12751392
{
12761393
public NodeId SessionId { get; set; }

‎Libraries/Opc.Ua.Server/Configuration/IConfigurationNodeManager.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,22 @@
2727
* http://opcfoundation.org/License/MIT/1.00/
2828
* ======================================================================*/
2929

30+
using System;
31+
3032
namespace Opc.Ua.Server
3133
{
3234
/// <summary>
3335
/// The Server Configuration Node Manager.
3436
/// </summary>
3537
public interface IConfigurationNodeManager : INodeManager3
3638
{
39+
/// <summary>
40+
/// Raised when the <c>DefaultRolePermissions</c> or <c>DefaultUserRolePermissions</c>
41+
/// property of any <see cref="NamespaceMetadataState"/> node changes.
42+
/// Subscribers should invalidate any cached role-permission validation results.
43+
/// </summary>
44+
event EventHandler DefaultPermissionsChanged;
45+
3746
/// <summary>
3847
/// Gets or creates the <see cref="NamespaceMetadataState"/> node for the specified NamespaceUri.
3948
/// </summary>

‎Libraries/Opc.Ua.Server/NodeManager/MonitoredItem/MonitoredNode.cs‎

Lines changed: 61 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,16 @@ public bool HasMonitoredItems
107107
/// <param name="datachangeItem">The monitored item.</param>
108108
public void Add(IDataChangeMonitoredItem2 datachangeItem)
109109
{
110+
bool wasEmpty = DataChangeMonitoredItems.IsEmpty;
110111
DataChangeMonitoredItems.TryAdd(datachangeItem.Id, datachangeItem);
111112

112113
Node.OnStateChanged = OnMonitoredNodeChanged;
114+
115+
// Subscribe to namespace default permission changes when the first item is added.
116+
if (wasEmpty && m_server.ConfigurationNodeManager != null)
117+
{
118+
m_server.ConfigurationNodeManager.DefaultPermissionsChanged += OnDefaultPermissionsChanged;
119+
}
113120
}
114121

115122
/// <summary>
@@ -122,12 +129,18 @@ public void Remove(IDataChangeMonitoredItem2 datachangeItem)
122129
{
123130
// Remove the cached context for the monitored item
124131
m_contextCache.TryRemove(datachangeItem.Id, out _);
125-
m_operationContextCache.TryRemove(datachangeItem.Id, out _);
132+
m_permissionCache.TryRemove(datachangeItem.Id, out _);
126133
}
127134

128135
if (DataChangeMonitoredItems.IsEmpty)
129136
{
130137
Node.OnStateChanged = null;
138+
139+
// Unsubscribe from namespace default permission changes when the last item is removed.
140+
if (m_server.ConfigurationNodeManager != null)
141+
{
142+
m_server.ConfigurationNodeManager.DefaultPermissionsChanged -= OnDefaultPermissionsChanged;
143+
}
131144
}
132145
}
133146

@@ -234,39 +247,60 @@ public void OnMonitoredNodeChanged(
234247
return;
235248
}
236249

250+
// If RolePermissions or UserRolePermissions have changed, invalidate the permission cache
251+
// so it is revalidated on the next value change notification.
252+
if ((changes & NodeStateChangeMasks.RolePermissions) != 0)
253+
{
254+
m_permissionCache.Clear();
255+
}
256+
237257
foreach (KeyValuePair<uint, IDataChangeMonitoredItem2> kvp in DataChangeMonitoredItems)
238258
{
239259
IDataChangeMonitoredItem2 monitoredItem = kvp.Value;
260+
OperationContext operationContext;
261+
ISystemContext contextToUse;
262+
263+
if (context is ServerSystemContext serverContext)
264+
{
265+
ServerSystemContext serverSystemContextToUse = GetOrCreateContext(serverContext, monitoredItem);
266+
operationContext = serverSystemContextToUse.OperationContext;
267+
contextToUse = serverSystemContextToUse;
268+
}
269+
else
270+
{
271+
operationContext = new OperationContext(monitoredItem);
272+
contextToUse = context;
273+
}
240274

241275
if (monitoredItem.AttributeId == Attributes.Value &&
242276
(changes & NodeStateChangeMasks.Value) != 0)
243277
{
244-
if (!m_operationContextCache.TryGetValue(monitoredItem.Id, out OperationContext operationContext))
278+
// Use cached permission result to avoid validating on every value change.
279+
// The cache is invalidated when RolePermissions/UserRolePermissions change
280+
// or when the user identity of the monitored item changes.
281+
if (!m_permissionCache.TryGetValue(monitoredItem.Id, out ServiceResult validationResult))
245282
{
246-
operationContext = new OperationContext(monitoredItem);
247-
m_operationContextCache[monitoredItem.Id] = operationContext;
283+
validationResult = NodeManager.ValidateRolePermissions(
284+
operationContext,
285+
node.NodeId,
286+
PermissionType.Read);
287+
m_permissionCache[monitoredItem.Id] = validationResult;
248288
}
249289

250-
// validate if the monitored item has the required role permissions to read the value
251-
ServiceResult validationResult = NodeManager.ValidateRolePermissions(
252-
operationContext,
253-
node.NodeId,
254-
PermissionType.Read);
255-
256290
if (ServiceResult.IsBad(validationResult))
257291
{
258292
// skip if the monitored item does not have permission to read
259293
continue;
260294
}
261295

262-
QueueValue(context, node, monitoredItem);
296+
QueueValue(contextToUse, node, monitoredItem);
263297
continue;
264298
}
265299

266300
if (monitoredItem.AttributeId != Attributes.Value &&
267301
(changes & NodeStateChangeMasks.NonValue) != 0)
268302
{
269-
QueueValue(context, node, monitoredItem);
303+
QueueValue(contextToUse, node, monitoredItem);
270304
}
271305
}
272306
}
@@ -287,16 +321,8 @@ public void QueueValue(
287321
SourceTimestamp = DateTime.MinValue,
288322
StatusCode = StatusCodes.Good
289323
};
290-
291-
ISystemContext contextToUse = context;
292-
293-
if (context is ServerSystemContext systemContext)
294-
{
295-
contextToUse = GetOrCreateContext(systemContext, monitoredItem);
296-
}
297-
298324
ServiceResult error = node.ReadAttribute(
299-
contextToUse,
325+
context,
300326
monitoredItem.AttributeId,
301327
monitoredItem.IndexRange,
302328
monitoredItem.DataEncoding,
@@ -336,12 +362,14 @@ private ServerSystemContext GetOrCreateContext(
336362
(currentTicks - cachedEntry.CreatedAtTicks) > m_cacheLifetimeTicks)
337363
{
338364
operationContext = new OperationContext(monitoredItem);
339-
m_operationContextCache[monitoredItemId] = operationContext;
340365

341366
ServerSystemContext updatedContext = context.Copy(
342367
operationContext);
343368
m_contextCache[monitoredItemId] = (updatedContext, currentTicks);
344369

370+
// Invalidate the permission cache since the user identity may have changed.
371+
m_permissionCache.TryRemove(monitoredItemId, out _);
372+
345373
return updatedContext;
346374
}
347375

@@ -352,15 +380,24 @@ private ServerSystemContext GetOrCreateContext(
352380
operationContext = new OperationContext(monitoredItem);
353381
ServerSystemContext newContext = context.Copy(operationContext);
354382
m_contextCache.TryAdd(monitoredItemId, (newContext, currentTicks));
355-
m_operationContextCache[monitoredItemId] = operationContext;
356383

357384
return newContext;
358385
}
359386

387+
/// <summary>
388+
/// Called when the namespace default permissions (<c>DefaultRolePermissions</c> or
389+
/// <c>DefaultUserRolePermissions</c>) change. Invalidates the entire permission cache
390+
/// so that all entries are re-validated on the next value change.
391+
/// </summary>
392+
private void OnDefaultPermissionsChanged(object sender, EventArgs e)
393+
{
394+
m_permissionCache.Clear();
395+
}
396+
360397
private readonly ConcurrentDictionary<uint, (ServerSystemContext Context, int CreatedAtTicks)> m_contextCache =
361398
new();
362399

363-
private readonly ConcurrentDictionary<uint, OperationContext> m_operationContextCache =
400+
private readonly ConcurrentDictionary<uint, ServiceResult> m_permissionCache =
364401
new();
365402

366403
private readonly int m_cacheLifetimeTicks = (int)TimeSpan.FromMinutes(5).TotalMilliseconds;

‎Stack/Opc.Ua.Types/State/NodeState.cs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ public RolePermissionTypeCollection RolePermissions
553553
{
554554
if (m_rolePermissions != value)
555555
{
556-
m_changeMasks |= NodeStateChangeMasks.NonValue;
556+
m_changeMasks |= NodeStateChangeMasks.NonValue | NodeStateChangeMasks.RolePermissions;
557557
}
558558

559559
m_rolePermissions = value;
@@ -571,7 +571,7 @@ public RolePermissionTypeCollection UserRolePermissions
571571
{
572572
if (m_userRolePermissions != value)
573573
{
574-
m_changeMasks |= NodeStateChangeMasks.NonValue;
574+
m_changeMasks |= NodeStateChangeMasks.NonValue | NodeStateChangeMasks.RolePermissions;
575575
}
576576

577577
m_userRolePermissions = value;
@@ -4207,6 +4207,7 @@ NodeAttributeEventHandler<AttributeWriteMask> onWriteUserWriteMask
42074207
if (ServiceResult.IsGood(result))
42084208
{
42094209
m_rolePermissions = rolePermissions;
4210+
m_changeMasks |= NodeStateChangeMasks.NonValue | NodeStateChangeMasks.RolePermissions;
42104211
}
42114212

42124213
return result;
@@ -5206,7 +5207,13 @@ public enum NodeStateChangeMasks
52065207
/// <summary>
52075208
/// The node has been deleted.
52085209
/// </summary>
5209-
Deleted = 0x10
5210+
Deleted = 0x10,
5211+
5212+
/// <summary>
5213+
/// The RolePermissions or UserRolePermissions attribute has changed.
5214+
/// This is a subset of <see cref="NonValue"/> and is set in addition to it.
5215+
/// </summary>
5216+
RolePermissions = 0x20
52105217
}
52115218

52125219
/// <summary>

0 commit comments

Comments
 (0)