From 8415f35abd3649c273b6c33c00b133f2c8e7c55b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 15 Jun 2026 09:42:40 -0400 Subject: [PATCH] =?UTF-8?q?feat(gateway):=20thread=20ClientCorrelationId?= =?UTF-8?q?=20into=20constraint-denial=20audit=20(=C2=A71.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../WorkerLiveMxAccessSmokeTests.cs | 1 + .../Grpc/MxAccessGatewayService.cs | 35 ++++++++++++---- .../Authorization/ConstraintEnforcer.cs | 10 +++-- .../Authorization/IConstraintEnforcer.cs | 5 +++ .../Authorization/ConstraintEnforcerTests.cs | 40 ++++++++++++++++++- .../TestSupport/AllowAllConstraintEnforcer.cs | 1 + .../PredicateConstraintEnforcer.cs | 7 ++-- 7 files changed, 84 insertions(+), 15 deletions(-) diff --git a/src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs b/src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs index f7c122f..8d92057 100644 --- a/src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs +++ b/src/ZB.MOM.WW.MxGateway.IntegrationTests/WorkerLiveMxAccessSmokeTests.cs @@ -1607,6 +1607,7 @@ public sealed class WorkerLiveMxAccessSmokeTests(ITestOutputHelper output) string commandKind, string target, ConstraintFailure failure, + string? correlationId, CancellationToken cancellationToken) => Task.CompletedTask; } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs index 362ad46..90a0e54 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Grpc/MxAccessGatewayService.cs @@ -105,6 +105,7 @@ public sealed class MxAccessGatewayService( BulkConstraintPlan? bulkConstraintPlan = await ApplyConstraintsAsync( session, command, + request.ClientCorrelationId, context.CancellationToken) .ConfigureAwait(false); @@ -279,17 +280,18 @@ public sealed class MxAccessGatewayService( private async Task ApplyConstraintsAsync( GatewaySession session, MxCommand command, + string? correlationId, CancellationToken cancellationToken) { ApiKeyIdentity? identity = identityAccessor.Current; switch (command.Kind) { case MxCommandKind.AddItem: - await EnforceReadTagAsync(identity, command.Kind, command.AddItem.ItemDefinition, cancellationToken) + await EnforceReadTagAsync(identity, command.Kind, command.AddItem.ItemDefinition, correlationId, cancellationToken) .ConfigureAwait(false); return null; case MxCommandKind.AddItem2: - await EnforceReadTagAsync(identity, command.Kind, command.AddItem2.ItemDefinition, cancellationToken) + await EnforceReadTagAsync(identity, command.Kind, command.AddItem2.ItemDefinition, correlationId, cancellationToken) .ConfigureAwait(false); return null; case MxCommandKind.AddItemBulk: @@ -298,6 +300,7 @@ public sealed class MxAccessGatewayService( command, command.AddItemBulk.ServerHandle, command.AddItemBulk.TagAddresses, + correlationId, cancellationToken) .ConfigureAwait(false); case MxCommandKind.SubscribeBulk: @@ -306,6 +309,7 @@ public sealed class MxAccessGatewayService( command, command.SubscribeBulk.ServerHandle, command.SubscribeBulk.TagAddresses, + correlationId, cancellationToken) .ConfigureAwait(false); case MxCommandKind.AdviseItemBulk: @@ -315,6 +319,7 @@ public sealed class MxAccessGatewayService( command, command.AdviseItemBulk.ServerHandle, command.AdviseItemBulk.ItemHandles, + correlationId, cancellationToken) .ConfigureAwait(false); case MxCommandKind.ReadBulk: @@ -323,6 +328,7 @@ public sealed class MxAccessGatewayService( command, command.ReadBulk.ServerHandle, command.ReadBulk.TagAddresses, + correlationId, cancellationToken) .ConfigureAwait(false); case MxCommandKind.WriteBulk: @@ -333,6 +339,7 @@ public sealed class MxAccessGatewayService( command.WriteBulk.ServerHandle, command.WriteBulk.Entries, entry => entry.ItemHandle, + correlationId, cancellationToken) .ConfigureAwait(false); case MxCommandKind.Write2Bulk: @@ -343,6 +350,7 @@ public sealed class MxAccessGatewayService( command.Write2Bulk.ServerHandle, command.Write2Bulk.Entries, entry => entry.ItemHandle, + correlationId, cancellationToken) .ConfigureAwait(false); case MxCommandKind.WriteSecuredBulk: @@ -353,6 +361,7 @@ public sealed class MxAccessGatewayService( command.WriteSecuredBulk.ServerHandle, command.WriteSecuredBulk.Entries, entry => entry.ItemHandle, + correlationId, cancellationToken) .ConfigureAwait(false); case MxCommandKind.WriteSecured2Bulk: @@ -363,6 +372,7 @@ public sealed class MxAccessGatewayService( command.WriteSecured2Bulk.ServerHandle, command.WriteSecured2Bulk.Entries, entry => entry.ItemHandle, + correlationId, cancellationToken) .ConfigureAwait(false); case MxCommandKind.Write: @@ -372,6 +382,7 @@ public sealed class MxAccessGatewayService( command.Kind, command.Write.ServerHandle, command.Write.ItemHandle, + correlationId, cancellationToken) .ConfigureAwait(false); return null; @@ -382,6 +393,7 @@ public sealed class MxAccessGatewayService( command.Kind, command.Write2.ServerHandle, command.Write2.ItemHandle, + correlationId, cancellationToken) .ConfigureAwait(false); return null; @@ -392,6 +404,7 @@ public sealed class MxAccessGatewayService( command.Kind, command.WriteSecured.ServerHandle, command.WriteSecured.ItemHandle, + correlationId, cancellationToken) .ConfigureAwait(false); return null; @@ -402,6 +415,7 @@ public sealed class MxAccessGatewayService( command.Kind, command.WriteSecured2.ServerHandle, command.WriteSecured2.ItemHandle, + correlationId, cancellationToken) .ConfigureAwait(false); return null; @@ -414,6 +428,7 @@ public sealed class MxAccessGatewayService( ApiKeyIdentity? identity, MxCommandKind commandKind, string tagAddress, + string? correlationId, CancellationToken cancellationToken) { ConstraintFailure? failure = await constraintEnforcer @@ -424,7 +439,7 @@ public sealed class MxAccessGatewayService( return; } - await constraintEnforcer.RecordDenialAsync(identity, commandKind.ToString(), tagAddress, failure, cancellationToken) + await constraintEnforcer.RecordDenialAsync(identity, commandKind.ToString(), tagAddress, failure, correlationId, cancellationToken) .ConfigureAwait(false); throw new RpcException(new Status(StatusCode.PermissionDenied, failure.Message)); } @@ -435,6 +450,7 @@ public sealed class MxAccessGatewayService( MxCommandKind commandKind, int serverHandle, int itemHandle, + string? correlationId, CancellationToken cancellationToken) { ConstraintFailure? failure = await constraintEnforcer @@ -445,7 +461,7 @@ public sealed class MxAccessGatewayService( return; } - await constraintEnforcer.RecordDenialAsync(identity, commandKind.ToString(), itemHandle.ToString(System.Globalization.CultureInfo.InvariantCulture), failure, cancellationToken) + await constraintEnforcer.RecordDenialAsync(identity, commandKind.ToString(), itemHandle.ToString(System.Globalization.CultureInfo.InvariantCulture), failure, correlationId, cancellationToken) .ConfigureAwait(false); throw new RpcException(new Status(StatusCode.PermissionDenied, failure.Message)); } @@ -455,6 +471,7 @@ public sealed class MxAccessGatewayService( MxCommand command, int serverHandle, IReadOnlyList tagAddresses, + string? correlationId, CancellationToken cancellationToken) { Dictionary denied = []; @@ -471,7 +488,7 @@ public sealed class MxAccessGatewayService( continue; } - await constraintEnforcer.RecordDenialAsync(identity, command.Kind.ToString(), tagAddress, failure, cancellationToken) + await constraintEnforcer.RecordDenialAsync(identity, command.Kind.ToString(), tagAddress, failure, correlationId, cancellationToken) .ConfigureAwait(false); denied[index] = new SubscribeResult { @@ -507,6 +524,7 @@ public sealed class MxAccessGatewayService( MxCommand command, int serverHandle, IReadOnlyList tagAddresses, + string? correlationId, CancellationToken cancellationToken) { // Mirrors FilterTagBulkAsync but produces BulkReadResult denial entries @@ -526,7 +544,7 @@ public sealed class MxAccessGatewayService( continue; } - await constraintEnforcer.RecordDenialAsync(identity, command.Kind.ToString(), tagAddress, failure, cancellationToken) + await constraintEnforcer.RecordDenialAsync(identity, command.Kind.ToString(), tagAddress, failure, correlationId, cancellationToken) .ConfigureAwait(false); denied[index] = new BulkReadResult { @@ -557,6 +575,7 @@ public sealed class MxAccessGatewayService( int serverHandle, Google.Protobuf.Collections.RepeatedField entries, Func getItemHandle, + string? correlationId, CancellationToken cancellationToken) where TEntry : class { // The four bulk-write families each carry a different per-entry message @@ -586,6 +605,7 @@ public sealed class MxAccessGatewayService( command.Kind.ToString(), itemHandle.ToString(System.Globalization.CultureInfo.InvariantCulture), failure, + correlationId, cancellationToken) .ConfigureAwait(false); denied[index] = new BulkWriteResult @@ -637,6 +657,7 @@ public sealed class MxAccessGatewayService( MxCommand command, int serverHandle, IReadOnlyList itemHandles, + string? correlationId, CancellationToken cancellationToken) { Dictionary denied = []; @@ -653,7 +674,7 @@ public sealed class MxAccessGatewayService( continue; } - await constraintEnforcer.RecordDenialAsync(identity, command.Kind.ToString(), itemHandle.ToString(System.Globalization.CultureInfo.InvariantCulture), failure, cancellationToken) + await constraintEnforcer.RecordDenialAsync(identity, command.Kind.ToString(), itemHandle.ToString(System.Globalization.CultureInfo.InvariantCulture), failure, correlationId, cancellationToken) .ConfigureAwait(false); denied[index] = new SubscribeResult { 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 5865a5d..031e4b3 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs @@ -120,20 +120,22 @@ public sealed class ConstraintEnforcer( /// The command type (e.g., read, write). /// The target being accessed (tag address or handle). /// The constraint failure details. + /// + /// The per-request client correlation id, if any. Persisted as the audit record's + /// CorrelationId when it parses as a GUID; a non-GUID value is dropped (left null). + /// /// Token to observe for cancellation. public async Task RecordDenialAsync( ApiKeyIdentity? identity, string commandKind, string target, ConstraintFailure failure, + string? correlationId, CancellationToken cancellationToken) { // 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(), @@ -144,7 +146,7 @@ public sealed class ConstraintEnforcer( Category = CanonicalForwardingApiKeyAuditStore.ApiKeyCategory, Target = $"{commandKind}:{target}", SourceNode = null, - CorrelationId = null, + CorrelationId = Guid.TryParse(correlationId, out var cid) ? cid : (Guid?)null, DetailsJson = JsonSerializer.Serialize(new Dictionary { ["constraint"] = failure.ConstraintName, diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/IConstraintEnforcer.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/IConstraintEnforcer.cs index 91d6891..12a7f0c 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/IConstraintEnforcer.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/IConstraintEnforcer.cs @@ -45,11 +45,16 @@ public interface IConstraintEnforcer /// The kind of command denied. /// The target of the denied command. /// The constraint failure details. + /// + /// The per-request client correlation id, if any. Stored on the audit record's + /// CorrelationId when it parses as a GUID; otherwise left null. + /// /// Token to observe for cancellation. Task RecordDenialAsync( ApiKeyIdentity? identity, string commandKind, string target, ConstraintFailure failure, + string? correlationId, CancellationToken cancellationToken); } 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 379ce2d..627b01a 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs @@ -69,7 +69,7 @@ public sealed class ConstraintEnforcerTests CancellationToken.None); Assert.NotNull(failure); - await enforcer.RecordDenialAsync(identity, "Write", "42", failure, CancellationToken.None); + await enforcer.RecordDenialAsync(identity, "Write", "42", failure, correlationId: null, CancellationToken.None); AuditEvent auditEvent = Assert.Single(auditWriter.Events); Assert.Equal("operator01", auditEvent.Actor); @@ -83,6 +83,43 @@ public sealed class ConstraintEnforcerTests Assert.Null(auditEvent.CorrelationId); } + /// A denial carrying a parseable correlation id stores it on the audit record. + [Fact] + public async Task RecordDenialAsync_WithGuidCorrelationId_StoresCorrelationId() + { + ConstraintEnforcer enforcer = CreateEnforcer(out FakeAuditWriter auditWriter); + Guid correlationId = Guid.NewGuid(); + + await enforcer.RecordDenialAsync( + identity: null, + "Read", + "Secret.Tag", + new ConstraintFailure("read_scope", "Tag is outside the API key read scope."), + correlationId.ToString(), + CancellationToken.None); + + AuditEvent auditEvent = Assert.Single(auditWriter.Events); + Assert.Equal(correlationId, auditEvent.CorrelationId); + } + + /// A denial with a non-GUID correlation id leaves the audit correlation id null. + [Fact] + public async Task RecordDenialAsync_WithNonGuidCorrelationId_LeavesCorrelationIdNull() + { + 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."), + "cli-xyz", + CancellationToken.None); + + AuditEvent auditEvent = Assert.Single(auditWriter.Events); + Assert.Null(auditEvent.CorrelationId); + } + /// A denial with no identity records the canonical "anonymous" actor. [Fact] public async Task RecordDenialAsync_WithoutIdentity_UsesAnonymousActor() @@ -94,6 +131,7 @@ public sealed class ConstraintEnforcerTests "Read", "Secret.Tag", new ConstraintFailure("read_scope", "Tag is outside the API key read scope."), + correlationId: null, CancellationToken.None); AuditEvent auditEvent = Assert.Single(auditWriter.Events); diff --git a/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/AllowAllConstraintEnforcer.cs b/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/AllowAllConstraintEnforcer.cs index 0bdaf36..316b061 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/AllowAllConstraintEnforcer.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/AllowAllConstraintEnforcer.cs @@ -38,5 +38,6 @@ public sealed class AllowAllConstraintEnforcer : IConstraintEnforcer string commandKind, string target, ConstraintFailure failure, + string? correlationId, CancellationToken cancellationToken) => Task.CompletedTask; } diff --git a/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/PredicateConstraintEnforcer.cs b/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/PredicateConstraintEnforcer.cs index 27537fe..fe129fa 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/PredicateConstraintEnforcer.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/TestSupport/PredicateConstraintEnforcer.cs @@ -23,8 +23,8 @@ public sealed class PredicateConstraintEnforcer : IConstraintEnforcer /// Deny predicate keyed on (serverHandle, itemHandle) (returns true to deny). public Func DenyWriteHandle { get; init; } = (_, _) => false; - /// Recorded denial messages — (commandKind, target) tuples. - public List<(string CommandKind, string Target)> RecordedDenials { get; } = []; + /// Recorded denial messages — (commandKind, target, correlationId) tuples. + public List<(string CommandKind, string Target, string? CorrelationId)> RecordedDenials { get; } = []; /// public Task CheckReadTagAsync( @@ -81,9 +81,10 @@ public sealed class PredicateConstraintEnforcer : IConstraintEnforcer string commandKind, string target, ConstraintFailure failure, + string? correlationId, CancellationToken cancellationToken) { - RecordedDenials.Add((commandKind, target)); + RecordedDenials.Add((commandKind, target, correlationId)); return Task.CompletedTask; } }