test(dashboard)+docs: SEC-25 live-LDAP ACL coverage; design marked implemented

The per-session dashboard event ACL shipped in 693a78d + 7ec0b35 with unit
coverage over a fabricated principal. What a fabricated principal cannot show is
that the group names the shared directory actually returns -- short RDN values,
not DNs -- are the ones Dashboard:GroupToTag keys match. Two [LiveLdapFact]s
close that: gw-viewer binds for real, its GwReader membership grants team-a, and
IDashboardSessionAcl then admits a team-a-tagged session and refuses a
team-b-tagged one; multi-role takes the Administrator bypass. The mapping is
config-side only -- no GLAuth entry, group, or membership was added, and
glauth.md records that explicitly so a future reader does not go looking for a
directory change that never happened.

multi-role is a member of GwReader as well as GwAdmin, so it holds team-a too.
Its bypass is therefore asserted on team-b and on the untagged session -- the two
it would lose if the Administrator branch were ever dropped -- rather than on
team-a, which would pass either way.

One cheap hardening from a prior review: a GatewayOptionsTests case binds
Dashboard:GroupToTag through a real ConfigurationBuilder and looks the group up
mis-cased. The property initializer seeds an OrdinalIgnoreCase dictionary, but
only the binder decides whether that instance survives; if it did not, a
mis-cased group name from the directory would grant no tags and the ACL would
deny with no diagnostic.

Docs follow the shipped shape: docs/Sessions.md gains the session-tag model
(owner-key sourced, immutable, visibility-not-access), gateway.md and CLAUDE.md
gain the ACL in their dashboard-auth paragraphs, and three
GatewayDashboardDesign.md passages that still described the ACL as outstanding
now describe both gated seams and the decision order. GatewayConfiguration.md's
ShowTagValues row no longer claims the redaction is the only thing between a
Viewer and another session's values -- it is now the second of two independent
layers. gateway.md's hub-token lifetime corrected 30 minutes -> 5, matching
HubTokenService. Authentication.md disambiguates --dashboard-tags as the only
constraint flag that splits on commas. The plan doc header is Implemented; its
as-built section 12 already existed and is not duplicated.

Verified: NonWindows.slnx builds clean; GatewayOptions/DashboardSessionAcl/
EventsHub filters 37/37; the live-LDAP suite skips cleanly without the env var
and runs 7/7 green against the shared GLAuth with it.
This commit is contained in:
Joseph Doherty
2026-08-17 04:48:34 -04:00
parent d9ea8a81f1
commit 1d8a4a6442
13 changed files with 326 additions and 27 deletions
+2 -1
View File
@@ -227,7 +227,7 @@ Full design + implementation for each row lives in the linked domain doc under i
| TST-12 | Medium | P0 | S | — | Done | CLAUDE.md misstates default retention behaviour |
| TST-13 | Medium | P2 | S | — | Done | gateway.md carries stale design-era sketches |
| TST-14 | Medium | P2 | S | — | Not started | Repo-root working artifacts need triage |
| TST-15 | Medium | P2 | M | TST-04 | Not started | Dashboard EventsHub has no per-session ACL |
| TST-15 | Medium | P2 | M | TST-04 | Done | Dashboard EventsHub has no per-session ACL |
| TST-16 | Medium | — | S | — | Not started | `Dashboard:ShowTagValues` is a dead flag |
| TST-17 | Medium | — | S | — | Not started | Vendor-gated alarm parity residuals silently lossy |
| TST-18 | Low | — | S | — | Not started | Hosted-service wrappers untested |
@@ -253,6 +253,7 @@ Findings the review flagged as one coordinated design pass — sequence them tog
| Date | Change |
|---|---|
| 2026-08-17 | **TST-15 → `Done` (discharges the ACL half of SEC-25): per-session dashboard event ACL shipped** (branch `feat/deferred-closeout`, commits `693a78d` + `7ec0b35`). Implements `docs/plans/2026-07-10-dashboard-session-acl-tst15.md`, whose header is now `Implemented` with as-built notes in its §12. `IDashboardSessionAcl.CanViewSession` is the single decision **both** subscribe seams consult — `EventsHub.SubscribeSession` (denial is a `HubException`; the caller is neither joined to the group nor registered in `EventsHubViewerRegistry`, so the mirror stays off) and `SessionDetailsPage`'s in-process subscribe (inline denial, no subscription) — so neither path is the weaker one and the `TODO(per-session-acl)` is gone. Decision order, fail-closed on every branch: authenticated Administrator → allow (evaluated **before** the registry lookup, so Admin × unknown-session allows — pinned by a test because reordering the two checks is a plausible refactor); session not found → deny; untagged session → `Dashboard:UntaggedSessionVisibility` (`AdminOnly` default); else allow iff `session.Tags ∩ zb:dashboardtag` claims, ordinal-ignore-case. Session tags are inherited from the owning API key's `dashboard_tags` constraint (`apikey --dashboard-tags`, already in the `ApiKeyConstraints` JSON blob — no SQLite migration) and never from the client's wire request. Viewer grants come from `Dashboard:GroupToTag` over the user's LDAP groups, stamped at cookie login (`DashboardAuthenticator.CreatePrincipal`) and **re-resolved, not copied**, at hub-token mint (`HubTokenService.Issue`), so the 5-minute token lifetime bounds a stale grant. Anonymous localhost is an empty-grant Viewer; `Dashboard:DisableLogin` auto-login carries both roles and so takes the admin bypass unchanged. Tests: `DashboardSessionAclTests` (decision table, every branch asserted in its denying direction too), `EventsHubTests`, `DashboardAuthenticatorTests`, `HubTokenServiceTests`, a `GatewayOptionsTests` case proving `Dashboard:GroupToTag` keeps its ordinal-ignore-case lookup through configuration binding, and two `[LiveLdapFact]`s in `DashboardLdapLiveTests` that drive a real bind against the shared GLAuth (`gw-viewer``team-a` grant admits the `team-a` session and refuses the `team-b` one; `multi-role` bypasses on the sessions its own grant does not cover). The live pair needed **no GLAuth change** — the tag layer is config-side, keyed on the existing `GwAdmin`/`GwReader` groups (recorded in `glauth.md`). Docs: `docs/Sessions.md` (session-tag model), `gateway.md` + CLAUDE.md dashboard-auth paragraphs, `docs/GatewayDashboardDesign.md` (three passages that described the ACL as outstanding), `docs/GatewayConfiguration.md` (`ShowTagValues` row: redaction is now the second of two layers, not the only one), `docs/Authentication.md` (`--dashboard-tags` is the only *constraint* flag that splits on commas). |
| 2026-08-10 | **TST-25 acceptance Check 6 (forced-failure nightly issue) → Done.** The 2026-07-13 record wrote this check off as "abandoned to shared-runner congestion"; that was wrong on both counts. The 2026-07-13 probe *did* land (issue #125, `[CHECK6 PROBE]`, run 375), and since 2026-07-17 the `nightly-windev` `if: failure()` step has filed an issue on **every** red nightly — #126#139, all authored by the `gitea-actions` bot. Traced run 672 (schedule, main, red) line by line: main step fails → `exitcode '1': failure` → the `if: failure()` step runs → `POST /api/v1/repos/dohertj2/mxaccessgw/issues` with the built-in token masked to `***` → issue #139 created at the matching timestamp. Re-confirmed by a fresh forced-failure probe on the throwaway branch `test/tst25-check6-nightly-issue` (temporary `tst25-check6-probe.yml` reproducing the job shape with `exit 1` for the live step; run 677 → issue #140). Branch deleted, issues #125 and #140 closed with explanatory comments. **One real defect found and fixed** (`fix/tst25-nightly-issue-path`, not merged): `${{ github.server_url }}` is the runner-internal `http://gitea:3000`, so every filed issue's run link was unreachable from a browser. The API call must keep using it (the job container resolves `gitea` only on the docker network and has no LAN egress to the public origin), so the fix adds a `PUBLIC_SERVER_URL: https://gitea.dohertylan.com` job env used **only** for the browser-facing link in the issue body; the probe validated the fixed template (#140 carries a `https://gitea.dohertylan.com/...` link that returns 200). **Separately observed, not fixed:** the nightly has been red continuously since at least 2026-07-17 (run 672: `x86 Worker.Tests failed with exit code 1`, 1 failed / 398 passed / 11 skipped — the known `EventBurst_DrainLoopCoalescesFlushes` class of flake), and the step de-duplicates nothing, so 14 issues are open, seven of them (#132#138) for the identical SHA `47c0b64`. Worth a follow-up: fix the red nightly, and consider having the step reuse an open issue with the same title instead of filing a new one. |
| 2026-08-10 | **TST-24 → `Done`: per-client wire tests land for the two clients that lacked them** (branch `feat/tst-24-client-wire-tests`). Audit first corrected the finding's premise: **Go, Rust, and Java already had real-server wire tests**`newBufconnClient`/`fakeGatewayServer` over `grpc/test/bufconn`, `spawn_fake_gateway` over a loopback `TcpListener` with tonic's `Server`, and `InProcessGateway`/`TestGatewayService` over `InProcessServerBuilder` — each already asserting the round trip, the server-observed `authorization` bearer header, and the `ReplayGap` sentinel. The real gaps were **.NET** (every test substituted `FakeGatewayTransport` for `IMxGatewayClientTransport`, and the test project had no server package) and **Python** (stub monkeypatching everywhere except one opt-in TLS test serving only `OpenSession`). Added `WireFakeGatewayServer` + `MxGatewayClientWireTests` (Kestrel h2c on `127.0.0.1:0` serving `MxAccessGatewayBase`; new `Grpc.AspNetCore.Server` 2.76.0 + `Microsoft.AspNetCore.App` refs on the test project) and `clients/python/tests/test_wire_fake_gateway.py` (`grpc.aio` server on `127.0.0.1:0`, no new deps). Four shapes each: full round trip with every reply field asserted, the bearer header **as received by the server** on the streaming RPC too, the `ReplayGap` sentinel surfaced as the client's typed signal, and a genuine `PERMISSION_DENIED` mapping to the typed authorization error. CI: the `portable` job only *built* the .NET client, so a `dotnet test` step was added. **The new tests immediately caught a shipped bug** — Python `GatewayClient.connect()`/`GalaxyRepositoryClient.connect()` constructed the `grpc.aio` channel inside `asyncio.to_thread`, which raises `RuntimeError: There is no current event loop in thread 'asyncio_0'` because a `grpc.aio` channel binds to the loop current on the constructing thread; every non-stub connection failed, and the one test guarding the off-loop behaviour (Client.Python-028) monkeypatched `create_channel` and so asserted the bug. Fixed by splitting `resolve_channel_security` (blocking TOFU probe, off-loop) from `create_channel` (on-loop), with the `-028` tests retargeted to assert both halves. Verified: .NET 133 passed/1 skipped (pre-existing live-gateway skip), Python 168 passed/1 skipped plus 6/6 opt-in TLS. Docs: `docs/GatewayTesting.md` § Client Wire Tests, `clients/dotnet/README.md`, `clients/python/README.md`. |
| 2026-08-10 | **TST-05 revisited under the restored Windows tier → `Partially done`** (branch `feat/tst-24-client-wire-tests`, doc/tracker-only). The finding's **scheduling** half is closed: cycle-2 TST-25's `nightly-windev` job (cron `0 6 * * *`) runs `scripts/ci/run-windev-ci.sh live``windev-worker-ci.ps1 -Mode live`, which sets `MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`, runs `WorkerLiveMxAccessSmokeTests` on windev after the x86 build/Worker.Tests/full-slnx steps, and files a Gitea issue when red. The **coverage-audit** half is *not* closed, and the audit the design asked for now has a negative answer: the suite's eight `[LiveMxAccessFact]`s cover all six late-added COM commands (`Suspend`, `Activate`, `AuthenticateUser`, `ArchestrAUserToId`, `AddBufferedItem`, `SetBufferedUpdateInterval`) but zero of the five control commands — `MxCommandKind.{Ping,GetSessionState,GetWorkerInfo,DrainEvents,ShutdownWorker}` appear nowhere in `WorkerLiveMxAccessSmokeTests.cs`, so the exact paths the Finding calls masked are still only proven against `FakeWorkerHarness` canned replies while the real implementations live in `Worker/Ipc/WorkerPipeSession.cs`. Residual work (two `[LiveMxAccessFact]`s, windev-only to author and verify) is specified in [60-testing-docs-gaps.md](60-testing-docs-gaps.md#tst-05--real-worker-controlcom-paths-verified-opt-in-only---medium--p1). |
@@ -457,6 +457,8 @@ This document turns every finding in the Security/Dashboard/Observability review
- Tests: broadcaster test asserting values redacted when `ShowTagValues=false`.
- Docs: `docs/GatewayDashboardDesign.md` — clarify the current v1 posture.
**Update 2026-08-17 — the deferred half landed.** The scoping mechanism this finding waited on shipped as TST-15 (`693a78d` + `7ec0b35`): `IDashboardSessionAcl` gates `SubscribeSession` *and* the session-details page's in-process subscribe, so the `TODO(per-session-acl)` is gone and the redaction is no longer the only thing between a low-trust Viewer and another session's events. See the TST-15 section in [60-testing-docs-gaps.md](60-testing-docs-gaps.md#tst-15--dashboard-eventshub-has-no-per-session-acl) and the 2026-08-17 change-log row in [00-tracking.md](00-tracking.md#change-log). Redaction stays — the two layers are independent: the ACL decides who may subscribe, `ShowTagValues` decides what a permitted subscriber sees.
**Verification.** `dotnet build src/ZB.MOM.WW.MxGateway.Server` and the events-hub/broadcaster test filter.
---
@@ -335,11 +335,11 @@ If TST-02's interim mitigation (flip retention off) is chosen instead of impleme
**Impact.** Acceptable for a single-tenant dashboard; wrong the moment `GroupToRole` admits low-trust viewers. It is the dashboard-side twin of the gRPC owner-revalidation gap (TST-02).
**Design.** Fully fleshed out in `docs/plans/2026-07-10-dashboard-session-acl-tst15.md` (epic Phase 4, Tasks 1619, TST-04). In brief: the dashboard authenticates LDAP users (Admin/Viewer) while sessions are API-key-owned (`OwnerKeyId`) — two disjoint identity domains — so the ACL needs a bridge: a **session tag** sourced from the owning API key (riding in the existing `ApiKeyConstraints` JSON blob, no SQLite migration). Admin sees all; a Viewer may `SubscribeSession` iff `session.Tags ∩ viewer.GrantedTags ≠ ∅` (granted via a new `Dashboard:GroupToTag` map, carried into the hub token as tag claims); untagged sessions are Admin-only by default. The Viewer-default decision (admin-sees-all vs strict) is settled there. Until Phase 4 lands, keep the TODO (it correctly documents the accepted single-tenant assumption); do not silently remove it.
**Design.** Fully fleshed out in `docs/plans/2026-07-10-dashboard-session-acl-tst15.md` (epic Phase 4, Tasks 1619, TST-04). In brief: the dashboard authenticates LDAP users (Admin/Viewer) while sessions are API-key-owned (`OwnerKeyId`) — two disjoint identity domains — so the ACL needs a bridge: a **session tag** sourced from the owning API key (riding in the existing `ApiKeyConstraints` JSON blob, no SQLite migration). Admin sees all; a Viewer may `SubscribeSession` iff `session.Tags ∩ viewer.GrantedTags ≠ ∅` (granted via a new `Dashboard:GroupToTag` map, carried into the hub token as tag claims); untagged sessions are Admin-only by default. The Viewer-default decision (admin-sees-all vs strict) is settled there.
**Implementation.** `Dashboard/Hubs/EventsHub.cs` (ACL check on group join), hub-token minting to carry the session tag, `Configuration/DashboardOptions.cs` for any group-to-tag config (Task 17). Tests: `...Tests/Gateway/Dashboard/` hub ACL cases incl. live-LDAP users (Task 19). Docs: `docs/Sessions.md`/`gateway.md` dashboard section document the ACL model; CLAUDE.md dashboard-auth paragraph.
**Implementation.** Shipped 2026-08-17 on `feat/deferred-closeout` (`693a78d` + `7ec0b35`); the `TODO(per-session-acl)` is gone. `Dashboard/IDashboardSessionAcl.cs` + `Dashboard/DashboardSessionAcl.cs` hold the single decision, consulted by `Dashboard/Hubs/EventsHub.cs` (`SubscribeSession``HubException` on denial, no group join and no viewer registration) and by `Dashboard/Components/Pages/SessionDetailsPage.razor`'s in-process subscribe — the design's one correction, since the page path was not a hub client and would otherwise have been the unguarded seam. Tags ride from the owning key via `ISessionManager.OpenSessionAsync`'s tagged overload into the immutable `GatewaySession.Tags`; grants are stamped by `DashboardAuthenticator.CreatePrincipal` and re-resolved at `HubTokenService.Issue`. Config: `Dashboard:GroupToTag` and `Dashboard:UntaggedSessionVisibility` on `Configuration/DashboardOptions.cs`. Tests: `Tests/Gateway/Dashboard/DashboardSessionAclTests.cs`, `EventsHubTests.cs`, a `Configuration/GatewayOptionsTests.cs` binding case for the `GroupToTag` comparer, and two `[LiveLdapFact]`s in `IntegrationTests/DashboardLdapLiveTests.cs`. Docs: `docs/Sessions.md`, `gateway.md`, CLAUDE.md, `docs/GatewayDashboardDesign.md`, `docs/GatewayConfiguration.md`, `glauth.md`.
**Verification.** `dotnet test ... --filter FullyQualifiedName~EventsHub`; `dotnet build src/ZB.MOM.WW.MxGateway.Server`.
**Verification.** `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx`; `dotnet test ... --filter FullyQualifiedName~DashboardSessionAclTests`, `~EventsHubTests`, `~GatewayOptionsTests`; live-LDAP pair run green against the shared GLAuth with `MXGATEWAY_RUN_LIVE_LDAP_TESTS=1` (and skipping cleanly without it).
---