diff --git a/apps/backend/AGENTS.md b/apps/backend/AGENTS.md index 29b25fbe86..c510ce1153 100644 --- a/apps/backend/AGENTS.md +++ b/apps/backend/AGENTS.md @@ -170,7 +170,7 @@ Standalone agentctl is launched in its own process group so terminal Ctrl+C is h **Remote SSH lifecycle:** Resolve and run remote preparation, then verify the canonical checkout, origin, and `HEAD` before starting agentctl. Retain the remote task directory in session state for stop/resume; terminal archive/delete/cascade cleanup runs over the live SSH connection before teardown, while ordinary stop and backend shutdown preserve the workspace. -**Opt-in authentication & per-user scoping** (`internal/auth/`): auth is OFF by default; the global middleware (`auth/httpmw`, installed after CORS in `backendapp.buildHTTPServer`) then injects a synthetic admin identity for the pre-auth single user, so behavior is unchanged. Enablement is the `features.auth` runtime flag (`KANDEV_FEATURES_AUTH`, Settings > System > Feature Toggles) — the auth service derives its mode from `cfg.Features.Auth` (`disabled` / `setup` = flag-on-no-admin / `enabled` = flag-on-admin-exists); there is no separate `auth.mode` setting. When enabled, requests authenticate via a `kandev_session` cookie (opaque token, SHA-256 at rest, DB-backed) or a `kandev_pat_*` bearer token. Scoping rules: +**Opt-in authentication & per-user scoping** (`internal/auth/`): auth is OFF by default; the global middleware (`auth/httpmw`, installed after CORS in `backendapp.buildHTTPServer`) then injects a synthetic admin identity for the pre-auth single user, so behavior is unchanged. Enablement is the `features.auth` runtime flag (`KANDEV_FEATURES_AUTH`, Settings > System > Feature Toggles) — the auth service derives its mode from `cfg.Features.Auth` (`disabled` / `setup` = flag-on-no-admin / `enabled` = flag-on-admin-exists); there is no separate `auth.mode` setting. When enabled, requests authenticate via the session cookie (base name `kandev_session`; the effective name is derived from the request host — port-scoped on a ported host via `internal/common/httpcookie`, plain on a default-port host) or a `kandev_pat_*` bearer token. Scoping rules: - **Identity travels in the request context** (`authn.IdentityFromContext`). No identity = internal caller (pollers, event bus, office schedulers) = unscoped. Synthetic identity = auth disabled = unscoped. - **In-session agent MCP is scoped to the task owner.** The MCP tools an agent gets inside its own session are relayed over the agent's WebSocket stream, which carries no credential of its own. `internal/mcp/scope` resolves the stream's task → workspace → owner and attaches that user's real identity before dispatch (`lifecycle.Manager.SetMCPIdentityScoper`), so the same `authorize*` checks apply as for the PAT-authenticated `/mcp` endpoint. The owning task comes from the `AgentExecution`, never from the agent-supplied payload — do not "improve" this by reading `session_id`/`task_id` out of the request. Tool handlers stay identity-agnostic. Under enforced auth every dispatch is scoped to *somebody*: a resolvable active owner gets their identity; an unowned workspace gets a sentinel user ID that reaches unowned rows only; anything unresolvable (missing task/workspace row, or an owner whose account was deleted or disabled) is **denied**. Never return an identity-free context from this path — the task service reads that as an internal caller and grants everything. - **Workspaces are per-user** (`workspaces.owner_id`); the task service filters/denies at the service layer with `*NotFound` sentinels (no existence leak). Unowned rows (`owner_id=''`) stay visible until the setup wizard claims them for the admin. diff --git a/apps/backend/internal/auth/httpapi/handlers.go b/apps/backend/internal/auth/httpapi/handlers.go index 4d5d758b64..1c33af645c 100644 --- a/apps/backend/internal/auth/httpapi/handlers.go +++ b/apps/backend/internal/auth/httpapi/handlers.go @@ -93,7 +93,7 @@ func (h *Handlers) setup(c *gin.Context) { h.writeAuthError(c, err) return } - setSessionCookie(c, h.svc.CookieName(), token, h.svc.SessionTTL()) + setSessionCookie(c, h.svc.CookieNameForRequest(c.Request), token, h.svc.SessionTTL()) c.JSON(http.StatusOK, gin.H{"user": user}) } @@ -108,10 +108,14 @@ func (h *Handlers) login(c *gin.Context) { h.writeAuthError(c, err) return } - setSessionCookie(c, h.svc.CookieName(), token, h.svc.SessionTTL()) + setSessionCookie(c, h.svc.CookieNameForRequest(c.Request), token, h.svc.SessionTTL()) c.JSON(http.StatusOK, gin.H{"user": user}) } +// logout terminates the current session (if any) and clears its port-scoped +// session cookie. The unprefixed base name is deliberately left untouched — +// on a host serving a default-port instance it is that instance's live +// session cookie (see spec: no proactive legacy scrubbing). func (h *Handlers) logout(c *gin.Context) { identity, ok := authn.FromGin(c) if ok && identity.SessionID != "" { @@ -120,7 +124,13 @@ func (h *Handlers) logout(c *gin.Context) { return } } - clearSessionCookie(c, h.svc.CookieName()) + // Clear only the effective (port-scoped) name. The unprefixed base name is + // deliberately left alone: on a host serving a default-port instance (or a + // not-yet-upgraded one), it is that instance's LIVE session cookie, not an + // inert legacy jar — expiring it would log the other instance out (see + // docs/specs/fix-multi-instance-cookie-isolation/spec.md: the upgraded + // instance does not proactively delete the legacy cookie). + clearSessionCookie(c, h.svc.CookieNameForRequest(c.Request)) c.Status(http.StatusNoContent) } diff --git a/apps/backend/internal/auth/httpapi/handlers_admin.go b/apps/backend/internal/auth/httpapi/handlers_admin.go index 5bbddb10cf..b362c0ddca 100644 --- a/apps/backend/internal/auth/httpapi/handlers_admin.go +++ b/apps/backend/internal/auth/httpapi/handlers_admin.go @@ -128,7 +128,7 @@ func (h *Handlers) acceptInvite(c *gin.Context) { h.writeAuthError(c, err) return } - setSessionCookie(c, h.svc.CookieName(), token, h.svc.SessionTTL()) + setSessionCookie(c, h.svc.CookieNameForRequest(c.Request), token, h.svc.SessionTTL()) c.JSON(http.StatusOK, gin.H{"user": user}) } diff --git a/apps/backend/internal/auth/httpapi/handlers_test.go b/apps/backend/internal/auth/httpapi/handlers_test.go index 8f647af978..4251c17998 100644 --- a/apps/backend/internal/auth/httpapi/handlers_test.go +++ b/apps/backend/internal/auth/httpapi/handlers_test.go @@ -12,23 +12,31 @@ import ( "github.com/gin-gonic/gin" "github.com/jmoiron/sqlx" _ "github.com/mattn/go-sqlite3" + "go.uber.org/zap" "github.com/kandev/kandev/internal/auth" authhttpmw "github.com/kandev/kandev/internal/auth/httpmw" authstore "github.com/kandev/kandev/internal/auth/store" "github.com/kandev/kandev/internal/common/config" + commonhttpmw "github.com/kandev/kandev/internal/common/httpmw" + "github.com/kandev/kandev/internal/common/logger" userstore "github.com/kandev/kandev/internal/user/store" ) -// newAPIFixture builds the full production HTTP stack for auth: global -// middleware + auth API routes on one router. authEnabled maps to the -// features.auth flag (on ⇒ setup mode until the wizard runs). The router -// mirrors production by default: gin.New() trusts all proxies, so -// SetTrustedProxies is called with the variadic trusted list (nil clears the -// insecure default) exactly like buildHTTPServer does. -func newAPIFixture(t *testing.T, authEnabled bool, trustedProxies ...string) (*gin.Engine, *auth.Service) { +// testLogger returns a discard logger for fixtures that must pass one. +func testLogger(t *testing.T) *logger.Logger { + t.Helper() + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + t.Fatalf("nop logger: %v", err) + } + return log +} + +// newAuthService builds an auth service from cfg over an in-memory store +// pair. Shared by the plain and CORS-gated fixtures. +func newAuthService(t *testing.T, cfg *config.Config) *auth.Service { t.Helper() - gin.SetMode(gin.TestMode) conn, err := sqlx.Open("sqlite3", ":memory:") if err != nil { t.Fatalf("open sqlite: %v", err) @@ -43,15 +51,35 @@ func newAPIFixture(t *testing.T, authEnabled bool, trustedProxies ...string) (*g if err != nil { t.Fatalf("auth store: %v", err) } - cfg := &config.Config{} - cfg.Features.Auth = authEnabled - cfg.Auth.SessionTTLHours = 720 svc, err := auth.NewService(context.Background(), auth.Deps{ Cfg: cfg, Store: store, Users: users, }) if err != nil { t.Fatalf("auth service: %v", err) } + return svc +} + +// newAPIFixture builds the full production HTTP stack for auth: global +// middleware + auth API routes on one router. authEnabled maps to the +// features.auth flag (on ⇒ setup mode until the wizard runs). The router +// mirrors production by default: gin.New() trusts all proxies, so +// SetTrustedProxies is called with the variadic trusted list (nil clears the +// insecure default) exactly like buildHTTPServer does. +func newAPIFixture(t *testing.T, authEnabled bool, trustedProxies ...string) (*gin.Engine, *auth.Service) { + t.Helper() + cfg := &config.Config{} + cfg.Features.Auth = authEnabled + cfg.Auth.SessionTTLHours = 720 + return newAPIFixtureWithCfg(t, cfg, trustedProxies...) +} + +// newAPIFixtureWithCfg is newAPIFixture with an explicit config (custom +// auth.cookieName, loaded defaults, …). +func newAPIFixtureWithCfg(t *testing.T, cfg *config.Config, trustedProxies ...string) (*gin.Engine, *auth.Service) { + t.Helper() + gin.SetMode(gin.TestMode) + svc := newAuthService(t, cfg) router := gin.New() if len(trustedProxies) == 0 { trustedProxies = nil @@ -64,10 +92,40 @@ func newAPIFixture(t *testing.T, authEnabled bool, trustedProxies ...string) (*g return router, svc } +// newCORSGatedAPIFixture mirrors the production stack: the CORS/WS origin +// gate (httpmw.AllowedOrigin, the same decision branch as +// newCORSGatedAPIFixture mirrors the production chain for the reverse-proxy +// contract tests: the CORS origin gate (as in backendapp.corsMiddleware) runs +// before the auth middleware, as in buildHTTPServer, and — like +// buildHTTPServer — X-Forwarded-Host is honored only when the request's peer +// is in the given trusted-proxy list (mirroring KANDEV_TRUSTED_PROXIES). +func newCORSGatedAPIFixture(t *testing.T, cfg *config.Config, trustedProxies ...string) (*gin.Engine, *auth.Service) { + t.Helper() + gin.SetMode(gin.TestMode) + svc := newAuthService(t, cfg) + router := gin.New() + router.Use(authhttpmw.StripUntrustedForwardedHost(trustedProxies, testLogger(t))) + router.Use(func(c *gin.Context) { + if origin := c.GetHeader("Origin"); origin != "" { + if !commonhttpmw.AllowedOrigin(origin, c.Request.Host) { + c.AbortWithStatus(http.StatusForbidden) + return + } + } + c.Next() + }) + router.Use(authhttpmw.Middleware(svc)) + RegisterRoutes(router, svc, nil) + return router, svc +} + type apiClient struct { t *testing.T router *gin.Engine cookie *http.Cookie + // cookiePrefix filters the session Set-Cookie names this client captures + // (default "kandev_session"); custom-name tests override it. + cookiePrefix string } func (c *apiClient) do(method, path string, body any, mutate ...func(*http.Request)) *httptest.ResponseRecorder { @@ -93,8 +151,12 @@ func (c *apiClient) do(method, path string, body any, mutate ...func(*http.Reque rec := httptest.NewRecorder() c.router.ServeHTTP(rec, req) // Capture session cookie updates (login/setup/logout). + prefix := c.cookiePrefix + if prefix == "" { + prefix = "kandev_session" + } for _, cookie := range rec.Result().Cookies() { - if strings.Contains(cookie.Name, "kandev_session") { + if strings.Contains(cookie.Name, prefix) { if cookie.MaxAge < 0 { c.cookie = nil } else { @@ -105,6 +167,47 @@ func (c *apiClient) do(method, path string, body any, mutate ...func(*http.Reque return rec } +// assertSessionCookieNames asserts the response's session cookies (names +// containing filter) are exactly want, in order — a positive-only assertion +// would miss an implementation emitting both the scoped and unprefixed names. +func assertSessionCookieNames(t *testing.T, rec *httptest.ResponseRecorder, filter string, want ...string) { + t.Helper() + var got []string + for _, cookie := range rec.Result().Cookies() { + if strings.Contains(cookie.Name, filter) { + got = append(got, cookie.Name) + } + } + if !equalStrings(got, want) { + t.Fatalf("session Set-Cookie names = %v, want %v", got, want) + } +} + +// equalStrings reports whether two string slices are identical, in order. +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +// setupOn logs the admin in via the setup wizard on the given request Host. +func setupOn(t *testing.T, client *apiClient, host string) *httptest.ResponseRecorder { + t.Helper() + rec := client.do(http.MethodPost, "/api/v1/auth/setup", map[string]any{ + "email": "admin@x.dev", "password": "adminpass123", "display_name": "Admin", + }, func(req *http.Request) { req.Host = host }) + if rec.Code != http.StatusOK { + t.Fatalf("setup: %d body=%s", rec.Code, rec.Body.String()) + } + return rec +} + // loginSessionIP bootstraps the admin via setup, performs a login request // with the given transport attributes, and returns the stored IP of the // current session (the login cookie is current; the setup session stays in @@ -363,3 +466,396 @@ func TestValidationErrors(t *testing.T) { t.Fatalf("short password: %d", rec.Code) } } + +// TestSessionCookieScopedToPortedHost pins the core isolation contract: login +// on a ported Host sets kandev_session_, a request carrying only the +// legacy unprefixed cookie is rejected (mixed-version tier 1), and the scoped +// cookie authenticates subsequent requests. +func TestSessionCookieScopedToPortedHost(t *testing.T) { + router, _ := newAPIFixture(t, true) + client := &apiClient{t: t, router: router} + host := "127.0.0.1:8443" + + rec := setupOn(t, client, host) + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session_8443") + if client.cookie == nil || client.cookie.Name != "kandev_session_8443" { + t.Fatalf("captured cookie = %+v, want kandev_session_8443", client.cookie) + } + + // A request carrying only the legacy unprefixed cookie is rejected on the + // ported host (old→new rejection; pre-change it authenticated). + legacy := &apiClient{t: t, router: router} + rec = legacy.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = host + req.Header.Set("Cookie", "kandev_session="+client.cookie.Value) + }) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("legacy cookie on ported host: %d, want 401", rec.Code) + } + + // The scoped cookie authenticates on the ported host (and only there). + rec = client.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = host + }) + if rec.Code != http.StatusOK { + t.Fatalf("scoped cookie on ported host: %d, want 200", rec.Code) + } + if rec := client.do(http.MethodGet, "/api/v1/users", nil); rec.Code != http.StatusUnauthorized { + t.Fatalf("scoped cookie on default-port host: %d, want 401", rec.Code) + } +} + +// TestSessionCookieLogoutClearsScopedName pins the logout/clear path: the +// same scoped name login wrote is cleared, and the unprefixed base name is +// left untouched — on a host serving a default-port instance it is that +// instance's live session cookie (see spec: no proactive legacy scrubbing). +func TestSessionCookieLogoutClearsScopedName(t *testing.T) { + router, _ := newAPIFixture(t, true) + client := &apiClient{t: t, router: router} + host := "127.0.0.1:8443" + setupOn(t, client, host) + + rec := client.do(http.MethodPost, "/api/v1/auth/logout", nil, func(req *http.Request) { + req.Host = host + }) + if rec.Code != http.StatusNoContent { + t.Fatalf("logout: %d", rec.Code) + } + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session_8443") + for _, cookie := range rec.Result().Cookies() { + if cookie.Name == "kandev_session_8443" && cookie.MaxAge >= 0 { + t.Fatalf("logout did not expire the scoped cookie: %+v", cookie) + } + } + if client.cookie != nil { + t.Fatal("logout must clear the client cookie") + } + if rec := client.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = host + }); rec.Code != http.StatusUnauthorized { + t.Fatalf("post-logout ported request: %d, want 401", rec.Code) + } +} + +// TestInviteAcceptSetsScopedCookie covers the invite-accept set path (there +// is no separate admin-login handler — the first admin's login flows through +// setup). +func TestInviteAcceptSetsScopedCookie(t *testing.T) { + router, _ := newAPIFixture(t, true) + admin := &apiClient{t: t, router: router} + host := "127.0.0.1:8443" + setupOn(t, admin, host) + + rec := admin.do(http.MethodPost, "/api/v1/auth/invites", map[string]any{"email": "m@x.dev", "role": "member"}, + func(req *http.Request) { req.Host = host }) + if rec.Code != http.StatusCreated { + t.Fatalf("create invite: %d body=%s", rec.Code, rec.Body.String()) + } + token := strings.TrimPrefix(decode(t, rec)["url"].(string), "/invite?token=") + + member := &apiClient{t: t, router: router} + rec = member.do(http.MethodPost, "/api/v1/auth/invites/accept", map[string]any{ + "token": token, "email": "m@x.dev", "password": "memberpass123", "display_name": "M", + }, func(req *http.Request) { req.Host = host }) + if rec.Code != http.StatusOK { + t.Fatalf("accept invite: %d body=%s", rec.Code, rec.Body.String()) + } + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session_8443") + if member.cookie == nil || member.cookie.Name != "kandev_session_8443" { + t.Fatalf("member cookie = %+v, want kandev_session_8443", member.cookie) + } +} + +// TestSessionCookieSuffixWithLoadedDefaultConfig pins the config-default +// regression: with the loaded default config (empty auth.cookieName) login on +// a ported Host still sets kandev_session_ — a seeded non-empty default +// must not suppress suffixing in production. +func TestSessionCookieSuffixWithLoadedDefaultConfig(t *testing.T) { + t.Setenv("KANDEV_AUTH_COOKIE_NAME", "") + cfg, err := config.LoadWithPath(t.TempDir()) + if err != nil { + t.Fatalf("load config: %v", err) + } + if cfg.Auth.CookieName != "" { + t.Fatalf("loaded default auth.cookieName = %q, want empty", cfg.Auth.CookieName) + } + cfg.Features.Auth = true + + router, _ := newAPIFixtureWithCfg(t, cfg) + client := &apiClient{t: t, router: router} + rec := setupOn(t, client, "127.0.0.1:8443") + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session_8443") +} + +// TestCustomCookieNameUsedVerbatim pins the custom-name contract: an +// explicitly configured auth.cookieName is never suffixed, and authenticates +// through the same middleware path. +func TestCustomCookieNameUsedVerbatim(t *testing.T) { + cfg := &config.Config{} + cfg.Features.Auth = true + cfg.Auth.SessionTTLHours = 720 + cfg.Auth.CookieName = "my_session" + router, _ := newAPIFixtureWithCfg(t, cfg) + client := &apiClient{t: t, router: router, cookiePrefix: "my_session"} + + rec := setupOn(t, client, "127.0.0.1:8443") + assertSessionCookieNames(t, rec, "my_session", "my_session") + if client.cookie == nil || client.cookie.Name != "my_session" { + t.Fatalf("captured cookie = %+v, want my_session (verbatim)", client.cookie) + } + rec = client.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = "127.0.0.1:8443" + }) + if rec.Code != http.StatusOK { + t.Fatalf("custom-name cookie on ported host: %d, want 200", rec.Code) + } +} + +// TestSessionCookiePlainOnDefaultPort covers the no-port branch through the +// real handlers: login on a default-port Host sets the plain kandev_session, +// the middleware authenticates with it, and logout clears it (invariant +// back-compat guard: identical before and after the change). +func TestSessionCookiePlainOnDefaultPort(t *testing.T) { + router, _ := newAPIFixture(t, true) + client := &apiClient{t: t, router: router} + + rec := client.do(http.MethodPost, "/api/v1/auth/setup", map[string]any{ + "email": "admin@x.dev", "password": "adminpass123", "display_name": "Admin", + }) + if rec.Code != http.StatusOK { + t.Fatalf("setup: %d body=%s", rec.Code, rec.Body.String()) + } + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session") + if client.cookie == nil || client.cookie.Name != "kandev_session" { + t.Fatalf("captured cookie = %+v, want plain kandev_session", client.cookie) + } + if rec := client.do(http.MethodGet, "/api/v1/users", nil); rec.Code != http.StatusOK { + t.Fatalf("default-port session: %d, want 200", rec.Code) + } + rec = client.do(http.MethodPost, "/api/v1/auth/logout", nil) + if rec.Code != http.StatusNoContent { + t.Fatalf("logout: %d", rec.Code) + } + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session") +} + +// TestSessionCookieForeignPortRejectedOnPortedHost is the tier-1b rollback +// guard: a request carrying a cookie under a name the resolver does not +// derive for its Host is rejected both before and after the change, so a +// regression where the middleware matches any scoped name regardless of port +// cannot slip in. +func TestSessionCookieForeignPortRejectedOnPortedHost(t *testing.T) { + router, _ := newAPIFixture(t, true) + client := &apiClient{t: t, router: router} + setupOn(t, client, "127.0.0.1:8443") + + foreign := &apiClient{t: t, router: router} + rec := foreign.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = "127.0.0.1:8443" + req.Header.Set("Cookie", "kandev_session_9443="+client.cookie.Value) + }) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("foreign-port scoped cookie: %d, want 401", rec.Code) + } +} + +// TestTwoInstancesSessionIsolation drives the two-instance behavioral +// regression: independent stores and request Hosts 127.0.0.1:8443 / +// 127.0.0.1:9443. Logging into one must not authenticate (or invalidate) the +// other, and each instance authenticates only with its own port's cookie +// name. +func TestTwoInstancesSessionIsolation(t *testing.T) { + routerA, _ := newAPIFixture(t, true) + routerB, _ := newAPIFixture(t, true) + clientA := &apiClient{t: t, router: routerA} + clientB := &apiClient{t: t, router: routerB} + + setupOn(t, clientA, "127.0.0.1:8443") + if clientA.cookie == nil || clientA.cookie.Name != "kandev_session_8443" { + t.Fatalf("A cookie = %+v, want kandev_session_8443", clientA.cookie) + } + + // B rejects A's scoped cookie (failing-before: pre-change both wrote the + // same unprefixed name, so A's token authenticated on B). + rec := clientB.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = "127.0.0.1:9443" + req.Header.Set("Cookie", clientA.cookie.Name+"="+clientA.cookie.Value) + }) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("B with A's cookie: %d, want 401", rec.Code) + } + + // Logging into B keeps A's session intact. + setupOn(t, clientB, "127.0.0.1:9443") + if clientB.cookie == nil || clientB.cookie.Name != "kandev_session_9443" { + t.Fatalf("B cookie = %+v, want kandev_session_9443", clientB.cookie) + } + if rec := clientA.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = "127.0.0.1:8443" + }); rec.Code != http.StatusOK { + t.Fatalf("A after B login: %d, want 200 (A's session must survive)", rec.Code) + } + if rec := clientB.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = "127.0.0.1:9443" + }); rec.Code != http.StatusOK { + t.Fatalf("B after B login: %d, want 200", rec.Code) + } +} + +// authFixtureConfig is the minimal auth-enabled config for the proxy-contract +// fixtures. +func authFixtureConfig() *config.Config { + cfg := &config.Config{} + cfg.Features.Auth = true + cfg.Auth.SessionTTLHours = 720 + return cfg +} + +// TestProxyContractPreservedHost drives the compliant reverse-proxy setup (a): +// the proxy preserves the full Host (host:port). The origin gate passes +// (hostname match) — reachability is invariant — and the failing-before +// assertion is the paired Set-Cookie: kandev_session_8443. +func TestProxyContractPreservedHost(t *testing.T) { + router, _ := newCORSGatedAPIFixture(t, authFixtureConfig()) + client := &apiClient{t: t, router: router} + host := "public.example:8443" + origin := "https://public.example:8443" + + rec := client.do(http.MethodPost, "/api/v1/auth/setup", map[string]any{ + "email": "admin@x.dev", "password": "adminpass123", "display_name": "Admin", + }, func(req *http.Request) { + req.Host = host + req.Header.Set("Origin", origin) + }) + if rec.Code != http.StatusOK { + t.Fatalf("setup through preserved Host: %d body=%s", rec.Code, rec.Body.String()) + } + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session_8443") + + if rec := client.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = host + req.Header.Set("Origin", origin) + }); rec.Code != http.StatusOK { + t.Fatalf("authenticated call through preserved Host: %d, want 200", rec.Code) + } +} + +// TestProxyContractPortRewriteDerivesPublicPortFromXFH drives the compliant +// setup (b): a same-hostname port rewrite with a correct X-Forwarded-Host +// from a TRUSTED reverse proxy (peer inside KANDEV_TRUSTED_PROXIES, as in +// buildHTTPServer). The gate passes (ports are not compared) and the +// failing-before assertion is the cookie resolver deriving _8443 from XFH, +// not _38429 from the internal Host. +func TestProxyContractPortRewriteDerivesPublicPortFromXFH(t *testing.T) { + router, _ := newCORSGatedAPIFixture(t, authFixtureConfig(), "10.0.0.0/8") + client := &apiClient{t: t, router: router} + + rec := client.do(http.MethodPost, "/api/v1/auth/setup", map[string]any{ + "email": "admin@x.dev", "password": "adminpass123", "display_name": "Admin", + }, func(req *http.Request) { + req.RemoteAddr = "10.0.0.5:5555" // inside the trusted proxy range + req.Host = "public.example:38429" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + req.Header.Set("Origin", "https://public.example:8443") + }) + if rec.Code != http.StatusOK { + t.Fatalf("setup through port rewrite: %d body=%s", rec.Code, rec.Body.String()) + } + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session_8443") + + if rec := client.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.RemoteAddr = "10.0.0.5:5555" + req.Host = "public.example:38429" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + req.Header.Set("Origin", "https://public.example:8443") + }); rec.Code != http.StatusOK { + t.Fatalf("authenticated call through port rewrite: %d, want 200", rec.Code) + } +} + +// TestProxyContractPortRewriteUntrustedXFHIgnored pins the secure default: +// an UNTRUSTED peer's X-Forwarded-Host is stripped before the cookie resolver +// runs (browsers never send the header, so only proxies or scripts do), and +// the suffix falls back to the request Host. This is the silent no-op the +// pre-fix resolver would have performed; the strip middleware now drops the +// header with a warning instead. +func TestProxyContractPortRewriteUntrustedXFHIgnored(t *testing.T) { + router, _ := newCORSGatedAPIFixture(t, authFixtureConfig()) + client := &apiClient{t: t, router: router} + + rec := client.do(http.MethodPost, "/api/v1/auth/setup", map[string]any{ + "email": "admin@x.dev", "password": "adminpass123", "display_name": "Admin", + }, func(req *http.Request) { + req.Host = "public.example:38429" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + req.Header.Set("Origin", "https://public.example:8443") + }) + if rec.Code != http.StatusOK { + t.Fatalf("setup through untrusted XFH: %d body=%s", rec.Code, rec.Body.String()) + } + assertSessionCookieNames(t, rec, "kandev_session", "kandev_session_38429") + + if rec := client.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = "public.example:38429" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + req.Header.Set("Origin", "https://public.example:8443") + }); rec.Code != http.StatusOK { + t.Fatalf("authenticated call through untrusted XFH: %d, want 200", rec.Code) + } +} + +// TestProxyContractHostnameRewriteRejected403 drives the non-compliant +// reverse-proxy setup: the proxy rewrites a non-loopback hostname (Host is a +// DIFFERENT non-loopback hostname, not a different port). The origin gate +// compares Origin hostname to Host and ignores X-Forwarded-Host, so the +// request is rejected 403 before auth — an invariant branch that must not be +// weakened by this fix. +func TestProxyContractHostnameRewriteRejected403(t *testing.T) { + router, _ := newCORSGatedAPIFixture(t, authFixtureConfig()) + client := &apiClient{t: t, router: router} + + rec := client.do(http.MethodPost, "/api/v1/auth/setup", map[string]any{ + "email": "admin@x.dev", "password": "adminpass123", "display_name": "Admin", + }, func(req *http.Request) { + req.Host = "internal:38429" // different non-loopback hostname + req.Header.Set("X-Forwarded-Host", "public.example:8443") + req.Header.Set("Origin", "https://public.example:8443") + }) + if rec.Code != http.StatusForbidden { + t.Fatalf("hostname rewrite: %d, want 403 (origin gate rejects before auth)", rec.Code) + } + assertSessionCookieNames(t, rec, "kandev_session") +} + +// TestSameCustomCookieNameStillConflicts pins the documented exception: a +// custom auth.cookieName disables automatic port isolation, so two instances +// configured with the same name on one cookie host write the identical cookie +// name regardless of port (and would overwrite each other in a shared jar). +func TestSameCustomCookieNameStillConflicts(t *testing.T) { + makeCfg := func() *config.Config { + cfg := authFixtureConfig() + cfg.Auth.CookieName = "shared_session" + return cfg + } + routerA, _ := newAPIFixtureWithCfg(t, makeCfg()) + routerB, _ := newAPIFixtureWithCfg(t, makeCfg()) + clientA := &apiClient{t: t, router: routerA, cookiePrefix: "shared_session"} + clientB := &apiClient{t: t, router: routerB, cookiePrefix: "shared_session"} + + rec := setupOn(t, clientA, "127.0.0.1:8443") + assertSessionCookieNames(t, rec, "shared_session", "shared_session") + rec = setupOn(t, clientB, "127.0.0.1:9443") + assertSessionCookieNames(t, rec, "shared_session", "shared_session") + + // Same name on both ports: no isolation — B's middleware looks up the + // same cookie name A wrote (a browser jar would hold exactly one). + if clientA.cookie.Name != clientB.cookie.Name { + t.Fatalf("custom names differ across ports (%q vs %q), isolation must be disabled by design", + clientA.cookie.Name, clientB.cookie.Name) + } + if rec := clientB.do(http.MethodGet, "/api/v1/users", nil, func(req *http.Request) { + req.Host = "127.0.0.1:9443" + }); rec.Code != http.StatusOK { + t.Fatalf("B's own custom-name session: %d, want 200", rec.Code) + } +} diff --git a/apps/backend/internal/auth/httpmw/middleware.go b/apps/backend/internal/auth/httpmw/middleware.go index 3a6a9a2ea9..7ea7d24caf 100644 --- a/apps/backend/internal/auth/httpmw/middleware.go +++ b/apps/backend/internal/auth/httpmw/middleware.go @@ -13,13 +13,17 @@ package httpmw import ( + "net" "net/http" + "net/netip" "strings" "github.com/gin-gonic/gin" + "go.uber.org/zap" "github.com/kandev/kandev/internal/auth" "github.com/kandev/kandev/internal/auth/authn" + "github.com/kandev/kandev/internal/common/logger" userstore "github.com/kandev/kandev/internal/user/store" ) @@ -49,6 +53,114 @@ func Middleware(svc *auth.Service) gin.HandlerFunc { } } +// StripUntrustedForwardedHost removes X-Forwarded-Host from requests whose +// immediate peer is not a trusted proxy. It is a pure header-modifier: it +// never aborts, so gin continues to the next handler automatically once the +// function returns (no c.Next() call needed). Browsers never send +// X-Forwarded-Host; only a reverse proxy in front of the backend should, and +// only to carry the browser's original host:port through a port-rewrite to +// the port-scoped cookie-name resolver (httpcookie.PortSuffix). Without this +// gate a non-browser client could pick its own forwarded host, and with it +// its own port-scoped cookie name, by setting the header directly. The same +// trusted list that gates X-Forwarded-For (gin's ClientIP via +// SetTrustedProxies) gates this header so the two trust decisions cannot +// diverge; an untrusted value is dropped (with a warning) and the resolver +// falls back to the request Host. backendapp passes the list returned by its +// configureTrustedProxies, so the two uses share one configuration. +func StripUntrustedForwardedHost(trusted []string, log *logger.Logger) gin.HandlerFunc { + matcher := newTrustedProxyMatcher(trusted) + return func(c *gin.Context) { + if c.GetHeader("X-Forwarded-Host") == "" { + return + } + peer := remoteAddrHost(c.Request.RemoteAddr) + if matcher.contains(peer) { + return + } + log.Warn("ignoring X-Forwarded-Host from untrusted peer", + zap.String("peer", peer), + zap.String("forwarded_host", c.GetHeader("X-Forwarded-Host"))) + c.Request.Header.Del("X-Forwarded-Host") + } +} + +// trustedProxyMatcher matches a peer IP against the trusted-proxy list (bare +// IPs and CIDRs, the same entries configureTrustedProxies handed to gin). +type trustedProxyMatcher struct { + addresses []netip.Addr + prefixes []netip.Prefix +} + +// newTrustedProxyMatcher indexes the trusted-proxy list (bare IPs and CIDRs) +// for O(1) peer matching. IPv4-mapped entries are normalized to their IPv4 +// form so the matcher agrees with gin's trust decision (see contains). +func newTrustedProxyMatcher(trusted []string) *trustedProxyMatcher { + m := &trustedProxyMatcher{} + for _, entry := range trusted { + if strings.Contains(entry, "/") { + prefix, err := netip.ParsePrefix(entry) + if err != nil { + continue + } + if prefix.Addr().Is4In6() { + // gin normalizes IPv4-mapped CIDRs to their IPv4 form + // (::ffff:10.0.0.0/120 → 10.0.0.0/24, mask sliced 96 bits + // shorter); mirror it so the two trust decisions cannot + // diverge. A mapped-form CIDR with exactly 96 bits + // degenerates to 0.0.0.0/0 in gin (trusts every IPv4 peer) + // and is mirrored as such; with fewer than 96 bits gin's + // entry matches no IPv4-form peer and the matcher skips it, + // failing closed (unsupported configuration). + prefix = netip.PrefixFrom(prefix.Addr().Unmap(), prefix.Bits()-96) + } + if prefix.IsValid() { + m.prefixes = append(m.prefixes, prefix) + } + continue + } + if addr, err := netip.ParseAddr(entry); err == nil { + m.addresses = append(m.addresses, addr.Unmap()) + } + } + return m +} + +// contains reports whether host is one of the trusted addresses or inside one +// of the trusted prefixes. Unparsable peers (Unix sockets, empty RemoteAddr) +// are never trusted. IPv4-mapped IPv6 peers are unmapped so the matcher +// agrees with gin's trust decision (which compares the IPv4 form): if the two +// ever diverged, the strip would fail CLOSED (dropping a header gin trusts), +// breaking the port-rewrite cookie flow rather than security. +func (m *trustedProxyMatcher) contains(host string) bool { + addr, err := netip.ParseAddr(host) + if err != nil { + return false + } + addr = addr.Unmap() + for _, a := range m.addresses { + if a == addr { + return true + } + } + for _, p := range m.prefixes { + if p.Contains(addr) { + return true + } + } + return false +} + +// remoteAddrHost returns the host part of a RemoteAddr ("IP:port"), or the +// whole value when it has no port. +// remoteAddrHost returns the host part of a RemoteAddr ("IP:port"), or the +// whole value when it has no port. +func remoteAddrHost(remoteAddr string) string { + if host, _, err := net.SplitHostPort(remoteAddr); err == nil { + return host + } + return remoteAddr +} + // SyntheticIdentity is the implicit identity used while auth is disabled. func SyntheticIdentity() authn.Identity { return authn.Identity{UserID: userstore.DefaultUserID, Role: authn.RoleAdmin, Synthetic: true} @@ -58,7 +170,7 @@ func SyntheticIdentity() authn.Identity { // bearer. Shared with the WS gateway's upgrade-time check. func ResolveRequest(c *gin.Context, svc *auth.Service) (authn.Identity, bool) { ctx := c.Request.Context() - if cookie, err := c.Cookie(svc.CookieName()); err == nil && cookie != "" { + if cookie, err := c.Cookie(svc.CookieNameForRequest(c.Request)); err == nil && cookie != "" { if identity, ok := svc.ResolveSessionToken(ctx, cookie); ok { return identity, true } diff --git a/apps/backend/internal/auth/httpmw/middleware_test.go b/apps/backend/internal/auth/httpmw/middleware_test.go index 9812f374d8..b89722805d 100644 --- a/apps/backend/internal/auth/httpmw/middleware_test.go +++ b/apps/backend/internal/auth/httpmw/middleware_test.go @@ -10,11 +10,15 @@ import ( "github.com/gin-gonic/gin" "github.com/jmoiron/sqlx" _ "github.com/mattn/go-sqlite3" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" "github.com/kandev/kandev/internal/auth" "github.com/kandev/kandev/internal/auth/authn" authstore "github.com/kandev/kandev/internal/auth/store" "github.com/kandev/kandev/internal/common/config" + "github.com/kandev/kandev/internal/common/logger" userstore "github.com/kandev/kandev/internal/user/store" ) @@ -300,6 +304,224 @@ func TestInvalidCredentialsAreRejected(t *testing.T) { } } +// TestEnabledModeScopedSessionCookieAuthenticates pins the middleware's +// request-aware cookie resolution: on a ported Host it reads the +// port-scoped session cookie name. +func TestEnabledModeScopedSessionCookieAuthenticates(t *testing.T) { + svc := newTestService(t, true) + token := setupAdmin(t, svc) + router := newTestRouter(svc) + + rec := doRequest(router, http.MethodGet, "/api/v1/probe", func(r *http.Request) { + r.Host = "127.0.0.1:8443" + r.AddCookie(&http.Cookie{Name: "kandev_session_8443", Value: token}) + }) + if rec.Code != http.StatusOK { + t.Fatalf("probe with scoped session: %d body=%s", rec.Code, rec.Body.String()) + } + if contains(rec.Body.String(), `"synthetic":true`) { + t.Fatal("real session must not be synthetic") + } +} + +// TestEnabledModeRejectsForeignPortSessionCookie pins per-port resolution: a +// token carried under another port's scoped name is not read for this Host. +func TestEnabledModeRejectsForeignPortSessionCookie(t *testing.T) { + svc := newTestService(t, true) + token := setupAdmin(t, svc) + router := newTestRouter(svc) + + rec := doRequest(router, http.MethodGet, "/api/v1/probe", func(r *http.Request) { + r.Host = "127.0.0.1:8443" + r.AddCookie(&http.Cookie{Name: "kandev_session_9443", Value: token}) + }) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("probe with foreign-port session: %d, want 401", rec.Code) + } +} + func contains(haystack, needle string) bool { return strings.Contains(haystack, needle) } + +// echoXFH handler reports whether X-Forwarded-Host reached the handler. +// echoXFH reports whether X-Forwarded-Host reached the handler. +func echoXFH(c *gin.Context) { + if xfh := c.GetHeader("X-Forwarded-Host"); xfh != "" { + c.String(http.StatusOK, xfh) + return + } + c.String(http.StatusOK, "stripped") +} + +// stripRouter builds a gin router with the X-Forwarded-Host strip middleware +// (trusted list as given) and an echoXFH handler that reports what reached it. +func stripRouter(trusted ...string) *gin.Engine { + gin.SetMode(gin.TestMode) + router := gin.New() + router.Use(StripUntrustedForwardedHost(trusted, mustNopLogger())) + router.GET("/", echoXFH) + return router +} + +// mustNopLogger returns a discard logger for strip-middleware tests. +func mustNopLogger() *logger.Logger { + log, err := logger.NewFromZap(zap.NewNop()) + if err != nil { + panic(err) + } + return log +} + +// TestStripUntrustedForwardedHostUntrustedPeerStrips pins the secure default: +// without a trusted-proxy list, an X-Forwarded-Host presented by the peer is +// dropped before the handler (and the cookie resolver) sees it. +func TestStripUntrustedForwardedHostUntrustedPeerStrips(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("X-Forwarded-Host", "public.example:8443") + rec := httptest.NewRecorder() + stripRouter().ServeHTTP(rec, req) + if got := rec.Body.String(); got != "stripped" { + t.Fatalf("untrusted XFH body = %q, want stripped", got) + } +} + +// TestStripUntrustedForwardedHostTrustedPeerKeeps pins the opt-in behavior: a +// peer inside the trusted list may carry X-Forwarded-Host (the proxy-authored +// browser host:port) through to the cookie resolver. +func TestStripUntrustedForwardedHostTrustedPeerKeeps(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "10.0.0.5:5555" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + rec := httptest.NewRecorder() + stripRouter("10.0.0.0/8").ServeHTTP(rec, req) + if got := rec.Body.String(); got != "public.example:8443" { + t.Fatalf("trusted XFH body = %q, want public.example:8443", got) + } +} + +// TestStripUntrustedForwardedHostIPv4MappedPeerKeeps pins the IPv4-mapped +// IPv6 equivalence: gin compares the IPv4 form of an IPv4-mapped peer against +// the trusted list, and the matcher must agree (fail-closed here would break +// the port-rewrite cookie flow for dual-stack proxies). +func TestStripUntrustedForwardedHostIPv4MappedPeerKeeps(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "[::ffff:10.0.0.5]:5555" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + rec := httptest.NewRecorder() + stripRouter("10.0.0.0/8", "10.0.0.5").ServeHTTP(rec, req) + if got := rec.Body.String(); got != "public.example:8443" { + t.Fatalf("IPv4-mapped trusted XFH body = %q, want public.example:8443", got) + } +} + +// TestStripUntrustedForwardedHostMappedFormCIDRKeeps pins the mapped-form +// CIDR normalization: gin treats ::ffff:10.0.0.0/120 as 10.0.0.0/24 and +// trusts peers under it in either representation; the matcher must agree. +func TestStripUntrustedForwardedHostMappedFormCIDRKeeps(t *testing.T) { + for _, peer := range []string{"10.0.0.5:5555", "[::ffff:10.0.0.5]:5555"} { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = peer + req.Header.Set("X-Forwarded-Host", "public.example:8443") + rec := httptest.NewRecorder() + stripRouter("::ffff:10.0.0.0/120").ServeHTTP(rec, req) + if got := rec.Body.String(); got != "public.example:8443" { + t.Fatalf("mapped-form CIDR peer %q: XFH body = %q, want public.example:8443", peer, got) + } + } +} + +// TestStripUntrustedForwardedHostDegenerateMappedCIDRFailsClosed pins the +// unsupported corner: a mapped-form CIDR with fewer than 96 bits matches no +// IPv4-form peer in gin (dead entry); the matcher mirrors that by skipping it +// (fail closed) rather than silently trusting something gin would not. +func TestStripUntrustedForwardedHostDegenerateMappedCIDRFailsClosed(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "203.0.113.9:5555" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + rec := httptest.NewRecorder() + stripRouter("::ffff:10.0.0.0/90").ServeHTTP(rec, req) + if got := rec.Body.String(); got != "stripped" { + t.Fatalf("degenerate mapped CIDR body = %q, want stripped", got) + } +} + +// TestStripUntrustedForwardedHostMappedForm96MirrorsGin pins the /96 +// boundary: a mapped-form /96 degenerates to 0.0.0.0/0 in gin (trusts every +// IPv4 peer), and the matcher mirrors that trust instead of failing closed — +// failing closed there would strip a header gin trusts and break the +// port-rewrite cookie flow for the config. +func TestStripUntrustedForwardedHostMappedForm96MirrorsGin(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "203.0.113.9:5555" // any IPv4 peer + req.Header.Set("X-Forwarded-Host", "public.example:8443") + rec := httptest.NewRecorder() + stripRouter("::ffff:10.0.0.0/96").ServeHTTP(rec, req) + if got := rec.Body.String(); got != "public.example:8443" { + t.Fatalf("mapped /96 body = %q, want public.example:8443 (gin trusts all IPv4)", got) + } +} + +// TestStripUntrustedForwardedHostCIDRExcludesOutOfRangePeer pins the CIDR +// boundary: the peer just outside the trusted prefix is untrusted. +func TestStripUntrustedForwardedHostCIDRExcludesOutOfRangePeer(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "192.168.1.7:5555" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + rec := httptest.NewRecorder() + stripRouter("192.168.0.0/24").ServeHTTP(rec, req) + if got := rec.Body.String(); got != "stripped" { + t.Fatalf("out-of-CIDR XFH body = %q, want stripped", got) + } +} + +// TestStripUntrustedForwardedHostNoHeaderUntouched pins the no-op path: a +// request without the header passes through unchanged. +func TestStripUntrustedForwardedHostNoHeaderUntouched(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + stripRouter().ServeHTTP(rec, req) + if got := rec.Body.String(); got != "stripped" { + t.Fatalf("no-header body = %q, want stripped (handler default)", got) + } +} + +// TestStripUntrustedForwardedHostWarns pins the operator signal: stripping an +// untrusted X-Forwarded-Host emits a warning naming the peer, so a +// misconfigured proxy (or a client trying to pick its own cookie scope) is +// visible in the logs instead of silently no-opping. +func TestStripUntrustedForwardedHostWarns(t *testing.T) { + core, logs := observer.New(zapcore.WarnLevel) + log, err := logger.NewFromZap(zap.New(core)) + if err != nil { + t.Fatalf("observer logger: %v", err) + } + gin.SetMode(gin.TestMode) + router := gin.New() + router.Use(StripUntrustedForwardedHost(nil, log)) + router.GET("/", echoXFH) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.RemoteAddr = "203.0.113.9:5555" + req.Header.Set("X-Forwarded-Host", "public.example:8443") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if got := rec.Body.String(); got != "stripped" { + t.Fatalf("warn-test body = %q, want stripped", got) + } + warned := false + for _, entry := range logs.All() { + if !strings.Contains(entry.Message, "X-Forwarded-Host") { + continue + } + warned = true + for _, field := range entry.Context { + if field.Key == "peer" && field.String != "203.0.113.9" { + t.Fatalf("warning peer = %q, want 203.0.113.9", field.String) + } + } + } + if !warned { + t.Fatal("no X-Forwarded-Host warning was logged") + } +} diff --git a/apps/backend/internal/auth/service.go b/apps/backend/internal/auth/service.go index a3cac120b7..598ef4bb51 100644 --- a/apps/backend/internal/auth/service.go +++ b/apps/backend/internal/auth/service.go @@ -11,6 +11,7 @@ package auth import ( "context" "errors" + "net/http" "strings" "sync" "sync/atomic" @@ -19,6 +20,7 @@ import ( "github.com/kandev/kandev/internal/auth/authn" "github.com/kandev/kandev/internal/auth/store" "github.com/kandev/kandev/internal/common/config" + "github.com/kandev/kandev/internal/common/httpcookie" "github.com/kandev/kandev/internal/common/logger" usermodels "github.com/kandev/kandev/internal/user/models" userstore "github.com/kandev/kandev/internal/user/store" @@ -47,6 +49,13 @@ const ( sessionTouchInterval = time.Minute maxUserAgentLen = 256 + + // baseSessionCookieName is the session cookie base name. The effective + // name is request-host derived (see CookieNameForRequest): port-scoped on + // a ported host so multiple instances on one host keep isolated sessions, + // plain on a default-port host. An explicit auth.cookieName config value + // replaces the base name entirely (verbatim, never suffixed). + baseSessionCookieName = "kandev_session" ) // Sentinel errors surfaced to HTTP handlers. @@ -128,12 +137,29 @@ func (s *Service) Mode() Mode { return ModeDisabled } -// CookieName returns the configured session cookie name. +// CookieName returns the configured session cookie name (request-less form). +// An explicitly configured auth.cookieName is returned verbatim; the empty +// default resolves to the base name. Contexts without a request (tests) must +// pass this form or use CookieNameForRequest with a request. func (s *Service) CookieName() string { if name := strings.TrimSpace(s.cfg.Auth.CookieName); name != "" { return name } - return "kandev_session" + return baseSessionCookieName +} + +// CookieNameForRequest returns the session cookie name for a specific +// request. A non-empty configured auth.cookieName is returned verbatim — +// custom names disable automatic port isolation and must be unique per +// cookie host. The empty default is port-scoped from the request host +// (kandev_session_ on a ported host, plain kandev_session otherwise) +// so two instances on one host (same IP, different ports) keep isolated +// sessions instead of overwriting each other's token. +func (s *Service) CookieNameForRequest(r *http.Request) string { + if name := strings.TrimSpace(s.cfg.Auth.CookieName); name != "" { + return name + } + return httpcookie.ScopedName(r, baseSessionCookieName) } // SessionTTL returns the sliding session lifetime. diff --git a/apps/backend/internal/auth/service_sso.go b/apps/backend/internal/auth/service_sso.go index 3c98639e46..9e9030c181 100644 --- a/apps/backend/internal/auth/service_sso.go +++ b/apps/backend/internal/auth/service_sso.go @@ -62,8 +62,8 @@ type ExternalIdentity struct { // New users are always provisioned as members; external login never creates or // elevates an admin (role changes stay with admin user management). A disabled // account is rejected. The returned string is the raw session token to place -// in the kandev_session cookie — the caller (the plugin webhook relay) sets the -// cookie; the plugin never receives this token. +// in the session cookie (name derived from the request host) — the caller (the +// plugin webhook relay) sets the cookie; the plugin never receives this token. func (s *Service) AuthenticateExternal(ctx context.Context, ext ExternalIdentity, userAgent, ip string) (*usermodels.User, string, error) { if s.Mode() != ModeEnabled { return nil, "", ErrSSONotEnabled diff --git a/apps/backend/internal/auth/store/models.go b/apps/backend/internal/auth/store/models.go index dd06a36a91..d771f8dcf2 100644 --- a/apps/backend/internal/auth/store/models.go +++ b/apps/backend/internal/auth/store/models.go @@ -24,7 +24,8 @@ type LoginIdentity struct { UpdatedAt time.Time `db:"updated_at" json:"updated_at"` } -// Session is a browser session backed by the kandev_session cookie. +// Session is a browser session backed by the session cookie (base name +// kandev_session; effective name derived from the request host). type Session struct { ID string `db:"id" json:"id"` UserID string `db:"user_id" json:"user_id"` diff --git a/apps/backend/internal/backendapp/auth.go b/apps/backend/internal/backendapp/auth.go index 95911ef52e..77d3c6f5d5 100644 --- a/apps/backend/internal/backendapp/auth.go +++ b/apps/backend/internal/backendapp/auth.go @@ -20,10 +20,10 @@ import ( // pluginSSOBridge adapts the auth service to plugins.AuthLoginBridge: it // authenticates a plugin-asserted external identity (OIDC/SAML) and sets the -// kandev_session cookie, so an auth-capable plugin can complete SSO login -// without ever holding the raw session token. AuthenticateExternal enforces -// that auth is enabled, so this returns an error (surfaced as 403 by the -// plugin webhook relay) when it is not. +// session cookie (name derived from the request host), so an auth-capable +// plugin can complete SSO login without ever holding the raw session token. +// AuthenticateExternal enforces that auth is enabled, so this returns an error +// (surfaced as 403 by the plugin webhook relay) when it is not. type pluginSSOBridge struct { auth *auth.Service } @@ -38,7 +38,7 @@ func (b pluginSSOBridge) LoginExternal(c *gin.Context, provider, subject, email, if err != nil { return err } - authhttpapi.SetSessionCookie(c, b.auth.CookieName(), token, b.auth.SessionTTL()) + authhttpapi.SetSessionCookie(c, b.auth.CookieNameForRequest(c.Request), token, b.auth.SessionTTL()) return nil } diff --git a/apps/backend/internal/backendapp/auth_test.go b/apps/backend/internal/backendapp/auth_test.go new file mode 100644 index 0000000000..6c98680cc1 --- /dev/null +++ b/apps/backend/internal/backendapp/auth_test.go @@ -0,0 +1,118 @@ +package backendapp + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" + + "github.com/kandev/kandev/internal/auth" + authstore "github.com/kandev/kandev/internal/auth/store" + "github.com/kandev/kandev/internal/common/config" + userstore "github.com/kandev/kandev/internal/user/store" +) + +// newEnabledAuthService builds an auth service in ModeEnabled (features.auth +// on + an admin identity exists) over an in-memory store pair — the state the +// plugin SSO bridge requires. +func newEnabledAuthService(t *testing.T, cfg *config.Config) *auth.Service { + t.Helper() + conn, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + users, cleanup, err := userstore.Provide(conn, conn) + if err != nil { + t.Fatalf("user store: %v", err) + } + t.Cleanup(func() { _ = cleanup() }) + st, err := authstore.New(conn, conn) + if err != nil { + t.Fatalf("auth store: %v", err) + } + svc, err := auth.NewService(context.Background(), auth.Deps{Cfg: cfg, Store: st, Users: users}) + if err != nil { + t.Fatalf("auth service: %v", err) + } + if _, _, err := svc.Setup(context.Background(), "admin@x.dev", "adminpass123", "Admin", "", ""); err != nil { + t.Fatalf("setup admin: %v", err) + } + if svc.Mode() != auth.ModeEnabled { + t.Fatalf("mode = %s, want enabled", svc.Mode()) + } + return svc +} + +// ssoBridgeCookieNames runs the real pluginSSOBridge LoginExternal on a Gin +// context whose request Host carries the given host, and returns the session +// cookie names set on the response. +func ssoBridgeCookieNames(t *testing.T, svc *auth.Service, host string) []string { + t.Helper() + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = httptest.NewRequest(http.MethodPost, "/api/plugins/p/webhooks/cb", nil) + c.Request.Host = host + + bridge := pluginSSOBridge{auth: svc} + if err := bridge.LoginExternal(c, "oidc:iss", "sub-1", "alice@x.dev", "Alice"); err != nil { + t.Fatalf("LoginExternal: %v", err) + } + var names []string + for _, cookie := range rec.Result().Cookies() { + names = append(names, cookie.Name) + } + return names +} + +// TestPluginSSOBridgeSetsScopedSessionCookie drives the REAL bridge +// (backendapp/auth.go), not the plugins-package fake: LoginExternal on a Gin +// request Host 127.0.0.1:8443 must set kandev_session_8443. A stale bridge +// call to b.auth.CookieName() would pass resolver tests while emitting the +// plain name for ported SSO. +func TestPluginSSOBridgeSetsScopedSessionCookie(t *testing.T) { + cfg := &config.Config{} + cfg.Features.Auth = true + cfg.Auth.SessionTTLHours = 720 + svc := newEnabledAuthService(t, cfg) + + names := ssoBridgeCookieNames(t, svc, "127.0.0.1:8443") + if len(names) != 1 || names[0] != "kandev_session_8443" { + t.Fatalf("SSO Set-Cookie names = %v, want exactly [kandev_session_8443]", names) + } +} + +// TestPluginSSOBridgeCustomCookieNameVerbatim pins the custom-name contract +// through the real bridge: a configured auth.cookieName is set verbatim on a +// ported Host (never suffixed). +func TestPluginSSOBridgeCustomCookieNameVerbatim(t *testing.T) { + cfg := &config.Config{} + cfg.Features.Auth = true + cfg.Auth.SessionTTLHours = 720 + cfg.Auth.CookieName = "sso_session" + svc := newEnabledAuthService(t, cfg) + + names := ssoBridgeCookieNames(t, svc, "127.0.0.1:8443") + if len(names) != 1 || names[0] != "sso_session" { + t.Fatalf("SSO Set-Cookie names = %v, want exactly [sso_session]", names) + } +} + +// TestPluginSSOBridgeDefaultPortPlainName pins the no-port branch through the +// real bridge: a default-port Host keeps the plain base name. +func TestPluginSSOBridgeDefaultPortPlainName(t *testing.T) { + cfg := &config.Config{} + cfg.Features.Auth = true + cfg.Auth.SessionTTLHours = 720 + svc := newEnabledAuthService(t, cfg) + + names := ssoBridgeCookieNames(t, svc, "example.com") + if len(names) != 1 || names[0] != "kandev_session" { + t.Fatalf("SSO Set-Cookie names = %v, want exactly [kandev_session]", names) + } +} diff --git a/apps/backend/internal/backendapp/boot_state_routes.go b/apps/backend/internal/backendapp/boot_state_routes.go index 74ae2b8e46..5b28c27657 100644 --- a/apps/backend/internal/backendapp/boot_state_routes.go +++ b/apps/backend/internal/backendapp/boot_state_routes.go @@ -8,6 +8,7 @@ import ( "go.uber.org/zap" + "github.com/kandev/kandev/internal/common/httpcookie" officedashboard "github.com/kandev/kandev/internal/office/dashboard" taskdto "github.com/kandev/kandev/internal/task/dto" taskmodels "github.com/kandev/kandev/internal/task/models" @@ -391,7 +392,10 @@ func (b bootStateBuilder) addOfficeRouteState(ctx context.Context, req *http.Req state["office"] = b.officeState(ctx, activeID) } -// officeWorkspaces resolves the office workspaces and the active workspace id from the request cookie. +// officeWorkspaces resolves the office workspaces and the active workspace id +// from the request cookies and user settings. Candidates, each validated +// against the office workspace set: general cookie (when it names an office +// workspace) → office cookie → settings → first office workspace. func (b bootStateBuilder) officeWorkspaces(ctx context.Context, req *http.Request) ([]taskdto.WorkspaceDTO, string, error) { if b.p.taskSvc == nil { return nil, "", nil @@ -412,7 +416,16 @@ func (b bootStateBuilder) officeWorkspaces(ctx context.Context, req *http.Reques officeItems = append(officeItems, item) } } - return items, resolveActiveOfficeWorkspaceID(officeItems, readActiveWorkspaceCookie(req)), nil + settingsWorkspaceID := "" + if settings, ok := b.userSettings(ctx); ok { + settingsWorkspaceID = settings.Settings.WorkspaceID + } + return items, resolveActiveOfficeWorkspaceID( + officeItems, + readActiveWorkspaceCookie(req), + readOfficeWorkspaceCookie(req), + settingsWorkspaceID, + ), nil } // officeState assembles the office boot state (agents, projects, inbox, dashboard) for the active workspace. @@ -778,11 +791,20 @@ func mapTaskCreateLastUsed(value usermodels.TaskCreateLastUsed) map[string]any { } } -// resolveActiveOfficeWorkspaceID returns the cookie workspace id when it is valid, otherwise the first workspace id. -func resolveActiveOfficeWorkspaceID(workspaces []taskdto.WorkspaceDTO, cookieWorkspaceID string) string { - for _, workspace := range workspaces { - if workspace.ID == cookieWorkspaceID { - return workspace.ID +// resolveActiveOfficeWorkspaceID returns the first candidate — general cookie, +// office cookie, settings — that names a valid office workspace, otherwise the +// first office workspace id. +func resolveActiveOfficeWorkspaceID( + workspaces []taskdto.WorkspaceDTO, + generalCookieID string, + officeCookieID string, + settingsID string, +) string { + for _, candidate := range []string{generalCookieID, officeCookieID, settingsID} { + for _, workspace := range workspaces { + if workspace.ID == candidate { + return workspace.ID + } } } if len(workspaces) > 0 { @@ -857,12 +879,50 @@ func tasksListGroupForRoute(queryValue, settingsValue string) string { return usermodels.NormalizeTasksListGroup(settingsValue) } -// readActiveWorkspaceCookie reads the active workspace id from the cookies. +// readActiveWorkspaceCookie reads the active workspace id from the GENERAL +// cookie family only: the port-scoped name first, then the legacy unprefixed +// name as a read-only fallback (a pre-upgrade selection survives). The +// office-family cookies are deliberately not consulted — generic boot paths +// resolve settings/first when only an office cookie exists (parity with the +// frontend generic reader). The suffix derives from the request host +// (X-Forwarded-Host precedence), the same port the SPA derives from the API +// base URL. func readActiveWorkspaceCookie(req *http.Request) string { if req == nil { return "" } - for _, name := range []string{activeWorkspaceCookie, legacyOfficeWorkspaceCookie} { + // On a default-port host the scoped name IS the legacy name; reading the + // same cookie twice is a no-op, so only probe the legacy name when the + // port actually scopes it. + names := []string{httpcookie.ScopedName(req, activeWorkspaceCookie)} + if scoped := names[0]; scoped != activeWorkspaceCookie { + names = append(names, activeWorkspaceCookie) + } + for _, name := range names { + cookie, err := req.Cookie(name) + if err == nil { + if value := strings.TrimSpace(cookie.Value); value != "" { + return value + } + } + } + return "" +} + +// readOfficeWorkspaceCookie reads the active workspace id from the OFFICE +// cookie family only: the port-scoped name first, then the legacy unprefixed +// name as a read-only fallback. The general cookie is never consulted here. +func readOfficeWorkspaceCookie(req *http.Request) string { + if req == nil { + return "" + } + // Same default-port dedupe as readActiveWorkspaceCookie: the scoped name + // equals the legacy name when the Host carries no (non-default) port. + names := []string{httpcookie.ScopedName(req, legacyOfficeWorkspaceCookie)} + if scoped := names[0]; scoped != legacyOfficeWorkspaceCookie { + names = append(names, legacyOfficeWorkspaceCookie) + } + for _, name := range names { cookie, err := req.Cookie(name) if err == nil { if value := strings.TrimSpace(cookie.Value); value != "" { diff --git a/apps/backend/internal/backendapp/boot_state_workspace_test.go b/apps/backend/internal/backendapp/boot_state_workspace_test.go index 2c65a1d445..fc1d76cd16 100644 --- a/apps/backend/internal/backendapp/boot_state_workspace_test.go +++ b/apps/backend/internal/backendapp/boot_state_workspace_test.go @@ -1,11 +1,16 @@ package backendapp import ( + "context" + "encoding/json" "net/http" "net/http/httptest" "testing" + "time" + "github.com/kandev/kandev/internal/task/models" taskservice "github.com/kandev/kandev/internal/task/service" + userservice "github.com/kandev/kandev/internal/user/service" "github.com/kandev/kandev/internal/webapp" ) @@ -92,3 +97,411 @@ func TestBootInitialStateSettingsPrefersTheActiveWorkspaceCookie(t *testing.T) { t.Fatalf("activeId = %v, want %s", got, created.ID) } } + +// scopedRequest builds a GET request on the given ported Host with the given +// cookies set (browser-jar style: several cookies may coexist). +func scopedRequest(host string, cookies ...http.Cookie) *http.Request { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Host = host + for _, cookie := range cookies { + req.AddCookie(&cookie) + } + return req +} + +func bootStateActiveID(t *testing.T, req *http.Request, params routeParams, route webapp.RouteName) string { + t.Helper() + state := bootInitialState(t.Context(), req, params, webapp.RouteClassification{Route: route}) + block, ok := state["workspaces"].(map[string]any) + if !ok { + t.Fatalf("route %s: no workspaces block in state %v", route, state) + } + activeID, _ := block["activeId"].(string) + return activeID +} + +// TestReadActiveWorkspaceCookiePrefersScopedName pins the reader contract: +// the port-scoped general name wins over the legacy unprefixed one on a +// ported Host. +func TestReadActiveWorkspaceCookiePrefersScopedName(t *testing.T) { + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "kandev-active-workspace", Value: "legacy-1"}, + http.Cookie{Name: "kandev-active-workspace_8443", Value: "scoped-1"}, + ) + if got := readActiveWorkspaceCookie(req); got != "scoped-1" { + t.Fatalf("readActiveWorkspaceCookie = %q, want scoped-1", got) + } +} + +// TestReadActiveWorkspaceCookieFallsBackToLegacy pins the read-only legacy +// fallback: a pre-upgrade selection survives when no scoped cookie exists. +func TestReadActiveWorkspaceCookieFallsBackToLegacy(t *testing.T) { + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "kandev-active-workspace", Value: "legacy-1"}, + ) + if got := readActiveWorkspaceCookie(req); got != "legacy-1" { + t.Fatalf("readActiveWorkspaceCookie = %q, want legacy-1", got) + } +} + +// TestReadActiveWorkspaceCookieIgnoresOfficeFamily pins family separation: +// the general reader never consults the office-family cookies (pre-change it +// read the legacy office name — fails pre-change). +func TestReadActiveWorkspaceCookieIgnoresOfficeFamily(t *testing.T) { + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "office-active-workspace_8443", Value: "office-scoped"}, + http.Cookie{Name: "office-active-workspace", Value: "office-legacy"}, + ) + if got := readActiveWorkspaceCookie(req); got != "" { + t.Fatalf("readActiveWorkspaceCookie = %q, want empty (office family must be ignored)", got) + } +} + +// TestReadOfficeWorkspaceCookieScopedFirstLegacyFallback pins the office +// reader: scoped office name first, legacy office name as fallback. +func TestReadOfficeWorkspaceCookieScopedFirstLegacyFallback(t *testing.T) { + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "office-active-workspace", Value: "legacy"}, + http.Cookie{Name: "office-active-workspace_8443", Value: "scoped"}, + ) + if got := readOfficeWorkspaceCookie(req); got != "scoped" { + t.Fatalf("readOfficeWorkspaceCookie = %q, want scoped", got) + } + req = scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "office-active-workspace", Value: "legacy"}, + ) + if got := readOfficeWorkspaceCookie(req); got != "legacy" { + t.Fatalf("readOfficeWorkspaceCookie = %q, want legacy", got) + } +} + +// TestReadOfficeWorkspaceCookieIgnoresGeneralFamily pins the office reader's +// family boundary: the general cookie is never an office candidate. +func TestReadOfficeWorkspaceCookieIgnoresGeneralFamily(t *testing.T) { + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "kandev-active-workspace_8443", Value: "general-scoped"}, + http.Cookie{Name: "kandev-active-workspace", Value: "general-legacy"}, + ) + if got := readOfficeWorkspaceCookie(req); got != "" { + t.Fatalf("readOfficeWorkspaceCookie = %q, want empty (general family must be ignored)", got) + } +} + +// TestBootActiveWorkspaceCookieIsolatedPerHost models the browser jar: BOTH +// ports' scoped cookies are present in one request, and each Host resolves +// its own port's selection. Failing-before: the reader ignored all scoped +// names, so both requests resolved the first workspace (non-first fixtures). +func TestBootActiveWorkspaceCookieIsolatedPerHost(t *testing.T) { + harness := newBootStateTestHarness(t) + ctx := context.Background() + repo := harness.taskRepo + for _, workspace := range []*models.Workspace{ + {ID: "ws-first", Name: "First"}, + {ID: "ws-a", Name: "A"}, + {ID: "ws-b", Name: "B"}, + } { + if err := repo.CreateWorkspace(ctx, workspace); err != nil { + t.Fatalf("CreateWorkspace(%s): %v", workspace.ID, err) + } + } + params := routeParams{taskSvc: harness.taskSvc, userCtrl: harness.userCtrl} + + jar := []http.Cookie{ + {Name: "kandev-active-workspace_8443", Value: "ws-a"}, + {Name: "kandev-active-workspace_9443", Value: "ws-b"}, + } + if got := bootStateActiveID(t, scopedRequest("127.0.0.1:8443", jar...), params, webapp.RouteHome); got != "ws-a" { + t.Fatalf("Host 8443 activeId = %q, want ws-a", got) + } + if got := bootStateActiveID(t, scopedRequest("127.0.0.1:9443", jar...), params, webapp.RouteHome); got != "ws-b" { + t.Fatalf("Host 9443 activeId = %q, want ws-b", got) + } +} + +// TestBootActiveWorkspaceLegacyFallbackAndScopedPrecedence covers the +// downstream boot-state regression (scenario 3): with no scoped cookie a +// VALID unprefixed legacy id is selected (invariant), a foreign/invalid +// legacy id falls through to the instance-owned fallback (invariant), and +// when both exist the scoped value wins (failing-before). +func TestBootActiveWorkspaceLegacyFallbackAndScopedPrecedence(t *testing.T) { + harness := newBootStateTestHarness(t) + ctx := context.Background() + repo := harness.taskRepo + // ListWorkspaces orders by created_at DESC, so ws-first (created last) is + // the fallback "first workspace" — distinct from the legacy selection. + for _, workspace := range []*models.Workspace{ + {ID: "ws-legacy", Name: "Legacy"}, + {ID: "ws-first", Name: "First"}, + } { + if err := repo.CreateWorkspace(ctx, workspace); err != nil { + t.Fatalf("CreateWorkspace(%s): %v", workspace.ID, err) + } + } + params := routeParams{taskSvc: harness.taskSvc, userCtrl: harness.userCtrl} + + // Invariant: valid unprefixed legacy id selected when no scoped cookie. + req := scopedRequest("127.0.0.1:8443", http.Cookie{Name: "kandev-active-workspace", Value: "ws-legacy"}) + if got := bootStateActiveID(t, req, params, webapp.RouteHome); got != "ws-legacy" { + t.Fatalf("valid legacy id: activeId = %q, want ws-legacy", got) + } + + // Invariant: foreign/invalid legacy id falls to the first workspace. + req = scopedRequest("127.0.0.1:8443", http.Cookie{Name: "kandev-active-workspace", Value: "ws-foreign"}) + if got := bootStateActiveID(t, req, params, webapp.RouteHome); got != "ws-first" { + t.Fatalf("foreign legacy id: activeId = %q, want ws-first", got) + } + + // Failing-before: scoped + legacy both present — the scoped value wins. + req = scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "kandev-active-workspace", Value: "ws-legacy"}, + http.Cookie{Name: "kandev-active-workspace_8443", Value: "ws-first"}, + ) + if got := bootStateActiveID(t, req, params, webapp.RouteHome); got != "ws-first" { + t.Fatalf("scoped over legacy: activeId = %q, want ws-first", got) + } +} + +// TestBootGenericRoutesIgnoreOfficeCookies drives the downstream generic-route +// family separation: every generic route that resolves an active workspace id +// resolves settings/first — never the office id — when only office-family +// cookies are present. The legacy-office row fails before the change (the old +// generic reader consulted the legacy office name); the scoped-office row is +// the paired invariant guard. +func TestBootGenericRoutesIgnoreOfficeCookies(t *testing.T) { + harness := newBootStateTestHarness(t) + ctx := context.Background() + repo := harness.taskRepo + for _, workspace := range []*models.Workspace{ + {ID: "ws-o1", Name: "Office 1", OfficeWorkflowID: "wf-1"}, + {ID: "ws-o2", Name: "Office 2", OfficeWorkflowID: "wf-2"}, + {ID: "ws-kanban", Name: "Kanban"}, + } { + if err := repo.CreateWorkspace(ctx, workspace); err != nil { + t.Fatalf("CreateWorkspace(%s): %v", workspace.ID, err) + } + } + settingsID := "ws-kanban" + if _, err := harness.userSvc.UpdateUserSettings(ctx, &userservice.UpdateUserSettingsRequest{ + WorkspaceID: &settingsID, + }); err != nil { + t.Fatalf("UpdateUserSettings: %v", err) + } + params := routeParams{taskSvc: harness.taskSvc, userCtrl: harness.userCtrl} + + routes := []webapp.RouteName{ + webapp.RouteUnknown, + webapp.RouteHome, + webapp.RouteTasks, + webapp.RouteSettings, + webapp.RouteGitHub, + webapp.RouteGitLab, + webapp.RouteJira, + webapp.RouteLinear, + webapp.RouteStats, + } + for _, route := range routes { + t.Run(string(route)+"/legacy-office-cookie", func(t *testing.T) { + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "office-active-workspace", Value: "ws-o1"}, + ) + if got := bootStateActiveID(t, req, params, route); got != "ws-kanban" { + t.Fatalf("activeId = %q, want settings workspace ws-kanban (office cookie must be ignored)", got) + } + }) + t.Run(string(route)+"/scoped-office-cookie", func(t *testing.T) { + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "office-active-workspace_8443", Value: "ws-o1"}, + ) + if got := bootStateActiveID(t, req, params, route); got != "ws-kanban" { + t.Fatalf("activeId = %q, want settings workspace ws-kanban (scoped office cookie must be ignored)", got) + } + }) + } + + // Tasks routeData resolves the same settings/first workspace. + t.Run("tasks-route-data", func(t *testing.T) { + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "office-active-workspace_8443", Value: "ws-o1"}, + ) + routeData := bootRouteData(ctx, req, params, webapp.RouteClassification{Route: webapp.RouteTasks}) + tasksPage, ok := routeData["tasksPage"].(map[string]any) + if !ok { + t.Fatalf("tasks routeData missing tasksPage: %v", routeData) + } + if got := tasksPage["activeWorkspaceId"]; got != "ws-kanban" { + t.Fatalf("tasks routeData activeWorkspaceId = %v, want ws-kanban", got) + } + }) +} + +// TestBootQuickChatFallsBackToSettingsWithOnlyOfficeCookies covers the +// quick-chat boot fallback separately: on a non-task route with only an +// office cookie present, quick chat follows settings/first — never the office +// id. +func TestBootQuickChatFallsBackToSettingsWithOnlyOfficeCookies(t *testing.T) { + harness := newBootStateTestHarness(t) + ctx := context.Background() + repo := harness.taskRepo + for _, workspace := range []*models.Workspace{ + {ID: "ws-o1", Name: "Office 1", OfficeWorkflowID: "wf-1"}, + {ID: "ws-kanban", Name: "Kanban"}, + } { + if err := repo.CreateWorkspace(ctx, workspace); err != nil { + t.Fatalf("CreateWorkspace(%s): %v", workspace.ID, err) + } + } + settingsID := "ws-kanban" + if _, err := harness.userSvc.UpdateUserSettings(ctx, &userservice.UpdateUserSettingsRequest{ + WorkspaceID: &settingsID, + }); err != nil { + t.Fatalf("UpdateUserSettings: %v", err) + } + base := time.Date(2026, 7, 3, 12, 0, 0, 0, time.UTC) + seedBootQuickChatTask(t, repo, ctx, bootQuickChatFixture{ + id: "chat-kanban", workspaceID: "ws-kanban", title: "Settings Chat", + updatedAt: base, sessionUpdatedAt: base, agentProfileID: "agent-1", + }) + seedBootQuickChatTask(t, repo, ctx, bootQuickChatFixture{ + id: "chat-office", workspaceID: "ws-o1", title: "Office Chat", + updatedAt: base.Add(time.Minute), sessionUpdatedAt: base.Add(time.Minute), agentProfileID: "agent-2", + }) + + req := scopedRequest("127.0.0.1:8443", + http.Cookie{Name: "office-active-workspace_8443", Value: "ws-o1"}, + ) + params := routeParams{taskSvc: harness.taskSvc, userCtrl: harness.userCtrl} + state := bootInitialState(t.Context(), req, params, webapp.RouteClassification{Route: webapp.RouteHome}) + raw, err := json.Marshal(state) + if err != nil { + t.Fatalf("Marshal state: %v", err) + } + var decoded struct { + QuickChat struct { + Sessions []struct { + SessionID string `json:"sessionId"` + WorkspaceID string `json:"workspaceId"` + } `json:"sessions"` + } `json:"quickChat"` + } + if err := json.Unmarshal(raw, &decoded); err != nil { + t.Fatalf("Unmarshal state: %v", err) + } + sessions := decoded.QuickChat.Sessions + if len(sessions) != 1 { + t.Fatalf("quickChat sessions = %#v, want exactly the settings workspace's session", sessions) + } + if got := sessions[0].WorkspaceID; got != "ws-kanban" { + t.Fatalf("quickChat session workspaceId = %v, want ws-kanban (settings), not the office cookie's", got) + } +} + +// officeWorkspacesPrecedenceHarness builds three office workspaces and a +// kanban workspace with REAL user settings pointing at O2. ListWorkspaces +// orders by created_at DESC, so ws-o3 (created last) is the first office +// fallback — distinct from O1/O2, which the non-first fixtures need. +func officeWorkspacesPrecedenceHarness(t *testing.T) (bootStateBuilder, context.Context) { + t.Helper() + harness := newBootStateTestHarness(t) + ctx := context.Background() + repo := harness.taskRepo + for _, workspace := range []*models.Workspace{ + {ID: "ws-o1", Name: "Office 1", OfficeWorkflowID: "wf-1"}, + {ID: "ws-o2", Name: "Office 2", OfficeWorkflowID: "wf-2"}, + {ID: "ws-o3", Name: "Office 3 (first fallback)", OfficeWorkflowID: "wf-3"}, + {ID: "ws-kanban", Name: "Kanban"}, + } { + if err := repo.CreateWorkspace(ctx, workspace); err != nil { + t.Fatalf("CreateWorkspace(%s): %v", workspace.ID, err) + } + } + settingsID := "ws-o2" + if _, err := harness.userSvc.UpdateUserSettings(ctx, &userservice.UpdateUserSettingsRequest{ + WorkspaceID: &settingsID, + }); err != nil { + t.Fatalf("UpdateUserSettings: %v", err) + } + return bootStateBuilder{p: routeParams{taskSvc: harness.taskSvc, userCtrl: harness.userCtrl}}, ctx +} + +// TestOfficeWorkspacesScopedCookiePrecedence drives the office boot +// resolution with SCOPED cookie names: general → office → settings → first. +// Every row is failing-before — the scoped names did not exist pre-change, so +// the reader returned empty and the resolver fell back to the first office +// workspace (never the asserted non-first winner). +func TestOfficeWorkspacesScopedCookiePrecedence(t *testing.T) { + builder, ctx := officeWorkspacesPrecedenceHarness(t) + + cases := []struct { + name string + cookies []http.Cookie + want string + }{ + {"scoped general kanban + scoped office O2 → O2", []http.Cookie{ + {Name: "kandev-active-workspace_8443", Value: "ws-kanban"}, + {Name: "office-active-workspace_8443", Value: "ws-o2"}, + }, "ws-o2"}, + {"scoped general O1 + scoped office O2 → O1", []http.Cookie{ + {Name: "kandev-active-workspace_8443", Value: "ws-o1"}, + {Name: "office-active-workspace_8443", Value: "ws-o2"}, + }, "ws-o1"}, + {"scoped office O1 + settings O2 → O1", []http.Cookie{ + {Name: "office-active-workspace_8443", Value: "ws-o1"}, + }, "ws-o1"}, + {"scoped general O1 only → O1", []http.Cookie{ + {Name: "kandev-active-workspace_8443", Value: "ws-o1"}, + }, "ws-o1"}, + {"no cookies → settings O2 (new settings candidate)", nil, "ws-o2"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, activeID, err := builder.officeWorkspaces(ctx, scopedRequest("127.0.0.1:8443", tc.cookies...)) + if err != nil { + t.Fatalf("officeWorkspaces: %v", err) + } + if activeID != tc.want { + t.Fatalf("activeID = %q, want %q", activeID, tc.want) + } + }) + } +} + +// TestOfficeWorkspacesLegacyCookiePrecedence drives the office boot +// resolution with LEGACY (unprefixed) names. The kanban-general row is +// failing-before (pre-change the office cookie was never consulted once the +// general cookie named a kanban workspace); the office-wins rows are +// invariants (office beat general-miss and settings both before and after). +func TestOfficeWorkspacesLegacyCookiePrecedence(t *testing.T) { + builder, ctx := officeWorkspacesPrecedenceHarness(t) + + cases := []struct { + name string + cookies []http.Cookie + want string + }{ + {"legacy general kanban + legacy office O2 → O2", []http.Cookie{ + {Name: "kandev-active-workspace", Value: "ws-kanban"}, + {Name: "office-active-workspace", Value: "ws-o2"}, + }, "ws-o2"}, + {"legacy general O1 + legacy office O2 → O1", []http.Cookie{ + {Name: "kandev-active-workspace", Value: "ws-o1"}, + {Name: "office-active-workspace", Value: "ws-o2"}, + }, "ws-o1"}, + {"legacy office O1 + settings O2 → O1", []http.Cookie{ + {Name: "office-active-workspace", Value: "ws-o1"}, + }, "ws-o1"}, + {"legacy general O1 only → O1", []http.Cookie{ + {Name: "kandev-active-workspace", Value: "ws-o1"}, + }, "ws-o1"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, activeID, err := builder.officeWorkspaces(ctx, scopedRequest("127.0.0.1:8443", tc.cookies...)) + if err != nil { + t.Fatalf("officeWorkspaces: %v", err) + } + if activeID != tc.want { + t.Fatalf("activeID = %q, want %q", activeID, tc.want) + } + }) + } +} diff --git a/apps/backend/internal/backendapp/helpers_test.go b/apps/backend/internal/backendapp/helpers_test.go index 7de9eb691f..48fe58ace7 100644 --- a/apps/backend/internal/backendapp/helpers_test.go +++ b/apps/backend/internal/backendapp/helpers_test.go @@ -2036,9 +2036,25 @@ func TestResolveActiveOfficeWorkspaceIDPrefersCookie(t *testing.T) { {ID: "ws-b", OfficeWorkflowID: "office-b"}, } - got := resolveActiveOfficeWorkspaceID(workspaces, "ws-b") - if got != "ws-b" { - t.Fatalf("expected cookie workspace to win, got %q", got) + // General cookie wins when it names an office workspace. + if got := resolveActiveOfficeWorkspaceID(workspaces, "ws-b", "ws-a", ""); got != "ws-b" { + t.Fatalf("expected general cookie workspace to win, got %q", got) + } + // Office cookie wins over settings when the general cookie misses. + if got := resolveActiveOfficeWorkspaceID(workspaces, "ws-missing", "ws-a", "ws-b"); got != "ws-a" { + t.Fatalf("expected office cookie workspace to win, got %q", got) + } + // Settings wins when both cookies miss. + if got := resolveActiveOfficeWorkspaceID(workspaces, "", "", "ws-b"); got != "ws-b" { + t.Fatalf("expected settings workspace to win, got %q", got) + } + // A kanban general cookie (not in the office set) falls through. + if got := resolveActiveOfficeWorkspaceID(workspaces, "ws-kanban", "ws-a", "ws-b"); got != "ws-a" { + t.Fatalf("expected kanban general cookie to fall through to the office cookie, got %q", got) + } + // No candidate matches: first office workspace. + if got := resolveActiveOfficeWorkspaceID(workspaces, "", "", ""); got != "ws-a" { + t.Fatalf("expected first office workspace, got %q", got) } } diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index 1bd4d10c34..c617682b2f 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -1974,7 +1974,11 @@ func buildHTTPServer( // env var to the proxy's IPs/CIDRs; a directly-reachable backend with the // var set can have X-Forwarded-For spoofed, which also defeats the // ClientIP-keyed login rate limiter. - configureTrustedProxies(router, log) + // X-Forwarded-Host feeds the port-scoped cookie-name resolver; honor it + // only from the same trusted proxies that may rewrite it (an untrusted + // value is stripped with a warning, so the resolver falls back to Host). + trusted := configureTrustedProxies(router, log) + router.Use(authhttpmw.StripUntrustedForwardedHost(trusted, log)) router.Use(httpmw.RequestLogger(log, kandevName)) router.Use(httpmw.OtelTracing(kandevName)) router.Use(gin.Recovery()) diff --git a/apps/backend/internal/backendapp/trustedproxies.go b/apps/backend/internal/backendapp/trustedproxies.go index 958749ea3f..f6c31ddf9a 100644 --- a/apps/backend/internal/backendapp/trustedproxies.go +++ b/apps/backend/internal/backendapp/trustedproxies.go @@ -63,8 +63,10 @@ func resolveTrustedProxies(raw string) (trusted []string, invalid []string) { // (SetTrustedProxies(nil)); an invalid entry logs a warning naming the bad // value(s). gin trusts all proxies out of the box, so without this call a // directly reachable backend would accept a spoofed X-Forwarded-For and -// defeat the ClientIP-keyed login rate limiter. -func configureTrustedProxies(router *gin.Engine, log *logger.Logger) { +// defeat the ClientIP-keyed login rate limiter. It returns the effective +// trusted list (nil when nothing is trusted) so callers can apply the same +// trust decision to other forwarded headers. +func configureTrustedProxies(router *gin.Engine, log *logger.Logger) []string { trusted, invalid := resolveTrustedProxies(os.Getenv(trustedProxiesEnv)) if len(invalid) > 0 { log.Warn(trustedProxiesWarningMsg, zap.Strings("invalid_entries", invalid)) @@ -73,4 +75,5 @@ func configureTrustedProxies(router *gin.Engine, log *logger.Logger) { if err := router.SetTrustedProxies(trusted); err != nil { log.Warn("failed to configure trusted proxies", zap.Error(err)) } + return trusted } diff --git a/apps/backend/internal/common/config/config.go b/apps/backend/internal/common/config/config.go index 7d9f0ca3df..12b857d4c0 100644 --- a/apps/backend/internal/common/config/config.go +++ b/apps/backend/internal/common/config/config.go @@ -340,8 +340,12 @@ type AuthConfig struct { // is extended whenever it is used with less than TTL-24h remaining. SessionTTLHours int `mapstructure:"sessionTTLHours"` - // CookieName is the session cookie name. Only override for unusual - // reverse-proxy setups that need distinct cookie names per instance. + // CookieName is the session cookie base name. Empty (the default) means + // the service uses the "kandev_session" base and port-scopes it from the + // request host, isolating instances on one host (see + // auth.Service.CookieNameForRequest). Only override for unusual + // reverse-proxy setups that need a fixed cookie name; a custom value is + // used verbatim (never port-suffixed) and disables automatic isolation. CookieName string `mapstructure:"cookieName"` } @@ -537,11 +541,15 @@ func setDefaults(v *viper.Viper) { v.SetDefault("agent.standaloneHost", "localhost") v.SetDefault("agent.standalonePort", ports.AgentCtl) - // Auth defaults + // Auth defaults. auth.cookieName defaults to empty on purpose: the auth + // service keeps the "kandev_session" base fallback internally and + // port-scopes it from the request host, so an empty default must not be + // seeded with a concrete name or the suffixing would never fire in + // production. A non-empty configured value is used verbatim. v.SetDefault("auth.jwtSecret", "") v.SetDefault("auth.tokenDuration", 3600) // 1 hour v.SetDefault("auth.sessionTTLHours", 720) // 30 days, sliding - v.SetDefault("auth.cookieName", "kandev_session") + v.SetDefault("auth.cookieName", "") // Office defaults v.SetDefault("office.jwtSigningKey", "") @@ -658,6 +666,9 @@ func LoadWithPath(configPath string) (*Config, error) { _ = v.BindEnv("events.namespace", "KANDEV_EVENTS_NAMESPACE") _ = v.BindEnv("debug.devMode", "KANDEV_DEBUG_DEV_MODE") _ = v.BindEnv("debug.pprofEnabled", "KANDEV_DEBUG_PPROF_ENABLED") + // camelCase key, so AutomaticEnv would expose the undocumented + // KANDEV_AUTH_COOKIENAME; the canonical override is KANDEV_AUTH_COOKIE_NAME. + _ = v.BindEnv("auth.cookieName", "KANDEV_AUTH_COOKIE_NAME") _ = v.BindEnv( "githubCredentialBroker.publicBaseUrl", "KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL", diff --git a/apps/backend/internal/common/config/config_test.go b/apps/backend/internal/common/config/config_test.go index 0798eeef4d..99b876a39f 100644 --- a/apps/backend/internal/common/config/config_test.go +++ b/apps/backend/internal/common/config/config_test.go @@ -506,3 +506,71 @@ func TestFeaturesConfig_JSONShape(t *testing.T) { t.Fatal("retired appStatusBar remains in FeaturesConfig JSON") } } + +// TestAuthCookieNameDefaultsEmpty pins the config-default contract: the +// seeded default is EMPTY so the auth service port-scopes its internal +// "kandev_session" base from the request host; a seeded non-empty default +// would suppress suffixing in production. +func TestAuthCookieNameDefaultsEmpty(t *testing.T) { + t.Setenv("KANDEV_AUTH_COOKIE_NAME", "") + cfg, err := LoadWithPath(t.TempDir()) + if err != nil { + t.Fatalf("LoadWithPath: %v", err) + } + if got := cfg.Auth.CookieName; got != "" { + t.Fatalf("default auth.cookieName = %q, want empty", got) + } +} + +// TestAuthCookieNameFromConfigFile pins config.yaml precedence for +// auth.cookieName. +func TestAuthCookieNameFromConfigFile(t *testing.T) { + dir := t.TempDir() + cfgYAML := "auth:\n cookieName: my_session\n" + if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(cfgYAML), 0o600); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + t.Setenv("KANDEV_AUTH_COOKIE_NAME", "") + + cfg, err := LoadWithPath(dir) + if err != nil { + t.Fatalf("LoadWithPath: %v", err) + } + if got := cfg.Auth.CookieName; got != "my_session" { + t.Fatalf("auth.cookieName from config = %q, want %q", got, "my_session") + } +} + +// TestAuthCookieNameFromEnv pins the canonical environment override +// KANDEV_AUTH_COOKIE_NAME (explicit BindEnv — the automatic camelCase +// mapping would expose the undocumented KANDEV_AUTH_COOKIENAME instead). +func TestAuthCookieNameFromEnv(t *testing.T) { + t.Setenv("KANDEV_AUTH_COOKIE_NAME", "env_session") + + cfg, err := LoadWithPath(t.TempDir()) + if err != nil { + t.Fatalf("LoadWithPath: %v", err) + } + if got := cfg.Auth.CookieName; got != "env_session" { + t.Fatalf("auth.cookieName from env = %q, want %q", got, "env_session") + } +} + +// TestAuthCookieNameEnvOverridesFile pins precedence: environment wins over +// config.yaml for auth.cookieName. +func TestAuthCookieNameEnvOverridesFile(t *testing.T) { + dir := t.TempDir() + cfgYAML := "auth:\n cookieName: file_session\n" + if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(cfgYAML), 0o600); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + t.Setenv("KANDEV_AUTH_COOKIE_NAME", "env_session") + + cfg, err := LoadWithPath(dir) + if err != nil { + t.Fatalf("LoadWithPath: %v", err) + } + if got := cfg.Auth.CookieName; got != "env_session" { + t.Fatalf("auth.cookieName = %q, want env override %q", got, "env_session") + } +} diff --git a/apps/backend/internal/common/httpcookie/httpcookie.go b/apps/backend/internal/common/httpcookie/httpcookie.go new file mode 100644 index 0000000000..ea74c074d2 --- /dev/null +++ b/apps/backend/internal/common/httpcookie/httpcookie.go @@ -0,0 +1,78 @@ +// Package httpcookie derives port-scoped cookie names from a request host. +// +// Browsers match cookies by host, domain, and path — port is not part of +// cookie scope — so multiple kandev instances on one host (same IP, +// different ports) share one cookie jar. Suffixing cookie names with the +// request's port lets each instance read and write only its own cookies +// (see docs/specs/fix-multi-instance-cookie-isolation/spec.md). +package httpcookie + +import ( + "net" + "net/http" + "strconv" + "strings" +) + +// PortSuffix returns "_" derived from the request host, or "" when no +// usable port is present. X-Forwarded-Host (first value, trimmed) wins over +// Host so a proxy that rewrites only the port can carry the browser's +// host:port. The HTTP boundary (httpmw.StripUntrustedForwardedHost, +// registered in backendapp.buildHTTPServer) removes X-Forwarded-Host from +// untrusted peers, so a value present here is proxy-authored. A suffix +// requires a decimal port in 1..65535: nonnumeric service names and +// out-of-range values yield no suffix. The scheme's default port yields NO +// suffix: 443 for TLS (direct or X-Forwarded-Proto: https) and 80 otherwise. +// This mirrors the SPA's API-origin derivation, where URL.port reports "" for +// the scheme-default port only — so http://host:443 stays "_443" on both +// sides instead of splitting one instance's cookies across two names. A nil +// request yields "". +func PortSuffix(r *http.Request) string { + if r == nil { + return "" + } + host := r.Host + if xfh := r.Header.Get("X-Forwarded-Host"); xfh != "" { + host = firstValue(xfh) + } + _, port, err := net.SplitHostPort(host) + if err != nil { + return "" + } + n, err := strconv.Atoi(port) + if err != nil || n < 1 || n > 65535 { + return "" + } + if n == defaultPortForRequest(r) { + return "" + } + return "_" + port +} + +// defaultPortForRequest returns the scheme's default port: 443 when the +// request arrived over TLS (directly or via X-Forwarded-Proto), 80 otherwise. +// Mirrors the Secure-cookie flag logic (requestIsTLS) so the two scheme +// decisions cannot diverge. +func defaultPortForRequest(r *http.Request) int { + if r.TLS != nil || strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") { + return 443 + } + return 80 +} + +// ScopedName returns base suffixed with the request's port suffix, or "" when +// base is empty (a suffix is meaningless without a base name). +func ScopedName(r *http.Request, base string) string { + if base == "" { + return "" + } + return base + PortSuffix(r) +} + +// firstValue returns the first comma-separated value, trimmed. +func firstValue(v string) string { + if i := strings.IndexByte(v, ','); i >= 0 { + return strings.TrimSpace(v[:i]) + } + return strings.TrimSpace(v) +} diff --git a/apps/backend/internal/common/httpcookie/httpcookie_test.go b/apps/backend/internal/common/httpcookie/httpcookie_test.go new file mode 100644 index 0000000000..7b33b66e55 --- /dev/null +++ b/apps/backend/internal/common/httpcookie/httpcookie_test.go @@ -0,0 +1,87 @@ +package httpcookie + +import ( + "crypto/tls" + "net/http" + "net/http/httptest" + "testing" +) + +func req(host, xfh string) *http.Request { + r := httptest.NewRequest("GET", "/", nil) + r.Host = host + if xfh != "" { + r.Header.Set("X-Forwarded-Host", xfh) + } + return r +} + +// tlsReq builds a request over TLS (direct), like req but with r.TLS set. +func tlsReq(host, xfh string) *http.Request { + r := req(host, xfh) + r.TLS = &tls.ConnectionState{} + return r +} + +// xfpReq builds a request marked https via X-Forwarded-Proto (proxied TLS). +func xfpReq(host, xfh string) *http.Request { + r := req(host, xfh) + r.Header.Set("X-Forwarded-Proto", "https") + return r +} + +func TestPortSuffix(t *testing.T) { + tests := []struct { + name string + req *http.Request + want string + }{ + {"ported host", req("127.0.0.1:8443", ""), "_8443"}, + {"no port", req("example.com", ""), ""}, + {"default port 80 http", req("example.com:80", ""), ""}, + {"default port 443 https", tlsReq("example.com:443", ""), ""}, + {"default port 443 https via xfp", xfpReq("example.com:443", ""), ""}, + {"non-default 443 on http", req("example.com:443", ""), "_443"}, + {"non-default 80 on https", tlsReq("example.com:80", ""), "_80"}, + {"XFH wins over host", req("internal:38429", "public.example:8443"), "_8443"}, + {"XFH wins on conflicting ports", req("public.example:8443", "public.example:9443"), "_9443"}, + {"XFH comma separated first", req("internal:38429", "public.example:8443, other:9443"), "_8443"}, + {"XFH whitespace padded", req("internal:38429", " public.example:8443 , other:9443"), "_8443"}, + {"XFH whitespace only", req("internal:38429", " "), ""}, + {"XFH no port", req("internal:38429", "public.example"), ""}, + {"XFH scheme-aware default", tlsReq("internal:38429", "public.example:443"), ""}, + {"IPv6 bracket", req("[::1]:8080", ""), "_8080"}, + {"IPv6 zone", req("[fe80::1%25eth0]:8080", ""), "_8080"}, + {"service name port", req("example.com:http", ""), ""}, + {"out of range port", req("example.com:99999", ""), ""}, + {"zero port", req("example.com:0", ""), ""}, + {"negative port", req("example.com:-1", ""), ""}, + {"malformed host", req("example.com:abc:def", ""), ""}, + {"unbracketed ipv6", req("::1", ""), ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := PortSuffix(tt.req); got != tt.want { + t.Fatalf("PortSuffix(%q) = %q, want %q", tt.req.Host, got, tt.want) + } + }) + } +} + +func TestPortSuffixNilRequest(t *testing.T) { + if got := PortSuffix(nil); got != "" { + t.Fatalf("PortSuffix(nil) = %q, want empty", got) + } +} + +func TestScopedName(t *testing.T) { + if got := ScopedName(req("127.0.0.1:8443", ""), "kandev_session"); got != "kandev_session_8443" { + t.Fatalf("ScopedName ported = %q, want kandev_session_8443", got) + } + if got := ScopedName(req("example.com", ""), "kandev_session"); got != "kandev_session" { + t.Fatalf("ScopedName no-port = %q, want kandev_session", got) + } + if got := ScopedName(req("127.0.0.1:8443", ""), ""); got != "" { + t.Fatalf("ScopedName empty base = %q, want empty", got) + } +} diff --git a/apps/backend/internal/gateway/websocket/port_proxy_auth_test.go b/apps/backend/internal/gateway/websocket/port_proxy_auth_test.go index 300e56aaac..1006b65f84 100644 --- a/apps/backend/internal/gateway/websocket/port_proxy_auth_test.go +++ b/apps/backend/internal/gateway/websocket/port_proxy_auth_test.go @@ -183,8 +183,10 @@ type proxyAuthGatewayResponse struct { // proxyAuthGateway issues requests against the full production chain: CORS, // auth middleware, gateway routes (requireConnectionAuth + port proxy), a -// token-enforcing fake agentctl, and the fake app. -func proxyAuthGateway(t *testing.T, handler http.Handler) func(method, path string, headers map[string]string) proxyAuthGatewayResponse { +// token-enforcing fake agentctl, and the fake app. It returns the request +// closure plus the fixture server URL so callers can derive the port-scoped +// session cookie name for its Host. +func proxyAuthGateway(t *testing.T, handler http.Handler) (func(method, path string, headers map[string]string) proxyAuthGatewayResponse, string) { t.Helper() srv := httptest.NewUnstartedServer(handler) srv.Config.SetKeepAlivesEnabled(false) @@ -219,7 +221,23 @@ func proxyAuthGateway(t *testing.T, handler http.Handler) func(method, path stri t.Fatalf("read body for %s: %v", path, err) } return proxyAuthGatewayResponse{status: resp.StatusCode, body: string(body), headers: resp.Header} + }, srv.URL +} + +// scopedSessionCookieHeader builds the Cookie header for the gateway fixture's +// Host (127.0.0.1:): the session cookie must carry the +// port-scoped name the auth middleware derives from that request Host. +func scopedSessionCookieHeader(t *testing.T, srvURL, sessionToken string) map[string]string { + t.Helper() + u, err := url.Parse(srvURL) + if err != nil { + t.Fatalf("parse server URL: %v", err) } + port := u.Port() + if port == "" { + t.Fatal("fixture server URL has no port") + } + return map[string]string{"Cookie": "kandev_session_" + port + "=" + sessionToken} } var manifestLinkPattern = regexp.MustCompile(`href="([^"]*manifest\.webmanifest[^"]*)"`) @@ -274,8 +292,8 @@ func TestPortProxyServesCredentiallessSubresourcesAfterDocumentAuth(t *testing.T gateway.SetPortProxy(manager) gateway.SetupRoutes(router) - request := proxyAuthGateway(t, router) - cookieHeader := map[string]string{"Cookie": svc.CookieName() + "=" + sessionToken} + request, srvURL := proxyAuthGateway(t, router) + cookieHeader := scopedSessionCookieHeader(t, srvURL, sessionToken) // 1. The document request authenticates with the session cookie. doc := request(http.MethodGet, "/port-proxy/sess-auth/5173/", cookieHeader) @@ -552,9 +570,9 @@ func TestPortProxyDecompressesAndRewritesCompressedHTML(t *testing.T) { gateway.SetPortProxy(manager) gateway.SetupRoutes(router) - request := proxyAuthGateway(t, router) + request, srvURL := proxyAuthGateway(t, router) headers := map[string]string{ - "Cookie": svc.CookieName() + "=" + sessionToken, + "Cookie": scopedSessionCookieHeader(t, srvURL, sessionToken)["Cookie"], "Accept-Encoding": "gzip", } doc := request(http.MethodGet, "/port-proxy/sess-gz/5173/", headers) @@ -616,8 +634,8 @@ func TestPortProxyRewritesNestedCSSWithPublicBase(t *testing.T) { gateway.SetPortProxy(manager) gateway.SetupRoutes(router) - request := proxyAuthGateway(t, router) - headers := map[string]string{"Cookie": svc.CookieName() + "=" + sessionToken} + request, srvURL := proxyAuthGateway(t, router) + headers := map[string]string{"Cookie": scopedSessionCookieHeader(t, srvURL, sessionToken)["Cookie"]} doc := request(http.MethodGet, "/port-proxy/sess-css/5173/css/main.css", headers) if doc.status != http.StatusOK { t.Fatalf("css status = %d, want %d (body=%s)", doc.status, http.StatusOK, doc.body) diff --git a/apps/backend/internal/plugins/auth_login.go b/apps/backend/internal/plugins/auth_login.go index 0574de2fdf..852111009d 100644 --- a/apps/backend/internal/plugins/auth_login.go +++ b/apps/backend/internal/plugins/auth_login.go @@ -28,11 +28,12 @@ type externalLoginAssertion struct { // AuthLoginBridge establishes an authenticated kandev browser session for an // external identity that an auth-capable plugin has validated against an -// external IdP (OIDC/SAML SSO). The host mints and sets the kandev_session -// cookie on the response; the plugin never receives the raw session token — -// its only channel is the reserved login directive header on its webhook -// response (see handlers.go). Implemented in internal/backendapp over -// internal/auth so this package need not depend on the auth service. +// external IdP (OIDC/SAML SSO). The host mints and sets the session cookie +// (name derived from the request host) on the response; the plugin never +// receives the raw session token — its only channel is the reserved login +// directive header on its webhook response (see handlers.go). Implemented in +// internal/backendapp over internal/auth so this package need not depend on +// the auth service. type AuthLoginBridge interface { // LoginExternal maps the asserted external identity to a kandev user // (linking or just-in-time provisioning as needed) and sets the session diff --git a/apps/web/app/office/agents/page.tsx b/apps/web/app/office/agents/page.tsx deleted file mode 100644 index e43b474534..0000000000 --- a/apps/web/app/office/agents/page.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import { listAgentProfiles } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "../lib/get-active-workspace"; -import { AgentsPageClient } from "./agents-page-client"; -import type { AgentProfile } from "@/lib/state/slices/office/types"; - -export default async function AgentsPage() { - const workspaceId = await getActiveWorkspaceId(); - - let agents: AgentProfile[] = []; - if (workspaceId) { - const res = await listAgentProfiles(workspaceId, { cache: "no-store" }).catch(() => ({ - agents: [], - })); - agents = res.agents ?? []; - } - - return ; -} diff --git a/apps/web/app/office/components/office-shell.tsx b/apps/web/app/office/components/office-shell.tsx index ddac8db916..2302a6ec62 100644 --- a/apps/web/app/office/components/office-shell.tsx +++ b/apps/web/app/office/components/office-shell.tsx @@ -94,9 +94,9 @@ function OfficePageNav({ onClose }: { onClose: () => void }) { /** * Office page chrome on the shared `PageShell`. List pages show a static * title; detail pages render a portal target (#office-topbar-slot) that the - * page component fills with its breadcrumb via OfficeTopbarPortal. Both paint - * paths (`src/office-routes.tsx` and `app/office/layout.tsx`) wrap their route - * output in this shell so they cannot drift. + * page component fills with its breadcrumb via OfficeTopbarPortal. The SPA + * office paint path (`src/office-routes.tsx`) wraps every route's output in + * this shell so the chrome cannot drift. */ export function OfficeShell({ children, @@ -147,9 +147,9 @@ export function OfficeShell({ the specific route, so a mis-pointed URL cannot match another screen's anchor. - The shell owns this outlet (`scroll="none"`) so both paint paths — - `src/office-routes.tsx` and `app/office/layout.tsx` — carry the - anchor without either one re-declaring it. */} + The shell owns this outlet (`scroll="none"`) so the single SPA paint + path (`src/office-routes.tsx`) carries the anchor without + re-declaring it. */}
{children}
diff --git a/apps/web/app/office/inbox/page.tsx b/apps/web/app/office/inbox/page.tsx deleted file mode 100644 index a440048252..0000000000 --- a/apps/web/app/office/inbox/page.tsx +++ /dev/null @@ -1,25 +0,0 @@ -import { getInbox } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "../lib/get-active-workspace"; -import { InboxPageClient } from "./inbox-page-client"; -import type { InboxItem } from "@/lib/state/slices/office/types"; - -export default async function InboxPage() { - const workspaceId = await getActiveWorkspaceId(); - - let items: InboxItem[] = []; - let count = 0; - if (workspaceId) { - // Single round-trip: getInbox returns items + total_count - // (Stream F of office optimization). Was 2 parallel calls. - const res = await getInbox(workspaceId, { cache: "no-store" }).catch(() => ({ - items: [] as InboxItem[], - total_count: 0, - })); - items = res.items ?? []; - count = res.total_count ?? items.length; - } - - return ( - - ); -} diff --git a/apps/web/app/office/layout.tsx b/apps/web/app/office/layout.tsx deleted file mode 100644 index 1268df5698..0000000000 --- a/apps/web/app/office/layout.tsx +++ /dev/null @@ -1,211 +0,0 @@ -import { notFound } from "@/lib/routing/server-navigation"; -import { TooltipProvider } from "@kandev/ui/tooltip"; -import { StateHydrator } from "@/components/state-hydrator"; -import { getFeatureFlagsAction } from "@/app/actions/features"; -import { listWorkspaces, fetchUserSettings } from "@/lib/api"; -import { - getInbox, - getMeta, - getOnboardingState, - listAgentProfiles, - listProjects, -} from "@/lib/api/domains/office-api"; -import { mapUserSettingsResponse } from "@/lib/ssr/user-settings"; -import { readCookies } from "@/lib/server/cookies"; -import type { AppState } from "@/lib/state/store"; -import { OfficeShell } from "./components/office-shell"; - -function resolveActiveOfficeWorkspaceId( - workspaceItems: { id: string }[], - cookieWorkspaceId: string | null, - settingsWorkspaceId: string | null, -): string | null { - return ( - workspaceItems.find((w) => w.id === cookieWorkspaceId)?.id ?? - workspaceItems.find((w) => w.id === settingsWorkspaceId)?.id ?? - workspaceItems[0]?.id ?? - null - ); -} - -function mapWorkspaceItem(ws: { - id: string; - name: string; - description?: string | null; - owner_id: string; - default_executor_id?: string | null; - default_environment_id?: string | null; - default_agent_profile_id?: string | null; - default_config_agent_profile_id?: string | null; - office_workflow_id?: string | null; - created_at: string; - updated_at: string; -}) { - return { - id: ws.id, - name: ws.name, - description: ws.description ?? null, - owner_id: ws.owner_id, - default_executor_id: ws.default_executor_id ?? null, - default_environment_id: ws.default_environment_id ?? null, - default_agent_profile_id: ws.default_agent_profile_id ?? null, - default_config_agent_profile_id: ws.default_config_agent_profile_id ?? null, - office_workflow_id: ws.office_workflow_id ?? null, - created_at: ws.created_at, - updated_at: ws.updated_at, - }; -} - -/** - * Wraps a workspace-scoped value in the `Record` shape the - * office slice stores. No active workspace means no entry at all, rather than - * data filed under an empty-string key that nothing would ever read. - */ -function keyByWorkspace(workspaceId: string | null, value: T): Record { - return workspaceId ? { [workspaceId]: value } : {}; -} - -/** - * The office slice as this layout hydrates it: the four workspace-scoped - * collections filed under the workspace they were fetched for, and every other - * field at its default. `meta` is supplied by the caller. - */ -function officeHydrationState({ - activeWorkspaceId, - agents, - projects, - inboxItems, - inboxCount, -}: { - activeWorkspaceId: string | null; - agents: AppState["office"]["agentProfilesByWorkspaceId"][string]; - projects: AppState["office"]["projectsByWorkspaceId"][string]; - inboxItems: AppState["office"]["inboxItemsByWorkspaceId"][string]; - inboxCount: number; -}): Omit { - return { - agentProfilesByWorkspaceId: keyByWorkspace(activeWorkspaceId, agents), - skills: [], - projectsByWorkspaceId: keyByWorkspace(activeWorkspaceId, projects), - approvals: [], - activity: [], - costSummary: null, - budgetPolicies: [], - routines: [], - inboxItemsByWorkspaceId: keyByWorkspace(activeWorkspaceId, inboxItems), - inboxCountByWorkspaceId: keyByWorkspace(activeWorkspaceId, inboxCount), - runs: [], - dashboardByWorkspaceId: {}, - tasks: { - items: [], - filters: { statuses: [], priorities: [], assigneeIds: [], projectIds: [], search: "" }, - viewMode: "list", - sortField: "updated", - sortDir: "desc", - groupBy: "none", - nestingEnabled: true, - isLoading: false, - }, - isLoading: false, - refetchTrigger: null, - routing: { byWorkspace: {}, knownProviders: [], preview: { byWorkspace: {} } }, - providerHealth: { byWorkspace: {} }, - runAttempts: { byRunId: {} }, - agentRouting: { byAgentId: {} }, - }; -} - -export default async function OfficeLayout({ children }: { children: React.ReactNode }) { - // Feature gate: production releases ship with features.office=false and - // the backend's /api/v1/office/* routes are not registered. Return 404 - // for every Office page so even a guessed URL looks like a non-existent - // route, not "you don't have permission". - // See docs/decisions/0007-runtime-feature-flags.md. - const { office: officeEnabled } = await getFeatureFlagsAction(); - if (!officeEnabled) { - notFound(); - } - - // Check onboarding before rendering the office chrome. When not complete, - // render only the children (the setup wizard) without the workspace rail, - // sidebar, or topbar — prevents a flash of stale workspace UI. - const onboarding = await getOnboardingState({ cache: "no-store" }).catch(() => ({ - completed: false, - fsWorkspaces: [], - })); - if (!onboarding.completed) { - return <>{children}; - } - - const [workspacesResponse, userSettingsResponse, metaResponse, cookieStore] = await Promise.all([ - listWorkspaces({ cache: "no-store" }).catch(() => ({ workspaces: [] })), - fetchUserSettings({ cache: "no-store" }).catch(() => null), - getMeta({ cache: "no-store" }).catch(() => null), - readCookies().catch(() => null), - ]); - - // Only office workspaces can be active inside /office, but hydrate every - // workspace into the shared sidebar picker so users can switch back to Kanban. - const officeWorkspaceItems = workspacesResponse.workspaces - .filter((ws) => ws.office_workflow_id) - .map(mapWorkspaceItem); - const workspaceItems = workspacesResponse.workspaces.map(mapWorkspaceItem); - const settingsWorkspaceId = userSettingsResponse?.settings?.workspace_id || null; - // office-active-workspace cookie is set by the setup wizard and workspace rail so the - // layout can load the correct workspace even when userSettings.workspace_id still points - // to a kanban workspace (we never write to userSettings from office). - const cookieWorkspaceId = cookieStore?.get("office-active-workspace")?.value ?? null; - const activeWorkspaceId = resolveActiveOfficeWorkspaceId( - officeWorkspaceItems, - cookieWorkspaceId, - settingsWorkspaceId, - ); - - // Fetch agents + projects + inbox for the active workspace so the - // sidebar renders them — including the inbox count badge — on first - // paint without a client refetch. - const [agentsResponse, projectsResponse, inboxResponse] = activeWorkspaceId - ? await Promise.all([ - listAgentProfiles(activeWorkspaceId, { cache: "no-store" }).catch(() => ({ - agents: [], - })), - listProjects(activeWorkspaceId, { cache: "no-store" }).catch(() => ({ - projects: [], - })), - getInbox(activeWorkspaceId, { cache: "no-store" }).catch(() => ({ - items: [], - total_count: 0, - })), - ]) - : [{ agents: [] }, { projects: [] }, { items: [], total_count: 0 }]; - - const initialState: Partial = { - workspaces: { - items: workspaceItems, - activeId: activeWorkspaceId, - }, - userSettings: { - ...mapUserSettingsResponse(userSettingsResponse), - workspaceId: activeWorkspaceId, - }, - office: { - ...officeHydrationState({ - activeWorkspaceId, - agents: agentsResponse.agents as AppState["office"]["agentProfilesByWorkspaceId"][string], - projects: projectsResponse.projects as AppState["office"]["projectsByWorkspaceId"][string], - inboxItems: inboxResponse.items as AppState["office"]["inboxItemsByWorkspaceId"][string], - inboxCount: inboxResponse.total_count, - }), - meta: metaResponse, - }, - }; - - return ( - - -
- {children} -
- - ); -} diff --git a/apps/web/app/office/lib/get-active-workspace.test.ts b/apps/web/app/office/lib/get-active-workspace.test.ts deleted file mode 100644 index a7dd8f3ee1..0000000000 --- a/apps/web/app/office/lib/get-active-workspace.test.ts +++ /dev/null @@ -1,141 +0,0 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; - -vi.mock("@/lib/server/cookies", () => ({ - readCookies: vi.fn(), -})); - -vi.mock("@/lib/api", () => ({ - listWorkspaces: vi.fn(), - fetchUserSettings: vi.fn(), -})); - -vi.mock("@/lib/api/domains/settings-api", () => ({ - updateUserSettings: vi.fn(), -})); - -import { readCookies } from "@/lib/server/cookies"; -import { listWorkspaces, fetchUserSettings } from "@/lib/api"; -import { updateUserSettings } from "@/lib/api/domains/settings-api"; -import { getActiveWorkspaceId } from "./get-active-workspace"; - -const mockReadCookies = vi.mocked(readCookies); -const mockListWorkspaces = vi.mocked(listWorkspaces); -const mockFetchUserSettings = vi.mocked(fetchUserSettings); -const mockUpdateUserSettings = vi.mocked(updateUserSettings); - -const OFFICE_WS_ID = "ws-office-1"; -const OFFICE_WS_ID_2 = "ws-office-2"; -const KANBAN_WS_ID = "ws-kanban-1"; -const OFFICE_WS = { id: OFFICE_WS_ID, name: "Office", office_workflow_id: "wf-1" }; -const OFFICE_WS_2 = { id: OFFICE_WS_ID_2, name: "Office 2", office_workflow_id: "wf-2" }; -const KANBAN_WS = { id: KANBAN_WS_ID, name: "Kanban" }; // no office_workflow_id - -function makeSettings(workspaceId: string) { - return { settings: { workspace_id: workspaceId } }; -} - -function makeCookieStore(workspaceId: string | null) { - return { - get: (name: string) => - name === "office-active-workspace" && workspaceId ? { value: workspaceId } : undefined, - }; -} - -beforeEach(() => { - mockReadCookies.mockReset(); - mockListWorkspaces.mockReset(); - mockFetchUserSettings.mockReset(); - mockUpdateUserSettings.mockReset(); - mockUpdateUserSettings.mockResolvedValue({} as never); - // Default: no cookie set - mockReadCookies.mockResolvedValue(makeCookieStore(null) as never); -}); - -afterEach(() => { - vi.clearAllMocks(); -}); - -describe("getActiveWorkspaceId", () => { - describe("URL workspaceId arg wins when it matches an office workspace", () => { - it("returns the URL workspace id and does NOT call updateUserSettings", async () => { - mockListWorkspaces.mockResolvedValue({ workspaces: [OFFICE_WS] } as never); - mockFetchUserSettings.mockResolvedValue(makeSettings("ws-other-1") as never); - - const result = await getActiveWorkspaceId(OFFICE_WS_ID); - - expect(result).toBe(OFFICE_WS_ID); - expect(mockUpdateUserSettings).not.toHaveBeenCalled(); - }); - - it("ignores the URL param when it does not match any office workspace", async () => { - mockListWorkspaces.mockResolvedValue({ workspaces: [OFFICE_WS] } as never); - mockFetchUserSettings.mockResolvedValue(makeSettings(OFFICE_WS_ID) as never); - - // "ws-unknown" not in office list - falls back to settings match - const result = await getActiveWorkspaceId("ws-unknown-1"); - - expect(result).toBe(OFFICE_WS_ID); - expect(mockUpdateUserSettings).not.toHaveBeenCalled(); - }); - }); - - describe("cookie wins after URL param misses", () => { - it("returns the cookie workspace and does NOT call updateUserSettings", async () => { - mockReadCookies.mockResolvedValue(makeCookieStore(OFFICE_WS_ID_2) as never); - mockListWorkspaces.mockResolvedValue({ workspaces: [OFFICE_WS, OFFICE_WS_2] } as never); - mockFetchUserSettings.mockResolvedValue(makeSettings(KANBAN_WS_ID) as never); - - const result = await getActiveWorkspaceId(); - - expect(result).toBe(OFFICE_WS_ID_2); - expect(mockUpdateUserSettings).not.toHaveBeenCalled(); - }); - - it("ignores cookie when it does not match any office workspace", async () => { - mockReadCookies.mockResolvedValue(makeCookieStore("ws-stale") as never); - mockListWorkspaces.mockResolvedValue({ workspaces: [OFFICE_WS] } as never); - mockFetchUserSettings.mockResolvedValue(makeSettings(OFFICE_WS_ID) as never); - - const result = await getActiveWorkspaceId(); - - expect(result).toBe(OFFICE_WS_ID); - expect(mockUpdateUserSettings).not.toHaveBeenCalled(); - }); - }); - - describe("regression: kanban workspace in settings must not trigger updateUserSettings", () => { - it("falls back to first office workspace and does NOT call updateUserSettings", async () => { - mockListWorkspaces.mockResolvedValue({ workspaces: [KANBAN_WS, OFFICE_WS] } as never); - mockFetchUserSettings.mockResolvedValue(makeSettings(KANBAN_WS_ID) as never); - - const result = await getActiveWorkspaceId(); - - expect(result).toBe(OFFICE_WS_ID); - expect(mockUpdateUserSettings).not.toHaveBeenCalled(); - }); - }); - - describe("settings.workspace_id points to a valid office workspace", () => { - it("returns the settings workspace id and does NOT call updateUserSettings", async () => { - mockListWorkspaces.mockResolvedValue({ workspaces: [OFFICE_WS] } as never); - mockFetchUserSettings.mockResolvedValue(makeSettings(OFFICE_WS_ID) as never); - - const result = await getActiveWorkspaceId(); - - expect(result).toBe(OFFICE_WS_ID); - expect(mockUpdateUserSettings).not.toHaveBeenCalled(); - }); - }); - - describe("no office workspaces exist", () => { - it("returns null and does not call updateUserSettings", async () => { - mockListWorkspaces.mockResolvedValue({ workspaces: [] } as never); - mockFetchUserSettings.mockResolvedValue(makeSettings(OFFICE_WS_ID) as never); - - const result = await getActiveWorkspaceId(); - - expect(result).toBeNull(); - expect(mockUpdateUserSettings).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/apps/web/app/office/lib/get-active-workspace.ts b/apps/web/app/office/lib/get-active-workspace.ts deleted file mode 100644 index a350646ed8..0000000000 --- a/apps/web/app/office/lib/get-active-workspace.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { listWorkspaces, fetchUserSettings } from "@/lib/api"; -import { readCookies } from "@/lib/server/cookies"; - -/** - * Server-side helper to resolve the active workspace ID. - * Re-fetches workspaces and user settings (Next.js will deduplicate - * these calls within the same request when the layout also fetches them). - * - * Priority order: - * 1. urlWorkspaceId when it matches a valid office workspace. - * 2. office-active-workspace cookie when it matches a valid office workspace. - * 3. userSettings.workspace_id when it matches a valid office workspace. - * 4. First available office workspace as fallback. - * - * Does NOT write to user settings - the caller must not pollute the shared - * workspace_id that kanban uses. - */ -export async function getActiveWorkspaceId(urlWorkspaceId?: string): Promise { - const [workspacesRes, settingsRes, cookieStore] = await Promise.all([ - listWorkspaces({ cache: "no-store" }).catch(() => ({ workspaces: [] })), - fetchUserSettings({ cache: "no-store" }).catch(() => null), - readCookies().catch(() => null), - ]); - - // Only consider office workspaces (those with office_workflow_id set). - const workspaces = workspacesRes.workspaces.filter((w) => w.office_workflow_id); - - // 1. URL param wins when it matches a valid office workspace. - if (urlWorkspaceId) { - const urlMatch = workspaces.find((w) => w.id === urlWorkspaceId); - if (urlMatch) return urlMatch.id; - } - - // 2. Cookie set by the setup wizard and workspace rail - keeps page and - // layout in sync since layouts cannot read URL search params. - const cookieWorkspaceId = cookieStore?.get("office-active-workspace")?.value; - if (cookieWorkspaceId) { - const cookieMatch = workspaces.find((w) => w.id === cookieWorkspaceId); - if (cookieMatch) return cookieMatch.id; - } - - // 3. Check if the settings workspace_id points to a valid office workspace. - const settingsWorkspaceId = settingsRes?.settings?.workspace_id || null; - const matched = workspaces.find((w) => w.id === settingsWorkspaceId); - if (matched) { - return matched.id; - } - - // 4. Fall back to the first available office workspace. - // Do NOT persist this - userSettings.workspace_id belongs to kanban. - return workspaces[0]?.id ?? null; -} diff --git a/apps/web/app/office/page.test.tsx b/apps/web/app/office/page.test.tsx deleted file mode 100644 index 5b1aa05870..0000000000 --- a/apps/web/app/office/page.test.tsx +++ /dev/null @@ -1,51 +0,0 @@ -import { describe, expect, it, vi, beforeEach } from "vitest"; - -const mocks = vi.hoisted(() => ({ - redirect: vi.fn((path: string) => { - throw new Error(`redirect:${path}`); - }), - getOnboardingState: vi.fn(), - getDashboard: vi.fn(), - getActiveWorkspaceId: vi.fn(), -})); - -vi.mock("@/lib/routing/server-navigation", () => ({ - redirect: mocks.redirect, -})); - -vi.mock("@/lib/api/domains/office-api", () => ({ - getOnboardingState: mocks.getOnboardingState, - getDashboard: mocks.getDashboard, -})); - -vi.mock("./lib/get-active-workspace", () => ({ - getActiveWorkspaceId: mocks.getActiveWorkspaceId, -})); - -vi.mock("./page-client", () => ({ - OfficePageClient: ({ initialDashboard }: { initialDashboard: unknown }) => ( -
{JSON.stringify(initialDashboard)}
- ), -})); - -import OfficePage from "./page"; - -describe("OfficePage", () => { - beforeEach(() => { - mocks.redirect.mockClear(); - mocks.getOnboardingState.mockReset(); - mocks.getDashboard.mockReset(); - mocks.getActiveWorkspaceId.mockReset(); - }); - - it("redirects to setup-new when no office workspace exists", async () => { - mocks.getOnboardingState.mockResolvedValue({ completed: true }); - mocks.getActiveWorkspaceId.mockResolvedValue(null); - - await expect(OfficePage({ searchParams: Promise.resolve({}) })).rejects.toThrow( - "redirect:/office/setup?mode=new", - ); - expect(mocks.redirect).toHaveBeenCalledWith("/office/setup?mode=new"); - expect(mocks.getDashboard).not.toHaveBeenCalled(); - }); -}); diff --git a/apps/web/app/office/page.tsx b/apps/web/app/office/page.tsx deleted file mode 100644 index d670634a77..0000000000 --- a/apps/web/app/office/page.tsx +++ /dev/null @@ -1,25 +0,0 @@ -import { redirect } from "@/lib/routing/server-navigation"; -import { getDashboard, getOnboardingState } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "./lib/get-active-workspace"; -import { OfficePageClient } from "./page-client"; - -type SearchParams = Promise<{ workspaceId?: string }>; - -export default async function OfficePage({ searchParams }: { searchParams?: SearchParams }) { - const onboarding = await getOnboardingState({ cache: "no-store" }).catch(() => ({ - completed: true, - })); - if (!onboarding.completed) { - redirect("/office/setup"); - } - - const params = searchParams ? await searchParams : {}; - const workspaceId = await getActiveWorkspaceId(params.workspaceId); - if (!workspaceId) { - redirect("/office/setup?mode=new"); - } - - const dashboard = await getDashboard(workspaceId, { cache: "no-store" }).catch(() => null); - - return ; -} diff --git a/apps/web/app/office/projects/page.tsx b/apps/web/app/office/projects/page.tsx deleted file mode 100644 index 4251c748af..0000000000 --- a/apps/web/app/office/projects/page.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import { listProjects } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "../lib/get-active-workspace"; -import { ProjectsPageClient } from "./projects-page-client"; -import type { Project } from "@/lib/state/slices/office/types"; - -export default async function ProjectsPage() { - const workspaceId = await getActiveWorkspaceId(); - - let projects: Project[] = []; - if (workspaceId) { - const res = await listProjects(workspaceId, { cache: "no-store" }).catch(() => ({ - projects: [], - })); - projects = res.projects ?? []; - } - - return ; -} diff --git a/apps/web/app/office/routines/page.tsx b/apps/web/app/office/routines/page.tsx deleted file mode 100644 index 2bb2345450..0000000000 --- a/apps/web/app/office/routines/page.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import { listRoutines } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "../lib/get-active-workspace"; -import { RoutinesPageClient } from "./routines-page-client"; -import type { Routine } from "@/lib/state/slices/office/types"; - -export default async function RoutinesPage() { - const workspaceId = await getActiveWorkspaceId(); - - let routines: Routine[] = []; - if (workspaceId) { - const res = await listRoutines(workspaceId, { cache: "no-store" }).catch(() => ({ - routines: [], - })); - routines = res.routines ?? []; - } - - return ; -} diff --git a/apps/web/app/office/tasks/page.tsx b/apps/web/app/office/tasks/page.tsx deleted file mode 100644 index fe0ddcb493..0000000000 --- a/apps/web/app/office/tasks/page.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import { listTasks } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "../lib/get-active-workspace"; -import { TasksPageClient } from "./tasks-page-client"; -import type { OfficeTask } from "@/lib/state/slices/office/types"; - -export default async function TasksPage() { - const workspaceId = await getActiveWorkspaceId(); - - let tasks: OfficeTask[] = []; - if (workspaceId) { - const res = await listTasks(workspaceId, { cache: "no-store" }).catch(() => ({ - tasks: [], - })); - tasks = res.tasks ?? []; - } - - return ; -} diff --git a/apps/web/app/office/workspace/activity/page.tsx b/apps/web/app/office/workspace/activity/page.tsx deleted file mode 100644 index ee38505d1f..0000000000 --- a/apps/web/app/office/workspace/activity/page.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import { listActivity } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "../../lib/get-active-workspace"; -import { ActivityPageClient } from "./activity-page-client"; -import type { ActivityEntry } from "@/lib/state/slices/office/types"; - -export default async function ActivityPage() { - const workspaceId = await getActiveWorkspaceId(); - - let activity: ActivityEntry[] = []; - if (workspaceId) { - const res = await listActivity(workspaceId, undefined, { cache: "no-store" }).catch(() => ({ - activity: [], - })); - activity = res.activity ?? []; - } - - return ; -} diff --git a/apps/web/app/office/workspace/costs/page.tsx b/apps/web/app/office/workspace/costs/page.tsx deleted file mode 100644 index 9bd9364e24..0000000000 --- a/apps/web/app/office/workspace/costs/page.tsx +++ /dev/null @@ -1,15 +0,0 @@ -import { getCosts } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "../../lib/get-active-workspace"; -import { CostsPageClient } from "./costs-page-client"; -import type { CostSummary } from "@/lib/state/slices/office/types"; - -export default async function CostsPage() { - const workspaceId = await getActiveWorkspaceId(); - - let costSummary: CostSummary | null = null; - if (workspaceId) { - costSummary = await getCosts(workspaceId, { cache: "no-store" }).catch(() => null); - } - - return ; -} diff --git a/apps/web/app/office/workspace/skills/page.tsx b/apps/web/app/office/workspace/skills/page.tsx deleted file mode 100644 index 66fa5c36b4..0000000000 --- a/apps/web/app/office/workspace/skills/page.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import { listSkills } from "@/lib/api/domains/office-api"; -import { getActiveWorkspaceId } from "../../lib/get-active-workspace"; -import { SkillsPageClient } from "./skills-page-client"; -import type { Skill } from "@/lib/state/slices/office/types"; - -export default async function SkillsPage() { - const workspaceId = await getActiveWorkspaceId(); - - let skills: Skill[] = []; - if (workspaceId) { - const res = await listSkills(workspaceId, { cache: "no-store" }).catch(() => ({ - skills: [], - })); - skills = res.skills ?? []; - } - - return ; -} diff --git a/apps/web/app/page.test.ts b/apps/web/app/page.test.ts deleted file mode 100644 index 0bd8a48561..0000000000 --- a/apps/web/app/page.test.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { workspaceId, workflowId } from "@/lib/types/ids"; -import type { ListWorkspacesResponse } from "@/lib/types/http"; -import { resolveActiveKanbanWorkspaceId } from "./page"; - -type WorkspaceRow = ListWorkspacesResponse["workspaces"][number]; - -describe("resolveActiveKanbanWorkspaceId", () => { - it("ignores office workspaces when resolving cookie and settings IDs", () => { - const activeId = resolveActiveKanbanWorkspaceId( - [workspaceRow("ws-office", "office-flow"), workspaceRow("ws-kanban", null)], - undefined, - "ws-office", - "ws-kanban", - ); - - expect(activeId).toBe("ws-kanban"); - }); - - it("falls back when explicit URL workspace ID is an office workspace", () => { - const activeId = resolveActiveKanbanWorkspaceId( - [workspaceRow("ws-office", "office-flow"), workspaceRow("ws-kanban", null)], - "ws-office", - null, - "ws-kanban", - ); - - expect(activeId).toBe("ws-kanban"); - }); -}); - -function workspaceRow(id: string, officeWorkflowId: string | null): WorkspaceRow { - return { - id: workspaceId(id), - name: id, - owner_id: "owner-1", - office_workflow_id: officeWorkflowId ? workflowId(officeWorkflowId) : undefined, - created_at: "2026-01-01T00:00:00Z", - updated_at: "2026-01-01T00:00:00Z", - } as WorkspaceRow; -} diff --git a/apps/web/app/page.tsx b/apps/web/app/page.tsx deleted file mode 100644 index 572cce89cb..0000000000 --- a/apps/web/app/page.tsx +++ /dev/null @@ -1,277 +0,0 @@ -import { PageClient } from "@/app/page-client"; -import { StateHydrator } from "@/components/state-hydrator"; -import { - fetchWorkflowSnapshot, - fetchUserSettings, - listWorkflows, - listRepositories, - listWorkspaces, - listTaskSessionMessages, - listQuickChatSessions, - listQuickTerminalTabs, -} from "@/lib/api"; -import { listWorkspaceTaskPRs } from "@/lib/api/domains/github-api"; -import { toQuickChatSessions } from "@/lib/quick-chat/map-sessions"; -import { toQuickTerminalTab } from "@/lib/api/domains/quick-terminal-api"; -import { snapshotToState } from "@/lib/ssr/mapper"; -import { mapUserSettingsResponse } from "@/lib/ssr/user-settings"; -import { resolveDesiredWorkflowId } from "@/lib/kanban/resolve-workflow"; -import { ACTIVE_WORKSPACE_COOKIE } from "@/lib/routing/route-bootstrap"; -import { resolveActiveId } from "@/lib/ssr/resolve-active-id"; -import { readCookies } from "@/lib/server/cookies"; -import type { AppState } from "@/lib/state/store"; -import type { ListWorkspacesResponse, UserSettingsResponse } from "@/lib/types/http"; - -// Root page loader: keeps the old route shape while SPA boot data owns hydration. -type PageProps = { - searchParams?: Promise>; -}; - -function resolveParam(value: string | string[] | undefined): string | undefined { - return Array.isArray(value) ? value[0] : value; -} - -type WorkspaceItem = ListWorkspacesResponse["workspaces"][number]; -function mapWorkspaceItem(ws: WorkspaceItem) { - return { - id: ws.id, - name: ws.name, - description: ws.description ?? null, - owner_id: ws.owner_id, - default_executor_id: ws.default_executor_id ?? null, - default_environment_id: ws.default_environment_id ?? null, - default_agent_profile_id: ws.default_agent_profile_id ?? null, - default_config_agent_profile_id: ws.default_config_agent_profile_id ?? null, - office_workflow_id: ws.office_workflow_id ?? null, - created_at: ws.created_at, - updated_at: ws.updated_at, - }; -} - -function buildUserSettingsState( - resp: UserSettingsResponse | null, - workspaceId: string | null, -): AppState["userSettings"] { - return { ...mapUserSettingsResponse(resp), workspaceId }; -} - -function buildBaseState( - workspaces: ListWorkspacesResponse, - userSettingsResponse: UserSettingsResponse | null, - activeWorkspaceId: string | null, -): Partial { - return { - workspaces: { - items: workspaces.workspaces.map(mapWorkspaceItem), - activeId: activeWorkspaceId, - }, - userSettings: buildUserSettingsState(userSettingsResponse, activeWorkspaceId), - }; -} - -export function resolveActiveKanbanWorkspaceId( - workspaces: WorkspaceItem[], - workspaceId: string | undefined, - cookieWorkspaceId: string | null, - settingsWorkspaceId: string | null, -): string | null { - const kanbanWorkspaces = workspaces.filter((workspace) => !workspace.office_workflow_id); - return resolveActiveId(kanbanWorkspaces, workspaceId, cookieWorkspaceId, settingsWorkspaceId); -} - -async function loadSnapshotState( - workflowId: string, - taskId: string | undefined, - sessionId: string | undefined, -): Promise> { - const [snapshot, messagesResponse] = await Promise.all([ - fetchWorkflowSnapshot(workflowId, { cache: "no-store" }), - taskId && sessionId - ? listTaskSessionMessages( - sessionId, - { limit: 50, sort: "desc" }, - { cache: "no-store" }, - ).catch(() => null) - : Promise.resolve(null), - ]); - const state: Partial = { ...snapshotToState(snapshot) }; - - if (sessionId && messagesResponse) { - const messages = [...(messagesResponse.messages ?? [])].reverse(); - state.messages = { - bySession: { [sessionId]: messages }, - metaBySession: { - [sessionId]: { - isLoading: false, - isLoadingMore: false, - hasMore: messagesResponse.has_more ?? false, - oldestCursor: messages[0]?.id ?? null, - }, - }, - }; - } - return state; -} - -async function loadWorkspaceState({ - activeWorkspaceId, - initialState, - settingsWorkflowId, - workflowIdParam, -}: { - activeWorkspaceId: string; - initialState: Partial; - settingsWorkflowId: string | null; - workflowIdParam: string | undefined; -}) { - // Fire-and-forget: warm the backend PR cache for this workspace. - // The client will fetch the data after mount via useWorkspacePRs. - listWorkspaceTaskPRs(activeWorkspaceId, { cache: "no-store" }).catch(() => {}); - - const [workflowList, repositoriesResponse, quickChatResponse, quickTerminalResponse] = - await Promise.all([ - listWorkflows(activeWorkspaceId, { cache: "no-store", includeHidden: true }), - listRepositories(activeWorkspaceId, undefined, { cache: "no-store" }) - .then((response) => ({ ...response, loaded: true })) - .catch(() => ({ repositories: [], loaded: false })), - listQuickChatSessions(activeWorkspaceId, { cache: "no-store" }).catch(() => ({ - sessions: [], - task_sessions: [], - })), - listQuickTerminalTabs(activeWorkspaceId, { cache: "no-store" }).catch(() => ({ tabs: [] })), - ]); - // null preserves the user's explicit "All Workflows" choice. - const workflowId = resolveDesiredWorkflowId({ - activeWorkflowId: workflowIdParam ?? null, - settingsWorkflowId, - workspaceWorkflows: workflowList.workflows, - }); - const quickChatSessions = toQuickChatSessions(quickChatResponse.sessions); - const quickTerminalTabs = quickTerminalResponse.tabs.map(toQuickTerminalTab); - - return { - workflowId, - initialState: { - ...initialState, - userSettings: { - ...(initialState.userSettings as AppState["userSettings"]), - workflowId, - }, - workflows: { - items: workflowList.workflows.map((workflow) => ({ - id: workflow.id, - workspaceId: workflow.workspace_id, - name: workflow.name, - hidden: workflow.hidden, - })), - activeId: workflowId, - }, - repositories: { - itemsByWorkspaceId: { [activeWorkspaceId]: repositoriesResponse.repositories }, - loadingByWorkspaceId: { [activeWorkspaceId]: false }, - // A failed SSR fetch must remain retryable on the client. Marking an - // empty fallback as loaded leaves task creation permanently disabled - // because the dialog never gets a repository selection. - loadedByWorkspaceId: { [activeWorkspaceId]: repositoriesResponse.loaded }, - }, - quickChat: { - isOpen: false, - sessions: quickChatSessions, - activeSessionId: null, - terminalTabs: quickTerminalTabs, - activeKind: "conversation" as const, - activeTerminalTabId: null, - lastTerminalTabIdByWorkspace: {}, - unseenIdleByWorkspace: {}, - lastSettledAtBySession: {}, - sessionOwnership: {}, - syncRevisionByWorkspace: {}, - tombstonedSessions: {}, - }, - }, - }; -} - -function renderPageClient( - initialState: Partial, - workspaceId: string, - taskId?: string, - sessionId?: string, -) { - return ( - <> - - - - ); -} - -function renderUnresolvedPage(initialState: Partial) { - return ( - <> - - - - ); -} - -export default async function Page({ searchParams }: PageProps) { - try { - const resolvedParams = searchParams ? await searchParams : {}; - const workspaceId = resolveParam(resolvedParams.workspaceId); - const workflowIdParam = resolveParam(resolvedParams.workflowId); - const taskId = resolveParam(resolvedParams.taskId); - const sessionId = resolveParam(resolvedParams.sessionId); - - const [workspaces, userSettingsResponse, cookieStore] = await Promise.all([ - listWorkspaces({ cache: "no-store" }), - fetchUserSettings({ cache: "no-store" }).catch(() => null), - readCookies().catch((error) => { - console.error("Failed to read cookies on Kanban page:", error); - return null; - }), - ]); - const settingsWorkspaceId = userSettingsResponse?.settings?.workspace_id || null; - const settingsWorkflowId = userSettingsResponse?.settings?.workflow_filter_id || null; - // The sidebar picker writes the selected workspace to this cookie so the - // choice survives a refresh even when userSettings is not updated on select. - // Kanban home only resolves against kanban workspaces; office workspaces - // belong under /office. - const cookieWorkspaceId = cookieStore?.get(ACTIVE_WORKSPACE_COOKIE)?.value ?? null; - // `readCookies()` is client-only in this code path; during SSR this is empty. - // Workspace selection still works because spa-routes.tsx re-hydrates from - // `readActiveWorkspaceCookie()` and the generic resolver on first client render. - const activeWorkspaceId = resolveActiveKanbanWorkspaceId( - workspaces.workspaces, - workspaceId, - cookieWorkspaceId, - settingsWorkspaceId, - ); - - const initialState = buildBaseState(workspaces, userSettingsResponse, activeWorkspaceId); - - if (!activeWorkspaceId) { - return renderUnresolvedPage(initialState); - } - - const workspaceState = await loadWorkspaceState({ - activeWorkspaceId, - initialState, - settingsWorkflowId, - workflowIdParam, - }); - - if (!workspaceState.workflowId) - return renderPageClient(workspaceState.initialState, activeWorkspaceId); - - const snapshotState = await loadSnapshotState(workspaceState.workflowId, taskId, sessionId); - return renderPageClient( - { ...workspaceState.initialState, ...snapshotState }, - activeWorkspaceId, - taskId, - sessionId, - ); - } catch { - return ; - } -} diff --git a/apps/web/app/settings/layout.tsx b/apps/web/app/settings/layout.tsx deleted file mode 100644 index 28aa3aeab7..0000000000 --- a/apps/web/app/settings/layout.tsx +++ /dev/null @@ -1,108 +0,0 @@ -import { SettingsLayoutClient } from "@/components/settings/settings-layout-client"; -import { StateHydrator } from "@/components/state-hydrator"; -import { - fetchUserSettings, - listAgentDiscovery, - listAgents, - listAvailableAgents, - listExecutors, - listWorkspaces, -} from "@/lib/api"; -import { - ACTIVE_WORKSPACE_COOKIE, - mapWorkspaceItem, - resolveSettingsActiveWorkspaceId, -} from "@/lib/routing/route-bootstrap"; -import { mapUserSettingsResponse } from "@/lib/ssr/user-settings"; -import { readCookies } from "@/lib/server/cookies"; -import { toAgentProfileOption } from "@/lib/state/slices/settings/types"; - -export default function SettingsLayout({ children }: { children: React.ReactNode }) { - return {children}; -} - -async function SettingsLayoutServer({ children }: { children: React.ReactNode }) { - let initialState = {}; - try { - // Fetch discovery + available agents alongside the DB-backed list so a - // hard refresh of /settings/agents/[name] (where no profile exists yet) - // can still render the agent from the discovered set. - const [workspaces, executors, agents, discovery, available, userSettingsResponse, cookieStore] = - await Promise.all([ - listWorkspaces({ cache: "no-store" }), - listExecutors({ cache: "no-store" }), - listAgents({ cache: "no-store" }), - listAgentDiscovery({ cache: "no-store" }), - listAvailableAgents({ cache: "no-store" }), - fetchUserSettings({ cache: "no-store" }).catch(() => null), - readCookies().catch(() => null), - ]); - // Hydrate userSettings into the ROOT store so app-global, override-driven - // shortcuts (TOGGLE_SIDEBAR, Quick Chat) work on settings routes too. The - // settings/general page mounts its own nested store for editing; that store - // is invisible to the root-mounted GlobalCommands/useAppShortcuts. - const mappedUserSettings = mapUserSettingsResponse(userSettingsResponse); - const workspaceItems = workspaces.workspaces.map(mapWorkspaceItem); - const activeWorkspaceId = resolveSettingsActiveWorkspaceId( - workspaceItems, - // `readCookies()` is client-only in this path; during SSR this is empty. - // Settings active workspace selection is completed on first client render by - // spa-routes via `readActiveWorkspaceCookie()`. - cookieStore?.get(ACTIVE_WORKSPACE_COOKIE)?.value ?? null, - userSettingsResponse?.settings?.workspace_id ?? null, - ); - initialState = { - workspaces: { - items: workspaceItems, - activeId: activeWorkspaceId, - }, - executors: { - items: executors.executors, - }, - agentProfiles: { - items: agents.agents.flatMap((agent) => - agent.profiles.map((profile) => toAgentProfileOption(agent, profile)), - ), - }, - settingsAgents: { - items: agents.agents, - }, - agentDiscovery: { - items: discovery.agents, - loading: false, - loaded: true, - }, - availableAgents: { - items: available.agents, - tools: available.tools ?? [], - loading: false, - loaded: true, - }, - settingsData: { - executorsLoaded: true, - agentsLoaded: true, - }, - ...(mappedUserSettings.loaded - ? { - userSettings: { - ...mappedUserSettings, - workspaceId: activeWorkspaceId, - }, - } - : {}), - }; - } catch { - // If any non-settings fetch (workspaces, executors, agents, …) throws, we - // render with empty initial state — losing the userSettings overrides too. - // That's acceptable: the page is already degraded (no executors/agents), so - // override-driven shortcuts being inactive until client hydration is fine. - initialState = {}; - } - - return ( - <> - - {children} - - ); -} diff --git a/apps/web/components/app-sidebar/app-sidebar-workspace-navigation.test.ts b/apps/web/components/app-sidebar/app-sidebar-workspace-navigation.test.ts index 9ced56a750..2854953a27 100644 --- a/apps/web/components/app-sidebar/app-sidebar-workspace-navigation.test.ts +++ b/apps/web/components/app-sidebar/app-sidebar-workspace-navigation.test.ts @@ -1,5 +1,12 @@ -import { beforeEach, describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE } from "@/lib/routing/route-bootstrap"; + +// The write path scopes names with the API-origin port; pin it so the +// assertions are deterministic. +vi.mock("@/lib/config", () => ({ + getBackendConfig: () => ({ apiBaseUrl: "http://localhost:8443" }), +})); + import { rememberWorkspaceSelection, rememberWorkspaceSelectionById, @@ -14,10 +21,26 @@ const officeWithReservedChars = { office_workflow_id: "wf-office-reserved", }; +// Reads a cookie's value, treating an empty entry (lingering `name=` after an +// expired write) as absent — the same semantics as the production reader +// (readCookie: "an empty cookie value is treated as absent"). +function cookieValue(name: string): string | null { + const prefix = `${name}=`; + const entry = document.cookie + .split(";") + .map((part) => part.trim()) + .find((part) => part.startsWith(prefix)); + if (!entry) return null; + const value = decodeURIComponent(entry.slice(prefix.length)); + return value || null; +} + describe("app sidebar workspace navigation", () => { beforeEach(() => { document.cookie = "kandev-active-workspace=; path=/; max-age=0"; document.cookie = "office-active-workspace=; path=/; max-age=0"; + document.cookie = "kandev-active-workspace_8443=; path=/; max-age=0"; + document.cookie = "office-active-workspace_8443=; path=/; max-age=0"; }); it("routes workspace home by active workspace type", () => { @@ -26,12 +49,18 @@ describe("app sidebar workspace navigation", () => { expect(workspaceHomeHref(undefined)).toBe("/?home=overview"); }); - it("records the active workspace in one write", () => { + it("records the active workspace in one write under the port-scoped names", () => { rememberWorkspaceSelection(kanban); rememberWorkspaceSelection(office); - expect(document.cookie).toContain(`${ACTIVE_WORKSPACE_COOKIE}=office-1`); - expect(document.cookie).toContain(`${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}=office-1`); + expect(document.cookie).toContain(`${ACTIVE_WORKSPACE_COOKIE}_8443=office-1`); + expect(document.cookie).toContain(`${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}_8443=office-1`); + // The legacy unprefixed names are read-only fallback — never written. + // Assert on VALUES, not presence: some environments keep an empty + // `name=` entry after a max-age=0 cleanup, and an empty value is + // equivalent to absent (readCookie treats it as such). + expect(cookieValue(ACTIVE_WORKSPACE_COOKIE)).toBeNull(); + expect(cookieValue(LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE)).toBeNull(); }); it("does not write the legacy office cookie for a kanban selection", () => { @@ -41,18 +70,18 @@ describe("app sidebar workspace navigation", () => { // The office boot paths read the legacy cookie to pick an office workspace // when the unified cookie names a kanban board, so a kanban selection must // leave it pointing at the office workspace last used. - expect(document.cookie).toContain(`${ACTIVE_WORKSPACE_COOKIE}=kanban-1`); - expect(document.cookie).toContain(`${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}=office-1`); + expect(document.cookie).toContain(`${ACTIVE_WORKSPACE_COOKIE}_8443=kanban-1`); + expect(document.cookie).toContain(`${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}_8443=office-1`); }); - it("writes active and legacy office workspace cookies with an encoded id", () => { + it("writes scoped active and office workspace cookies with an encoded id", () => { rememberWorkspaceSelection(officeWithReservedChars); expect(document.cookie).toContain( - `${ACTIVE_WORKSPACE_COOKIE}=${encodeURIComponent(officeWithReservedChars.id)}`, + `${ACTIVE_WORKSPACE_COOKIE}_8443=${encodeURIComponent(officeWithReservedChars.id)}`, ); expect(document.cookie).toContain( - `${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}=${encodeURIComponent(officeWithReservedChars.id)}`, + `${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}_8443=${encodeURIComponent(officeWithReservedChars.id)}`, ); }); @@ -61,7 +90,26 @@ describe("app sidebar workspace navigation", () => { // else, so there is no record to pass. rememberWorkspaceSelectionById("office-new", "office"); - expect(document.cookie).toContain(`${ACTIVE_WORKSPACE_COOKIE}=office-new`); - expect(document.cookie).toContain(`${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}=office-new`); + expect(document.cookie).toContain(`${ACTIVE_WORKSPACE_COOKIE}_8443=office-new`); + expect(document.cookie).toContain(`${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}_8443=office-new`); + expect(cookieValue(ACTIVE_WORKSPACE_COOKIE)).toBeNull(); + expect(cookieValue(LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE)).toBeNull(); + }); + + it("leaves a pre-existing legacy cookie untouched when the scoped twin is written", () => { + // Pre-upgrade jar (or a default-port instance sharing the host): the + // unprefixed names hold a value that must survive — it is either the + // migration fallback for other instances or another instance's live + // selection. The write path must not scrub it (spec: no proactive legacy + // cookie deletion). + document.cookie = `${ACTIVE_WORKSPACE_COOKIE}=pre-upgrade; path=/`; + document.cookie = `${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}=pre-upgrade; path=/`; + + rememberWorkspaceSelection(office); + + expect(document.cookie).toContain(`${ACTIVE_WORKSPACE_COOKIE}_8443=office-1`); + expect(document.cookie).toContain(`${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}_8443=office-1`); + expect(document.cookie).toContain(`${ACTIVE_WORKSPACE_COOKIE}=pre-upgrade`); + expect(document.cookie).toContain(`${LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE}=pre-upgrade`); }); }); diff --git a/apps/web/components/app-sidebar/app-sidebar-workspace-navigation.ts b/apps/web/components/app-sidebar/app-sidebar-workspace-navigation.ts index 2314bccede..c4035dabe6 100644 --- a/apps/web/components/app-sidebar/app-sidebar-workspace-navigation.ts +++ b/apps/web/components/app-sidebar/app-sidebar-workspace-navigation.ts @@ -1,6 +1,7 @@ import { ACTIVE_WORKSPACE_COOKIE, LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE, + scopedCookieName, } from "@/lib/routing/route-bootstrap"; import { isOfficeWorkspace, type ModeWorkspace } from "@/lib/state/slices/workspace/selectors"; @@ -27,15 +28,26 @@ export function rememberWorkspaceSelection(workspace: ModeWorkspace | undefined) export function rememberWorkspaceSelectionById(id: string, kind: "office" | "kanban"): void { if (!id || typeof document === "undefined") return; writeWorkspaceCookie(ACTIVE_WORKSPACE_COOKIE, id); - // The office boot paths (`app/office/layout.tsx` and the office route - // bootstrap) still read the legacy cookie to pick an office workspace when - // the unified one names a kanban board, so it is kept in step here. + // The office boot path (`src/office-routes.tsx`) reads the office-family + // cookie (scoped name first, legacy unprefixed name as read-only fallback) + // to pick an office workspace when the unified one names a kanban board, so + // it is kept in step here. Only the port-scoped names are ever written. if (kind === "office") { writeWorkspaceCookie(LEGACY_OFFICE_ACTIVE_WORKSPACE_COOKIE, id); } } +/** + * Writes one workspace-selection cookie under its port-scoped name (API-origin + * port; the plain name on a no-port instance). The legacy unprefixed name is + * deliberately left untouched: on a host serving a default-port instance it is + * that instance's LIVE selection cookie, and for upgraded ported instances it + * is the validated migration fallback until each writes its own scoped value. + * Scrubbing it from one instance would change the other instances' next boot + * (see docs/specs/fix-multi-instance-cookie-isolation/spec.md: the upgraded + * instance does not proactively delete the legacy cookie). + */ function writeWorkspaceCookie(name: string, value: string): void { if (typeof document === "undefined") return; - document.cookie = `${name}=${encodeURIComponent(value)}; path=/; max-age=${ACTIVE_WORKSPACE_COOKIE_MAX_AGE}; samesite=strict`; + document.cookie = `${scopedCookieName(name)}=${encodeURIComponent(value)}; path=/; max-age=${ACTIVE_WORKSPACE_COOKIE_MAX_AGE}; samesite=strict`; } diff --git a/apps/web/components/app-sidebar/app-sidebar-workspace-picker.test.tsx b/apps/web/components/app-sidebar/app-sidebar-workspace-picker.test.tsx index 299b8a7b2d..eeb77d2d73 100644 --- a/apps/web/components/app-sidebar/app-sidebar-workspace-picker.test.tsx +++ b/apps/web/components/app-sidebar/app-sidebar-workspace-picker.test.tsx @@ -1,6 +1,12 @@ import { describe, it, expect, beforeEach, vi, afterEach } from "vitest"; import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +// The sidebar write path scopes names with the API-origin port; pin it so the +// captured write assertions are deterministic. +vi.mock("@/lib/config", () => ({ + getBackendConfig: () => ({ apiBaseUrl: "http://localhost:8443" }), +})); + const navigationMock = vi.hoisted(() => ({ push: vi.fn() })); vi.mock("@/lib/routing/client-router", () => ({ @@ -159,7 +165,7 @@ describe("AppSidebarWorkspacePicker — workspace select", () => { fireEvent.click(screen.getByTestId(OFFICE_WORKSPACE_ITEM)); - expect(cookieWrites.some((c) => c.startsWith("office-active-workspace=w2"))).toBe(true); + expect(cookieWrites.some((c) => c.startsWith("office-active-workspace_8443=w2"))).toBe(true); expect(storeState.setActiveWorkspace).toHaveBeenCalledWith("w2"); expect(navigationMock.push).not.toHaveBeenCalled(); }); @@ -191,13 +197,13 @@ describe("AppSidebarWorkspacePicker — workspace select", () => { it("does not overwrite the office workspace cookie when selecting a kanban workspace", () => { storeState.features.office = true; storeState.workspaces.activeId = "w2"; - cookieWrites = ["office-active-workspace=w2; path=/"]; + cookieWrites = ["office-active-workspace_8443=w2; path=/"]; render(); fireEvent.click(screen.getByTestId(KANBAN_WORKSPACE_ITEM)); - expect(cookieWrites.some((c) => c.startsWith("office-active-workspace=w1"))).toBe(false); - expect(cookieWrites.some((c) => c.startsWith("office-active-workspace=w2"))).toBe(true); + expect(cookieWrites.some((c) => c.startsWith("office-active-workspace_8443=w1"))).toBe(false); + expect(cookieWrites.some((c) => c.startsWith("office-active-workspace_8443=w2"))).toBe(true); expect(storeState.setActiveWorkspace).toHaveBeenCalledWith("w1"); expect(navigationMock.push).toHaveBeenCalledWith("/?home=overview&workspaceId=w1"); }); @@ -217,13 +223,13 @@ describe("AppSidebarWorkspacePicker — workspace select", () => { storeState.workspaces.activeId = "w2"; // Written when w2 was selected. A workspace is recorded on the way in, not // on the way out, so leaving it must simply not disturb this. - cookieWrites = ["office-active-workspace=w2; path=/"]; + cookieWrites = ["office-active-workspace_8443=w2; path=/"]; render(); fireEvent.click(screen.getByTestId(KANBAN_WORKSPACE_ITEM)); - expect(cookieWrites.some((c) => c.startsWith("office-active-workspace=w1"))).toBe(false); - expect(cookieWrites.some((c) => c.startsWith("office-active-workspace=w2"))).toBe(true); + expect(cookieWrites.some((c) => c.startsWith("office-active-workspace_8443=w1"))).toBe(false); + expect(cookieWrites.some((c) => c.startsWith("office-active-workspace_8443=w2"))).toBe(true); expect(storeState.setActiveWorkspace).toHaveBeenCalledWith("w1"); expect(navigationMock.push).toHaveBeenCalledWith("/?home=overview&workspaceId=w1"); }); @@ -277,7 +283,7 @@ describe("AppSidebarWorkspacePicker — active workspace display and routing", ( fireEvent.click(screen.getByTestId(KANBAN_WORKSPACE_ITEM)); - expect(cookieWrites.some((c) => c.startsWith("kandev-active-workspace=w1"))).toBe(true); + expect(cookieWrites.some((c) => c.startsWith("kandev-active-workspace_8443=w1"))).toBe(true); // Re-selecting the workspace that is already active goes through the same // one selection path as any other pick; the store action ignores a // no-op change, so there is no reason for this branch to skip it. diff --git a/apps/web/e2e/helpers/auth.ts b/apps/web/e2e/helpers/auth.ts index 9d1e4f710a..4cbbd401d7 100644 --- a/apps/web/e2e/helpers/auth.ts +++ b/apps/web/e2e/helpers/auth.ts @@ -3,9 +3,10 @@ import { expect, type BrowserContext } from "@playwright/test"; /** * Helpers for the opt-in authentication E2E suite. * - * All calls go through a BrowserContext's request API so the kandev_session - * cookie set by login/setup/invite-accept is shared with pages opened from the - * same context (matching how the real SPA authenticates). + * All calls go through a BrowserContext's request API so the session cookie + * (port-scoped name, e.g. kandev_session_) set by login/setup/invite + * accept is shared with pages opened from the same context (matching how the + * real SPA authenticates). */ export type Credentials = { diff --git a/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts b/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts index 18bcb7724d..792328c671 100644 --- a/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts +++ b/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts @@ -118,6 +118,29 @@ test.describe.serial("opt-in authentication", () => { await context.close(); }); + test("session cookie is port-scoped to the instance", async ({ browser, backend }) => { + // The session cookie is HttpOnly, so the name is asserted from the login + // response's Set-Cookie headers, not document.cookie. Two instances on one + // host (different ports) must not share a cookie name. + const context = await browser.newContext({ baseURL: backend.frontendUrl }); + + const res = await context.request.post(`${backend.baseUrl}/api/v1/auth/login`, { + data: { email: ADMIN.email, password: ADMIN.password }, + }); + expect(res.ok()).toBeTruthy(); + const sessionNames = res + .headersArray() + .filter((h) => h.name.toLowerCase() === "set-cookie") + .map((h) => h.value.split(";")[0].split("=")[0]) + .filter((name) => name.startsWith("kandev_session")); + expect(sessionNames).toEqual([`kandev_session_${backend.port}`]); + + // The scoped session authenticates: the app renders authenticated content. + expect((await context.request.get(`${backend.baseUrl}/api/v1/workspaces`)).ok()).toBeTruthy(); + + await context.close(); + }); + test("two users are fully segregated over HTTP, boot payload, and WS", async ({ browser, backend, diff --git a/apps/web/e2e/tests/office/execution-indicator.spec.ts b/apps/web/e2e/tests/office/execution-indicator.spec.ts index 611688f9de..25667301af 100644 --- a/apps/web/e2e/tests/office/execution-indicator.spec.ts +++ b/apps/web/e2e/tests/office/execution-indicator.spec.ts @@ -8,7 +8,7 @@ test.describe("Execution indicator", () => { await testPage.goto("/office/tasks"); // Post-overhaul: the unified AppSidebar's Tasks section also lists tasks, so // the title text appears in BOTH the global rail and the page table. Scope - // to `
` (the office layout's page content, which excludes the + // to `
` (the office shell's page content, which excludes the // `