diff --git a/docs/phase-4-runbook.md b/docs/phase-4-runbook.md index 1e05e5a..46b656e 100644 --- a/docs/phase-4-runbook.md +++ b/docs/phase-4-runbook.md @@ -60,29 +60,42 @@ Docker-free testing could have caught: `/docs/security/threat-model.md`'s "Read this first" section for the full account. -**What's still not verified, and why**: §3a (OIDC) and §12 (tenant -picker, both single- and multi-membership paths) are now closed -- -verified against a real Auth0 developer tenant, full browser round -trips, real session cookies authorizing correctly, including selecting -between two real tenant memberships and getting back the right -tenant/role each time (see §3a and §12 below). That pass also found and -fixed a real bug: `web/Dockerfile` only declared `ARG`/`ENV` for -`VITE_API_BASE_URL`, so `docker-compose.yml`'s build args for -`VITE_ALERTING_API_BASE_URL`/`VITE_ENTERPRISE_AUTH_BASE_URL` were -silently dropped by Docker (an undeclared `--build-arg` is dropped, not -an error) -- `enterpriseAuthBase` came out `undefined` in the built -bundle, so the tenant-picker page threw "enterprise-auth is not -configured" against a real running container even though -`docker-compose.yml` looked correct. Fixed by declaring all three. +**What's still not verified, and why**: §3a (OIDC), §3b (SAML), and §12 +(tenant picker, both single- and multi-membership paths) are now all +closed -- verified against a real Auth0 developer tenant acting as both +an OIDC and a SAML IdP, full browser round trips, real session cookies +authorizing correctly, including selecting between two real tenant +memberships and getting back the right tenant/role each time (see §3a, +§3b, and §12 below). That pass found and fixed two real bugs: -§3b (SAML) still needs a real external IdP with SAML app support, which -this environment doesn't have credentials for -- the fake-IdP test (real -XML signing/verification, no stand-in shortcuts) is the strongest -evidence available without one. §7 and §11's live-cluster steps need -`kind`/`kubectl`, which aren't installed in this environment -- their -offline-only checks (`go build`/`go vet`/`go test`, `helm lint`, `helm -template` + parsing the rendered YAML) all pass and are documented as -such below. +1. `web/Dockerfile` only declared `ARG`/`ENV` for `VITE_API_BASE_URL`, + so `docker-compose.yml`'s build args for + `VITE_ALERTING_API_BASE_URL`/`VITE_ENTERPRISE_AUTH_BASE_URL` were + silently dropped by Docker (an undeclared `--build-arg` is dropped, + not an error) -- `enterpriseAuthBase` came out `undefined` in the + built bundle, so the tenant-picker page threw "enterprise-auth is not + configured" against a real running container even though + `docker-compose.yml` looked correct. +2. `enterprise/internal/loginhandler.go` decided every cookie's `Secure` + attribute from `r.TLS != nil` alone -- wrong for the deployment shape + this handler actually runs in, since `enterprise-auth` never + terminates TLS itself. Closing SAML's real-IdP gap required a genuine + TLS-terminating proxy in front of it (SAML's request cookie is + `SameSite=None`, which the cookie spec requires to be paired with + `Secure`, and Chrome silently drops it otherwise), which is exactly + what surfaced this: `r.TLS` was nil at the Go process even over a + real HTTPS client connection, so the cookie came back non-`Secure` + and Chrome dropped it, breaking the flow. Fixed with an + `isSecureRequest(r)` helper that also honors + `X-Forwarded-Proto: https` -- affects every cookie this handler sets, + not just SAML's, so this would have hit any real reverse-proxied + deployment, not just this test. + +§7 and §11's live-cluster steps still need `kind`/`kubectl`, which +aren't installed in this environment -- their offline-only checks +(`go build`/`go vet`/`go test`, `helm lint`, `helm template` + parsing +the rendered YAML) all pass and are documented as such below. That's the +only remaining gap in this entire runbook that isn't already closed. If you're reading this to decide whether Phase 4 is production-ready: closer than before, but not yet -- see @@ -210,44 +223,71 @@ the one thing here still only reachable via the HTTP endpoints directly, no operator flag, since `sentryctl dashboards permissions list|grant|revoke` already exists as that surface instead (§5a). -## 3b. `enterprise-auth`: human login via SAML (new -- same "verified -live in this session, not against a real running container or a real -external IdP" caveat as §3a) +## 3b. `enterprise-auth`: human login via SAML (now genuinely verified +against a real external IdP, over real HTTPS) `enterprise/internal/loginhandler`'s SAML tests already prove the mechanism works end to end against a real fake SAML IdP (`go test ./internal/loginhandler/... -run SAML -v` from `enterprise/`, no Docker -needed). What's still unverified is wiring it into this actual running -stack. To try that for real, point `docker-compose.yml`'s -`enterprise-auth` service at a real SAML IdP (many identity providers +needed). **Closed for real in this pass**, using the same Auth0 +developer tenant §3a used, via that app's **SAML2 Web App addon** +(Auth0 acting as a real SAML IdP, not just OIDC). This is where SAML's +`SameSite=None` request cookie (which the cookie spec requires to be +paired with `Secure`) made the plain-HTTP setup this environment +otherwise defaults to a hard blocker -- getting a real assertion back +required standing up a genuine (self-signed, dev-only) TLS-terminating +nginx proxy in front of `enterprise-auth`. Doing that surfaced a real, +previously-undiscovered bug: every cookie `loginhandler.go` sets decided +`Secure` from `r.TLS != nil` alone, which is wrong for the deployment +shape this handler actually runs in -- `enterprise-auth` never +terminates TLS itself, so in any real deployment (behind an ingress/load +balancer, exactly what the proxy here stands in for) `r.TLS` is nil even +over a genuinely HTTPS client connection. The `Secure` attribute was +silently missing behind the proxy, and Chrome dropped the cookie outright +-- fixed with a new `isSecureRequest(r)` helper that also honors +`X-Forwarded-Proto: https`, with its own regression test +(`TestHandleSAMLLoginSetsSecureCookieBehindATLSProxy`). This bug affects +every cookie this handler sets, not just SAML's, and would have hit +production the same way, so this generalizes well beyond closing this +one section. + +With that fixed, the full flow completed for real: login redirected to +Auth0's real SAML SSO endpoint, Auth0 posted back a real signed +assertion, `enterprise/internal/saml` validated it (audience, destination, +signature) and extracted the email attribute, and the identity landed on +the real `/select-tenant` page with both real tenant memberships, +exactly like §12's OIDC walkthrough -- confirmed via +`POST /internal/authorize` returning the selected tenant/role. + +To try this yourself, point `docker-compose.yml`'s `enterprise-auth` +service at a real SAML IdP (many identity providers, including Auth0, offer a free developer/trial tenant with SAML app support): ```sh # Add to enterprise-auth's environment in docker-compose.yml (or a # docker-compose.override.yml): -# SAML_ENTITY_ID: "http://localhost:8082/saml/metadata" -# SAML_ACS_URL: "http://localhost:8082/auth/saml/acs" +# SAML_ENTITY_ID: "https:///saml/metadata" +# SAML_ACS_URL: "https:///auth/saml/acs" # SAML_IDP_METADATA_URL: "https://your-idp.example.com/metadata" -# Register SAML_ENTITY_ID/SAML_ACS_URL with the IdP's application config -# -- the IdP needs Sentry's ACS URL to know where to POST the assertion. +# Register SAML_ENTITY_ID (as "audience")/SAML_ACS_URL (as the +# "Application Callback URL") with the IdP's application config -- the +# IdP needs Sentry's ACS URL to know where to POST the assertion, and +# the two must actually agree or the SP-side validation fails with a +# generic "Authentication failed" (found the hard way -- crewjam/saml's +# error here doesn't distinguish audience mismatch from other causes). +# must be real HTTPS, not plain HTTP -- see above. docker compose up -d --build enterprise-auth -curl -s http://localhost:8082/auth/features +curl -s https:///auth/features # expect: {"sso_configured":true,"oidc_enabled":false,"saml_enabled":true} ``` Bootstrapping the first `tenant_memberships` row uses the same `-create-tenant`/`-grant-membership-*` flags as §3a (log in once, it fails with 403, grant the membership using the email you logged in -with, log in again). Then visit -`http://localhost:8082/auth/saml/login` in a real browser, complete the -IdP's login, and confirm a `sentry_session` cookie lands after redirect -to `POST_LOGIN_REDIRECT_URL`. Note SAML's `sentry_saml_request` cookie -is `SameSite=None`, which requires `Secure` -- i.e. this only works over -HTTPS in a real deployment, unlike OIDC's redirect-based callback which -tolerates plain HTTP for local dev (see -`enterprise/internal/loginhandler/loginhandler.go`'s `handleSAMLLogin` -doc comment for why). +with, log in again). Then visit `/auth/saml/login` on your HTTPS host in +a real browser, complete the IdP's login, and confirm a `sentry_session` +cookie lands after redirect to `POST_LOGIN_REDIRECT_URL`. ## 4. Turn on RBAC enforcement and prove it actually blocks/allows diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 550f72d..01987f1 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -52,20 +52,20 @@ per record would be a real throughput cost neither implementation accepts), so a minute-wide staleness window remains on both sides by design, not by oversight. What's left: two tenants (`acme`, `globex`) have been provisioned and exercised end-to-end in this environment. -Human login itself is now verified for real — a real Auth0 identity -logged in via OIDC, selected between both tenants, and +Human login itself is now verified for real, via both protocols — a +real Auth0 identity logged in via OIDC and (separately) via Auth0's +SAML2 Web App addon, selected between both tenants each way, and `POST /internal/authorize` confirmed each selection issued the right -tenant/role (see §3a/§12 below) — but that walkthrough ran against plain -`api` serving `web`'s traffic, not `enterprise-api`, so the specific -combination of "real human OIDC session" and "real per-tenant ClickHouse -routing via `chrunner`" in the same request hasn't been driven end to -end yet; each half is independently confirmed (real ClickHouse -connections and Go integration tests for the routing half, real Auth0 -sessions for the human-login half), just not together in one request. -SAML's human-login half still needs a real external IdP with SAML app -support (see §3b below). And **whether a given deployment actually runs -the isolated binaries** remains a deployment-time decision, not a -code-level guarantee — see below. +tenant/role (see §3a/§3b/§12 below) — but that walkthrough ran against +plain `api` serving `web`'s traffic, not `enterprise-api`, so the +specific combination of "real human SSO session" and "real per-tenant +ClickHouse routing via `chrunner`" in the same request hasn't been +driven end to end yet; each half is independently confirmed (real +ClickHouse connections and Go integration tests for the routing half, +real Auth0 sessions for the human-login half), just not together in one +request. And **whether a given deployment actually runs the isolated +binaries** remains a deployment-time decision, not a code-level +guarantee — see below. **ClickHouse (the SQL path) is built and now genuinely verified live.** `enterprise/internal/tenantprovision` (real `CREATE DATABASE`/`CREATE @@ -563,7 +563,7 @@ terms: | Deployment actually routing traffic to `enterprise-api` (Helm) | **Enforced** — `api`/`enterprise-api` are mutually exclusive, same flag as RBAC/audit/SSO | | Deployment actually routing traffic to `enterprise-api` (docker-compose) | **Enforced, verified live** — `api`/`enterprise-api` are mutually exclusive via `COMPOSE_PROFILES`, same flag choice as Helm's `enterprise.enabled`; a real `docker compose up` of `enterprise-api` was run in this environment (and caught/fixed a startup-crashing duplicate `GET /healthz` route registration bug in the process), not just `docker compose config` | | Human SSO login — OIDC | **Enforced, verified live** — real login against a real Auth0 developer tenant, full browser round trip; correctly failed closed on an identity with no `tenant_memberships` row, then succeeded and issued a real session after `-grant-membership-*`, with `POST /internal/authorize` returning exactly the granted tenant/role | -| Human SSO login — SAML | **Built, verified with a real fake IdP** (not yet tried against a real external IdP) | +| Human SSO login — SAML | **Enforced, verified live** — real login against Auth0's SAML2 Web App addon acting as a real SAML IdP, over real (self-signed, dev-only) TLS; a real signed assertion validated (audience, destination, signature), landed on `/select-tenant` with real memberships, and `POST /internal/authorize` confirmed the selected tenant/role. Found and fixed a real bug in the process: `loginhandler.go`'s cookies decided `Secure` from `r.TLS != nil` alone, which is wrong behind any TLS-terminating reverse proxy (the deployment shape this handler actually runs in) — `enterprise-auth` never terminates TLS itself, so `r.TLS` was nil even over a genuinely HTTPS connection, silently dropping `Secure` and breaking SAML's `SameSite=None` cookie | | Multi-tenant-membership login (tenant picker) | **Enforced, verified live** — a real Auth0 identity with two real tenant memberships (`acme` Admin, `globex` Viewer) landed on the real `/select-tenant` page against the real `enterprise-auth` container, rendered both with correct display names/roles via a real credentialed cross-origin `GET /auth/memberships`, and selecting either one issued a session that `POST /internal/authorize` confirmed matched — the selection genuinely determines the issued session's tenant, not just renders correctly. This pass also found and fixed a real bug: `web/Dockerfile` never declared `ARG`/`ENV` for `VITE_ALERTING_API_BASE_URL`/`VITE_ENTERPRISE_AUTH_BASE_URL`, so `docker-compose.yml`'s build args for them were silently dropped, leaving `enterpriseAuthBase` `undefined` in the built bundle | | Per-resource dashboard grants (`own/granted`) | **Enforced, verified live** — real Postgres integration tests for `dashboard_permissions` CRUD and the `PermissionStore` adapter all pass (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`) |