feat(audit): ScadaBridge IAuditActorAccessor + wire audit Actor from Auth principal at authenticated emit sites (Phase 3)

2026-06-02 15:33:01 -04:00
parent bc0e5bfd37
commit b3de8408fa
9 changed files with 463 additions and 30 deletions
@@ -0,0 +1,64 @@
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
+
+namespace ZB.MOM.WW.ScadaBridge.Commons.Tests.Types.Audit;
+
+/// <summary>
+/// Phase 3 (wire audit Actor from the Auth principal): the
+/// <see cref="ScadaBridgeAuditEventFactory"/> is the single construction point and
+/// records the <c>actor</c> the CALLER passes in — it never injects a principal of
+/// its own. These tests pin that contract so the per-emit-site decision holds:
+/// authenticated emit sites pass the principal's actor (sourced via
+/// <c>IAuditActorAccessor</c> at the inbound middleware), while system-originated
+/// emitters (notification / script DB-outbound) keep passing their own system/script
+/// actor unchanged. The factory does not blur the two.
+/// </summary>
+public class ScadaBridgeAuditEventFactoryActorTests
+{
+    [Theory]
+    // Mirrors the literal system actors the outbound emitters pass:
+    // NotificationOutboxActor → "system"; AuditingDbCommand → the source script.
+    [InlineData("system")]
+    [InlineData("order-sync.caspx")]
+    public void SystemOriginatedEmit_PreservesCallerActor_Verbatim(string systemActor)
+    {
+        var evt = ScadaBridgeAuditEventFactory.Create(
+            channel: AuditChannel.Notification,
+            kind: AuditKind.NotifyDeliver,
+            status: AuditStatus.Delivered,
+            actor: systemActor);
+
+        // The system emit keeps its system/script actor — the factory does not
+        // overwrite it with any authenticated principal.
+        Assert.Equal(systemActor, evt.Actor);
+    }
+
+    [Fact]
+    public void AuthenticatedEmit_PreservesCallerActor_Verbatim()
+    {
+        // An authenticated emit site (e.g. the inbound middleware) passes the
+        // principal's actor; the factory records it as-is.
+        var evt = ScadaBridgeAuditEventFactory.Create(
+            channel: AuditChannel.ApiInbound,
+            kind: AuditKind.InboundRequest,
+            status: AuditStatus.Delivered,
+            actor: "alice");
+
+        Assert.Equal("alice", evt.Actor);
+    }
+
+    [Fact]
+    public void NullActor_MapsToEmptyString_OnCanonicalRecord()
+    {
+        // The canonical AuditEvent.Actor is a non-null string; a null actor (no
+        // authenticated principal AND no system fallback supplied) maps to empty.
+        // AuditRowProjection then surfaces empty as null at the row boundary.
+        var evt = ScadaBridgeAuditEventFactory.Create(
+            channel: AuditChannel.ApiInbound,
+            kind: AuditKind.InboundAuthFailure,
+            status: AuditStatus.Failed,
+            actor: null);
+
+        Assert.Equal(string.Empty, evt.Actor);
+    }
+}
@@ -85,12 +85,26 @@ public class AuditWriteMiddlewareTests
    private static AuditWriteMiddleware CreateMiddleware(
        RequestDelegate next,
        ICentralAuditWriter writer,
-        AuditLogOptions? options = null) =>
+        AuditLogOptions? options = null,
+        IAuditActorAccessor? actorAccessor = null) =>
        new(
            next,
            writer,
            NullLogger<AuditWriteMiddleware>.Instance,
-            new StaticAuditLogOptionsMonitor(options ?? new AuditLogOptions()));
+            new StaticAuditLogOptionsMonitor(options ?? new AuditLogOptions()),
+            actorAccessor);
+
+    /// <summary>
+    /// File-local <see cref="IAuditActorAccessor"/> test double returning a fixed
+    /// actor string — stands in for the HTTP-backed accessor that reads the
+    /// authenticated principal off the ambient request (Phase 3).
+    /// </summary>
+    private sealed class StubAuditActorAccessor : IAuditActorAccessor
+    {
+        public StubAuditActorAccessor(string? currentActor) => CurrentActor = currentActor;
+
+        public string? CurrentActor { get; }
+    }

    /// <summary>
    /// File-local <see cref="IOptionsMonitor{TOptions}"/> test double — returns the
@@ -278,6 +292,105 @@ public class AuditWriteMiddlewareTests
        Assert.Equal("integration-svc", evt.Actor);
    }

+    // ---------------------------------------------------------------------
+    // 5b. Phase 3 — Actor from the authenticated principal. When no API-key
+    //     name was stashed, the actor is sourced from IAuditActorAccessor
+    //     (the authenticated interactive cookie/LDAP user). The API-key stash
+    //     still takes precedence, and auth-failure / no-principal paths stay
+    //     null — never echo an unauthenticated principal back.
+    // ---------------------------------------------------------------------
+
+    [Fact]
+    public async Task AuthenticatedUser_FromAccessor_RecordedAsActor_WhenNoApiKeyStash()
+    {
+        var writer = new RecordingAuditWriter();
+        var ctx = BuildContext();
+        var mw = CreateMiddleware(
+            _ =>
+            {
+                // No API-key name stashed — this is an interactive cookie/LDAP
+                // authenticated inbound user, surfaced via the accessor.
+                ctx.Response.StatusCode = 200;
+                return Task.CompletedTask;
+            },
+            writer,
+            actorAccessor: new StubAuditActorAccessor("alice"));
+
+        await mw.InvokeAsync(ctx);
+
+        var evt = Assert.Single(writer.Events);
+        Assert.Equal("alice", evt.Actor);
+    }
+
+    [Fact]
+    public async Task ApiKeyStash_TakesPrecedence_OverAuthenticatedPrincipal()
+    {
+        // A key-authenticated caller: the endpoint handler stashed the API key
+        // name. Even if an accessor would resolve a principal, the API-key
+        // identity is the canonical actor for the key-authenticated path.
+        var writer = new RecordingAuditWriter();
+        var ctx = BuildContext();
+        var mw = CreateMiddleware(
+            _ =>
+            {
+                ctx.Items[AuditWriteMiddleware.AuditActorItemKey] = "integration-svc";
+                ctx.Response.StatusCode = 200;
+                return Task.CompletedTask;
+            },
+            writer,
+            actorAccessor: new StubAuditActorAccessor("should-not-win"));
+
+        await mw.InvokeAsync(ctx);
+
+        var evt = Assert.Single(writer.Events);
+        Assert.Equal("integration-svc", evt.Actor);
+    }
+
+    [Fact]
+    public async Task AuthFailure_KeepsActorNull_EvenWhenAccessorResolvesPrincipal()
+    {
+        // 401/403 force the actor null BEFORE resolution — an auth failure must
+        // never echo a principal back, even one the accessor could produce.
+        var writer = new RecordingAuditWriter();
+        var ctx = BuildContext();
+        var mw = CreateMiddleware(
+            _ =>
+            {
+                ctx.Response.StatusCode = 401;
+                return Task.CompletedTask;
+            },
+            writer,
+            actorAccessor: new StubAuditActorAccessor("attacker"));
+
+        await mw.InvokeAsync(ctx);
+
+        var evt = Assert.Single(writer.Events);
+        Assert.Equal(AuditKind.InboundAuthFailure, evt.Kind);
+        Assert.Null(evt.Actor);
+    }
+
+    [Fact]
+    public async Task NoApiKey_NoAuthenticatedPrincipal_LeavesActorNull()
+    {
+        // Accessor present but returns null (no authenticated interactive user)
+        // and no API-key stash — the actor stays null rather than empty/echoed.
+        var writer = new RecordingAuditWriter();
+        var ctx = BuildContext();
+        var mw = CreateMiddleware(
+            _ =>
+            {
+                ctx.Response.StatusCode = 200;
+                return Task.CompletedTask;
+            },
+            writer,
+            actorAccessor: new StubAuditActorAccessor(currentActor: null));
+
+        await mw.InvokeAsync(ctx);
+
+        var evt = Assert.Single(writer.Events);
+        Assert.Null(evt.Actor);
+    }
+
    // ---------------------------------------------------------------------
    // 6. Writer failure must NEVER alter the HTTP response
    // ---------------------------------------------------------------------
@@ -0,0 +1,114 @@
+using System.Security.Claims;
+using Microsoft.AspNetCore.Http;
+using ZB.MOM.WW.Auth.AspNetCore;
+using ZB.MOM.WW.ScadaBridge.Security;
+
+namespace ZB.MOM.WW.ScadaBridge.Security.Tests;
+
+/// <summary>
+/// Phase 3 (wire audit Actor from the Auth principal): unit tests for
+/// <see cref="HttpAuditActorAccessor"/>. The accessor resolves the audit
+/// <c>Actor</c> from the authenticated principal on the ambient
+/// <see cref="IHttpContextAccessor.HttpContext"/> — the canonical username claim
+/// with an <see cref="System.Security.Principal.IIdentity.Name"/> fallback — and
+/// returns <c>null</c> whenever there is no authenticated interactive user, so the
+/// caller keeps its existing actor/fallback rather than echoing an unauthenticated
+/// principal.
+/// </summary>
+public class HttpAuditActorAccessorTests
+{
+    /// <summary>
+    /// Minimal <see cref="IHttpContextAccessor"/> test double returning a fixed
+    /// (possibly null) <see cref="HttpContext"/>.
+    /// </summary>
+    private sealed class StubHttpContextAccessor : IHttpContextAccessor
+    {
+        public HttpContext? HttpContext { get; set; }
+    }
+
+    private static HttpContext AuthenticatedContext(params Claim[] claims)
+    {
+        var ctx = new DefaultHttpContext
+        {
+            User = new ClaimsPrincipal(new ClaimsIdentity(claims, authenticationType: "TestAuth")),
+        };
+        return ctx;
+    }
+
+    [Fact]
+    public void CurrentActor_Authenticated_ReturnsUsernameClaim()
+    {
+        var ctx = AuthenticatedContext(
+            new Claim(JwtTokenService.UsernameClaimType, "alice"),
+            // A different Identity.Name proves the username claim is preferred.
+            new Claim(ClaimTypes.Name, "Alice Liddell"));
+        var accessor = new HttpAuditActorAccessor(
+            new StubHttpContextAccessor { HttpContext = ctx });
+
+        Assert.Equal("alice", accessor.CurrentActor);
+    }
+
+    [Fact]
+    public void CurrentActor_Authenticated_NoUsernameClaim_FallsBackToIdentityName()
+    {
+        // No canonical username claim; Identity.Name (pinned to ZbClaimTypes.Name)
+        // is the documented fallback. DefaultHttpContext maps the ClaimTypes.Name
+        // claim onto Identity.Name.
+        var ctx = AuthenticatedContext(new Claim(ClaimTypes.Name, "bob"));
+        var accessor = new HttpAuditActorAccessor(
+            new StubHttpContextAccessor { HttpContext = ctx });
+
+        Assert.Equal("bob", accessor.CurrentActor);
+    }
+
+    [Fact]
+    public void CurrentActor_Authenticated_PrefersUsernameOverZbName()
+    {
+        // Both the canonical username and the canonical name claim present — the
+        // username claim wins.
+        var ctx = AuthenticatedContext(
+            new Claim(JwtTokenService.UsernameClaimType, "svc-user"),
+            new Claim(ZbClaimTypes.Name, "Service User"));
+        var accessor = new HttpAuditActorAccessor(
+            new StubHttpContextAccessor { HttpContext = ctx });
+
+        Assert.Equal("svc-user", accessor.CurrentActor);
+    }
+
+    [Fact]
+    public void CurrentActor_Unauthenticated_ReturnsNull()
+    {
+        // An anonymous identity (no authenticationType) is NOT authenticated —
+        // never echo it back as an actor even if a name claim is somehow present.
+        var ctx = new DefaultHttpContext
+        {
+            User = new ClaimsPrincipal(
+                new ClaimsIdentity(new[] { new Claim(ClaimTypes.Name, "ghost") })),
+        };
+        var accessor = new HttpAuditActorAccessor(
+            new StubHttpContextAccessor { HttpContext = ctx });
+
+        Assert.Null(accessor.CurrentActor);
+    }
+
+    [Fact]
+    public void CurrentActor_NoAmbientHttpContext_ReturnsNull()
+    {
+        var accessor = new HttpAuditActorAccessor(
+            new StubHttpContextAccessor { HttpContext = null });
+
+        Assert.Null(accessor.CurrentActor);
+    }
+
+    [Fact]
+    public void CurrentActor_AuthenticatedButNoUsableName_ReturnsNull()
+    {
+        // Authenticated identity carrying only an unrelated claim (no username,
+        // no name) — there is nothing usable to record, so fall back to null.
+        var ctx = AuthenticatedContext(new Claim(ZbClaimTypes.Role, "Administrator"));
+        var accessor = new HttpAuditActorAccessor(
+            new StubHttpContextAccessor { HttpContext = ctx });
+
+        Assert.Null(accessor.CurrentActor);
+    }
+}
@@ -8,14 +8,19 @@
    <IsPackable>false</IsPackable>
  </PropertyGroup>

+  <ItemGroup>
+    <!-- HttpAuditActorAccessorTests construct DefaultHttpContext + an
+         IHttpContextAccessor stub to drive the principal-to-actor seam. -->
+    <FrameworkReference Include="Microsoft.AspNetCore.App" />
+  </ItemGroup>
+
  <ItemGroup>
    <PackageReference Include="coverlet.collector" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" />
-    <PackageReference Include="Microsoft.Extensions.DependencyInjection" />
-    <PackageReference Include="Microsoft.Extensions.Logging" />
-    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
-    <PackageReference Include="Microsoft.Extensions.Options" />
-    <PackageReference Include="Microsoft.AspNetCore.Authorization" />
+    <!-- Microsoft.Extensions.* and Microsoft.AspNetCore.Authorization are provided
+         by the Microsoft.AspNetCore.App shared framework referenced above (added so
+         HttpAuditActorAccessorTests can build DefaultHttpContext), so they are no
+         longer listed as PackageReferences here (NU1510). -->
    <PackageReference Include="Microsoft.NET.Test.Sdk" />
    <PackageReference Include="xunit" />
    <PackageReference Include="xunit.runner.visualstudio" />