docs(archreview): flesh out TST-15 dashboard per-session ACL design
Add docs/plans/2026-07-10-dashboard-session-acl-tst15.md: the fleshed-out Phase-4 design for the deferred TST-15 finding. Resolves the crux the deferral left open — the dashboard authenticates LDAP users (Admin/Viewer) while sessions are API-key-owned (OwnerKeyId), two disjoint identity domains — via a session tag sourced from the owning API key (carried in the existing ApiKeyConstraints JSON blob, no SQLite migration). Admin-sees-all; a Viewer may SubscribeSession iff session.Tags intersects the Viewer's granted tags (new Dashboard:GroupToTag map -> hub-token tag claims); untagged sessions Admin-only by default. Includes the enforcement path, task breakdown (epic Tasks 16-19), test plan incl. live-LDAP, and rejected alternatives. Design only — TST-15 stays Not started (no implementation). The tracker and the 60-testing-docs-gaps TST-15 section point at the design doc; the change-log also records the TST-03 finding (zero registered runners; needs a runner co-located on the gitea Docker network). Claude-Session: https://claude.ai/code/session_01DMXXvNuPekkkrTEyPNxEkW
This commit is contained in:
@@ -0,0 +1,297 @@
|
||||
# Dashboard EventsHub per-session ACL (TST-15 / SEC-25 · session-resilience epic Phase 4)
|
||||
|
||||
Status: **Design** — approved-to-implement pending the schema-touch call in §9.
|
||||
Findings: TST-15 (`Medium`, P2), SEC-25 (`Low`, P2). Epic tasks: 16–19 of
|
||||
`docs/plans/2026-06-15-session-resilience.md`.
|
||||
Depends on: TST-02 (owner-scoped gRPC attach, shipped P0), SEC-25 near-term
|
||||
mitigation (value redaction in the dashboard mirror, shipped P2).
|
||||
|
||||
## 1. Problem
|
||||
|
||||
`EventsHub.SubscribeSession(sessionId)` joins the caller's SignalR connection to
|
||||
the `session:{id}` group with no per-session check. The hub-level
|
||||
`[Authorize(Policy = HubClientsPolicy)]` only requires *any* dashboard role, so
|
||||
**every authenticated Viewer (and, via SEC-02, anonymous localhost) can subscribe
|
||||
to any session's raw `MxEvent` feed** — the one production-source `TODO` left in
|
||||
the repo (`Dashboard/Hubs/EventsHub.cs:42`).
|
||||
|
||||
SEC-25's shipped mitigation already strips tag *values* from the mirrored events
|
||||
when `Dashboard:ShowTagValues` is false (the default), so no payload leaks through
|
||||
this seam today. What remains is **event-metadata visibility**: which sessions'
|
||||
event streams (worker sequence, family, server/item handles, alarm references) a
|
||||
given Viewer may observe. That is what this ACL gates.
|
||||
|
||||
### 1.1 The crux: two disjoint identity domains
|
||||
|
||||
This is *not* a copy of the gRPC owner check (TST-02), and the reason is the whole
|
||||
design problem:
|
||||
|
||||
| | gRPC `StreamEvents` (TST-02) | Dashboard EventsHub (TST-15) |
|
||||
|---|---|---|
|
||||
| Caller identity | API key (`mxgw_<id>_<secret>`) | LDAP user → `Administrator`/`Viewer` role |
|
||||
| Session identity | `GatewaySession.OwnerKeyId` (an API key id) | same session, but caller has **no** key id |
|
||||
| Check | `caller.KeyId == session.OwnerKeyId` — same domain, trivial | caller is an LDAP principal; there is **nothing on the session to compare it to** |
|
||||
|
||||
TST-02's binding is API-key-id ↔ API-key-id. A dashboard Viewer has no API key id,
|
||||
so "sessions they own" does not translate directly. The design must introduce a
|
||||
**bridge concept** that both a *session* and a *dashboard group* can be associated
|
||||
with. That bridge is the **session tag** (epic Task 17, "Session Tag + dashboard
|
||||
group-to-tag config").
|
||||
|
||||
## 2. Decision summary
|
||||
|
||||
1. **Admin sees all.** A connection carrying `DashboardRoles.Admin` bypasses the
|
||||
ACL entirely — it may subscribe to any session. (Admin already reaches every
|
||||
destructive surface; event-metadata visibility is strictly weaker.)
|
||||
2. **Viewer is strict.** A Viewer-only connection may subscribe to a session iff
|
||||
`session.Tags ∩ viewer.GrantedTags ≠ ∅`.
|
||||
3. **A session's tags come from its owning API key**, not from the client wire
|
||||
request. The API key is the tenant principal (TST-02 already treats it as the
|
||||
session's owner); deriving the tag from the key makes the dashboard boundary
|
||||
match the gRPC owner boundary. See §3.1 and the rejected alternative in §10.1.
|
||||
4. **Untagged sessions are Admin-only by default.** If the owning key carries no
|
||||
tag, no Viewer sees the session. A config knob
|
||||
(`Dashboard:UntaggedSessionVisibility`) can relax this to `AllViewers` for a
|
||||
genuinely single-tenant deployment, preserving today's behaviour for operators
|
||||
who want it. Default is the safe `AdminOnly`.
|
||||
5. **Enforcement is server-side and stateless**, carried in the existing
|
||||
short-lived hub bearer token (§4). No new server-side session/among-connection
|
||||
state.
|
||||
|
||||
## 3. The session tag
|
||||
|
||||
A session gains an immutable set of tags, assigned once at `OpenSession` and never
|
||||
changed for the session's life (so no re-check on an already-joined SignalR
|
||||
group is needed).
|
||||
|
||||
```csharp
|
||||
// GatewaySession — new read-only property, set from the owner key at construction.
|
||||
public IReadOnlySet<string> Tags { get; } // empty set == untagged
|
||||
```
|
||||
|
||||
### 3.1 Source: the owning API key
|
||||
|
||||
Sessions inherit the tag set of the API key that opened them. The tag rides in the
|
||||
**existing `ApiKeyConstraints` JSON blob** (`ApiKeyConstraintSerializer`), so there
|
||||
is **no SQLite schema migration** — a new column is not required:
|
||||
|
||||
```csharp
|
||||
// ApiKeyConstraints — add one field (serialized into the existing constraints column).
|
||||
public sealed record ApiKeyConstraints(
|
||||
...,
|
||||
bool ReadHistorizedOnly,
|
||||
IReadOnlyList<string> DashboardTags); // NEW — default empty
|
||||
```
|
||||
|
||||
At `OpenSession`, the resolved `ApiKeyIdentity.EffectiveConstraints.DashboardTags`
|
||||
is copied onto the new `GatewaySession.Tags`. The `apikey` admin CLI gains
|
||||
`--dashboard-tags team-a,team-b` on `create`/`update`.
|
||||
|
||||
Semantic note: `ApiKeyConstraints` today scopes *data-access* authorization
|
||||
(read/write subtrees, globs, classification). A dashboard *visibility* tag is a
|
||||
distinct concern, but piggy-backing on the same serialized blob is the pragmatic
|
||||
choice — it avoids a migration and keeps all per-key policy in one place. The
|
||||
field is documented as visibility-only; it does **not** gate any data-access path.
|
||||
|
||||
### 3.2 Dashboard group → tag grant
|
||||
|
||||
A new config map, mirroring the shape of `Dashboard:GroupToRole`:
|
||||
|
||||
```jsonc
|
||||
"MxGateway": {
|
||||
"Dashboard": {
|
||||
"GroupToRole": { "GwAdmin": "Administrator", "GwViewer": "Viewer" },
|
||||
"GroupToTag": { "GwViewer": ["team-a"], "TeamBViewers": ["team-b"] },
|
||||
"UntaggedSessionVisibility": "AdminOnly" // or "AllViewers"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`GroupToTag` maps an LDAP group (short RDN, matching the `GroupToRole` convention
|
||||
in `DashboardGroupRoleMapping`) to the tags that group is granted. A Viewer's
|
||||
`GrantedTags` is the union over their LDAP groups. Admin's grant is irrelevant
|
||||
(they bypass). Bound on `DashboardOptions`:
|
||||
|
||||
```csharp
|
||||
public Dictionary<string, string[]> GroupToTag { get; init; }
|
||||
= new(StringComparer.OrdinalIgnoreCase);
|
||||
public UntaggedSessionVisibility UntaggedSessionVisibility { get; init; }
|
||||
= UntaggedSessionVisibility.AdminOnly;
|
||||
```
|
||||
|
||||
Validated in `GatewayOptionsValidator` exactly like `GroupToRole` (keys non-empty;
|
||||
`UntaggedSessionVisibility` a known enum). No validation coupling to `GroupToRole`
|
||||
— a group may appear in one, the other, or both.
|
||||
|
||||
## 4. Enforcement path
|
||||
|
||||
The tag grant must reach `SubscribeSession`. A hub connection authenticates by
|
||||
either the dashboard cookie or the 5-minute bearer minted by `HubTokenService`.
|
||||
`HubTokenPayload` today carries `(Name, NameIdentifier, Roles)` — extend it with
|
||||
the resolved granted-tag set:
|
||||
|
||||
```csharp
|
||||
private sealed record HubTokenPayload(
|
||||
string? Name, string? NameIdentifier, string[]? Roles, string[]? Tags);
|
||||
```
|
||||
|
||||
- **Mint (`HubTokenService.Issue`)** — resolve the principal's granted tags from
|
||||
their LDAP-group claims (`mxgateway:ldap_group`) via a `GroupToTag` mapper
|
||||
(sibling of `DashboardGroupRoleMapping`), and stamp them into the payload as a
|
||||
`zb:dashboardtag` claim set. Admin needs no tags stamped.
|
||||
- **Cookie path** — the same tag claims are added in
|
||||
`DashboardAuthenticator.CreatePrincipal` so a cookie-authenticated circuit
|
||||
carries them without a token round-trip.
|
||||
- **Validate (`HubTokenService.Validate`)** — rehydrate the tag claims onto the
|
||||
reconstructed principal, alongside the existing role claims.
|
||||
- The 5-minute lifetime already bounds staleness of a changed grant
|
||||
(documented at `HubTokenService.cs:30-37`) — this now also bounds a changed
|
||||
tag grant, which is the natural place the SEC-25 "tokens gain session binding"
|
||||
note anticipated.
|
||||
|
||||
`SubscribeSession` becomes an authorizing, `SessionManager`-aware method:
|
||||
|
||||
```csharp
|
||||
public async Task SubscribeSession(string sessionId)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(sessionId)) return;
|
||||
|
||||
if (!_sessionAcl.CanViewSession(Context.User, sessionId))
|
||||
{
|
||||
// Deny quietly: do NOT join the group. Optionally surface a HubException
|
||||
// so the client can distinguish "denied" from "no events yet".
|
||||
throw new HubException("Not authorized for this session.");
|
||||
}
|
||||
|
||||
await Groups.AddToGroupAsync(Context.ConnectionId, GroupName(sessionId));
|
||||
}
|
||||
```
|
||||
|
||||
`IDashboardSessionAcl.CanViewSession(ClaimsPrincipal, sessionId)`:
|
||||
1. Admin role → `true`.
|
||||
2. Session not found in `SessionManager` → `false` (no group join for a phantom id).
|
||||
3. Session untagged → `UntaggedSessionVisibility == AllViewers`.
|
||||
4. Otherwise → `session.Tags ∩ principal.GrantedTags ≠ ∅`.
|
||||
|
||||
Because the broadcaster only ever publishes to `session:{id}` and only joined
|
||||
connections receive, gating at join is sufficient — there is no second seam to
|
||||
guard. Tags are immutable per session, so a mid-life re-check is unnecessary.
|
||||
|
||||
### 4.1 Interaction with SEC-02 (anonymous localhost)
|
||||
|
||||
Anonymous-localhost satisfies `HubClientsPolicy` but carries no role and no tags.
|
||||
Under this ACL it is treated as a **Viewer with an empty tag grant** → sees only
|
||||
untagged sessions, and only when `UntaggedSessionVisibility == AllViewers`. That
|
||||
tightens SEC-02's current "loopback sees every session" posture without a separate
|
||||
change. `DisableLogin` auto-login (both roles) keeps admin-sees-all — unchanged.
|
||||
|
||||
## 5. End-to-end data flow
|
||||
|
||||
```
|
||||
apikey create --dashboard-tags team-a → key.Constraints.DashboardTags = [team-a]
|
||||
client OpenSession (Bearer mxgw_<id>_..) → session.OwnerKeyId = <id>,
|
||||
session.Tags = [team-a]
|
||||
LDAP login gw-viewer ∈ group GwViewer → GroupToTag[GwViewer] = [team-a]
|
||||
principal grantedTags = {team-a}
|
||||
hub token / cookie → payload.Tags = [team-a]
|
||||
SubscribeSession("s-123") (session.Tags={team-a})→ {team-a} ∩ {team-a} ≠ ∅ → JOIN
|
||||
SubscribeSession("s-999") (session.Tags={team-b})→ {team-a} ∩ {team-b} = ∅ → DENY
|
||||
Admin → bypass → JOIN either
|
||||
```
|
||||
|
||||
## 6. Task breakdown (maps to epic Tasks 16–19)
|
||||
|
||||
- **Task 16 — gRPC all-sessions admin scope (already largely shipped).** The gRPC
|
||||
owner gate is TST-02. This task is the *dashboard* admin bypass in
|
||||
`IDashboardSessionAcl` (§4). No gRPC change required; the "all-sessions admin
|
||||
scope" for gRPC is out of scope for TST-15 and stays with the broader epic.
|
||||
- **Task 17 — session tag + group-to-tag config.** `ApiKeyConstraints.DashboardTags`
|
||||
+ serializer + `apikey` CLI flag; `GatewaySession.Tags` set at `OpenSession`;
|
||||
`DashboardOptions.GroupToTag` + `UntaggedSessionVisibility` + validator.
|
||||
- **Task 18 — EventsHub ACL + hub-token tag claim.** `HubTokenPayload.Tags`,
|
||||
mint/validate, cookie-principal tag claims, `IDashboardSessionAcl` +
|
||||
`SubscribeSession` gate. Remove the `TODO(per-session-acl)`.
|
||||
- **Task 19 — ACL tests, incl. live LDAP users.** §7.
|
||||
|
||||
Sequence: 17 → 18 → 19; 16's dashboard slice folds into 18.
|
||||
|
||||
## 7. Test plan
|
||||
|
||||
Unit / fake-worker (default suite, no LDAP, no COM):
|
||||
- `DashboardSessionAclTests` — the four `CanViewSession` branches: admin bypass;
|
||||
session-not-found deny; untagged under both `UntaggedSessionVisibility` values;
|
||||
tag-intersection match and non-match.
|
||||
- `HubTokenServiceTests` — extend: tags round-trip through `Issue`/`Validate`;
|
||||
a token minted before a grant change still carries the old tags until expiry.
|
||||
- `EventsHubTests` — `SubscribeSession` joins on allow, throws `HubException`
|
||||
and does **not** join on deny (assert via a fake `IGroupManager`).
|
||||
- `GroupToTag` mapper tests — union across multiple groups; unknown group → no tags.
|
||||
- Config: `GatewayOptionsValidator` accepts/ rejects `GroupToTag` /
|
||||
`UntaggedSessionVisibility` shapes.
|
||||
|
||||
Live LDAP (`MXGATEWAY_RUN_LIVE_LDAP_TESTS=1`, `LiveLdapFactAttribute`) — extend
|
||||
`DashboardLdapLiveTests`: `gw-viewer`/`password` (Viewer) is granted `team-a` via
|
||||
`GroupToTag`, subscribes to a `team-a` session (allow) and a `team-b` session
|
||||
(deny); `multi-role`/`password` (Admin) subscribes to both (allow). Uses the
|
||||
shared GLAuth server per `glauth.md`.
|
||||
|
||||
Verification: `dotnet test ... --filter FullyQualifiedName~DashboardSessionAcl`,
|
||||
`~EventsHub`, `~HubTokenService`; `dotnet build src/ZB.MOM.WW.MxGateway.Server`.
|
||||
|
||||
## 8. Docs to update (same change as source)
|
||||
|
||||
- `docs/Sessions.md` — dashboard event visibility / session-tag model.
|
||||
- `gateway.md` dashboard section — the ACL and the tag source.
|
||||
- `docs/GatewayConfiguration.md` — `Dashboard:GroupToTag`,
|
||||
`Dashboard:UntaggedSessionVisibility`; `apikey --dashboard-tags`.
|
||||
- `docs/Authorization.md` — note `DashboardTags` is visibility-only, not a
|
||||
data-access constraint.
|
||||
- `CLAUDE.md` dashboard-auth paragraph — Viewer is tag-scoped; Admin sees all;
|
||||
anonymous-localhost is an empty-grant Viewer.
|
||||
- `glauth.md` — the test-user tag grants used by the live tests.
|
||||
|
||||
## 9. Open call for the implementer
|
||||
|
||||
**Does the tag ride in the existing `ApiKeyConstraints` JSON blob (no migration,
|
||||
recommended) or a new dedicated key column/table?** The blob avoids a SQLite
|
||||
migration and keeps per-key policy in one place; a dedicated column is cleaner
|
||||
semantically (visibility ≠ data-access constraint) but costs a migration + audit
|
||||
touch. Recommendation: **blob**, documented as visibility-only. Settle before
|
||||
Task 17.
|
||||
|
||||
## 10. Rejected alternatives
|
||||
|
||||
### 10.1 Client-supplied session tag (proto field on `OpenSession`)
|
||||
Lighter — one proto field, one session field, no API-key-store touch. Rejected as
|
||||
*primary* because the tag would then be attacker-controlled: a low-trust gRPC
|
||||
client could label its session with any tenant's tag, choosing which dashboard
|
||||
groups observe its event feed. Deriving the tag from the owning API key keeps the
|
||||
boundary aligned with TST-02's trust model (the key is the tenant). (If a future
|
||||
requirement wants client-declared sub-tags *within* an owner's grant, they can be
|
||||
intersected with the key's tags — additive, not a replacement.)
|
||||
|
||||
### 10.2 Map dashboard groups directly to owner key ids (reuse `OwnerKeyId`, no tag)
|
||||
Zero API-key-store change — `Dashboard:GroupToOwnerKeys` maps a group to a set of
|
||||
key ids. Rejected: key ids are opaque and rotate; operators would maintain a map
|
||||
of GUID-like ids, and every key rotation breaks the dashboard grant. A named tag
|
||||
is the right operator-facing abstraction and survives key rotation.
|
||||
|
||||
### 10.3 Full jti-denylist token revocation
|
||||
Out of scope. The 5-minute token lifetime bounds a changed grant; the tokens are
|
||||
data-protection-encrypted and single-purpose. Matches the SEC-25 recommendation
|
||||
to defer heavy revocation.
|
||||
|
||||
## 11. Security considerations
|
||||
|
||||
- **No new value-leak surface.** SEC-25 already redacts values in the mirror; this
|
||||
change only narrows *which sessions'* metadata a Viewer sees. Strictly a
|
||||
tightening.
|
||||
- **Fail closed.** Session-not-found, empty grant, and untagged-under-`AdminOnly`
|
||||
all deny. A misconfigured/empty `GroupToTag` yields Viewers who see nothing
|
||||
(except untagged-under-`AllViewers`) — safe, not open.
|
||||
- **Default preserves single-tenant ergonomics** only if the operator opts into
|
||||
`UntaggedSessionVisibility=AllViewers`; the shipped default (`AdminOnly`) is the
|
||||
strict one, so an upgrade tightens rather than loosens.
|
||||
- **Staleness bound.** A revoked tag grant takes effect within one token lifetime
|
||||
(≤5 min) for token-auth connections and immediately for a fresh cookie login.
|
||||
```
|
||||
Reference in New Issue
Block a user