From 298e8e3cd772e93a62738a4950038d214f6eaf4c Mon Sep 17 00:00:00 2001 From: Nguyen Van Long Date: Tue, 6 Oct 2026 12:17:37 +0700 Subject: [PATCH 1/5] fix(api,web): poll marker so background polling cannot slide idle (FIX-IDLE) Interval polls on RequireAuth routes (workspaces list/detail/events, data list/detail, the session page's "starting" poll, /v1/me backoff retries) used to keep a visible-but-unattended portal tab's session alive forever. The SPA now marks timer-driven reads with X-TCDI-Poll: background and requireAuth reads marked requests with Peek instead of Get; navigation, manual refresh, return-to-visible reloads and mutations still slide. The marker is honoured on every authenticated route and can only withhold an idle slide, never earn one. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- docs/security/threat-model.md | 27 ++++++- internal/api/middleware.go | 17 +++- internal/api/middleware_test.go | 89 +++++++++++++++++++++ internal/api/openapi.yaml | 13 +++ web/src/api/client.ts | 9 +++ web/src/app/me.tsx | 41 +++++++--- web/src/auth/AuthGate.tsx | 9 ++- web/src/data/DataDetailPage.tsx | 5 +- web/src/data/DataListPage.tsx | 5 +- web/src/data/api.ts | 8 +- web/src/session/SessionPage.tsx | 9 ++- web/src/workspaces/WorkspaceDetailPage.tsx | 29 ++++--- web/src/workspaces/WorkspaceListPage.tsx | 2 +- web/src/workspaces/api.ts | 18 ++++- web/src/workspaces/resource.ts | 66 ++++++++------- web/tests/unit/api/client.test.ts | 40 ++++++++- web/tests/unit/workspaces/resource.test.tsx | 42 ++++++++++ 17 files changed, 361 insertions(+), 68 deletions(-) diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 0763c69b..974d3888 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -186,8 +186,13 @@ Controls: `ParseTrustedProxies`). - **Passive auth** — `GET /v1/connections/.../status` authenticates via `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`, `internal/api/connection_status.go`) — + A6-S6's second authenticated path exists to be reviewed. The portal's + interval polls additionally carry `X-TCDI-Poll: background`, which makes + `requireAuth` read the session with `Peek` on any route: a + visible-but-unattended tab cannot hold a session open, and the marker + can only withhold an idle slide so a forged one gains nothing + (`TestRequireAuth_BackgroundPollMarker`). - **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`; @@ -545,6 +550,24 @@ Additional items found while writing this document (not from A6): - **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 background polling vs the idle window** — Implemented: before + this fix the SPA's interval polls of `GET /v1/workspaces`, + `/v1/workspaces/{id}` and `/v1/workspaces/{id}/events` (all mounted + behind sliding `RequireAuth`) kept a visible-but-unattended tab's + session alive forever. The SPA now marks timer-driven reads with + `X-TCDI-Poll: background` (useResource ticks, the session page's + "starting" poll, `/v1/me` backoff retries) and `requireAuth` peeks + instead of sliding on marked requests; navigation, user-triggered + refresh, return-to-visible reloads and mutations still slide. Desktop + streams keep the window open only through server-measured RFB input + (`InputHook` → `TouchPrincipal`, throttle 1/min per principal), so an + active desktop user is not signed out mid-work while an open-but-idle + stream is not portal activity. Residual, unchanged here: lease + redeem/renew/rehydrate never consult the portal idle window — a lease + outlives idle expiry by design and dies on its own TTL, on revoke, or + on the bound session's absolute expiry (S17), so tearing an open stream + down at portal-idle expiry remains a separate decision, not covered by + this fix. - **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/middleware.go b/internal/api/middleware.go index a4681d43..5a9b0168 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -72,10 +72,23 @@ func RequestIDFromContext(ctx context.Context) string { // Authentication // --------------------------------------------------------------------------- +// PollHeader marks a request as portal background polling — automated +// traffic, not user activity. When it carries PollHeaderValue the session +// is read with Peek instead of Get, so the request authenticates normally +// but cannot slide the idle deadline. It is honoured on every +// authenticated route and any method: the marker can only withhold an +// idle slide, never earn one, so a forged marker gains nothing (P4, D18). +const PollHeader = "X-TCDI-Poll" + +// PollHeaderValue is the marker value the portal sends on its interval +// polls; any other value leaves the request a normal activity touch. +const PollHeaderValue = "background" + // RequireAuth rejects requests without a valid server-side session (opaque // host-only cookie) and attaches the verified Principal and Session to the // request context. Handlers must derive owner/tenant from that principal. -// Each authenticated request slides the session's idle deadline (Get). +// Each authenticated request slides the session's idle deadline (Get) +// unless the client marked it background polling (PollHeader). func (a *Authenticator) RequireAuth(next http.Handler) http.Handler { return a.requireAuth(next, true) } @@ -95,7 +108,7 @@ func (a *Authenticator) requireAuth(next http.Handler, slide bool) http.Handler return } var sess *Session - if slide { + if slide && r.Header.Get(PollHeader) != PollHeaderValue { sess, err = a.sessions.Get(r.Context(), c.Value) } else { sess, err = a.sessions.Peek(r.Context(), c.Value) diff --git a/internal/api/middleware_test.go b/internal/api/middleware_test.go index fd07e319..bb28964f 100644 --- a/internal/api/middleware_test.go +++ b/internal/api/middleware_test.go @@ -10,6 +10,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" @@ -433,3 +434,91 @@ func TestRequireAuth_SessionStoreErrors(t *testing.T) { }) } } + +// TestRequireAuth_BackgroundPollMarker (FIX-IDLE): a request carrying +// X-TCDI-Poll: background authenticates but never slides the idle window, +// so the portal's interval polls cannot keep a visible-but-unattended +// session alive. An unmarked request on the same RequireAuth route still +// slides; once the window lapses both shapes answer 401. +func TestRequireAuth_BackgroundPollMarker(t *testing.T) { + fc := &fakeClock{now: time.Now()} + store := NewInMemorySessionStore(time.Minute).WithClock(fc.Now) + auth := &Authenticator{ + cfg: &AuthConfig{SessionCookieName: "__Host-tcdi_session"}, + sessions: store, + now: fc.Now, + } + save := func(id string) { + if err := store.Save(context.Background(), &Session{ + ID: id, + Principal: Principal{Issuer: "iss", Subject: "sub", TenantID: "tenant-a"}, + CreatedAt: fc.Now(), + LastSeenAt: fc.Now(), + ExpiresAt: fc.Now().Add(time.Hour), + }); err != nil { + t.Fatal(err) + } + } + handler := auth.RequireAuth(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + })) + do := func(id string, marked bool) int { + req := httptest.NewRequest(http.MethodGet, "/v1/workspaces", nil) + req.AddCookie(&http.Cookie{Name: "__Host-tcdi_session", Value: id}) + if marked { + req.Header.Set(PollHeader, PollHeaderValue) + } + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + return rec.Code + } + + // Marked polls authenticate but never slide: created at t=0, the poll + // at +50s succeeds yet the window still lapses at +60s. + save("sess-poll") + fc.Advance(50 * time.Second) + if code := do("sess-poll", true); code != http.StatusNoContent { + t.Fatalf("marked poll inside the window rejected: %d", code) + } + fc.Advance(15 * time.Second) // t=65s — past idle despite the +50s poll + if code := do("sess-poll", true); code != http.StatusUnauthorized { + t.Fatalf("marked poll past idle: %d, want 401", code) + } + + // The marker is opt-out only: a wrong value is ordinary activity. + save("sess-other-value") + req := httptest.NewRequest(http.MethodGet, "/v1/workspaces", nil) + req.AddCookie(&http.Cookie{Name: "__Host-tcdi_session", Value: "sess-other-value"}) + req.Header.Set(PollHeader, "1") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusNoContent { + t.Fatalf("other marker value rejected: %d", rec.Code) + } + sess, err := store.Get(context.Background(), "sess-other-value") + if err != nil { + t.Fatal(err) + } + if !sess.LastSeenAt.Equal(fc.Now()) { + t.Fatalf("unrecognized marker value did not slide: LastSeenAt=%v now=%v", sess.LastSeenAt, fc.Now()) + } + + // Without the marker the same cadence slides the window as before. + save("sess-active") + fc.Advance(50 * time.Second) + if code := do("sess-active", false); code != http.StatusNoContent { + t.Fatalf("unmarked request rejected: %d", code) + } + fc.Advance(50 * time.Second) // 50s since the slide — still inside + if code := do("sess-active", false); code != http.StatusNoContent { + t.Fatalf("unmarked request past one window: %d", code) + } + + // Idle expiry answers 401 to marked and unmarked requests alike. + fc.Advance(61 * time.Second) + for _, marked := range []bool{false, true} { + if code := do("sess-active", marked); code != http.StatusUnauthorized { + t.Fatalf("expired session marked=%v: %d, want 401", marked, code) + } + } +} diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 3c7fbafe..84518be1 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -27,6 +27,19 @@ info: transactional outbox (see ADR 0002); Kubernetes RBAC denies direct CR writes by end users. + ## Session idle timeout and background polling + + Sessions carry a sliding idle timeout: an authenticated request normally + slides the idle deadline, while a request carrying the header + `X-TCDI-Poll: background` is authenticated identically but does **not** + slide it. The portal sends the marker on its interval polls so a + visible-but-unattended tab cannot hold a session open; interactive + desktop input slides the same window through a server-measured signal, + not through this API. The marker is honoured on every authenticated + endpoint — it can only withhold an idle slide, never earn one, so a + client gains nothing by sending it. `GET /v1/session` and + `GET /v1/workspaces/{workspaceId}/connection` are always passive. + ## Idempotency Mutating operations that allocate resources (`POST /v1/workspaces`, diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 60623676..93bb8402 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -10,6 +10,15 @@ export type ApiClient = Client; // export reads document.cookie; the tcdi_csrf cookie is gone in v0.2. export const CSRF_HEADER = "X-CSRF-Token"; +// Background-poll marker (D18): reads issued by the portal's polling loops +// carry `X-TCDI-Poll: background`; the server then authenticates them with +// a peek at the session instead of a sliding read, so a visible but +// unattended tab cannot hold the idle window open. The marker can only +// withhold an idle slide, never earn one — there is nothing to spoof. +// Navigation, user-triggered reloads and mutations send nothing. +export const POLL_HEADER = "X-TCDI-Poll"; +export const POLL_HEADERS: Record = { [POLL_HEADER]: "background" }; + let csrfToken: string | undefined; export function setCsrfToken(token: string | undefined): void { diff --git a/web/src/app/me.tsx b/web/src/app/me.tsx index 002310ae..9f4a951e 100644 --- a/web/src/app/me.tsx +++ b/web/src/app/me.tsx @@ -1,5 +1,5 @@ import { createContext, useCallback, useContext, useEffect, useRef, useState, type ReactNode } from "react"; -import { setCsrfToken, unwrap, type ApiClient } from "../api/client"; +import { POLL_HEADERS, setCsrfToken, unwrap, type ApiClient } from "../api/client"; import { useApi } from "../api/context"; import type { components } from "../api/generated/schema"; @@ -77,8 +77,13 @@ export function isTransientMeError(e: unknown): boolean { return e instanceof TypeError; } -export async function fetchMe(fetchImpl: typeof fetch = fetch): Promise { - const res = await fetchImpl("/v1/me", { credentials: "same-origin", headers: { Accept: "application/json" } }); +// background marks a retry-loop attempt as automated traffic: the request +// authenticates without sliding the portal idle window (POLL_HEADERS). +export async function fetchMe(background = false, fetchImpl: typeof fetch = fetch): Promise { + const res = await fetchImpl("/v1/me", { + credentials: "same-origin", + headers: { Accept: "application/json", ...(background ? POLL_HEADERS : {}) }, + }); if (res.status === 404 || res.status === 501) return STUB_ME; if (!res.ok) throw new MeHttpError(res.status); return parseMe(await res.json()); @@ -95,8 +100,8 @@ export function MeProvider({ load = fetchMe, }: { children: ReactNode; - /** Injectable for tests. */ - load?: () => Promise; + /** Injectable for tests; `background` marks a backoff retry. */ + load?: (background: boolean) => Promise; }) { const [state, setState] = useState({ status: "loading", me: null }); useEffect(() => { @@ -105,7 +110,10 @@ export function MeProvider({ let failures = 0; const attempt = async () => { try { - const me = await load(); + // The first attempt is part of the page load — real navigation; + // backoff retries are background and must not slide the idle + // window while the shell sits on an unreachable API (D18). + const me = await load(failures > 0); if (cancelled) return; // The API client reads the token from module state, not from a // cookie (D17) — install what /v1/me published. @@ -230,24 +238,37 @@ type Query = Record; interface LooseGet { GET( path: string, - init: { params?: { query?: Query } }, + init: { params?: { query?: Query }; headers?: Record }, ): Promise<{ data?: unknown; error?: unknown; response: Response }>; } -export async function get(api: ApiClient, path: string, query?: Query): Promise { +export async function get( + api: ApiClient, + path: string, + query?: Query, + background = false, +): Promise { const loose = api as unknown as LooseGet; - return unwrap(await loose.GET(path, query ? { params: { query } } : {})) as T; + return unwrap( + await loose.GET(path, { + ...(query ? { params: { query } } : {}), + // background marks an automated poll: the server authenticates the + // request without sliding the portal idle window (D18). + ...(background ? { headers: POLL_HEADERS } : {}), + }), + ) as T; } export async function listAll( api: ApiClient, path: string, query: Query, + background = false, ): Promise<{ items: T[]; truncated: boolean }> { const items: T[] = []; let pageToken: string | undefined; for (let i = 0; i < MAX_PAGES; i++) { - const page = await get>(api, path, { ...query, limit: PAGE_LIMIT, pageToken }); + const page = await get>(api, path, { ...query, limit: PAGE_LIMIT, pageToken }, background); items.push(...page.items); if (!page.nextPageToken) return { items, truncated: false }; pageToken = page.nextPageToken; diff --git a/web/src/auth/AuthGate.tsx b/web/src/auth/AuthGate.tsx index 994dcf3b..996e1525 100644 --- a/web/src/auth/AuthGate.tsx +++ b/web/src/auth/AuthGate.tsx @@ -1,7 +1,7 @@ import { useEffect, useState, type ReactNode } from "react"; import { t } from "../i18n"; import { useApi } from "../api/context"; -import { unwrap } from "../api/client"; +import { POLL_HEADERS, unwrap } from "../api/client"; import { isPortalApiError } from "../api/errors"; // Login lives on the portal origin ahead of the API (OIDC-backed session @@ -49,8 +49,11 @@ export function AuthGate({ if (!cancelled) onUnauthenticated(); return; } - // MeProvider reads the body again once the gate opens. - unwrap(await api.GET("/v1/me")); + // MeProvider reads the body again once the gate opens. Retries + // after the first failure are background traffic — they carry the + // poll marker so a gate stuck retrying cannot hold the idle window + // open (D18); the first attempt rides on the page load itself. + unwrap(await api.GET("/v1/me", failures > 0 ? { headers: POLL_HEADERS } : {})); if (!cancelled) setReady(true); } catch (e) { if (cancelled) return; diff --git a/web/src/data/DataDetailPage.tsx b/web/src/data/DataDetailPage.tsx index e4b53646..5b487c81 100644 --- a/web/src/data/DataDetailPage.tsx +++ b/web/src/data/DataDetailPage.tsx @@ -33,7 +33,10 @@ export function DataDetailPage({ const api = useApi(); const me = useMeLoaded(); const admin = isTenantAdmin(me.data); - const load = useCallback(() => getRetainedData(api, dataId), [api, dataId]); + const load = useCallback( + (background: boolean) => getRetainedData(api, dataId, background), + [api, dataId], + ); // Poll while the record is Purging (same cadence as the list); a Purged // record either stops being returned (404 -> null) or reports Purged — // either way the poll stops. diff --git a/web/src/data/DataListPage.tsx b/web/src/data/DataListPage.tsx index 7808062c..5c314423 100644 --- a/web/src/data/DataListPage.tsx +++ b/web/src/data/DataListPage.tsx @@ -89,7 +89,10 @@ export function DataListPage({ pollIntervalMs }: { pollIntervalMs?: number }) { const admin = isTenantAdmin(me.data); const [scope, setScope] = useState("mine"); const effectiveScope: Scope = admin ? scope : "mine"; - const load = useCallback(() => listRetainedData(api, effectiveScope), [api, effectiveScope]); + const load = useCallback( + (background: boolean) => listRetainedData(api, effectiveScope, background), + [api, effectiveScope], + ); const list = useResource(load, (data) => purgePollDelay(data, pollIntervalMs)); const [attaching, setAttaching] = useState(null); diff --git a/web/src/data/api.ts b/web/src/data/api.ts index 5971360c..eeba2e37 100644 --- a/web/src/data/api.ts +++ b/web/src/data/api.ts @@ -24,11 +24,14 @@ export interface AttachDataBody { // GET /v1/data. `Purged` records are excluded server-side; the filter here // keeps the list honest if an old backend still returns them. +// `background` marks a scheduled poll tick (useResource) — the request +// authenticates without sliding the portal idle window (D18). export async function listRetainedData( api: ApiClient, scope: Scope, + background = false, ): Promise<{ items: ScopedRetainedData[]; truncated: boolean }> { - const page = await listAll(api, "/v1/data", { scope }); + const page = await listAll(api, "/v1/data", { scope }, background); return { ...page, items: page.items.filter((r) => r.state !== "Purged") }; } @@ -38,9 +41,10 @@ export async function listRetainedData( export async function getRetainedData( api: ApiClient, id: string, + background = false, ): Promise { try { - return await get(api, `/v1/data/${encodeURIComponent(id)}`); + return await get(api, `/v1/data/${encodeURIComponent(id)}`, undefined, background); } catch (e) { if (isPortalApiError(e) && e.httpStatus === 404) return null; throw e; diff --git a/web/src/session/SessionPage.tsx b/web/src/session/SessionPage.tsx index 0bdb495e..d15f9cbc 100644 --- a/web/src/session/SessionPage.tsx +++ b/web/src/session/SessionPage.tsx @@ -3,7 +3,7 @@ import { t, formatTime } from "../i18n"; import { formatDuration } from "../templates/format"; import { phaseLabelKey } from "../workspaces/helpers"; import { useApi } from "../api/context"; -import { newIdempotencyKey, unwrap } from "../api/client"; +import { newIdempotencyKey, POLL_HEADERS, unwrap } from "../api/client"; import { isPortalApiError } from "../api/errors"; import { LifecycleProgress } from "../progress/LifecycleProgress"; import { opPollMs, startInFlight, withJitter } from "../progress/derive"; @@ -678,8 +678,13 @@ export function SessionPage({ let delay = 1_000; const tick = async () => { try { + // Timer-driven poll, not user activity: the marker keeps it from + // sliding the portal idle window (D18). const ws = unwrap( - await api.GET("/v1/workspaces/{workspaceId}", { params: { path: { workspaceId } } }), + await api.GET("/v1/workspaces/{workspaceId}", { + params: { path: { workspaceId } }, + headers: POLL_HEADERS, + }), ); if (cancelled || !mounted.current) return; failures = 0; diff --git a/web/src/workspaces/WorkspaceDetailPage.tsx b/web/src/workspaces/WorkspaceDetailPage.tsx index 2e84a0a8..63ac0a51 100644 --- a/web/src/workspaces/WorkspaceDetailPage.tsx +++ b/web/src/workspaces/WorkspaceDetailPage.tsx @@ -145,19 +145,22 @@ export function WorkspaceDetailPage({ const [busy, setBusy] = useState(null); const [actionError, setActionError] = useState(null); - const load = useCallback(async (): Promise => { - const [workspace, events] = await Promise.all([ - getWorkspace(api, workspaceId), - listWorkspaceEvents(api, workspaceId), - ]); - // The retained disk this workspace mounts: named so the user can tell - // where its home came from (T5.4). Best effort — the record may be - // gone or not visible to this caller. - const retained = workspace.retainedDataRef - ? await getRetainedData(api, workspace.retainedDataRef).catch(() => null) - : null; - return { workspace, events, retained }; - }, [api, workspaceId]); + const load = useCallback( + async (background: boolean): Promise => { + const [workspace, events] = await Promise.all([ + getWorkspace(api, workspaceId, background), + listWorkspaceEvents(api, workspaceId, background), + ]); + // The retained disk this workspace mounts: named so the user can tell + // where its home came from (T5.4). Best effort — the record may be + // gone or not visible to this caller. + const retained = workspace.retainedDataRef + ? await getRetainedData(api, workspace.retainedDataRef, background).catch(() => null) + : null; + return { workspace, events, retained }; + }, + [api, workspaceId], + ); const detail = useResource(load, pollIntervalMs ?? pollDelay); const { toast } = useToast(); // A delete "completes" when the API drops the row (FX-R19 hides finalised diff --git a/web/src/workspaces/WorkspaceListPage.tsx b/web/src/workspaces/WorkspaceListPage.tsx index 36fec877..3d983dac 100644 --- a/web/src/workspaces/WorkspaceListPage.tsx +++ b/web/src/workspaces/WorkspaceListPage.tsx @@ -79,7 +79,7 @@ const COLUMNS: Column[] = [ export function WorkspaceListPage({ pollIntervalMs }: { pollIntervalMs?: number }) { const api = useApi(); - const load = useCallback(() => listWorkspaces(api), [api]); + const load = useCallback((background: boolean) => listWorkspaces(api, background), [api]); const list = useResource(load, pollIntervalMs ?? pollDelay); const [announcement, setAnnouncement] = useState(null); diff --git a/web/src/workspaces/api.ts b/web/src/workspaces/api.ts index f8e145d0..6f2d9f4d 100644 --- a/web/src/workspaces/api.ts +++ b/web/src/workspaces/api.ts @@ -1,4 +1,4 @@ -import { unwrap, type ApiClient } from "../api/client"; +import { POLL_HEADERS, unwrap, type ApiClient } from "../api/client"; import { noteServerDateHeader } from "../progress/derive"; import type { WorkspaceEvent, WorkspaceView } from "./helpers"; @@ -6,16 +6,21 @@ import type { WorkspaceEvent, WorkspaceView } from "./helpers"; // /v1/workspaces?scope=…, GET /v1/workspaces/{id}/events). Each response's // Date header feeds the progress module's skew correction — the elapsed // counters anchor on server-side updated_at, not the client clock. +// +// `background` marks a read as an automated poll (POLL_HEADERS): the +// server authenticates it without sliding the portal idle window. Only +// useResource's scheduled ticks pass true. const MAX_PAGES = 10; /** All of the caller's workspaces (`scope=mine`; the tenant view is admin's). */ -export async function listWorkspaces(api: ApiClient): Promise { +export async function listWorkspaces(api: ApiClient, background = false): Promise { const items: WorkspaceView[] = []; let pageToken: string | undefined; for (let i = 0; i < MAX_PAGES; i++) { const res = await api.GET("/v1/workspaces", { params: { query: { scope: "mine", limit: 200, ...(pageToken ? { pageToken } : {}) } }, + ...(background ? { headers: POLL_HEADERS } : {}), }); noteServerDateHeader(res.response.headers.get("date")); const page = unwrap(res); @@ -26,9 +31,14 @@ export async function listWorkspaces(api: ApiClient): Promise { return items; } -export async function getWorkspace(api: ApiClient, workspaceId: string): Promise { +export async function getWorkspace( + api: ApiClient, + workspaceId: string, + background = false, +): Promise { const res = await api.GET("/v1/workspaces/{workspaceId}", { params: { path: { workspaceId } }, + ...(background ? { headers: POLL_HEADERS } : {}), }); noteServerDateHeader(res.response.headers.get("date")); return unwrap(res); @@ -38,10 +48,12 @@ export async function getWorkspace(api: ApiClient, workspaceId: string): Promise export async function listWorkspaceEvents( api: ApiClient, workspaceId: string, + background = false, ): Promise { const res = unwrap( await api.GET("/v1/workspaces/{workspaceId}/events", { params: { path: { workspaceId } }, + ...(background ? { headers: POLL_HEADERS } : {}), }), ); return res.items ?? []; diff --git a/web/src/workspaces/resource.ts b/web/src/workspaces/resource.ts index e58e6d39..d96defd4 100644 --- a/web/src/workspaces/resource.ts +++ b/web/src/workspaces/resource.ts @@ -24,7 +24,14 @@ export type PollInterval = number | null | ((data: T | undefined) => number | const MAX_BACKOFF_MS = 30_000; -export function useResource(load: () => Promise, interval: PollInterval = null): Resource { +// The `background` flag tells the loader whether the call is a scheduled +// poll tick (true → the request carries POLL_HEADERS and cannot slide the +// portal idle window) or user-facing activity (false → mount loads, +// manual refresh, return-to-visible reloads all slide normally). +export function useResource( + load: (background: boolean) => Promise, + interval: PollInterval = null, +): Resource { const [data, setData] = useState(undefined); const [error, setError] = useState(null); const [loading, setLoading] = useState(true); @@ -43,26 +50,29 @@ export function useResource(load: () => Promise, interval: PollInterval // poll is untouched, so always-on consumers are unchanged. const scheduleRef = useRef<() => void>(() => {}); - const run = useCallback(async () => { - const started = epoch.current; - try { - const d = await load(); - if (started !== epoch.current) return; - dataRef.current = d; - failures.current = 0; - retryAfterMs.current = null; - setData(d); - setError(null); - } catch (e) { - if (started !== epoch.current) return; - failures.current += 1; - const ra = (e as { retryAfterMs?: unknown }).retryAfterMs; - retryAfterMs.current = typeof ra === "number" && Number.isFinite(ra) ? ra : null; - setError(e); - } finally { - if (started === epoch.current) setLoading(false); - } - }, [load]); + const run = useCallback( + async (background: boolean) => { + const started = epoch.current; + try { + const d = await load(background); + if (started !== epoch.current) return; + dataRef.current = d; + failures.current = 0; + retryAfterMs.current = null; + setData(d); + setError(null); + } catch (e) { + if (started !== epoch.current) return; + failures.current += 1; + const ra = (e as { retryAfterMs?: unknown }).retryAfterMs; + retryAfterMs.current = typeof ra === "number" && Number.isFinite(ra) ? ra : null; + setError(e); + } finally { + if (started === epoch.current) setLoading(false); + } + }, + [load], + ); useEffect(() => { let timer: ReturnType | undefined; @@ -88,15 +98,17 @@ export function useResource(load: () => Promise, interval: PollInterval armed = false; const delay = nextDelay(); if (delay === null || document.visibilityState === "hidden") return; - timer = setTimeout(() => void tick(), delay); + timer = setTimeout(() => void tick(true), delay); armed = true; }; - const tick = async () => { - await run(); + const tick = async (background: boolean) => { + await run(background); schedule(); }; const onVisible = () => { - if (document.visibilityState === "visible") void tick(); + // Returning to a hidden tab is the user showing up — the reload + // counts as activity; only the timer-driven ticks are background. + if (document.visibilityState === "visible") void tick(false); else { clearTimeout(timer); armed = false; @@ -105,7 +117,7 @@ export function useResource(load: () => Promise, interval: PollInterval scheduleRef.current = () => { if (!armed) schedule(); }; - void tick(); + void tick(false); document.addEventListener("visibilitychange", onVisible); return () => { stopped = true; @@ -126,7 +138,7 @@ export function useResource(load: () => Promise, interval: PollInterval const refresh = useCallback(() => { epoch.current += 1; - return run().then(() => scheduleRef.current()); + return run(false).then(() => scheduleRef.current()); }, [run]); const clearError = useCallback(() => setError(null), []); diff --git a/web/tests/unit/api/client.test.ts b/web/tests/unit/api/client.test.ts index 520a7983..0d234a48 100644 --- a/web/tests/unit/api/client.test.ts +++ b/web/tests/unit/api/client.test.ts @@ -1,5 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { createApi, setCsrfToken, CSRF_HEADER, unwrap } from "../../../src/api/client"; +import { createApi, setCsrfToken, CSRF_HEADER, POLL_HEADER, POLL_HEADERS, unwrap } from "../../../src/api/client"; +import { fetchMe } from "../../../src/app/me"; +import { getWorkspace, listWorkspaces } from "../../../src/workspaces/api"; // The CSRF token arrives in the GET /v1/me body (P1/D17) and lives only in // module state — setCsrfToken installs it, the client echoes it on mutations, @@ -121,3 +123,39 @@ describe("api client CSRF", () => { expect(posts).toHaveLength(2); }); }); + +// FIX-IDLE — background polls carry X-TCDI-Poll: background so the server +// authenticates them without sliding the portal idle window; foreground +// reads send nothing. +describe("background-poll marker", () => { + it("sends X-TCDI-Poll only on marked reads", async () => { + const { calls, fetchImpl } = recordingFetch(() => json(200, { items: [] })); + const client = createApi(fetchImpl); + + await listWorkspaces(client, true); + await listWorkspaces(client); + await getWorkspace(client, WS, true); + + const [markedList, plainList, markedGet] = calls; + expect(markedList.headers.get(POLL_HEADER)).toBe(POLL_HEADERS[POLL_HEADER]); + expect(plainList.headers.get(POLL_HEADER)).toBeNull(); + expect(markedGet.headers.get(POLL_HEADER)).toBe(POLL_HEADERS[POLL_HEADER]); + }); + + it("fetchMe marks retry attempts", async () => { + const calls: Request[] = []; + // fetchMe hits the relative /v1/me, so give the Request a base URL. + const fetchImpl = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? new URL(input, "https://portal.test") : input; + const req = new Request(url, init); + calls.push(req); + return json(200, ME_BODY); + }) as typeof fetch; + + await fetchMe(true, fetchImpl); + await fetchMe(false, fetchImpl); + + expect(calls[0].headers.get(POLL_HEADER)).toBe(POLL_HEADERS[POLL_HEADER]); + expect(calls[1].headers.get(POLL_HEADER)).toBeNull(); + }); +}); diff --git a/web/tests/unit/workspaces/resource.test.tsx b/web/tests/unit/workspaces/resource.test.tsx index 93741627..fe008b66 100644 --- a/web/tests/unit/workspaces/resource.test.tsx +++ b/web/tests/unit/workspaces/resource.test.tsx @@ -79,3 +79,45 @@ describe("useResource refresh re-arm (FX-R29)", () => { expect(load).toHaveBeenCalledTimes(5); }); }); + +// FIX-IDLE — the loader's `background` flag marks which reads may slide +// the portal idle window: scheduled ticks are background (the server +// peeks instead of touching last_seen_at) while mount loads, manual +// refresh and the return-to-visible reload are real activity. +describe("useResource background-poll marker", () => { + it("marks timer ticks background; mount and refresh stay foreground", async () => { + const load = vi.fn((_background: boolean) => Promise.resolve("x")); + const { result } = renderHook(() => useResource(load, 1_000)); + await flush(); + expect(load).toHaveBeenLastCalledWith(false); + + await advance(1_000); + expect(load).toHaveBeenLastCalledWith(true); + await advance(1_000); + expect(load).toHaveBeenLastCalledWith(true); + + await act(async () => { + await result.current.refresh(); + }); + expect(load).toHaveBeenLastCalledWith(false); + }); + + it("treats the return-to-visible reload as activity", async () => { + const visibility = vi + .spyOn(Document.prototype, "visibilityState", "get") + .mockReturnValue("hidden"); + const load = vi.fn((_background: boolean) => Promise.resolve("x")); + renderHook(() => useResource(load, 1_000)); + await flush(); + expect(load).toHaveBeenLastCalledWith(false); + // Hidden: the timer never fires. + await advance(5_000); + expect(load).toHaveBeenCalledTimes(1); + + visibility.mockReturnValue("visible"); + await act(async () => { + document.dispatchEvent(new Event("visibilitychange")); + }); + expect(load).toHaveBeenLastCalledWith(false); + }); +}); From c5745e6cae4db4fe4717f2a507c8b950b90625c6 Mon Sep 17 00:00:00 2001 From: Nguyen Van Long Date: Tue, 6 Oct 2026 12:20:21 +0700 Subject: [PATCH 2/5] test(api): poll-marker regression uses wire literals (FIX-IDLE) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Literal "X-TCDI-Poll: background" instead of the new constants keeps the regression test compiling on v0.5.0, where it then fails at the "marked poll past idle" step (204, want 401) — proving the pre-fix slide. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- internal/api/middleware_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/api/middleware_test.go b/internal/api/middleware_test.go index bb28964f..bb3fedc1 100644 --- a/internal/api/middleware_test.go +++ b/internal/api/middleware_test.go @@ -466,7 +466,10 @@ func TestRequireAuth_BackgroundPollMarker(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/v1/workspaces", nil) req.AddCookie(&http.Cookie{Name: "__Host-tcdi_session", Value: id}) if marked { - req.Header.Set(PollHeader, PollHeaderValue) + // Literal wire values, not PollHeader/PollHeaderValue, so this + // file still compiles on a tree without the marker — where the + // "poll past idle" step then fails, proving the regression. + req.Header.Set("X-TCDI-Poll", "background") } rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) @@ -489,7 +492,7 @@ func TestRequireAuth_BackgroundPollMarker(t *testing.T) { save("sess-other-value") req := httptest.NewRequest(http.MethodGet, "/v1/workspaces", nil) req.AddCookie(&http.Cookie{Name: "__Host-tcdi_session", Value: "sess-other-value"}) - req.Header.Set(PollHeader, "1") + req.Header.Set("X-TCDI-Poll", "1") rec := httptest.NewRecorder() handler.ServeHTTP(rec, req) if rec.Code != http.StatusNoContent { From e33ba5d40922e5ae909d6e83c136c3919c1fff58 Mon Sep 17 00:00:00 2001 From: Nguyen Van Long Date: Tue, 6 Oct 2026 12:58:06 +0700 Subject: [PATCH 3/5] fix(api,broker,store): reads are passive; leases honour the idle window (FIX-IDLE) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Advisor-steered redesign (supersedes the client marker at c5745e6): the server decides what is activity — every cookie-authenticated GET mounts RequireAuthPassive, so the portal's interval polls can no longer hold a visible-but-unattended session open; only mutations and server-measured desktop input slide the idle window. The SPA needs no marker. SR-1-F2: the lease layer now consults the same liveness rule as RequireAuth — WithSessionIdle plumbs cfg.SessionIdle into the broker and the shared portalSessionLiveSQL predicate (epoch + absolute expiry + last_seen_at inside the window) is applied by loadLease's CASE and RedeemTicket's session re-check, so redeem, renew and rehydrate all fail closed on an idled-out session and the lease is revoked on the spot. Renew deliberately only consults, never slides: a sliding renew would let a connected-but-idle stream pin the session open — the same bug through the desktop path. SR-1-F3: desktop input now credits exactly the session the stream's lease was minted under — loadLease selects the bound portal_session_digest and the input hook carries it to the new digest-scoped store method TouchSessionDigest, instead of sliding every session of the principal. A NULL-digest legacy lease keeps the principal-wide fallback so pre-upgrade streams survive a rolling deploy. Tests: idlepoll_test.go pins that polled GETs never slide (fails on the old sliding mounts) while mutations still do; lease_session_test gains idle-dead renew/redeem/rehydrate cases on real Postgres; me_test pins digest-scoped input vs a same-principal sibling; session_touch_test pins the pkey index and the scoping/no-revival behaviour. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- docs/security/threat-model.md | 54 +++++----- internal/api/adminquota.go | 6 +- internal/api/adminuserlimit.go | 4 +- internal/api/auth.go | 70 +++++++++---- internal/api/auth_test.go | 2 +- internal/api/data.go | 7 +- internal/api/idlepoll_test.go | 104 ++++++++++++++++++++ internal/api/inputhook_throttle_test.go | 6 +- internal/api/me.go | 7 +- internal/api/me_test.go | 76 +++++++++++++- internal/api/middleware.go | 17 +--- internal/api/middleware_test.go | 95 +----------------- internal/api/openapi.yaml | 24 ++--- internal/api/quota.go | 4 +- internal/api/workspaces.go | 7 +- internal/backend/wire.go | 11 ++- internal/broker/activity.go | 10 +- internal/broker/connection_state.go | 11 ++- internal/broker/connection_state_test.go | 2 +- internal/broker/lease_session_test.go | 96 ++++++++++++++++++ internal/broker/leases.go | 45 +++++++-- internal/broker/tickets.go | 37 +++++-- internal/store/sessions.go | 39 ++++++++ tests/integration/api_admission_test.go | 4 + tests/integration/session_touch_test.go | 74 ++++++++++++-- web/src/api/client.ts | 9 -- web/src/app/me.tsx | 41 ++------ web/src/auth/AuthGate.tsx | 9 +- web/src/data/DataDetailPage.tsx | 5 +- web/src/data/DataListPage.tsx | 5 +- web/src/data/api.ts | 8 +- web/src/session/SessionPage.tsx | 9 +- web/src/workspaces/WorkspaceDetailPage.tsx | 29 +++--- web/src/workspaces/WorkspaceListPage.tsx | 2 +- web/src/workspaces/api.ts | 18 +--- web/src/workspaces/resource.ts | 66 +++++-------- web/tests/unit/api/client.test.ts | 40 +------- web/tests/unit/workspaces/resource.test.tsx | 42 -------- 38 files changed, 653 insertions(+), 442 deletions(-) create mode 100644 internal/api/idlepoll_test.go diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 984a7abb..75b82724 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -184,15 +184,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`, `internal/api/connection_status.go`) — - A6-S6's second authenticated path exists to be reviewed. The portal's - interval polls additionally carry `X-TCDI-Poll: background`, which makes - `requireAuth` read the session with `Peek` on any route: a - visible-but-unattended tab cannot hold a session open, and the marker - can only withhold an idle slide so a forged one gains nothing - (`TestRequireAuth_BackgroundPollMarker`). + (`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`; @@ -547,27 +547,23 @@ 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 background polling vs the idle window** — Implemented: before - this fix the SPA's interval polls of `GET /v1/workspaces`, - `/v1/workspaces/{id}` and `/v1/workspaces/{id}/events` (all mounted - behind sliding `RequireAuth`) kept a visible-but-unattended tab's - session alive forever. The SPA now marks timer-driven reads with - `X-TCDI-Poll: background` (useResource ticks, the session page's - "starting" poll, `/v1/me` backoff retries) and `requireAuth` peeks - instead of sliding on marked requests; navigation, user-triggered - refresh, return-to-visible reloads and mutations still slide. Desktop - streams keep the window open only through server-measured RFB input - (`InputHook` → `TouchPrincipal`, throttle 1/min per principal), so an - active desktop user is not signed out mid-work while an open-but-idle - stream is not portal activity. Residual, unchanged here: lease - redeem/renew/rehydrate never consult the portal idle window — a lease - outlives idle expiry by design and dies on its own TTL, on revoke, or - on the bound session's absolute expiry (S17), so tearing an open stream - down at portal-idle expiry remains a separate decision, not covered by - this fix. +- **Portal idle-extension depends on lease activity** — Implemented + (FIX-IDLE): portal reads are all passive server-side, so no HTTP request + a client can shape extends the idle window; extension only ever credits + (a) mutations and (b) RFB input measured broker-side. 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 b963c884..c3631e92 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 e9447fb5..ee54fd82 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/auth.go b/internal/api/auth.go index afbff79b..16f1cbbc 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -196,6 +196,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 } @@ -292,6 +298,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() @@ -866,26 +886,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 { @@ -894,15 +921,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) } } @@ -914,11 +947,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 } func (a *Authenticator) tenantAllowed(tenant string) bool { diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 6aa87339..9e9d8bd6 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -456,7 +456,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 51ba5a9e..bd4f21e4 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..ff7f1081 --- /dev/null +++ b/internal/api/idlepoll_test.go @@ -0,0 +1,104 @@ +// 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) + } +} 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.go b/internal/api/middleware.go index 5a9b0168..a4681d43 100644 --- a/internal/api/middleware.go +++ b/internal/api/middleware.go @@ -72,23 +72,10 @@ func RequestIDFromContext(ctx context.Context) string { // Authentication // --------------------------------------------------------------------------- -// PollHeader marks a request as portal background polling — automated -// traffic, not user activity. When it carries PollHeaderValue the session -// is read with Peek instead of Get, so the request authenticates normally -// but cannot slide the idle deadline. It is honoured on every -// authenticated route and any method: the marker can only withhold an -// idle slide, never earn one, so a forged marker gains nothing (P4, D18). -const PollHeader = "X-TCDI-Poll" - -// PollHeaderValue is the marker value the portal sends on its interval -// polls; any other value leaves the request a normal activity touch. -const PollHeaderValue = "background" - // RequireAuth rejects requests without a valid server-side session (opaque // host-only cookie) and attaches the verified Principal and Session to the // request context. Handlers must derive owner/tenant from that principal. -// Each authenticated request slides the session's idle deadline (Get) -// unless the client marked it background polling (PollHeader). +// Each authenticated request slides the session's idle deadline (Get). func (a *Authenticator) RequireAuth(next http.Handler) http.Handler { return a.requireAuth(next, true) } @@ -108,7 +95,7 @@ func (a *Authenticator) requireAuth(next http.Handler, slide bool) http.Handler return } var sess *Session - if slide && r.Header.Get(PollHeader) != PollHeaderValue { + if slide { sess, err = a.sessions.Get(r.Context(), c.Value) } else { sess, err = a.sessions.Peek(r.Context(), c.Value) diff --git a/internal/api/middleware_test.go b/internal/api/middleware_test.go index bb3fedc1..7160614c 100644 --- a/internal/api/middleware_test.go +++ b/internal/api/middleware_test.go @@ -10,7 +10,6 @@ import ( "net/http/httptest" "strings" "testing" - "time" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" @@ -382,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". @@ -434,94 +436,3 @@ func TestRequireAuth_SessionStoreErrors(t *testing.T) { }) } } - -// TestRequireAuth_BackgroundPollMarker (FIX-IDLE): a request carrying -// X-TCDI-Poll: background authenticates but never slides the idle window, -// so the portal's interval polls cannot keep a visible-but-unattended -// session alive. An unmarked request on the same RequireAuth route still -// slides; once the window lapses both shapes answer 401. -func TestRequireAuth_BackgroundPollMarker(t *testing.T) { - fc := &fakeClock{now: time.Now()} - store := NewInMemorySessionStore(time.Minute).WithClock(fc.Now) - auth := &Authenticator{ - cfg: &AuthConfig{SessionCookieName: "__Host-tcdi_session"}, - sessions: store, - now: fc.Now, - } - save := func(id string) { - if err := store.Save(context.Background(), &Session{ - ID: id, - Principal: Principal{Issuer: "iss", Subject: "sub", TenantID: "tenant-a"}, - CreatedAt: fc.Now(), - LastSeenAt: fc.Now(), - ExpiresAt: fc.Now().Add(time.Hour), - }); err != nil { - t.Fatal(err) - } - } - handler := auth.RequireAuth(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusNoContent) - })) - do := func(id string, marked bool) int { - req := httptest.NewRequest(http.MethodGet, "/v1/workspaces", nil) - req.AddCookie(&http.Cookie{Name: "__Host-tcdi_session", Value: id}) - if marked { - // Literal wire values, not PollHeader/PollHeaderValue, so this - // file still compiles on a tree without the marker — where the - // "poll past idle" step then fails, proving the regression. - req.Header.Set("X-TCDI-Poll", "background") - } - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - return rec.Code - } - - // Marked polls authenticate but never slide: created at t=0, the poll - // at +50s succeeds yet the window still lapses at +60s. - save("sess-poll") - fc.Advance(50 * time.Second) - if code := do("sess-poll", true); code != http.StatusNoContent { - t.Fatalf("marked poll inside the window rejected: %d", code) - } - fc.Advance(15 * time.Second) // t=65s — past idle despite the +50s poll - if code := do("sess-poll", true); code != http.StatusUnauthorized { - t.Fatalf("marked poll past idle: %d, want 401", code) - } - - // The marker is opt-out only: a wrong value is ordinary activity. - save("sess-other-value") - req := httptest.NewRequest(http.MethodGet, "/v1/workspaces", nil) - req.AddCookie(&http.Cookie{Name: "__Host-tcdi_session", Value: "sess-other-value"}) - req.Header.Set("X-TCDI-Poll", "1") - rec := httptest.NewRecorder() - handler.ServeHTTP(rec, req) - if rec.Code != http.StatusNoContent { - t.Fatalf("other marker value rejected: %d", rec.Code) - } - sess, err := store.Get(context.Background(), "sess-other-value") - if err != nil { - t.Fatal(err) - } - if !sess.LastSeenAt.Equal(fc.Now()) { - t.Fatalf("unrecognized marker value did not slide: LastSeenAt=%v now=%v", sess.LastSeenAt, fc.Now()) - } - - // Without the marker the same cadence slides the window as before. - save("sess-active") - fc.Advance(50 * time.Second) - if code := do("sess-active", false); code != http.StatusNoContent { - t.Fatalf("unmarked request rejected: %d", code) - } - fc.Advance(50 * time.Second) // 50s since the slide — still inside - if code := do("sess-active", false); code != http.StatusNoContent { - t.Fatalf("unmarked request past one window: %d", code) - } - - // Idle expiry answers 401 to marked and unmarked requests alike. - fc.Advance(61 * time.Second) - for _, marked := range []bool{false, true} { - if code := do("sess-active", marked); code != http.StatusUnauthorized { - t.Fatalf("expired session marked=%v: %d, want 401", marked, code) - } - } -} diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 8879d162..e816e98c 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -27,18 +27,18 @@ info: transactional outbox (see ADR 0002); Kubernetes RBAC denies direct CR writes by end users. - ## Session idle timeout and background polling - - Sessions carry a sliding idle timeout: an authenticated request normally - slides the idle deadline, while a request carrying the header - `X-TCDI-Poll: background` is authenticated identically but does **not** - slide it. The portal sends the marker on its interval polls so a - visible-but-unattended tab cannot hold a session open; interactive - desktop input slides the same window through a server-measured signal, - not through this API. The marker is honoured on every authenticated - endpoint — it can only withhold an idle slide, never earn one, so a - client gains nothing by sending it. `GET /v1/session` and - `GET /v1/workspaces/{workspaceId}/connection` are always passive. + ## 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. Only + mutations (any non-`GET` endpoint, `POST /v1/logout` included) and + server-measured interactive desktop input slide the window. 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 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/workspaces.go b/internal/api/workspaces.go index fe0844be..0d50f1fc 100644 --- a/internal/api/workspaces.go +++ b/internal/api/workspaces.go @@ -206,10 +206,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 d659c3fa..7e506e21 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 @@ -873,6 +878,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/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/client.ts b/web/src/api/client.ts index 93bb8402..60623676 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -10,15 +10,6 @@ export type ApiClient = Client; // export reads document.cookie; the tcdi_csrf cookie is gone in v0.2. export const CSRF_HEADER = "X-CSRF-Token"; -// Background-poll marker (D18): reads issued by the portal's polling loops -// carry `X-TCDI-Poll: background`; the server then authenticates them with -// a peek at the session instead of a sliding read, so a visible but -// unattended tab cannot hold the idle window open. The marker can only -// withhold an idle slide, never earn one — there is nothing to spoof. -// Navigation, user-triggered reloads and mutations send nothing. -export const POLL_HEADER = "X-TCDI-Poll"; -export const POLL_HEADERS: Record = { [POLL_HEADER]: "background" }; - let csrfToken: string | undefined; export function setCsrfToken(token: string | undefined): void { diff --git a/web/src/app/me.tsx b/web/src/app/me.tsx index 9f4a951e..002310ae 100644 --- a/web/src/app/me.tsx +++ b/web/src/app/me.tsx @@ -1,5 +1,5 @@ import { createContext, useCallback, useContext, useEffect, useRef, useState, type ReactNode } from "react"; -import { POLL_HEADERS, setCsrfToken, unwrap, type ApiClient } from "../api/client"; +import { setCsrfToken, unwrap, type ApiClient } from "../api/client"; import { useApi } from "../api/context"; import type { components } from "../api/generated/schema"; @@ -77,13 +77,8 @@ export function isTransientMeError(e: unknown): boolean { return e instanceof TypeError; } -// background marks a retry-loop attempt as automated traffic: the request -// authenticates without sliding the portal idle window (POLL_HEADERS). -export async function fetchMe(background = false, fetchImpl: typeof fetch = fetch): Promise { - const res = await fetchImpl("/v1/me", { - credentials: "same-origin", - headers: { Accept: "application/json", ...(background ? POLL_HEADERS : {}) }, - }); +export async function fetchMe(fetchImpl: typeof fetch = fetch): Promise { + const res = await fetchImpl("/v1/me", { credentials: "same-origin", headers: { Accept: "application/json" } }); if (res.status === 404 || res.status === 501) return STUB_ME; if (!res.ok) throw new MeHttpError(res.status); return parseMe(await res.json()); @@ -100,8 +95,8 @@ export function MeProvider({ load = fetchMe, }: { children: ReactNode; - /** Injectable for tests; `background` marks a backoff retry. */ - load?: (background: boolean) => Promise; + /** Injectable for tests. */ + load?: () => Promise; }) { const [state, setState] = useState({ status: "loading", me: null }); useEffect(() => { @@ -110,10 +105,7 @@ export function MeProvider({ let failures = 0; const attempt = async () => { try { - // The first attempt is part of the page load — real navigation; - // backoff retries are background and must not slide the idle - // window while the shell sits on an unreachable API (D18). - const me = await load(failures > 0); + const me = await load(); if (cancelled) return; // The API client reads the token from module state, not from a // cookie (D17) — install what /v1/me published. @@ -238,37 +230,24 @@ type Query = Record; interface LooseGet { GET( path: string, - init: { params?: { query?: Query }; headers?: Record }, + init: { params?: { query?: Query } }, ): Promise<{ data?: unknown; error?: unknown; response: Response }>; } -export async function get( - api: ApiClient, - path: string, - query?: Query, - background = false, -): Promise { +export async function get(api: ApiClient, path: string, query?: Query): Promise { const loose = api as unknown as LooseGet; - return unwrap( - await loose.GET(path, { - ...(query ? { params: { query } } : {}), - // background marks an automated poll: the server authenticates the - // request without sliding the portal idle window (D18). - ...(background ? { headers: POLL_HEADERS } : {}), - }), - ) as T; + return unwrap(await loose.GET(path, query ? { params: { query } } : {})) as T; } export async function listAll( api: ApiClient, path: string, query: Query, - background = false, ): Promise<{ items: T[]; truncated: boolean }> { const items: T[] = []; let pageToken: string | undefined; for (let i = 0; i < MAX_PAGES; i++) { - const page = await get>(api, path, { ...query, limit: PAGE_LIMIT, pageToken }, background); + const page = await get>(api, path, { ...query, limit: PAGE_LIMIT, pageToken }); items.push(...page.items); if (!page.nextPageToken) return { items, truncated: false }; pageToken = page.nextPageToken; diff --git a/web/src/auth/AuthGate.tsx b/web/src/auth/AuthGate.tsx index 996e1525..994dcf3b 100644 --- a/web/src/auth/AuthGate.tsx +++ b/web/src/auth/AuthGate.tsx @@ -1,7 +1,7 @@ import { useEffect, useState, type ReactNode } from "react"; import { t } from "../i18n"; import { useApi } from "../api/context"; -import { POLL_HEADERS, unwrap } from "../api/client"; +import { unwrap } from "../api/client"; import { isPortalApiError } from "../api/errors"; // Login lives on the portal origin ahead of the API (OIDC-backed session @@ -49,11 +49,8 @@ export function AuthGate({ if (!cancelled) onUnauthenticated(); return; } - // MeProvider reads the body again once the gate opens. Retries - // after the first failure are background traffic — they carry the - // poll marker so a gate stuck retrying cannot hold the idle window - // open (D18); the first attempt rides on the page load itself. - unwrap(await api.GET("/v1/me", failures > 0 ? { headers: POLL_HEADERS } : {})); + // MeProvider reads the body again once the gate opens. + unwrap(await api.GET("/v1/me")); if (!cancelled) setReady(true); } catch (e) { if (cancelled) return; diff --git a/web/src/data/DataDetailPage.tsx b/web/src/data/DataDetailPage.tsx index 5b487c81..e4b53646 100644 --- a/web/src/data/DataDetailPage.tsx +++ b/web/src/data/DataDetailPage.tsx @@ -33,10 +33,7 @@ export function DataDetailPage({ const api = useApi(); const me = useMeLoaded(); const admin = isTenantAdmin(me.data); - const load = useCallback( - (background: boolean) => getRetainedData(api, dataId, background), - [api, dataId], - ); + const load = useCallback(() => getRetainedData(api, dataId), [api, dataId]); // Poll while the record is Purging (same cadence as the list); a Purged // record either stops being returned (404 -> null) or reports Purged — // either way the poll stops. diff --git a/web/src/data/DataListPage.tsx b/web/src/data/DataListPage.tsx index 5c314423..7808062c 100644 --- a/web/src/data/DataListPage.tsx +++ b/web/src/data/DataListPage.tsx @@ -89,10 +89,7 @@ export function DataListPage({ pollIntervalMs }: { pollIntervalMs?: number }) { const admin = isTenantAdmin(me.data); const [scope, setScope] = useState("mine"); const effectiveScope: Scope = admin ? scope : "mine"; - const load = useCallback( - (background: boolean) => listRetainedData(api, effectiveScope, background), - [api, effectiveScope], - ); + const load = useCallback(() => listRetainedData(api, effectiveScope), [api, effectiveScope]); const list = useResource(load, (data) => purgePollDelay(data, pollIntervalMs)); const [attaching, setAttaching] = useState(null); diff --git a/web/src/data/api.ts b/web/src/data/api.ts index eeba2e37..5971360c 100644 --- a/web/src/data/api.ts +++ b/web/src/data/api.ts @@ -24,14 +24,11 @@ export interface AttachDataBody { // GET /v1/data. `Purged` records are excluded server-side; the filter here // keeps the list honest if an old backend still returns them. -// `background` marks a scheduled poll tick (useResource) — the request -// authenticates without sliding the portal idle window (D18). export async function listRetainedData( api: ApiClient, scope: Scope, - background = false, ): Promise<{ items: ScopedRetainedData[]; truncated: boolean }> { - const page = await listAll(api, "/v1/data", { scope }, background); + const page = await listAll(api, "/v1/data", { scope }); return { ...page, items: page.items.filter((r) => r.state !== "Purged") }; } @@ -41,10 +38,9 @@ export async function listRetainedData( export async function getRetainedData( api: ApiClient, id: string, - background = false, ): Promise { try { - return await get(api, `/v1/data/${encodeURIComponent(id)}`, undefined, background); + return await get(api, `/v1/data/${encodeURIComponent(id)}`); } catch (e) { if (isPortalApiError(e) && e.httpStatus === 404) return null; throw e; diff --git a/web/src/session/SessionPage.tsx b/web/src/session/SessionPage.tsx index d15f9cbc..0bdb495e 100644 --- a/web/src/session/SessionPage.tsx +++ b/web/src/session/SessionPage.tsx @@ -3,7 +3,7 @@ import { t, formatTime } from "../i18n"; import { formatDuration } from "../templates/format"; import { phaseLabelKey } from "../workspaces/helpers"; import { useApi } from "../api/context"; -import { newIdempotencyKey, POLL_HEADERS, unwrap } from "../api/client"; +import { newIdempotencyKey, unwrap } from "../api/client"; import { isPortalApiError } from "../api/errors"; import { LifecycleProgress } from "../progress/LifecycleProgress"; import { opPollMs, startInFlight, withJitter } from "../progress/derive"; @@ -678,13 +678,8 @@ export function SessionPage({ let delay = 1_000; const tick = async () => { try { - // Timer-driven poll, not user activity: the marker keeps it from - // sliding the portal idle window (D18). const ws = unwrap( - await api.GET("/v1/workspaces/{workspaceId}", { - params: { path: { workspaceId } }, - headers: POLL_HEADERS, - }), + await api.GET("/v1/workspaces/{workspaceId}", { params: { path: { workspaceId } } }), ); if (cancelled || !mounted.current) return; failures = 0; diff --git a/web/src/workspaces/WorkspaceDetailPage.tsx b/web/src/workspaces/WorkspaceDetailPage.tsx index 63ac0a51..2e84a0a8 100644 --- a/web/src/workspaces/WorkspaceDetailPage.tsx +++ b/web/src/workspaces/WorkspaceDetailPage.tsx @@ -145,22 +145,19 @@ export function WorkspaceDetailPage({ const [busy, setBusy] = useState(null); const [actionError, setActionError] = useState(null); - const load = useCallback( - async (background: boolean): Promise => { - const [workspace, events] = await Promise.all([ - getWorkspace(api, workspaceId, background), - listWorkspaceEvents(api, workspaceId, background), - ]); - // The retained disk this workspace mounts: named so the user can tell - // where its home came from (T5.4). Best effort — the record may be - // gone or not visible to this caller. - const retained = workspace.retainedDataRef - ? await getRetainedData(api, workspace.retainedDataRef, background).catch(() => null) - : null; - return { workspace, events, retained }; - }, - [api, workspaceId], - ); + const load = useCallback(async (): Promise => { + const [workspace, events] = await Promise.all([ + getWorkspace(api, workspaceId), + listWorkspaceEvents(api, workspaceId), + ]); + // The retained disk this workspace mounts: named so the user can tell + // where its home came from (T5.4). Best effort — the record may be + // gone or not visible to this caller. + const retained = workspace.retainedDataRef + ? await getRetainedData(api, workspace.retainedDataRef).catch(() => null) + : null; + return { workspace, events, retained }; + }, [api, workspaceId]); const detail = useResource(load, pollIntervalMs ?? pollDelay); const { toast } = useToast(); // A delete "completes" when the API drops the row (FX-R19 hides finalised diff --git a/web/src/workspaces/WorkspaceListPage.tsx b/web/src/workspaces/WorkspaceListPage.tsx index 3d983dac..36fec877 100644 --- a/web/src/workspaces/WorkspaceListPage.tsx +++ b/web/src/workspaces/WorkspaceListPage.tsx @@ -79,7 +79,7 @@ const COLUMNS: Column[] = [ export function WorkspaceListPage({ pollIntervalMs }: { pollIntervalMs?: number }) { const api = useApi(); - const load = useCallback((background: boolean) => listWorkspaces(api, background), [api]); + const load = useCallback(() => listWorkspaces(api), [api]); const list = useResource(load, pollIntervalMs ?? pollDelay); const [announcement, setAnnouncement] = useState(null); diff --git a/web/src/workspaces/api.ts b/web/src/workspaces/api.ts index 6f2d9f4d..f8e145d0 100644 --- a/web/src/workspaces/api.ts +++ b/web/src/workspaces/api.ts @@ -1,4 +1,4 @@ -import { POLL_HEADERS, unwrap, type ApiClient } from "../api/client"; +import { unwrap, type ApiClient } from "../api/client"; import { noteServerDateHeader } from "../progress/derive"; import type { WorkspaceEvent, WorkspaceView } from "./helpers"; @@ -6,21 +6,16 @@ import type { WorkspaceEvent, WorkspaceView } from "./helpers"; // /v1/workspaces?scope=…, GET /v1/workspaces/{id}/events). Each response's // Date header feeds the progress module's skew correction — the elapsed // counters anchor on server-side updated_at, not the client clock. -// -// `background` marks a read as an automated poll (POLL_HEADERS): the -// server authenticates it without sliding the portal idle window. Only -// useResource's scheduled ticks pass true. const MAX_PAGES = 10; /** All of the caller's workspaces (`scope=mine`; the tenant view is admin's). */ -export async function listWorkspaces(api: ApiClient, background = false): Promise { +export async function listWorkspaces(api: ApiClient): Promise { const items: WorkspaceView[] = []; let pageToken: string | undefined; for (let i = 0; i < MAX_PAGES; i++) { const res = await api.GET("/v1/workspaces", { params: { query: { scope: "mine", limit: 200, ...(pageToken ? { pageToken } : {}) } }, - ...(background ? { headers: POLL_HEADERS } : {}), }); noteServerDateHeader(res.response.headers.get("date")); const page = unwrap(res); @@ -31,14 +26,9 @@ export async function listWorkspaces(api: ApiClient, background = false): Promis return items; } -export async function getWorkspace( - api: ApiClient, - workspaceId: string, - background = false, -): Promise { +export async function getWorkspace(api: ApiClient, workspaceId: string): Promise { const res = await api.GET("/v1/workspaces/{workspaceId}", { params: { path: { workspaceId } }, - ...(background ? { headers: POLL_HEADERS } : {}), }); noteServerDateHeader(res.response.headers.get("date")); return unwrap(res); @@ -48,12 +38,10 @@ export async function getWorkspace( export async function listWorkspaceEvents( api: ApiClient, workspaceId: string, - background = false, ): Promise { const res = unwrap( await api.GET("/v1/workspaces/{workspaceId}/events", { params: { path: { workspaceId } }, - ...(background ? { headers: POLL_HEADERS } : {}), }), ); return res.items ?? []; diff --git a/web/src/workspaces/resource.ts b/web/src/workspaces/resource.ts index d96defd4..e58e6d39 100644 --- a/web/src/workspaces/resource.ts +++ b/web/src/workspaces/resource.ts @@ -24,14 +24,7 @@ export type PollInterval = number | null | ((data: T | undefined) => number | const MAX_BACKOFF_MS = 30_000; -// The `background` flag tells the loader whether the call is a scheduled -// poll tick (true → the request carries POLL_HEADERS and cannot slide the -// portal idle window) or user-facing activity (false → mount loads, -// manual refresh, return-to-visible reloads all slide normally). -export function useResource( - load: (background: boolean) => Promise, - interval: PollInterval = null, -): Resource { +export function useResource(load: () => Promise, interval: PollInterval = null): Resource { const [data, setData] = useState(undefined); const [error, setError] = useState(null); const [loading, setLoading] = useState(true); @@ -50,29 +43,26 @@ export function useResource( // poll is untouched, so always-on consumers are unchanged. const scheduleRef = useRef<() => void>(() => {}); - const run = useCallback( - async (background: boolean) => { - const started = epoch.current; - try { - const d = await load(background); - if (started !== epoch.current) return; - dataRef.current = d; - failures.current = 0; - retryAfterMs.current = null; - setData(d); - setError(null); - } catch (e) { - if (started !== epoch.current) return; - failures.current += 1; - const ra = (e as { retryAfterMs?: unknown }).retryAfterMs; - retryAfterMs.current = typeof ra === "number" && Number.isFinite(ra) ? ra : null; - setError(e); - } finally { - if (started === epoch.current) setLoading(false); - } - }, - [load], - ); + const run = useCallback(async () => { + const started = epoch.current; + try { + const d = await load(); + if (started !== epoch.current) return; + dataRef.current = d; + failures.current = 0; + retryAfterMs.current = null; + setData(d); + setError(null); + } catch (e) { + if (started !== epoch.current) return; + failures.current += 1; + const ra = (e as { retryAfterMs?: unknown }).retryAfterMs; + retryAfterMs.current = typeof ra === "number" && Number.isFinite(ra) ? ra : null; + setError(e); + } finally { + if (started === epoch.current) setLoading(false); + } + }, [load]); useEffect(() => { let timer: ReturnType | undefined; @@ -98,17 +88,15 @@ export function useResource( armed = false; const delay = nextDelay(); if (delay === null || document.visibilityState === "hidden") return; - timer = setTimeout(() => void tick(true), delay); + timer = setTimeout(() => void tick(), delay); armed = true; }; - const tick = async (background: boolean) => { - await run(background); + const tick = async () => { + await run(); schedule(); }; const onVisible = () => { - // Returning to a hidden tab is the user showing up — the reload - // counts as activity; only the timer-driven ticks are background. - if (document.visibilityState === "visible") void tick(false); + if (document.visibilityState === "visible") void tick(); else { clearTimeout(timer); armed = false; @@ -117,7 +105,7 @@ export function useResource( scheduleRef.current = () => { if (!armed) schedule(); }; - void tick(false); + void tick(); document.addEventListener("visibilitychange", onVisible); return () => { stopped = true; @@ -138,7 +126,7 @@ export function useResource( const refresh = useCallback(() => { epoch.current += 1; - return run(false).then(() => scheduleRef.current()); + return run().then(() => scheduleRef.current()); }, [run]); const clearError = useCallback(() => setError(null), []); diff --git a/web/tests/unit/api/client.test.ts b/web/tests/unit/api/client.test.ts index 0d234a48..520a7983 100644 --- a/web/tests/unit/api/client.test.ts +++ b/web/tests/unit/api/client.test.ts @@ -1,7 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { createApi, setCsrfToken, CSRF_HEADER, POLL_HEADER, POLL_HEADERS, unwrap } from "../../../src/api/client"; -import { fetchMe } from "../../../src/app/me"; -import { getWorkspace, listWorkspaces } from "../../../src/workspaces/api"; +import { createApi, setCsrfToken, CSRF_HEADER, unwrap } from "../../../src/api/client"; // The CSRF token arrives in the GET /v1/me body (P1/D17) and lives only in // module state — setCsrfToken installs it, the client echoes it on mutations, @@ -123,39 +121,3 @@ describe("api client CSRF", () => { expect(posts).toHaveLength(2); }); }); - -// FIX-IDLE — background polls carry X-TCDI-Poll: background so the server -// authenticates them without sliding the portal idle window; foreground -// reads send nothing. -describe("background-poll marker", () => { - it("sends X-TCDI-Poll only on marked reads", async () => { - const { calls, fetchImpl } = recordingFetch(() => json(200, { items: [] })); - const client = createApi(fetchImpl); - - await listWorkspaces(client, true); - await listWorkspaces(client); - await getWorkspace(client, WS, true); - - const [markedList, plainList, markedGet] = calls; - expect(markedList.headers.get(POLL_HEADER)).toBe(POLL_HEADERS[POLL_HEADER]); - expect(plainList.headers.get(POLL_HEADER)).toBeNull(); - expect(markedGet.headers.get(POLL_HEADER)).toBe(POLL_HEADERS[POLL_HEADER]); - }); - - it("fetchMe marks retry attempts", async () => { - const calls: Request[] = []; - // fetchMe hits the relative /v1/me, so give the Request a base URL. - const fetchImpl = (async (input: RequestInfo | URL, init?: RequestInit) => { - const url = typeof input === "string" ? new URL(input, "https://portal.test") : input; - const req = new Request(url, init); - calls.push(req); - return json(200, ME_BODY); - }) as typeof fetch; - - await fetchMe(true, fetchImpl); - await fetchMe(false, fetchImpl); - - expect(calls[0].headers.get(POLL_HEADER)).toBe(POLL_HEADERS[POLL_HEADER]); - expect(calls[1].headers.get(POLL_HEADER)).toBeNull(); - }); -}); diff --git a/web/tests/unit/workspaces/resource.test.tsx b/web/tests/unit/workspaces/resource.test.tsx index fe008b66..93741627 100644 --- a/web/tests/unit/workspaces/resource.test.tsx +++ b/web/tests/unit/workspaces/resource.test.tsx @@ -79,45 +79,3 @@ describe("useResource refresh re-arm (FX-R29)", () => { expect(load).toHaveBeenCalledTimes(5); }); }); - -// FIX-IDLE — the loader's `background` flag marks which reads may slide -// the portal idle window: scheduled ticks are background (the server -// peeks instead of touching last_seen_at) while mount loads, manual -// refresh and the return-to-visible reload are real activity. -describe("useResource background-poll marker", () => { - it("marks timer ticks background; mount and refresh stay foreground", async () => { - const load = vi.fn((_background: boolean) => Promise.resolve("x")); - const { result } = renderHook(() => useResource(load, 1_000)); - await flush(); - expect(load).toHaveBeenLastCalledWith(false); - - await advance(1_000); - expect(load).toHaveBeenLastCalledWith(true); - await advance(1_000); - expect(load).toHaveBeenLastCalledWith(true); - - await act(async () => { - await result.current.refresh(); - }); - expect(load).toHaveBeenLastCalledWith(false); - }); - - it("treats the return-to-visible reload as activity", async () => { - const visibility = vi - .spyOn(Document.prototype, "visibilityState", "get") - .mockReturnValue("hidden"); - const load = vi.fn((_background: boolean) => Promise.resolve("x")); - renderHook(() => useResource(load, 1_000)); - await flush(); - expect(load).toHaveBeenLastCalledWith(false); - // Hidden: the timer never fires. - await advance(5_000); - expect(load).toHaveBeenCalledTimes(1); - - visibility.mockReturnValue("visible"); - await act(async () => { - document.dispatchEvent(new Event("visibilitychange")); - }); - expect(load).toHaveBeenLastCalledWith(false); - }); -}); From 8b9b4a0916e981b052297dff337904ba6e961f9a Mon Sep 17 00:00:00 2001 From: Nguyen Van Long Date: Tue, 6 Oct 2026 13:36:20 +0700 Subject: [PATCH 4/5] feat(api,web): POST /v1/session:touch activity beat (FIX-IDLE) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With every cookie-authenticated GET now passive, real user interaction needed a way to slide the idle window — read-only portal use is activity. POST /v1/session:touch is a bodiless mutation: RequireAuth's sliding read extends the deadline, RequireCSRF guards it like any other write, the login-family limiter keys it on the session digest, and the audited wrapper emits session.touch. The SPA sends it on pointerdown, keydown and route changes only — throttled to one call per minute in useActivityTouch (web/src/session/activity.ts), never from timer polls. Server tests pin the semantics: polls alone let the session expire, one touch slides it, CSRF/session are both required (403/401). Portal tests pin that pointer/key/route events beat while useResource's timer ticks never do. OpenAPI + generated types updated. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- docs/lifecycle-reasons.md | 9 +++ docs/security/threat-model.md | 12 ++- internal/api/auditroutes.go | 3 + internal/api/auth_test.go | 1 + internal/api/idlepoll_test.go | 49 ++++++++++++ internal/api/openapi.yaml | 49 ++++++++++-- internal/api/session_probe.go | 30 +++++++- internal/backend/wire.go | 1 + internal/observability/metrics.go | 4 +- web/src/api/generated/schema.d.ts | 58 +++++++++++++++ web/src/app/shell.tsx | 5 ++ web/src/session/activity.ts | 55 ++++++++++++++ web/tests/mock-api/auth.ts | 5 ++ web/tests/unit/session/activity.test.tsx | 94 ++++++++++++++++++++++++ 14 files changed, 363 insertions(+), 12 deletions(-) create mode 100644 web/src/session/activity.ts create mode 100644 web/tests/unit/session/activity.test.tsx 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 3335f692..bf8af688 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -568,9 +568,15 @@ Additional items found while writing this document (not from A6): 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** — Implemented - (FIX-IDLE): portal reads are all passive server-side, so no HTTP request - a client can shape extends the idle window; extension only ever credits - (a) mutations and (b) RFB input measured broker-side. That input touch + (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` (RequireAuth+CSRF, 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`, 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_test.go b/internal/api/auth_test.go index 237f7acc..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 diff --git a/internal/api/idlepoll_test.go b/internal/api/idlepoll_test.go index ff7f1081..0109205d 100644 --- a/internal/api/idlepoll_test.go +++ b/internal/api/idlepoll_test.go @@ -102,3 +102,52 @@ func TestMeRead_DoesNotSlideIdleWindow(t *testing.T) { 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 no slide. + sess2, _ := login(t, env, "user-b") + 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) + } + // 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/openapi.yaml b/internal/api/openapi.yaml index e816e98c..a8d8b4e0 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -33,12 +33,13 @@ info: 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. Only - mutations (any non-`GET` endpoint, `POST /v1/logout` included) and - server-measured interactive desktop input slide the window. 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. + 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 @@ -143,6 +144,42 @@ 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). 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. + 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/session_probe.go b/internal/api/session_probe.go index d0f9f4e9..edde2489 100644 --- a/internal/api/session_probe.go +++ b/internal/api/session_probe.go @@ -5,9 +5,12 @@ package api import ( "net/http" + + "github.com/tinyorbitvn/tinycdi/internal/observability" ) -// 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 +51,28 @@ 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 mutation needs no body: RequireAuth's sliding session read +// IS the touch, RequireCSRF guards it like every other state change, and +// 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.RequireAuth( + 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 — the request is authenticated and CSRF- +// checked already, and the sliding read inside RequireAuth extended the +// idle deadline; there is nothing else to do. +func (a *Authenticator) SessionTouchHandler(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) +} diff --git a/internal/backend/wire.go b/internal/backend/wire.go index cd0d3c77..9e36224c 100644 --- a/internal/backend/wire.go +++ b/internal/backend/wire.go @@ -716,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) 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/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); + }); +}); From 0cf93a1e6dd988b2a2749e04ef82cfc7b57ffe0d Mon Sep 17 00:00:00 2001 From: Nguyen Van Long Date: Tue, 6 Oct 2026 14:29:29 +0700 Subject: [PATCH 5/5] fix(api): slide session:touch only after CSRF passes (FIX-IDLE) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer finding: the beat's RequireAuth ran its sliding read before RequireCSRF, so a cookie-only POST that failed the token check still extended the idle window — a slide the caller never earned. The chain is now RequireAuthPassive + RequireCSRF and the handler slides explicitly with Get only after both gates pass. Denied requests change nothing: the test proves a 403'd touch leaves the window to lapse on schedule. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- docs/security/threat-model.md | 13 ++++++------ internal/api/idlepoll_test.go | 12 ++++++++++- internal/api/openapi.yaml | 15 +++++++------- internal/api/session_probe.go | 39 +++++++++++++++++++++++++++-------- 4 files changed, 56 insertions(+), 23 deletions(-) diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 9f007039..d3e3a37a 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -571,12 +571,13 @@ Additional items found while writing this document (not from A6): (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` (RequireAuth+CSRF, 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 + `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`, diff --git a/internal/api/idlepoll_test.go b/internal/api/idlepoll_test.go index 0109205d..7191076a 100644 --- a/internal/api/idlepoll_test.go +++ b/internal/api/idlepoll_test.go @@ -135,14 +135,24 @@ func TestSessionTouch_SlidesIdleWindow(t *testing.T) { t.Fatalf("idle-expired session accepted: %d", r.StatusCode) } - // No CSRF token: denied and no slide. + // 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) diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index a8d8b4e0..ea07e0a2 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -149,13 +149,14 @@ paths: operationId: touchSession summary: 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. + 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 diff --git a/internal/api/session_probe.go b/internal/api/session_probe.go index edde2489..3696f367 100644 --- a/internal/api/session_probe.go +++ b/internal/api/session_probe.go @@ -4,9 +4,11 @@ package api import ( + "errors" "net/http" "github.com/tinyorbitvn/tinycdi/internal/observability" + "github.com/tinyorbitvn/tinycdi/internal/store" ) // session_probe.go implements the session surface's two one-liners: @@ -55,13 +57,15 @@ func (a *Authenticator) SessionProbeHandler(w http.ResponseWriter, r *http.Reque // 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 mutation needs no body: RequireAuth's sliding session read -// IS the touch, RequireCSRF guards it like every other state change, and -// the audited wrapper emits the session.touch event. Optional middleware -// wraps the chain (the production mount applies the session-digest-keyed +// 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.RequireAuth( + 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 { @@ -70,9 +74,26 @@ func MountSessionTouchRoute(mux *http.ServeMux, authn *Authenticator, wrap ...fu mux.Handle(routeSessionTouch, h) } -// SessionTouchHandler answers 204 — the request is authenticated and CSRF- -// checked already, and the sliding read inside RequireAuth extended the -// idle deadline; there is nothing else to do. -func (a *Authenticator) SessionTouchHandler(w http.ResponseWriter, _ *http.Request) { +// 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) }