diff --git a/docs/lifecycle-reasons.md b/docs/lifecycle-reasons.md index 70a951ff..4e6de378 100644 --- a/docs/lifecycle-reasons.md +++ b/docs/lifecycle-reasons.md @@ -121,3 +121,12 @@ Documented values: `params` here is the same flat string map convention as `WorkspaceEvent.params` — clients may localize the refusal instead of parsing `message`, and unknown keys must be ignored. + +## Portal session idle + +`POST /v1/session:touch` is the portal's explicit activity beat: every +cookie-authenticated `GET` is passive, so the only signals that extend the +session's sliding idle window are mutations, desktop input (measured +broker-side), and this call — which the portal sends only on user +interaction (pointer, key, navigation), throttled to one per minute. +`GET /v1/session` stays anonymous and passive. diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 43dba068..d3e3a37a 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -185,10 +185,15 @@ Controls: peer, or the right-most untrusted X-Forwarded-For entry when the peer sits inside `-trusted-proxies` CIDRs (`ratelimit.go:174`, `ParseTrustedProxies`). -- **Passive auth** — `GET /v1/connections/.../status` authenticates via +- **Passive auth** — since FIX-IDLE every cookie-authenticated `GET` mounts `RequireAuthPassive`, which never extends the idle clock - (`internal/api/middleware.go:83-86`, `internal/api/connection_status.go:78`) — - A6-S6's second authenticated path exists to be reviewed. + (`internal/api/middleware.go`, and the `safe`/`RequireAuthPassive` mounts + in `workspaces.go`, `data.go`, `me.go`, `quota.go`, `adminquota.go`, + `adminuserlimit.go`): the portal's interval polls can no longer hold a + visible-but-unattended session open. Only mutations and server-measured + desktop input slide the window. `GET /v1/session` is anonymous (Peek) and + `GET /v1/connections/.../status` was already passive — A6-S6's second + authenticated path exists to be reviewed. - **Tenant scoping** — the principal is built only from verified claims (`internal/api/principal.go`); tenant-admin surface is scoped and events are curated, not raw (`internal/api/events.go`, `statusview.go`; @@ -562,9 +567,30 @@ Additional items found while writing this document (not from A6): `ci.yml` job `partitioned`): real Set-Cookie attributes, in-frame reconnect across a backend rollout, revoked-lease cookie rejection. Same-site topology only; a cross-site deployment is not exercised. -- **Portal idle-extension depends on lease activity** — verify a stolen - portal cookie alone cannot extend itself, and that idle extension only - credits input activity measured server-side. +- **Portal idle-extension depends on lease activity** — Implemented + (FIX-IDLE): portal reads are all passive server-side, so no GET a client + can shape extends the idle window; extension only ever credits + (a) mutations — including the explicit activity beat + `POST /v1/session:touch` — passive auth + CSRF with the slide applied + explicitly only after the token check, so a cookie-only request never + earns a slide; login-family rate limit keyed on the session digest; the + SPA sends it on pointer/key/navigation events throttled to 1/min, never + from timer polls — and (b) RFB input measured broker-side. Forging a + touch requires the session cookie + CSRF token — the same bar as any + mutation, so the beat grants nothing a caller could not already do. That input touch + is now scoped to the bound portal session digest — the session the + stream's lease was minted under — rather than every session of the + principal (SR-1-F3; `TouchSessionDigest`, + `internal/store/sessions.go`), with the principal-wide path kept only + as the NULL-digest fallback for pre-binding leases. Lease + redeem/renew/rehydrate honour the portal idle window (SR-1-F2): the + liveness re-checks in `loadLease` and `RedeemTicket` consult + `last_seen_at` under `WithSessionIdle`, so an idled-out session's lease + is revoked at the next renew (reason `invalid`) and a stale cookie + cannot rehydrate it. Renew deliberately does NOT slide the window — + otherwise a connected-but-idle stream would pin the session open + forever — so an abandoned desktop tab dies with its portal session + inside one renew cycle. - **Metrics listener** is scrape-only but has no auth — Implemented: served on the dedicated ClusterIP `backend-metrics` Service; the only rule opening the metrics port is `allow-metrics-scrape` admitting diff --git a/internal/api/adminquota.go b/internal/api/adminquota.go index 13fad2c3..627869fb 100644 --- a/internal/api/adminquota.go +++ b/internal/api/adminquota.go @@ -213,11 +213,11 @@ func NewAdminQuotaHandler(src AdminQuotaSource, dir Directory, t TenantResolver, } // MountAdminQuotaRoutes registers the admin quota routes audited (see -// MountWorkspaceRoutes): RequireAuth+audit on the read, RequireAuth+audit+ -// RequireCSRF on the write — the read is inside the wrapper too, so admin +// MountWorkspaceRoutes): RequireAuthPassive+audit on the read, +// RequireAuth+audit+RequireCSRF on the write — the read is inside the wrapper too, so admin // API coverage is total: every /v1/admin/ request leaves an audit event. func MountAdminQuotaRoutes(mux *http.ServeMux, authn *Authenticator, h *AdminQuotaHandler) { - mux.Handle(routeAdminQuotaGet, authn.RequireAuth( + mux.Handle(routeAdminQuotaGet, authn.RequireAuthPassive( audited(h.audit, routeAdminQuotaGet, http.HandlerFunc(h.Get)))) mux.Handle(routeAdminQuotaSet, authn.RequireAuth( audited(h.audit, routeAdminQuotaSet, diff --git a/internal/api/adminuserlimit.go b/internal/api/adminuserlimit.go index 325f7609..97304e1a 100644 --- a/internal/api/adminuserlimit.go +++ b/internal/api/adminuserlimit.go @@ -123,12 +123,12 @@ func (h *AdminUserLimitsHandler) WithAuditSink(s observability.AuditSink) *Admin } // MountAdminUserLimitRoutes registers the routes audited (see -// MountAdminQuotaRoutes): RequireAuth+audit on the read, +// MountAdminQuotaRoutes): RequireAuthPassive+audit on the read, // RequireAuth+audit+RequireCSRF on the writes — denied (non-admin, // cross-tenant) attempts emit the route's table action with outcome // denied; a write that decodes resolves to the set or clear variant. func MountAdminUserLimitRoutes(mux *http.ServeMux, authn *Authenticator, h *AdminUserLimitsHandler) { - mux.Handle(routeAdminUserLimitsGet, authn.RequireAuth( + mux.Handle(routeAdminUserLimitsGet, authn.RequireAuthPassive( audited(h.audit, routeAdminUserLimitsGet, http.HandlerFunc(h.Get)))) mux.Handle(routeAdminUserLimitsPut, authn.RequireAuth( audited(h.audit, routeAdminUserLimitsPut, diff --git a/internal/api/auditroutes.go b/internal/api/auditroutes.go index 7697875f..0409e904 100644 --- a/internal/api/auditroutes.go +++ b/internal/api/auditroutes.go @@ -21,6 +21,7 @@ import ( const ( auditActionSessionLogout = "session.logout" auditActionSessionRevokeAll = "session.revoke_all" + auditActionSessionTouch = "session.touch" auditActionWorkspaceCreate = "workspace.create" auditActionWorkspaceStart = "workspace.start" auditActionWorkspaceStop = "workspace.stop" @@ -42,6 +43,7 @@ const ( // pattern can never drift from its action. const ( routeLogout = "POST /v1/logout" + routeSessionTouch = "POST /v1/session:touch" routeSessionRevokeAll = "POST /v1/me/sessions:revoke-all" routeWorkspaceCreate = "POST /v1/workspaces" routeWorkspaceDelete = "DELETE /v1/workspaces/{id}" @@ -71,6 +73,7 @@ type auditedRoute struct { // under /v1/admin/. var auditedRoutes = map[string]auditedRoute{ routeLogout: {auditActionSessionLogout, ""}, + routeSessionTouch: {auditActionSessionTouch, ""}, routeSessionRevokeAll: {auditActionSessionRevokeAll, ""}, routeWorkspaceCreate: {auditActionWorkspaceCreate, ""}, routeWorkspaceDelete: {auditActionWorkspaceDelete, "id"}, diff --git a/internal/api/auth.go b/internal/api/auth.go index 5aea442d..03c4a9db 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -192,6 +192,12 @@ type SessionStore interface { // (D18). principal is the "issuer|subject" owner string. Returns the // number of sessions touched. TouchPrincipal(ctx context.Context, principal string) (int64, error) + // TouchSessionDigest slides last_seen_at for exactly one session — the + // row keyed by digestHex (hex of SHA-256(session id), the form a lease + // records) — while it is still inside the idle window and before its + // absolute expiry (SR-1-F3). Returns the number of sessions touched + // (0 or 1). + TouchSessionDigest(ctx context.Context, digestHex string) (int64, error) Delete(ctx context.Context, id string) error } @@ -288,6 +294,20 @@ func (s *InMemorySessionStore) TouchPrincipal(_ context.Context, principal strin return n, nil } +// TouchSessionDigest slides last_seen_at for the single session the +// digestHex row key names (SR-1-F3) — same liveness guards as +// TouchPrincipal, so an expired session is never revived. +func (s *InMemorySessionStore) TouchSessionDigest(_ context.Context, digestHex string) (int64, error) { + s.mu.Lock() + defer s.mu.Unlock() + sess := s.peekLocked(digestHex, s.now()) + if sess == nil { + return 0, nil + } + sess.LastSeenAt = s.now() + return 1, nil +} + func (s *InMemorySessionStore) Delete(_ context.Context, id string) error { s.mu.Lock() defer s.mu.Unlock() @@ -880,26 +900,33 @@ const inputTouchMinInterval = time.Minute const inputThrottleMaxEntries = 10_000 // InputHook returns the broker input hook (broker.WithInputHook): each -// recorded "input" event slides the portal idle timer of the lease's -// principal — the Principal.Owner() string "issuer|subject" — throttled to -// one store write per principal per minute. Input never revives a session -// that already expired; the store's TouchPrincipal WHERE clause excludes -// sessions outside the idle window (D18). -func (a *Authenticator) InputHook() func(ctx context.Context, principal string) { +// recorded "input" event slides the portal idle timer of the session the +// input arrived under — the lease's bound portal_session_digest — throttled +// to one store write per key per minute (SR-1-F3). A legacy lease carrying +// no digest falls back to the principal-wide touch. Input never revives a +// session that already expired; the store's WHERE clauses exclude sessions +// outside the idle window (D18). +func (a *Authenticator) InputHook() func(ctx context.Context, principal, sessionDigest string) { hook, _ := a.newInputHook(inputThrottleMaxEntries) return hook } // newInputHook builds the throttled hook over an LRU of at most max -// principals; size reports the current entry count (tests). -func (a *Authenticator) newInputHook(max int) (hook func(ctx context.Context, principal string), size func() int) { +// keys — a session digest when the lease names one, else the principal — +// so a multi-session principal throttles per session, not per identity; +// size reports the current entry count (tests). +func (a *Authenticator) newInputHook(max int) (hook func(ctx context.Context, principal, sessionDigest string), size func() int) { var mu sync.Mutex order := list.New() // front = most recently seen; elements are *throttleEntry - byPrincipal := map[string]*list.Element{} - hook = func(ctx context.Context, principal string) { + byKey := map[string]*list.Element{} + hook = func(ctx context.Context, principal, sessionDigest string) { + key := "p:" + principal + if sessionDigest != "" { + key = "s:" + sessionDigest + } now := a.now() mu.Lock() - if el, ok := byPrincipal[principal]; ok { + if el, ok := byKey[key]; ok { e := el.Value.(*throttleEntry) order.MoveToFront(el) if now.Sub(e.at) < inputTouchMinInterval { @@ -908,15 +935,21 @@ func (a *Authenticator) newInputHook(max int) (hook func(ctx context.Context, pr } e.at = now } else { - byPrincipal[principal] = order.PushFront(&throttleEntry{principal: principal, at: now}) + byKey[key] = order.PushFront(&throttleEntry{key: key, at: now}) if order.Len() > max { oldest := order.Back() order.Remove(oldest) - delete(byPrincipal, oldest.Value.(*throttleEntry).principal) + delete(byKey, oldest.Value.(*throttleEntry).key) } } mu.Unlock() - if _, err := a.sessions.TouchPrincipal(ctx, principal); err != nil { + var err error + if sessionDigest != "" { + _, err = a.sessions.TouchSessionDigest(ctx, sessionDigest) + } else { + _, err = a.sessions.TouchPrincipal(ctx, principal) + } + if err != nil { a.log.Warn("session idle touch failed", "err", err) } } @@ -928,11 +961,12 @@ func (a *Authenticator) newInputHook(max int) (hook func(ctx context.Context, pr return hook, size } -// throttleEntry is one LRU element: when the principal's session was last -// touched. +// throttleEntry is one LRU element: when this key's session(s) were last +// touched — "s:"+digest keys a single bound session, "p:"+principal the +// legacy principal-wide touch. type throttleEntry struct { - principal string - at time.Time + key string + at time.Time } // groupsAllowed enforces RequiredGroups: with none configured the gate is diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 0c23233a..e1131753 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -109,6 +109,7 @@ func newTestEnvFull(t *testing.T, issuer func(*oidctest.Issuer), mutate func(*Au MountRevokeAllRoute(mux, a) MountMeRoutes(mux, a, NewMeHandler(testSessionDomain)) MountSessionProbeRoute(mux, a) + MountSessionTouchRoute(mux, a) mux.Handle("/v1/echo-owner", a.RequireAuth(a.RequireCSRF(http.HandlerFunc(echoOwnerHandler)))) var h http.Handler = mux @@ -456,7 +457,7 @@ func TestSessionIdleAndAbsoluteExpiry(t *testing.T) { resp.Body.Close() sess = findCookie(cookies, env.auth.SessionCookieName()) fc.Advance(30 * time.Second) - r = env.authedGet(t, sess, "/v1/me") // touches idle + r = env.authedGet(t, sess, "/v1/me") // passive read — does not touch idle r.Body.Close() if r.StatusCode != http.StatusOK { t.Fatalf("second session rejected early: %d", r.StatusCode) diff --git a/internal/api/data.go b/internal/api/data.go index dade268c..24a49bdf 100644 --- a/internal/api/data.go +++ b/internal/api/data.go @@ -254,10 +254,11 @@ func (h *DataHandler) WithDirectory(d Directory) *DataHandler { return h } -// MountDataRoutes registers the retained-data routes: RequireAuth on the -// list read, RequireAuth+RequireCSRF on attach/purge writes. +// MountDataRoutes registers the retained-data routes: RequireAuthPassive +// on the reads (the portal polls them), RequireAuth+RequireCSRF on +// attach/purge writes. func MountDataRoutes(mux *http.ServeMux, authn *Authenticator, h *DataHandler) { - safe := func(h http.Handler) http.Handler { return authn.RequireAuth(h) } + safe := func(h http.Handler) http.Handler { return authn.RequireAuthPassive(h) } // unsafe mounts an audited mutation route (see MountWorkspaceRoutes). unsafe := func(pattern string, next http.Handler) { mux.Handle(pattern, authn.RequireAuth( diff --git a/internal/api/idlepoll_test.go b/internal/api/idlepoll_test.go new file mode 100644 index 00000000..7191076a --- /dev/null +++ b/internal/api/idlepoll_test.go @@ -0,0 +1,163 @@ +// Copyright (c) 2026 TinyOrbit +// SPDX-License-Identifier: MIT + +package api + +// FIX-IDLE: the portal's read endpoints are mounted behind +// RequireAuthPassive — the SPA polls workspace list/detail/events and the +// data lists on an 8-10 s interval, and under the old sliding mounts a +// visible-but-unattended tab kept its session alive forever. Only +// mutations and server-measured desktop input count as user activity (D18). + +import ( + "net/http" + "testing" + "time" +) + +// TestPolledReads_DoNotSlideIdleWindow: interval polls on the read surface +// authenticate but never slide — the +50 s poll succeeds yet the idle +// window still lapses at +60 s. Regression: on the unfixed mounts this +// poll slid the window and the session never expired. +func TestPolledReads_DoNotSlideIdleWindow(t *testing.T) { + env := newWorkspaceEnv(t, newFakeBackend(), defaultCatalog(), defaultTenants()) + fc := &fakeClock{now: time.Now()} + env.store.WithClock(fc.Now) + env.auth.now = fc.Now + env.store.idle = time.Minute + + sess, _ := login(t, env, "user-a") + + fc.Advance(50 * time.Second) + r := env.authedGet(t, sess, "/v1/workspaces") + r.Body.Close() + if r.StatusCode != http.StatusOK { + t.Fatalf("poll inside the idle window rejected: %d", r.StatusCode) + } + fc.Advance(15 * time.Second) // past idle — the poll did not slide + r = env.authedGet(t, sess, "/v1/workspaces") + r.Body.Close() + if r.StatusCode != http.StatusUnauthorized { + t.Fatalf("idle-expired session kept alive by polling: %d", r.StatusCode) + } +} + +// TestMutation_StillSlidesIdleWindow: a POST on the write surface extends +// the window — the GETs in between never do, so the +50 s read lands +// inside idle only because the mutation slid. +func TestMutation_StillSlidesIdleWindow(t *testing.T) { + env := newWorkspaceEnv(t, newFakeBackend(), defaultCatalog(), defaultTenants()) + fc := &fakeClock{now: time.Now()} + env.store.WithClock(fc.Now) + env.auth.now = fc.Now + env.store.idle = time.Minute + + sess, csrf := login(t, env, "user-a") + + fc.Advance(50 * time.Second) + r := doReq(t, env, sess, csrf, http.MethodPost, "/v1/workspaces", + `{"name":"research-desktop","templateRef":"tpl_linuxdesktop"}`, + map[string]string{"Idempotency-Key": "key-mut-slide-1"}) + r.Body.Close() + if r.StatusCode != http.StatusCreated { + t.Fatalf("create inside the idle window rejected: %d", r.StatusCode) + } + fc.Advance(50 * time.Second) // inside idle only because the POST slid + r = env.authedGet(t, sess, "/v1/workspaces") + r.Body.Close() + if r.StatusCode != http.StatusOK { + t.Fatalf("session rejected inside the window a mutation opened: %d", r.StatusCode) + } + fc.Advance(61 * time.Second) // window lapses — nothing slid since the POST + r = env.authedGet(t, sess, "/v1/workspaces") + r.Body.Close() + if r.StatusCode != http.StatusUnauthorized { + t.Fatalf("idle-expired session accepted: %d", r.StatusCode) + } +} + +// TestMeRead_DoesNotSlideIdleWindow: GET /v1/me is the portal's bootstrap +// read, re-issued by retry loops — it is passive too (FIX-IDLE). +func TestMeRead_DoesNotSlideIdleWindow(t *testing.T) { + env := newTestEnv(t, nil) + fc := &fakeClock{now: time.Now()} + env.store.WithClock(fc.Now) + env.auth.now = fc.Now + env.store.idle = time.Minute + + resp, cookies := env.login(t) + resp.Body.Close() + sess := findCookie(cookies, env.auth.SessionCookieName()) + + fc.Advance(50 * time.Second) + r := env.authedGet(t, sess, "/v1/me") + r.Body.Close() + if r.StatusCode != http.StatusOK { + t.Fatalf("read inside the idle window rejected: %d", r.StatusCode) + } + fc.Advance(15 * time.Second) + r = env.authedGet(t, sess, "/v1/me") + r.Body.Close() + if r.StatusCode != http.StatusUnauthorized { + t.Fatalf("idle-expired session kept alive by /v1/me reads: %d", r.StatusCode) + } +} + +// TestSessionTouch_SlidesIdleWindow (FIX-IDLE): POST /v1/session:touch is +// the explicit user-activity beat — RequireAuth's sliding read extends the +// window while the passive reads around it never do. CSRF is required like +// every mutation; a missing token answers 403 and does not slide. +func TestSessionTouch_SlidesIdleWindow(t *testing.T) { + env := newTestEnv(t, nil) + fc := &fakeClock{now: time.Now()} + env.store.WithClock(fc.Now) + env.auth.now = fc.Now + env.store.idle = time.Minute + + sess, csrf := login(t, env, "user-a") + + fc.Advance(50 * time.Second) + r := doReq(t, env, sess, csrf, http.MethodPost, "/v1/session:touch", "", nil) + r.Body.Close() + if r.StatusCode != http.StatusNoContent { + t.Fatalf("touch rejected: %d", r.StatusCode) + } + fc.Advance(50 * time.Second) // inside idle only because the touch slid + r = env.authedGet(t, sess, "/v1/me") + r.Body.Close() + if r.StatusCode != http.StatusOK { + t.Fatalf("session rejected inside the window a touch opened: %d", r.StatusCode) + } + fc.Advance(61 * time.Second) + r = env.authedGet(t, sess, "/v1/me") + r.Body.Close() + if r.StatusCode != http.StatusUnauthorized { + t.Fatalf("idle-expired session accepted: %d", r.StatusCode) + } + + // No CSRF token: denied, and the denied request must NOT slide — the + // chain is RequireAuthPassive + RequireCSRF, so a cookie-only POST can + // never extend the window. The session then expires on schedule. + sess2, _ := login(t, env, "user-b") + fc.Advance(50 * time.Second) + r = doReq(t, env, sess2, &http.Cookie{Value: "forged"}, http.MethodPost, + "/v1/session:touch", "", nil) + r.Body.Close() + if r.StatusCode != http.StatusForbidden { + t.Fatalf("touch without CSRF: %d, want 403", r.StatusCode) + } + fc.Advance(15 * time.Second) // past idle — the denied touch did not slide + r = env.authedGet(t, sess2, "/v1/me") + r.Body.Close() + if r.StatusCode != http.StatusUnauthorized { + t.Fatalf("denied touch slid the idle window: %d", r.StatusCode) + } + + // Anonymous: 401. + r = doReq(t, env, &http.Cookie{Value: "no-such-session"}, csrf, + http.MethodPost, "/v1/session:touch", "", nil) + r.Body.Close() + if r.StatusCode != http.StatusUnauthorized { + t.Fatalf("touch without session: %d, want 401", r.StatusCode) + } +} diff --git a/internal/api/inputhook_throttle_test.go b/internal/api/inputhook_throttle_test.go index 9e960461..5b779ae9 100644 --- a/internal/api/inputhook_throttle_test.go +++ b/internal/api/inputhook_throttle_test.go @@ -24,7 +24,7 @@ func TestInputHookThrottle_Bounded(t *testing.T) { ctx := context.Background() for i := 0; i < 3*max; i++ { - hook(ctx, fmt.Sprintf("%s|user-%d", env.issuer.URL(), i)) + hook(ctx, fmt.Sprintf("%s|user-%d", env.issuer.URL(), i), "") if n := size(); n > max { t.Fatalf("throttle map holds %d entries after %d principals, want <= %d", n, i+1, max) } @@ -39,13 +39,13 @@ func TestInputHookThrottle_Bounded(t *testing.T) { counting := &countingTouchStore{SessionStore: env.auth.sessions, n: &touches} env.auth.sessions = counting recent := fmt.Sprintf("%s|user-%d", env.issuer.URL(), 3*max-1) - hook(ctx, recent) + hook(ctx, recent, "") if touches != 0 { t.Fatalf("recent principal touched the store again within the minute (%d writes)", touches) } // …and writes again once the minute has passed. fc.Advance(inputTouchMinInterval + time.Second) - hook(ctx, recent) + hook(ctx, recent, "") if touches != 1 { t.Fatalf("principal not touched after the throttle interval (%d writes)", touches) } diff --git a/internal/api/me.go b/internal/api/me.go index ffc1d8a3..1e617650 100644 --- a/internal/api/me.go +++ b/internal/api/me.go @@ -25,10 +25,11 @@ func NewMeHandler(sessionDomain string) *MeHandler { return &MeHandler{sessionDomain: sessionDomain} } -// MountMeRoutes registers GET /v1/me behind sliding auth (a portal page load -// counts as activity). +// MountMeRoutes registers GET /v1/me behind passive auth: it is the +// portal bootstrap read, re-issued by retry loops, so it must not slide +// the idle timer (D18) — only mutations and desktop input do. func MountMeRoutes(mux *http.ServeMux, authn *Authenticator, h *MeHandler) { - mux.Handle("GET /v1/me", authn.RequireAuth(http.HandlerFunc(h.Get))) + mux.Handle("GET /v1/me", authn.RequireAuthPassive(http.HandlerFunc(h.Get))) } // meView matches the Me schema in openapi.yaml. diff --git a/internal/api/me_test.go b/internal/api/me_test.go index 7f7fc5b0..5918353a 100644 --- a/internal/api/me_test.go +++ b/internal/api/me_test.go @@ -222,7 +222,7 @@ func TestInputActivity_ExtendsIdle(t *testing.T) { hook := env.auth.InputHook() fc.Advance(25 * time.Minute) - hook(context.Background(), env.issuer.URL()+"|alice") + hook(context.Background(), env.issuer.URL()+"|alice", sessionKey(sess.Value)) fc.Advance(15 * time.Minute) // t = 40 min — inside idle only because of the input r := env.authedGet(t, sess, "/v1/me") @@ -247,7 +247,7 @@ func TestInputActivity_DoesNotReviveExpiredSession(t *testing.T) { hook := env.auth.InputHook() fc.Advance(31 * time.Minute) // past the 30 m idle window - hook(context.Background(), env.issuer.URL()+"|alice") + hook(context.Background(), env.issuer.URL()+"|alice", sessionKey(sess.Value)) r := env.authedGet(t, sess, "/v1/me") defer r.Body.Close() @@ -275,7 +275,7 @@ func TestInputActivity_OtherPrincipalUntouched(t *testing.T) { hook := env.auth.InputHook() fc.Advance(25 * time.Minute) - hook(context.Background(), env.issuer.URL()+"|alice") + hook(context.Background(), env.issuer.URL()+"|alice", sessionKey(sessX.Value)) fc.Advance(10 * time.Minute) // t = 35 min: X fresh (25 m), Y dead (35 m) r := env.authedGet(t, sessX, "/v1/me") @@ -289,3 +289,73 @@ func TestInputActivity_OtherPrincipalUntouched(t *testing.T) { t.Fatalf("session Y slid by X's input: %d", r.StatusCode) } } + +// TestInputActivity_SlidesOnlyBoundSession (SR-1-F3): input under a lease +// credits the session digest the lease was minted under — a second, +// zero-request session of the SAME principal idles out on schedule; a +// legacy NULL digest (no binding recorded) keeps the principal-wide +// fallback. +func TestInputActivity_SlidesOnlyBoundSession(t *testing.T) { + env, fc := inputEnv(t) + sessA, _ := login(t, env, "alice") + now := fc.Now + // A sibling session of the same principal — e.g. a second browser tab's + // login — that has seen no activity of its own. + err := env.store.Save(context.Background(), &Session{ + ID: "sibling-session", + Principal: Principal{Issuer: env.issuer.URL(), Subject: "alice", TenantID: "tenant-a"}, + CreatedAt: now(), LastSeenAt: now(), ExpiresAt: now().Add(12 * time.Hour), + }) + if err != nil { + t.Fatalf("plant session: %v", err) + } + sessB := &http.Cookie{Name: env.auth.SessionCookieName(), Value: "sibling-session"} + + hook := env.auth.InputHook() + fc.Advance(25 * time.Minute) + hook(context.Background(), env.issuer.URL()+"|alice", sessionKey(sessA.Value)) + + fc.Advance(10 * time.Minute) // t = 35 min: A fresh (25 m), B dead (35 m) + r := env.authedGet(t, sessA, "/v1/me") + r.Body.Close() + if r.StatusCode != http.StatusOK { + t.Fatalf("bound session rejected after own input: %d", r.StatusCode) + } + r = env.authedGet(t, sessB, "/v1/me") + r.Body.Close() + if r.StatusCode != http.StatusUnauthorized { + t.Fatalf("sibling session slid by a digest-scoped input: %d", r.StatusCode) + } +} + +// TestInputActivity_NullDigestFallsBackToPrincipal (SR-1-F3): a lease minted +// before the binding column exists carries no digest — the hook degrades to +// the principal-wide touch so pre-upgrade streams keep working during a +// rolling deploy. +func TestInputActivity_NullDigestFallsBackToPrincipal(t *testing.T) { + env, fc := inputEnv(t) + sessA, _ := login(t, env, "alice") + now := fc.Now + err := env.store.Save(context.Background(), &Session{ + ID: "sibling-session", + Principal: Principal{Issuer: env.issuer.URL(), Subject: "alice", TenantID: "tenant-a"}, + CreatedAt: now(), LastSeenAt: now(), ExpiresAt: now().Add(12 * time.Hour), + }) + if err != nil { + t.Fatalf("plant session: %v", err) + } + sessB := &http.Cookie{Name: env.auth.SessionCookieName(), Value: "sibling-session"} + + hook := env.auth.InputHook() + fc.Advance(25 * time.Minute) + hook(context.Background(), env.issuer.URL()+"|alice", "") // legacy lease + + fc.Advance(10 * time.Minute) + for _, sess := range []*http.Cookie{sessA, sessB} { + r := env.authedGet(t, sess, "/v1/me") + r.Body.Close() + if r.StatusCode != http.StatusOK { + t.Fatalf("principal-fallback input did not slide %s: %d", sess.Value, r.StatusCode) + } + } +} diff --git a/internal/api/middleware_test.go b/internal/api/middleware_test.go index fd07e319..7160614c 100644 --- a/internal/api/middleware_test.go +++ b/internal/api/middleware_test.go @@ -381,6 +381,9 @@ func (s failingSessionStore) Peek(context.Context, string) (*Session, error) { r func (s failingSessionStore) TouchPrincipal(context.Context, string) (int64, error) { return 0, s.err } +func (s failingSessionStore) TouchSessionDigest(context.Context, string) (int64, error) { + return 0, s.err +} func (s failingSessionStore) Delete(context.Context, string) error { return s.err } // TestRequireAuth_SessionStoreErrors: a store error is not "no session". diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index d8620311..ea07e0a2 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -27,6 +27,20 @@ info: transactional outbox (see ADR 0002); Kubernetes RBAC denies direct CR writes by end users. + ## Session idle timeout + + Sessions carry a sliding idle timeout (default 30 min) inside a fixed + absolute cap. The server decides what counts as activity — never the + client: **every `GET` endpoint on this API is passive** and + authenticates without extending the idle deadline, so the portal's + interval polls cannot hold a visible-but-unattended session open. + Sliding happens only through mutations — including the explicit + user-interaction beat `POST /v1/session:touch` — and server-measured + interactive desktop input. A session that has idled out is dead + everywhere: lease redeem, renew and rehydrate consult the same window, + so an open desktop stream dies with the session it was launched under + within one renew cycle. + ## Idempotency Mutating operations that allocate resources (`POST /v1/workspaces`, @@ -130,6 +144,43 @@ paths: "503": $ref: "#/components/responses/Unavailable" + /v1/session:touch: + post: + operationId: touchSession + summary: Activity beat + description: | + The explicit "the user is here" signal (FIX-IDLE). The session is + authenticated passively, the CSRF token checked, and only then does + the handler slide the idle deadline explicitly — a cookie-only + request that fails the token check can never extend the window. + The handler answers `204` with no body. It exists because every + `GET` on this API is passive: the portal calls it only on real + user interaction (pointer, key, navigation), never from a + timer-driven poll, and throttles to at most one call per minute. + + Requires the session cookie and the `X-CSRF-Token` header (`401` + without a session, `403` without a valid token) — forging a touch + is therefore exactly as hard as forging any mutation. The call is + rate-limited like the rest of the login/session family, keyed on + the session digest when the cookie is valid. + + A session that already idled out cannot be revived by a touch: + authentication fails `401` before the write, same as any route. + tags: [session] + responses: + "204": + description: Idle deadline extended. + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" + "429": + $ref: "#/components/responses/RateLimited" + "500": + $ref: "#/components/responses/InternalError" + "503": + $ref: "#/components/responses/Unavailable" + /v1/logout: post: operationId: logout diff --git a/internal/api/quota.go b/internal/api/quota.go index b5c39fb7..2583612a 100644 --- a/internal/api/quota.go +++ b/internal/api/quota.go @@ -86,9 +86,9 @@ func NewQuotaHandler(src QuotaSource, dir Directory, t TenantResolver) *QuotaHan return &QuotaHandler{source: src, dir: dir, tenants: t} } -// MountQuotaRoutes registers GET /v1/quota behind RequireAuth. +// MountQuotaRoutes registers GET /v1/quota behind passive auth (D18). func MountQuotaRoutes(mux *http.ServeMux, authn *Authenticator, h *QuotaHandler) { - mux.Handle("GET /v1/quota", authn.RequireAuth(http.HandlerFunc(h.Get))) + mux.Handle("GET /v1/quota", authn.RequireAuthPassive(http.HandlerFunc(h.Get))) } // Get handles GET /v1/quota: tenant admins see every owner's usage row; diff --git a/internal/api/session_probe.go b/internal/api/session_probe.go index d0f9f4e9..3696f367 100644 --- a/internal/api/session_probe.go +++ b/internal/api/session_probe.go @@ -4,10 +4,15 @@ package api import ( + "errors" "net/http" + + "github.com/tinyorbitvn/tinycdi/internal/observability" + "github.com/tinyorbitvn/tinycdi/internal/store" ) -// session_probe.go implements GET /v1/session — the anonymous, passive +// session_probe.go implements the session surface's two one-liners: +// GET /v1/session — the anonymous, passive // "is there a live portal session?" probe (FX-R13b). The portal calls it // before GET /v1/me so a signed-out load never issues a request that fails // with 401: the browser logs every failed fetch to the console and page @@ -48,3 +53,47 @@ func (a *Authenticator) SessionProbeHandler(w http.ResponseWriter, r *http.Reque w.Header().Set("Cache-Control", "no-store") respondJSON(w, sessionProbeView{Authenticated: authenticated}) } + +// MountSessionTouchRoute registers POST /v1/session:touch — the explicit +// activity beat (FIX-IDLE): every cookie-authenticated GET is passive, so +// the portal sends this on real user interaction to slide the idle +// deadline. The chain is deliberately RequireAuthPassive + RequireCSRF: +// authentication must NOT slide, because a cookie-only POST that fails the +// token check would otherwise extend the window it could not earn — the +// slide happens explicitly inside the handler, after CSRF passed. The +// audited wrapper emits the session.touch event; optional middleware wraps +// the chain (the production mount applies the session-digest-keyed +// login-family limiter). +func MountSessionTouchRoute(mux *http.ServeMux, authn *Authenticator, wrap ...func(http.Handler) http.Handler) { + var h http.Handler = authn.RequireAuthPassive( + audited(lateAuditSink{func() observability.AuditSink { return authn.auditSink }}, routeSessionTouch, + authn.RequireCSRF(http.HandlerFunc(authn.SessionTouchHandler)))) + for _, w := range wrap { + h = w(h) + } + mux.Handle(routeSessionTouch, h) +} + +// SessionTouchHandler answers 204 after sliding the session's idle +// deadline explicitly: Get is the only write, and it runs only here — +// after authentication and CSRF succeeded — so a denied request never +// extends the window. +func (a *Authenticator) SessionTouchHandler(w http.ResponseWriter, r *http.Request) { + sess, ok := SessionFromContext(r.Context()) + if !ok || sess == nil { + writeError(w, r, CodeUnauthenticated, "authentication required") + return + } + if _, err := a.sessions.Get(r.Context(), sess.ID); err != nil { + switch { + case errors.Is(err, ErrSessionNotFound): + writeError(w, r, CodeUnauthenticated, "session missing or expired") + case store.IsTransient(err): + writeError(w, r, CodeUnavailable, "session store unavailable") + default: + writeError(w, r, CodeInternal, "internal error") + } + return + } + w.WriteHeader(http.StatusNoContent) +} diff --git a/internal/api/workspaces.go b/internal/api/workspaces.go index 7d792b48..cdbb106d 100644 --- a/internal/api/workspaces.go +++ b/internal/api/workspaces.go @@ -213,10 +213,11 @@ func (h *WorkspaceHandler) WithImageBlockAfter(d time.Duration) *WorkspaceHandle } // MountWorkspaceRoutes registers the workspace/template routes with the -// authn middleware applied: RequireAuth on reads, RequireAuth+RequireCSRF -// on writes. +// authn middleware applied: RequireAuthPassive on reads — the portal polls +// them on an interval, and only mutations and desktop input are user +// activity (D18) — RequireAuth+RequireCSRF on writes. func MountWorkspaceRoutes(mux *http.ServeMux, authn *Authenticator, h *WorkspaceHandler, th *TemplateHandler) { - safe := func(h http.Handler) http.Handler { return authn.RequireAuth(h) } + safe := func(h http.Handler) http.Handler { return authn.RequireAuthPassive(h) } // unsafe mounts an audited mutation route: RequireAuth verifies the // principal, audited records the request's outcome on the shared // routeAudit cell, RequireCSRF guards the state change itself, so a diff --git a/internal/backend/wire.go b/internal/backend/wire.go index 5eae4b80..9e36224c 100644 --- a/internal/backend/wire.go +++ b/internal/backend/wire.go @@ -399,7 +399,12 @@ func (b *Backend) wireMerged(ctx context.Context, cfg Config, id broker.GatewayI broker.WithGatewayAudience(id.Audience), broker.WithCredentialSource(broker.NewK8sCredentialSource(kc, tenants)), broker.WithMetrics(metrics), - broker.WithLogger(log)) + broker.WithLogger(log), + // The lease layer honours the portal session idle window too + // (FIX-IDLE / SR-1-F2): an idled-out session can no longer redeem, + // renew or rehydrate a lease — the stream dies inside one renew + // cycle instead of outliving the session it was minted under. + broker.WithSessionIdle(cfg.SessionIdle)) // Expiry planner (design §8): periodically evaluate running workspaces // against recorded session activity and emit generation-fenced stop @@ -711,6 +716,7 @@ func appMux(authn *api.Authenticator, ws *api.WorkspaceHandler, tpl *api.Templat mux.Handle("GET /v1/auth/callback", callbackLimit(http.HandlerFunc(authn.CallbackHandler))) api.MountLogoutRoute(mux, authn) api.MountRevokeAllRoute(mux, authn, sessionLimit) + api.MountSessionTouchRoute(mux, authn, sessionLimit) api.MountSessionProbeRoute(mux, authn, sessionLimit) api.MountMeRoutes(mux, authn, me) api.MountWorkspaceRoutes(mux, authn, ws, tpl) @@ -874,6 +880,10 @@ func (a sessionStoreAdapter) TouchPrincipal(ctx context.Context, principal strin return a.s.TouchPrincipal(ctx, principal) } +func (a sessionStoreAdapter) TouchSessionDigest(ctx context.Context, digestHex string) (int64, error) { + return a.s.TouchSessionDigest(ctx, digestHex) +} + func (a sessionStoreAdapter) Delete(ctx context.Context, id string) error { return a.s.Delete(ctx, id) } diff --git a/internal/broker/activity.go b/internal/broker/activity.go index 7fd6af8d..142e5c56 100644 --- a/internal/broker/activity.go +++ b/internal/broker/activity.go @@ -158,11 +158,13 @@ func (b *Broker) ReportActivity(ctx context.Context, gw GatewayIdentity, leaseID if err := b.recordActivity(ctx, l.ID, PlatformID(l.WorkspaceUID), l.RuntimeGeneration, ev, now); err != nil { return err } - // Desktop input extends the owning user's PORTAL session idle timer - // (D18): the hook receives the lease's principal — the "iss|sub" owner - // string — after the event is durably recorded. + // Desktop input extends the owning session's PORTAL idle timer (D18): + // the hook receives the lease's principal and the bound + // portal_session_digest — input credits exactly the session the + // stream was launched under (SR-1-F3); a legacy NULL digest degrades + // to the principal-wide touch — after the event is durably recorded. if ev.Type == ActivityInput && b.inputHook != nil { - b.inputHook(ctx, l.PrincipalSubject) + b.inputHook(ctx, l.PrincipalSubject, l.PortalSessionDigest) } return nil } diff --git a/internal/broker/connection_state.go b/internal/broker/connection_state.go index 805931e0..4557ef10 100644 --- a/internal/broker/connection_state.go +++ b/internal/broker/connection_state.go @@ -119,11 +119,12 @@ func (b *Broker) ConnectionState(ctx context.Context, workspaceUID PlatformID, p } // WithInputHook registers fn to be called with the lease's principal -// (principal_subject — the "issuer|subject" owner string) on each recorded -// "input" activity event. The portal wires this to slide the owning user's -// portal session idle timer (D18); connected/disconnect events and rejected -// reports never invoke it. -func WithInputHook(fn func(ctx context.Context, principal string)) Option { +// (principal_subject — the "issuer|subject" owner string — plus the lease's +// bound portal_session_digest, "" for legacy NULL rows) on each recorded +// "input" activity event. The portal wires this to slide the idle timer of +// the session the input arrived under (D18, SR-1-F3); +// connected/disconnect events and rejected reports never invoke it. +func WithInputHook(fn func(ctx context.Context, principal, sessionDigest string)) Option { return func(b *Broker) { b.inputHook = fn } } diff --git a/internal/broker/connection_state_test.go b/internal/broker/connection_state_test.go index ac353f59..b61a0e9b 100644 --- a/internal/broker/connection_state_test.go +++ b/internal/broker/connection_state_test.go @@ -104,7 +104,7 @@ func TestInputHook_CalledWithPrincipal(t *testing.T) { got []string ) b := broker.New(db, src, broker.WithClock(clock), - broker.WithInputHook(func(_ context.Context, principal string) { + broker.WithInputHook(func(_ context.Context, principal, sessionDigest string) { mu.Lock() got = append(got, principal) mu.Unlock() diff --git a/internal/broker/lease_session_test.go b/internal/broker/lease_session_test.go index dc34f2a2..adf0424d 100644 --- a/internal/broker/lease_session_test.go +++ b/internal/broker/lease_session_test.go @@ -368,3 +368,99 @@ func TestLeaseSession_NoDoubleCount(t *testing.T) { t.Fatalf("metric mismatch:\n%v", err) } } + +// idleOutPortalSession ages the seeded sessions row's last_seen_at beyond +// any configured idle window — the shape an unattended session has when +// the lease layer next consults it (SR-1-F2). +func idleOutPortalSession(t *testing.T, db *store.DB, portalSessionID string) { + t.Helper() + tag, err := db.Pool().Exec(ctx, + `UPDATE sessions SET last_seen_at = now() - interval '1 hour' WHERE id = $1`, + portalSessionRowKey(portalSessionID)) + if err != nil || tag.RowsAffected() != 1 { + t.Fatalf("idle-out portal session: %v (rows=%d)", err, tag.RowsAffected()) + } +} + +// TestLeaseSession_IdleDeadSessionRevokesOnRenew (SR-1-F2): the portal idle +// window is honoured at the lease layer — a bound session whose +// last_seen_at is older than the window is dead exactly as RequireAuth +// reports it, so the next renew revokes the lease (reason 'invalid') and +// the cookie resolve fails closed; a freshly-touched session renews fine. +func TestLeaseSession_IdleDeadSessionRevokesOnRenew(t *testing.T) { + db, _, clock, src := setup(t) + seedWorkspace(t, db, "tenant-a", alice.Owner(), "ws-1") + src.set(readyBinding("ws-1", "tenant-a", alice.Owner(), 1, "rt-1", clock.Now())) + reg := prometheus.NewRegistry() + m := observability.NewMetrics(reg, nil) + b := broker.New(db, src, broker.WithClock(clock), broker.WithMetrics(m), + broker.WithSessionIdle(time.Minute)) + + lease := leaseForSess(t, db, b, gwA, "ws-1", false, "sess-1") + d := digestOf("cookie-1") + if err := b.BindSession(ctx, gwA, lease.ID, d); err != nil { + t.Fatalf("BindSession: %v", err) + } + + // Inside the window the lease renews. + if _, err := b.RenewLease(ctx, gwA, lease.ID, fenceOf(lease)); err != nil { + t.Fatalf("renew under live session = %v", err) + } + + idleOutPortalSession(t, db, "sess-1") + + // The next renew is terminal — same ErrRevoked the S17 barrier emits. + if _, err := b.RenewLease(ctx, gwA, lease.ID, fenceOf(lease)); !errors.Is(err, broker.ErrRevoked) { + t.Fatalf("renew under idle-dead session = %v, want ErrRevoked", err) + } + state, closed := leaseState(t, db, lease.ID) + if state != "revoked" || !closed { + t.Fatalf("lease state=%q closed=%v, want revoked+closed_at", state, closed) + } + if err := testutil.GatherAndCompare(reg, + strings.NewReader(leaseMissingSeries("invalid", 1)), + "tinycdi_lease_session_missing_total"); err != nil { + t.Fatalf("metric mismatch:\n%v", err) + } + + // Rehydrate (cookie -> lease resolve) fails closed too. + if _, err := b.LeaseBySession(ctx, gwA, d); !errors.Is(err, broker.ErrRevoked) { + t.Fatalf("rehydrate under idle-dead session = %v, want ErrRevoked", err) + } +} + +// TestLeaseSession_RedeemRejectsIdleDeadSession (SR-1-F2): a ticket minted +// while the portal session was live cannot redeem once the session has +// idled out — the redeem-time re-check consults the same idle window. +func TestLeaseSession_RedeemRejectsIdleDeadSession(t *testing.T) { + db, _, clock, src := setup(t) + seedWorkspace(t, db, "tenant-a", alice.Owner(), "ws-1") + src.set(readyBinding("ws-1", "tenant-a", alice.Owner(), 1, "rt-1", clock.Now())) + b := broker.New(db, src, broker.WithClock(clock), broker.WithSessionIdle(time.Minute)) + + seedPortalSession(t, db, "sess-1") + tk, err := b.IssueTicket(ctx, alice, "ws-1", false, "", "sess-1") + if err != nil { + t.Fatalf("IssueTicket: %v", err) + } + idleOutPortalSession(t, db, "sess-1") + if _, err := b.RedeemTicket(ctx, gwA, tk.Token); !errors.Is(err, broker.ErrRevoked) { + t.Fatalf("redeem under idle-dead session = %v, want ErrRevoked", err) + } +} + +// TestLeaseSession_IdleCheckDisabledWithoutOption: a broker configured +// without WithSessionIdle keeps the S17 predicate exactly as before — +// epoch + absolute expiry only — so replicas that never wire the option +// degrade to the old behaviour rather than revoking on sight. +func TestLeaseSession_IdleCheckDisabledWithoutOption(t *testing.T) { + db, b, clock, src := setup(t) // no WithSessionIdle + seedWorkspace(t, db, "tenant-a", alice.Owner(), "ws-1") + src.set(readyBinding("ws-1", "tenant-a", alice.Owner(), 1, "rt-1", clock.Now())) + + lease := leaseForSess(t, db, b, gwA, "ws-1", false, "sess-1") + idleOutPortalSession(t, db, "sess-1") + if _, err := b.RenewLease(ctx, gwA, lease.ID, fenceOf(lease)); err != nil { + t.Fatalf("renew without idle option = %v, want nil", err) + } +} diff --git a/internal/broker/leases.go b/internal/broker/leases.go index 44911ee4..ce4ffa6c 100644 --- a/internal/broker/leases.go +++ b/internal/broker/leases.go @@ -41,6 +41,12 @@ type Lease struct { // claim carried no valid id. Gateway-side correlator only: never a // metric label or log field. StreamOwnerTab string `json:"-"` + // PortalSessionDigest is the sessions.id key form (hex of the SHA-256 + // of the portal session id) the lease was minted under — populated on + // every loadLease read so input activity can be credited to exactly + // that session (SR-1-F3). "" for pre-migration-018 rows minted before + // the binding column existed. + PortalSessionDigest string `json:"-"` // ClipboardPolicy is the workspace template's clipboard policy as // recorded on the ticket at issue — populated only on redemption, so // the gateway's post-redemption redirect can re-assert the client's @@ -58,8 +64,9 @@ const ( portalSessionOK portalSessionCheck = "ok" // portalSessionAbsent — the bound sessions row is gone. portalSessionAbsent portalSessionCheck = "absent" - // portalSessionInvalid — the bound row exists but fails the epoch or - // absolute-expiry check (same semantics as RedeemTicket's re-check). + // portalSessionInvalid — the bound row exists but fails the epoch, + // absolute-expiry or idle-window check (same semantics as + // RedeemTicket's re-check; idle needs WithSessionIdle). portalSessionInvalid portalSessionCheck = "invalid" ) @@ -68,6 +75,26 @@ const ( // invalidates every bound lease on its very next check. const currentSessionEpochSQL = `(SELECT value FROM platform_meta WHERE key = 'session_epoch')` +// portalSessionLiveSQL returns the predicate asserting that the joined +// sessions row (alias s) backing a ticket/lease is still live: current +// epoch (post-restore rotations fail), inside its absolute expiry, and — +// when the broker knows the portal idle window (WithSessionIdle) — a +// last_seen_at still inside it (FIX-IDLE / SR-1-F2): a session that has +// idled out under RequireAuth is dead to the lease layer on the same +// terms. argIdx is the bind index the idle interval occupies; the second +// return carries the interval argument to append, or nil when no idle +// window is configured. +func (b *Broker) portalSessionLiveSQL(argIdx int) (string, []any) { + live := `s.epoch IS NOT DISTINCT FROM ` + currentSessionEpochSQL + ` + AND (s.expires_at IS NULL OR s.expires_at > now())` + if b.sessionIdle <= 0 { + return live, nil + } + return live + fmt.Sprintf(` + AND s.last_seen_at > now() - $%d::interval`, argIdx), + []any{fmt.Sprintf("%dms", b.sessionIdle.Milliseconds())} +} + // loadLease fetches the lease row including its lifecycle state and the // liveness of its bound portal session (S17 defence-in-depth): the CASE // rides the same row read, joined to sessions by primary key, so the check @@ -79,25 +106,28 @@ func (b *Broker) loadLease(ctx context.Context, leaseID string) (Lease, string, ownerTab *string ownerEpoch *int64 portalSess portalSessionCheck + digestHex *string ) + livePred, liveArgs := b.portalSessionLiveSQL(2) + args := append([]any{leaseID}, liveArgs...) err := b.db.Pool().QueryRow(ctx, `SELECT l.id, l.workspace_id, l.tenant_id, l.principal_subject, l.runtime_generation, l.runtime_uid, l.fencing_version, l.gateway_id, l.state, l.expires_at, l.stream_epoch, l.stream_owner_tab, l.stream_owner_epoch, + encode(l.portal_session_digest, 'hex'), CASE WHEN l.portal_session_digest IS NULL THEN 'ok' WHEN s.id IS NULL THEN 'absent' - WHEN s.epoch IS DISTINCT FROM `+currentSessionEpochSQL+` - OR (s.expires_at IS NOT NULL AND s.expires_at <= now()) THEN 'invalid' + WHEN NOT (`+livePred+`) THEN 'invalid' ELSE 'ok' END FROM connection_lease l LEFT JOIN sessions s ON s.id = encode(l.portal_session_digest, 'hex') - WHERE l.id = $1`, leaseID). + WHERE l.id = $1`, args...). Scan(&l.ID, &l.WorkspaceUID, &l.TenantID, &l.PrincipalSubject, &l.RuntimeGeneration, &l.RuntimeUID, &l.FencingVersion, &l.GatewayID, &state, &l.ExpiresAt, &l.StreamEpoch, &ownerTab, &ownerEpoch, - &portalSess) + &digestHex, &portalSess) if errors.Is(err, pgx.ErrNoRows) { return Lease{}, "", "", ErrLeaseInvalid } @@ -112,6 +142,9 @@ func (b *Broker) loadLease(ctx context.Context, leaseID string) (Lease, string, uint64(*ownerEpoch) == l.StreamEpoch { l.StreamOwnerTab = *ownerTab } + if digestHex != nil { + l.PortalSessionDigest = *digestHex + } return l, state, portalSess, nil } diff --git a/internal/broker/tickets.go b/internal/broker/tickets.go index 5e81cb30..5319f8b4 100644 --- a/internal/broker/tickets.go +++ b/internal/broker/tickets.go @@ -102,6 +102,14 @@ func WithLeaseTTL(d time.Duration) Option { return func(b *Broker) { b.leaseTTL // WithMaxBindingAge overrides MaxBindingAge. func WithMaxBindingAge(d time.Duration) Option { return func(b *Broker) { b.maxBindingAge = d } } +// WithSessionIdle sets the portal session idle window the lease layer +// honours (FIX-IDLE / SR-1-F2): a bound session whose last_seen_at is +// older than d fails the liveness re-checks on redeem, renew and +// rehydrate, matching the window RequireAuth enforces on the session +// itself. <=0 disables the idle clause — the epoch and absolute-expiry +// checks still apply. +func WithSessionIdle(d time.Duration) Option { return func(b *Broker) { b.sessionIdle = d } } + // WithGatewayAudience sets the session-gateway audience tickets are bound to. // The gateway's mTLS identity maps to this audience at the internal API. func WithGatewayAudience(aud string) Option { @@ -198,14 +206,22 @@ type Broker struct { maxBindingAge time.Duration audience string creds CredentialSource + // sessionIdle is the portal session's sliding inactivity window: a + // bound session whose last_seen_at is older than it fails the lease + // layer's liveness re-check, so an idled-out session can neither mint + // (redeem) nor keep (renew) nor rebuild (rehydrate) a stream + // (FIX-IDLE / SR-1-F2). <=0 disables the idle clause — the epoch and + // absolute-expiry checks still apply. + sessionIdle time.Duration // metrics counts broker-initiated lease revocations on a dead portal // session; nil disables them. metrics *observability.Metrics // log emits the one-line records for those revocations; nil disables. log *slog.Logger - // inputHook is invoked with the lease's principal on each recorded - // "input" activity event (D18); nil disables it. - inputHook func(ctx context.Context, principal string) + // inputHook is invoked with the lease's principal and bound portal + // session digest ("" for legacy NULL rows) on each recorded "input" + // activity event (D18); nil disables it. + inputHook func(ctx context.Context, principal, sessionDigest string) synthOnce sync.Once synthCreds *synthesizedCredentials @@ -479,18 +495,19 @@ func (b *Broker) RedeemTicket(ctx context.Context, gw GatewayIdentity, opaque st // revocation transaction, and a live session's tickets are revoked // with it — this read is the second barrier, so an outstanding // ticket dies with its session however the revoke itself fared. - // Epoch + absolute expiry mirror SessionStore's liveness rule; a + // Epoch + absolute expiry + idle window mirror SessionStore's + // liveness rule (idle needs WithSessionIdle, FIX-IDLE/SR-1-F2); a // restored dump or a rotated epoch fails closed. if portalSession != nil { var alive bool + livePred, liveArgs := b.portalSessionLiveSQL(2) + args := append([]any{portalSession}, liveArgs...) if err := tx.QueryRow(ctx, `SELECT EXISTS ( - SELECT 1 FROM sessions - WHERE id = encode($1, 'hex') - AND epoch = (SELECT value FROM platform_meta - WHERE key = 'session_epoch') - AND (expires_at IS NULL OR expires_at > now()))`, - portalSession).Scan(&alive); err != nil { + SELECT 1 FROM sessions s + WHERE s.id = encode($1, 'hex') + AND `+livePred+`)`, + args...).Scan(&alive); err != nil { return fmt.Errorf("broker: portal session check: %w", err) } if !alive { diff --git a/internal/observability/metrics.go b/internal/observability/metrics.go index 6085bdc3..d76524ae 100644 --- a/internal/observability/metrics.go +++ b/internal/observability/metrics.go @@ -99,8 +99,8 @@ var ( // a new series. auditEventActions = map[string]struct{}{ "http.request": {}, "session.logout": {}, "session.revoke": {}, - "session.revoke_all": {}, - "session.list": {}, "session.host_mismatch": {}, + "session.revoke_all": {}, "session.touch": {}, + "session.list": {}, "session.host_mismatch": {}, "workspace.create": {}, "workspace.start": {}, "workspace.stop": {}, "workspace.delete": {}, "connection.create": {}, "data.attach": {}, "data.purge": {}, diff --git a/internal/store/sessions.go b/internal/store/sessions.go index f52b1432..1cde9439 100644 --- a/internal/store/sessions.go +++ b/internal/store/sessions.go @@ -283,6 +283,45 @@ func (s *SessionStore) TouchPrincipal(ctx context.Context, principal string) (in return tag.RowsAffected(), nil } +// TouchSessionDigestSQL and TouchSessionDigestIdleSQL are the statements +// behind TouchSessionDigest (without / with an idle window): they address +// exactly one session row — $1 is the sessions.id key form (hex of the +// SHA-256 digest a lease recorded as portal_session_digest); the idle +// variant takes the window as $2. +const ( + TouchSessionDigestSQL = ` + UPDATE sessions SET last_seen_at = now() + WHERE id = $1 + AND epoch = ` + currentEpochSQL + ` + AND (expires_at IS NULL OR expires_at > now())` + TouchSessionDigestIdleSQL = TouchSessionDigestSQL + ` + AND last_seen_at > now() - $2::interval` +) + +// TouchSessionDigest slides last_seen_at for exactly the session row the +// digest names — the session the desktop stream's input arrived under +// (SR-1-F3): input under one session's lease no longer refreshes the +// principal's other sessions. The guards are identical to TouchPrincipal — +// live epoch, inside absolute expiry, inside the idle window — so input +// still can never revive an expired session. Returns the row count +// actually updated (0 for an unknown or dead row). +func (s *SessionStore) TouchSessionDigest(ctx context.Context, digestHex string) (int64, error) { + var ( + tag pgconn.CommandTag + err error + ) + if s.idle > 0 { + tag, err = s.db.Pool().Exec(ctx, TouchSessionDigestIdleSQL, + digestHex, fmt.Sprintf("%dms", s.idle.Milliseconds())) + } else { + tag, err = s.db.Pool().Exec(ctx, TouchSessionDigestSQL, digestHex) + } + if err != nil { + return 0, fmt.Errorf("session touch: %w", err) + } + return tag.RowsAffected(), nil +} + // Delete removes the session; deleting a missing ID is a no-op. func (s *SessionStore) Delete(ctx context.Context, id string) error { _, err := s.db.Pool().Exec(ctx, `DELETE FROM sessions WHERE id = $1`, sessionKey(id)) diff --git a/tests/integration/api_admission_test.go b/tests/integration/api_admission_test.go index eb2f3ec5..118ee7ba 100644 --- a/tests/integration/api_admission_test.go +++ b/tests/integration/api_admission_test.go @@ -636,6 +636,10 @@ func (a *pgSessionAdapter) TouchPrincipal(ctx context.Context, principal string) return a.s.TouchPrincipal(ctx, principal) } +func (a *pgSessionAdapter) TouchSessionDigest(ctx context.Context, digestHex string) (int64, error) { + return a.s.TouchSessionDigest(ctx, digestHex) +} + func (a *pgSessionAdapter) Delete(ctx context.Context, id string) error { return a.s.Delete(ctx, id) } diff --git a/tests/integration/session_touch_test.go b/tests/integration/session_touch_test.go index 1e461584..66b485bf 100644 --- a/tests/integration/session_touch_test.go +++ b/tests/integration/session_touch_test.go @@ -7,6 +7,8 @@ package integration import ( "context" + "crypto/sha256" + "encoding/hex" "fmt" "strings" "testing" @@ -40,12 +42,15 @@ func TestTouchPrincipal_UsesIndex(t *testing.T) { db := newDB(t) ctx := context.Background() for _, tc := range []struct { - name string - sql string - idle bool + name string + sql string + idle bool + index string }{ - {"idle window", store.TouchPrincipalIdleSQL, true}, - {"no idle window", store.TouchPrincipalSQL, false}, + {"principal idle window", store.TouchPrincipalIdleSQL, true, "sessions_issuer_subject"}, + {"principal no idle window", store.TouchPrincipalSQL, false, "sessions_issuer_subject"}, + {"digest idle window", store.TouchSessionDigestIdleSQL, true, "sessions_pkey"}, + {"digest no idle window", store.TouchSessionDigestSQL, false, "sessions_pkey"}, } { tx, err := db.Pool().Begin(ctx) if err != nil { @@ -68,8 +73,8 @@ func TestTouchPrincipal_UsesIndex(t *testing.T) { } rows.Close() _ = tx.Rollback(ctx) - if !strings.Contains(plan.String(), "sessions_issuer_subject") { - t.Errorf("%s: plan does not use the (issuer, subject) index:\n%s", tc.name, plan.String()) + if !strings.Contains(plan.String(), tc.index) { + t.Errorf("%s: plan does not use %s:\n%s", tc.name, tc.index, plan.String()) } } } @@ -114,3 +119,58 @@ func TestTouchPrincipal_SplitsAtFirstSeparator(t *testing.T) { t.Fatal(fmt.Sprintf("untouched session slid: %v != %v", seen, old)) } } + +// TestTouchSessionDigest_ScopesToBoundSession (SR-1-F3): the digest-scoped +// touch slides exactly the one session the lease recorded — a second live +// session of the same principal keeps its own last_seen_at, and a session +// already past the idle window is never revived. +func TestTouchSessionDigest_ScopesToBoundSession(t *testing.T) { + db := newDB(t) + ss := store.NewSessionStore(db, time.Hour, nil) + ctx := context.Background() + old := time.Now().UTC().Add(-10 * time.Minute).Truncate(time.Millisecond) + save := func(id string, lastSeen time.Time) { + t.Helper() + if err := ss.Save(ctx, &store.Session{ + ID: id, Issuer: "https://idp.test", Subject: "user-1", TenantID: "tenant-a", + CreatedAt: old, LastSeenAt: lastSeen, ExpiresAt: time.Now().Add(time.Hour), + }); err != nil { + t.Fatalf("save %s: %v", id, err) + } + } + save("s-bound", old) + save("s-sibling", old) + + digestOf := func(id string) string { d := sha256.Sum256([]byte(id)); return hex.EncodeToString(d[:]) } + digest := digestOf("s-bound") + n, err := ss.TouchSessionDigest(ctx, digest) + if err != nil || n != 1 { + t.Fatalf("touch bound session: n=%d err=%v, want 1", n, err) + } + var seen time.Time + if err := db.Pool().QueryRow(ctx, + `SELECT last_seen_at FROM sessions WHERE subject = 'user-1' AND last_seen_at <> $1`, old). + Scan(&seen); err != nil { + t.Fatalf("bound session not touched: %v", err) + } + var sibling time.Time + if err := db.Pool().QueryRow(ctx, + `SELECT last_seen_at FROM sessions WHERE id = $1`, + digestOf("s-sibling")).Scan(&sibling); err != nil { + t.Fatal(err) + } + if !sibling.Equal(old) { + t.Fatalf("sibling session slid: %v != %v", sibling, old) + } + + // An idle-dead session is never revived: age the bound row past the + // window, then touch — zero rows. + if _, err := db.Pool().Exec(ctx, + `UPDATE sessions SET last_seen_at = now() - interval '2 hours' + WHERE id = $1`, digest); err != nil { + t.Fatal(err) + } + if n, err = ss.TouchSessionDigest(ctx, digest); err != nil || n != 0 { + t.Fatalf("idle-dead session touched: n=%d err=%v, want 0", n, err) + } +} diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index 49d0a3bd..07eeeec4 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -56,6 +56,41 @@ export interface paths { patch?: never; trace?: never; }; + "/v1/session:touch": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** + * Activity beat + * @description The explicit "the user is here" signal (FIX-IDLE). Reaching this + * endpoint slides the session's idle deadline — the sliding session + * read inside authentication IS the touch — and the handler answers + * `204` with no body. It exists because every `GET` on this API is + * passive: the portal calls it only on real user interaction + * (pointer, key, navigation), never from a timer-driven poll, and + * throttles to at most one call per minute. + * + * Requires the session cookie and the `X-CSRF-Token` header (`401` + * without a session, `403` without a valid token) — forging a touch + * is therefore exactly as hard as forging any mutation. The call is + * rate-limited like the rest of the login/session family, keyed on + * the session digest when the cookie is valid. + * + * A session that already idled out cannot be revived by a touch: + * authentication fails `401` before the write, same as any route. + */ + post: operations["touchSession"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/v1/logout": { parameters: { query?: never; @@ -1606,6 +1641,29 @@ export interface operations { 503: components["responses"]["Unavailable"]; }; }; + touchSession: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Idle deadline extended. */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + 401: components["responses"]["Unauthorized"]; + 403: components["responses"]["Forbidden"]; + 429: components["responses"]["RateLimited"]; + 500: components["responses"]["InternalError"]; + 503: components["responses"]["Unavailable"]; + }; + }; logout: { parameters: { query?: never; diff --git a/web/src/app/shell.tsx b/web/src/app/shell.tsx index 100547ab..403c478e 100644 --- a/web/src/app/shell.tsx +++ b/web/src/app/shell.tsx @@ -20,6 +20,7 @@ import { useToast, } from "../design"; import { useApi } from "../api/context"; +import { useActivityTouch } from "../session/activity"; import { signOut, signOutEverywhere } from "../auth/signOut"; import { Link, navigate, NavLink, usePathname } from "./router"; import { @@ -223,6 +224,10 @@ function routeTitle(result: RouteResult): string | undefined { export function AppShell({ areas = ROUTE_AREAS }: { areas?: RouteArea[] }) { const pathname = usePathname(); + // Real user interaction keeps the portal session alive (FIX-IDLE): the + // read surface is passive, so pointer/key/route events send the + // throttled POST /v1/session:touch beat — never polls. + useActivityTouch(useApi()); const meState = useMe(); const branding = useBranding(); const { resolved: resolvedTheme } = useTheme(); diff --git a/web/src/session/activity.ts b/web/src/session/activity.ts new file mode 100644 index 00000000..53fb5932 --- /dev/null +++ b/web/src/session/activity.ts @@ -0,0 +1,55 @@ +import { useCallback, useEffect, useRef } from "react"; +import { usePathname } from "../app/router"; +import type { ApiClient } from "../api/client"; + +// FIX-IDLE: the portal's read surface is passive server-side — a polling +// loop can never extend the idle window — so real user interaction is what +// keeps a session alive. This hook turns interaction into an explicit +// authenticated beat: POST /v1/session:touch on pointer, key and route +// events, throttled to one call per SESSION_TOUCH_INTERVAL_MS. The beat is +// a real mutation (cookie + CSRF, same bar as any other write); timer +// polls and background fetches never send it. +export const SESSION_TOUCH_INTERVAL_MS = 60_000; + +/** + * Sends the activity beat at most once per interval. The beat is + * fire-and-forget — a failed call (401 mid-sign-out, transient 503) is + * ignored; the rest of the app surfaces expiry via its normal requests. + * State lives on refs so repeat mounts throttle independently. + */ +export function useActivityTouch(api: ApiClient): void { + const lastSentAt = useRef(-Infinity); + const inFlight = useRef(false); + const pathname = usePathname(); + const prevPath = useRef(pathname); + + const beat = useCallback(() => { + const now = Date.now(); + if (inFlight.current || now - lastSentAt.current < SESSION_TOUCH_INTERVAL_MS) return; + lastSentAt.current = now; + inFlight.current = true; + void api + .POST("/v1/session:touch", {}) + .catch(() => {}) + .finally(() => { + inFlight.current = false; + }); + }, [api]); + + useEffect(() => { + const opts = { passive: true }; + window.addEventListener("pointerdown", beat, opts); + window.addEventListener("keydown", beat, opts); + return () => { + window.removeEventListener("pointerdown", beat); + window.removeEventListener("keydown", beat); + }; + }, [beat]); + + // Navigating is interaction: a pathname change beats once per interval. + useEffect(() => { + if (prevPath.current === pathname) return; + prevPath.current = pathname; + beat(); + }, [pathname, beat]); +} diff --git a/web/tests/mock-api/auth.ts b/web/tests/mock-api/auth.ts index 39ac0b05..025d129f 100644 --- a/web/tests/mock-api/auth.ts +++ b/web/tests/mock-api/auth.ts @@ -60,6 +60,11 @@ export function authArea(_ctx: MockContext): MockArea { endSessionUrl = null; }, api: (req) => { + // POST /v1/session:touch (openapi.yaml touchSession): the activity + // beat — session cookie + CSRF were already enforced; the slide is + // implicit, the answer is empty. + if (req.path === "/v1/session:touch" && req.method === "POST") + return { status: 204, headers: {}, body: "" }; // POST /v1/me/sessions:revoke-all (openapi.yaml revokeAllSessions): // same answer shape as logout — the caller's session dies too. if (req.path === "/v1/me/sessions:revoke-all" && req.method === "POST") return logout(); diff --git a/web/tests/unit/session/activity.test.tsx b/web/tests/unit/session/activity.test.tsx new file mode 100644 index 00000000..8524966f --- /dev/null +++ b/web/tests/unit/session/activity.test.tsx @@ -0,0 +1,94 @@ +// FIX-IDLE — the portal's read surface is passive server-side, so the +// idle window only extends on real user interaction: POST +// /v1/session:touch is fired by pointer, key and route events (throttled +// to one beat per SESSION_TOUCH_INTERVAL_MS) and never by timer-driven +// polls or background fetches. +import { act, renderHook } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { createApi, setCsrfToken } from "../../../src/api/client"; +import { navigate } from "../../../src/lib/router"; +import { + SESSION_TOUCH_INTERVAL_MS, + useActivityTouch, +} from "../../../src/session/activity"; +import { useResource } from "../../../src/workspaces/resource"; + +const flush = () => act(async () => {}); +const advance = (ms: number) => + act(async () => { + await vi.advanceTimersByTimeAsync(ms); + }); +const fire = (type: string) => + act(() => { + window.dispatchEvent(new Event(type)); + }); + +function recordingClient(calls: string[]) { + const fetchImpl = (async (input: RequestInfo | URL) => { + const req = new Request(input); + calls.push(`${req.method} ${new URL(req.url).pathname}`); + return new Response(null, { status: 204 }); + }) as typeof fetch; + return createApi(fetchImpl); +} + +const touches = (calls: string[]) => calls.filter((c) => c === "POST /v1/session:touch"); + +beforeEach(() => { + vi.useFakeTimers(); + setCsrfToken("csrf-test"); + window.history.pushState(null, "", "/"); +}); +afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +describe("useActivityTouch (FIX-IDLE)", () => { + it("timer polls never send the beat", async () => { + const calls: string[] = []; + // The beat hook and a generic interval poll live in separate roots: + // combining useSyncExternalStore (usePathname) with useResource's + // timers under fake timers deadlocks the test runner — the poll is + // exercised by the same client, which is what matters. + renderHook(() => useActivityTouch(recordingClient(calls))); + const load = vi.fn(() => Promise.resolve("x")); + renderHook(() => useResource(load, 1_000)); + await flush(); + + await advance(5_000); // five poll ticks — none may touch + expect(load.mock.calls.length).toBeGreaterThan(1); + expect(touches(calls)).toHaveLength(0); + }); + + it("pointer and key events beat, throttled to the interval", async () => { + const calls: string[] = []; + renderHook(() => useActivityTouch(recordingClient(calls))); + await flush(); + + fire("pointerdown"); + fire("keydown"); // collapses into the same window + await flush(); + expect(touches(calls)).toHaveLength(1); + + await advance(SESSION_TOUCH_INTERVAL_MS - 1_000); + fire("pointerdown"); + await flush(); + expect(touches(calls)).toHaveLength(1); + + await advance(1_001); + fire("keydown"); + await flush(); + expect(touches(calls)).toHaveLength(2); + }); + + it("a route change beats", async () => { + const calls: string[] = []; + renderHook(() => useActivityTouch(recordingClient(calls))); + await flush(); + + act(() => navigate("/workspaces")); + await flush(); + expect(touches(calls)).toHaveLength(1); + }); +});