diff --git a/CLAUDE.md b/CLAUDE.md index 56fd95e..3f617f7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,7 +123,7 @@ Gateway gRPC clients authenticate with an API key in metadata: `authorization: B Session event streaming is **owner-scoped**: the API key that opened a session is recorded on the session, and every `StreamEvents` attach/reattach is rejected with `PermissionDenied` unless the caller's key id matches the owner. Possessing the `event` scope and knowing a session id is not sufficient — this closes the reconnect/fan-out trust boundary (detach-grace and replay retention are on by default) so an `event`-scoped key cannot attach to another key's retained session. -Dashboard auth is LDAP-backed (separate from the gRPC API-key model). `/login` binds against `MxGateway:Ldap` and maps the user's LDAP groups to `Admin` or `Viewer` via `MxGateway:Dashboard:GroupToRole`, then issues an HTTP-only secure `__Host-MxGatewayDashboard` cookie. SignalR hubs at `/hubs/{snapshot,alarms,events}` accept either the cookie or a 30-minute bearer minted at `/hubs/token`. `Dashboard:AllowAnonymousLocalhost` bypasses auth on loopback when enabled. `Dashboard:DisableLogin` (default `false`) auto-authenticates every dashboard request — including remote browsers — as `Dashboard:AutoLoginUser` (default `multi-role`) with both Admin and Viewer roles; dev/test only, never enable in production. +Dashboard auth is LDAP-backed (separate from the gRPC API-key model). `/login` binds against `MxGateway:Ldap` and maps the user's LDAP groups to `Admin` or `Viewer` via `MxGateway:Dashboard:GroupToRole`, then issues an HTTP-only secure `__Host-MxGatewayDashboard` cookie. SignalR hubs at `/hubs/{snapshot,alarms,events}` accept either the cookie or a 5-minute bearer minted at `/hubs/token`. `Dashboard:AllowAnonymousLocalhost` (default `true`) grants **read-only** loopback access — it satisfies the Viewer requirement but never the Admin-only requirement, so anonymous localhost can view the dashboard but not reach API-key CRUD or session Close/Kill (`Authentication:Mode=Disabled` is scoped the same way). `Dashboard:DisableLogin` (default `false`) auto-authenticates every dashboard request — including remote browsers — as `Dashboard:AutoLoginUser` (default `multi-role`) with both Admin and Viewer roles; dev/test only, never enable in production. ## Process / Platform Notes diff --git a/archreview/remediation/00-tracking.md b/archreview/remediation/00-tracking.md index 0d2a49d..004223d 100644 --- a/archreview/remediation/00-tracking.md +++ b/archreview/remediation/00-tracking.md @@ -140,7 +140,7 @@ Full design + implementation for each row lives in the linked domain doc under i | ID | Sev | Tier | Eff | Dep | Status | Title | |---|---|:-:|:-:|---|---|---| | SEC-01 | Medium | P1 | M | — | Done | Windows-absolute default paths become relative files off-Windows | -| SEC-02 | Medium | P1 | S | — | Not started | `AllowAnonymousLocalhost` satisfies AdminOnly, not just Viewer | +| SEC-02 | Medium | P1 | S | — | Done | `AllowAnonymousLocalhost` satisfies AdminOnly, not just Viewer | | SEC-03 | Medium | P2 | S | — | Not started | Dashboard cookie lost its `__Host-` prefix; four docs still promise it | | SEC-04 | Medium | P1 | S | SEC-03 | Done | `DisableLogin` has no production guard | | SEC-05 | Medium | P1 | M | — | Done | Hub bearer tokens irrevocable for 30 min, carried in query string | @@ -150,7 +150,7 @@ Full design + implementation for each row lives in the linked domain doc under i | SEC-09 | Medium | P2 | S | — | Not started | Dashboard design-doc `GroupToRole` sample now fails startup validation | | SEC-10 | Medium | P1 | L | — | Done | No API-key expiry | | SEC-11 | Medium | P1 | M | SEC-08 | Done | No rate limiting or lockout on either auth surface | -| SEC-12 | Medium | P1 | M | — | Not started | Dashboard Close/Kill bypass the canonical audit store | +| SEC-12 | Medium | P1 | M | — | Done | Dashboard Close/Kill bypass the canonical audit store | | SEC-13 | Low | — | S | SEC-30 | Not started | Redactor's credential-command list omits secured-bulk variants | | SEC-14 | Low | — | S | SEC-02,20 | Not started | `/metrics` + `/health` unauthenticated; a metric leaks session ids | | SEC-15 | Low | — | S | — | Not started | GET `/logout` skips antiforgery | @@ -158,7 +158,7 @@ Full design + implementation for each row lives in the linked domain doc under i | SEC-17 | Low | — | S | — | Not started | Interceptor does not override client-streaming/duplex handlers | | SEC-18 | Low | — | S | — | Not started | Pepper-unavailable detection matches library message text | | SEC-19 | Low | — | S | — | Not started | Canonical audit store re-issues `CREATE TABLE` per write/read | -| SEC-20 | Low | P1 | S | — | Not started | `session_id` metric tag is unbounded cardinality | +| SEC-20 | Low | P1 | S | — | Done | `session_id` metric tag is unbounded cardinality | | SEC-21 | Low | — | M | — | Not started | Snapshot publisher works every second regardless of audience | | SEC-22 | Low | P2 | M | — | Not started | `docs/Authentication.md` documents types no longer in this repo | | SEC-23 | Low | — | S | SEC-03,04 | Not started | Validator ignores `DisableLogin`/`AutoLoginUser`/`RequireHttpsCookie`/`CookieName` | @@ -255,6 +255,7 @@ Findings the review flagged as one coordinated design pass — sequence them tog |---|---| | 2026-07-09 | Initial tracking doc generated from the six domain remediation designs. All 153 findings `Not started`. | | 2026-07-09 | P0 tier executed via parallel agents. GWC-01/02/03, TST-02, TST-12, CLI-01, CLI-03 → `Done` (NonWindows build clean; gateway 135/135 targeted tests pass, Go `go test ./...` + `-race` pass, Rust `test`/`clippy -D warnings` pass). | +| 2026-07-09 | P1 S-misc (dashboard/observability hardening). SEC-02/12/20 → `Done`. SEC-02: `DashboardAuthorizationHandler` restricts the loopback + `Authentication:Mode=Disabled` bypasses to read-only (they satisfy a Viewer-bearing requirement but never `AdminOnly`), closing the policy-layer gap where anonymous localhost was authorized for Admin surfaces. SEC-12: `DashboardSessionAdminService` now emits canonical `AuditEvent`s (`dashboard-close-session`/`dashboard-kill-worker`, category `SessionAdmin`) through `IAuditWriter` on Success/Failure/Denied, so Close/Kill land durable audit rows. SEC-20: dropped the unbounded `session_id` tag from the exported `mxgateway.heartbeats.failed` counter. Docs updated same-change (CLAUDE.md, GatewayDashboardDesign.md, Metrics.md). Server build clean (0 warnings); targeted classes 30/30 pass; broader Dashboard+Security+GatewayApplication+Metrics sweep 295/295 pass. | | 2026-07-09 | P1 Wave 2b (security authz+hub). SEC-05/07/08/11 → `Done` (hub-token lifetime; QueryActiveAlarms scope arm; gateway-side verification cache + last-used coalescing; login rate limit + per-peer gRPC failure limiter). Full-suite checkpoint caught + fixed regressions the earlier narrow SEC-01/04/06 filter missed: cross-platform path-rooting, an `IHostEnvironment` fallback for minimal DI containers, a test-assembly `ASPNETCORE_ENVIRONMENT=Development` default, and a platform-correct default-path assertion. Suite: 747 passed / 42 failed, all 42 pre-existing macOS named-pipe-harness env failures. | | 2026-07-09 | P1 Wave 2a (security). SEC-01/04/06 → `Done` (config path-rooting + production validator guards; Server build clean, validator+hygiene tests 53/53). SEC-10 → `Done`: the shared `ZB.MOM.WW.Auth.ApiKeys` gained optional `ExpiresUtc` (expired keys rejected, auth DB auto-migrates to schema v3) via a concurrent HistorianGateway-remediation session's "G-2"; this repo consumes it by bumping the four `Auth.*` refs 0.1.2→0.1.4 (commit 197731a). Remaining SEC-10 polish (`apikey create --expires` + dashboard staleness badge) tracked as a small follow-up. | | 2026-07-09 | P1 Wave 1 (CI + codegen freshness + Rust buildability) via parallel agents. IPC-01/09/19/20, CLI-02 → `Done`; TST-03 → `In review` (CI pipeline authored + YAML/layout-validated, but not yet executed on a Gitea runner — proven on first push). Verified on macOS: NonWindows build clean, `ClientProtoInputTests` 5/5, `publish-client-proto-inputs.ps1 -Check` exit 0, `cargo package` (no `--no-verify`) compiles standalone. Added a vendored-Rust-proto drift guard (Check 3) to `check-codegen.ps1` closing the CLI-02 static-copy risk. IPC-09 script guards not executed end-to-end (need pinned python/JRE toolchains); logic is PATH-resolution + version assertion. | diff --git a/docs/GatewayDashboardDesign.md b/docs/GatewayDashboardDesign.md index 9778a74..412f6d2 100644 --- a/docs/GatewayDashboardDesign.md +++ b/docs/GatewayDashboardDesign.md @@ -257,6 +257,15 @@ gated by a confirmation dialog before it reaches `ISessionManager`. is killed immediately with no graceful-shutdown attempt. The session is removed from the registry and the open-session slot is released either way. +Both actions write a canonical `AuditEvent` through `IAuditWriter` into the +`audit_event` store (category `SessionAdmin`, actions `dashboard-close-session` +and `dashboard-kill-worker`) in addition to the operational `ILogger` line — so a +worker killed mid-production leaves a durable, queryable row rather than only a +rotatable log entry. The event records the LDAP actor, the session id (`Target`), +the remote address, and an outcome of `Success`, `Failure`, or `Denied` +(an unauthorized attempt is still audited as `Denied`). This mirrors the API-key +management audit path. + ### Workers page Show: @@ -436,11 +445,16 @@ Three authorization policies are registered: cookie OR a `MxGateway.Dashboard.HubToken` bearer (used by WebSocket upgrades where the cookie can't be forwarded). -Two environmental bypasses still apply: `MxGateway:Authentication:Mode = Disabled` -authorizes every request, and `MxGateway:Dashboard:AllowAnonymousLocalhost` -(default `true`) authorizes any loopback request without a role check. Remote -requests always require an authenticated principal carrying at least the -Viewer role. +Two environmental bypasses still apply, both scoped to **read-only** access: +`MxGateway:Authentication:Mode = Disabled` and `MxGateway:Dashboard:AllowAnonymousLocalhost` +(default `true`, loopback only) each satisfy a requirement that includes the Viewer +role (`AnyDashboardRole`) but **never** the `AdminOnly` requirement. Destructive/admin +surfaces (API-key CRUD, session Close, worker Kill) still demand a real `Administrator` +role claim, so the "anonymous localhost is read-only" contract holds at the policy layer +rather than only in downstream service re-checks. Remote requests (outside the `Disabled` +bypass) always require an authenticated principal carrying at least the Viewer role. The +loopback test trusts `Connection.RemoteIpAddress`; if forwarded-headers middleware is ever +added upstream, `DashboardAuthorizationHandler.IsLoopbackRequest()` must be revisited. ### DisableLogin dev bypass diff --git a/docs/Metrics.md b/docs/Metrics.md index 64836a4..6b40611 100644 --- a/docs/Metrics.md +++ b/docs/Metrics.md @@ -44,7 +44,7 @@ All counters are `Counter`. Tag values come from the call sites listed und | `mxgateway.faults` | `category` | Faults reported by session, event, or worker code paths. The category is a `SessionManagerErrorCode` or `WorkerClientErrorCode` name. | | `mxgateway.workers.killed` | `reason` | Forced terminations of worker processes. | | `mxgateway.workers.exited` | `reason` | Clean or fault-driven worker exits. | -| `mxgateway.heartbeats.failed` | `session_id` | Worker heartbeat misses tracked per session. | +| `mxgateway.heartbeats.failed` | *(none)* | Aggregate worker heartbeat misses. The exported counter carries **no** `session_id` tag — per-session attribution is unbounded cardinality (every session mints a new time series), so it is kept out of the exporter; use the dashboard snapshot or the log scope for per-session detail. | | `mxgateway.grpc.streams.disconnected` | `reason` | Detachments of the dashboard or client gRPC event stream. | | `mxgateway.retries.attempted` | `area` | Resilience retries executed by gateway components. | diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthorizationHandler.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthorizationHandler.cs index 8e157bd..3692867 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthorizationHandler.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardAuthorizationHandler.cs @@ -11,6 +11,18 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard; /// (b) authentication is fully disabled, or (c) the request is from loopback /// and MxGateway:Dashboard:AllowAnonymousLocalhost is on. /// +/// +/// The environment bypasses in (b) and (c) grant read-only access only: they +/// satisfy a requirement that includes (i.e. +/// ) but never the +/// requirement. Destructive/admin +/// surfaces still require a real Admin role claim, preserving the documented +/// "anonymous localhost is read-only" contract at the policy layer rather than by accident +/// in downstream service re-checks. The loopback test trusts +/// Connection.RemoteIpAddress; if forwarded-headers middleware is ever added upstream, +/// must be revisited so a spoofed X-Forwarded-For +/// cannot masquerade as loopback. +/// public sealed class DashboardAuthorizationHandler( IHttpContextAccessor httpContextAccessor, IOptions options) : AuthorizationHandler @@ -22,14 +34,18 @@ public sealed class DashboardAuthorizationHandler( { GatewayOptions gatewayOptions = options.Value; - if (gatewayOptions.Authentication.Mode == AuthenticationMode.Disabled) + // Environment bypasses are read-only: they satisfy the Viewer requirement + // (AnyDashboardRole) but never AdminOnly, which lacks Viewer in RequiredRoles. + bool grantsReadOnly = requirement.RequiredRoles.Contains(DashboardRoles.Viewer); + + if (grantsReadOnly && gatewayOptions.Authentication.Mode == AuthenticationMode.Disabled) { context.Succeed(requirement); return Task.CompletedTask; } - if (gatewayOptions.Dashboard.AllowAnonymousLocalhost && IsLoopbackRequest()) + if (grantsReadOnly && gatewayOptions.Dashboard.AllowAnonymousLocalhost && IsLoopbackRequest()) { context.Succeed(requirement); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs index b19b1a1..595530b 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSessionAdminService.cs @@ -1,6 +1,8 @@ using System.Security.Claims; +using System.Text.Json; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using ZB.MOM.WW.Audit; using ZB.MOM.WW.MxGateway.Server.Sessions; namespace ZB.MOM.WW.MxGateway.Server.Dashboard; @@ -8,21 +10,35 @@ namespace ZB.MOM.WW.MxGateway.Server.Dashboard; /// /// Default implementation of : gates /// destructive session actions on the role, -/// audit-logs successful operations, and converts -/// (and any other unexpected exceptions) into -/// so the Blazor pages never see a raw exception. +/// records each attempt as a canonical through +/// (in addition to the operational line), and converts +/// (and any other unexpected exceptions) into +/// so the Blazor pages never see a raw exception. /// /// /// The constant dashboard-admin-kill is the reason passed to /// and forwarded as the /// reason tag on the mxgateway.workers.killed counter and in -/// the worker-kill audit log entries. +/// the worker-kill audit log entries. Destructive dashboard actions write durable, +/// queryable audit rows (actions dashboard-close-session / dashboard-kill-worker) +/// to the canonical audit_event store, mirroring the API-key management path so a +/// worker killed mid-production is not visible only in a rotatable line. /// public sealed class DashboardSessionAdminService( ISessionManager sessionManager, IHttpContextAccessor httpContextAccessor, + IAuditWriter auditWriter, ILogger? logger = null) : IDashboardSessionAdminService { + /// Canonical for dashboard session-admin actions. + internal const string SessionAdminCategory = "SessionAdmin"; + + /// Canonical for a dashboard session close. + internal const string CloseSessionAction = "dashboard-close-session"; + + /// Canonical for a dashboard worker kill. + internal const string KillWorkerAction = "dashboard-kill-worker"; + private const string UnauthorizedMessage = "Sign in as an Admin to close sessions or kill workers."; private const string KillReason = "dashboard-admin-kill"; @@ -46,6 +62,8 @@ public sealed class DashboardSessionAdminService( { if (!CanManage(user)) { + await WriteAuditAsync(user, CloseSessionAction, sessionId, AuditOutcome.Denied, "unauthorized", cancellationToken) + .ConfigureAwait(false); return DashboardSessionAdminResult.Fail(UnauthorizedMessage); } @@ -68,6 +86,15 @@ public sealed class DashboardSessionAdminService( ResolveRemoteAddress(), result.AlreadyClosed); + await WriteAuditAsync( + user, + CloseSessionAction, + sessionId, + AuditOutcome.Success, + result.AlreadyClosed ? "alreadyClosed" : "closed", + cancellationToken) + .ConfigureAwait(false); + return DashboardSessionAdminResult.Success( result.AlreadyClosed ? $"Session {sessionId} was already closed." @@ -75,6 +102,8 @@ public sealed class DashboardSessionAdminService( } catch (SessionManagerException exception) when (exception.ErrorCode == SessionManagerErrorCode.SessionNotFound) { + await WriteAuditAsync(user, CloseSessionAction, sessionId, AuditOutcome.Failure, "not-found", cancellationToken) + .ConfigureAwait(false); return DashboardSessionAdminResult.Fail($"Session {sessionId} was not found."); } catch (SessionManagerException exception) @@ -84,6 +113,8 @@ public sealed class DashboardSessionAdminService( "Dashboard admin {Actor} close failed for session {SessionId}.", actor, sessionId); + await WriteAuditAsync(user, CloseSessionAction, sessionId, AuditOutcome.Failure, exception.Message, cancellationToken) + .ConfigureAwait(false); return DashboardSessionAdminResult.Fail( $"Close failed: {exception.Message}"); } @@ -98,6 +129,8 @@ public sealed class DashboardSessionAdminService( "Dashboard admin {Actor} close failed unexpectedly for session {SessionId}.", actor, sessionId); + await WriteAuditAsync(user, CloseSessionAction, sessionId, AuditOutcome.Failure, "unexpected", cancellationToken) + .ConfigureAwait(false); return DashboardSessionAdminResult.Fail( $"Close failed unexpectedly for session {sessionId}. See the gateway log for details."); } @@ -111,6 +144,8 @@ public sealed class DashboardSessionAdminService( { if (!CanManage(user)) { + await WriteAuditAsync(user, KillWorkerAction, sessionId, AuditOutcome.Denied, "unauthorized", cancellationToken) + .ConfigureAwait(false); return DashboardSessionAdminResult.Fail(UnauthorizedMessage); } @@ -133,6 +168,15 @@ public sealed class DashboardSessionAdminService( ResolveRemoteAddress(), result.AlreadyClosed); + await WriteAuditAsync( + user, + KillWorkerAction, + sessionId, + AuditOutcome.Success, + result.AlreadyClosed ? "alreadyClosed" : "killed", + cancellationToken) + .ConfigureAwait(false); + return DashboardSessionAdminResult.Success( result.AlreadyClosed ? $"Session {sessionId} was already closed." @@ -140,6 +184,8 @@ public sealed class DashboardSessionAdminService( } catch (SessionManagerException exception) when (exception.ErrorCode == SessionManagerErrorCode.SessionNotFound) { + await WriteAuditAsync(user, KillWorkerAction, sessionId, AuditOutcome.Failure, "not-found", cancellationToken) + .ConfigureAwait(false); return DashboardSessionAdminResult.Fail($"Session {sessionId} was not found."); } catch (SessionManagerException exception) @@ -149,6 +195,8 @@ public sealed class DashboardSessionAdminService( "Dashboard admin {Actor} kill failed for session {SessionId}.", actor, sessionId); + await WriteAuditAsync(user, KillWorkerAction, sessionId, AuditOutcome.Failure, exception.Message, cancellationToken) + .ConfigureAwait(false); return DashboardSessionAdminResult.Fail( $"Kill failed: {exception.Message}"); } @@ -163,6 +211,8 @@ public sealed class DashboardSessionAdminService( "Dashboard admin {Actor} kill failed unexpectedly for session {SessionId}.", actor, sessionId); + await WriteAuditAsync(user, KillWorkerAction, sessionId, AuditOutcome.Failure, "unexpected", cancellationToken) + .ConfigureAwait(false); return DashboardSessionAdminResult.Fail( $"Kill failed unexpectedly for session {sessionId}. See the gateway log for details."); } @@ -177,4 +227,51 @@ public sealed class DashboardSessionAdminService( { return httpContextAccessor.HttpContext?.Connection.RemoteIpAddress?.ToString(); } + + /// + /// Writes a canonical for a dashboard session-admin action through the + /// best-effort (failures are swallowed/logged by the writer, so this + /// never throws and never masks the operation result). is the LDAP + /// operator, the session id, and is wrapped + /// as the detail field of the JSON extension bag. + /// + private async Task WriteAuditAsync( + ClaimsPrincipal user, + string action, + string sessionId, + AuditOutcome outcome, + string? detail, + CancellationToken cancellationToken) + { + AuditEvent auditEvent = new() + { + EventId = Guid.NewGuid(), + OccurredAtUtc = DateTimeOffset.UtcNow, + Actor = ResolveActor(user), + Action = action, + Outcome = outcome, + Category = SessionAdminCategory, + Target = sessionId, + SourceNode = ResolveRemoteAddress(), + CorrelationId = ParseCorrelationId(), + DetailsJson = WrapDetail(detail), + }; + + await auditWriter.WriteAsync(auditEvent, cancellationToken).ConfigureAwait(false); + } + + /// + /// Derives a correlation id from the 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 }); } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs b/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs index 2164661..c42468f 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Metrics/GatewayMetrics.cs @@ -351,7 +351,11 @@ public sealed class GatewayMetrics : IDisposable _heartbeatFailures++; } - _heartbeatFailuresCounter.Add(1, new KeyValuePair("session_id", sessionId)); + // Exported without a session_id tag: per-session attribution is unbounded cardinality + // (every session ever opened mints a new exporter time series, bloating Prometheus/OTLP + // storage on long-running gateways). Per-session detail stays available via the dashboard + // snapshot and the log scope; the exported counter tracks only the aggregate. + _heartbeatFailuresCounter.Add(1); } /// diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs index 9d4ab2d..7423c2e 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardAuthorizationHandlerTests.cs @@ -128,11 +128,58 @@ public sealed class DashboardAuthorizationHandlerTests Assert.False(context.HasSucceeded); } + /// + /// Verifies that the anonymous-localhost bypass is read-only: a loopback request with + /// AllowAnonymousLocalhost on is denied the + /// requirement, so destructive/admin surfaces are never reachable without a real Admin role. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task HandleAsync_AnonymousLocalhostAllowed_DoesNotSatisfyAdminPolicy() + { + AuthorizationHandlerContext context = await AuthorizeAsync( + new ClaimsPrincipal(new ClaimsIdentity()), + IPAddress.Loopback, + allowAnonymousLocalhost: true, + DashboardAuthorizationRequirement.AdminOnly); + + Assert.False(context.HasSucceeded); + } + + /// + /// Verifies that with authentication fully disabled the environment bypass stays read-only: + /// a remote request satisfies + /// (Viewer) but is denied . + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task HandleAsync_AuthenticationDisabled_GrantsViewerButNotAdmin() + { + AuthorizationHandlerContext viewerContext = await AuthorizeAsync( + new ClaimsPrincipal(new ClaimsIdentity()), + IPAddress.Parse("10.0.0.5"), + allowAnonymousLocalhost: false, + DashboardAuthorizationRequirement.AnyDashboardRole, + authenticationMode: AuthenticationMode.Disabled); + + Assert.True(viewerContext.HasSucceeded); + + AuthorizationHandlerContext adminContext = await AuthorizeAsync( + new ClaimsPrincipal(new ClaimsIdentity()), + IPAddress.Parse("10.0.0.5"), + allowAnonymousLocalhost: false, + DashboardAuthorizationRequirement.AdminOnly, + authenticationMode: AuthenticationMode.Disabled); + + Assert.False(adminContext.HasSucceeded); + } + private static async Task AuthorizeAsync( ClaimsPrincipal principal, IPAddress remoteAddress, bool allowAnonymousLocalhost, - DashboardAuthorizationRequirement requirement) + DashboardAuthorizationRequirement requirement, + AuthenticationMode authenticationMode = AuthenticationMode.ApiKey) { DefaultHttpContext httpContext = new(); httpContext.Connection.RemoteIpAddress = remoteAddress; @@ -140,6 +187,10 @@ public sealed class DashboardAuthorizationHandlerTests new HttpContextAccessor { HttpContext = httpContext }, Options.Create(new GatewayOptions { + Authentication = new AuthenticationOptions + { + Mode = authenticationMode, + }, Dashboard = new DashboardOptions { AllowAnonymousLocalhost = allowAnonymousLocalhost, diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSessionAdminServiceTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSessionAdminServiceTests.cs index 08d7657..f254ba1 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSessionAdminServiceTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Gateway/Dashboard/DashboardSessionAdminServiceTests.cs @@ -1,6 +1,8 @@ +using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Security.Claims; using Microsoft.AspNetCore.Http; +using ZB.MOM.WW.Audit; using ZB.MOM.WW.MxGateway.Contracts.Proto; using ZB.MOM.WW.MxGateway.Server.Dashboard; using ZB.MOM.WW.MxGateway.Server.Sessions; @@ -198,14 +200,91 @@ public sealed class DashboardSessionAdminServiceTests Assert.False(string.IsNullOrWhiteSpace(result.Message)); } - private static DashboardSessionAdminService CreateService(ISessionManager sessionManager) + /// + /// Verifies that a successful close writes a canonical dashboard-close-session + /// — actor, session-id target, and Success outcome — to the + /// audit store, not only the operational log (SEC-12). + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task CloseSessionAsync_AdminClose_WritesCanonicalAuditEvent() + { + FakeSessionManager sessionManager = new(); + RecordingAuditWriter auditWriter = new(); + DashboardSessionAdminService service = CreateService(sessionManager, auditWriter); + + DashboardSessionAdminResult result = await service.CloseSessionAsync( + CreateUser(DashboardRoles.Admin), + "session-1", + CancellationToken.None); + + Assert.True(result.Succeeded); + AuditEvent audit = Assert.Single(auditWriter.Events); + Assert.Equal(DashboardSessionAdminService.CloseSessionAction, audit.Action); + Assert.Equal(DashboardSessionAdminService.SessionAdminCategory, audit.Category); + Assert.Equal("session-1", audit.Target); + Assert.Equal("tester", audit.Actor); + Assert.Equal(AuditOutcome.Success, audit.Outcome); + } + + /// + /// Verifies that a successful kill writes a canonical dashboard-kill-worker + /// to the audit store (SEC-12). + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task KillWorkerAsync_AdminKill_WritesCanonicalAuditEvent() + { + FakeSessionManager sessionManager = new(); + RecordingAuditWriter auditWriter = new(); + DashboardSessionAdminService service = CreateService(sessionManager, auditWriter); + + DashboardSessionAdminResult result = await service.KillWorkerAsync( + CreateUser(DashboardRoles.Admin), + "session-1", + CancellationToken.None); + + Assert.True(result.Succeeded); + AuditEvent audit = Assert.Single(auditWriter.Events); + Assert.Equal(DashboardSessionAdminService.KillWorkerAction, audit.Action); + Assert.Equal("session-1", audit.Target); + Assert.Equal(AuditOutcome.Success, audit.Outcome); + } + + /// + /// Verifies that an unauthorized (viewer) close attempt still writes a Denied + /// audit row, so rejected destructive attempts are durably recorded (SEC-12). + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task CloseSessionAsync_ViewerDenied_WritesDeniedAuditEvent() + { + FakeSessionManager sessionManager = new(); + RecordingAuditWriter auditWriter = new(); + DashboardSessionAdminService service = CreateService(sessionManager, auditWriter); + + DashboardSessionAdminResult result = await service.CloseSessionAsync( + CreateUser(DashboardRoles.Viewer), + "session-1", + CancellationToken.None); + + Assert.False(result.Succeeded); + AuditEvent audit = Assert.Single(auditWriter.Events); + Assert.Equal(DashboardSessionAdminService.CloseSessionAction, audit.Action); + Assert.Equal(AuditOutcome.Denied, audit.Outcome); + } + + private static DashboardSessionAdminService CreateService( + ISessionManager sessionManager, + IAuditWriter? auditWriter = null) { DefaultHttpContext httpContext = new(); httpContext.Connection.RemoteIpAddress = System.Net.IPAddress.Loopback; return new DashboardSessionAdminService( sessionManager, - new HttpContextAccessor { HttpContext = httpContext }); + new HttpContextAccessor { HttpContext = httpContext }, + auditWriter ?? new RecordingAuditWriter()); } private static ClaimsPrincipal CreateUser(string role) @@ -219,6 +298,21 @@ public sealed class DashboardSessionAdminServiceTests return new ClaimsPrincipal(identity); } + private sealed class RecordingAuditWriter : IAuditWriter + { + private readonly ConcurrentQueue _events = new(); + + /// Gets the audit events written through this writer, in order. + public IReadOnlyList Events => _events.ToArray(); + + /// + public Task WriteAsync(AuditEvent evt, CancellationToken ct = default) + { + _events.Enqueue(evt); + return Task.CompletedTask; + } + } + private sealed class FakeSessionManager : ISessionManager { /// Gets the number of times CloseSessionAsync was invoked. diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs index 0181f5e..2dec8bd 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Metrics/GatewayMetricsTests.cs @@ -157,6 +157,56 @@ public sealed class GatewayMetricsTests Assert.Equal(2, capturedMode); } + /// + /// Verifies that increments + /// mxgateway.heartbeats.failed without emitting a session_id tag: the tag is + /// unbounded cardinality (every session mints a new exporter time series), so per-session + /// attribution is deliberately kept out of the exported counter (SEC-20). + /// + [Fact] + public void HeartbeatFailed_IncrementsCounterWithoutSessionIdTag() + { + using GatewayMetrics metrics = new(); + using MeterListener listener = new(); + + long capturedValue = 0; + bool sawSessionIdTag = false; + + listener.InstrumentPublished = (instrument, meterListener) => + { + if (ReferenceEquals(instrument.Meter, metrics.Meter) + && instrument.Name == "mxgateway.heartbeats.failed") + { + meterListener.EnableMeasurementEvents(instrument); + } + }; + listener.SetMeasurementEventCallback( + (instrument, measurement, tags, _) => + { + if (!ReferenceEquals(instrument.Meter, metrics.Meter) + || instrument.Name != "mxgateway.heartbeats.failed") + { + return; + } + + capturedValue += measurement; + foreach (KeyValuePair tag in tags) + { + if (tag.Key == "session_id") + { + sawSessionIdTag = true; + } + } + }); + listener.Start(); + + metrics.HeartbeatFailed("session-1"); + + Assert.Equal(1, capturedValue); + Assert.False(sawSessionIdTag); + Assert.Equal(1, metrics.GetSnapshot().HeartbeatFailures); + } + /// Verifies that removing session events only affects that session. [Fact] public void RemoveSessionEvents_RemovesOnlyThatSession()