refactor(acl): NodeAcl.LdapGroup -> Principal, with the proc that reads it (#520)

Stage 1a of the API-key ACL design. Behaviour-neutral: the trie walker already
matched an opaque string, so only what produces that string changes. No
enforcement is wired by this commit and none of the two live role gates move.

Renamed through the ACL model: NodeAcl.Principal, TrieGrant.Principal,
MatchedGrant.Principal, AuthorizationDecision.Principal,
UserAuthorizationState.Principals, and PermissionTrie.CollectMatches's parameter.
LdapGroupRoleMapping is a DIFFERENT entity that also has an LdapGroup column and
is deliberately untouched — it backs AdminUI login, which keeps LDAP.

The migration is the interesting part. EF scaffolds RenameColumn/RenameIndex, so
rows survive — but it cannot know that dbo.sp_ComputeGenerationDiff checksums
NodeAcl.LdapGroup. That proc is created in V3Initial.StoredProcedures and never
redefined since, and SQL Server does not validate a procedure body when a column
is renamed out from under it: the object keeps reporting healthy and fails at
CALL time with Invalid column name, silently breaking config-drift detection.
Verified against the live dev DB that the deployed proc really does reference the
column. The migration therefore re-emits the proc, as one parameterised template
shared by Up and Down so the two cannot drift.

Prose that describes what is enforced TODAY (fleet-wide LDAP group -> role) is
left alone, because it is still accurate until stage 2 lands.

UserAuthorizationState's refresh machinery is retained unchanged but now carries
a note that it is LDAP-shaped and largely vestigial here: an API-key principal
set cannot change out of band, since keys and ACL rows both ride the deployment
artifact, so the generation stamp rather than the freshness timer is the real
invalidation signal.

41 authorization tests pass; solution builds clean.

Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
This commit is contained in:
Joseph Doherty
2026-07-31 12:17:57 -04:00
parent 804c32e5c0
commit 4d376f727f
16 changed files with 1729 additions and 52 deletions
@@ -3,7 +3,7 @@ using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
namespace ZB.MOM.WW.OtOpcUa.Configuration.Entities;
/// <summary>
/// One ACL grant: an LDAP group gets a set of <see cref="NodePermissions"/> at a specific scope.
/// One ACL grant: a principal gets a set of <see cref="NodePermissions"/> at a specific scope.
/// Generation-versioned per decision. See <c>acl-design.md</c> for evaluation algorithm.
/// </summary>
public sealed class NodeAcl
@@ -17,8 +17,8 @@ public sealed class NodeAcl
/// <summary>Gets or sets the cluster ID for this ACL entry.</summary>
public required string ClusterId { get; set; }
/// <summary>Gets or sets the LDAP group for this ACL entry.</summary>
public required string LdapGroup { get; set; }
/// <summary>Gets or sets the principal (API-key principal name, or the reserved <c>anonymous</c>) this ACL entry grants to.</summary>
public required string Principal { get; set; }
/// <summary>Gets or sets the scope kind for this ACL entry.</summary>
public required NodeAclScopeKind ScopeKind { get; set; }
@@ -0,0 +1,104 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace ZB.MOM.WW.OtOpcUa.Configuration.Migrations
{
/// <summary>
/// #520 — <c>NodeAcl.LdapGroup</c> becomes <c>NodeAcl.Principal</c>. The OPC UA data plane is
/// machine-to-machine, so an ACL grant keys on an API-key principal (or the reserved
/// <c>anonymous</c>), not on an LDAP group. EF scaffolds this as RenameColumn/RenameIndex, so
/// existing rows survive.
/// </summary>
/// <remarks>
/// ⚠️ The column is also read by <c>dbo.sp_ComputeGenerationDiff</c>, created in
/// <c>V3Initial.StoredProcedures</c> and never redefined since. A bare column rename leaves that
/// proc referencing a column that no longer exists — SQL Server does not validate a procedure
/// body at rename time, so the object keeps reporting healthy and fails at CALL time with
/// <c>Invalid column name 'LdapGroup'</c>, silently breaking config-drift detection. The proc is
/// therefore re-emitted below in the same migration, body otherwise byte-identical. If you add or
/// rename a column on any table that proc checksums, it needs the same treatment.
/// </remarks>
public partial class RenameNodeAclLdapGroupToPrincipal : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.RenameColumn(
name: "LdapGroup",
table: "NodeAcl",
newName: "Principal");
migrationBuilder.RenameIndex(
name: "UX_NodeAcl_GroupScope",
table: "NodeAcl",
newName: "UX_NodeAcl_PrincipalScope");
migrationBuilder.RenameIndex(
name: "IX_NodeAcl_Group",
table: "NodeAcl",
newName: "IX_NodeAcl_Principal");
migrationBuilder.Sql(ComputeGenerationDiff("Principal"));
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.RenameColumn(
name: "Principal",
table: "NodeAcl",
newName: "LdapGroup");
migrationBuilder.RenameIndex(
name: "UX_NodeAcl_PrincipalScope",
table: "NodeAcl",
newName: "UX_NodeAcl_GroupScope");
migrationBuilder.RenameIndex(
name: "IX_NodeAcl_Principal",
table: "NodeAcl",
newName: "IX_NodeAcl_Group");
migrationBuilder.Sql(ComputeGenerationDiff("LdapGroup"));
}
/// <summary>
/// <c>dbo.sp_ComputeGenerationDiff</c> re-emitted with the NodeAcl grantee column named by
/// <paramref name="granteeColumn"/>. Kept as one parameterised template so Up and Down cannot
/// drift from each other, and so the diff shows exactly one token changing.
/// </summary>
/// <param name="granteeColumn">The NodeAcl grantee column name to checksum.</param>
/// <returns>The <c>CREATE OR ALTER PROCEDURE</c> statement.</returns>
private static string ComputeGenerationDiff(string granteeColumn) => $@"
CREATE OR ALTER PROCEDURE dbo.sp_ComputeGenerationDiff
@FromGenerationId bigint,
@ToGenerationId bigint
AS
BEGIN
SET NOCOUNT ON;
-- @FromGenerationId / @ToGenerationId are ignored: the v3 config tables are single live-state
-- (no GenerationId column). Emit a current-state signature per row so the shape (TableName,
-- LogicalId, Sig) still round-trips through any legacy caller.
SELECT TableName, LogicalId, Sig FROM (
SELECT 'RawFolder' AS TableName, RawFolderId AS LogicalId, CHECKSUM(ClusterId, ParentRawFolderId, Name, SortOrder) AS Sig FROM dbo.RawFolder
UNION ALL
SELECT 'DriverInstance', DriverInstanceId, CHECKSUM(ClusterId, RawFolderId, Name, DriverType, Enabled, CONVERT(varchar(max), DriverConfig)) FROM dbo.DriverInstance
UNION ALL
SELECT 'Device', DeviceId, CHECKSUM(DriverInstanceId, Name, Enabled, CONVERT(varchar(max), DeviceConfig)) FROM dbo.Device
UNION ALL
SELECT 'TagGroup', TagGroupId, CHECKSUM(DeviceId, ParentTagGroupId, Name, SortOrder) FROM dbo.TagGroup
UNION ALL
SELECT 'Tag', TagId, CHECKSUM(DeviceId, TagGroupId, PollGroupId, Name, DataType, AccessLevel, WriteIdempotent, CONVERT(varchar(max), TagConfig)) FROM dbo.Tag
UNION ALL
SELECT 'Equipment', EquipmentId, CHECKSUM(EquipmentUuid, UnsLineId, Name, MachineCode, ZTag, SAPID, EquipmentClassRef, Manufacturer, Model, SerialNumber) FROM dbo.Equipment
UNION ALL
SELECT 'UnsTagReference', UnsTagReferenceId, CHECKSUM(EquipmentId, TagId, DisplayNameOverride, SortOrder) FROM dbo.UnsTagReference
UNION ALL
SELECT 'NodeAcl', NodeAclId, CHECKSUM(ClusterId, {granteeColumn}, ScopeKind, ScopeId, PermissionFlags, Notes) FROM dbo.NodeAcl
) AS inventory;
END
";
}
}
@@ -709,11 +709,6 @@ namespace ZB.MOM.WW.OtOpcUa.Configuration.Migrations
.HasMaxLength(64)
.HasColumnType("nvarchar(64)");
b.Property<string>("LdapGroup")
.IsRequired()
.HasMaxLength(256)
.HasColumnType("nvarchar(256)");
b.Property<string>("NodeAclId")
.HasMaxLength(64)
.HasColumnType("nvarchar(64)");
@@ -725,6 +720,11 @@ namespace ZB.MOM.WW.OtOpcUa.Configuration.Migrations
b.Property<int>("PermissionFlags")
.HasColumnType("int");
b.Property<string>("Principal")
.IsRequired()
.HasMaxLength(256)
.HasColumnType("nvarchar(256)");
b.Property<byte[]>("RowVersion")
.IsConcurrencyToken()
.IsRequired()
@@ -745,21 +745,21 @@ namespace ZB.MOM.WW.OtOpcUa.Configuration.Migrations
b.HasIndex("ClusterId")
.HasDatabaseName("IX_NodeAcl_Cluster");
b.HasIndex("LdapGroup")
.HasDatabaseName("IX_NodeAcl_Group");
b.HasIndex("NodeAclId")
.IsUnique()
.HasDatabaseName("UX_NodeAcl_LogicalId")
.HasFilter("[NodeAclId] IS NOT NULL");
b.HasIndex("Principal")
.HasDatabaseName("IX_NodeAcl_Principal");
b.HasIndex("ScopeKind", "ScopeId")
.HasDatabaseName("IX_NodeAcl_Scope")
.HasFilter("[ScopeId] IS NOT NULL");
b.HasIndex("ClusterId", "LdapGroup", "ScopeKind", "ScopeId")
b.HasIndex("ClusterId", "Principal", "ScopeKind", "ScopeId")
.IsUnique()
.HasDatabaseName("UX_NodeAcl_GroupScope")
.HasDatabaseName("UX_NodeAcl_PrincipalScope")
.HasFilter("[ScopeId] IS NOT NULL");
b.ToTable("NodeAcl", (string)null);
@@ -449,7 +449,7 @@ public sealed class OtOpcUaConfigDbContext(DbContextOptions<OtOpcUaConfigDbConte
e.Property(x => x.NodeAclRowId).HasDefaultValueSql("NEWSEQUENTIALID()");
e.Property(x => x.NodeAclId).HasMaxLength(64);
e.Property(x => x.ClusterId).HasMaxLength(64);
e.Property(x => x.LdapGroup).HasMaxLength(256);
e.Property(x => x.Principal).HasMaxLength(256);
e.Property(x => x.ScopeKind).HasConversion<string>().HasMaxLength(16);
e.Property(x => x.ScopeId).HasMaxLength(64);
e.Property(x => x.PermissionFlags).HasConversion<int>();
@@ -457,13 +457,13 @@ public sealed class OtOpcUaConfigDbContext(DbContextOptions<OtOpcUaConfigDbConte
e.Property(x => x.RowVersion).IsRowVersion();
e.HasIndex(x => x.ClusterId).HasDatabaseName("IX_NodeAcl_Cluster");
e.HasIndex(x => x.LdapGroup).HasDatabaseName("IX_NodeAcl_Group");
e.HasIndex(x => x.Principal).HasDatabaseName("IX_NodeAcl_Principal");
e.HasIndex(x => new { x.ScopeKind, x.ScopeId })
.HasFilter("[ScopeId] IS NOT NULL")
.HasDatabaseName("IX_NodeAcl_Scope");
e.HasIndex(x => x.NodeAclId).IsUnique().HasDatabaseName("UX_NodeAcl_LogicalId");
e.HasIndex(x => new { x.ClusterId, x.LdapGroup, x.ScopeKind, x.ScopeId }).IsUnique()
.HasDatabaseName("UX_NodeAcl_GroupScope");
e.HasIndex(x => new { x.ClusterId, x.Principal, x.ScopeKind, x.ScopeId }).IsUnique()
.HasDatabaseName("UX_NodeAcl_PrincipalScope");
});
}
@@ -43,10 +43,10 @@ public enum AuthorizationVerdict
}
/// <summary>One grant that contributed to an Allow verdict — for audit / UI diagnostics.</summary>
/// <param name="LdapGroup">LDAP group the matched grant belongs to.</param>
/// <param name="Principal">Principal the matched grant belongs to.</param>
/// <param name="Scope">Where in the hierarchy the grant was anchored.</param>
/// <param name="PermissionFlags">The bitmask the grant contributed.</param>
public sealed record MatchedGrant(
string LdapGroup,
string Principal,
NodeAclScopeKind Scope,
NodePermissions PermissionFlags);
@@ -6,7 +6,7 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Authorization;
/// In-memory permission trie for one <c>(ClusterId, GenerationId)</c>. Walk from the cluster
/// root down through namespace → UNS levels → tag, OR-ing the
/// <see cref="TrieGrant.PermissionFlags"/> granted at each visited level for each of the session's
/// LDAP groups. The accumulated bitmask is compared to the permission required by the
/// principals. The accumulated bitmask is compared to the permission required by the
/// requested <see cref="Abstractions.OpcUaOperation"/>.
/// </summary>
/// <remarks>
@@ -28,18 +28,18 @@ public sealed class PermissionTrie
/// <summary>
/// Walk the trie collecting grants that apply to <paramref name="scope"/> for any of the
/// session's <paramref name="ldapGroups"/>. Returns the matched-grant list; the caller
/// session's <paramref name="principals"/>. Returns the matched-grant list; the caller
/// OR-s the flag bits to decide whether the requested permission is carried.
/// </summary>
/// <param name="scope">The node scope to match permissions for.</param>
/// <param name="ldapGroups">The user's LDAP group memberships.</param>
/// <param name="principals">The principal names carried by the session (API-key principals, or the reserved "anonymous").</param>
/// <returns>The matched grants accumulated across every trie level visited for the scope.</returns>
public IReadOnlyList<MatchedGrant> CollectMatches(NodeScope scope, IEnumerable<string> ldapGroups)
public IReadOnlyList<MatchedGrant> CollectMatches(NodeScope scope, IEnumerable<string> principals)
{
ArgumentNullException.ThrowIfNull(scope);
ArgumentNullException.ThrowIfNull(ldapGroups);
ArgumentNullException.ThrowIfNull(principals);
var groups = ldapGroups.ToHashSet(StringComparer.OrdinalIgnoreCase);
var groups = principals.ToHashSet(StringComparer.OrdinalIgnoreCase);
if (groups.Count == 0) return [];
var matches = new List<MatchedGrant>();
@@ -83,8 +83,8 @@ public sealed class PermissionTrie
{
foreach (var grant in node.Grants)
{
if (groups.Contains(grant.LdapGroup))
matches.Add(new MatchedGrant(grant.LdapGroup, level, grant.PermissionFlags));
if (groups.Contains(grant.Principal))
matches.Add(new MatchedGrant(grant.Principal, level, grant.PermissionFlags));
}
}
}
@@ -104,4 +104,4 @@ public sealed class PermissionTrieNode
}
/// <summary>Projection of a <see cref="Configuration.Entities.NodeAcl"/> row into the trie.</summary>
public sealed record TrieGrant(string LdapGroup, NodePermissions PermissionFlags);
public sealed record TrieGrant(string Principal, NodePermissions PermissionFlags);
@@ -57,7 +57,7 @@ public static class PermissionTrieBuilder
foreach (var row in rows)
{
if (!string.Equals(row.ClusterId, clusterId, StringComparison.OrdinalIgnoreCase)) continue;
var grant = new TrieGrant(row.LdapGroup, row.PermissionFlags);
var grant = new TrieGrant(row.Principal, row.PermissionFlags);
var node = row.ScopeKind switch
{
@@ -48,7 +48,7 @@ public sealed class TriePermissionEvaluator : IPermissionEvaluator
if (trie is null) return AuthorizationDecision.NotGranted();
}
var matches = trie.CollectMatches(scope, session.LdapGroups);
var matches = trie.CollectMatches(scope, session.Principals);
if (matches.Count == 0) return AuthorizationDecision.NotGranted();
var required = MapOperationToPermission(operation);
@@ -2,14 +2,25 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Authorization;
/// <summary>
/// Per-session authorization state cached on the OPC UA session object + keyed on the
/// session id. Captures the LDAP group memberships resolved at sign-in, the generation
/// the membership was resolved against, and the bounded freshness window.
/// session id. Captures the principals resolved at session activation, the generation
/// they were resolved against, and the bounded freshness window.
/// </summary>
/// <remarks>
/// ⚠️ <b>The refresh machinery below is LDAP-shaped and is largely vestigial under the
/// API-key model (#520).</b> It exists because an LDAP group membership can change
/// out-of-band in the directory, so a long-lived session had to re-resolve periodically.
/// An API-key principal set cannot change out-of-band: keys and ACL rows both ride the
/// deployment artifact (driver-only nodes have no ConfigDb since Phase 4), so the only
/// thing that can change a session's grants is a new deploy — which already bumps
/// <see cref="AuthGenerationId"/>. The generation stamp, not the freshness timer, is the
/// real invalidation signal. Revisit whether
/// <see cref="MembershipFreshnessInterval"/> / <see cref="NeedsRefresh"/> earn their keep
/// when the session lifecycle lands (design doc stage 4); they are retained unchanged here
/// so this rename stays behaviour-neutral.
///
/// The membership is bounded by <see cref="MembershipFreshnessInterval"/>
/// (default 5 min). After that, the next hot-path authz call re-resolves LDAP group
/// memberships; failure to re-resolve (LDAP unreachable) flips the session to fail-closed
/// until a refresh succeeds.
/// (default 5 min). After that, the next hot-path authz call re-resolves the principal
/// set; failure to re-resolve flips the session to fail-closed until a refresh succeeds.
///
/// <see cref="AuthCacheMaxStaleness"/> (default 15 min) is separate from
/// Phase 6.1's availability-oriented 24h cache — beyond this window the evaluator returns
@@ -30,12 +41,14 @@ public sealed record UserAuthorizationState
public required string ClusterId { get; init; }
/// <summary>
/// LDAP groups the user is a member of as resolved at sign-in / last membership refresh.
/// Principals the session carries, as resolved at session activation: the principal names
/// attached to the presented API key, or the single reserved <c>anonymous</c> principal for
/// an anonymous session when anonymous access is enabled.
/// Case comparison is handled downstream by the evaluator (OrdinalIgnoreCase).
/// </summary>
public required IReadOnlyList<string> LdapGroups { get; init; }
public required IReadOnlyList<string> Principals { get; init; }
/// <summary>Timestamp when <see cref="LdapGroups"/> was last resolved from the directory.</summary>
/// <summary>Timestamp when <see cref="Principals"/> was last resolved.</summary>
public required DateTime MembershipResolvedUtc { get; init; }
/// <summary>
@@ -52,8 +52,8 @@ else
<InputText id="aid" @bind-Value="_form.NodeAclId" disabled="@(!IsNew)" class="form-control form-control-sm mono" />
</div>
<div class="col-md-6 mb-3">
<label class="form-label" for="grp">LDAP group</label>
<InputText id="grp" @bind-Value="_form.LdapGroup" class="form-control form-control-sm mono"
<label class="form-label" for="grp">Principal</label>
<InputText id="grp" @bind-Value="_form.Principal" class="form-control form-control-sm mono"
placeholder="cn=Operators,ou=FleetAdmin,dc=zb,dc=local" />
</div>
</div>
@@ -150,7 +150,7 @@ else
_form = new FormModel
{
NodeAclId = _existing.NodeAclId,
LdapGroup = _existing.LdapGroup,
Principal = _existing.Principal,
ScopeKind = _existing.ScopeKind,
ScopeId = _existing.ScopeId,
PermissionFlags = _existing.PermissionFlags,
@@ -176,7 +176,7 @@ else
{
NodeAclId = _form.NodeAclId,
ClusterId = ClusterId,
LdapGroup = _form.LdapGroup,
Principal = _form.Principal,
ScopeKind = _form.ScopeKind,
ScopeId = string.IsNullOrWhiteSpace(_form.ScopeId) ? null : _form.ScopeId,
PermissionFlags = _form.PermissionFlags,
@@ -188,7 +188,7 @@ else
var entity = await db.NodeAcls.FirstOrDefaultAsync(a => a.ClusterId == ClusterId && a.NodeAclId == NodeAclId);
if (entity is null) { _error = "Row no longer exists."; return; }
db.Entry(entity).Property(e => e.RowVersion).OriginalValue = _form.RowVersion;
entity.LdapGroup = _form.LdapGroup;
entity.Principal = _form.Principal;
entity.ScopeKind = _form.ScopeKind;
entity.ScopeId = string.IsNullOrWhiteSpace(_form.ScopeId) ? null : _form.ScopeId;
entity.PermissionFlags = _form.PermissionFlags;
@@ -224,7 +224,7 @@ else
private sealed class FormModel
{
[Required, RegularExpression("^[A-Za-z0-9_-]+$")] public string NodeAclId { get; set; } = "";
[Required] public string LdapGroup { get; set; } = "";
[Required] public string Principal { get; set; } = "";
public NodeAclScopeKind ScopeKind { get; set; } = NodeAclScopeKind.Cluster;
public string? ScopeId { get; set; }
public NodePermissions PermissionFlags { get; set; } = NodePermissions.None;
@@ -33,7 +33,7 @@ else
</div>
<section class="panel notice rise" style="animation-delay:.02s">
ACL rows grant LDAP groups specific <span class="mono">NodePermissions</span> on a scope
ACL rows grant principals specific <span class="mono">NodePermissions</span> on a scope
(a folder, an equipment, a tag). Per-cluster role grants were dropped in favour of
fleet-wide LDAP-group → role mapping; ACLs here are the finer-grained per-node scope.
</section>
@@ -51,7 +51,7 @@ else
<thead>
<tr>
<th>NodeAclId</th>
<th>LDAP group</th>
<th>Principal</th>
<th>Scope</th>
<th>Scope target</th>
<th>Permissions</th>
@@ -64,7 +64,7 @@ else
{
<tr>
<td><span class="mono small">@a.NodeAclId</span></td>
<td><span class="mono">@a.LdapGroup</span></td>
<td><span class="mono">@a.Principal</span></td>
<td>@a.ScopeKind</td>
<td><span class="mono small">@(a.ScopeId ?? "—")</span></td>
<td>
@@ -29,7 +29,7 @@ public sealed class PermissionTrieBuilderTests
NodeAclRowId = Guid.NewGuid(),
NodeAclId = $"acl-{Guid.NewGuid():N}",
ClusterId = clusterId,
LdapGroup = group,
Principal = group,
ScopeKind = scope,
ScopeId = scopeId,
PermissionFlags = flags,
@@ -15,7 +15,7 @@ public sealed class PermissionTrieTests
NodeAclRowId = Guid.NewGuid(),
NodeAclId = $"acl-{Guid.NewGuid():N}",
ClusterId = clusterId,
LdapGroup = group,
Principal = group,
ScopeKind = scope,
ScopeId = scopeId,
PermissionFlags = flags,
@@ -28,7 +28,7 @@ public sealed class TriePermissionEvaluatorTests
NodeAclRowId = Guid.NewGuid(),
NodeAclId = $"acl-{Guid.NewGuid():N}",
ClusterId = "c1",
LdapGroup = group,
Principal = group,
ScopeKind = scope,
ScopeId = scopeId,
PermissionFlags = flags,
@@ -40,7 +40,7 @@ public sealed class TriePermissionEvaluatorTests
{
SessionId = "sess",
ClusterId = clusterId,
LdapGroups = groups,
Principals = groups,
MembershipResolvedUtc = resolvedUtc ?? Now,
AuthGenerationId = authGenerationId,
MembershipVersion = 1,
@@ -13,7 +13,7 @@ public sealed class UserAuthorizationStateTests
{
SessionId = "s",
ClusterId = "c1",
LdapGroups = ["cn=ops"],
Principals = ["cn=ops"],
MembershipResolvedUtc = resolved,
AuthGenerationId = 1,
MembershipVersion = 1,