feat(audit): OtOpcUa ConfigAuditLog.Outcome column + migration + ClusterAudit visibility fix (Task 2.2)

Persist the canonical AuditOutcome and make structured audit rows visible. - ConfigAuditLog gains a nullable Outcome column, stored as the AuditOutcome enum member name (nvarchar(16), mirroring how AdminRole is persisted). The AuditWriterActor flush now writes Outcome = evt.Outcome.ToString(). Nullable so legacy rows and the bespoke stored-procedure path (no derived outcome) write NULL. - Migration 20260602135350_AddConfigAuditLogOutcome: additive nullable column, no backfill. Up adds the column, Down drops it. Chains after 20260602112419_CanonicalizeAdminRoles; `dotnet ef migrations has-pending-model-changes` is clean. - ClusterAudit visibility fix: the page filtered solely on ClusterId, but the structured AuditWriterActor path stamps NodeId (ClusterId null), so those rows were invisible. Extracted ClusterAuditQuery.ForClusterAsync (shared by the page and tests) which ORs in rows whose NodeId belongs to a node in the cluster — membership resolved from ClusterNode (NodeId -> ClusterId). SP-path ClusterId-stamped rows still match. Tests: ControlPlane 45/45 (adds Outcome persistence + Denied-outcome asserts); new Configuration ClusterAuditQueryTests 3/3 (both-paths visible, other-cluster excluded, page-size cap); AdminUI 121/121. Configuration Unit suite is green on a clean run (a pre-existing timing flake in ResilientConfigReaderTests, untouched here, occasionally fails under parallel load and passes in isolation).
2026-06-02 09:59:22 -04:00
parent 933dd1a874
commit b7f5e887ee
10 changed files with 1986 additions and 5 deletions
@@ -41,4 +41,10 @@ public sealed class ConfigAuditLog
    /// <summary>Correlation ID from <c>AuditEvent.CorrelationId</c> so an audit row joins to its
    /// originating request/workflow. Nullable for the same backfill reason as <see cref="EventId"/>.</summary>
    public Guid? CorrelationId { get; set; }
+
+    /// <summary>Normalized outcome from <c>AuditEvent.Outcome</c> (the canonical
+    /// <c>ZB.MOM.WW.Audit.AuditOutcome</c>: <c>Success</c> | <c>Failure</c> | <c>Denied</c>),
+    /// stored as its enum member name. Nullable so pre-Outcome rows backfill cleanly and the
+    /// bespoke stored-procedure audit path (which does not derive an outcome) writes NULL.</summary>
+    public string? Outcome { get; set; }
 }
@@ -0,0 +1,35 @@
+using Microsoft.EntityFrameworkCore.Migrations;
+
+#nullable disable
+
+namespace ZB.MOM.WW.OtOpcUa.Configuration.Migrations
+{
+    /// <summary>
+    ///     Task 2.2 — adds the nullable <c>Outcome</c> column to <c>ConfigAuditLog</c> for the
+    ///     canonical <c>ZB.MOM.WW.Audit.AuditOutcome</c> (stored as its enum member name,
+    ///     <c>nvarchar(16)</c>, mirroring how <c>AdminRole</c> is persisted). Purely additive:
+    ///     nullable with no backfill, so existing rows and the bespoke stored-procedure audit
+    ///     path (which does not derive an outcome) keep writing NULL.
+    /// </summary>
+    public partial class AddConfigAuditLogOutcome : Migration
+    {
+        /// <inheritdoc />
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.AddColumn<string>(
+                name: "Outcome",
+                table: "ConfigAuditLog",
+                type: "nvarchar(16)",
+                maxLength: 16,
+                nullable: true);
+        }
+
+        /// <inheritdoc />
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.DropColumn(
+                name: "Outcome",
+                table: "ConfigAuditLog");
+        }
+    }
+}
@@ -186,6 +186,10 @@ namespace ZB.MOM.WW.OtOpcUa.Configuration.Migrations
                        .HasMaxLength(64)
                        .HasColumnType("nvarchar(64)");

+                    b.Property<string>("Outcome")
+                        .HasMaxLength(16)
+                        .HasColumnType("nvarchar(16)");
+
                    b.Property<string>("Principal")
                        .IsRequired()
                        .HasMaxLength(128)
@@ -445,6 +445,9 @@ public sealed class OtOpcUaConfigDbContext(DbContextOptions<OtOpcUaConfigDbConte
            e.Property(x => x.DetailsJson).HasColumnType("nvarchar(max)");
            e.Property(x => x.EventId);
            e.Property(x => x.CorrelationId);
+            // Stored as the AuditOutcome enum member name (mirrors AdminRole's string storage):
+            // "Success" | "Failure" | "Denied" all fit nvarchar(16). Nullable for legacy + SP-path rows.
+            e.Property(x => x.Outcome).HasMaxLength(16);

            e.HasIndex(x => new { x.ClusterId, x.Timestamp })
                .IsDescending(false, true)
@@ -0,0 +1,45 @@
+using Microsoft.EntityFrameworkCore;
+using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
+
+namespace ZB.MOM.WW.OtOpcUa.Configuration.Queries;
+
+/// <summary>
+/// Shared query for the cluster-scoped audit view. Audit rows reach <c>ConfigAuditLog</c> by two
+/// paths that stamp different columns:
+/// <list type="bullet">
+/// <item>the bespoke stored-procedure path stamps <c>ClusterId</c> directly;</item>
+/// <item>the structured <c>AuditWriterActor</c> path stamps <c>NodeId</c> (leaving
+/// <c>ClusterId</c> null).</item>
+/// </list>
+/// A cluster-scoped view must surface both, so this query matches rows whose <c>ClusterId</c>
+/// equals the cluster <em>or</em> whose <c>NodeId</c> belongs to a node in the cluster
+/// (membership from <see cref="ClusterNode"/>: <c>NodeId → ClusterId</c>).
+/// </summary>
+public static class ClusterAuditQuery
+{
+    /// <summary>
+    /// Returns the newest <paramref name="pageSize"/> audit rows visible for
+    /// <paramref name="clusterId"/>, newest first. Executes one query to resolve the cluster's
+    /// node IDs, then one filtered query against <c>ConfigAuditLog</c>.
+    /// </summary>
+    /// <param name="db">The config database context.</param>
+    /// <param name="clusterId">The cluster whose audit rows to fetch.</param>
+    /// <param name="pageSize">Maximum number of rows to return.</param>
+    /// <param name="ct">Cancellation token.</param>
+    /// <returns>The matching audit rows, newest first.</returns>
+    public static async Task<List<ConfigAuditLog>> ForClusterAsync(
+        OtOpcUaConfigDbContext db, string clusterId, int pageSize, CancellationToken ct = default)
+    {
+        var nodeIds = await db.ClusterNodes.AsNoTracking()
+            .Where(n => n.ClusterId == clusterId)
+            .Select(n => n.NodeId)
+            .ToListAsync(ct);
+
+        return await db.ConfigAuditLogs.AsNoTracking()
+            .Where(a => a.ClusterId == clusterId
+                || (a.ClusterId == null && a.NodeId != null && nodeIds.Contains(a.NodeId)))
+            .OrderByDescending(a => a.Timestamp)
+            .Take(pageSize)
+            .ToListAsync(ct);
+    }
+}
@@ -4,6 +4,7 @@
@using Microsoft.EntityFrameworkCore
@using ZB.MOM.WW.OtOpcUa.Configuration
@using ZB.MOM.WW.OtOpcUa.Configuration.Entities
+@using ZB.MOM.WW.OtOpcUa.Configuration.Queries
@inject IDbContextFactory<OtOpcUaConfigDbContext> DbFactory

 <div class="d-flex justify-content-between align-items-center mb-3">
@@ -74,10 +75,8 @@ else
    protected override async Task OnInitializedAsync()
    {
        await using var db = await DbFactory.CreateDbContextAsync();
-        _rows = await db.ConfigAuditLogs.AsNoTracking()
-            .Where(a => a.ClusterId == ClusterId)
-            .OrderByDescending(a => a.Timestamp)
-            .Take(PageSize)
-            .ToListAsync();
+        // Shared query: matches both the SP path (stamps ClusterId) and the structured
+        // AuditWriterActor path (stamps NodeId, ClusterId null) so the latter's rows are visible.
+        _rows = await ClusterAuditQuery.ForClusterAsync(db, ClusterId, PageSize);
    }
 }
@@ -103,6 +103,7 @@ public sealed class AuditWriterActor : ReceiveActor, IWithTimers, IAuditWriter
                    DetailsJson = evt.DetailsJson,
                    EventId = evt.EventId,
                    CorrelationId = evt.CorrelationId,
+                    Outcome = evt.Outcome.ToString(),
                });
            }
            db.SaveChanges();
@@ -0,0 +1,108 @@
+using Microsoft.EntityFrameworkCore;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
+using ZB.MOM.WW.OtOpcUa.Configuration.Queries;
+
+namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests;
+
+/// <summary>
+/// Verifies <see cref="ClusterAuditQuery.ForClusterAsync"/> — the cluster-scoped audit view used
+/// by the AdminUI ClusterAudit page. The structured AuditWriterActor path stamps NodeId (not
+/// ClusterId), so before the Task 2.2 fix those rows were invisible to a cluster filtered only on
+/// ClusterId. These tests pin the OR-predicate that joins NodeId back to its cluster.
+/// </summary>
+[Trait("Category", "Unit")]
+public sealed class ClusterAuditQueryTests : IDisposable
+{
+    private readonly OtOpcUaConfigDbContext _db;
+
+    /// <summary>Initializes a new instance with a fresh in-memory config database.</summary>
+    public ClusterAuditQueryTests()
+    {
+        var options = new DbContextOptionsBuilder<OtOpcUaConfigDbContext>()
+            .UseInMemoryDatabase($"cluster-audit-{Guid.NewGuid():N}")
+            .Options;
+        _db = new OtOpcUaConfigDbContext(options);
+    }
+
+    /// <summary>Disposes the database context.</summary>
+    public void Dispose() => _db.Dispose();
+
+    private void SeedNode(string clusterId, string nodeId) =>
+        _db.ClusterNodes.Add(new ClusterNode
+        {
+            NodeId = nodeId,
+            ClusterId = clusterId,
+            Host = $"{nodeId}.local",
+            ApplicationUri = $"urn:{nodeId}",
+            CreatedBy = "test",
+        });
+
+    private void SeedAudit(string eventType, string? clusterId, string? nodeId, DateTime ts) =>
+        _db.ConfigAuditLogs.Add(new ConfigAuditLog
+        {
+            Principal = "tester",
+            EventType = eventType,
+            ClusterId = clusterId,
+            NodeId = nodeId,
+            Timestamp = ts,
+        });
+
+    /// <summary>Structured rows (ClusterId null, NodeId set) for a node in the cluster are now
+    /// visible, alongside the SP-path rows that stamp ClusterId directly.</summary>
+    [Fact]
+    public async Task Surfaces_both_clusterId_rows_and_structured_nodeId_rows()
+    {
+        SeedNode("LINE3-OPCUA", "LINE3-OPCUA-A");
+        SeedNode("LINE3-OPCUA", "LINE3-OPCUA-B");
+        SeedNode("OTHER-CLUSTER", "OTHER-A");
+        var t0 = new DateTime(2026, 6, 2, 10, 0, 0, DateTimeKind.Utc);
+
+        // SP path: stamps ClusterId.
+        SeedAudit("Published", clusterId: "LINE3-OPCUA", nodeId: null, ts: t0);
+        // Structured AuditWriterActor path: stamps NodeId, ClusterId null — these were invisible.
+        SeedAudit("DraftEdited", clusterId: null, nodeId: "LINE3-OPCUA-A", ts: t0.AddMinutes(1));
+        SeedAudit("NodeApplied", clusterId: null, nodeId: "LINE3-OPCUA-B", ts: t0.AddMinutes(2));
+        // Noise that must NOT appear: other cluster's structured row + an orphan NodeId.
+        SeedAudit("Published", clusterId: null, nodeId: "OTHER-A", ts: t0.AddMinutes(3));
+        SeedAudit("Published", clusterId: null, nodeId: "UNKNOWN-NODE", ts: t0.AddMinutes(4));
+        await _db.SaveChangesAsync();
+
+        var rows = await ClusterAuditQuery.ForClusterAsync(_db, "LINE3-OPCUA", pageSize: 200);
+
+        rows.Select(r => r.EventType).ShouldBe(
+            ["NodeApplied", "DraftEdited", "Published"], // newest first
+            ignoreOrder: false);
+    }
+
+    /// <summary>An audit row stamped with another cluster's ClusterId never appears.</summary>
+    [Fact]
+    public async Task Does_not_surface_other_cluster_rows()
+    {
+        SeedNode("LINE3-OPCUA", "LINE3-OPCUA-A");
+        var t0 = new DateTime(2026, 6, 2, 10, 0, 0, DateTimeKind.Utc);
+        SeedAudit("Published", clusterId: "OTHER-CLUSTER", nodeId: null, ts: t0);
+        await _db.SaveChangesAsync();
+
+        var rows = await ClusterAuditQuery.ForClusterAsync(_db, "LINE3-OPCUA", pageSize: 200);
+
+        rows.ShouldBeEmpty();
+    }
+
+    /// <summary>Respects the page-size cap, newest first.</summary>
+    [Fact]
+    public async Task Caps_to_page_size_newest_first()
+    {
+        SeedNode("LINE3-OPCUA", "LINE3-OPCUA-A");
+        var t0 = new DateTime(2026, 6, 2, 10, 0, 0, DateTimeKind.Utc);
+        for (var i = 0; i < 5; i++)
+            SeedAudit("DraftEdited", clusterId: null, nodeId: "LINE3-OPCUA-A", ts: t0.AddMinutes(i));
+        await _db.SaveChangesAsync();
+
+        var rows = await ClusterAuditQuery.ForClusterAsync(_db, "LINE3-OPCUA", pageSize: 3);
+
+        rows.Count.ShouldBe(3);
+        rows.First().Timestamp.ShouldBe(t0.AddMinutes(4)); // newest
+    }
+}
@@ -172,6 +172,27 @@ public sealed class AuditWriterActorTests : ControlPlaneActorTestBase
        row.EventId.ShouldBe(eventId);
        row.EventType.ShouldBe("Config:Published");
        row.NodeId.ShouldBe("node-a");
+        // The derived canonical Outcome is persisted as its enum member name (Task 2.2 column).
+        row.Outcome.ShouldBe(nameof(AuditOutcome.Success));
+    }
+
+    /// <summary>Verifies that a Denied-outcome event persists "Denied" to the Outcome column.</summary>
+    [Fact]
+    public void Denied_outcome_is_persisted_as_its_enum_member_name()
+    {
+        var dbFactory = NewInMemoryDbFactory();
+        var actor = Sys.ActorOf(AuditWriterActor.Props(dbFactory));
+
+        actor.Tell(NewEvent(Guid.NewGuid(), action: "OpcUaAccessDenied"));
+
+        Watch(actor);
+        actor.Tell(PoisonPill.Instance);
+        ExpectTerminated(actor);
+
+        using var db = dbFactory.CreateDbContext();
+        var row = db.ConfigAuditLogs.Single();
+        row.Outcome.ShouldBe(nameof(AuditOutcome.Denied));
+        row.EventType.ShouldBe("Config:OpcUaAccessDenied");
    }

    /// <summary>Verifies the Outcome derivation table: config verbs → Success, the two