From a038363e74faf7792bfbf6506c3b0dba1f0b838e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 9 Jul 2026 09:48:09 -0400 Subject: [PATCH] feat(security): apikey --expires CLI flag + dashboard expiry/staleness badge (SEC-10 polish) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gateway-side follow-up to the shared auth-lib expiry core (delivered via G-2): - apikey create-key gains optional --expires — absolute ISO-8601 UTC or relative d/h from now; omitted means non-expiring (opt-in, unchanged default). Threaded ApiKeyAdminCommand -> parser -> runner into the library's CreateKeyAsync(..., expiresUtc, ...). list-keys shows an expiry column and an active/expired/revoked status. - Dashboard API Keys page surfaces expiry: an Expires column (Never when unset) and a status badge reading Expired (red) / Expiring (<=7d, amber) / Revoked / Active. DashboardApiKeySummary.ExpiresUtc projected in DashboardSnapshotService; StatusBadge maps the new states. Tests: parser (absolute/relative/invalid/none) + end-to-end past-expiry rejection through the live verifier; dashboard summary suite green. Docs: Authentication.md (verification-flow expiry step, CLI table + examples, dashboard badge). Closes the tracked SEC-10 polish (SEC-10 core was already Done). --- archreview/remediation/00-tracking.md | 1 + docs/Authentication.md | 10 +++- .../Components/Pages/ApiKeysPage.razor | 33 ++++++++++- .../Components/Shared/StatusBadge.razor | 4 +- .../Dashboard/DashboardApiKeySummary.cs | 3 +- .../Dashboard/DashboardSnapshotService.cs | 3 +- .../Authentication/ApiKeyAdminCliRunner.cs | 16 +++++- .../Authentication/ApiKeyAdminCommand.cs | 3 +- .../ApiKeyAdminCommandLineParser.cs | 41 +++++++++++++- .../Authentication/ApiKeyAdminListedKey.cs | 3 +- .../ApiKeyAdminCliRunnerTests.cs | 34 ++++++++++++ .../ApiKeyAdminCommandLineParserTests.cs | 55 +++++++++++++++++++ 12 files changed, 192 insertions(+), 14 deletions(-) diff --git a/archreview/remediation/00-tracking.md b/archreview/remediation/00-tracking.md index 7f729e3..c5c3a63 100644 --- a/archreview/remediation/00-tracking.md +++ b/archreview/remediation/00-tracking.md @@ -257,6 +257,7 @@ Findings the review flagged as one coordinated design pass — sequence them tog | 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 | Windows full-suite temp-file-lock flakiness (discovered via TST-08) → **fixed** (commit `11a716a`). Two shared-state parallel collisions that macOS never surfaces: (1) self-signed cert generation — `SelfSignedCertificateProvider` now stages the PFX in a unique `..tmp` (was a fixed name that concurrent/interrupted writers collided on); `TestHostEnvironmentInitializer` defaults the cert path to a per-process temp dir (so the suite never writes shared `ProgramData` or fights the deployed service); `GatewayTlsBootstrapTests` moved to a `DisableParallelization` collection so its process-global env-var mutation can't bleed into parallel tests. (2) `AuthStoreHealthCheckTests` — reuse the existing `TempDatabaseDirectory` helper (`SqliteConnection.ClearAllPools()` before delete) so the Microsoft.Data.Sqlite pool releases the temp `.db` handle. **windev-verified:** 3 consecutive full-suite runs all **793 passed / 2 failed, 0 new orphans** (was flaky 2–4 fails, count-varying). The 2 stable remaining failures are pre-existing windev-environmental (`SelfSignedCertificateProviderTests.GenerateCertificate_HasExpectedSansEkuAndValidity` machine-name/FQDN SAN assertion; `EventStreamServiceTests…TracksAggregateQueueDepth` timing) — both pass on macOS, unrelated to this change. | | 2026-07-09 | P1 TST-08 (orphaned testhost) → `Done` by evidence: the claimed leak does **not** reproduce. Full `ZB.MOM.WW.MxGateway.Tests` suite exits cleanly on macOS (0 survivors) and on the Windows dev box (windev, `origin/main` worktree: 0 new `testhost` and 0 new `ZB.MOM.WW.MxGateway.Worker` processes after a full run, isolated by StartTime). Static audit confirms the prime-suspect fixtures already dispose deterministically (WorkerClient cancels its `_stopCts` + awaits read/write/heartbeat tasks with a 5 s timeout; GatewaySession disposes the client; E2E/harness fixtures use `await using`). CLAUDE.md "Source Update Workflow" updated to drop the stale "leaves orphaned testhost processes" rationale (filtered runs kept as a speed guideline). **Discovered separately (NOT TST-08, still open):** Windows-only temp-file-lock flakiness in ~4 full-host test classes — `AuthStoreHealthCheckTests` (Microsoft.Data.Sqlite connection-pool holds the temp `.db` handle after `await using`, so the finally `File.Delete` throws "used by another process"), `GatewayTlsBootstrapTests`/`DashboardCookieOptionsTests`/`DashboardHubsRegistrationTests` (a `gw.pfx.tmp` handle from `SelfSignedCertificateProvider`'s fixed-name `path+".tmp"` atomic-write racing teardown). Count varies run-to-run (2–4 fails); macOS never sees it (Unix allows deleting open files). Fix = `SqliteConnection.ClearAllPools()` before temp delete + a unique/guarded `.tmp` name; verify on windev. | +| 2026-07-09 | SEC-10 polish (gateway-side follow-up to the G-2 auth-lib core) → done. `apikey create-key` gains an optional `--expires` (absolute ISO-8601 UTC or relative `d`/`h`; omit = non-expiring, opt-in) threaded through `ApiKeyAdminCommand`/parser/runner into the library's `CreateKeyAsync(..., expiresUtc, ...)`; `list-keys` shows an expiry column + `active`/`expired`/`revoked` status. Dashboard API Keys page surfaces expiry: an `Expires` column (`Never` when unset) and a status badge reading `Expired` (red) / `Expiring` (≤7 days, amber) / `Revoked` / `Active` (`DashboardApiKeySummary.ExpiresUtc` projected in `DashboardSnapshotService`; `StatusBadge` maps the new states). Server build clean; targeted tests 32/32 (parser abs/relative/invalid + end-to-end past-expiry rejection via the live verifier) + dashboard 28/28. Docs: `docs/Authentication.md` (verification flow expiry step, CLI table + examples, dashboard badge). SEC-10 was already `Done` (core); this closes the tracked polish. | | 2026-07-09 | P1 Wave 3 (event-channel decoupling). GWC-04 → `Done`. `WorkerClient`'s read loop awaited `EnqueueWorkerEventAsync` inline, which blocks in the bounded event channel's timed `WriteAsync` (≤ `EventChannelFullModeTimeout`, 5 s) when the channel is full with a slow/absent `StreamEvents` consumer — stalling any `WorkerCommandReply`/heartbeat queued behind an event frame, so an in-flight `InvokeAsync` could hit `CommandTimeout` despite a timely worker reply. Fix mirrors the existing outbound `WriteLoopAsync`: an unbounded event **staging** channel + a dedicated `EventWriteLoopAsync`; `DispatchEnvelope` is now fully synchronous and the `WorkerEvent` branch hands off with a non-blocking `TryWrite`, so the read loop never awaits event enqueue. The event write loop owns the timed write + the sustained-overflow `ProtocolViolation` fault (unchanged contract); registered in `WaitForBackgroundTasks`, completed on close/fault/dispose. Server build clean (0 warnings); non-pipe event tests 37/38 (the 1 is the known parallel-load `EventStreamServiceTests…TracksAggregateQueueDepth` timing flake — passes in isolation). New pipe-harness test (reply after events with a full consumer-less channel dispatches without `CommandTimeout`) verified on windev. Docs: GatewayProcessDesign read/write/event-loop section. Commit `32d6b48`. **Wave 3 complete.** | | 2026-07-09 | P1 Wave 3 (size/backpressure topology + write ordering). IPC-02/03/04 + WRK-04/07 → `Done`. **Size negotiation (IPC-02):** added `GatewayHello.max_frame_bytes` (regen `Generated/` + `clients/proto` descriptor refresh with pinned protoc 34.1); the gateway sends its negotiated worker-frame max and the worker adopts it (`WorkerFrameProtocolOptions.AdoptNegotiatedMaxMessageBytes`, 0 = keep default, >256 MiB rejected) instead of a hard-coded default. **Headroom (IPC-03):** the pipe frame max now sits `EnvelopeOverheadReserveBytes` (64 KiB) above the public gRPC cap (default `Worker.MaxMessageBytes` 16 MiB→16 MiB+64 KiB), cross-validated at startup; `WorkerClient` pre-checks command envelope size and fails only the offending correlation (`ResourceExhausted`) instead of `SetFaulted`ing the session. **Drain bound (IPC-04):** gateway request validator rejects `DrainEvents max_events` above 10 000; the worker caps each reply at `MaxDrainEventsPerReply` (10 000) and treats `max_events = 0` as that cap, not "drain all". **Sequence (WRK-04):** `WorkerFrameWriter` stamps the envelope `Sequence` at the point of writing under the write lock, so wire order and stamped sequence always agree under concurrent producers. **Priority (WRK-07):** the worker writer is now a cooperative priority scheduler — control frames (reply/fault/heartbeat/shutdown-ack) drain ahead of event frames; per-frame validation/size rejections fail only that frame, a stream failure fails all queued. Docs same-change (GatewayConfiguration, WorkerFrameProtocol, gateway.md). **Verified:** macOS NonWindows build clean + validator/grpc tests green; **windev** x86 worker builds clean, `Worker.Tests` 352 passed / 0 failed / 11 skipped (incl. new monotonic-sequence, control-before-event priority, negotiated-max, drain-bound tests), gateway `Tests` 799 passed / 3 failed — all 3 pre-existing windev-environmental (SelfSigned SAN + 2 `EventStreamServiceTests` timing, both pass in isolation). Commits `c8b3a22` (gateway half), `ebe6aea` (worker half), `309296f` (descriptor + default-expectation refresh). GWC-04 (event-channel decoupling) is the remaining Wave 3 item. | | 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. | diff --git a/docs/Authentication.md b/docs/Authentication.md index 29a17a3..ab3f0e6 100644 --- a/docs/Authentication.md +++ b/docs/Authentication.md @@ -73,7 +73,7 @@ The pepper is intentionally not stored alongside the hash: an attacker who exfil 1. Parse the `Authorization` header into a `ParsedApiKey`. 2. Look up the `ApiKeyRecord` by `KeyId` through `IApiKeyStore.FindByKeyIdAsync`. -3. Reject revoked records (`RevokedUtc is not null`). +3. Reject revoked records (`RevokedUtc is not null`) and expired records (`ExpiresUtc` in the past). Expiry is opt-in — keys created without an expiry never expire; an expired key fails opaquely, indistinguishable to the client from any other auth failure. 4. Hash the presented secret with the configured pepper. 5. Compare hashes with `CryptographicOperations.FixedTimeEquals` to avoid timing oracles. 6. Record a `LastUsedUtc` timestamp via `MarkKeyUsedAsync` and return an `ApiKeyIdentity`. @@ -193,6 +193,8 @@ public static ApiKeyRecord Read(SqliteDataReader reader) Because `RotateAsync` clears `revoked_utc`, rotating a previously revoked key reactivates it. The dashboard API Keys page therefore offers the Rotate (and Revoke) actions only for keys whose status is `Active`; revoked keys instead show a Delete action that calls `DeleteAsync`, so an operator can permanently remove a revoked row without ever risking un-revocation as a side effect of a rotation. +The dashboard API Keys page also surfaces expiry: each row shows an `Expires` column (`Never` when unset) and a status badge that reads `Expired` (past expiry, red), `Expiring` (within seven days, amber), `Revoked`, or `Active`. This is display-only staleness surfacing; expiry is set at creation time via the `apikey create-key --expires` CLI, not from the dashboard. + ### Audit trail `SqliteApiKeyAuditStore` (`IApiKeyAuditStore`) appends `ApiKeyAuditEntry` values to the `api_key_audit` table and stamps each row with a UTC timestamp inside the store rather than trusting the caller. `ListRecentAsync` returns the most recent rows ordered by `audit_id` descending and projects them into `ApiKeyAuditRecord`. Rows are kept even after the referenced key is revoked because the audit history is the durable record of administrative action; the `key_id` column is nullable to accommodate non-key-scoped events such as `init-db`. @@ -226,8 +228,8 @@ The supported subcommands match `ApiKeyAdminCommandKind` exactly: | Subcommand | Required options | Behaviour | |------------|------------------|-----------| | `init-db` | none | Runs the migrator and records an audit entry. | -| `create-key` | `--key-id`, `--display-name` | Generates a new secret, stores its peppered hash and optional constraints, and prints the assembled `mxgw__` token. | -| `list-keys` | none | Lists every stored key with its scopes, constraints, and revocation state. | +| `create-key` | `--key-id`, `--display-name` | Generates a new secret, stores its peppered hash and optional constraints, and prints the assembled `mxgw__` token. Optional `--expires` sets an expiry (absolute ISO-8601 UTC, or a relative `d`/`h` from now); omit it for a non-expiring key. | +| `list-keys` | none | Lists every stored key with its scopes, constraints, revocation state, and expiry (`active`/`expired`/`revoked`). | | `revoke-key` | `--key-id` | Sets `revoked_utc` if the key is currently active. | | `rotate-key` | `--key-id` | Replaces the secret hash and prints the new token. | @@ -237,6 +239,8 @@ Examples: mxgateway apikey init-db mxgateway apikey create-key --key-id ops.alice --display-name "Alice (ops)" --scopes read,write mxgateway apikey create-key --key-id area1.reader --display-name "Area 1 reader" --scopes invoke:read,metadata:read --read-subtree "Area1/*" --browse-subtree "Area1/*" +mxgateway apikey create-key --key-id ops.temp --display-name "Temp contractor" --scopes invoke:read --expires 90d +mxgateway apikey create-key --key-id ops.audit --display-name "Audit window" --scopes metadata:read --expires 2027-01-01T00:00:00Z mxgateway apikey list-keys --json mxgateway apikey revoke-key --key-id ops.alice mxgateway apikey rotate-key --key-id ops.alice diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor index e51d4a0..0a32be6 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Pages/ApiKeysPage.razor @@ -164,6 +164,7 @@ else Constraints Created Last Used + Expires @if (CanManageApiKeys) { Actions @@ -175,12 +176,13 @@ else { @key.KeyId - + @DashboardDisplay.Text(key.DisplayName) @DashboardDisplay.Text(string.Join(", ", key.Scopes.Order(StringComparer.Ordinal))) @DashboardDisplay.Text(ConstraintText(key.Constraints)) @DashboardDisplay.DateTime(key.CreatedUtc) @DashboardDisplay.DateTime(key.LastUsedUtc) + @(key.ExpiresUtc is null ? "Never" : DashboardDisplay.DateTime(key.ExpiresUtc)) @if (CanManageApiKeys) { @@ -468,6 +470,35 @@ else } } + // Window before an expiry within which a key is flagged as "Expiring" (warn) rather than "Active". + private static readonly TimeSpan ExpiringSoonWindow = TimeSpan.FromDays(7); + + // Status vocabulary understood by StatusBadge: Revoked wins over expiry; a past expiry is Expired + // (bad), an expiry inside ExpiringSoonWindow is Expiring (warn), otherwise Active (SEC-10). + private static string KeyStatus(DashboardApiKeySummary key) + { + if (key.RevokedUtc is not null) + { + return "Revoked"; + } + + if (key.ExpiresUtc is { } expiresAt) + { + DateTimeOffset now = DateTimeOffset.UtcNow; + if (expiresAt <= now) + { + return "Expired"; + } + + if (expiresAt - now <= ExpiringSoonWindow) + { + return "Expiring"; + } + } + + return "Active"; + } + private static string ConstraintText(ApiKeyConstraints constraints) { if (constraints.IsEmpty) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Shared/StatusBadge.razor b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Shared/StatusBadge.razor index e97f59f..e7e96a2 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Shared/StatusBadge.razor +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/Components/Shared/StatusBadge.razor @@ -9,8 +9,8 @@ { "Ready" or "Healthy" or "Active" => StatusState.Ok, "Creating" or "StartingWorker" or "WaitingForPipe" or "InitializingWorker" or "Closing" - or "Stale" or "Degraded" => StatusState.Warn, - "Faulted" or "Unavailable" => StatusState.Bad, + or "Stale" or "Degraded" or "Expiring" => StatusState.Warn, + "Faulted" or "Unavailable" or "Expired" => StatusState.Bad, _ => StatusState.Idle, }; } diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardApiKeySummary.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardApiKeySummary.cs index 820342b..41328d4 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardApiKeySummary.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardApiKeySummary.cs @@ -9,4 +9,5 @@ public sealed record DashboardApiKeySummary( ApiKeyConstraints Constraints, DateTimeOffset CreatedUtc, DateTimeOffset? LastUsedUtc, - DateTimeOffset? RevokedUtc); + DateTimeOffset? RevokedUtc, + DateTimeOffset? ExpiresUtc = null); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs index 9d2675b..7039f47 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Dashboard/DashboardSnapshotService.cs @@ -273,7 +273,8 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService Constraints: ApiKeyConstraintSerializer.Deserialize(key.ConstraintsJson), CreatedUtc: key.CreatedUtc, LastUsedUtc: key.LastUsedUtc, - RevokedUtc: key.RevokedUtc)) + RevokedUtc: key.RevokedUtc, + ExpiresUtc: key.ExpiresUtc)) .ToArray(); Volatile.Write(ref _apiKeySummaries, summaries); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCliRunner.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCliRunner.cs index 643b14e..e852397 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCliRunner.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCliRunner.cs @@ -67,6 +67,7 @@ public sealed class ApiKeyAdminCliRunner(ApiKeyAdminCommands commands) Required(command.DisplayName), command.Scopes, ApiKeyConstraintSerializer.Serialize(command.Constraints), + command.ExpiresUtc, remoteAddress: null, cancellationToken) .ConfigureAwait(false); @@ -134,8 +135,16 @@ public sealed class ApiKeyAdminCliRunner(ApiKeyAdminCommands commands) foreach (ApiKeyAdminListedKey key in result.Keys) { - string revoked = key.RevokedUtc is null ? "active" : "revoked"; - await output.WriteLineAsync($"{key.KeyId}\t{key.DisplayName}\t{revoked}\t{string.Join(',', key.Scopes)}") + string status = key.RevokedUtc is not null + ? "revoked" + : key.ExpiresUtc is { } expiresAt && expiresAt <= DateTimeOffset.UtcNow + ? "expired" + : "active"; + string expiry = key.ExpiresUtc is { } expires + ? expires.ToUniversalTime().ToString("u", System.Globalization.CultureInfo.InvariantCulture) + : "-"; + await output.WriteLineAsync( + $"{key.KeyId}\t{key.DisplayName}\t{status}\t{expiry}\t{string.Join(',', key.Scopes)}") .ConfigureAwait(false); } } @@ -150,7 +159,8 @@ public sealed class ApiKeyAdminCliRunner(ApiKeyAdminCommands commands) Constraints: ApiKeyConstraintSerializer.Deserialize(key.ConstraintsJson), CreatedUtc: key.CreatedUtc, LastUsedUtc: key.LastUsedUtc, - RevokedUtc: key.RevokedUtc); + RevokedUtc: key.RevokedUtc, + ExpiresUtc: key.ExpiresUtc); } private static string Required(string? value) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommand.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommand.cs index b2d5105..46c6f2d 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommand.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommand.cs @@ -8,4 +8,5 @@ public sealed record ApiKeyAdminCommand( string? KeyId, string? DisplayName, IReadOnlySet Scopes, - ApiKeyConstraints Constraints); + ApiKeyConstraints Constraints, + DateTimeOffset? ExpiresUtc = null); diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommandLineParser.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommandLineParser.cs index 315c0fc..6ec44da 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommandLineParser.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminCommandLineParser.cs @@ -82,9 +82,11 @@ public static class ApiKeyAdminCommandLineParser string? displayName = GetOption(options, "display-name"); IReadOnlySet scopes = ParseScopes(GetOption(options, "scopes")); ApiKeyConstraints constraints; + DateTimeOffset? expiresUtc; try { constraints = ParseConstraints(options); + expiresUtc = ParseExpiry(GetOption(options, "expires")); } catch (FormatException exception) { @@ -111,7 +113,8 @@ public static class ApiKeyAdminCommandLineParser KeyId: keyId, DisplayName: displayName, Scopes: scopes, - Constraints: constraints)); + Constraints: constraints, + ExpiresUtc: expiresUtc)); } private static bool TryParseKind(string value, out ApiKeyAdminCommandKind kind) @@ -233,6 +236,42 @@ public static class ApiKeyAdminCommandLineParser ReadHistorizedOnly: HasFlag(options, "read-historized-only")); } + // Parses the optional --expires value into an absolute UTC expiry (SEC-10). Accepts a relative + // "d"/"h" duration from now (operator-friendly) or an absolute ISO-8601 instant/date + // (assumed UTC). Null/blank means no expiry — expiry stays opt-in, preserving prior behaviour. + private static DateTimeOffset? ParseExpiry(string? value) + { + if (string.IsNullOrWhiteSpace(value)) + { + return null; + } + + string trimmed = value.Trim(); + char unit = char.ToLowerInvariant(trimmed[^1]); + if (unit is 'd' or 'h' + && int.TryParse( + trimmed[..^1], + System.Globalization.NumberStyles.None, + System.Globalization.CultureInfo.InvariantCulture, + out int amount)) + { + TimeSpan offset = unit == 'd' ? TimeSpan.FromDays(amount) : TimeSpan.FromHours(amount); + return DateTimeOffset.UtcNow + offset; + } + + if (DateTimeOffset.TryParse( + trimmed, + System.Globalization.CultureInfo.InvariantCulture, + System.Globalization.DateTimeStyles.AssumeUniversal | System.Globalization.DateTimeStyles.AdjustToUniversal, + out DateTimeOffset parsed)) + { + return parsed; + } + + throw new FormatException( + "--expires must be a relative duration ('d' or 'h') or an absolute ISO-8601 UTC timestamp."); + } + private static int? ParseNullableInt(string? value) { if (string.IsNullOrWhiteSpace(value)) diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminListedKey.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminListedKey.cs index f621282..2aec91a 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminListedKey.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authentication/ApiKeyAdminListedKey.cs @@ -8,4 +8,5 @@ public sealed record ApiKeyAdminListedKey( ApiKeyConstraints Constraints, DateTimeOffset CreatedUtc, DateTimeOffset? LastUsedUtc, - DateTimeOffset? RevokedUtc); + DateTimeOffset? RevokedUtc, + DateTimeOffset? ExpiresUtc = null); diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCliRunnerTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCliRunnerTests.cs index e95d5cc..6890a75 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCliRunnerTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCliRunnerTests.cs @@ -52,6 +52,40 @@ public sealed class ApiKeyAdminCliRunnerTests : IDisposable Assert.Contains(auditRecords, record => record.EventType == "create-key" && record.KeyId == "operator01"); } + /// + /// Verifies that a key created with an already-past --expires is rejected by the verifier + /// — the CLI expiry wiring reaches the store and the library enforces it end-to-end (SEC-10). + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task CreateKeyAsync_WithPastExpiry_KeyIsRejectedByVerifier() + { + await using ServiceProvider services = BuildServices(CreateTempDatabasePath()); + ApiKeyAdminCliRunner runner = services.GetRequiredService(); + StringWriter output = new(); + + await runner.RunAsync( + new ApiKeyAdminCommand( + Kind: ApiKeyAdminCommandKind.CreateKey, + Json: true, + SqlitePath: null, + Pepper: null, + KeyId: "operator01", + DisplayName: "Operator", + Scopes: new HashSet(StringComparer.Ordinal) { "session:open" }, + Constraints: ApiKeyConstraints.Empty, + ExpiresUtc: DateTimeOffset.UtcNow - TimeSpan.FromMinutes(1)), + output, + CancellationToken.None); + + string apiKey = ReadApiKey(output.ToString()); + + IApiKeyVerifier verifier = services.GetRequiredService(); + ApiKeyVerification verification = await verifier.VerifyAsync($"Bearer {apiKey}", CancellationToken.None); + + Assert.False(verification.Succeeded); + } + /// Verifies that ListKeysAsync does not print the raw secret. /// A task that represents the asynchronous operation. [Fact] diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCommandLineParserTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCommandLineParserTests.cs index 3f82499..9fe0eba 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCommandLineParserTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authentication/ApiKeyAdminCommandLineParserTests.cs @@ -52,6 +52,61 @@ public sealed class ApiKeyAdminCommandLineParserTests Assert.Contains("events:read", result.Command.Scopes); } + /// A create-key command without --expires leaves the key non-expiring (opt-in, SEC-10). + [Fact] + public void Parse_CreateKeyCommand_WithoutExpires_HasNoExpiry() + { + ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse( + ["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", "--scopes", "session:open"]); + + Assert.NotNull(result.Command); + Assert.Null(result.Command.ExpiresUtc); + } + + /// An absolute ISO-8601 --expires value is parsed to that exact UTC instant (SEC-10). + [Fact] + public void Parse_CreateKeyCommand_WithAbsoluteExpires_ParsesInstant() + { + ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse( + ["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", "--scopes", "session:open", + "--expires", "2030-01-02T03:04:05Z"]); + + Assert.NotNull(result.Command); + Assert.Equal( + new DateTimeOffset(2030, 1, 2, 3, 4, 5, TimeSpan.Zero), + result.Command.ExpiresUtc); + } + + /// A relative "<N>d" --expires value resolves to a future UTC instant (SEC-10). + [Fact] + public void Parse_CreateKeyCommand_WithRelativeExpires_ResolvesToFuture() + { + DateTimeOffset before = DateTimeOffset.UtcNow; + ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse( + ["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", "--scopes", "session:open", + "--expires", "30d"]); + + Assert.NotNull(result.Command); + Assert.NotNull(result.Command.ExpiresUtc); + Assert.InRange( + result.Command.ExpiresUtc!.Value, + before + TimeSpan.FromDays(30) - TimeSpan.FromMinutes(1), + DateTimeOffset.UtcNow + TimeSpan.FromDays(30) + TimeSpan.FromMinutes(1)); + } + + /// An unparseable --expires value fails at parse time (SEC-10). + [Fact] + public void Parse_CreateKeyCommand_WithInvalidExpires_Fails() + { + ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse( + ["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", "--scopes", "session:open", + "--expires", "not-a-date"]); + + Assert.Null(result.Command); + Assert.NotNull(result.Error); + Assert.Contains("--expires", result.Error, StringComparison.Ordinal); + } + /// /// A create-key command with a non-canonical scope /// string (e.g. CLAUDE.md's stale invoke instead of invoke:read)