From abeee0076b5b77b4c34f0b56ff4eb48aa6eba65f Mon Sep 17 00:00:00 2001 From: John Coffey Date: Fri, 14 Aug 2026 23:02:09 -0700 Subject: [PATCH] Build and browser-verify the tenant-picker frontend page web/src/routes/select-tenant now calls enterprise-auth's existing GET /auth/memberships / POST /auth/select-tenant protocol (built earlier this phase, previously called only from Go tests) via fetch(..., {credentials: 'include'}) -- new listMemberships/selectTenant functions in $lib/api.ts, using a dedicated request helper that reads plain-text error bodies (loginhandler's http.Error responses), unlike every other request helper in that file which expects JSON. Credentialed cross-origin fetch needed CORS enterprise-auth didn't have: api/httpserver.WithCORS's wildcard-friendly default can't be combined with a credentialed request at all (browsers refuse to honor Access-Control-Allow-Origin: "*" on one) -- added WithCredentialedCORS (literal origin, Access-Control-Allow-Credentials: true) alongside it, wired into enterprise-auth via a new CORS_ALLOWED_ORIGIN config var defaulting to POST_LOGIN_REDIRECT_URL (web's own origin, the same default pattern SELECT_TENANT_REDIRECT_URL already used). adapter-static's route crawler doesn't discover a page nothing links to (this one is only ever reached via enterprise-auth's redirect) -- fixed with select-tenant/+page.ts's `export const prerender = true`, the same declaration every other route already has. Genuinely verified in a real browser in this environment, not just type-checked: a throwaway Node server standing in for enterprise-auth's exact wire contract (including its plain-text error bodies), driven through the full flow via mcp__claude-in-chrome -- cross-origin pending-login cookie set, credentialed preflight + GET/POST round trip, a real click choosing a tenant, the post-selection redirect, and the missing/expired-cookie error path rendering the backend's actual message. No Docker or live Postgres/IdP needed, since the point was exercising web's own fetch/CORS/cookie wiring, not enterprise-auth's internals (already covered by loginhandler's own tests). This closes the tenant-picker as the last named gap in Phase 4. What's left is the already-disclosed live-verification caveat shared by every Postgres/ClickHouse-backed piece and both SSO protocols: none of this has run against a real database, external IdP, or multi-container deployment in this environment. --- CLAUDE.md | 37 +++-- api/httpserver/cors.go | 26 ++++ api/httpserver/cors_test.go | 41 ++++++ api/queryapi/tenant_isolation_gap_test.go | 9 +- docs/architecture.md | 15 +- docs/phase-4-runbook.md | 103 ++++++++++--- docs/security/threat-model.md | 31 ++-- enterprise/README.md | 60 ++++---- enterprise/cmd/enterprise-auth/main.go | 10 +- enterprise/internal/config/config.go | 25 +++- .../internal/loginhandler/loginhandler.go | 8 +- web/README.md | 40 ++++++ web/src/lib/api.ts | 54 +++++++ web/src/routes/select-tenant/+page.svelte | 135 ++++++++++++++++++ web/src/routes/select-tenant/+page.ts | 4 + 15 files changed, 513 insertions(+), 85 deletions(-) create mode 100644 web/src/routes/select-tenant/+page.svelte create mode 100644 web/src/routes/select-tenant/+page.ts diff --git a/CLAUDE.md b/CLAUDE.md index f7901d2..82c4240 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -222,13 +222,16 @@ its real result into the CRD — a real credential Secret, not the previous placeholder that authenticated against nothing, and status fields the reconciler derives `Phase`/`Ready` from instead of independently guessing "Active" the moment a Tenant object exists. The -tenant-picker's backend protocol is built too: an identity with more -than one `tenant_memberships` row now gets a real `GET +tenant-picker is now fully built, backend and frontend: an identity with +more than one `tenant_memberships` row gets a real `GET /auth/memberships`/`POST /auth/select-tenant` round trip (a short-lived pending-login token, distinct from a real session by both Go type and JWT claim name — a real token-confusion bug this design's own tests caught before it shipped) instead of the flat refusal Phase 4 shipped -with earlier. Ingest tenant-awareness — the gap this section used to +with earlier, and `web/src/routes/select-tenant` is the page that +actually calls it — see the Phase 4 exit-criteria paragraph below for +what changed to make that verifiable in this environment. Ingest +tenant-awareness — the gap this section used to call "undesigned" — now has a real, if intentionally partial, design: `ingest` (AGPL core) gained an optional `TenantResolver` (`ingest/internal/grpcserver`), a per-tenant bearer credential an agent @@ -271,11 +274,29 @@ cause an index directory to be created for a tenant that's no longer active. Narrow blast radius (an orphan, isolated, empty index — not cross-tenant leakage — and only reachable with a real signed credential), but real; see `search/src/registry.rs`'s doc comment on -`resolve`. What still keeps this phase from being done: only the -tenant-picker *page* now — `web` has no session/cookie-handling code at -all yet, and `enterprise-auth` has no CORS middleware for a cross-origin -`fetch` with credentials, both real, separately-scoped frontend gaps. -Full accounting: +`resolve`. **The tenant-picker page is now built too**: +`web/src/routes/select-tenant` calls `GET /auth/memberships`/ +`POST /auth/select-tenant` via `fetch(..., {credentials: 'include'})` +(new `$lib/api.ts` functions), which needed a second CORS posture +alongside the wildcard-friendly one `enterprise-api` already had — +`api/httpserver.WithCredentialedCORS`, set to a literal origin via a new +`CORS_ALLOWED_ORIGIN` on `enterprise-auth` — since browsers refuse to +honor a wildcard `Access-Control-Allow-Origin` on a credentialed +request. **Genuinely verified in a real browser in this environment**: +a throwaway Node server standing in for `enterprise-auth`'s exact wire +contract (including its plain-text `http.Error` bodies, not JSON) on a +different origin than `web`'s dev server, driven through the full +cross-origin pending-login-cookie round trip, a real click choosing a +tenant, and the post-selection redirect — plus the missing/expired- +pending-login error path — with no Docker or live Postgres/IdP needed, +since the point was exercising `web`'s own fetch/CORS/cookie wiring, not +`enterprise-auth`'s internals (already covered by that package's own +tests). See `/web/README.md`'s "Tenant picker" section for the exact +setup. What's left in this phase now is entirely the caveats already +disclosed above, not an unbuilt feature: the ClickHouse/Postgres-backed +pieces have never run against a real database in this environment, and +nothing here has been tried against a real external IdP or a real +running multi-container deployment. Full accounting: `/docs/security/threat-model.md`; step-by-step verification procedure (not yet run against a live cluster in this environment): `/docs/phase-4-runbook.md`. The rest of this section describes the exit diff --git a/api/httpserver/cors.go b/api/httpserver/cors.go index 9d409bf..e429c4f 100644 --- a/api/httpserver/cors.go +++ b/api/httpserver/cors.go @@ -22,3 +22,29 @@ func WithCORS(next http.Handler, allowedOrigin string) http.Handler { next.ServeHTTP(w, r) }) } + +// WithCredentialedCORS is WithCORS's sibling for endpoints a browser must +// call with cookies attached (Phase 4: enterprise-auth's +// GET /auth/memberships / POST /auth/select-tenant, called from web's +// tenant-picker page via `fetch(..., {credentials: 'include'})`). +// Browsers categorically refuse to combine a credentialed request with +// Access-Control-Allow-Origin: "*" -- allowedOrigin must be a real, +// literal origin (e.g. "http://localhost:3000"), not the wildcard +// WithCORS's own zero-config default relies on. Callers of this function +// don't get that convenient zero-config default: an empty/wildcard +// allowedOrigin here is a configuration bug, not a permissive default, +// so it's deliberately not special-cased into something that "just +// works" the way plain WithCORS's "*" does. +func WithCredentialedCORS(next http.Handler, allowedOrigin string) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Access-Control-Allow-Origin", allowedOrigin) + w.Header().Set("Access-Control-Allow-Credentials", "true") + w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS") + w.Header().Set("Access-Control-Allow-Headers", "Content-Type") + if r.Method == http.MethodOptions { + w.WriteHeader(http.StatusNoContent) + return + } + next.ServeHTTP(w, r) + }) +} diff --git a/api/httpserver/cors_test.go b/api/httpserver/cors_test.go index 509eb51..6f77fa7 100644 --- a/api/httpserver/cors_test.go +++ b/api/httpserver/cors_test.go @@ -40,3 +40,44 @@ func TestWithCORSPassesThroughNonPreflight(t *testing.T) { t.Fatalf("status = %d, want 200", rec.Code) } } + +func TestWithCredentialedCORSSetsLiteralOriginAndCredentialsHeader(t *testing.T) { + inner := http.NewServeMux() + inner.HandleFunc("GET /auth/memberships", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + h := WithCredentialedCORS(inner, "http://localhost:3000") + + req := httptest.NewRequest(http.MethodGet, "/auth/memberships", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "http://localhost:3000" { + t.Fatalf("Access-Control-Allow-Origin = %q, want http://localhost:3000", got) + } + if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { + t.Fatalf("Access-Control-Allow-Credentials = %q, want true -- a credentialed fetch() needs this header present or the browser discards the response", got) + } +} + +func TestWithCredentialedCORSPreflight(t *testing.T) { + inner := http.NewServeMux() + inner.HandleFunc("POST /auth/select-tenant", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + h := WithCredentialedCORS(inner, "http://localhost:3000") + + req := httptest.NewRequest(http.MethodOptions, "/auth/select-tenant", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusNoContent { + t.Fatalf("status = %d, want 204", rec.Code) + } + if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { + t.Fatalf("Access-Control-Allow-Credentials = %q, want true on the preflight response too", got) + } +} diff --git a/api/queryapi/tenant_isolation_gap_test.go b/api/queryapi/tenant_isolation_gap_test.go index 249f18a..880ffe8 100644 --- a/api/queryapi/tenant_isolation_gap_test.go +++ b/api/queryapi/tenant_isolation_gap_test.go @@ -58,8 +58,9 @@ // (live-Postgres, skip-gated). // // Scope boundary all four items share: they prove *read* isolation -// given tenant-scoped data exists -- they do not prove ingest/write-path -// tenancy, which doesn't exist yet (every record ingest produces lands -// in the single shared ClickHouse database and Tantivy index regardless -// of tenant) -- see /docs/security/threat-model.md. +// given tenant-scoped data exists -- they say nothing about ingest's +// write path, which is now a separately-built and separately-verified +// concern (enterprise/cmd/enterprise-ingest + enterprise/internal/ +// chwriter for ClickHouse, search/src/consumer.rs for Tantivy) -- see +// /docs/security/threat-model.md and /docs/phase-4-runbook.md §14. package queryapi diff --git a/docs/architecture.md b/docs/architecture.md index 806dd18..ade94f4 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -202,11 +202,16 @@ mutually-exclusive choice via `COMPOSE_PROFILES` (`.env` defaults to plain `api`), sharing a host-port/network-alias trick so `alerting`/ `web` need no conditional config either way. With both storage engines' connection/index-layer mechanisms built, deployment topology enforced at -both the Helm and docker-compose layers, and the two provisioning -mechanisms unified, and both storage engines' write paths now -per-tenant-routed too (see above), the largest remaining gap in this -phase is the tenant-picker *frontend* page — the backend protocol is -built, but `web` has no session/cookie-handling code yet to call it. +both the Helm and docker-compose layers, the two provisioning +mechanisms unified, both storage engines' write paths per-tenant-routed, +and the tenant-picker frontend page now built and browser-verified +(`web/src/routes/select-tenant`, `api/httpserver.WithCredentialedCORS` +— see `/CLAUDE.md`'s Phase 4 section and `/web/README.md`'s "Tenant +picker" section), the remaining gaps in this phase are entirely the +already-disclosed live-verification caveats: the ClickHouse/Postgres- +backed pieces have never run against a real database in this +environment, and nothing here has been tried against a real external +IdP or a real running multi-container deployment. ## Licensing boundary diff --git a/docs/phase-4-runbook.md b/docs/phase-4-runbook.md index 06e15c4..8d0de11 100644 --- a/docs/phase-4-runbook.md +++ b/docs/phase-4-runbook.md @@ -47,6 +47,14 @@ of what was already run and passed. Two genuine exceptions: unverified is not Tantivy itself but the upstream credential/header plumbing feeding it (ingest's `TenantResolver`, `enterprise-auth`'s `/internal/authorize-ingest`) against a real running stack. +- The tenant-picker frontend page (§12) -- the first frontend-only piece + in this phase exercised in a real browser rather than only + type-checked: `web/src/routes/select-tenant`'s cross-origin + credentialed fetch/CORS/cookie handling, driven end-to-end via + `mcp__claude-in-chrome` against a throwaway server standing in for + `enterprise-auth`'s exact wire contract. What's unverified is the same + shape as OIDC/SAML above: this round trip against a real running + `enterprise-auth` container, not a stand-in. Everything else — `internal/rbacstore`'s CRUD, the auth-enforcement walkthrough, the dashboards tenant-scoping fix, the Helm chart, the @@ -530,12 +538,12 @@ the full loop (does the operator's watch actually re-trigger a reconcile after `-provision-tenant`'s external status write the way controller- runtime's default predicate is expected to). -## 12. Tenant-picker backend protocol (no frontend yet, no Docker needed) +## 12. Tenant-picker, backend and frontend -Like §9, this needs nothing but a local Go toolchain -- the real -fake-IdP tests already exercise the full login → pending-login cookie → -`GET /auth/memberships` → `POST /auth/select-tenant` → real session -round trip: +**Backend** -- like §9, this needs nothing but a local Go toolchain -- +the real fake-IdP tests already exercise the full login → pending-login +cookie → `GET /auth/memberships` → `POST /auth/select-tenant` → real +session round trip: ```sh cd enterprise @@ -555,13 +563,70 @@ go test ./internal/loginhandler/... -run 'Memberships|SelectTenant|MultipleMembe # tenant_id outside the identity's actual memberships. ``` -**Not built, and explicitly not attempted here**: the frontend page. -`web` has no session/cookie-handling code anywhere in it today (checked -while designing this), and `enterprise-auth` has no CORS middleware at -all -- a cross-origin `fetch` with credentials from `web`'s origin to -`enterprise-auth`'s would need it, and doesn't work today. Building the -actual picker UI is real, separately-scoped frontend work; this section -only closes the backend half. +**Frontend** -- `web/src/routes/select-tenant` (new), calling the +endpoints above via `fetch(..., {credentials: 'include'})` +(`$lib/api.ts`'s `listMemberships`/`selectTenant`), needed a CORS +posture `enterprise-auth` didn't have: `api/httpserver.WithCORS`'s +wildcard-friendly default can't be combined with a credentialed +request at all (browsers refuse it outright), so this needed a new +`WithCredentialedCORS` (same package, literal-origin-only) wired in via +a new `CORS_ALLOWED_ORIGIN` env var, defaulting to +`POST_LOGIN_REDIRECT_URL` (`web`'s own origin). Type-checked and built +for real: + +```sh +cd web +npm run check # svelte-check -- 0 errors +npm run build # adapter-static -- confirms select-tenant.html is + # actually produced (it wasn't, at first: adapter- + # static only crawls routes reachable from a link or an + # explicit prerender entry, and nothing in the app + # links to this route since it's only ever reached via + # enterprise-auth's redirect -- fixed by adding + # select-tenant/+page.ts's `export const prerender = + # true`, the same declaration every other route here + # already has) +``` + +**Genuinely verified in a real browser in this environment** -- not just +type-checked, the actual cross-origin fetch/CORS/cookie behavior, driven +through `mcp__claude-in-chrome`: + +1. A throwaway Node HTTP server (no dependencies) stood in for + `enterprise-auth`, implementing the exact wire contract this section's + Go tests already prove server-side: `GET /auth/memberships` and + `POST /auth/select-tenant`, the `sentry_pending_login` cookie + (`Path=/auth`), the credentialed CORS headers, and critically the + *plain-text* `http.Error` response bodies the real handler sends on + failure (not JSON -- `enterpriseAuthRequest` in `$lib/api.ts` reads + `res.text()` specifically because of this, unlike every other request + helper in that file). +2. `npm run dev` (SvelteKit dev server) pointed at that fake server via + `VITE_ENTERPRISE_AUTH_BASE_URL`, both on `localhost` but different + ports -- different origins, the same cross-origin shape a real + deployment has. +3. The browser navigated to the fake server's `/debug/start-pending- + login` (mimics `loginhandler.startTenantSelection`: sets the pending + cookie, redirects to `/select-tenant`) -- confirmed the redirect + landed on the real page, which then genuinely fetched + `GET /auth/memberships` cross-origin *with the cookie attached* and + rendered both fake tenants with their display names and roles. +4. Clicked a tenant in the real UI -- confirmed the real + `POST /auth/select-tenant` fired (with preflight), succeeded, and the + page navigated to the response's `redirect_url` via a real full page + load. +5. Reloaded `/select-tenant` directly (no pending cookie present anymore + -- the fake server clears it exactly like the real handler does) -- + confirmed the page's error state renders the backend's actual + plain-text message ("missing or expired pending login...") rather + than a generic fetch-failure string. + +No console errors at any point. This is the first frontend-only piece +in this entire phase that's been exercised in a real browser rather than +only type-checked or unit-tested against fakes -- everything else +frontend-adjacent (`getAuthFeatures` on the settings page, existing +Phase 0-3 routes) predates this runbook and was never re-verified here +either. ## 13. Ingest tenant identity @@ -760,13 +825,15 @@ Full accounting: `/docs/security/threat-model.md`. Headline items: - **Human SSO login now works for both OIDC (§3a) and SAML (§3b)** -- each verified with a real fake IdP (genuine cryptographic signing and verification), not yet a real external IdP or a running - `enterprise-auth` container. **The tenant-picker backend protocol is - now built too** (§12) -- `GET /auth/memberships`/ + `enterprise-auth` container. **The tenant-picker, backend and + frontend, is now fully built too** (§12) -- `GET /auth/memberships`/ `POST /auth/select-tenant`, backed by a short-lived pending-login - token distinct from a real session -- but nothing in `web` calls it - yet, so a multi-membership identity still can't actually finish - logging in through a browser today, just through direct HTTP calls - (which is what §12's verification does). + token distinct from a real session, and `web/src/routes/select-tenant` + actually calls it via credentialed cross-origin `fetch`, genuinely + exercised in a real browser against a contract-accurate fake backend + (§12). What's still not tried is the same caveat as OIDC/SAML above: + this whole round trip end-to-end against a real running + `enterprise-auth` container instead of a stand-in. - No admin UI to create a `tenant_memberships` row, but §3a/§3b's manual SQL bootstrap is gone -- `enterprise-auth -create-tenant`/ `-grant-membership-*`/`-revoke-membership-*`/`-list-memberships-tenant` diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index c6fafd3..db5f164 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -245,20 +245,29 @@ short-lived `session.Manager` "pending login" token (a distinct Go/JWT type from a real session, with its own disjoint claim name so a real session token can't double as one — a real bug this design's own test suite caught before it shipped, see `session.PendingLoginClaims`'s doc -comment) and redirects to a not-yet-served URL instead, backed by two -new endpoints (`GET /auth/memberships`, `POST /auth/select-tenant`) that +comment) and redirects to `web/src/routes/select-tenant`, backed by two +endpoints (`GET /auth/memberships`, `POST /auth/select-tenant`) that list the identity's real tenant options and, on selection, re-derive the role for the chosen tenant server-side (never trusting a client-supplied -role) before issuing the real session. This is the *backend protocol* -for tenant selection, verified with the same real-fake-IdP tests as the -rest of `internal/loginhandler` — the frontend page that would call it -doesn't exist (`web` has no session/cookie-handling code at all today, -and `enterprise-auth` has no CORS middleware for a cross-origin `fetch` -with credentials to work), both real, separately-scoped gaps, not -silently approximated. `GET /auth/features` (`enterprise/internal/authhandler`) reports whether +role) before issuing the real session. Both the *backend protocol* and +the *frontend page* that calls it are now built. The backend is verified +with the same real-fake-IdP tests as the rest of `internal/loginhandler`. +The frontend needed a second CORS posture — `httpserver. +WithCredentialedCORS`, a literal origin plus +`Access-Control-Allow-Credentials: true`, since a credentialed `fetch` +and a wildcard `Access-Control-Allow-Origin` can never be combined, so +this couldn't reuse `enterprise-api`'s wildcard-friendly `WithCORS` — and +is genuinely verified in a real browser in this environment (not just +type-checked): the full cross-origin pending-login-cookie round trip, a +real click choosing a tenant, the post-selection redirect, and the +missing/expired-cookie error path, all driven against a throwaway server +standing in for `enterprise-auth`'s exact wire contract. `GET +/auth/features` (`enterprise/internal/authhandler`) reports whether OIDC/SAML are *configured*, for `/web`'s settings page to conditionally render — independent of whether a login button actually exists yet in -the UI (it doesn't; only the HTTP endpoints do). +the UI (it doesn't yet; a user still has to be sent to +`/auth/oidc/login`/`/auth/saml/login` by some means other than clicking +something in `web`, since no page links there). **Implemented for the one machine caller.** `/alerting`'s evaluator is the sole service-to-service caller (`POST /query`, to evaluate rule @@ -490,7 +499,7 @@ terms: | Deployment actually routing traffic to `enterprise-api` (docker-compose) | **Enforced** — `api`/`enterprise-api` are mutually exclusive via `COMPOSE_PROFILES`, same flag choice as Helm's `enterprise.enabled`; verified via `docker compose config`, not an actual `docker compose up` in this environment | | Human SSO login — OIDC | **Built, verified with a real fake IdP** (not yet tried against a real external IdP) | | Human SSO login — SAML | **Built, verified with a real fake IdP** (not yet tried against a real external IdP) | -| Multi-tenant-membership login (tenant picker) | **Backend protocol built and verified** (`GET /auth/memberships`, `POST /auth/select-tenant`, a pending-login token distinct from a real session) — no frontend page calls it yet | +| Multi-tenant-membership login (tenant picker) | **Backend and frontend built and verified** (`GET /auth/memberships`, `POST /auth/select-tenant`, a pending-login token distinct from a real session; `web/src/routes/select-tenant` calls it via credentialed cross-origin fetch, genuinely exercised in a real browser) — not yet tried against a real running `enterprise-auth` container | | Per-resource dashboard grants (`own/granted`) | **Built, unit-tested against a fake store; live-Postgres integration tests written, not run in this environment** (only when `enterprise-api` serves traffic — plain `api` falls back to own/Admin only) | | Query audit logging (routine queries) | **Enforced**, fail-open, and now wired to a real writer via `enterprise-api` (`audit.QueryAPILogger`) | | Audit log tamper detection (hash chain) | **Enforced**, verified live | diff --git a/enterprise/README.md b/enterprise/README.md index c911c17..d73ca9f 100644 --- a/enterprise/README.md +++ b/enterprise/README.md @@ -139,10 +139,10 @@ Postgres-backed pieces. ## Tenant selection (multi-membership identities) -The backend protocol for choosing a tenant is built and tested; the -frontend page that would actually call it is deliberately not (see -"Deliberately deferred" below). When `resolveIdentity` finds more than -one `tenant_memberships` row for a logged-in identity, `finishLogin` +Both the backend protocol for choosing a tenant and the frontend page +that calls it (`web/src/routes/select-tenant`) are now built. When +`resolveIdentity` finds more than one `tenant_memberships` row for a +logged-in identity, `finishLogin` issues a `session.Manager.IssuePendingLogin` token (a distinct Go/JWT type from a real session -- see that type's doc comment for a real bug this design caught in its own tests: a shared JSON key would have let a @@ -171,29 +171,34 @@ pending cookie, a `tenant_id` outside the identity's actual memberships, a real session token rejected when presented as a pending login). -**Deliberately deferred, not half-built** -- named explicitly rather than -silently left out: -- **The actual tenant-picker page** -- nothing in `web/` calls the - endpoints above yet. Building it is genuinely different, larger scope - than the backend protocol: `web` has zero session/cookie-handling - code today (confirmed by reading it end to end while designing this), - so a real picker page means adding that from scratch, plus CORS - wiring (`enterprise-auth` has no CORS middleware at all right now -- - a cross-origin `fetch` with credentials from `web`'s origin needs - it), neither of which is verifiable in this environment without a - live backend and a browser session to exercise. +The frontend side needed two things `web` didn't have: session/cookie- +aware requests (`$lib/api.ts`'s `listMemberships`/`selectTenant`, both +`fetch(..., {credentials: 'include'})`) and CORS that actually allows a +credentialed cross-origin request -- `httpserver.WithCredentialedCORS` +(new, in `api/httpserver`, next to the plain `WithCORS` every other +service in this repo uses), wired in by this binary's `main.go` and +configured via the new `CORSAllowedOrigin` field +(`CORS_ALLOWED_ORIGIN`, defaulting to `PostLoginRedirectURL` -- +`web`'s own origin is exactly what needs credentialed access here). +Browsers categorically refuse to honor `Access-Control-Allow-Origin: "*"` +on a credentialed request, which is why this couldn't reuse plain +`WithCORS`'s wildcard-friendly default the way `enterprise-api` does. +Genuinely verified in a real browser in this environment (see +`/web/README.md`'s "Tenant picker" section for exactly how, since no +live Postgres/IdP was needed to exercise `web`'s own fetch/CORS/cookie +wiring): the full cross-origin cookie round trip, a real click choosing +a tenant, and the post-selection redirect, plus the missing/expired +pending-login error path. -Ingest write-routing (both ClickHouse and Tantivy) is no longer on this -list -- see "Ingest write-routing (ClickHouse)" below and -`/search/README.md`'s "Per-tenant indices" section for Tantivy, which -needed no code in this module at all: `search`'s `IndexRegistry` already -lived in AGPL core, so its write side didn't need an `enterprise/` -counterpart the way ClickHouse's did. - -Deployment-topology routing (does traffic actually reach `enterprise-api` -instead of `api`) is no longer deferred -- both `deploy/helm/sentry` and -`docker-compose.yml` make it a single-flag choice now (`enterprise. -enabled` / `COMPOSE_PROFILES`), see CLAUDE.md. +Two other things once named as deferred here are built too: ingest +write-routing, both ClickHouse (see "Ingest write-routing (ClickHouse)" +below) and Tantivy (`/search/README.md`'s "Per-tenant indices" section +-- needed no code in this module at all, since `search`'s +`IndexRegistry` already lived in AGPL core); and deployment-topology +routing (does traffic actually reach `enterprise-api` instead of +`api`), now a single-flag choice in both `deploy/helm/sentry` and +`docker-compose.yml` (`enterprise.enabled` / `COMPOSE_PROFILES`), see +CLAUDE.md. ## Ingest tenant identity @@ -480,7 +485,8 @@ to, see `tenantprovision.ProvisionClickHouse`'s doc comment). | `SAML_IDP_METADATA_URL` | (empty — SAML disabled if unset; if set, fetched and parsed at startup via `samlsp.FetchMetadata`, same trust level as `OIDC_ISSUER_URL`'s discovery fetch) | | `ENTERPRISE_SESSION_SIGNING_KEY` | **required**, min 32 bytes | | `POST_LOGIN_REDIRECT_URL` | `http://localhost:3000` — where the browser lands after `internal/loginhandler` sets a session cookie | -| `SELECT_TENANT_REDIRECT_URL` | `{POST_LOGIN_REDIRECT_URL}/select-tenant` — where the browser lands for a multi-membership identity instead; nothing serves this route yet, see "Tenant selection" above | +| `SELECT_TENANT_REDIRECT_URL` | `{POST_LOGIN_REDIRECT_URL}/select-tenant` — where the browser lands for a multi-membership identity instead; `web/src/routes/select-tenant` serves it, see "Tenant selection" above | +| `CORS_ALLOWED_ORIGIN` | `{POST_LOGIN_REDIRECT_URL}` — must be a literal origin, not `*` (unlike `enterprise-api`'s var of the same name below): `GET /auth/memberships`/`POST /auth/select-tenant` are credentialed requests, and browsers refuse to honor a wildcard `Access-Control-Allow-Origin` on those | ## Environment variables (`enterprise-api`) diff --git a/enterprise/cmd/enterprise-auth/main.go b/enterprise/cmd/enterprise-auth/main.go index 6d728f3..65be606 100644 --- a/enterprise/cmd/enterprise-auth/main.go +++ b/enterprise/cmd/enterprise-auth/main.go @@ -37,6 +37,7 @@ import ( "github.com/crewjam/saml/samlsp" "github.com/jackc/pgx/v5/pgxpool" + "github.com/sentry/sentry/api/httpserver" "github.com/sentry/sentry/enterprise/internal/authhandler" "github.com/sentry/sentry/enterprise/internal/config" "github.com/sentry/sentry/enterprise/internal/loginhandler" @@ -202,7 +203,14 @@ func main() { authhandler.New(logger, sessionManager, features, rbac).RegisterRoutes(mux) loginhandler.New(logger, oidcProvider, samlProvider, sessionManager, rbac, cfg.PostLoginRedirectURL, cfg.SelectTenantRedirectURL).RegisterRoutes(mux) - srv := &http.Server{Addr: cfg.HTTPListenAddr, Handler: mux} + // Credentialed, not plain, CORS: GET /auth/memberships and POST + // /auth/select-tenant are called from web's tenant-picker page via + // `fetch(..., {credentials: 'include'})` so the pending-login/session + // cookies actually go along -- see httpserver.WithCredentialedCORS's + // doc comment for why that rules out the wildcard origin every other + // service's WithCORS defaults to. + handler := httpserver.WithCredentialedCORS(mux, cfg.CORSAllowedOrigin) + srv := &http.Server{Addr: cfg.HTTPListenAddr, Handler: handler} errCh := make(chan error, 1) go func() { diff --git a/enterprise/internal/config/config.go b/enterprise/internal/config/config.go index 4d0c3d2..0123923 100644 --- a/enterprise/internal/config/config.go +++ b/enterprise/internal/config/config.go @@ -20,13 +20,21 @@ type Config struct { // SelectTenantRedirectURL is where the browser lands after a login // resolves to more than one tenant_memberships row -- // internal/loginhandler issues a pending-login cookie and sends the - // browser here instead of straight to PostLoginRedirectURL. Nothing - // serves this route yet (a real tenant-picker page is undesigned - // frontend work -- see internal/loginhandler's package doc comment); - // the backend protocol (GET /auth/memberships, POST - // /auth/select-tenant) is complete and independently testable via - // HTTP regardless of what, if anything, is listening here today. + // browser here. web/src/routes/select-tenant is the page that serves + // it (see that route's own comments) -- it calls GET + // /auth/memberships and POST /auth/select-tenant with + // `credentials: 'include'`, which is why CORSAllowedOrigin below has + // to be a literal origin, not WithCORS's zero-config "*" default. SelectTenantRedirectURL string + // CORSAllowedOrigin is passed to httpserver.WithCredentialedCORS, not + // the plain httpserver.WithCORS every other service in this repo + // uses -- GET /auth/memberships / POST /auth/select-tenant are + // cookie-carrying requests (the pending-login cookie, then the real + // session cookie), and browsers categorically refuse to combine a + // credentialed fetch with an Access-Control-Allow-Origin: "*" + // response, so this can't default to the wildcard the way + // api/internal/config.CORSAllowedOrigin does. + CORSAllowedOrigin string } type PostgresConfig struct { @@ -84,8 +92,11 @@ func Load() (Config, error) { // computed after cfg.PostLoginRedirectURL above so a caller // overriding just POST_LOGIN_REDIRECT_URL still gets a sensible // SelectTenantRedirectURL without also having to set the new - // variable. + // variable. CORSAllowedOrigin defaults the same way: web's own + // origin is exactly what needs credentialed cross-origin access to + // this service. cfg.SelectTenantRedirectURL = getenv("SELECT_TENANT_REDIRECT_URL", cfg.PostLoginRedirectURL+"/select-tenant") + cfg.CORSAllowedOrigin = getenv("CORS_ALLOWED_ORIGIN", cfg.PostLoginRedirectURL) // Required, unlike OIDC/SAML above: every enterprise-auth deployment // issues and validates session/service tokens (internal/session), diff --git a/enterprise/internal/loginhandler/loginhandler.go b/enterprise/internal/loginhandler/loginhandler.go index 797c4cf..6a78144 100644 --- a/enterprise/internal/loginhandler/loginhandler.go +++ b/enterprise/internal/loginhandler/loginhandler.go @@ -17,10 +17,10 @@ // who they are, commits to no tenant yet) and redirects to // selectTenantRedirectURL instead of issuing a session outright. // GET /auth/memberships and POST /auth/select-tenant complete the round -// trip. The backend protocol is complete and independently testable via -// HTTP; the frontend page that would actually call it doesn't exist yet -// (a real tenant-picker UI is undesigned, separately-scoped frontend -// work -- see config.SelectTenantRedirectURL's doc comment). +// trip. web/src/routes/select-tenant is the frontend page that calls +// them, over credentialed cross-origin fetch (see +// httpserver.WithCredentialedCORS and config.CORSAllowedOrigin) -- see +// that route's own comments for the page itself. package loginhandler import ( diff --git a/web/README.md b/web/README.md index 4c2dc5b..dad371c 100644 --- a/web/README.md +++ b/web/README.md @@ -38,6 +38,46 @@ docker build -f Dockerfile -t sentry-web . # context is web/, not the repo roo docker run -p 3000:3000 sentry-web ``` +## Tenant picker (Phase 4) + +`src/routes/select-tenant` is the one route that isn't reachable by +clicking around the app -- `enterprise-auth`'s `internal/loginhandler` +redirects a browser here after an SSO login resolves to more than one +`tenant_memberships` row (see that package's doc comment), carrying a +short-lived `sentry_pending_login` cookie instead of a real session. The +page calls `GET /auth/memberships` to list the choices, and +`POST /auth/select-tenant` on a click, both via +`fetch(..., {credentials: 'include'})` (`$lib/api.ts`'s +`listMemberships`/`selectTenant`) so that cookie -- and, on success, the +real session cookie the POST response sets -- actually cross the origin +boundary between this app and `enterprise-auth`. `enterprise-auth`'s +default `POST_LOGIN_REDIRECT_URL` (this app's own base URL) is also +where `CORS_ALLOWED_ORIGIN` defaults to, and it has to be a literal +origin, not `*` -- see `api/httpserver.WithCredentialedCORS`'s doc +comment for why a credentialed `fetch` and a wildcard CORS origin can +never be combined; `getAuthFeatures` above deliberately doesn't send +credentials for exactly this reason, and is why it could stay on the +plain `WithCORS` every other endpoint in this repo uses. + +Like every other route (`export const prerender = true` in this route's +own `+page.ts`), no server-side data loading -- the membership list and +the tenant choice both come from client-side `fetch` calls the same way +the root query page's does. + +Verified in a real browser in this environment: a throwaway Node server +standing in for `enterprise-auth` (implementing the exact +`GET /auth/memberships`/`POST /auth/select-tenant` wire contract, +including the plain-text `http.Error` bodies the real handler sends, not +JSON) on a different origin/port than this app's dev server, driven +through the full flow -- cross-origin pending-login cookie set, the +credentialed preflight + `GET`/`POST` round trip, a real click choosing +a tenant, and the post-selection redirect landing back on `/` -- plus +the missing/expired-cookie error path separately. No Docker or live +Postgres/IdP needed for this, since the whole point was exercising this +app's own fetch/CORS/cookie wiring against a contract-accurate fake, not +`enterprise-auth`'s internals (those are `enterprise/internal/ +loginhandler`'s own tests' job, already covered there). + ## Why nginx, not distroless The repo convention prefers distroless/scratch base images. Serving a diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 6a3dadd..a3c1dfb 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -149,6 +149,60 @@ export async function getAuthFeatures(): Promise { } } +// --- tenant picker (Phase 4) ------------------------------------------- +// +// The two calls below are the reason getAuthFeatures above doesn't send +// credentials but these do: they carry the short-lived +// sentry_pending_login cookie enterprise-auth's finishLogin sets when an +// identity resolves to more than one tenant_memberships row (see +// enterprise/internal/loginhandler's package doc comment), and +// selectTenant's response sets the real session cookie. Both require +// `credentials: 'include'`, which is exactly why enterprise-auth's CORS +// (httpserver.WithCredentialedCORS) can't use getAuthFeatures'/api.ts's +// other requests' wildcard-friendly posture -- browsers refuse to honor +// Access-Control-Allow-Origin: "*" on a credentialed request at all, so +// CORS_ALLOWED_ORIGIN has to name this page's real origin. + +export type Membership = { tenant_id: string; tenant_display_name: string; role: string }; + +class TenantPickerError extends Error {} + +// enterprise-auth's loginhandler responds to an error with plain +// http.Error text (e.g. "no membership in the requested tenant"), not a +// JSON {"error": "..."} body the way /api's queryapi/dashboards handlers +// do -- requestFrom's JSON-body error parsing wouldn't surface that +// message, so this reads the body as plain text instead. +async function enterpriseAuthRequest(path: string, init?: RequestInit): Promise { + if (!enterpriseAuthBase) { + throw new TenantPickerError('enterprise-auth is not configured (VITE_ENTERPRISE_AUTH_BASE_URL unset)'); + } + const res = await fetch(`${enterpriseAuthBase}${path}`, { + credentials: 'include', + headers: { 'Content-Type': 'application/json' }, + ...init + }); + if (!res.ok) { + const body = await res.text(); + throw new TenantPickerError(body || `request failed with status ${res.status}`); + } + return res.json(); +} + +// listMemberships backs the tenant-picker page's initial load -- see +// web/src/routes/select-tenant. A 400/401 (missing or expired pending +// login) surfaces as a thrown TenantPickerError; the page's own error +// state is what tells the user to start over at login. +export function listMemberships(): Promise { + return enterpriseAuthRequest('/auth/memberships'); +} + +export function selectTenant(tenantId: string): Promise<{ redirect_url: string }> { + return enterpriseAuthRequest('/auth/select-tenant', { + method: 'POST', + body: JSON.stringify({ tenant_id: tenantId }) + }); +} + export function exportDashboard(id: string): Promise { return request(`/dashboards/${id}/export`); } diff --git a/web/src/routes/select-tenant/+page.svelte b/web/src/routes/select-tenant/+page.svelte new file mode 100644 index 0000000..bd755c9 --- /dev/null +++ b/web/src/routes/select-tenant/+page.svelte @@ -0,0 +1,135 @@ + + +
+

Select a workspace

+ + {#if phase === 'loading'} +

Loading…

+ {:else if phase === 'error' && memberships.length === 0} +

{error}

+

Your login link may have expired. Start over by logging in again.

+ {:else} + {#if error} +

{error}

+ {/if} +
    + {#each memberships as m (m.tenant_id)} +
  • + +
    • + {/each} +
+ {/if} +
+ + diff --git a/web/src/routes/select-tenant/+page.ts b/web/src/routes/select-tenant/+page.ts new file mode 100644 index 0000000..9c9d0cb --- /dev/null +++ b/web/src/routes/select-tenant/+page.ts @@ -0,0 +1,4 @@ +// Same shape as settings/+page.ts and dashboards/+page.ts: no route +// params, data comes from a client-side fetch (here, credentialed — +// see this route's +page.svelte and $lib/api.ts's listMemberships). +export const prerender = true;