agent build

go-api/internal/httpserver/samesite_test.go

@@ -0,0 +1,133 @@
+package httpserver_test
+
+import (
+	"io"
+	"log/slog"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/krow/krow-backend/go-api/internal/config"
+	"github.com/krow/krow-backend/go-api/internal/db"
+	"github.com/krow/krow-backend/go-api/internal/httpserver"
+	"github.com/krow/krow-backend/go-api/internal/testutil"
+)
+
+// The session cookie's SameSite mode.
+//
+// This exists because the mode is a security decision that nothing else in the
+// suite observes, and because it was silently unreadable for a release: config
+// parsed and validated HTTP_COOKIE_SAMESITE and no code path consulted it, so
+// a deployment that set `lax` got `none` and lost its only CSRF protection.
+//
+// CORS and SameSite answer different questions. CORS is about ORIGIN;
+// SameSite is about SITE. A frontend on platform.krowforce.com calling
+// mcp.krowforce.com is cross-origin — it needs the allowlist — and same-site,
+// so a Lax cookie reaches it regardless. Deriving None from "an allowlist
+// exists" is therefore a guess, and these tests pin who gets the final word.
+
+// sameSiteFor builds a server with the given cookie and CORS configuration and
+// reports the SameSite attribute it writes. Read off the logout response,
+// because clearSessionCookie writes the same attributes the login path does and
+// needs no credentials to reach.
+func sameSiteFor(t *testing.T, h *testutil.Harness, configured string, origins []string) string {
+	t.Helper()
+	cfg := &config.Config{
+		AppEnv: "production",
+		HTTP: config.HTTPConfig{
+			Host: "127.0.0.1", Port: 0, ShutdownTimeout: time.Second,
+			CookieSameSite: configured,
+			CORSOrigins:    origins,
+		},
+		DB: config.DBConfig{Schema: "public"},
+	}
+	log := slog.New(slog.NewTextHandler(io.Discard, nil))
+	srv, err := httpserver.New(cfg, &db.DB{Pool: h.Pool, Schema: "public"}, log)
+	if err != nil {
+		t.Fatalf("build the server: %v", err)
+	}
+
+	rec := httptest.NewRecorder()
+	srv.Handler().ServeHTTP(rec, httptest.NewRequest("POST", "/api/v1/auth/logout", nil))
+
+	for _, c := range rec.Header().Values("Set-Cookie") {
+		if !strings.HasPrefix(c, sessionCookie+"=") {
+			continue
+		}
+		for _, part := range strings.Split(c, ";") {
+			part = strings.TrimSpace(part)
+			if v, ok := strings.CutPrefix(part, "SameSite="); ok {
+				return v
+			}
+		}
+		return "(absent)"
+	}
+	return "(no cookie)"
+}
+
+func TestSessionCookieSameSite(t *testing.T) {
+	h := testutil.New(t)
+	origins := []string{"https://platform.krowforce.com"}
+
+	cases := []struct {
+		name       string
+		configured string
+		origins    []string
+		want       string
+	}{
+		// The deployment this was written for: CORS is genuinely required
+		// (cross-origin) and Lax is genuinely correct (same-site). Before the
+		// fix this combination was unreachable.
+		{"explicit lax survives a CORS allowlist", "lax", origins, "Lax"},
+		{"explicit none is honoured", "none", nil, "None"},
+		{"explicit strict is honoured", "strict", origins, "Strict"},
+
+		// Unset: the allowlist decides, which is the behaviour b6f8655
+		// introduced and the right default for an unconfigured deployment.
+		{"unset with an allowlist defaults to None", "", origins, "None"},
+		{"unset with no allowlist defaults to Lax", "", nil, "Lax"},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := sameSiteFor(t, h, c.configured, c.origins); got != c.want {
+				t.Fatalf("SameSite=%s, want %s", got, c.want)
+			}
+		})
+	}
+}
+
+// SameSite=None is meaningless without Secure — browsers reject the pairing
+// outright, so the cookie would simply never be stored.
+func TestSameSiteNoneAlwaysCarriesSecure(t *testing.T) {
+	h := testutil.New(t)
+	cfg := &config.Config{
+		AppEnv: "development", // Secure would otherwise be off
+		HTTP: config.HTTPConfig{
+			Host: "127.0.0.1", Port: 0, ShutdownTimeout: time.Second,
+			CookieSameSite: "none",
+		},
+		DB: config.DBConfig{Schema: "public"},
+	}
+	log := slog.New(slog.NewTextHandler(io.Discard, nil))
+	srv, err := httpserver.New(cfg, &db.DB{Pool: h.Pool, Schema: "public"}, log)
+	if err != nil {
+		t.Fatalf("build the server: %v", err)
+	}
+	rec := httptest.NewRecorder()
+	srv.Handler().ServeHTTP(rec, httptest.NewRequest("POST", "/api/v1/auth/logout", nil))
+
+	var cookie string
+	for _, c := range rec.Header().Values("Set-Cookie") {
+		if strings.HasPrefix(c, sessionCookie+"=") {
+			cookie = c
+		}
+	}
+	if cookie == "" {
+		t.Fatal("no session cookie written")
+	}
+	if !strings.Contains(cookie, "SameSite=None") || !strings.Contains(cookie, "Secure") {
+		t.Fatalf("SameSite=None must be paired with Secure, got %q", cookie)
+	}
+}