3 Commits

Author	SHA1	Message	Date
Joseph Doherty	2ef7b93761	fix(SEC-41, SEC-40): re-resolve dashboard tag grants; gate the alarm feed on a real role ... Committed together because the two changes share the same doc paragraphs (the ACL section of GatewayDashboardDesign.md, the GroupToTag / AllowAnonymousLocalhost rows of GatewayConfiguration.md, and CLAUDE.md's authentication paragraph), and splitting them would leave one of the two describing behaviour that does not yet exist. SEC-41 — tag grants no longer trusted from login-stamped claims --------------------------------------------------------------- DashboardSessionAcl read the zb:dashboardtag claims stamped into the cookie at login. GatewayOptions binds once at startup, so applying a GroupToTag edit needs a restart — but the restart does not clear the auth cookie (data-protection keys persist), so a browser returning afterwards re-authenticated with tag claims resolved under the OLD map. A revoked grant therefore survived on the cookie seam for the sliding 8-hour idle life, renewable indefinitely by activity, while the hub-token seam had no such gap because its mint re-resolves. CanViewSession now re-resolves the grant on every subscribe from the caller's mxgateway:ldap_group claims through DashboardGroupTagMapping.MapGroupsToTags against the currently bound map — the same one call HubTokenService.Issue makes. Fallback (documented on the class and in the design doc): a principal carrying NO group claims falls back to its stamped tag claims. That arm exists for the hub-token principal — HubTokenService.Validate rehydrates roles and tags but not groups, so re-resolution already happened at mint, one lifetime ago at worst. It cannot re-open the cookie hole: DashboardAuthenticator stamps group and tag claims from one group list, so a cookie principal holding a tag claim always holds the groups it came from, and a login yielding no groups yields no tags. A principal with neither kind of claim stays an empty-grant Viewer. SEC-40 — the alarm feed requires a real authenticated dashboard role -------------------------------------------------------------------- AlarmsHub sat behind HubClientsPolicy, which the read-only environment bypasses satisfy, so an anonymous-localhost caller or a Mode=Disabled deployment received galaxy-wide alarm metadata — alarm reference (a full tag path), state, severity, category, area, acknowledging operator — for the whole subscription expression. Decided: tighten the gate, keep the scope. New AuthenticatedDashboardRoleRequirement + its handler, and a MxGateway.Dashboard.Alarms policy built from it. Because DashboardAuthorizationHandler is typed to DashboardAuthorizationRequirement, it is never invoked for the new type, so neither Authentication:Mode=Disabled nor anonymous localhost can satisfy the policy — the same structural device the mounted Secrets UI already relies on. The policy keeps both schemes so external hub clients can still authenticate with a bearer token, and DisableLogin auto-login is unaffected (it mints a genuinely authenticated both-roles principal). The /alarms page carries the same policy, not just the hub: it reads the same central monitor in process, so gating only the hub would have left the disclosure one navigation away. The side rail hides the Alarms link for callers the policy denies, matching the Secrets link precedent. AlarmsHubPublisher is unchanged and stays unfiltered by design — audience is decided at hub admission, and the monitor carries no per-session provenance to narrow by. Alarm metadata deliberately remains galaxy-wide for authenticated Viewers: an AlarmFeedMessage has no session id or owning key for the SEC-25 tag intersection to work against, and equating a visibility tag with an alarm-reference prefix would turn GroupToTag into the data-access constraint it explicitly is not. Tests ----- DashboardSessionAclTests: the SEC-41 regression — a cookie-style principal whose stale tag claim is no longer granted by any of its groups under the current map is denied; plus the allowing direction (including with no stamped tag at all, which proves the decision comes from the groups), the ignore-case corner, group claims that map to nothing, and the hub-token fallback asserted against an EMPTY map so dropping the fallback would fail loudly. AlarmFeedAuthorizationTests (new): both handlers run together in the most permissive environment (Mode=Disabled + loopback + AllowAnonymousLocalhost) do not satisfy the alarm requirement, with the bypassable requirement as the control so the negative cannot pass because the bypass was misconfigured; authenticated Viewer/Admin do satisfy it; an unauthenticated role claim does not; the registered policy carries the non-bypassable requirement and both schemes; and both readers — the hub type's [Authorize] and the /alarms endpoint metadata — carry it. SecretsNavRenderTests: registers the new policy (an unregistered policy name throws rather than denying in an AuthorizeView) and gains the Alarms-link negative/control pair. Verified: dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx (zero warnings); filtered run of the touched classes — 92 passed, 0 failed.	2026-08-20 09:29:44 -04:00
Joseph Doherty	7ec0b3594c	fix(dashboard): close AttachEventsAsync re-entrancy window; pin ACL decision-table corners (SEC-25 review) ... Follow-up to the per-session event ACL. Part of that change rode into 693a78d via a concurrent agent's pathspec-less commit; this commit carries the review fixes and uses pathspecs on the commit itself so it cannot recur in either direction. Gating the page's subscribe seam made AttachEvents asynchronous — it awaits the authentication state — and that await is a suspension point the synchronous version did not have. On a rapid A -> B navigation the suspended A continuation resumes after B's parameter set has run to completion, re-reads the live SessionId (now B's), and attaches B a SECOND time. The ACL is not bypassed — the newer attach already cleared that same session — but the fields holding B's first subscription are overwritten in place, so nothing ever disposes it: its EventsHubViewerRegistry entry is never released, which keeps the mirror cloning events for a session the page is no longer watching through that handle, and its pump is never cancelled. A resource leak the ACL work introduced. OnParametersSetAsync now claims a monotonic _attachGeneration synchronously, before its first await, and AttachEventsAsync re-checks it after the await and before any field write or Subscribe call. A stale attach returns rather than detaching: it owns nothing, and tearing down there would destroy the newer attach's subscription. DetachEventsAsync needs no such guard — it captures and nulls the live fields synchronously before it awaits, so a resumed detach only unwinds what it already took ownership of. Same dispatcher-owned identity idea as the existing ReferenceEquals guards in PumpEventsAsync and MarkDisconnectedAsync, one level up. The interleaving is not expressible with the static HtmlRenderer idiom the other page tests use: it renders a root component once and exposes no parameter-update seam. The new test therefore adds a minimal Renderer subclass whose only job is to mount a component and drive a second SetParametersAsync into it while the first is parked on a gated AuthenticationStateProvider. That subclass is the lone reason for a narrowly scoped BL0006 suppression, justified in place: it is test-only scaffolding that never ships, and the cost of the warning coming true is a compile break in one test file on an SDK bump. Confirmed non-vacuous by mutation — with the generation check disabled the test goes red on the doubled subscription and the two passing ACL tests stay green. Two decision-table corners are now pinned rather than implied. Admin x nonexistent session id resolves to ALLOW, because the admin bypass is evaluated before the registry lookup; a plausible "look the session up first, it reads better" refactor would flip it, so a test documents the ordering. EventsHub's remarks said "an unknown session id is denied" without qualification, which read as universal; they now state that the bypass is checked first and every rule below it is a non-Admin rule. HubTokenServiceTests gains the truly-absent-field case: a hand-built payload JSON with no Tags key at all, protected through the same purpose, which is the shape every in-flight token has across the deploy that introduces the field. The existing test covered present-but-empty, which does not exercise the null coalesce that stands between a legacy token and a crash on the hub auth path. ProtectorPurpose became internal so the test cannot drift from the real purpose string. Tag-count cardinality cap considered and recorded as a deliberate non-goal. Build 0 warnings / 0 errors; 48 filtered (ACL/hub/token/page) and 257 dashboard tests pass.	2026-08-17 04:35:59 -04:00
Joseph Doherty	693a78db7d	feat(alarms): structural degraded-status signal for truncated alarm snapshots ... The truncation-cliff fix made alarm transitions truncation-safe but silent: when GetXmlCurrentAlarms2 returns exactly maxAlmCnt records the worker suppresses absence-implies-Clear inference and says so only in a rate-limited stderr warning. No client and no operator could tell a complete active set from a capped one. Two additive proto3 booleans carry the verdict out: - QueryActiveAlarmsReplyPayload.snapshot_truncated = 2 (worker IPC reply) - ActiveAlarmSnapshot.from_truncated_snapshot = 16 (per record) The per-record field is not an aesthetic choice. QueryActiveAlarms returns a bare `stream ActiveAlarmSnapshot` with no envelope, header, or trailer, so a per-record boolean is the only carrier that stays wire-compatible; an envelope message would change every existing client's stream element type. The reply payload states it too because a prefix filter can leave zero records and a truncated fetch with nothing to report still has to say so. The flag means "this set may be incomplete", never "this record is unreliable" — it is independent of the subtag-fallback `degraded` field. Detection is deliberately UNCHANGED: IsTruncatedFetch remains `fetchedRecordCount >= maxAlarmsPerFetch`. The live probe (docs/AlarmProbeFindings.md, ce5d8ae) could not verify whether ALARM_RECORDS/@COUNT reports the total active count or only the records in the reply, so @COUNT is not parsed for detection; switching to it stays blocked on probe evidence. The probe's comment annotations in WnWrapAlarmConsumer.cs are preserved. Reset semantics: not latched. WnWrapAlarmConsumer.FoldFetch replaces the verdict on every poll under the same lock as the snapshot merge, so the first sub-cap fetch clears it; GatewayAlarmMonitor.ClearCache drops it with the cache generation it describes. A caveat that never turns off is one operators learn to ignore. Flow: WnWrapAlarmConsumer.LastSnapshotTruncated -> AlarmDispatcher (stamps every record) / IAlarmCommandHandler (payload) -> MxAccessCommandExecutor reply -> GatewayAlarmMonitor._snapshotTruncated -> IGatewayAlarmService.SnapshotTruncated -> DashboardAlarmQueryResult -> AlarmsPage warning banner (render-side only; the poll loop and DisposeAsync drain are untouched). The public QueryActiveAlarms RPC forwards worker snapshots unmodified, so the per-record flag needed no mapper change — a test pins that. Parity: this describes OUR fetch mechanics — additive gateway metadata — not MXAccess provider behavior. No event is synthesized and no MXAccess-observable semantics change, so it is not a parity deviation. Tests: worker LastSnapshotTruncated set/reset/consecutive-burst (windev-run); gateway end-to-end truncated reply -> monitor -> public stream, with the complete-reply control as the load-bearing assertion; AlarmsPage banner present/absent. Docs: gateway.md alarm surface, docs/DesignDecisions.md entry.	2026-08-17 04:18:34 -04:00