Repository navigation
Enhance role-based access control and add permission tests - #3653
Conversation
- Improve ValidateRolePermissions logic and add ILogger support for detailed access denial logging - Expand default roles (add AuthenticatedUser, TrustedApplication) - Refine RoleBasedIdentity to merge roles and avoid duplicates - Set UserIdentity default GrantedRoleIds to Anonymous - Fix MasterNodeManager PrepareValidationCache - Add comprehensive unit tests for role permission validation and logging
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3653 +/- ##
==========================================
+ Coverage 69.94% 70.03% +0.08%
==========================================
Files 477 477
Lines 103319 103336 +17
Branches 17402 17411 +9
==========================================
+ Hits 72266 72367 +101
+ Misses 25891 25827 -64
+ Partials 5162 5142 -20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot Please fix the failing Opc.Ua.Gds.Tests |
…plicationSelfAdmin The RoleBasedIdentity constructor was filtering out roles with null RoleIds (!role.RoleId.IsNull), which broke the GDS ApplicationSelfAdmin mechanism. The GDS OnAddSelfAdminUserRolePermissions relies on GrantedRoleIds.Contains(NodeId.Null) to detect self-admin users (since GdsRole.ApplicationSelfAdmin has RoleId = ExpandedNodeId.Null). Also remove unused System.Data using statement. Agent-Logs-Url: https://github.com/OPCFoundation/UA-.NETStandard/sessions/3db86efa-c6d6-4e63-947f-156a88210b58 Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
Fixed in commit |
|
|
…yAndPermissionsCache
Improves role-based access control in the OPC UA server stack: fixes
ValidateRolePermissionslogic, expands default role assignments, refinesRoleBasedIdentitymerging, and adds comprehensive unit tests. Also fixes a regression that broke GDSApplicationSelfAdminsessions.Proposed changes
RoleBasedIdentityconstructorRoleBasedIdentityinner identity (avoids losing roles on re-wrap)GrantedRoleIdswhen composing from inner identity + new roles!role.RoleId.IsNullfilter that brokeGdsRole.ApplicationSelfAdmin: that role usesRoleId = ExpandedNodeId.Nullintentionally, andOnAddSelfAdminUserRolePermissionsrelies onGrantedRoleIds.Contains(NodeId.Null)to detect self-admin sessionsUserIdentityGrantedRoleIdsnow returns[WellKnownRole_Anonymous]instead of empty, preventingValidateRolePermissionsfrom failing with "no granted role" for unauthenticated sessionsValidateRolePermissions(MasterNodeManager)requestedPermission); applies the mask at match-time — logically equivalent but consistent across multi-entry rolesILoggerparameter for access-denial diagnosticsPrepareValidationCache<T>to correctly handleBrowseDescriptionandCallMethodRequestnode IDs (previously onlyReadValueIdwas handled, others threw on null nodeId)Role additions
Role.TrustedApplicationandRole.AuthenticatedUserstatic propertiesSessionManagerassignsTrustedApplicationrole when the session has a client certificate andSecurityMode >= SignGDS fix
System.DatastrayusingdirectiveTests
Opc.Ua.Server.TestscoveringValidateRolePermissionsscenarios: no permissions defined, anonymous access, role match/mismatch, and logger output on denialTypes of changes
Checklist
Further comments
The GDS regression root cause:
RoleBasedIdentitywas filteringrole.RoleId.IsNullbefore converting toNodeId, soApplicationSelfAdmin(intentionally null RoleId) never contributedNodeId.NulltoGrantedRoleIds. The GDSOnAddSelfAdminUserRolePermissionscallback usesGrantedRoleIds.Contains(NodeId.Null)as the signal to injectCallpermission intoUserRolePermissions— without it,ValidateRolePermissionsinCustomNodeManager.CallInternalAsyncreturnedBadUserAccessDeniedbefore the method handler was reached.