1d8a4a6442
The per-session dashboard event ACL shipped in693a78d+7ec0b35with 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.
371 lines
19 KiB
Markdown
371 lines
19 KiB
Markdown
# Dashboard EventsHub per-session ACL (TST-15 / SEC-25 · session-resilience epic Phase 4)
|
||
|
||
Status: **Implemented** — branch `feat/deferred-closeout`, 2026-08-17, commits
|
||
`693a78d` + `7ec0b35`. As-built notes in §12; the sections above are the design as
|
||
approved, kept for the rationale they record.
|
||
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.
|
||
```
|
||
|
||
## 12. As-built notes (enforcement landed 2026-08)
|
||
|
||
### 12.1 §4's "no second seam" no longer held — the ACL gates two
|
||
|
||
This design was written against a dashboard whose only route to a session's event
|
||
feed was the SignalR hub, which is why §4 concludes "gating at join is sufficient —
|
||
there is no second seam to guard". The 2026-08 in-process feed refactor invalidated
|
||
that premise: server-rendered pages stopped opening a loopback SignalR connection to
|
||
`/hubs/events` and now read the mirror directly through
|
||
`IDashboardSessionEventSubscriber.Subscribe(sessionId)` (`DashboardEventBroadcaster`
|
||
implements both the publish and subscribe interfaces). `SessionDetailsPage` is that
|
||
second consumer, and it never touches `SubscribeSession`, so a hub-only gate would
|
||
have left the page as an ungated path to the same events.
|
||
|
||
The shipped enforcement therefore puts the *same* `IDashboardSessionAcl` decision in
|
||
front of both subscribe calls:
|
||
|
||
- `EventsHub.SubscribeSession` — denies with `HubException("Not authorized for this
|
||
session.")` before the group join **and** before the `EventsHubViewerRegistry`
|
||
registration, so a denied caller neither receives events nor turns the mirror on.
|
||
- `SessionDetailsPage` — resolves the circuit principal via
|
||
`AuthenticationStateProvider` and checks the ACL *before* `Subscribe(SessionId)`.
|
||
A denial creates no subscription, starts no pump, and registers no viewer; the
|
||
events panel renders "Not authorized for this session's events." in place of its
|
||
empty state. The gate wraps only whether the subscription is created — the page's
|
||
generation/`ReferenceEquals` guards, `MarkDisconnectedAsync`, and the
|
||
`DisposeAsync`/`DetachEventsAsync` coupling are untouched.
|
||
|
||
Both seams remain subscribe-time-only. Session tags are immutable for the session's
|
||
life (§3), so a joined group or a live in-process subscription cannot go stale, and
|
||
no per-event check is needed on either path.
|
||
|
||
### 12.2 Where the grant is stamped
|
||
|
||
`zb:dashboardtag` claims are added at both principal-construction sites:
|
||
`DashboardAuthenticator.CreatePrincipal` (cookie login, so a circuit carries its
|
||
grant without a token round-trip) and `HubTokenService.Issue` (hub bearer). Both
|
||
resolve the grant from the caller's `mxgateway:ldap_group` claims through
|
||
`DashboardGroupTagMapping` + `Dashboard:GroupToTag` rather than copying tag claims
|
||
already on the principal — re-resolving at mint is what makes the token's 5-minute
|
||
lifetime an actual staleness bound on a changed grant, as §4 claims. Tags are
|
||
stamped for Administrators too; they are simply moot, because the ACL's admin bypass
|
||
is checked first.
|
||
|
||
### 12.3 Deviations worth knowing
|
||
|
||
- `CanViewSession` takes a **nullable** `ClaimsPrincipal`. `HubCallerContext.User` is
|
||
nullable, and null denies — the fail-closed reading.
|
||
- The admin bypass additionally requires `Identity.IsAuthenticated`, matching
|
||
`DashboardSessionAdminService.CanManage`. A role claim on an unauthenticated
|
||
identity does not bypass.
|
||
- Tag *values* are never logged at either seam; only the identifiers and the
|
||
allow/deny outcome are observable.
|
||
- **The page gate needed a re-entrancy guard.** Making `AttachEvents` async (it now
|
||
awaits the authentication state) introduced a suspension point the synchronous
|
||
version did not have, and with it a window: on a rapid A → B navigation the
|
||
suspended A continuation resumes, re-reads the live `SessionId` — now B's — and
|
||
attaches B a second time, overwriting the fields that hold B's first subscription.
|
||
That subscription is then unreachable: never disposed, its `EventsHubViewerRegistry`
|
||
entry never released (so the mirror keeps cloning events for it), its pump never
|
||
cancelled. Not an ACL bypass — the newer attach had already cleared the same session
|
||
— but a resource leak the ACL work created. `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`; a stale attach returns without attaching (it owns nothing, and tearing
|
||
down would destroy the newer attach's subscription). `DetachEventsAsync` needs no
|
||
guard: it captures and nulls the live fields synchronously before it awaits.
|
||
- The admin-bypass-before-lookup ordering means an Administrator naming a session id
|
||
the registry does not have is **allowed**, not denied. Deliberate, and pinned by a
|
||
test so a "look the session up first" refactor cannot flip it silently.
|