diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardApiKeyManagementService.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardApiKeyManagementService.cs index a58f3d4..82569aa 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardApiKeyManagementService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardApiKeyManagementService.cs @@ -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__ 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, + /// + /// Emits the dashboard's own canonical for a dashboard-* op + /// directly through the best-effort (Task 2.3 #6). This is in + /// addition to the create/revoke/rotate-key event that + /// emits via the canonical-forwarding IApiKeyAuditStore adapter — the doubled-audit + /// behaviour is preserved, both rows now land in the canonical audit_event store. + /// + 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); } + /// + /// Derives a correlation id from the ASP.NET Core request trace identifier when it is a + /// well-formed GUID; otherwise null (the default HttpContext.TraceIdentifier is the + /// connection:request form, not a GUID, so it correlates to null rather than fabricating one). + /// + 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 { ["detail"] = detail }); + private static bool IsPepperUnavailable(InvalidOperationException exception) => exception.Message.Contains(PepperUnavailableMarker, StringComparison.OrdinalIgnoreCase); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs index ef85a5a..5865a5d 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs @@ -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 { /// Checks read constraints on a tag address. /// The API key identity to check constraints for. @@ -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 (":") 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 + { + ["constraint"] = failure.ConstraintName, + ["message"] = failure.Message, + ["commandKind"] = commandKind, + ["target"] = target, + }), + }; + + await auditWriter.WriteAsync(auditEvent, cancellationToken).ConfigureAwait(false); } private ConstraintFailure? CheckReadTarget( diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardApiKeyManagementServiceTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardApiKeyManagementServiceTests.cs index c8e4faf..58e3348 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardApiKeyManagementServiceTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardApiKeyManagementServiceTests.cs @@ -250,7 +250,7 @@ public sealed class DashboardApiKeyManagementServiceTests : IDisposable new DashboardApiKeyAuthorization(), services.GetRequiredService(), services.GetRequiredService(), - services.GetRequiredService(), + services.GetRequiredService(), new HttpContextAccessor { HttpContext = httpContext }); } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs index f2ae363..379ce2d 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs @@ -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 ":" form. + Assert.Equal("Write:42", auditEvent.Target); + Assert.NotNull(auditEvent.DetailsJson); + Assert.Contains("max_write_classification", auditEvent.DetailsJson, StringComparison.Ordinal); + Assert.Null(auditEvent.CorrelationId); + } + + /// A denial with no identity records the canonical "anonymous" actor. + [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); } /// Verifies that historized-only constraint requires historized attribute. @@ -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 { - /// Gets the recorded audit entries. - public List Entries { get; } = []; + /// Gets the recorded canonical audit events. + public List Events { get; } = []; /// - public Task AppendAsync(ApiKeyAuditEntry entry, CancellationToken cancellationToken) + public Task WriteAsync(AuditEvent auditEvent, CancellationToken cancellationToken = default) { - Entries.Add(entry); + Events.Add(auditEvent); return Task.CompletedTask; } - - /// - public Task> ListRecentAsync(int limit, CancellationToken ct) - { - return Task.FromResult>([]); - } } }