fix(management): invalidate RoleMappingCache after role-mapping and scope-rule mutations commit (review 2026-10-21 lane K M1)

This commit is contained in:
Joseph Doherty
2026-10-22 09:26:54 -04:00
parent d495494074
commit df1f4efcfa
3 changed files with 108 additions and 0 deletions
@@ -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
@@ -2820,6 +2820,21 @@ public class ManagementActor : ReceiveActor
$"Role '{role}' is not a recognized role. Valid roles are: {string.Join(", ", Roles.All)}.");
}
/// <summary>
/// Drops this node's <see cref="RoleMappingCache"/> 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 <c>RoleMappingCacheSeconds</c>.
/// </summary>
private static Task InvalidateRoleMappingCacheAfterCommit(IServiceProvider sp)
{
var cache = sp.GetService<RoleMappingCache>();
return cache is null
? Task.CompletedTask
: AfterConfigurationCommit(sp, () => { cache.Invalidate(); return Task.CompletedTask; });
}
private static async Task<object?> 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<ISecurityRepository>();
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<ISecurityRepository>();
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;
}
@@ -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;
/// <summary>
/// Lane K review M1: the process-wide <see cref="RoleMappingCache"/> 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.
/// </summary>
public class RoleMappingCacheInvalidationTests : TestKit
{
private readonly ISecurityRepository _repo = Substitute.For<ISecurityRepository>();
private readonly RoleMappingCache _cache = new(TimeProvider.System);
private readonly ServiceCollection _services = new ServiceCollection().AddTestConfigurationTransaction();
private List<LdapGroupMapping> _mappings = [new("Ops", "Deployer") { Id = 5 }];
/// <summary>Initializes the harness with one Deployer mapping for group "Ops".</summary>
public RoleMappingCacheInvalidationTests()
{
_repo.GetAllMappingsAsync(Arg.Any<CancellationToken>()).Returns(_ => _mappings);
_repo.GetScopeRulesForMappingAsync(Arg.Any<int>(), Arg.Any<CancellationToken>())
.Returns(new List<SiteScopeRule>());
_repo.GetMappingByIdAsync(5, Arg.Any<CancellationToken>()).Returns(_ => _mappings.FirstOrDefault());
_services.AddScoped(_ => _repo);
_services.AddScoped(_ => Substitute.For<IAuditService>());
_services.AddSingleton(_cache);
}
private IActorRef CreateActor() =>
Sys.ActorOf(Props.Create(() => new ManagementActor(
_services.BuildServiceProvider(), NullLogger<ManagementActor>.Instance)));
private static ManagementEnvelope Admin(object command) =>
new(new AuthenticatedUser("admin", "Admin", new[] { "Administrator" }, Array.Empty<string>()),
command, Guid.NewGuid().ToString("N"));
private Task<RoleMappingResult> MapOps() => new RoleMapper(_repo, _cache).MapGroupsToRolesAsync(["Ops"]);
/// <summary>A deleted mapping is visible to the very next mapping on the same cache.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task DeleteRoleMapping_IsVisibleToTheNextMapping()
{
Assert.Contains("Deployer", (await MapOps()).Roles); // primes the snapshot
_mappings = [];
CreateActor().Tell(Admin(new DeleteRoleMappingCommand(5)));
ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
Assert.Empty((await MapOps()).Roles);
}
/// <summary>Every mapping / scope-rule mutation invalidates the snapshot.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[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<ManagementSuccess>(TimeSpan.FromSeconds(5));
await MapOps();
await _repo.Received(1).GetAllMappingsAsync(Arg.Any<CancellationToken>()); // re-read, not served stale
}
}
}