Tighten session cookies to SameSite=Lax and centralize the cookie attribute policy

AGENTS.md

@@ -16,6 +16,7 @@ read the file that matches your task:
 | Upgrading a consumer past the 2026-08-18 breaking changes | [docs/agents/migration-service-user.md](docs/agents/migration-service-user.md), [docs/agents/migration-modular-gorm-stores.md](docs/agents/migration-modular-gorm-stores.md) |
 | Upgrading a consumer of the removed `preset` package (2026-08-23) | [docs/agents/migration-remove-preset.md](docs/agents/migration-remove-preset.md) |
 | Upgrading a consumer of the removed `loginid` package (2026-08-23) | [docs/agents/migration-loginid-into-user.md](docs/agents/migration-loginid-into-user.md) |
+| Session cookies tightened to `SameSite=Lax` (2026-08-23) | [docs/agents/migration-cookie-samesite.md](docs/agents/migration-cookie-samesite.md) |
 | Anything under `flow/login/` (engine, policies, methods, attempt stores, JSON transport) | [docs/agents/loginflow.md](docs/agents/loginflow.md) |
 | "Is X implemented?" — feature status and gaps | [docs/agents/features.md](docs/agents/features.md) |
 | Writing/running tests, `make verify`, coverage, lint | [docs/agents/testing.md](docs/agents/testing.md) |

TODO.md

@@ -50,9 +50,9 @@ Notes for editors:
 # Security
 
 - [ ] No CSRF protection
-  Left entirely to the consumer, with no guidance or helpers.
-- [ ] Session cookies use SameSite=None
-  Both the registry session-id cookie (auth/cookieauth/sidcookie.go) and NewCookieStore (auth/cookieauth/cookiestore.go) set SameSite=None; combined with the profile example's bodyless POST to sessionsRevokeOthers and no CSRF, any site can log a victim out everywhere. Intent is to move both to Lax in the next minor; the //nolint:gosec suppressions stay until then.
+  Left entirely to the consumer, with no guidance or helpers. SameSite=Lax (auth/cookieauth/cookieattrs.go) is now the only mitigation the library provides; it withholds the cookie from cross-site POSTs but is not a substitute for tokens.
+- [x] Session cookies use SameSite=None
+  Fixed: auth/cookieauth/cookieattrs.go is now the single source of truth for Path/HttpOnly/Secure/SameSite, applied by NewCookieStore, NewFsStore (previously gorilla's bare defaults) and the registry session-id cookie alike. Default SameSite=Lax; Cfg.SameSite opts the registry flavour back into None. One //nolint:gosec remains, on the single cookie literal, because G124 cannot prove a non-constant value. See docs/agents/migration-cookie-samesite.md.
 
 # Error handling & observability
 

auth/cookieauth/cookie.go

@@ -49,7 +49,20 @@ type Cfg struct {
 	MinWriteSpace time.Duration
 	// CookieName is the session cookie name. Defaults to DefaultCookieName.
 	CookieName string
-	Logger     *slog.Logger
+
+	// SameSite sets the SameSite attribute of the session-identifier cookie.
+	// The zero value defaults to http.SameSiteLaxMode.
+	//
+	// It belongs to the registry flavour, which builds that cookie itself; in
+	// the stateless flavour the cookie is the Store's, so set it on the Store's
+	// own Options and leave this unset.
+	//
+	// Set http.SameSiteNoneMode only for a deliberate cross-site deployment: it
+	// makes the cookie travel on cross-site POSTs, and this library ships no
+	// CSRF protection, so that protection becomes yours to add.
+	SameSite http.SameSite
+
+	Logger *slog.Logger
 }
 
 // Manager manages session storage and validation. It implements chain.AuthHandler.
@@ -62,6 +75,7 @@ type Manager struct {
 	minWriteSpace time.Duration
 	maxSessionDur time.Duration
 	cookieName    string
+	sameSite      http.SameSite
 	logger        *slog.Logger
 }
 
@@ -90,6 +104,9 @@ func New(cfg Cfg) (*Manager, error) {
 	} else if len(cfg.HashKey) > 0 || len(cfg.BlockKey) > 0 {
 		return nil, fmt.Errorf("cookieauth: HashKey and BlockKey belong to the Registry flavour; " +
 			"the Store flavour's keys are the Store's own")
+	} else if cfg.SameSite != 0 {
+		return nil, fmt.Errorf("cookieauth: SameSite belongs to the Registry flavour; " +
+			"the Store flavour's cookie is the Store's, so set store.Options.SameSite instead")
 	}
 	if cfg.SessionDur == 0 {
 		cfg.SessionDur = time.Hour
@@ -113,6 +130,7 @@ func New(cfg Cfg) (*Manager, error) {
 		minWriteSpace: cfg.MinWriteSpace,
 		maxSessionDur: cfg.MaxSessionDur,
 		cookieName:    cfg.CookieName,
+		sameSite:      resolveSameSite(cfg.SameSite),
 		store:         cfg.Store,
 		registry:      cfg.Registry,
 		logger:        cfg.Logger.With("auth-handler", SessionMngrName),
@@ -147,7 +165,7 @@ func checkRegistryCfg(cfg Cfg) error {
 	if l := len(cfg.BlockKey); l != 0 && l != 16 && l != 24 && l != 32 {
 		return fmt.Errorf("cookieauth: BlockKey length should be 16, 24 or 32 bytes, got %d", l)
 	}
-	return nil
+	return checkSameSite(cfg.SameSite)
 }
 
 // Name implements chain.AuthHandler.

auth/cookieauth/cookieattrs.go

@@ -0,0 +1,69 @@
+package cookieauth
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/gorilla/sessions"
+)
+
+// This file is the single source of truth for the attributes of every session
+// cookie this package issues: the two convenience store constructors
+// (NewCookieStore, NewFsStore) for the stateless flavour, and the
+// session-identifier cookie the Manager builds by hand for the registry
+// flavour. Both have to behave identically in a browser, which only holds if
+// there is one place that says what "identically" means.
+//
+// SameSite=Lax is the default, and it is a security decision rather than a
+// preference: this library ships no CSRF protection, and its own logout and
+// session-revocation endpoints are state-changing requests with no body to
+// validate. A cookie that travels on cross-site POSTs therefore lets any site
+// act on a visitor's session — logging them out everywhere, at minimum. Lax
+// still sends the cookie on top-level cross-site navigation, so following a
+// link into the application keeps the user logged in.
+//
+// A deliberate cross-site deployment can opt back into
+// http.SameSiteNoneMode (Cfg.SameSite, or the Store's own Options), and owns
+// the CSRF protection that choice requires.
+const defaultSameSite = http.SameSiteLaxMode
+
+// defaultStoreMaxAge is the cookie lifetime the store constructors apply: the
+// callers pass it to the store's MaxAge, so it bounds the securecookie timestamp
+// validity as well as the browser's Max-Age. The registry flavour sets Max-Age
+// per session instead, from the record's absolute deadline, but is subject to
+// the same 30-day codec ceiling (see sidcookie.go).
+const defaultStoreMaxAge = 86400 * 30
+
+// storeOptions returns the cookie policy for a stateless-flavour session store.
+// The caller may still adjust the returned Options: these are defaults, not a
+// cage.
+func storeOptions() *sessions.Options {
+	return &sessions.Options{
+		Path:     "/",
+		MaxAge:   defaultStoreMaxAge,
+		HttpOnly: true,
+		Secure:   true,
+		SameSite: defaultSameSite,
+	}
+}
+
+// resolveSameSite maps the zero value onto the package default. http.SameSite
+// has no "unset" constant — its zero value is not SameSiteDefaultMode and makes
+// net/http omit the attribute entirely — so zero is what an unconfigured Cfg
+// looks like, and it must mean Lax rather than "no policy".
+func resolveSameSite(s http.SameSite) http.SameSite {
+	if s == 0 {
+		return defaultSameSite
+	}
+	return s
+}
+
+// checkSameSite rejects a value net/http would silently drop, which would
+// downgrade the cookie to no SameSite attribute at all.
+func checkSameSite(s http.SameSite) error {
+	switch s {
+	case 0, http.SameSiteDefaultMode, http.SameSiteLaxMode, http.SameSiteStrictMode, http.SameSiteNoneMode:
+		return nil
+	}
+	return fmt.Errorf("cookieauth: SameSite value %d is not one of http.SameSite{Default,Lax,Strict,None}Mode", s)
+}

auth/cookieauth/cookieattrs_test.go

@@ -0,0 +1,226 @@
+package cookieauth_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/go-bumbu/userauth/auth/cookieauth"
+	"github.com/go-bumbu/userauth/service/session"
+	sessionmem "github.com/go-bumbu/userauth/service/session/store/memory"
+	"github.com/google/go-cmp/cmp"
+	"github.com/gorilla/securecookie"
+	"github.com/gorilla/sessions"
+)
+
+// attrs is the browser-visible part of a session cookie: the security policy,
+// without the value or the lifetime. Both flavours and both store constructors
+// must agree on it.
+type attrs struct {
+	Path     string
+	HttpOnly bool
+	Secure   bool
+	SameSite http.SameSite
+}
+
+// wantAttrs is the policy this package issues. SameSite=Lax is the point of
+// these tests: the library ships no CSRF helpers, and its own logout and
+// session-revocation endpoints are bodyless POSTs, so a cookie that travels on
+// cross-site POSTs lets any site log a victim out everywhere.
+var wantAttrs = attrs{
+	Path:     "/",
+	HttpOnly: true,
+	Secure:   true,
+	SameSite: http.SameSiteLaxMode,
+}
+
+func cookieAttrs(c *http.Cookie) attrs {
+	return attrs{Path: c.Path, HttpOnly: c.HttpOnly, Secure: c.Secure, SameSite: c.SameSite}
+}
+
+func optionAttrs(o *sessions.Options) attrs {
+	return attrs{Path: o.Path, HttpOnly: o.HttpOnly, Secure: o.Secure, SameSite: o.SameSite}
+}
+
+// newSameSiteManager builds a registry-flavour Manager with an explicit
+// SameSite, so the override can be exercised end to end.
+func newSameSiteManager(t *testing.T, sameSite http.SameSite) *cookieauth.Manager {
+	t.Helper()
+	svc, err := session.NewService(sessionmem.New(), session.Opts{})
+	if err != nil {
+		t.Fatalf("NewService: %v", err)
+	}
+	mgr, err := cookieauth.New(cookieauth.Cfg{
+		Registry: svc,
+		HashKey:  securecookie.GenerateRandomKey(64),
+		BlockKey: securecookie.GenerateRandomKey(32),
+		SameSite: sameSite,
+	})
+	if err != nil {
+		t.Fatalf("cookieauth.New: %v", err)
+	}
+	return mgr
+}
+
+// TestStoreConstructorsApplyTheCookiePolicy pins the attributes of the two
+// convenience store constructors. NewFsStore is the one that used to inherit
+// gorilla's bare defaults, i.e. no HttpOnly, no Secure and no SameSite at all.
+func TestStoreConstructorsApplyTheCookiePolicy(t *testing.T) {
+	hashKey := securecookie.GenerateRandomKey(64)
+	blockKey := securecookie.GenerateRandomKey(32)
+
+	t.Run("NewCookieStore", func(t *testing.T) {
+		store, err := cookieauth.NewCookieStore(hashKey, blockKey)
+		if err != nil {
+			t.Fatalf("NewCookieStore: %v", err)
+		}
+		if diff := cmp.Diff(wantAttrs, optionAttrs(store.Options)); diff != "" {
+			t.Errorf("cookie policy mismatch (-want +got):\n%s", diff)
+		}
+	})
+
+	t.Run("NewFsStore", func(t *testing.T) {
+		store, err := cookieauth.NewFsStore(t.TempDir(), hashKey, blockKey)
+		if err != nil {
+			t.Fatalf("NewFsStore: %v", err)
+		}
+		if diff := cmp.Diff(wantAttrs, optionAttrs(store.Options)); diff != "" {
+			t.Errorf("cookie policy mismatch (-want +got):\n%s", diff)
+		}
+	})
+}
+
+// TestBothFlavoursIssueTheSameCookiePolicy pins the parity the package claims:
+// the registry flavour builds its session-identifier cookie by hand, and it has
+// to end up indistinguishable from the stateless flavour's store-issued one.
+func TestBothFlavoursIssueTheSameCookiePolicy(t *testing.T) {
+	registry := cookieAttrs(login(t, newSameSiteManager(t, 0), "user1", false)[0])
+	stateless := cookieAttrs(login(t, newManager(t, cookieauth.Cfg{}), "user1", false)[0])
+
+	if diff := cmp.Diff(wantAttrs, registry); diff != "" {
+		t.Errorf("registry flavour cookie policy mismatch (-want +got):\n%s", diff)
+	}
+	if diff := cmp.Diff(wantAttrs, stateless); diff != "" {
+		t.Errorf("stateless flavour cookie policy mismatch (-want +got):\n%s", diff)
+	}
+	if diff := cmp.Diff(registry, stateless); diff != "" {
+		t.Errorf("the two flavours disagree (-registry +stateless):\n%s", diff)
+	}
+}
+
+// TestLogoutCookieMatchesTheLoginCookie matters mechanically, not just for
+// tidiness: a browser only overwrites a cookie when the deletion cookie carries
+// the same name, path and domain, so a mismatch leaves the session cookie in
+// place after logout.
+func TestLogoutCookieMatchesTheLoginCookie(t *testing.T) {
+	mgr := newSameSiteManager(t, 0)
+	loginCookies := login(t, mgr, "user1", false)
+
+	r := httptest.NewRequest(http.MethodPost, "/logout", nil)
+	for _, c := range loginCookies {
+		r.AddCookie(c)
+	}
+	w := httptest.NewRecorder()
+	if err := mgr.LogoutUser(r, w); err != nil {
+		t.Fatalf("LogoutUser: %v", err)
+	}
+	res := w.Result()
+	defer func() { _ = res.Body.Close() }()
+	got := res.Cookies()
+	if len(got) == 0 {
+		t.Fatal("LogoutUser set no cookie")
+	}
+
+	if diff := cmp.Diff(cookieAttrs(loginCookies[0]), cookieAttrs(got[0])); diff != "" {
+		t.Errorf("deletion cookie will not overwrite the session cookie (-login +logout):\n%s", diff)
+	}
+	if got[0].MaxAge != -1 {
+		t.Errorf("deletion cookie MaxAge = %d, want -1", got[0].MaxAge)
+	}
+}
+
+// TestCfgSameSiteOverridesTheDefault covers the registry flavour's only knob
+// for the cookie attributes: the stateless flavour can already reach them
+// through its Store's Options, this is the equivalent seam.
+func TestCfgSameSiteOverridesTheDefault(t *testing.T) {
+	tcs := []struct {
+		name string
+		cfg  http.SameSite
+		want http.SameSite
+	}{
+		{name: "zero value defaults to Lax", cfg: 0, want: http.SameSiteLaxMode},
+		{name: "explicit Lax", cfg: http.SameSiteLaxMode, want: http.SameSiteLaxMode},
+		{name: "Strict", cfg: http.SameSiteStrictMode, want: http.SameSiteStrictMode},
+		{name: "None for a deliberate cross-site deployment", cfg: http.SameSiteNoneMode, want: http.SameSiteNoneMode},
+	}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			mgr := newSameSiteManager(t, tc.cfg)
+			c := login(t, mgr, "user1", false)[0]
+			if c.SameSite != tc.want {
+				t.Errorf("login cookie SameSite = %v, want %v", c.SameSite, tc.want)
+			}
+			if !c.Secure {
+				// SameSite=None is ignored by browsers without Secure, and the
+				// other modes have no reason to drop it either.
+				t.Error("login cookie is not Secure")
+			}
+
+			// The deletion cookie has to carry the override too, or it will not
+			// overwrite the cookie the override produced.
+			w := httptest.NewRecorder()
+			r := httptest.NewRequest(http.MethodPost, "/logout", nil)
+			r.AddCookie(c)
+			if err := mgr.LogoutUser(r, w); err != nil {
+				t.Fatalf("LogoutUser: %v", err)
+			}
+			res := w.Result()
+			defer func() { _ = res.Body.Close() }()
+			deletion := res.Cookies()
+			if len(deletion) == 0 {
+				t.Fatal("LogoutUser set no cookie")
+			}
+			if deletion[0].SameSite != tc.want {
+				t.Errorf("deletion cookie SameSite = %v, want %v", deletion[0].SameSite, tc.want)
+			}
+		})
+	}
+}
+
+// TestNewRejectsSameSiteWithoutRegistry keeps the flavour boundary honest: in
+// the stateless flavour the cookie attributes belong to the Store, so a
+// SameSite set here would be silently ignored.
+func TestNewRejectsSameSiteWithoutRegistry(t *testing.T) {
+	store, err := cookieauth.NewCookieStore(securecookie.GenerateRandomKey(64), securecookie.GenerateRandomKey(32))
+	if err != nil {
+		t.Fatalf("NewCookieStore: %v", err)
+	}
+	_, err = cookieauth.New(cookieauth.Cfg{Store: store, SameSite: http.SameSiteStrictMode})
+	if err == nil {
+		t.Fatal("expected an error when SameSite is set in the stateless flavour")
+	}
+	if !strings.Contains(err.Error(), "SameSite") {
+		t.Errorf("error should name the offending field, got: %v", err)
+	}
+}
+
+// TestNewRejectsInvalidSameSite guards against a garbage value silently
+// producing a cookie with no SameSite attribute at all.
+func TestNewRejectsInvalidSameSite(t *testing.T) {
+	svc, err := session.NewService(sessionmem.New(), session.Opts{})
+	if err != nil {
+		t.Fatalf("NewService: %v", err)
+	}
+	_, err = cookieauth.New(cookieauth.Cfg{
+		Registry: svc,
+		HashKey:  securecookie.GenerateRandomKey(64),
+		SameSite: http.SameSite(99),
+	})
+	if err == nil {
+		t.Fatal("expected an error for an out-of-range SameSite value")
+	}
+	if !strings.Contains(err.Error(), "SameSite") {
+		t.Errorf("error should name the offending field, got: %v", err)
+	}
+}