Self-heal duplicate session cookies from the Domain rollout

Browsers that logged in before the session cookie became Domain-scoped still hold the old host-only descrybe_session for api.descrybe.io. They then send BOTH cookies — older (stale) first — and Go reads the first match, so the stale relic shadows the fresh Domain cookie and every request 401s even immediately after a successful login. Clearing browser cookies fixed it manually; users should never have to. New DedupeSessionCookies middleware (mounted before scs LoadAndSave): when duplicate session cookies arrive, pick the token that resolves in the session store, rewrite the Cookie header to just that one, expire the host-only relic (Set-Cookie without Domain only touches the host-only variant), and re-issue the surviving token on the canonical Domain cookie. One request converges the browser to a single cookie. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-18 01:19:46 +02:00
parent d03c2a5c57
commit 6facfbb0aa
3 changed files with 194 additions and 0 deletions
@@ -349,6 +349,91 @@ func LoadSession(sm *scs.SessionManager) func(http.Handler) http.Handler {
 	return sm.LoadAndSave
 }

+// DedupeSessionCookies collapses duplicate session cookies into the one whose
+// token resolves in the session store, BEFORE scs reads the request.
+//
+// Why: the session cookie is Domain-scoped (Config.SessionCookieParentDomain)
+// so SvelteKit SSR on the web host receives it. Browsers that still hold the
+// old host-only cookie (pre-rollout) then send BOTH cookies, older first —
+// Go reads the first match, so a stale host-only relic shadows the fresh
+// Domain cookie and every request 401s even right after a successful login.
+// This middleware picks the token that exists in the store, rewrites the
+// Cookie header to just that one, expires the host-only relic, and re-issues
+// the surviving token on the canonical Domain cookie — users self-heal on the
+// next request instead of having to clear cookies.
+func (s *Server) DedupeSessionCookies(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		name := s.Config.SessionCookieName
+		var tokens []string
+		for _, c := range r.Cookies() {
+			if c.Name == name && c.Value != "" {
+				tokens = append(tokens, c.Value)
+			}
+		}
+		if len(tokens) <= 1 {
+			next.ServeHTTP(w, r)
+			return
+		}
+
+		chosen := ""
+		if s.Sessions != nil && s.Sessions.Store != nil {
+			for _, tok := range tokens {
+				if _, found, err := s.Sessions.Store.Find(tok); err == nil && found {
+					chosen = tok
+					break
+				}
+			}
+		}
+		if chosen == "" {
+			// No live session among the duplicates — keep the newest cookie
+			// (browsers send older cookies first) so login flows converge.
+			chosen = tokens[len(tokens)-1]
+		}
+
+		var rebuilt []string
+		for _, c := range r.Cookies() {
+			if c.Name == name {
+				continue
+			}
+			rebuilt = append(rebuilt, c.Name+"="+c.Value)
+		}
+		rebuilt = append(rebuilt, name+"="+chosen)
+		r.Header.Set("Cookie", strings.Join(rebuilt, "; "))
+
+		// Expire the host-only relic (Set-Cookie without Domain only touches the
+		// host-only variant) and re-issue the surviving token on the Domain
+		// cookie; if scs also writes the session cookie later in this response,
+		// its header comes after ours and wins.
+		secure := s.Config.CookieSecure()
+		http.SetCookie(w, &http.Cookie{
+			Name:     name,
+			Value:    "",
+			Path:     "/",
+			MaxAge:   -1,
+			HttpOnly: true,
+			Secure:   secure,
+			SameSite: http.SameSiteLaxMode,
+		})
+		if domain := s.Config.SessionCookieParentDomain(); domain != "" {
+			idle := s.Config.SessionIdleHours
+			if idle <= 0 {
+				idle = 24
+			}
+			http.SetCookie(w, &http.Cookie{
+				Name:     name,
+				Value:    chosen,
+				Path:     "/",
+				Domain:   domain,
+				MaxAge:   idle * 3600,
+				HttpOnly: true,
+				Secure:   secure,
+				SameSite: http.SameSiteLaxMode,
+			})
+		}
+		next.ServeHTTP(w, r)
+	})
+}
+
 // MaintenanceGate enforces MAINTENANCE_MODE / READ_ONLY_MODE.
 // /healthz and /readyz always pass so cutover rehearsal probes keep working.
 func (s *Server) MaintenanceGate(next http.Handler) http.Handler {
@@ -316,6 +316,9 @@ func (s *Server) Router() http.Handler {
 	r.Group(func(r chi.Router) {
 		// Maintenance/read-only before session+CSRF so freeze returns 503 (not csrf 403).
 		r.Use(s.MaintenanceGate)
+		// Collapse stale host-only + Domain session cookie duplicates before scs
+		// reads the request (see DedupeSessionCookies).
+		r.Use(s.DedupeSessionCookies)
 		r.Use(LoadSession(s.Sessions))
 		r.Use(s.CSRF)

@@ -0,0 +1,106 @@
+package httpapi
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/alexedwards/scs/v2"
+	"github.com/descrybe/descrybe-v2/apps/api/internal/config"
+)
+
+func dedupeTestServer(t *testing.T) (*Server, *scs.SessionManager) {
+	t.Helper()
+	sm := scs.New() // in-memory store
+	return &Server{
+		Config: config.Config{
+			SessionCookieName: "descrybe_session",
+			WebOrigin:         "https://descrybe.io",
+			PublicAPIURL:      "https://api.descrybe.io",
+			SessionIdleHours:  24,
+		},
+		Sessions: sm,
+	}, sm
+}
+
+func TestDedupeSessionCookies_staleShadowsValid(t *testing.T) {
+	t.Parallel()
+	s, sm := dedupeTestServer(t)
+	if err := sm.Store.Commit("valid-token", []byte("x"), time.Now().Add(time.Hour)); err != nil {
+		t.Fatal(err)
+	}
+
+	var seen string
+	h := s.DedupeSessionCookies(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		c, err := r.Cookie("descrybe_session")
+		if err != nil {
+			t.Fatalf("session cookie missing downstream: %v", err)
+		}
+		seen = c.Value
+		if n := len(r.Cookies()); n != 2 { // csrf + single session cookie
+			t.Fatalf("cookies=%d want 2 (%v)", n, r.Cookies())
+		}
+	}))
+
+	req := httptest.NewRequest(http.MethodGet, "/api/auth/me", nil)
+	// Browser order: older (stale host-only) cookie first — it used to shadow
+	// the fresh Domain cookie and 401 every request.
+	req.Header.Set("Cookie", "descrybe_session=stale-relic; descrybe_csrf=tok; descrybe_session=valid-token")
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+
+	if seen != "valid-token" {
+		t.Fatalf("downstream token=%q want valid-token", seen)
+	}
+	res := rec.Result()
+	var deleted, reissued bool
+	for _, c := range res.Cookies() {
+		if c.Name != "descrybe_session" {
+			continue
+		}
+		if c.MaxAge < 0 && c.Domain == "" {
+			deleted = true // host-only relic expired
+		}
+		if c.Value == "valid-token" && c.Domain == "descrybe.io" && c.MaxAge > 0 {
+			reissued = true // surviving token re-issued on the parent domain
+		}
+	}
+	if !deleted || !reissued {
+		t.Fatalf("want host-only deletion + domain re-issue, got %v", res.Cookies())
+	}
+}
+
+func TestDedupeSessionCookies_singleCookieUntouched(t *testing.T) {
+	t.Parallel()
+	s, _ := dedupeTestServer(t)
+	h := s.DedupeSessionCookies(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if c, err := r.Cookie("descrybe_session"); err != nil || c.Value != "only-one" {
+			t.Fatalf("cookie changed: %v %v", c, err)
+		}
+	}))
+	req := httptest.NewRequest(http.MethodGet, "/api/auth/me", nil)
+	req.Header.Set("Cookie", "descrybe_session=only-one")
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+	if n := len(rec.Result().Cookies()); n != 0 {
+		t.Fatalf("single cookie must not trigger Set-Cookie, got %v", rec.Result().Cookies())
+	}
+}
+
+func TestDedupeSessionCookies_noValidTokenKeepsNewest(t *testing.T) {
+	t.Parallel()
+	s, _ := dedupeTestServer(t)
+	var seen string
+	h := s.DedupeSessionCookies(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		c, _ := r.Cookie("descrybe_session")
+		seen = c.Value
+	}))
+	req := httptest.NewRequest(http.MethodGet, "/api/auth/me", nil)
+	req.Header.Set("Cookie", "descrybe_session=old-stale; descrybe_session=new-stale")
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+	if seen != "new-stale" {
+		t.Fatalf("want newest cookie kept, got %q", seen)
+	}
+}