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 031e4b3..abdd70d 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ConstraintEnforcer.cs @@ -121,8 +121,10 @@ public sealed class ConstraintEnforcer( /// 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). + /// The per-request client correlation id, if any. Persisted as the audit record's typed + /// CorrelationId when it parses as a GUID; a non-GUID value leaves that column null. + /// The raw string is always preserved in DetailsJson["clientCorrelationId"] so a + /// non-GUID id (e.g. from Rust/Python/Java clients) is never silently lost. /// /// Token to observe for cancellation. public async Task RecordDenialAsync( @@ -153,6 +155,11 @@ public sealed class ConstraintEnforcer( ["message"] = failure.Message, ["commandKind"] = commandKind, ["target"] = target, + // Always preserve the raw client correlation id here so it is never silently + // lost: the typed CorrelationId column only retains GUID-parseable ids, but + // clients (Rust/Python/Java) commonly send non-GUID or empty trace ids. The + // raw id is a client trace id, not a secret, so storing it is fine. + ["clientCorrelationId"] = correlationId ?? "", }), }; diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceConstraintTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceConstraintTests.cs index a01cfb8..9d226f7 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceConstraintTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Grpc/MxAccessGatewayServiceConstraintTests.cs @@ -548,6 +548,33 @@ public sealed class MxAccessGatewayServiceConstraintTests Assert.Equal("42", enforcer.RecordedDenials[0].Target); } + /// + /// End-to-end wiring (M-2): the per-request ClientCorrelationId must propagate + /// all the way through Invoke -> ApplyConstraintsAsync -> the unary write + /// enforce helper -> RecordDenialAsync, so the recorded denial carries the exact + /// id the client sent (including non-GUID trace ids used by Rust/Python/Java clients). + /// + [Fact] + public async Task Invoke_Write_WithDeniedHandle_ThreadsClientCorrelationIdIntoRecordedDenial() + { + const string CorrelationId = "rust-client-Write-7"; + PredicateConstraintEnforcer enforcer = new() + { + DenyWriteHandle = (serverHandle, itemHandle) => serverHandle == 7 && itemHandle == 42, + }; + FakeSessionManager sessionManager = CreateSessionManagerWithSeed(); + MxAccessGatewayService service = CreateService(sessionManager, enforcer); + + MxCommandRequest request = CreateWriteRequest(serverHandle: 7, itemHandle: 42); + request.ClientCorrelationId = CorrelationId; + + await Assert.ThrowsAsync( + async () => await service.Invoke(request, new TestServerCallContext())); + + Assert.Single(enforcer.RecordedDenials); + Assert.Equal(CorrelationId, enforcer.RecordedDenials[0].CorrelationId); + } + /// /// Unary WriteSecured against a denied handle takes the same enforce path /// and rejects identically — proving the four-arm switch in 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 627b01a..19a7917 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ConstraintEnforcerTests.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using ZB.MOM.WW.Audit; using ZB.MOM.WW.MxGateway.Contracts.Proto.Galaxy; using ZB.MOM.WW.MxGateway.Contracts.Proto; @@ -102,9 +103,12 @@ public sealed class ConstraintEnforcerTests Assert.Equal(correlationId, auditEvent.CorrelationId); } - /// A denial with a non-GUID correlation id leaves the audit correlation id null. + /// + /// A denial with a non-GUID correlation id leaves the typed audit correlation id null but + /// still preserves the raw client correlation id in DetailsJson so it is not lost. + /// [Fact] - public async Task RecordDenialAsync_WithNonGuidCorrelationId_LeavesCorrelationIdNull() + public async Task RecordDenialAsync_WithNonGuidCorrelationId_LeavesCorrelationIdNullButPreservesRawInDetails() { ConstraintEnforcer enforcer = CreateEnforcer(out FakeAuditWriter auditWriter); @@ -113,11 +117,17 @@ public sealed class ConstraintEnforcerTests "Read", "Secret.Tag", new ConstraintFailure("read_scope", "Tag is outside the API key read scope."), - "cli-xyz", + "rust-client-Write-7", CancellationToken.None); AuditEvent auditEvent = Assert.Single(auditWriter.Events); Assert.Null(auditEvent.CorrelationId); + Assert.NotNull(auditEvent.DetailsJson); + + Dictionary? details = + JsonSerializer.Deserialize>(auditEvent.DetailsJson); + Assert.NotNull(details); + Assert.Equal("rust-client-Write-7", details["clientCorrelationId"]); } /// A denial with no identity records the canonical "anonymous" actor.