From df1f4efcfaa84abc08547575c4e8c8bd13c9f5d4 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 09:26:54 -0400 Subject: [PATCH] fix(management): invalidate RoleMappingCache after role-mapping and scope-rule mutations commit (review 2026-10-21 lane K M1) --- .../Component-ManagementService.md | 1 + .../ManagementActor.cs | 20 +++++ .../RoleMappingCacheInvalidationTests.cs | 87 +++++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RoleMappingCacheInvalidationTests.cs diff --git a/docs/requirements/Component-ManagementService.md b/docs/requirements/Component-ManagementService.md index 83ded24c..38b1b7e7 100644 --- a/docs/requirements/Component-ManagementService.md +++ b/docs/requirements/Component-ManagementService.md @@ -192,6 +192,7 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B - **ListApiKeys** / **CreateApiKey** / **UpdateApiKey** / **EnableApiKey** / **DisableApiKey** / **DeleteApiKey**: Manage API keys. - **ListRoleMappings** / **CreateRoleMapping** / **UpdateRoleMapping** / **DeleteRoleMapping**: Manage LDAP group-to-role mappings. - **ListScopeRules** / **AddScopeRule** / **DeleteScopeRule**: Manage site scope rules on role mappings. +- Every role-mapping and scope-rule mutation (Create/Update/DeleteRoleMapping, Add/DeleteScopeRule) calls `RoleMappingCache.Invalidate()` as a **post-commit** action, so the change takes effect for the next role mapping on the node that served the command instead of after the `RoleMappingCacheSeconds` TTL (default 30 s); the other central node still converges within that TTL. Post-commit, because invalidating inside the transaction would let a concurrent mapping re-cache the pre-change table. ### Audit Log diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 57cc52f7..1ea6c920 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -2820,6 +2820,21 @@ public class ManagementActor : ReceiveActor $"Role '{role}' is not a recognized role. Valid roles are: {string.Join(", ", Roles.All)}."); } + /// + /// Drops this node's snapshot once a mapping / scope-rule + /// change has committed, so the next role mapping on this node sees it immediately rather + /// than after the cache TTL (lane K review M1). Queued post-commit: invalidating before the + /// commit would let a concurrent mapping re-cache the pre-change table. Other central + /// nodes still converge within RoleMappingCacheSeconds. + /// + private static Task InvalidateRoleMappingCacheAfterCommit(IServiceProvider sp) + { + var cache = sp.GetService(); + return cache is null + ? Task.CompletedTask + : AfterConfigurationCommit(sp, () => { cache.Invalidate(); return Task.CompletedTask; }); + } + private static async Task HandleCreateRoleMapping(IServiceProvider sp, CreateRoleMappingCommand cmd, string user) { var role = CanonicalizeMappingRole(cmd.Role); @@ -2827,6 +2842,7 @@ public class ManagementActor : ReceiveActor var mapping = new LdapGroupMapping(cmd.LdapGroupName, role); await repo.AddMappingAsync(mapping); await repo.SaveChangesAsync(); + await InvalidateRoleMappingCacheAfterCommit(sp); await AuditAsync(sp, user, "Create", "RoleMapping", mapping.Id.ToString(), $"{mapping.LdapGroupName}->{mapping.Role}", mapping); return mapping; } @@ -2841,6 +2857,7 @@ public class ManagementActor : ReceiveActor mapping.Role = role; await repo.UpdateMappingAsync(mapping); await repo.SaveChangesAsync(); + await InvalidateRoleMappingCacheAfterCommit(sp); await AuditAsync(sp, user, "Update", "RoleMapping", mapping.Id.ToString(), $"{mapping.LdapGroupName}->{mapping.Role}", mapping); return mapping; } @@ -2850,6 +2867,7 @@ public class ManagementActor : ReceiveActor var repo = sp.GetRequiredService(); await repo.DeleteMappingAsync(cmd.MappingId); await repo.SaveChangesAsync(); + await InvalidateRoleMappingCacheAfterCommit(sp); await AuditAsync(sp, user, "Delete", "RoleMapping", cmd.MappingId.ToString(), cmd.MappingId.ToString(), null); return true; } @@ -3807,6 +3825,7 @@ public class ManagementActor : ReceiveActor var rule = new SiteScopeRule { LdapGroupMappingId = cmd.MappingId, SiteId = cmd.SiteId }; await repo.AddScopeRuleAsync(rule); await repo.SaveChangesAsync(); + await InvalidateRoleMappingCacheAfterCommit(sp); await AuditAsync(sp, user, "Create", "ScopeRule", rule.Id.ToString(), $"Mapping:{cmd.MappingId}/Site:{cmd.SiteId}", rule); return rule; } @@ -3816,6 +3835,7 @@ public class ManagementActor : ReceiveActor var repo = sp.GetRequiredService(); await repo.DeleteScopeRuleAsync(cmd.ScopeRuleId); await repo.SaveChangesAsync(); + await InvalidateRoleMappingCacheAfterCommit(sp); await AuditAsync(sp, user, "Delete", "ScopeRule", cmd.ScopeRuleId.ToString(), cmd.ScopeRuleId.ToString(), null); return true; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RoleMappingCacheInvalidationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RoleMappingCacheInvalidationTests.cs new file mode 100644 index 00000000..42e758bd --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RoleMappingCacheInvalidationTests.cs @@ -0,0 +1,87 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Security; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; +using ZB.MOM.WW.ScadaBridge.Security; + +namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests; + +/// +/// Lane K review M1: the process-wide served a deleted or +/// changed LDAP-group mapping for up to its TTL on the node that made the change. Every +/// role-mapping / scope-rule mutation now invalidates it once the change is committed. +/// +public class RoleMappingCacheInvalidationTests : TestKit +{ + private readonly ISecurityRepository _repo = Substitute.For(); + private readonly RoleMappingCache _cache = new(TimeProvider.System); + private readonly ServiceCollection _services = new ServiceCollection().AddTestConfigurationTransaction(); + private List _mappings = [new("Ops", "Deployer") { Id = 5 }]; + + /// Initializes the harness with one Deployer mapping for group "Ops". + public RoleMappingCacheInvalidationTests() + { + _repo.GetAllMappingsAsync(Arg.Any()).Returns(_ => _mappings); + _repo.GetScopeRulesForMappingAsync(Arg.Any(), Arg.Any()) + .Returns(new List()); + _repo.GetMappingByIdAsync(5, Arg.Any()).Returns(_ => _mappings.FirstOrDefault()); + _services.AddScoped(_ => _repo); + _services.AddScoped(_ => Substitute.For()); + _services.AddSingleton(_cache); + } + + private IActorRef CreateActor() => + Sys.ActorOf(Props.Create(() => new ManagementActor( + _services.BuildServiceProvider(), NullLogger.Instance))); + + private static ManagementEnvelope Admin(object command) => + new(new AuthenticatedUser("admin", "Admin", new[] { "Administrator" }, Array.Empty()), + command, Guid.NewGuid().ToString("N")); + + private Task MapOps() => new RoleMapper(_repo, _cache).MapGroupsToRolesAsync(["Ops"]); + + /// A deleted mapping is visible to the very next mapping on the same cache. + /// A task that represents the asynchronous operation. + [Fact] + public async Task DeleteRoleMapping_IsVisibleToTheNextMapping() + { + Assert.Contains("Deployer", (await MapOps()).Roles); // primes the snapshot + + _mappings = []; + CreateActor().Tell(Admin(new DeleteRoleMappingCommand(5))); + ExpectMsg(TimeSpan.FromSeconds(5)); + + Assert.Empty((await MapOps()).Roles); + } + + /// Every mapping / scope-rule mutation invalidates the snapshot. + /// A task that represents the asynchronous operation. + [Fact] + public async Task EveryMappingMutation_InvalidatesTheSnapshot() + { + object[] commands = + [ + new CreateRoleMappingCommand("Eng", "Designer"), + new UpdateRoleMappingCommand(5, "Ops", "Viewer"), + new AddScopeRuleCommand(5, 2), + new DeleteScopeRuleCommand(9), + ]; + var actor = CreateActor(); + foreach (var command in commands) + { + await MapOps(); // prime + _repo.ClearReceivedCalls(); + + actor.Tell(Admin(command)); + ExpectMsg(TimeSpan.FromSeconds(5)); + + await MapOps(); + await _repo.Received(1).GetAllMappingsAsync(Arg.Any()); // re-read, not served stale + } + } +}