fix(audit): route SecuredWrite audit via ICentralAuditWriter for SourceNode stamping (#206)

2026-06-19 02:37:48 -04:00
parent fb18253f32
commit 69e0d546b5
3 changed files with 64 additions and 11 deletions
@@ -969,15 +969,25 @@ public class ManagementActor : ReceiveActor

    /// <summary>
    /// Best-effort emission of ONE secured-write AuditLog row via the central direct-write
-    /// path (<see cref="IAuditLogRepository.InsertIfNotExistsAsync"/>) — mirrors the
-    /// Notification Outbox / Inbound API central-origin pattern. The row is built through
-    /// the canonical <see cref="ScadaBridgeAuditEventFactory"/> so Action/Category/Outcome
-    /// map identically to every other emit site.
+    /// path (<see cref="ICentralAuditWriter.WriteAsync"/>) — mirrors the Notification
+    /// Outbox dispatch / Inbound API central-origin pattern. The row is built through the
+    /// canonical <see cref="ScadaBridgeAuditEventFactory"/> so Action/Category/Outcome map
+    /// identically to every other emit site.
    /// </summary>
    /// <remarks>
+    /// #206: routes through <see cref="ICentralAuditWriter"/> (rather than calling
+    /// <see cref="IAuditLogRepository.InsertIfNotExistsAsync"/> directly) so the writing
+    /// central node's <c>SourceNode</c> (<c>central-a</c>/<c>central-b</c>) is stamped via
+    /// the writer's <see cref="INodeIdentityProvider"/> — satisfying the design's
+    /// node-of-origin invariant. The rows are otherwise unchanged (same channel/kind,
+    /// same <c>CorrelationId</c> = the row id, same payload).
+    /// <para>
    /// Standing audit invariant: an audit-write failure NEVER aborts the secured-write
-    /// action. Every exception (repository resolution OR the insert) is caught, logged at
-    /// warning, and swallowed — the caller's own success/failure path is authoritative.
+    /// action. <see cref="ICentralAuditWriter"/> is best-effort and swallows its own
+    /// internal failures; the surrounding try/catch is defensive belt-and-braces (it also
+    /// guards writer resolution) — either way the caller's own success/failure path is
+    /// authoritative.
+    /// </para>
    /// </remarks>
    private static async Task EmitSecuredWriteAuditAsync(
        IServiceProvider sp,
@@ -1006,9 +1016,11 @@ public class ManagementActor : ReceiveActor
                    verifierUser = row.VerifierUser
                }));

-            using var scope = sp.CreateScope();
-            var auditRepo = scope.ServiceProvider.GetRequiredService<IAuditLogRepository>();
-            await auditRepo.InsertIfNotExistsAsync(evt);
+            // #206: the central direct-write writer is a singleton that opens its own
+            // per-call scope for the (scoped) IAuditLogRepository and stamps SourceNode
+            // from the local INodeIdentityProvider — no scope needed here.
+            var auditWriter = sp.GetRequiredService<ICentralAuditWriter>();
+            await auditWriter.WriteAsync(evt);
        }
        catch (Exception ex)
        {
@@ -4,9 +4,11 @@ using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging.Abstractions;
 using Microsoft.Extensions.Options;
 using NSubstitute;
+using ZB.MOM.WW.ScadaBridge.AuditLog.Central;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.SecuredWrites;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
 using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
+using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
 using ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection;
 using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management;
 using ZB.MOM.WW.ScadaBridge.Commons.Types;
@@ -24,6 +26,14 @@ namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests;
 /// </summary>
 public class SecuredWriteHandlerTests : TestKit, IDisposable
 {
+    /// <summary>
+    /// The central node the test writer stamps onto every emitted row. #206 routes
+    /// SecuredWrite audit through <see cref="ICentralAuditWriter"/>, whose
+    /// <see cref="INodeIdentityProvider"/> stamps <c>SourceNode</c>; the handler tests
+    /// assert each lifecycle row carries this value (proving the stamping actually runs).
+    /// </summary>
+    private const string ExpectedSourceNode = "central-a";
+
    private readonly ISiteRepository _siteRepo;
    private readonly ISecuredWriteRepository _securedWriteRepo;
    private readonly IAuditLogRepository _auditRepo;
@@ -42,11 +52,28 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
        _services.AddScoped(_ => _securedWriteRepo);
        _services.AddScoped(_ => _auditRepo);
        _services.AddSingleton<CommunicationService>(_comms);
+
+        // #206: route SecuredWrite audit through the REAL CentralAuditWriter (the same
+        // central-direct-write path Notification Outbox dispatch + Inbound API use) so
+        // these tests exercise SourceNode stamping end-to-end — actor → writer → repo.
+        // The writer stamps SourceNode from the INodeIdentityProvider (central-a/central-b)
+        // and persists via the captured IAuditLogRepository, so the captured rows now
+        // carry a non-null SourceNode. Mirrors AuditLog.Tests CentralAuditWriterTests'
+        // wiring (real writer + substituted repo + node identity).
+        var nodeIdentity = Substitute.For<INodeIdentityProvider>();
+        nodeIdentity.NodeName.Returns(ExpectedSourceNode);
+        _services.AddSingleton<INodeIdentityProvider>(nodeIdentity);
+        _services.AddSingleton<ICentralAuditWriter>(sp => new CentralAuditWriter(
+            sp,
+            NullLogger<CentralAuditWriter>.Instance,
+            nodeIdentity: sp.GetRequiredService<INodeIdentityProvider>()));
    }

    /// <summary>
-    /// Captures every <see cref="AuditEvent"/> handed to the substituted
-    /// <see cref="IAuditLogRepository.InsertIfNotExistsAsync"/>. Audit emission is
+    /// Captures every <see cref="AuditEvent"/> that lands at the substituted
+    /// <see cref="IAuditLogRepository.InsertIfNotExistsAsync"/> — i.e. the row AFTER the
+    /// real <see cref="CentralAuditWriter"/> has stamped <c>SourceNode</c> (#206), so the
+    /// captured events carry the production node-of-origin value. Audit emission is
    /// best-effort and asynchronous off the actor thread, so a short await-condition
    /// poll lets the captured list settle before assertions.
    /// </summary>
@@ -615,6 +642,9 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
        Assert.Equal(CorrelationFor(55L), evt.CorrelationId);
        Assert.Equal("SecuredWrite", evt.Category);
        Assert.Equal("SITE1/Mx1/Tag.Setpoint", evt.Target);
+        // #206: the Submit row is stamped with the writing central node's identifier
+        // (it routed through the real CentralAuditWriter), not the legacy null SourceNode.
+        Assert.Equal(ExpectedSourceNode, evt.SourceNode);
        // Exactly one row — no stray duplicates.
        Assert.Single(captured);
    }
@@ -636,6 +666,8 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
        var evt = SingleOfKind(captured, AuditKind.SecuredWriteReject);
        Assert.Equal("bob", evt.Actor);
        Assert.Equal(CorrelationFor(7L), evt.CorrelationId);
+        // #206: Reject row carries the writing central node's identifier.
+        Assert.Equal(ExpectedSourceNode, evt.SourceNode);
        Assert.Single(captured);
    }

@@ -657,10 +689,13 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
        var approve = SingleOfKind(captured, AuditKind.SecuredWriteApprove);
        Assert.Equal("bob", approve.Actor);
        Assert.Equal(CorrelationFor(7L), approve.CorrelationId);
+        // #206: Approve + Execute rows both carry the writing central node's identifier.
+        Assert.Equal(ExpectedSourceNode, approve.SourceNode);

        var execute = SingleOfKind(captured, AuditKind.SecuredWriteExecute);
        Assert.Equal("bob", execute.Actor);
        Assert.Equal(CorrelationFor(7L), execute.CorrelationId);
+        Assert.Equal(ExpectedSourceNode, execute.SourceNode);
        // Delivered outcome → canonical Success.
        Assert.Equal(ZB.MOM.WW.Audit.AuditOutcome.Success, execute.Outcome);
        Assert.Equal(2, captured.Count);
@@ -677,6 +712,9 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
        _securedWriteRepo.AddAsync(Arg.Any<PendingSecuredWrite>(), Arg.Any<CancellationToken>())
            .Returns(55L);
        // Best-effort: a thrown audit insert must NOT abort the secured-write action.
+        // #206: the throw now happens behind the real CentralAuditWriter (which swallows
+        // it), so this still exercises the standing "audit failure never aborts the
+        // action" invariant — now through the central-direct-write path.
        _auditRepo.InsertIfNotExistsAsync(Arg.Any<AuditEvent>(), Arg.Any<CancellationToken>())
            .Returns(Task.FromException(new InvalidOperationException("audit db down")));

@@ -25,5 +25,8 @@
    <ProjectReference Include="../../src/ZB.MOM.WW.ScadaBridge.ManagementService/ZB.MOM.WW.ScadaBridge.ManagementService.csproj" />
    <ProjectReference Include="../../src/ZB.MOM.WW.ScadaBridge.Commons/ZB.MOM.WW.ScadaBridge.Commons.csproj" />
    <ProjectReference Include="../../src/ZB.MOM.WW.ScadaBridge.Security/ZB.MOM.WW.ScadaBridge.Security.csproj" />
+    <!-- #206: SecuredWrite audit now routes through the real CentralAuditWriter so the
+         handler tests verify SourceNode stamping end-to-end (actor → writer → repo). -->
+    <ProjectReference Include="../../src/ZB.MOM.WW.ScadaBridge.AuditLog/ZB.MOM.WW.ScadaBridge.AuditLog.csproj" />
  </ItemGroup>
 </Project>