diff --git a/Directory.Packages.props b/Directory.Packages.props index d8f854e..a26aea0 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -29,6 +29,7 @@ + diff --git a/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs b/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs new file mode 100644 index 0000000..eb0f9c9 --- /dev/null +++ b/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs @@ -0,0 +1,266 @@ +using System.Diagnostics; +using System.Text; +using System.Text.Json; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; +using ScadaLink.Commons.Entities.Audit; +using ScadaLink.Commons.Interfaces.Services; +using ScadaLink.Commons.Types.Enums; + +namespace ScadaLink.InboundAPI.Middleware; + +/// +/// Audit Log #23 (M4 Bundle D, T7) — emits one +/// row per inbound API request via covering the +/// full set of response shapes: +/// +/// +/// 2xx / non-error → with . +/// 401/403 → with . +/// 4xx (non-auth) / 5xx / thrown exception → with . +/// +/// +/// +/// Best-effort contract (alog.md §13). Audit emission NEVER alters the +/// user-facing HTTP response — a thrown writer or any other failure during +/// emission is caught, logged at warning, and dropped. A handler exception is +/// recorded on the audit row then re-thrown so the framework error path stays +/// authoritative. +/// +/// +/// +/// Actor resolution. Inbound API auth runs inside the endpoint handler +/// (no UseAuthentication-backed scheme populates +/// for X-API-Key callers), so the handler stashes the resolved API key name on +/// under after +/// ApiKeyValidator.ValidateAsync succeeds. The middleware reads it in +/// its finally block — on auth failures the key remains absent and +/// stays null (we never echo back an +/// unauthenticated principal). +/// +/// +/// +/// Body capture. The request body is buffered via +/// then +/// rewound so the downstream endpoint handler still sees the full payload. +/// Response body capture is deferred to M5 — wrapping Response.Body +/// requires a memory-stream swap that interacts awkwardly with Minimal API's +/// Results.Json/Results.Text writers; the M4 deliverable emits +/// the audit row with left null. +/// +/// +public sealed class AuditWriteMiddleware +{ + /// + /// key used by the endpoint handler to publish + /// the resolved API key name once ApiKeyValidator.ValidateAsync has + /// succeeded. Exposed as a constant so the handler and middleware share a + /// single source of truth (no stringly-typed coupling). + /// + public const string AuditActorItemKey = "ScadaLink.InboundAPI.AuditActor"; + + private readonly RequestDelegate _next; + private readonly ICentralAuditWriter _auditWriter; + private readonly ILogger _logger; + + public AuditWriteMiddleware( + RequestDelegate next, + ICentralAuditWriter auditWriter, + ILogger logger) + { + _next = next ?? throw new ArgumentNullException(nameof(next)); + _auditWriter = auditWriter ?? throw new ArgumentNullException(nameof(auditWriter)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + } + + public async Task InvokeAsync(HttpContext ctx) + { + var sw = Stopwatch.StartNew(); + + // Buffer the request body up front so we can both audit it and let the + // downstream handler still parse it. EnableBuffering swaps the request + // stream for a seekable wrapper that the framework rewinds at the end + // of the pipeline for us — but we also rewind to position 0 after our + // own read so the very next reader starts from the top. + ctx.Request.EnableBuffering(); + var requestBody = await ReadBufferedRequestBodyAsync(ctx.Request).ConfigureAwait(false); + + Exception? thrown = null; + try + { + await _next(ctx).ConfigureAwait(false); + } + catch (Exception ex) + { + thrown = ex; + // Re-throw — audit emission is BEST EFFORT, but the user-facing + // request's own error path must remain authoritative (alog.md §13). + throw; + } + finally + { + sw.Stop(); + EmitInboundAudit(ctx, sw.ElapsedMilliseconds, thrown, requestBody); + } + } + + /// + /// Builds and writes the row for the + /// request. Wrapped in try/catch so a thrown writer or any other emission + /// failure stays out of the user-facing response (alog.md §13). + /// + private void EmitInboundAudit( + HttpContext ctx, + long durationMs, + Exception? thrown, + string? requestBody) + { + try + { + var statusCode = ctx.Response.StatusCode; + var isAuthFailure = statusCode is 401 or 403; + + var kind = isAuthFailure + ? AuditKind.InboundAuthFailure + : AuditKind.InboundRequest; + + // A thrown handler exception is always Failed; otherwise any 4xx/5xx + // response signals failure. 2xx/3xx are Delivered. + var status = (thrown != null || statusCode >= 400) + ? AuditStatus.Failed + : AuditStatus.Delivered; + + var actor = isAuthFailure ? null : ResolveActor(ctx); + var methodName = ResolveMethodName(ctx); + + var extra = JsonSerializer.Serialize(new + { + remoteIp = ctx.Connection.RemoteIpAddress?.ToString(), + userAgent = ctx.Request.Headers.UserAgent.ToString(), + }); + + var evt = new AuditEvent + { + EventId = Guid.NewGuid(), + OccurredAtUtc = DateTime.UtcNow, + Channel = AuditChannel.ApiInbound, + Kind = kind, + Actor = actor, + Target = methodName, + Status = status, + HttpStatus = statusCode, + DurationMs = (int)Math.Min(durationMs, int.MaxValue), + ErrorMessage = thrown?.Message, + RequestSummary = requestBody, + // Response body capture is deferred to M5 (see XML doc above). + ResponseSummary = null, + PayloadTruncated = false, + Extra = extra, + // Central direct-write — no site-local forwarding state. + ForwardState = null, + }; + + // Fire-and-forget — the writer itself swallows; the additional + // try/catch around the fire still protects us if WriteAsync throws + // synchronously before returning a task. + _ = _auditWriter.WriteAsync(evt); + } + catch (Exception ex) + { + _logger.LogWarning( + ex, + "AuditWriteMiddleware emission failed for {Method} {Path} (status {Status})", + ctx.Request.Method, ctx.Request.Path, ctx.Response.StatusCode); + } + } + + /// + /// Reads the buffered request body fully into a string and rewinds the + /// stream so the downstream handler sees the unconsumed payload. Returns + /// null for empty/missing bodies so the audit row's + /// stays null rather than + /// containing an empty string. + /// + private static async Task ReadBufferedRequestBodyAsync(HttpRequest request) + { + if (request.ContentLength is 0) + { + return null; + } + + try + { + request.Body.Position = 0; + using var reader = new StreamReader( + request.Body, + Encoding.UTF8, + detectEncodingFromByteOrderMarks: false, + bufferSize: 1024, + leaveOpen: true); + var content = await reader.ReadToEndAsync().ConfigureAwait(false); + request.Body.Position = 0; + return string.IsNullOrEmpty(content) ? null : content; + } + catch + { + // A failed body read must not abort the request — fall through + // with a null RequestSummary; the audit row still records the + // outcome. + return null; + } + } + + /// + /// Reads the API key name the endpoint handler stashed on + /// after successful auth. Falls back to + /// the authenticated user name when an ASP.NET scheme has populated + /// (defensive — currently unused for inbound + /// API but cheap and forward-compatible). + /// + private static string? ResolveActor(HttpContext ctx) + { + if (ctx.Items.TryGetValue(AuditActorItemKey, out var stashed) + && stashed is string name + && !string.IsNullOrWhiteSpace(name)) + { + return name; + } + + var user = ctx.User; + if (user?.Identity is { IsAuthenticated: true, Name: { Length: > 0 } userName }) + { + return userName; + } + + return null; + } + + /// + /// Pulls the {methodName} route value off the request. Falls back to + /// the last segment of when no route value + /// is bound (e.g. when the request never reached the matched endpoint). + /// + private static string? ResolveMethodName(HttpContext ctx) + { + if (ctx.Request.RouteValues.TryGetValue("methodName", out var raw) + && raw is string method + && !string.IsNullOrWhiteSpace(method)) + { + return method; + } + + var path = ctx.Request.Path.Value; + if (string.IsNullOrEmpty(path)) + { + return null; + } + + var lastSlash = path.LastIndexOf('/'); + if (lastSlash < 0 || lastSlash == path.Length - 1) + { + return null; + } + + return path[(lastSlash + 1)..]; + } +} diff --git a/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddlewareExtensions.cs b/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddlewareExtensions.cs new file mode 100644 index 0000000..29cc10d --- /dev/null +++ b/src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddlewareExtensions.cs @@ -0,0 +1,26 @@ +using Microsoft.AspNetCore.Builder; + +namespace ScadaLink.InboundAPI.Middleware; + +/// +/// extensions for wiring +/// into the ASP.NET Core request pipeline. +/// See for the placement contract (must run +/// after auth so the resolved API key name is available on +/// , and before the +/// inbound-API endpoint handler that owns script execution). +/// +public static class AuditWriteMiddlewareExtensions +{ + /// + /// Registers in the pipeline. + /// + /// must be registered in DI (typically via AddAuditLog) before this + /// middleware runs. + /// + public static IApplicationBuilder UseAuditWriteMiddleware(this IApplicationBuilder app) + { + ArgumentNullException.ThrowIfNull(app); + return app.UseMiddleware(); + } +} diff --git a/tests/ScadaLink.InboundAPI.Tests/Middleware/AuditWriteMiddlewareTests.cs b/tests/ScadaLink.InboundAPI.Tests/Middleware/AuditWriteMiddlewareTests.cs new file mode 100644 index 0000000..fab1521 --- /dev/null +++ b/tests/ScadaLink.InboundAPI.Tests/Middleware/AuditWriteMiddlewareTests.cs @@ -0,0 +1,373 @@ +using System.Net; +using System.Text; +using System.Text.Json; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Logging.Abstractions; +using ScadaLink.Commons.Entities.Audit; +using ScadaLink.Commons.Interfaces.Services; +using ScadaLink.Commons.Types.Enums; +using ScadaLink.InboundAPI.Middleware; + +namespace ScadaLink.InboundAPI.Tests.Middleware; + +/// +/// M4 Bundle D (D1) — verifies emits exactly one +/// row per request via +/// covering all outcome shapes: +/// success (InboundRequest/Delivered), client/server error (InboundRequest/Failed), +/// and unauthenticated (InboundAuthFailure/Failed). Audit-write failures must NEVER +/// alter the HTTP response (alog.md §13). +/// +public class AuditWriteMiddlewareTests +{ + /// + /// Test-only recording . Captures every + /// the middleware emits so each test can assert on + /// the shape of the row produced for one request. + /// + private sealed class RecordingAuditWriter : ICentralAuditWriter + { + public List Events { get; } = new(); + public Func? OnWrite { get; set; } + + public Task WriteAsync(AuditEvent evt, CancellationToken ct = default) + { + lock (Events) + { + Events.Add(evt); + } + + return OnWrite?.Invoke(evt) ?? Task.CompletedTask; + } + } + + /// + /// Builds an primed for the inbound API route shape: + /// POST /api/{methodName}, optional JSON body, RemoteIpAddress + User-Agent. + /// The route value resolver mirrors the production endpoint mapping so the + /// middleware can pull the method name without owning routing itself. + /// + private static DefaultHttpContext BuildContext( + string methodName = "echo", + string? body = null, + string? userAgent = "test-agent/1.0", + IPAddress? remoteIp = null) + { + var ctx = new DefaultHttpContext(); + ctx.Request.Method = "POST"; + ctx.Request.Path = $"/api/{methodName}"; + ctx.Request.RouteValues["methodName"] = methodName; + + if (body is not null) + { + var bytes = Encoding.UTF8.GetBytes(body); + ctx.Request.Body = new MemoryStream(bytes); + ctx.Request.ContentLength = bytes.Length; + ctx.Request.ContentType = "application/json"; + } + + if (userAgent is not null) + { + ctx.Request.Headers["User-Agent"] = userAgent; + } + + ctx.Connection.RemoteIpAddress = remoteIp ?? IPAddress.Parse("10.0.0.5"); + + return ctx; + } + + private static AuditWriteMiddleware CreateMiddleware( + RequestDelegate next, + ICentralAuditWriter writer) => + new(next, writer, NullLogger.Instance); + + // --------------------------------------------------------------------- + // 1. Happy path — InboundRequest/Delivered/HttpStatus 200 + // --------------------------------------------------------------------- + + [Fact] + public async Task Pipeline_Success_EmitsOneEvent_KindInboundRequest_StatusDelivered_HttpStatus200() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext(); + var mw = CreateMiddleware(_ => + { + ctx.Response.StatusCode = 200; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + Assert.Equal(AuditChannel.ApiInbound, evt.Channel); + Assert.Equal(AuditKind.InboundRequest, evt.Kind); + Assert.Equal(AuditStatus.Delivered, evt.Status); + Assert.Equal(200, evt.HttpStatus); + // Central direct-write — no ForwardState (alog.md §6). + Assert.Null(evt.ForwardState); + Assert.NotEqual(Guid.Empty, evt.EventId); + Assert.Equal("echo", evt.Target); + } + + // --------------------------------------------------------------------- + // 2. 400 — script/validation failure path + // --------------------------------------------------------------------- + + [Fact] + public async Task Pipeline_400_EmitsEvent_Status_Failed_HttpStatus400() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext(); + var mw = CreateMiddleware(_ => + { + ctx.Response.StatusCode = 400; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + // A 400 is a request the auth succeeded for — still InboundRequest, not + // InboundAuthFailure. Only 401/403 maps to the auth-failure kind. + Assert.Equal(AuditKind.InboundRequest, evt.Kind); + Assert.Equal(AuditStatus.Failed, evt.Status); + Assert.Equal(400, evt.HttpStatus); + } + + // --------------------------------------------------------------------- + // 3. 401 — auth failure path + // --------------------------------------------------------------------- + + [Fact] + public async Task Pipeline_401_EmitsEvent_KindInboundAuthFailure_StatusFailed() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext(); + var mw = CreateMiddleware(_ => + { + ctx.Response.StatusCode = 401; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + Assert.Equal(AuditKind.InboundAuthFailure, evt.Kind); + Assert.Equal(AuditStatus.Failed, evt.Status); + Assert.Equal(401, evt.HttpStatus); + // The candidate API key never resolved to a name, so Actor stays null — + // never echo back an unauthenticated principal. + Assert.Null(evt.Actor); + } + + [Fact] + public async Task Pipeline_403_EmitsEvent_KindInboundAuthFailure_StatusFailed() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext(); + var mw = CreateMiddleware(_ => + { + ctx.Response.StatusCode = 403; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + Assert.Equal(AuditKind.InboundAuthFailure, evt.Kind); + Assert.Equal(AuditStatus.Failed, evt.Status); + Assert.Equal(403, evt.HttpStatus); + } + + // --------------------------------------------------------------------- + // 4. 500 — handler threw OR returned 500 + // --------------------------------------------------------------------- + + [Fact] + public async Task Pipeline_500_EmitsEvent_Status_Failed() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext(); + var mw = CreateMiddleware(_ => + { + ctx.Response.StatusCode = 500; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + Assert.Equal(AuditKind.InboundRequest, evt.Kind); + Assert.Equal(AuditStatus.Failed, evt.Status); + Assert.Equal(500, evt.HttpStatus); + } + + [Fact] + public async Task Pipeline_Throws_EmitsEvent_Status_Failed_And_Rethrows() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext(); + var boom = new InvalidOperationException("kaboom"); + var mw = CreateMiddleware(_ => throw boom, writer); + + // The middleware MUST re-throw so the request's own error path is + // authoritative — audit emission is best-effort only. + var thrown = await Assert.ThrowsAsync( + () => mw.InvokeAsync(ctx)); + Assert.Same(boom, thrown); + + var evt = Assert.Single(writer.Events); + Assert.Equal(AuditStatus.Failed, evt.Status); + Assert.Equal("kaboom", evt.ErrorMessage); + } + + // --------------------------------------------------------------------- + // 5. Actor resolution — the endpoint handler stashes the API key name + // AFTER successful auth so the middleware can pick it up from + // HttpContext.Items. + // --------------------------------------------------------------------- + + [Fact] + public async Task ApiKeyName_Resolved_From_HttpContext_AsActor() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext(); + var mw = CreateMiddleware(_ => + { + // The endpoint handler is expected to stash the resolved API key + // name here once ApiKeyValidator.ValidateAsync has succeeded. + ctx.Items[AuditWriteMiddleware.AuditActorItemKey] = "integration-svc"; + ctx.Response.StatusCode = 200; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + Assert.Equal("integration-svc", evt.Actor); + } + + // --------------------------------------------------------------------- + // 6. Writer failure must NEVER alter the HTTP response + // --------------------------------------------------------------------- + + [Fact] + public async Task AuditWriter_Throws_HttpResponse_Unchanged_Success_Stays_Success() + { + var writer = new RecordingAuditWriter + { + OnWrite = _ => throw new InvalidOperationException("writer offline"), + }; + var ctx = BuildContext(); + var mw = CreateMiddleware(_ => + { + ctx.Response.StatusCode = 200; + return Task.CompletedTask; + }, writer); + + // Audit emission is best-effort; even a thrown writer must NOT bubble + // up and contaminate the user-facing response status. + await mw.InvokeAsync(ctx); + + Assert.Equal(200, ctx.Response.StatusCode); + } + + [Fact] + public async Task AuditWriter_Throws_OnFailedRequest_HttpResponse_Unchanged() + { + var writer = new RecordingAuditWriter + { + OnWrite = _ => throw new InvalidOperationException("writer offline"), + }; + var ctx = BuildContext(); + var mw = CreateMiddleware(_ => + { + ctx.Response.StatusCode = 500; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + Assert.Equal(500, ctx.Response.StatusCode); + } + + // --------------------------------------------------------------------- + // 7. Provenance — RemoteIp + User-Agent surface in Extra JSON + // --------------------------------------------------------------------- + + [Fact] + public async Task RemoteIp_And_UserAgent_AppearInExtra() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext( + userAgent: "curl/8.4.0", + remoteIp: IPAddress.Parse("192.168.50.50")); + var mw = CreateMiddleware(_ => + { + ctx.Response.StatusCode = 200; + return Task.CompletedTask; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + Assert.NotNull(evt.Extra); + using var doc = JsonDocument.Parse(evt.Extra!); + var root = doc.RootElement; + Assert.Equal("192.168.50.50", root.GetProperty("remoteIp").GetString()); + Assert.Equal("curl/8.4.0", root.GetProperty("userAgent").GetString()); + } + + // --------------------------------------------------------------------- + // Body capture — the small JSON body is buffered and stashed on + // RequestSummary so subsequent reads (the endpoint handler's + // JsonDocument.Parse) still see the full payload. + // --------------------------------------------------------------------- + + [Fact] + public async Task RequestBody_IsBuffered_AndStashed_OnRequestSummary() + { + var writer = new RecordingAuditWriter(); + var requestJson = "{\"x\":1}"; + var ctx = BuildContext(body: requestJson); + + string? observedAfterMiddleware = null; + var mw = CreateMiddleware(async hc => + { + // Downstream code must still be able to read the body — the + // middleware enables buffering and rewinds so the handler sees the + // unconsumed stream. + using var reader = new StreamReader(hc.Request.Body); + observedAfterMiddleware = await reader.ReadToEndAsync(); + hc.Response.StatusCode = 200; + }, writer); + + await mw.InvokeAsync(ctx); + + Assert.Equal(requestJson, observedAfterMiddleware); + var evt = Assert.Single(writer.Events); + Assert.Equal(requestJson, evt.RequestSummary); + } + + [Fact] + public async Task DurationMs_IsRecorded() + { + var writer = new RecordingAuditWriter(); + var ctx = BuildContext(); + var mw = CreateMiddleware(async _ => + { + // The middleware records elapsed milliseconds — a small delay + // ensures DurationMs is non-negative and roughly tracks reality + // without being flake-sensitive in CI. + await Task.Delay(5); + ctx.Response.StatusCode = 200; + }, writer); + + await mw.InvokeAsync(ctx); + + var evt = Assert.Single(writer.Events); + Assert.NotNull(evt.DurationMs); + Assert.True(evt.DurationMs >= 0); + } +} diff --git a/tests/ScadaLink.InboundAPI.Tests/ScadaLink.InboundAPI.Tests.csproj b/tests/ScadaLink.InboundAPI.Tests/ScadaLink.InboundAPI.Tests.csproj index a29815f..1ec503e 100644 --- a/tests/ScadaLink.InboundAPI.Tests/ScadaLink.InboundAPI.Tests.csproj +++ b/tests/ScadaLink.InboundAPI.Tests/ScadaLink.InboundAPI.Tests.csproj @@ -14,6 +14,7 @@ +