feat(audit): MxGateway local producers (dashboard + constraint-denial) emit canonical AuditEvent with Target/CorrelationId (Task 2.3 #6)

2026-06-02 10:13:54 -04:00
parent a5944bbe5d
commit 7ea8358c06
4 changed files with 124 additions and 52 deletions
@@ -1,7 +1,10 @@
 using System.Security.Claims;
+using System.Text.Json;
 using Microsoft.Data.Sqlite;
+using ZB.MOM.WW.Audit;
 using ZB.MOM.WW.Auth.Abstractions.ApiKeys;
 using ZB.MOM.WW.Auth.ApiKeys.Admin;
+using ZB.MOM.WW.MxGateway.Server.Security.Audit;
 using ZB.MOM.WW.MxGateway.Server.Security.Authentication;
 using ZB.MOM.WW.MxGateway.Server.Security.Authorization;

@@ -11,7 +14,7 @@ public sealed class DashboardApiKeyManagementService(
    DashboardApiKeyAuthorization authorization,
    ApiKeyAdminCommands adminCommands,
    IApiKeyAdminStore adminStore,
-    IApiKeyAuditStore auditStore,
+    IAuditWriter auditWriter,
    IHttpContextAccessor httpContextAccessor) : IDashboardApiKeyManagementService
 {
    private const string UnauthorizedMessage = "Sign in with an authorized LDAP account to manage API keys.";
@@ -50,8 +53,10 @@ public sealed class DashboardApiKeyManagementService(
        {
            // The shared command set generates the secret, hashes it with the pepper, persists the
            // record and assembles the mxgw_<id>_<secret> token (shown once). It also appends its own
-            // "create-key" audit entry; the dashboard layers a "dashboard-create-key" entry with the
-            // caller's remote address on top to preserve the dashboard audit vocabulary.
+            // "create-key" audit entry (now canonicalized through the IApiKeyAuditStore->IAuditWriter
+            // adapter); the dashboard layers a richer "dashboard-create-key" canonical AuditEvent
+            // (Target + CorrelationId + remote address) on top via IAuditWriter to preserve the
+            // dashboard audit vocabulary — both rows land in the canonical audit_event store.
            CreateKeyResult created = await adminCommands.CreateKeyAsync(
                    keyId,
                    request.DisplayName.Trim(),
@@ -61,7 +66,7 @@ public sealed class DashboardApiKeyManagementService(
                    cancellationToken)
                .ConfigureAwait(false);

-            await AppendAuditAsync(keyId, "dashboard-create-key", null, cancellationToken).ConfigureAwait(false);
+            await WriteDashboardAuditAsync(keyId, "dashboard-create-key", null, cancellationToken).ConfigureAwait(false);

            return DashboardApiKeyManagementResult.Success(
                "API key created. Copy the key now; it will not be shown again.",
@@ -102,7 +107,7 @@ public sealed class DashboardApiKeyManagementService(
            .RevokeKeyAsync(normalizedKeyId, RemoteAddress(), cancellationToken)
            .ConfigureAwait(false);

-        await AppendAuditAsync(
+        await WriteDashboardAuditAsync(
                normalizedKeyId,
                "dashboard-revoke-key",
                result.Succeeded ? "revoked" : "not-found-or-already-revoked",
@@ -144,7 +149,7 @@ public sealed class DashboardApiKeyManagementService(

            bool succeeded = rotated.Token is not null;

-            await AppendAuditAsync(
+            await WriteDashboardAuditAsync(
                    normalizedKeyId,
                    "dashboard-rotate-key",
                    succeeded ? "rotated" : "not-found",
@@ -188,7 +193,7 @@ public sealed class DashboardApiKeyManagementService(
            .DeleteAsync(normalizedKeyId, cancellationToken)
            .ConfigureAwait(false);

-        await AppendAuditAsync(
+        await WriteDashboardAuditAsync(
                normalizedKeyId,
                "dashboard-delete-key",
                deleted ? "deleted" : "not-found-or-active",
@@ -203,23 +208,51 @@ public sealed class DashboardApiKeyManagementService(
    private string? RemoteAddress() =>
        httpContextAccessor.HttpContext?.Connection.RemoteIpAddress?.ToString();

-    private async Task AppendAuditAsync(
-        string? keyId,
-        string eventType,
-        string? details,
+    /// <summary>
+    /// Emits the dashboard's own canonical <see cref="AuditEvent"/> for a <c>dashboard-*</c> op
+    /// directly through the best-effort <see cref="IAuditWriter"/> (Task 2.3 #6). This is in
+    /// addition to the <c>create/revoke/rotate-key</c> event that <see cref="ApiKeyAdminCommands"/>
+    /// emits via the canonical-forwarding <c>IApiKeyAuditStore</c> adapter — the doubled-audit
+    /// behaviour is preserved, both rows now land in the canonical <c>audit_event</c> store.
+    /// </summary>
+    private async Task WriteDashboardAuditAsync(
+        string keyId,
+        string action,
+        string? detail,
        CancellationToken cancellationToken)
    {
-        await auditStore.AppendAsync(
-                new ApiKeyAuditEntry(
-                    KeyId: keyId,
-                    EventType: eventType,
-                    RemoteAddress: RemoteAddress(),
-                    CreatedUtc: DateTimeOffset.UtcNow,
-                    Details: details),
-                cancellationToken)
-            .ConfigureAwait(false);
+        AuditEvent auditEvent = new()
+        {
+            EventId = Guid.NewGuid(),
+            OccurredAtUtc = DateTimeOffset.UtcNow,
+            Actor = keyId,
+            Action = action,
+            Outcome = AuditOutcome.Success,
+            Category = CanonicalForwardingApiKeyAuditStore.ApiKeyCategory,
+            Target = keyId,
+            SourceNode = RemoteAddress(),
+            CorrelationId = ParseCorrelationId(),
+            DetailsJson = WrapDetail(detail),
+        };
+
+        await auditWriter.WriteAsync(auditEvent, cancellationToken).ConfigureAwait(false);
    }

+    /// <summary>
+    /// Derives a correlation id from the ASP.NET Core request trace identifier when it is a
+    /// well-formed GUID; otherwise null (the default <c>HttpContext.TraceIdentifier</c> is the
+    /// connection:request form, not a GUID, so it correlates to null rather than fabricating one).
+    /// </summary>
+    private Guid? ParseCorrelationId() =>
+        Guid.TryParse(httpContextAccessor.HttpContext?.TraceIdentifier, out Guid correlationId)
+            ? correlationId
+            : null;
+
+    private static string? WrapDetail(string? detail) =>
+        detail is null
+            ? null
+            : JsonSerializer.Serialize(new Dictionary<string, string> { ["detail"] = detail });
+
    private static bool IsPepperUnavailable(InvalidOperationException exception) =>
        exception.Message.Contains(PepperUnavailableMarker, StringComparison.OrdinalIgnoreCase);

@@ -1,6 +1,8 @@
-using ZB.MOM.WW.Auth.Abstractions.ApiKeys;
+using System.Text.Json;
+using ZB.MOM.WW.Audit;
 using ZB.MOM.WW.MxGateway.Contracts.Proto.Galaxy;
 using ZB.MOM.WW.MxGateway.Server.Galaxy;
+using ZB.MOM.WW.MxGateway.Server.Security.Audit;
 using ZB.MOM.WW.MxGateway.Server.Security.Authentication;
 using ZB.MOM.WW.MxGateway.Server.Sessions;

@@ -12,7 +14,7 @@ namespace ZB.MOM.WW.MxGateway.Server.Security.Authorization;

 public sealed class ConstraintEnforcer(
    IGalaxyHierarchyCache cache,
-    IApiKeyAuditStore auditStore) : IConstraintEnforcer
+    IAuditWriter auditWriter) : IConstraintEnforcer
 {
    /// <summary>Checks read constraints on a tag address.</summary>
    /// <param name="identity">The API key identity to check constraints for.</param>
@@ -126,15 +128,33 @@ public sealed class ConstraintEnforcer(
        ConstraintFailure failure,
        CancellationToken cancellationToken)
    {
-        await auditStore.AppendAsync(
-                new ApiKeyAuditEntry(
-                    KeyId: identity?.KeyId,
-                    EventType: "constraint-denied",
-                    RemoteAddress: null,
-                    CreatedUtc: DateTimeOffset.UtcNow,
-                    Details: $"{commandKind}: {target}: {failure.ConstraintName}: {failure.Message}"),
-                cancellationToken)
-            .ConfigureAwait(false);
+        // Emit a canonical Denied AuditEvent directly through the best-effort IAuditWriter
+        // (Task 2.3 #6): structured Target ("<commandKind>:<target>") and a richer DetailsJson
+        // envelope carrying constraint/message/commandKind/target.
+        // TODO(Task 2.3): CorrelationId is left null here. Threading the per-request
+        // ClientCorrelationId down to RecordDenialAsync would require an invasive IConstraintEnforcer
+        // signature change across the gRPC call path; that is deferred to a follow-up.
+        AuditEvent auditEvent = new()
+        {
+            EventId = Guid.NewGuid(),
+            OccurredAtUtc = DateTimeOffset.UtcNow,
+            Actor = identity?.KeyId ?? "anonymous",
+            Action = "constraint-denied",
+            Outcome = AuditOutcome.Denied,
+            Category = CanonicalForwardingApiKeyAuditStore.ApiKeyCategory,
+            Target = $"{commandKind}:{target}",
+            SourceNode = null,
+            CorrelationId = null,
+            DetailsJson = JsonSerializer.Serialize(new Dictionary<string, string>
+            {
+                ["constraint"] = failure.ConstraintName,
+                ["message"] = failure.Message,
+                ["commandKind"] = commandKind,
+                ["target"] = target,
+            }),
+        };
+
+        await auditWriter.WriteAsync(auditEvent, cancellationToken).ConfigureAwait(false);
    }

    private ConstraintFailure? CheckReadTarget(
@@ -250,7 +250,7 @@ public sealed class DashboardApiKeyManagementServiceTests : IDisposable
            new DashboardApiKeyAuthorization(),
            services.GetRequiredService<ApiKeyAdminCommands>(),
            services.GetRequiredService<IApiKeyAdminStore>(),
-            services.GetRequiredService<IApiKeyAuditStore>(),
+            services.GetRequiredService<ZB.MOM.WW.Audit.IAuditWriter>(),
            new HttpContextAccessor { HttpContext = httpContext });
    }

@@ -1,4 +1,4 @@
-using ZB.MOM.WW.Auth.Abstractions.ApiKeys;
+using ZB.MOM.WW.Audit;
 using ZB.MOM.WW.MxGateway.Contracts.Proto.Galaxy;
 using ZB.MOM.WW.MxGateway.Contracts.Proto;
 using ZB.MOM.WW.MxGateway.Server.Dashboard;
@@ -38,7 +38,7 @@ public sealed class ConstraintEnforcerTests
    [Fact]
    public async Task CheckWriteHandleAsync_WhenClassificationTooHigh_ReturnsFailureAndAudits()
    {
-        ConstraintEnforcer enforcer = CreateEnforcer(out FakeAuditStore auditStore);
+        ConstraintEnforcer enforcer = CreateEnforcer(out FakeAuditWriter auditWriter);
        ApiKeyIdentity identity = CreateIdentity(ApiKeyConstraints.Empty with
        {
            WriteSubtrees = ["Area1/*"],
@@ -71,10 +71,35 @@ public sealed class ConstraintEnforcerTests

        await enforcer.RecordDenialAsync(identity, "Write", "42", failure, CancellationToken.None);

-        ApiKeyAuditEntry entry = Assert.Single(auditStore.Entries);
-        Assert.Equal("operator01", entry.KeyId);
-        Assert.Equal("constraint-denied", entry.EventType);
-        Assert.Contains("max_write_classification", entry.Details, StringComparison.Ordinal);
+        AuditEvent auditEvent = Assert.Single(auditWriter.Events);
+        Assert.Equal("operator01", auditEvent.Actor);
+        Assert.Equal("constraint-denied", auditEvent.Action);
+        Assert.Equal(AuditOutcome.Denied, auditEvent.Outcome);
+        Assert.Equal("ApiKey", auditEvent.Category);
+        // Target is the structured "<commandKind>:<target>" form.
+        Assert.Equal("Write:42", auditEvent.Target);
+        Assert.NotNull(auditEvent.DetailsJson);
+        Assert.Contains("max_write_classification", auditEvent.DetailsJson, StringComparison.Ordinal);
+        Assert.Null(auditEvent.CorrelationId);
+    }
+
+    /// <summary>A denial with no identity records the canonical "anonymous" actor.</summary>
+    [Fact]
+    public async Task RecordDenialAsync_WithoutIdentity_UsesAnonymousActor()
+    {
+        ConstraintEnforcer enforcer = CreateEnforcer(out FakeAuditWriter auditWriter);
+
+        await enforcer.RecordDenialAsync(
+            identity: null,
+            "Read",
+            "Secret.Tag",
+            new ConstraintFailure("read_scope", "Tag is outside the API key read scope."),
+            CancellationToken.None);
+
+        AuditEvent auditEvent = Assert.Single(auditWriter.Events);
+        Assert.Equal("anonymous", auditEvent.Actor);
+        Assert.Equal(AuditOutcome.Denied, auditEvent.Outcome);
+        Assert.Equal("Read:Secret.Tag", auditEvent.Target);
    }

    /// <summary>Verifies that historized-only constraint requires historized attribute.</summary>
@@ -134,10 +159,10 @@ public sealed class ConstraintEnforcerTests
        Assert.Equal("read_historized_only", failure.ConstraintName);
    }

-    private static ConstraintEnforcer CreateEnforcer(out FakeAuditStore auditStore)
+    private static ConstraintEnforcer CreateEnforcer(out FakeAuditWriter auditWriter)
    {
-        auditStore = new FakeAuditStore();
-        return new ConstraintEnforcer(new StubGalaxyHierarchyCache(CreateEntry()), auditStore);
+        auditWriter = new FakeAuditWriter();
+        return new ConstraintEnforcer(new StubGalaxyHierarchyCache(CreateEntry()), auditWriter);
    }

    private static ApiKeyIdentity CreateIdentity(ApiKeyConstraints constraints)
@@ -242,22 +267,16 @@ public sealed class ConstraintEnforcerTests
        public Task WaitForFirstLoadAsync(CancellationToken cancellationToken) => Task.CompletedTask;
    }

-    private sealed class FakeAuditStore : IApiKeyAuditStore
+    private sealed class FakeAuditWriter : IAuditWriter
    {
-        /// <summary>Gets the recorded audit entries.</summary>
-        public List<ApiKeyAuditEntry> Entries { get; } = [];
+        /// <summary>Gets the recorded canonical audit events.</summary>
+        public List<AuditEvent> Events { get; } = [];

        /// <inheritdoc />
-        public Task AppendAsync(ApiKeyAuditEntry entry, CancellationToken cancellationToken)
+        public Task WriteAsync(AuditEvent auditEvent, CancellationToken cancellationToken = default)
        {
-            Entries.Add(entry);
+            Events.Add(auditEvent);
            return Task.CompletedTask;
        }
-
-        /// <inheritdoc />
-        public Task<IReadOnlyList<ApiKeyAuditEntry>> ListRecentAsync(int limit, CancellationToken ct)
-        {
-            return Task.FromResult<IReadOnlyList<ApiKeyAuditEntry>>([]);
-        }
    }
 }