Skip to content

Enforce NodeManagement authorization and rollback (OPC 10000-4/18)#4046

Merged
marcschier merged 5 commits into
OPCFoundation:masterfrom
marcschier:marcschier/review-fix/node-management-authorization
Jul 23, 2026
Merged

Enforce NodeManagement authorization and rollback (OPC 10000-4/18)#4046
marcschier merged 5 commits into
OPCFoundation:masterfrom
marcschier:marcschier/review-fix/node-management-authorization

Conversation

@marcschier

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes missing AddNode, DeleteNode, AddReference, and RemoveReference authorization before NodeManagement mutations.
  • Enforces applicable namespace and Node AccessRestrictions.
  • Distinguishes local targets from remote targetServerUri, server-index, and namespace-URI targets.
  • Validates both local reference endpoints and uses the actual reciprocal NodeClass for cross-NodeManager mutations.
  • Adds bounded, cancellation-safe compensation when reciprocal add/delete mutations fail.

Specification

  • OPC 10000-4 §§5.8.2-5.8.3
  • OPC 10000-18 role-based access control

This is split from #4021.

Tests

Focused Release validation passed on both net10.0 and net48:

  • dotnet build tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f <TFM> -p:CustomTestTarget=<TFM>
  • dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f <TFM> -p:CustomTestTarget=<TFM> --no-build --no-restore --filter FullyQualifiedName~Opc.Ua.Server.Tests.MasterNodeManagerNodeManagementTests
  • 55 tests passed per TFM, including AddNodesDeniedAddNodePermissionDoesNotMutateAsync, AddNodesDeniedParentAddReferencePermissionDoesNotMutateAsync, DeleteNodesDeniedDeleteNodePermissionDoesNotMutateAsync, AddReferencesGrantedPermissionsMutateBothSidesAsync, AddReferencesTargetServerUriSkipsLocalTargetProcessingAsync, AddReferencesInverseFailureRollsBackSourceAsync, AddReferencesCancellationUsesIndependentRollbackToken, DeleteReferencesRemoteTargetSkipsReciprocalProcessingAsync, DeleteReferencesInverseFailureRestoresSourceAsync, and DeleteReferencesUnexpectedFailureRestoresSourceWithIndependentToken.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 10:49
Copilot AI review requested due to automatic review settings July 19, 2026 10:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Enforces OPC UA NodeManagement authorization semantics (role permissions + access restrictions) and adds rollback/compensation behavior when cross-NodeManager reciprocal reference mutations fail.

Changes:

  • Adds/updates NodeManagement tests for authorization gating, access restrictions, local-vs-remote target handling, and rollback behavior.
  • Updates MasterNodeManager NodeManagement dispatch paths to validate permissions/access restrictions and to compensate on inverse-mutation failures.
  • Updates NodeManagement documentation to describe the new routing, authorization, and rollback behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerNodeManagementTests.cs Adds extensive authorization/access-restriction/rollback tests and a reusable authorization harness.
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Adds permission checks for AddNode/DeleteNode/AddReference/RemoveReference, local-target detection, and bounded compensation logic.
docs/NodeManagement.md Documents updated routing rules, permission requirements, local-target rules, and compensation behavior.

Comment thread src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Outdated
Comment thread src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Outdated
Comment thread docs/NodeManagement.md
@marcschier
marcschier marked this pull request as draft July 19, 2026 11:36
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 11:55
@marcschier
marcschier marked this pull request as draft July 19, 2026 12:12
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 14:13
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as draft July 19, 2026 16:47
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 16:57
@marcschier

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.04469% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.69%. Comparing base (91d3305) to head (189c9ef).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs 98.04% 4 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4046      +/-   ##
==========================================
- Coverage   73.94%   73.69%   -0.26%     
==========================================
  Files        1347     1347              
  Lines      180826   181092     +266     
  Branches    31798    31825      +27     
==========================================
- Hits       133719   133458     -261     
- Misses      36343    36917     +574     
+ Partials    10764    10717      -47     
Files with missing lines Coverage Δ
src/Opc.Ua.Server/NodeManager/INodeManager.cs 100.00% <ø> (ø)
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs 79.11% <98.04%> (+6.88%) ⬆️

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Outdated
…ix/node-management-authorization

# Conflicts:
#	tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerNodeManagementTests.cs
@marcschier
marcschier merged commit dd6a591 into OPCFoundation:master Jul 23, 2026
219 of 223 checks passed
@marcschier
marcschier deleted the marcschier/review-fix/node-management-authorization branch July 23, 2026 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants