From 3b8280f08ae9580136c3e74b33f524017fb07f07 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 19 Apr 2026 09:39:06 -0400 Subject: [PATCH] =?UTF-8?q?Phase=206.2=20Stream=20D=20(data=20layer)=20?= =?UTF-8?q?=E2=80=94=20ValidatedNodeAclAuthoringService=20with=20write-tim?= =?UTF-8?q?e=20invariants?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ships the non-UI piece of Stream D: a draft-aware write surface over NodeAcl that enforces the Phase 6.2 plan's scope-uniqueness + grant-shape invariants. Blazor UI pieces (RoleGrantsTab + AclsTab refresh + SignalR invalidation + visual-compliance reviewer signoff) are deferred to the Phase 6.1-style follow-up task. Admin.Services: - ValidatedNodeAclAuthoringService — alongside existing NodeAclService (raw CRUD, kept for read + revoke paths). GrantAsync enforces: * Permissions != None (decision #129 — additive only, no empty grants). * Cluster scope has null ScopeId. * Sub-cluster scope requires a populated ScopeId. * No duplicate (GenerationId, ClusterId, LdapGroup, ScopeKind, ScopeId) tuple — operator updates the row instead of inserting a duplicate. UpdatePermissionsAsync also rejects None (operator revokes via NodeAclService). Violations throw InvalidNodeAclGrantException. Tests (10 new in Admin.Tests/ValidatedNodeAclAuthoringServiceTests): - Grant rejects None permissions. - Grant rejects Cluster-scope with ScopeId / sub-cluster without ScopeId. - Grant succeeds on well-formed row. - Grant rejects duplicate (group, scope) in same draft. - Grant allows same group at different scope. - Grant allows same (group, scope) in different draft. - UpdatePermissions rejects None. - UpdatePermissions round-trips new flags + notes. - UpdatePermissions on unknown rowid throws. Microsoft.EntityFrameworkCore.InMemory 10.0.0 added to Admin.Tests csproj. Full solution dotnet test: 1097 passing (was 1087, +10). Phase 6.2 total is now 1087+10 = 1097; baseline 906 → +191 net across Phase 6.1 (all streams) + Phase 6.2 (Streams A, B, C foundation, D data layer). Stream D follow-up task tracks: RoleGrantsTab CRUD over LdapGroupRoleMapping, AclsTab write-through + Probe-this-permission diagnostic, draft-diff ACL section, SignalR PermissionTrieCache invalidation push. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ValidatedNodeAclAuthoringService.cs | 117 ++++++++++++++ .../ValidatedNodeAclAuthoringServiceTests.cs | 146 ++++++++++++++++++ .../ZB.MOM.WW.OtOpcUa.Admin.Tests.csproj | 1 + 3 files changed, 264 insertions(+) create mode 100644 src/ZB.MOM.WW.OtOpcUa.Admin/Services/ValidatedNodeAclAuthoringService.cs create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/ValidatedNodeAclAuthoringServiceTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Admin/Services/ValidatedNodeAclAuthoringService.cs b/src/ZB.MOM.WW.OtOpcUa.Admin/Services/ValidatedNodeAclAuthoringService.cs new file mode 100644 index 0000000..15f3a13 --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Admin/Services/ValidatedNodeAclAuthoringService.cs @@ -0,0 +1,117 @@ +using Microsoft.EntityFrameworkCore; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; + +namespace ZB.MOM.WW.OtOpcUa.Admin.Services; + +/// +/// Draft-aware write surface over . Replaces direct +/// CRUD for Admin UI grant authoring; the raw service stays +/// as the read / delete surface. Enforces the invariants listed in Phase 6.2 Stream D.2: +/// scope-uniqueness per (LdapGroup, ScopeKind, ScopeId, GenerationId), grant shape +/// consistency, and no empty permission masks. +/// +/// +/// Per decision #129 grants are additive — is +/// rejected at write time. Explicit Deny is v2.1 and is not representable in the current +/// NodeAcl row; attempts to express it (e.g. empty permission set) surface as +/// . +/// +/// Draft scope: writes always target an unpublished (Draft-state) generation id. +/// Once a generation publishes, its rows are frozen. +/// +public sealed class ValidatedNodeAclAuthoringService(OtOpcUaConfigDbContext db) +{ + /// Add a new grant row to the given draft generation. + public async Task GrantAsync( + long draftGenerationId, + string clusterId, + string ldapGroup, + NodeAclScopeKind scopeKind, + string? scopeId, + NodePermissions permissions, + string? notes, + CancellationToken cancellationToken) + { + ArgumentException.ThrowIfNullOrWhiteSpace(clusterId); + ArgumentException.ThrowIfNullOrWhiteSpace(ldapGroup); + + ValidateGrantShape(scopeKind, scopeId, permissions); + await EnsureNoDuplicate(draftGenerationId, clusterId, ldapGroup, scopeKind, scopeId, cancellationToken).ConfigureAwait(false); + + var row = new NodeAcl + { + GenerationId = draftGenerationId, + NodeAclId = $"acl-{Guid.NewGuid():N}"[..20], + ClusterId = clusterId, + LdapGroup = ldapGroup, + ScopeKind = scopeKind, + ScopeId = scopeId, + PermissionFlags = permissions, + Notes = notes, + }; + db.NodeAcls.Add(row); + await db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + return row; + } + + /// + /// Replace an existing grant's permission set in place. Validates the new shape; + /// rejects attempts to blank-out to None (that's a Revoke via ). + /// + public async Task UpdatePermissionsAsync( + Guid nodeAclRowId, + NodePermissions newPermissions, + string? notes, + CancellationToken cancellationToken) + { + if (newPermissions == NodePermissions.None) + throw new InvalidNodeAclGrantException( + "Permission set cannot be None — revoke the row instead of writing an empty grant."); + + var row = await db.NodeAcls.FirstOrDefaultAsync(a => a.NodeAclRowId == nodeAclRowId, cancellationToken).ConfigureAwait(false) + ?? throw new InvalidNodeAclGrantException($"NodeAcl row {nodeAclRowId} not found."); + + row.PermissionFlags = newPermissions; + if (notes is not null) row.Notes = notes; + await db.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + return row; + } + + private static void ValidateGrantShape(NodeAclScopeKind scopeKind, string? scopeId, NodePermissions permissions) + { + if (permissions == NodePermissions.None) + throw new InvalidNodeAclGrantException( + "Permission set cannot be None — grants must carry at least one flag (decision #129, additive only)."); + + if (scopeKind == NodeAclScopeKind.Cluster && !string.IsNullOrEmpty(scopeId)) + throw new InvalidNodeAclGrantException( + "Cluster-scope grants must have null ScopeId. ScopeId only applies to sub-cluster scopes."); + + if (scopeKind != NodeAclScopeKind.Cluster && string.IsNullOrEmpty(scopeId)) + throw new InvalidNodeAclGrantException( + $"ScopeKind={scopeKind} requires a populated ScopeId."); + } + + private async Task EnsureNoDuplicate( + long generationId, string clusterId, string ldapGroup, NodeAclScopeKind scopeKind, string? scopeId, + CancellationToken cancellationToken) + { + var exists = await db.NodeAcls.AsNoTracking() + .AnyAsync(a => a.GenerationId == generationId + && a.ClusterId == clusterId + && a.LdapGroup == ldapGroup + && a.ScopeKind == scopeKind + && a.ScopeId == scopeId, + cancellationToken).ConfigureAwait(false); + + if (exists) + throw new InvalidNodeAclGrantException( + $"A grant for (LdapGroup={ldapGroup}, ScopeKind={scopeKind}, ScopeId={scopeId}) already exists in generation {generationId}. " + + "Update the existing row's permissions instead of inserting a duplicate."); + } +} + +/// Thrown when a grant authoring request violates an invariant. +public sealed class InvalidNodeAclGrantException(string message) : Exception(message); diff --git a/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/ValidatedNodeAclAuthoringServiceTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/ValidatedNodeAclAuthoringServiceTests.cs new file mode 100644 index 0000000..f91f89f --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/ValidatedNodeAclAuthoringServiceTests.cs @@ -0,0 +1,146 @@ +using Microsoft.EntityFrameworkCore; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Admin.Services; +using ZB.MOM.WW.OtOpcUa.Configuration; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; + +namespace ZB.MOM.WW.OtOpcUa.Admin.Tests; + +[Trait("Category", "Unit")] +public sealed class ValidatedNodeAclAuthoringServiceTests : IDisposable +{ + private readonly OtOpcUaConfigDbContext _db; + + public ValidatedNodeAclAuthoringServiceTests() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase($"val-nodeacl-{Guid.NewGuid():N}") + .Options; + _db = new OtOpcUaConfigDbContext(options); + } + + public void Dispose() => _db.Dispose(); + + [Fact] + public async Task Grant_Rejects_NonePermissions() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + + await Should.ThrowAsync(() => svc.GrantAsync( + draftGenerationId: 1, clusterId: "c1", ldapGroup: "cn=ops", + scopeKind: NodeAclScopeKind.Cluster, scopeId: null, + permissions: NodePermissions.None, notes: null, CancellationToken.None)); + } + + [Fact] + public async Task Grant_Rejects_ClusterScope_With_ScopeId() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + + await Should.ThrowAsync(() => svc.GrantAsync( + 1, "c1", "cn=ops", + NodeAclScopeKind.Cluster, scopeId: "not-null-wrong", + NodePermissions.Read, null, CancellationToken.None)); + } + + [Fact] + public async Task Grant_Rejects_SubClusterScope_Without_ScopeId() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + + await Should.ThrowAsync(() => svc.GrantAsync( + 1, "c1", "cn=ops", + NodeAclScopeKind.Equipment, scopeId: null, + NodePermissions.Read, null, CancellationToken.None)); + } + + [Fact] + public async Task Grant_Succeeds_When_Valid() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + + var row = await svc.GrantAsync( + 1, "c1", "cn=ops", + NodeAclScopeKind.Cluster, null, + NodePermissions.Read | NodePermissions.Browse, "fleet reader", CancellationToken.None); + + row.LdapGroup.ShouldBe("cn=ops"); + row.PermissionFlags.ShouldBe(NodePermissions.Read | NodePermissions.Browse); + row.NodeAclId.ShouldNotBeNullOrWhiteSpace(); + } + + [Fact] + public async Task Grant_Rejects_DuplicateScopeGroup_Pair() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + await svc.GrantAsync(1, "c1", "cn=ops", NodeAclScopeKind.Cluster, null, + NodePermissions.Read, null, CancellationToken.None); + + await Should.ThrowAsync(() => svc.GrantAsync( + 1, "c1", "cn=ops", NodeAclScopeKind.Cluster, null, + NodePermissions.WriteOperate, null, CancellationToken.None)); + } + + [Fact] + public async Task Grant_SameGroup_DifferentScope_IsAllowed() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + await svc.GrantAsync(1, "c1", "cn=ops", NodeAclScopeKind.Cluster, null, + NodePermissions.Read, null, CancellationToken.None); + + var tagRow = await svc.GrantAsync(1, "c1", "cn=ops", + NodeAclScopeKind.Tag, scopeId: "tag-xyz", + NodePermissions.WriteOperate, null, CancellationToken.None); + + tagRow.ScopeKind.ShouldBe(NodeAclScopeKind.Tag); + } + + [Fact] + public async Task Grant_SameGroupScope_DifferentDraft_IsAllowed() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + await svc.GrantAsync(1, "c1", "cn=ops", NodeAclScopeKind.Cluster, null, + NodePermissions.Read, null, CancellationToken.None); + + var draft2Row = await svc.GrantAsync(2, "c1", "cn=ops", + NodeAclScopeKind.Cluster, null, + NodePermissions.Read, null, CancellationToken.None); + + draft2Row.GenerationId.ShouldBe(2); + } + + [Fact] + public async Task UpdatePermissions_Rejects_None() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + var row = await svc.GrantAsync(1, "c1", "cn=ops", NodeAclScopeKind.Cluster, null, + NodePermissions.Read, null, CancellationToken.None); + + await Should.ThrowAsync( + () => svc.UpdatePermissionsAsync(row.NodeAclRowId, NodePermissions.None, null, CancellationToken.None)); + } + + [Fact] + public async Task UpdatePermissions_RoundTrips_NewFlags() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + var row = await svc.GrantAsync(1, "c1", "cn=ops", NodeAclScopeKind.Cluster, null, + NodePermissions.Read, null, CancellationToken.None); + + var updated = await svc.UpdatePermissionsAsync(row.NodeAclRowId, + NodePermissions.Read | NodePermissions.WriteOperate, "bumped", CancellationToken.None); + + updated.PermissionFlags.ShouldBe(NodePermissions.Read | NodePermissions.WriteOperate); + updated.Notes.ShouldBe("bumped"); + } + + [Fact] + public async Task UpdatePermissions_MissingRow_Throws() + { + var svc = new ValidatedNodeAclAuthoringService(_db); + + await Should.ThrowAsync( + () => svc.UpdatePermissionsAsync(Guid.NewGuid(), NodePermissions.Read, null, CancellationToken.None)); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/ZB.MOM.WW.OtOpcUa.Admin.Tests.csproj b/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/ZB.MOM.WW.OtOpcUa.Admin.Tests.csproj index 5c8d455..05b7bd1 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/ZB.MOM.WW.OtOpcUa.Admin.Tests.csproj +++ b/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/ZB.MOM.WW.OtOpcUa.Admin.Tests.csproj @@ -22,6 +22,7 @@ + -- 2.49.1