diff --git a/DECISIONS.md b/DECISIONS.md index c65a027..0c6d1e8 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1128,6 +1128,40 @@ Spec: `docs/specs/onboarding-wow-burst.md`. --- +## ADR-029 — Stateless session cookie (survives restarts, browser-close, idle) + +**Status:** Accepted (2026-06-11). Triggered by pilot feedback: "lots of clicking to log in again +on iPhone." Supersedes the in-memory session store in the ADR-011 login. + +**Context.** Three compounding causes made users re-login constantly: +1. **In-memory session store** (`sessionStore` map) — wiped on every pod restart, so each deploy + logged everyone out. During the active build period that was ~15 logouts. +2. **1-hour session TTL** — for a "check back tomorrow" reader, idle > 1h forced a re-login on + nearly every visit. +3. **No cookie Max-Age** — a session cookie (deleted on browser/app close); iPhone Safari closing + the tab dropped it. +Each re-login is the full Dex/Authentik redirect dance — many taps on mobile. + +**Decision.** Make the session **stateless**: the identity (subject + email) and an absolute +expiry live INSIDE the existing HMAC-signed (HS256) cookie — no server-side table. Plus: +- **30-day sliding TTL** (was 1h), re-signed on each request so an active user never lapses. +- **Persistent cookie** (`Max-Age` set) so it survives browser/app close. +The cookie is HttpOnly + Secure + SameSite=Lax; the HMAC (keyed by the stable ESO +`tapir-session-secret`, which does NOT rotate per deploy) makes it tamper-proof. The payload is +identity, not secrets — the OIDC access/ID tokens are still discarded after callback. + +**Consequences.** A deploy/restart no longer logs anyone out (proven by a test: a cookie issued by +one instance is accepted by a fresh instance with the same secret); works across replicas for +free. **Trade:** no server-side revocation — `logout` clears the cookie client-side, but a copied +cookie stays valid until expiry. Accepted for the Stage-0 reader pilot; revisit (server-side +revocation list, or shorter TTL + refresh) if it ever holds sensitive actions. Rotating +`tapir-session-secret` invalidates all sessions — the global logout lever. + +**Not addressed here:** the tap-count of the IdP login page itself is Authentik's UX; with +re-login now rare (30-day idle or explicit logout), it matters far less. + +--- + ## Rejected alternatives Approaches considered during the 2026-06-02 planning + grill session and **deliberately not diff --git a/internal/web/oidc/oidc.go b/internal/web/oidc/oidc.go index ca3c956..64d20a4 100644 --- a/internal/web/oidc/oidc.go +++ b/internal/web/oidc/oidc.go @@ -7,10 +7,11 @@ // Authentication is real (Dex OIDC) and is the only gate: any Dex-authenticated // subject may sign in (ADR-012 dropped ADR-011's single-subject allowlist). // Authorization/registration is layered on top in internal/web (an authenticated -// subject with no tapir user is routed to registration). Sessions are server-side -// (in-memory, fine for the single Stage-1 replica) addressed by an HMAC-signed -// (HS256) HttpOnly Secure SameSite=Lax cookie with a short TTL and sliding -// refresh. Tokens are never logged. +// subject with no tapir user is routed to registration). Sessions are STATELESS +// (ADR-029): the identity + expiry live inside an HMAC-signed (HS256) HttpOnly +// Secure SameSite=Lax persistent cookie with a long sliding TTL — no server-side +// table, so a deploy/restart never logs anyone out and the cookie also survives +// browser-close. Tokens are never logged; logout clears the cookie client-side. // // This is mcp-chassis's cousin but NOT the same code: mcp-chassis validates // inbound Bearer JWTs for MCP APIs; this is a browser session login. @@ -47,7 +48,11 @@ type Config struct { } const ( - defaultSessionTTL = time.Hour + // defaultSessionTTL is generous and sliding: Tapir is a "check back tomorrow" + // reader, so a short TTL meant a re-login (full IdP redirect dance) on almost + // every visit. 30 days, slid forward on each request, keeps a regular user + // logged in indefinitely while an abandoned session still lapses. + defaultSessionTTL = 30 * 24 * time.Hour pendingTTL = 10 * time.Minute sessionCookie = "tapir_session" loginPath = "/auth/login" @@ -59,7 +64,6 @@ type DexAuth struct { oauth *oauth2.Config verifier *oidc.IDTokenVerifier - sessions *sessionStore pending *pendingStore secret []byte sessionTTL time.Duration @@ -127,7 +131,6 @@ func New(ctx context.Context, cfg Config, opts ...Option) (*DexAuth, error) { RedirectURL: cfg.RedirectURL, Scopes: []string{oidc.ScopeOpenID, "profile", "email"}, }, - sessions: newSessionStore(), pending: newPendingStore(), secret: []byte(cfg.SessionSecret), sessionTTL: defaultSessionTTL, @@ -159,31 +162,31 @@ func (d *DexAuth) Middleware(h http.Handler) http.Handler { h.ServeHTTP(w, r) return } - sid, ok := d.sessionID(r) + c, err := r.Cookie(sessionCookie) + if err != nil { + d.redirectUnauthenticated(w, r) + return + } + user, _, ok := d.decodeSession(c.Value, d.now()) if !ok { d.redirectUnauthenticated(w, r) return } - if _, ok := d.sessions.get(sid, d.now()); !ok { - d.redirectUnauthenticated(w, r) - return - } - d.sessions.refresh(sid, d.now().Add(d.sessionTTL)) // sliding refresh + // Sliding refresh: re-issue the cookie with a fresh expiry so an active + // user never lapses (the expiry lives in the cookie, so sliding = re-sign). + d.setSessionCookie(w, d.encodeSession(user, d.now().Add(d.sessionTTL))) h.ServeHTTP(w, r) }) } -// CurrentUser resolves the authenticated principal from the session cookie. +// CurrentUser resolves the authenticated principal from the stateless cookie. func (d *DexAuth) CurrentUser(r *http.Request) (web.User, bool) { - sid, ok := d.sessionID(r) - if !ok { + c, err := r.Cookie(sessionCookie) + if err != nil { return web.User{}, false } - data, ok := d.sessions.get(sid, d.now()) - if !ok { - return web.User{}, false - } - return data.user, true + user, _, ok := d.decodeSession(c.Value, d.now()) + return user, ok } func (d *DexAuth) handleLogin(w http.ResponseWriter, r *http.Request) { @@ -248,23 +251,15 @@ func (d *DexAuth) handleCallback(w http.ResponseWriter, r *http.Request) { } _ = idToken.Claims(&claims) // email is best-effort; subject is the identity - sid, err := randToken() - if err != nil { - http.Error(w, "internal error", http.StatusInternalServerError) - return - } - d.sessions.put(sid, sessionData{ - user: web.User{Subject: idToken.Subject, Email: claims.Email}, - expiry: d.now().Add(d.sessionTTL), - }) - d.setSessionCookie(w, sid) + user := web.User{Subject: idToken.Subject, Email: claims.Email} + d.setSessionCookie(w, d.encodeSession(user, d.now().Add(d.sessionTTL))) http.Redirect(w, r, "/", http.StatusFound) } func (d *DexAuth) handleLogout(w http.ResponseWriter, r *http.Request) { - if sid, ok := d.sessionID(r); ok { - d.sessions.delete(sid) - } + // Stateless sessions: clearing the cookie logs the browser out. There is no + // server-side record to delete (ADR-029); a copy of the cookie stays valid + // until its expiry — an accepted trade for the Stage-0 reader app. d.clearSessionCookie(w) // Land on the public landing page, not the login endpoint: a just-logged-out // visitor should see /welcome, not be bounced straight back into a Dex login. @@ -287,22 +282,19 @@ func (d *DexAuth) redirectToLogin(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, loginPath, http.StatusFound) } -func (d *DexAuth) sessionID(r *http.Request) (string, bool) { - c, err := r.Cookie(sessionCookie) - if err != nil { - return "", false - } - return d.unsign(c.Value) -} - -func (d *DexAuth) setSessionCookie(w http.ResponseWriter, sid string) { +// setSessionCookie writes the signed session value as a PERSISTENT cookie +// (Max-Age set), so it survives the browser/app being closed — a session cookie +// (no Max-Age) was dropped on iPhone Safari close, forcing re-login. value is the +// already-signed payload from encodeSession. +func (d *DexAuth) setSessionCookie(w http.ResponseWriter, value string) { http.SetCookie(w, &http.Cookie{ Name: sessionCookie, - Value: d.sign(sid), + Value: value, Path: "/", HttpOnly: true, Secure: !d.insecure, SameSite: http.SameSiteLaxMode, + MaxAge: int(d.sessionTTL.Seconds()), }) } diff --git a/internal/web/oidc/oidc_test.go b/internal/web/oidc/oidc_test.go index e0ca192..af0540c 100644 --- a/internal/web/oidc/oidc_test.go +++ b/internal/web/oidc/oidc_test.go @@ -306,13 +306,42 @@ func TestLogoutClearsSession(t *testing.T) { require.Equal(t, http.StatusFound, rec.Code) require.Equal(t, "/welcome", rec.Header().Get("Location"), "logout lands on the public page") cleared := sessionCookie(t, rec.Result()) - require.Less(t, cleared.MaxAge, 0, "logout expires the cookie") + require.Less(t, cleared.MaxAge, 0, "logout expires the cookie so the browser drops it") + require.Empty(t, cleared.Value, "logout blanks the cookie value") - // The server-side session is gone: the original cookie no longer resolves. + // Sessions are stateless (ADR-029): logout clears the cookie client-side, so a + // request carrying the cleared (empty) cookie is unauthenticated. The original + // signed cookie remains technically valid until its expiry — the accepted + // trade for no server-side store; the browser no longer holds it. check := httptest.NewRequest(http.MethodGet, "/", nil) - check.AddCookie(cookie) + check.AddCookie(cleared) _, ok := auth.CurrentUser(check) - require.False(t, ok) + require.False(t, ok, "the cleared cookie does not authenticate") +} + +// TestSessionSurvivesRestart is the core of ADR-029: a cookie issued by one +// process is accepted by a FRESH instance with the same session secret — so a +// deploy/pod-restart no longer logs users out (the old in-memory store did). +func TestSessionSurvivesRestart(t *testing.T) { + f := newFakeIssuer(t) + auth1 := newAuth(t, f) + cookie := authenticate(t, auth1, f) + + auth2 := newAuth(t, f) // simulate a redeploy: new process, same SessionSecret + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.AddCookie(cookie) + user, ok := auth2.CurrentUser(req) + require.True(t, ok, "a session must survive a restart (stateless signed cookie)") + require.Equal(t, testSubject, user.Subject) +} + +// TestSessionCookieIsPersistent: the cookie carries a positive Max-Age so it +// survives the browser/app being closed (a session cookie was dropped on iOS). +func TestSessionCookieIsPersistent(t *testing.T) { + f := newFakeIssuer(t) + auth := newAuth(t, f) + cookie := authenticate(t, auth, f) + require.Greater(t, cookie.MaxAge, 0, "session cookie must be persistent (Max-Age set)") } func TestExpiredSessionRejected(t *testing.T) { diff --git a/internal/web/oidc/session.go b/internal/web/oidc/session.go index 45f1100..7c4ae14 100644 --- a/internal/web/oidc/session.go +++ b/internal/web/oidc/session.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "encoding/base64" "encoding/hex" + "encoding/json" "fmt" "strings" "sync" @@ -53,56 +54,44 @@ func (d *DexAuth) unsign(signed string) (string, bool) { return value, true } -// sessionData is the server-side session record. -type sessionData struct { - user web.User - expiry time.Time +// sessionClaims is the self-contained session payload carried INSIDE the signed +// cookie — there is no server-side session table. This is deliberate (ADR-029): +// an in-memory store was wiped on every pod restart, logging every user out on +// each deploy, and a stateless cookie also survives browser-close and works +// across replicas. It holds only the identity (subject + email, not secret) and +// an absolute expiry; the HMAC tag (sign/unsign) makes it tamper-proof. +type sessionClaims struct { + Sub string `json:"s"` + Email string `json:"e"` + Exp int64 `json:"x"` // unix seconds; absolute expiry } -// sessionStore is an in-memory session table. Single replica at Stage 0, so an -// in-process map is sufficient; it is safe for concurrent use. -type sessionStore struct { - mu sync.Mutex - m map[string]sessionData +// encodeSession produces the signed cookie value for a user with the given expiry. +func (d *DexAuth) encodeSession(u web.User, exp time.Time) string { + b, _ := json.Marshal(sessionClaims{Sub: u.Subject, Email: u.Email, Exp: exp.Unix()}) + return d.sign(base64.RawURLEncoding.EncodeToString(b)) } -func newSessionStore() *sessionStore { return &sessionStore{m: make(map[string]sessionData)} } - -func (s *sessionStore) put(id string, d sessionData) { - s.mu.Lock() - defer s.mu.Unlock() - s.m[id] = d -} - -// get returns the session if present and unexpired; expired entries are evicted. -func (s *sessionStore) get(id string, now time.Time) (sessionData, bool) { - s.mu.Lock() - defer s.mu.Unlock() - d, ok := s.m[id] +// decodeSession verifies the cookie's HMAC, parses the claims, and checks expiry. +// It returns the user and the absolute expiry on success. +func (d *DexAuth) decodeSession(cookieValue string, now time.Time) (web.User, time.Time, bool) { + payload, ok := d.unsign(cookieValue) if !ok { - return sessionData{}, false + return web.User{}, time.Time{}, false } - if !now.Before(d.expiry) { - delete(s.m, id) - return sessionData{}, false + raw, err := base64.RawURLEncoding.DecodeString(payload) + if err != nil { + return web.User{}, time.Time{}, false } - return d, true -} - -// refresh slides an existing session's expiry forward; a no-op for unknown ids. -func (s *sessionStore) refresh(id string, expiry time.Time) { - s.mu.Lock() - defer s.mu.Unlock() - if d, ok := s.m[id]; ok { - d.expiry = expiry - s.m[id] = d + var c sessionClaims + if err := json.Unmarshal(raw, &c); err != nil { + return web.User{}, time.Time{}, false } -} - -func (s *sessionStore) delete(id string) { - s.mu.Lock() - defer s.mu.Unlock() - delete(s.m, id) + exp := time.Unix(c.Exp, 0) + if !now.Before(exp) { + return web.User{}, time.Time{}, false // expired + } + return web.User{Subject: c.Sub, Email: c.Email}, exp, true } // pendingData holds the nonce bound to an in-flight authorization request.