fix(management): canonicalize role-mapping casing at write path, closing UI/actor case split (arch-review C5)

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj

docs/requirements/Component-Security.md

@@ -177,6 +177,7 @@ Role checks are expressed as named ASP.NET Core authorization policies (in `Auth
   - `SCADA-Verifiers` → Verifier role (approves secured writes)
 - A user can be a member of multiple groups, granting multiple independent roles.
 - Group mappings are stored in the configuration database and managed via the Central UI (Administrator role).
+- **Stored role casing is canonicalized at the write path** (arch-review C5): the mapping's `Role` arrives as a free string over the CLI/Management API, so `ManagementActor.HandleCreateRoleMapping`/`HandleUpdateRoleMapping` canonicalize it to the exact `Roles.All` casing (e.g. `deployer` → `Deployer`) before persisting, and reject any value not in `Roles.All`. This closes a CLI-works/UI-403s split-brain: the ASP.NET `RequireClaim` policies compare the role claim ordinal case-sensitively while the ManagementActor gates compare case-insensitively, so a row stored with off-canonical casing would pass every actor gate but fail every UI policy. Canonicalizing at this single server-side write path guarantees the two comparisons always agree.
 
 ## Permission Enforcement
 

src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs

@@ -2103,28 +2103,27 @@ public class ManagementActor : ReceiveActor
         return await repo.GetAllMappingsAsync();
     }
 
-    // An LDAP-group mapping's Role is a free string on the
-    // wire (CLI/API), so reject anything outside the canonical Roles.All set at the single
-    // server-side write path. A non-canonical role never functioned (no policy or authz
-    // check matches it), so rejecting it removes a silent-misconfiguration footgun rather
-    // than changing behaviour. Membership is checked case-insensitively to match the rest
-    // of the actor's role comparisons; the existing case-sensitivity asymmetry between the
-    // UI RequireClaim policies and the ManagementActor check is a separately-deferred change
-    // and is deliberately NOT altered here — the stored value's verbatim casing is preserved.
-    private static void ValidateMappingRole(string role)
+    // An LDAP-group mapping's Role is a free string on the wire (CLI/API), so both
+    // validate membership AND canonicalize casing at the single server-side write path.
+    // The stored Role must be CANONICAL casing (Roles.All): ASP.NET RequireClaim policies
+    // compare ordinal case-sensitively while the actor compares OrdinalIgnoreCase, so a
+    // row stored as "deployer" passes every actor gate but fails every UI policy — a
+    // CLI-works/UI-403s split-brain. Canonicalizing here closes the asymmetry (arch-review
+    // C5; closes the deferral formerly documented here). A role outside Roles.All never
+    // functioned (no policy or authz check matches it), so rejecting it removes a
+    // silent-misconfiguration footgun rather than changing behaviour.
+    private static string CanonicalizeMappingRole(string role)
     {
-        if (!Roles.All.Contains(role, StringComparer.OrdinalIgnoreCase))
-        {
-            throw new ManagementCommandException(
-                $"Role '{role}' is not a recognized role. Valid roles are: {string.Join(", ", Roles.All)}.");
-        }
+        var canonical = Roles.All.FirstOrDefault(r => string.Equals(r, role, StringComparison.OrdinalIgnoreCase));
+        return canonical ?? throw new ManagementCommandException(
+            $"Role '{role}' is not a recognized role. Valid roles are: {string.Join(", ", Roles.All)}.");
     }
 
     private static async Task<object?> HandleCreateRoleMapping(IServiceProvider sp, CreateRoleMappingCommand cmd, string user)
     {
-        ValidateMappingRole(cmd.Role);
+        var role = CanonicalizeMappingRole(cmd.Role);
         var repo = sp.GetRequiredService<ISecurityRepository>();
-        var mapping = new LdapGroupMapping(cmd.LdapGroupName, cmd.Role);
+        var mapping = new LdapGroupMapping(cmd.LdapGroupName, role);
         await repo.AddMappingAsync(mapping);
         await repo.SaveChangesAsync();
         await AuditAsync(sp, user, "Create", "RoleMapping", mapping.Id.ToString(), $"{mapping.LdapGroupName}->{mapping.Role}", mapping);
@@ -2133,12 +2132,12 @@ public class ManagementActor : ReceiveActor
 
     private static async Task<object?> HandleUpdateRoleMapping(IServiceProvider sp, UpdateRoleMappingCommand cmd, string user)
     {
-        ValidateMappingRole(cmd.Role);
+        var role = CanonicalizeMappingRole(cmd.Role);
         var repo = sp.GetRequiredService<ISecurityRepository>();
         var mapping = await repo.GetMappingByIdAsync(cmd.MappingId)
             ?? throw new ManagementCommandException($"RoleMapping with ID {cmd.MappingId} not found.");
         mapping.LdapGroupName = cmd.LdapGroupName;
-        mapping.Role = cmd.Role;
+        mapping.Role = role;
         await repo.UpdateMappingAsync(mapping);
         await repo.SaveChangesAsync();
         await AuditAsync(sp, user, "Update", "RoleMapping", mapping.Id.ToString(), $"{mapping.LdapGroupName}->{mapping.Role}", mapping);

tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/RoleMappingValidationTests.cs

@@ -109,9 +109,47 @@ public class RoleMappingValidationTests : TestKit, IDisposable
 
         ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
         Assert.NotNull(inserted);
-        // Membership check does not alter the stored value's casing (case-sensitivity
-        // asymmetry is a separately-deferred change).
+        // A correctly-cased canonical role round-trips unchanged.
         Assert.Equal(Roles.Deployer, inserted!.Role);
         Assert.Equal("SCADA-Deploy", inserted.LdapGroupName);
     }
+
+    [Fact]
+    public void CreateRoleMapping_LowercaseRole_StoresCanonicalCasing()
+    {
+        // arch-review C5: ASP.NET RequireClaim policies compare Role ordinal
+        // case-sensitively while the actor compares OrdinalIgnoreCase. A row stored
+        // as "deployer" would pass every actor gate but fail every UI policy (a
+        // CLI-works/UI-403s split-brain). The write path now canonicalizes to the
+        // Roles.All casing, so the stored value is always "Deployer".
+        LdapGroupMapping? inserted = null;
+        _securityRepo
+            .When(r => r.AddMappingAsync(Arg.Any<LdapGroupMapping>(), Arg.Any<CancellationToken>()))
+            .Do(ci => inserted = ci.Arg<LdapGroupMapping>());
+
+        var actor = CreateActor();
+        actor.Tell(Envelope(new CreateRoleMappingCommand("SCADA-Deploy", "deployer")));
+
+        ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+        Assert.NotNull(inserted);
+        Assert.Equal("Deployer", inserted!.Role); // canonical, not the verbatim "deployer"
+    }
+
+    [Fact]
+    public void UpdateRoleMapping_LowercaseRole_StoresCanonicalCasing()
+    {
+        var existing = new LdapGroupMapping("SCADA-Deploy", Roles.Viewer) { Id = 7 };
+        _securityRepo.GetMappingByIdAsync(7, Arg.Any<CancellationToken>()).Returns(existing);
+        LdapGroupMapping? updated = null;
+        _securityRepo
+            .When(r => r.UpdateMappingAsync(Arg.Any<LdapGroupMapping>(), Arg.Any<CancellationToken>()))
+            .Do(ci => updated = ci.Arg<LdapGroupMapping>());
+
+        var actor = CreateActor();
+        actor.Tell(Envelope(new UpdateRoleMappingCommand(7, "SCADA-Deploy", "deployer")));
+
+        ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+        Assert.NotNull(updated);
+        Assert.Equal("Deployer", updated!.Role);
+    }
 }