diff --git a/CLAUDE.md b/CLAUDE.md index fedbcd1..aac35ff 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -267,17 +267,24 @@ gate a commercially-licensed credential behind, so there was never an import-boundary reason to split it out), so read and write share one registry directly. The periodic Tantivy commit now commits every tenant index that's seen a write, not just the default one -(`IndexRegistry::commit_all`). **One gap disclosed, not fixed, in this -same change**: unlike `chwriter.Registry` (built from an -active-tenants-only snapshot at startup) and unlike the read side (gated -by `searchclient.TenantChecker`), this consumer's `resolve()` call has -no active-tenant check — this process has no Postgres access to check -against, so a still-valid-but-should-be-revoked ingest credential can -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`. **The tenant-picker page is now built too**: +(`IndexRegistry::commit_all`). **The active-tenant gap this same change +originally disclosed is now closed too**: `search/src/tenants.rs`'s +`ActiveTenantTracker` polls a new `GET /internal/active-tenants` +endpoint on `enterprise-auth` (`search` has no Postgres access, unlike +`chwriter.Registry`'s direct `rbacstore` query or the read side's +`searchclient.TenantChecker`, so this needed a network call — the same +"network boundary, not import boundary" shape `ingest`'s +`TenantResolver` already uses against the same service, authenticated +with a RoleService credential the same way `alerting` authenticates to +`api`) and `consumer.rs` refuses any tagged record whose tenant isn't in +the polled allowlist. Off unless both `ENTERPRISE_AUTH_URL` and +`ENTERPRISE_AUTH_SERVICE_TOKEN` are set (same "off unless configured" +default as everything else optional in this codebase); when they are, +startup blocks on the first fetch succeeding and later refresh failures +keep serving the last-known-good set rather than clearing it. Verified +with real HTTP round trips against a hand-rolled TCP test server in this +environment, no live enterprise-auth needed. **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 diff --git a/docker-compose.yml b/docker-compose.yml index 059af88..d626dc1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -187,6 +187,16 @@ services: # -- found by actually checking `docker compose logs search` and # seeing nothing, same silent-logging gap the agent had in Phase 0. RUST_LOG: "info" + # ENTERPRISE_AUTH_URL/ENTERPRISE_AUTH_SERVICE_TOKEN are deliberately + # NOT set here -- same reason enterprise-auth's own service below + # doesn't wire alerting's API_SERVICE_TOKEN in by default: the + # token can't be known ahead of time (mint it with + # `enterprise-auth -mint-service-token search` after enterprise-auth + # is up), so this is a manual step, not a default. Unset means + # search/src/tenants.rs's ActiveTenantTracker never starts and + # write-routing has no active-tenant gate, same as every deployment + # before this tracker existed -- see search/README.md's "Per-tenant + # indices" section for how to turn it on for manual testing. volumes: - search-index-data:/var/lib/sentry-search diff --git a/docs/architecture.md b/docs/architecture.md index ade94f4..ac136ca 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -171,17 +171,21 @@ escape hatch is opaque to any compiler-injected filter. index. No "second binary" needed here, unlike ClickHouse — Tantivy has no grant system to gate a commercially-licensed credential behind, so `IndexRegistry` already lived directly in this AGPL-core binary, and - read/write just share it. One gap is disclosed rather than closed by - this change: unlike `chwriter.Registry` (an active-tenants-only - snapshot built at startup) and unlike the read side (gated by - `searchclient.TenantChecker`), this consumer's `resolve()` call has no - active-tenant check — the process has no Postgres access to check - against — so a still-valid-but-should-be-revoked ingest credential can - 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`. + read/write just share it. The active-tenant gap this design left open + is now closed too: `search/src/tenants.rs`'s `ActiveTenantTracker` + polls a new `GET /internal/active-tenants` endpoint on + `enterprise-auth` — `search` has no Postgres access, so unlike + `chwriter.Registry`'s direct `rbacstore` query or the read side's + `searchclient.TenantChecker`, this needed a network call instead (the + same "network boundary, not import boundary" shape `ingest`'s + `TenantResolver` already uses against the same service, authenticated + with a RoleService credential like `alerting`↔`api`) — and + `consumer.rs` refuses any tagged record whose tenant isn't in the + polled allowlist. Off unless configured, fail-closed on the first + fetch, last-known-good on later refresh failures; see + `search/src/tenants.rs`'s doc comment for the full design and + `search/src/registry.rs`'s for how mechanism (index lifecycle) and + policy (the gate) responsibilities split. - `deploy/operator`'s `Tenant` CRD and `enterprise-api -provision-tenant` are now unified, deliberately lightweight: `-provision-tenant` stays the sole real actor (ClickHouse + `rbacstore`), and now also syncs its diff --git a/docs/phase-4-runbook.md b/docs/phase-4-runbook.md index f912a78..869423a 100644 --- a/docs/phase-4-runbook.md +++ b/docs/phase-4-runbook.md @@ -39,14 +39,18 @@ of what was already run and passed. Two genuine exceptions: OIDC: not yet tried against a real external IdP or a running `enterprise-auth` container. - Tantivy tenant isolation, both directions -- `search/src/registry.rs`'s - cross-tenant read isolation (§9) and, since this pass, - `search/src/consumer.rs`'s per-tenant write-routing (§14) -- verified - live, no disclaimer needed, because Tantivy is an embedded library with - no Docker/broker dependency: real indices, real documents, real - commits, run in this environment. The one thing about it that's still - 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. + cross-tenant read isolation (§9), `search/src/consumer.rs`'s per-tenant + write-routing (§14), and `search/src/tenants.rs`'s active-tenant gate + (§14) -- verified live, no disclaimer needed, because Tantivy is an + embedded library with no Docker/broker dependency, and the active- + tenant gate's only external dependency (`enterprise-auth`'s HTTP API) + was exercised against a real hand-rolled test server, not a live + container: real indices, real documents, real commits, real HTTP + requests, all run in this environment. The one thing about it that's + still unverified is not Tantivy itself but the upstream credential/ + header plumbing feeding it (ingest's `TenantResolver`, `enterprise-auth`'s + `/internal/authorize-ingest` and `/internal/active-tenants`) 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 @@ -746,21 +750,54 @@ cargo test --quiet # ingest/cmd/ingest's own guard test does on the Go side. ``` -**What's still not built, for either engine**: a live active-tenant -recheck at write time. `chwriter.Registry`'s per-tenant writer map is a -snapshot built once at `enterprise-ingest` startup from -`rbacstore.ListProvisionedDataSources` (active tenants only) -- an -unrecognized `tenant_id` is refused outright, but a tenant deprovisioned -*after* startup keeps writing successfully until the next restart. -`IndexRegistry.resolve()` has no allowlist at all on either the read or -write side -- `search` has no Postgres access to check tenant status -against -- so a still-valid-but-should-be-revoked ingest credential can -cause an orphan Tantivy index directory to be created for a tenant -that's no longer active. Narrow blast radius either way (isolated, not -cross-tenant leakage, reachable only with a real signed credential), but -real and disclosed, not silently accepted -- see -`search/src/registry.rs`'s doc comment on `resolve` and -`/docs/security/threat-model.md`'s "Read this first". +**Tantivy's write path is now active-tenant-gated too.** +`search/src/tenants.rs`'s `ActiveTenantTracker` polls a new +`GET /internal/active-tenants` endpoint on `enterprise-auth` (Go side: +`rbacstore.ListActiveTenantIDs` + `authhandler.handleActiveTenants`, +RoleService-credentialed -- mint one with +`enterprise-auth -mint-service-token search`, the same generic flag +`alerting` already uses, just a different subject name) and +`consumer.rs` refuses any tagged record whose tenant isn't in the polled +set. Off unless `ENTERPRISE_AUTH_URL`/`ENTERPRISE_AUTH_SERVICE_TOKEN` are +both set (`search/src/config.rs` rejects exactly one being set); startup +blocks on the first fetch succeeding (fail-closed cold start), and a +later refresh failure keeps serving the last-known-good set rather than +clearing it. Genuinely verified in this environment, no live +enterprise-auth needed: + +```sh +cd search +cargo test --quiet tenants:: +# start_fetches_and_serves_the_initial_list / start_sends_the_bearer_token +# run against a real (hand-rolled, dependency-free) TCP server -- actual +# reqwest request construction (the Authorization: Bearer header, the +# /internal/active-tenants path) and actual JSON response parsing, not a +# fake HTTP client. start_fails_closed_when_the_first_fetch_fails and +# start_fails_closed_when_the_server_is_unreachable prove the cold-start +# refusal: ActiveTenantTracker::start returns Err rather than falling +# back to an empty (accept-nothing, silently-safe-looking-but-wrong-for- +# operators) or permissive (accept-anything, the exact bug being closed) +# default. +go test ./internal/authhandler/... -run TestActiveTenants -v +# GET /internal/active-tenants requires a RoleService credential -- a +# valid human session (even for a real Owner) is rejected the same way +# an invalid/missing token is, the regression test for this endpoint's +# whole reason to distinguish token kinds. +``` + +**One asymmetry remains, disclosed rather than fixed**: `chwriter. +Registry`'s per-tenant writer map is a snapshot built once at +`enterprise-ingest` startup, never refreshed -- a tenant deprovisioned +after startup keeps writing successfully to ClickHouse until the next +restart, a real staleness window `ActiveTenantTracker`'s 60-second +refresh doesn't have on the Tantivy side. Neither is a live per-write +check (a database/HTTP round trip on every record would be a real +throughput cost neither implementation accepts), so both have *some* +staleness window by design -- the gap between the two windows is what's +disclosed as inconsistent here, not a claim that either is fully live. +Closing it would mean adding a periodic refresh to `chwriter.Registry` +too; not done in this pass. See `/docs/security/threat-model.md`'s "Read +this first". **Also not built**: Helm/`docker-compose.yml` do gate *whether* `enterprise-ingest` runs at all (`ingest.requireTenantCredential`, same @@ -811,12 +848,14 @@ Full accounting: `/docs/security/threat-model.md`. Headline items: see §14). `search/src/consumer.rs` reads the same header and routes each record into its own tenant's Tantivy index -- genuinely verified in this environment, unlike the ClickHouse side, since Tantivy needs - no Docker to exercise real logic. Both engines share one remaining, - disclosed gap: neither write path rechecks tenant-active status live - (ClickHouse: a startup-time snapshot, stale until restart; Tantivy: no - allowlist at all, since `search` has no Postgres access) -- narrow - blast radius, not cross-tenant leakage, but real; see §14 and - `/docs/security/threat-model.md`'s "Read this first". A + no Docker to exercise real logic. Both engines now also gate writes on + an active-tenant check: ClickHouse's is a startup-time snapshot with + no refresh (stale until the next `enterprise-ingest` restart), while + Tantivy's `ActiveTenantTracker` polls `enterprise-auth` every 60 + seconds, a tighter staleness bound the ClickHouse side wasn't updated + to match -- a real, disclosed asymmetry between the two, not a gap in + either alone; see §14 and `/docs/security/threat-model.md`'s "Read + this first". A newly-provisioned tenant's ClickHouse database and Tantivy index are both now real, isolated, and actually populated by write-routed traffic (the ClickHouse claim pending live confirmation, the Tantivy diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index db5f164..11b330a 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -9,21 +9,27 @@ for the full design rationale behind the controls described here. ## Read this first: the single most important open finding -**Updated a fourth time.** This section originally read "log data -queried through `POST /query` is not tenant-isolated at all," then -"ClickHouse is isolated but Tantivy isn't," then "ingest tags records -with a tenant identity but nothing routes the write," then "ClickHouse -write-routing is built but Tantivy's isn't." Both storage engines are -now isolated on both the read and write paths. What's left is narrower: -**whether a given deployment actually runs the isolated binaries** -(deployment-time, not code-level), and two disclosed write-side gaps, -different in kind: ClickHouse's write registry is a startup-time -snapshot of active tenants with no live recheck (a *deprovisioned* -tenant can keep writing successfully until the next `enterprise-ingest` -restart), while Tantivy's write path has no active-tenant allowlist at -all — it opens an index for *any* syntactically-valid `tenant_id` a -record carries, active, deprovisioned, or never real. See below for -both, in full. +**Updated a fifth time.** This section originally read "log data queried +through `POST /query` is not tenant-isolated at all," then "ClickHouse +is isolated but Tantivy isn't," then "ingest tags records with a tenant +identity but nothing routes the write," then "ClickHouse write-routing +is built but Tantivy's isn't," then "both are write-routed but neither +rechecks tenant-active status live." Both storage engines are now +isolated on both the read and write paths, **and both now gate writes +on an active-tenant check** — ClickHouse's (`chwriter.Registry`) and +Tantivy's (`search/src/tenants.rs`'s `ActiveTenantTracker`, new) differ +in staleness bound, not in whether the gate exists at all: ClickHouse's +is a startup-time snapshot with no refresh (a *deprovisioned* tenant can +keep writing successfully until the next `enterprise-ingest` restart); +Tantivy's polls `enterprise-auth` every 60 seconds and keeps serving the +last-known-good set through a transient refresh failure, a materially +tighter staleness window with no code changed on the ClickHouse side to +match it — a real, disclosed asymmetry between the two, not a claim +they're now identical. What's left is narrower still: **whether a given +deployment actually runs the isolated binaries** (deployment-time, not +code-level), and closing ClickHouse's staleness gap to match Tantivy's +if that inconsistency matters for a given deployment. See below for +both engines' write-routing, in full. **ClickHouse (the SQL path) is built.** `enterprise/internal/ tenantprovision` (real `CREATE DATABASE`/`CREATE USER`/`GRANT`) and @@ -117,25 +123,35 @@ library with no Docker dependency. No "second binary" was needed here, unlike ClickHouse: Tantivy has no grant system to gate a commercially-licensed credential behind, so `IndexRegistry` already lived directly in this AGPL-core `search` binary, and read/write simply -share it. +share it. This write path is now also active-tenant-gated: +`search/src/tenants.rs`'s `ActiveTenantTracker` polls a new +`GET /internal/active-tenants` endpoint on `enterprise-auth` every 60 +seconds (RoleService-credentialed, the same auth shape `alerting` uses +against `api`) and `consumer.rs` refuses any tagged record whose tenant +isn't in the polled allowlist — fail-closed on the first fetch (startup +blocks until it succeeds), last-known-good on any later refresh failure. +Off unless `ENTERPRISE_AUTH_URL`/`ENTERPRISE_AUTH_SERVICE_TOKEN` are +both set. Genuinely verified here too: real HTTP round trips (the Bearer +header actually sent, JSON parsing, both fail-closed paths) against a +hand-rolled TCP test server, no live enterprise-auth needed since +`reqwest` doesn't care that the other end is real. **Both engines share one open question** — deployment topology, covered -above — and Tantivy specifically has one gap ClickHouse's design doesn't: -`chwriter.Registry`'s map is built once at startup from an -active-tenants-only query, so an unrecognized `tenant_id` is refused -outright; `IndexRegistry.resolve()` (used for both read and write) has -no equivalent allowlist at all, because `search` has no Postgres access -to check tenant status against — a syntactically-valid `tenant_id` on a -still-valid-but-should-be-revoked ingest credential can cause an orphan -index directory to be created for a tenant that's no longer active. -Narrow blast radius (isolated, empty except for that traffic, not -cross-tenant leakage, and reachable only with a real signed credential), -but real, and not closed by this change; see -`search/src/registry.rs`'s doc comment on `resolve`. A newly-provisioned -tenant's ClickHouse database and Tantivy index are both now real, -isolated, and actually populated by write-routed agent traffic (the -ClickHouse claim pending live confirmation, the Tantivy claim already -verified). +above — and now differ only in staleness bound, not in whether an +active-tenant gate exists at all: `chwriter.Registry`'s map is built +once at `enterprise-ingest` startup from an active-tenants-only query +and never refreshed, so a tenant deprovisioned after startup keeps +writing successfully until the next restart; `ActiveTenantTracker` +refreshes every 60 seconds, a materially tighter window, with no +corresponding change made to the ClickHouse side to match it. Neither +is a live per-write check (that would mean a database round trip on +every record, a real throughput cost neither implementation accepts), +so both have *some* staleness window by design — the asymmetry between +the two windows is the one thing disclosed as inconsistent, not fixed, +here. A newly-provisioned tenant's ClickHouse database and Tantivy index +are both now real, isolated, and actually populated by write-routed +agent traffic (the ClickHouse claim pending live confirmation, the +Tantivy claim already verified). ## System overview @@ -494,7 +510,7 @@ terms: | Tantivy tenant_id resolution (`enterprise/internal/searchclient`) | **Enforced, verified live** — real gRPC wire-level test | | Ingest tenant *identity* (credential validation, tagging) | **Built and tested** — fail-closed `TenantResolver`, `tenant_id` Kafka header attached per record | | Ingest tenant *write-routing*, ClickHouse | **Built, not yet confirmed against a real ClickHouse** — `enterprise-ingest`/`chwriter.Registry` route each tagged batch to its tenant's own database, fail-closed on an untagged/unprovisioned tenant; Docker-free tests pass, live-database tests are skip-gated. Startup-time active-tenant snapshot, no live recheck — a deprovisioned tenant can keep writing until the next restart | -| Ingest tenant *write-routing*, Tantivy | **Built and genuinely verified** — `search/src/consumer.rs` routes each record into its own tenant's index via `IndexRegistry`, same registry the (already-verified) read side uses; no Docker needed, real tests pass. No active-tenant allowlist at all on write (`search` has no Postgres access) — a syntactically-valid `tenant_id` on a still-valid credential can create an orphan index for a no-longer-active tenant; narrow, disclosed, not cross-tenant leakage | +| Ingest tenant *write-routing*, Tantivy | **Built and genuinely verified** — `search/src/consumer.rs` routes each record into its own tenant's index via `IndexRegistry`, same registry the (already-verified) read side uses; no Docker needed, real tests pass. Active-tenant-gated too: `tenants::ActiveTenantTracker` polls `enterprise-auth` every 60s (off unless configured), refusing any tenant not in the polled allowlist — tighter staleness bound than ClickHouse's startup-only snapshot, an asymmetry disclosed above, not a gap on Tantivy's side specifically | | 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** — `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) | diff --git a/enterprise/README.md b/enterprise/README.md index 12a447e..15da22a 100644 --- a/enterprise/README.md +++ b/enterprise/README.md @@ -284,7 +284,7 @@ this environment. ## Package layout ``` -cmd/enterprise-auth/ config loading, OIDC discovery at startup, health/authorize/features/authorize-ingest endpoints, -mint-service-token, -create-tenant, -grant-membership-*, -revoke-membership-*, -list-memberships-tenant, -transfer-owner-*, -create-ingest-credential-tenant, -list-ingest-credentials-tenant, -revoke-ingest-credential +cmd/enterprise-auth/ config loading, OIDC discovery at startup, health/authorize/features/authorize-ingest/active-tenants endpoints, -mint-service-token, -create-tenant, -grant-membership-*, -revoke-membership-*, -list-memberships-tenant, -transfer-owner-*, -create-ingest-credential-tenant, -list-ingest-credentials-tenant, -revoke-ingest-credential cmd/enterprise-api/ multi-tenant-aware alternative to api/cmd/api -- see its own doc comment cmd/enterprise-ingest/ multi-tenant-aware alternative to ingest -mode=consumer -- see its own doc comment internal/tenant/ the ID type -- see its package doc comment before touching it @@ -390,6 +390,21 @@ TOKEN=$(docker compose run --rm enterprise-auth -mint-service-token=alerting) # alerting: set API_SERVICE_TOKEN=$TOKEN and restart ``` +Same shape for `search`'s write-side active-tenant gate +(`search/src/tenants.rs` -- see `/search/README.md`'s "Per-tenant +indices" section): + +```sh +docker compose up -d enterprise-auth +SEARCH_TOKEN=$(docker compose run --rm enterprise-auth -mint-service-token=search) +# search: set ENTERPRISE_AUTH_URL=http://enterprise-auth:8082 and +# ENTERPRISE_AUTH_SERVICE_TOKEN=$SEARCH_TOKEN, then restart -- unlike +# alerting/api above, this doesn't turn on request-level auth +# enforcement anywhere; it only gates which tenant_ids search will +# write-route into their own index, refusing anything not in +# GET /internal/active-tenants' response. +``` + ```sh docker build -f Dockerfile -t sentry-enterprise-auth . # context is enterprise/, not the repo root ``` diff --git a/enterprise/cmd/enterprise-auth/main.go b/enterprise/cmd/enterprise-auth/main.go index 6ac4798..e87cd6c 100644 --- a/enterprise/cmd/enterprise-auth/main.go +++ b/enterprise/cmd/enterprise-auth/main.go @@ -205,7 +205,7 @@ func main() { OIDCEnabled: cfg.OIDC.IssuerURL != "", SAMLEnabled: cfg.SAML.IDPMetadataURL != "", } - authhandler.New(logger, sessionManager, features, rbac).RegisterRoutes(mux) + authhandler.New(logger, sessionManager, features, rbac, rbac).RegisterRoutes(mux) loginhandler.New(logger, oidcProvider, samlProvider, sessionManager, rbac, cfg.PostLoginRedirectURL, cfg.SelectTenantRedirectURL).RegisterRoutes(mux) // Credentialed, not plain, CORS: GET /auth/memberships and POST diff --git a/enterprise/internal/authhandler/authhandler.go b/enterprise/internal/authhandler/authhandler.go index ea2d7d0..57f54b3 100644 --- a/enterprise/internal/authhandler/authhandler.go +++ b/enterprise/internal/authhandler/authhandler.go @@ -13,6 +13,9 @@ // api/authz) and a different credential type (an ingest bearer token // checked against rbacstore's ingest_credentials table, not a // session.Manager JWT) -- see ingestCredentialValidator's doc comment. +// +// GET /internal/active-tenants is a third sibling, for `search` +// (core/AGPL, Rust) -- see tenantLister's doc comment. package authhandler import ( @@ -50,21 +53,36 @@ type ingestCredentialValidator interface { ValidateIngestCredential(ctx context.Context, token string) (tenantID string, err error) } +// tenantLister is the narrow interface GET /internal/active-tenants +// needs -- *rbacstore.Store is the production implementation (the same +// concrete type ingestCredentials above already wires in, just a +// second narrow interface it happens to also satisfy). Backs +// search/src/tenants.rs's ActiveTenantTracker: search is AGPL core with +// no Postgres access and no enterprise/ import allowed, so its write- +// routing needed a network boundary to learn which tenants are active, +// the same shape ingest/internal/grpcserver.TenantResolver already uses +// against this exact service (see that package's doc comment). +type tenantLister interface { + ListActiveTenantIDs(ctx context.Context) ([]string, error) +} + type Handler struct { logger *slog.Logger manager *session.Manager features Features ingestCredentials ingestCredentialValidator + tenants tenantLister } -func New(logger *slog.Logger, manager *session.Manager, features Features, ingestCredentials ingestCredentialValidator) *Handler { - return &Handler{logger: logger, manager: manager, features: features, ingestCredentials: ingestCredentials} +func New(logger *slog.Logger, manager *session.Manager, features Features, ingestCredentials ingestCredentialValidator, tenants tenantLister) *Handler { + return &Handler{logger: logger, manager: manager, features: features, ingestCredentials: ingestCredentials, tenants: tenants} } func (h *Handler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("POST /internal/authorize", h.handleAuthorize) mux.HandleFunc("GET /auth/features", h.handleFeatures) mux.HandleFunc("POST /internal/authorize-ingest", h.handleAuthorizeIngest) + mux.HandleFunc("GET /internal/active-tenants", h.handleActiveTenants) } type featuresResponse struct { @@ -146,6 +164,44 @@ func (h *Handler) handleAuthorizeIngest(w http.ResponseWriter, r *http.Request) _ = json.NewEncoder(w).Encode(authorizeIngestResponse{TenantID: tenantID}) } +type activeTenantsResponse struct { + TenantIDs []string `json:"tenant_ids"` +} + +// handleActiveTenants is search/src/tenants.rs's ActiveTenantTracker's +// server side -- polled periodically, not per-write, to build a local +// allowlist for its write-routing gate (see that module's doc comment). +// Requires a RoleService Bearer credential (session.Manager-issued, +// Role == "service"), not a human session -- server-to-server, the same +// authentication shape /alerting presents to /api, minted via +// `enterprise-auth -mint-service-token search` (the flag is already +// generic over caller name; no change needed there for a new caller). +// Deliberately does NOT accept the tenant-scoped credential a human +// session or an ingest credential would carry: this endpoint answers +// "which tenants exist," a question no single tenant's identity should +// be able to ask on its own. +func (h *Handler) handleActiveTenants(w http.ResponseWriter, r *http.Request) { + token := bearerToken(r.Header.Get("Authorization")) + if token == "" { + http.Error(w, "no credentials presented", http.StatusUnauthorized) + return + } + claims, err := h.manager.Validate(token) + if err != nil || claims.Role != "service" { + http.Error(w, "invalid or expired credentials", http.StatusUnauthorized) + return + } + + ids, err := h.tenants.ListActiveTenantIDs(r.Context()) + if err != nil { + h.logger.Error("listing active tenant ids", "error", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(activeTenantsResponse{TenantIDs: ids}) +} + func bearerToken(header string) string { const prefix = "Bearer " if !strings.HasPrefix(header, prefix) { diff --git a/enterprise/internal/authhandler/authhandler_test.go b/enterprise/internal/authhandler/authhandler_test.go index 2503de3..146f92e 100644 --- a/enterprise/internal/authhandler/authhandler_test.go +++ b/enterprise/internal/authhandler/authhandler_test.go @@ -36,13 +36,27 @@ type fakeNotFoundError struct{} func (*fakeNotFoundError) Error() string { return "not found" } +// fakeTenantLister is an in-memory stand-in for *rbacstore.Store's +// ListActiveTenantIDs. +type fakeTenantLister struct { + ids []string + err error +} + +func (f *fakeTenantLister) ListActiveTenantIDs(_ context.Context) ([]string, error) { + if f.err != nil { + return nil, f.err + } + return f.ids, nil +} + func testHandler(t *testing.T) (*Handler, *session.Manager) { t.Helper() m, err := session.NewManager([]byte("this-is-a-32-byte-test-signing-key!")) if err != nil { t.Fatalf("session.NewManager: %v", err) } - return New(slog.New(slog.NewTextHandler(io.Discard, nil)), m, Features{}, newFakeIngestCredentialValidator()), m + return New(slog.New(slog.NewTextHandler(io.Discard, nil)), m, Features{}, newFakeIngestCredentialValidator(), &fakeTenantLister{}), m } func doAuthorize(t *testing.T, h *Handler, mutate func(*http.Request)) *httptest.ResponseRecorder { @@ -146,7 +160,7 @@ func TestFeaturesReflectsConfiguredMechanisms(t *testing.T) { if err != nil { t.Fatalf("session.NewManager: %v", err) } - h := New(slog.New(slog.NewTextHandler(io.Discard, nil)), m, Features{OIDCEnabled: true, SAMLEnabled: false}, newFakeIngestCredentialValidator()) + h := New(slog.New(slog.NewTextHandler(io.Discard, nil)), m, Features{OIDCEnabled: true, SAMLEnabled: false}, newFakeIngestCredentialValidator(), &fakeTenantLister{}) mux := http.NewServeMux() h.RegisterRoutes(mux) @@ -219,7 +233,7 @@ func TestAuthorizeIngestResolvesTenant(t *testing.T) { } validator := newFakeIngestCredentialValidator() validator.tenantByToken["real-token"] = "acme" - h := New(slog.New(slog.NewTextHandler(io.Discard, nil)), m, Features{}, validator) + h := New(slog.New(slog.NewTextHandler(io.Discard, nil)), m, Features{}, validator, &fakeTenantLister{}) rec := doAuthorizeIngest(t, h, func(r *http.Request) { r.Header.Set("Authorization", "Bearer real-token") @@ -273,3 +287,96 @@ func TestAuthorizeIngestRejectsSessionToken(t *testing.T) { t.Fatalf("status = %d, want 401 (a session token must not validate as an ingest credential)", rec.Code) } } + +func doActiveTenants(t *testing.T, h *Handler, mutate func(*http.Request)) *httptest.ResponseRecorder { + t.Helper() + mux := http.NewServeMux() + h.RegisterRoutes(mux) + req := httptest.NewRequest(http.MethodGet, "/internal/active-tenants", nil) + if mutate != nil { + mutate(req) + } + rec := httptest.NewRecorder() + mux.ServeHTTP(rec, req) + return rec +} + +func TestActiveTenantsViaServiceToken(t *testing.T) { + m, err := session.NewManager([]byte("this-is-a-32-byte-test-signing-key!")) + if err != nil { + t.Fatalf("session.NewManager: %v", err) + } + h := New(slog.New(slog.NewTextHandler(io.Discard, nil)), m, Features{}, newFakeIngestCredentialValidator(), &fakeTenantLister{ids: []string{"acme", "globex"}}) + token, err := m.IssueServiceToken("search") + if err != nil { + t.Fatalf("IssueServiceToken: %v", err) + } + + rec := doActiveTenants(t, h, func(r *http.Request) { + r.Header.Set("Authorization", "Bearer "+token) + }) + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var body activeTenantsResponse + if err := json.NewDecoder(rec.Body).Decode(&body); err != nil { + t.Fatalf("decoding response: %v", err) + } + if len(body.TenantIDs) != 2 || body.TenantIDs[0] != "acme" || body.TenantIDs[1] != "globex" { + t.Fatalf("unexpected response: %+v", body) + } +} + +func TestActiveTenantsNoCredentialsIsUnauthorized(t *testing.T) { + h, _ := testHandler(t) + rec := doActiveTenants(t, h, nil) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rec.Code) + } +} + +// TestActiveTenantsRejectsHumanSession is the regression test for this +// endpoint's whole reason to distinguish token kinds: a human session +// (even a real, validly-signed one) must not be able to list every +// active tenant in the deployment -- only a RoleService credential can. +func TestActiveTenantsRejectsHumanSession(t *testing.T) { + h, m := testHandler(t) + sessionToken, err := m.IssueUserSession("acme", "u1", "owner") + if err != nil { + t.Fatalf("IssueUserSession: %v", err) + } + rec := doActiveTenants(t, h, func(r *http.Request) { + r.Header.Set("Authorization", "Bearer "+sessionToken) + }) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401 (a human session must not satisfy the service-only active-tenants endpoint)", rec.Code) + } +} + +func TestActiveTenantsInvalidTokenIsUnauthorized(t *testing.T) { + h, _ := testHandler(t) + rec := doActiveTenants(t, h, func(r *http.Request) { + r.Header.Set("Authorization", "Bearer not-a-real-token") + }) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rec.Code) + } +} + +func TestActiveTenantsStoreErrorIsInternalError(t *testing.T) { + m, err := session.NewManager([]byte("this-is-a-32-byte-test-signing-key!")) + if err != nil { + t.Fatalf("session.NewManager: %v", err) + } + h := New(slog.New(slog.NewTextHandler(io.Discard, nil)), m, Features{}, newFakeIngestCredentialValidator(), &fakeTenantLister{err: errNotFound}) + token, err := m.IssueServiceToken("search") + if err != nil { + t.Fatalf("IssueServiceToken: %v", err) + } + rec := doActiveTenants(t, h, func(r *http.Request) { + r.Header.Set("Authorization", "Bearer "+token) + }) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500", rec.Code) + } +} diff --git a/enterprise/internal/rbacstore/rbacstore.go b/enterprise/internal/rbacstore/rbacstore.go index 3c96b99..03bad7c 100644 --- a/enterprise/internal/rbacstore/rbacstore.go +++ b/enterprise/internal/rbacstore/rbacstore.go @@ -198,6 +198,34 @@ func (s *Store) TenantIsActive(ctx context.Context, tenantID string) (bool, erro return t.Status == "active", nil } +// ListActiveTenantIDs backs enterprise-auth's GET /internal/active-tenants +// -- search (AGPL core) polls this to learn which tenant_ids it may +// safely write-route into their own Tantivy index, since it has no +// Postgres access of its own (see search/src/tenants.rs's doc comment +// for the full design). Deliberately narrower than +// ListProvisionedDataSources (which also requires ClickHouse +// credentials to be present, since chwriter.Registry needs those to +// actually connect): search's write path needs nothing but the id. +func (s *Store) ListActiveTenantIDs(ctx context.Context) ([]string, error) { + rows, err := s.pool.Query(ctx, `SELECT id FROM tenants WHERE status = 'active'`) + if err != nil { + return nil, fmt.Errorf("rbacstore: listing active tenant ids: %w", err) + } + defer rows.Close() + var ids []string + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, fmt.Errorf("rbacstore: scanning active tenant id: %w", err) + } + ids = append(ids, id) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("rbacstore: iterating active tenant ids: %w", err) + } + return ids, nil +} + // SetTenantStatus is the only way a tenant's status column changes -- // every tenant-resolution path elsewhere must re-check this via // GetTenant, never cache/assume 'active', per diff --git a/enterprise/internal/rbacstore/rbacstore_test.go b/enterprise/internal/rbacstore/rbacstore_test.go index 622eea3..be27529 100644 --- a/enterprise/internal/rbacstore/rbacstore_test.go +++ b/enterprise/internal/rbacstore/rbacstore_test.go @@ -470,6 +470,41 @@ func createTestDashboard(t *testing.T, s *Store, tenantID, createdBy string) str return id } +func TestListActiveTenantIDsExcludesNonActive(t *testing.T) { + s := testStore(t) + ctx := context.Background() + + active := "test-tenant-" + uniqueSuffix() + provisioning := "test-tenant-" + uniqueSuffix() + + for _, id := range []string{active, provisioning} { + if _, err := s.CreateTenant(ctx, id, id); err != nil { + t.Fatalf("CreateTenant %s: %v", id, err) + } + } + if err := s.SetTenantStatus(ctx, active, "active"); err != nil { + t.Fatalf("SetTenantStatus active: %v", err) + } + // provisioning stays in 'provisioning' (CreateTenant's default). + + ids, err := s.ListActiveTenantIDs(ctx) + if err != nil { + t.Fatalf("ListActiveTenantIDs: %v", err) + } + foundActive := false + for _, id := range ids { + if id == provisioning { + t.Fatalf("non-active tenant %q leaked into ListActiveTenantIDs", provisioning) + } + if id == active { + foundActive = true + } + } + if !foundActive { + t.Fatal("expected the active tenant to be in the list") + } +} + func TestSetDashboardPermissionThenGet(t *testing.T) { s := testStore(t) ctx := context.Background() diff --git a/search/Cargo.lock b/search/Cargo.lock index e932021..13e778e 100644 --- a/search/Cargo.lock +++ b/search/Cargo.lock @@ -308,6 +308,17 @@ dependencies = [ "subtle", ] +[[package]] +name = "displaydoc" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c6232dd377dcc64799954cbd3a9bb882e9cdc1308ccd87b1c098f1fb2eaf82a8" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.3", +] + [[package]] name = "downcast-rs" version = "1.2.1" @@ -382,6 +393,15 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" +[[package]] +name = "form_urlencoded" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb4cb245038516f5f85277875cdaa4f7d2c9a0fa0468de06ed190163b1581fcf" +dependencies = [ + "percent-encoding", +] + [[package]] name = "fs4" version = "0.8.4" @@ -667,13 +687,16 @@ version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96547c2556ec9d12fb1578c4eaf448b04993e7fb79cbaad930a656880a6bdfa0" dependencies = [ + "base64", "bytes", "futures-channel", "futures-util", "http", "http-body", "hyper", + "ipnet", "libc", + "percent-encoding", "pin-project-lite", "socket2 0.6.5", "tokio", @@ -681,6 +704,110 @@ dependencies = [ "tracing", ] +[[package]] +name = "icu_collections" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa68d21081c4a05d5a901a1c62add574c77048b6a1c67be3b50ce0b60d4ca513" +dependencies = [ + "displaydoc", + "potential_utf", + "utf8_iter", + "yoke", + "zerofrom", + "zerovec", +] + +[[package]] +name = "icu_locale_core" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d56e28588da92eee5c3201a6eff33fabdd49b62269c8938d4ff050ce4d900deb" +dependencies = [ + "displaydoc", + "litemap", + "tinystr", + "writeable", + "zerovec", +] + +[[package]] +name = "icu_normalizer" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "12f9cf5f235641ed274641dd81c3f28d870e276763d0797aeeab72317b1c646f" +dependencies = [ + "icu_collections", + "icu_normalizer_data", + "icu_properties", + "icu_provider", + "smallvec", + "zerovec", +] + +[[package]] +name = "icu_normalizer_data" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1563da1ed3e0b3bf3d74c9b85917ac9c56464d2f57242270c09c9e752f8021a0" + +[[package]] +name = "icu_properties" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e7ca276ad3145661a65914e6daf131ca5120cd3dcee8f8f3214b8875184a148" +dependencies = [ + "displaydoc", + "icu_collections", + "icu_locale_core", + "icu_properties_data", + "icu_provider", + "zerotrie", + "zerovec", +] + +[[package]] +name = "icu_properties_data" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e590f038c1464a96894fd6d10127e90a8be4509f56ff7ecef851b15cee0b7caa" + +[[package]] +name = "icu_provider" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92a7ed671a6aad807a8651a2e1782a6598fda9ce5185dd8158549e95a91c6428" +dependencies = [ + "displaydoc", + "icu_locale_core", + "writeable", + "yoke", + "zerofrom", + "zerotrie", + "zerovec", +] + +[[package]] +name = "idna" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b0875f23caa03898994f6ddc501886a45c7d3d62d04d2d90788d47be1b1e4de" +dependencies = [ + "idna_adapter", + "smallvec", + "utf8_iter", +] + +[[package]] +name = "idna_adapter" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb68373c0d6620ef8105e855e7745e18b0d00d3bdb07fb532e434244cdb9a714" +dependencies = [ + "icu_normalizer", + "icu_properties", +] + [[package]] name = "indexmap" version = "1.9.3" @@ -719,6 +846,12 @@ version = "4.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14c00403deb17c3221a1fe4fb571b9ed0370b3dcd116553c77fa294a3d918699" +[[package]] +name = "ipnet" +version = "2.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a756c3fac73139e83f14c2d742155dd2b78d3ee56597b419a0579b7bdd6dd78" + [[package]] name = "itertools" version = "0.12.1" @@ -800,6 +933,12 @@ version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" +[[package]] +name = "litemap" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47d9d19d1d6efa0109d2f65ff4c85cddd50bd572e5a00127ab10987290bcefae" + [[package]] name = "lock_api" version = "0.4.14" @@ -1080,6 +1219,15 @@ version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "19f132c84eca552bf34cab8ec81f1c1dcc229b811638f9d283dceabe58c5569e" +[[package]] +name = "potential_utf" +version = "0.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d83eb9bc6d8e5cf568e7a1101d60ee05e81ed50ea106026f3d18deeb046d7661" +dependencies = [ + "zerovec", +] + [[package]] name = "powerfmt" version = "0.2.0" @@ -1279,6 +1427,38 @@ version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" +[[package]] +name = "reqwest" +version = "0.12.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eddd3ca559203180a307f12d114c268abf583f59b03cb906fd0b3ff8646c1147" +dependencies = [ + "base64", + "bytes", + "futures-core", + "http", + "http-body", + "http-body-util", + "hyper", + "hyper-util", + "js-sys", + "log", + "percent-encoding", + "pin-project-lite", + "serde", + "serde_json", + "serde_urlencoded", + "sync_wrapper", + "tokio", + "tower 0.5.3", + "tower-http", + "tower-service", + "url", + "wasm-bindgen", + "wasm-bindgen-futures", + "web-sys", +] + [[package]] name = "rsasl" version = "2.3.1" @@ -1377,6 +1557,12 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" +[[package]] +name = "ryu" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f" + [[package]] name = "scopeguard" version = "1.2.0" @@ -1395,6 +1581,7 @@ version = "0.1.0" dependencies = [ "anyhow", "prost", + "reqwest", "rskafka", "serde", "serde_json", @@ -1450,6 +1637,18 @@ dependencies = [ "zmij", ] +[[package]] +name = "serde_urlencoded" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3491c14715ca2294c4d6a88f15e84739788c1d030eed8c110436aafdaa2f3fd" +dependencies = [ + "form_urlencoded", + "itoa", + "ryu", + "serde", +] + [[package]] name = "sha2" version = "0.10.9" @@ -1589,6 +1788,20 @@ name = "sync_wrapper" version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0bf256ce5efdfa370213c1dabab5935a12e49f2c58d15e9eac2870d3b4f27263" +dependencies = [ + "futures-core", +] + +[[package]] +name = "synstructure" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.119", +] [[package]] name = "tantivy" @@ -1823,6 +2036,16 @@ dependencies = [ "time-core", ] +[[package]] +name = "tinystr" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1e27c91459209c2986af3dcf603a5a74a4368754ce37414f59acc971167f643" +dependencies = [ + "displaydoc", + "zerovec", +] + [[package]] name = "tinyvec" version = "1.12.0" @@ -1964,10 +2187,29 @@ dependencies = [ "futures-util", "pin-project-lite", "sync_wrapper", + "tokio", "tower-layer", "tower-service", ] +[[package]] +name = "tower-http" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cfcf7e2740e6fc6d4d688b4ef00650406bb94adf4731e43c096c3a19fe40840" +dependencies = [ + "bitflags", + "bytes", + "futures-util", + "http", + "http-body", + "pin-project-lite", + "tower 0.5.3", + "tower-layer", + "tower-service", + "url", +] + [[package]] name = "tower-layer" version = "0.3.3" @@ -2080,12 +2322,30 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7df058c713841ad818f1dc5d3fd88063241cc61f49f5fbea4b951e8cf5a8d71d" +[[package]] +name = "url" +version = "2.5.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ff67a8a4397373c3ef660812acab3268222035010ab8680ec4215f38ba3d0eed" +dependencies = [ + "form_urlencoded", + "idna", + "percent-encoding", + "serde", +] + [[package]] name = "utf8-ranges" version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7fcfc827f90e53a02eaef5e535ee14266c1d569214c6aa70133a624d8a3164ba" +[[package]] +name = "utf8_iter" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" + [[package]] name = "uuid" version = "1.24.0" @@ -2138,6 +2398,16 @@ dependencies = [ "wasm-bindgen-shared", ] +[[package]] +name = "wasm-bindgen-futures" +version = "0.4.77" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6b7777d5cc23d0e91404e53ce2d5e8ec7acae3026b16233dba62cd3246457950" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + [[package]] name = "wasm-bindgen-macro" version = "0.2.127" @@ -2299,6 +2569,35 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "writeable" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ad82d2a33cdc9674dc7465672f271e096168fcdbe0f799d9e6db8c5892679dc" + +[[package]] +name = "yoke" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "709fe23a0424b6a435d82152b1bd3fdfb0833487d5fa90d05d42762a9891fef5" +dependencies = [ + "stable_deref_trait", + "yoke-derive", + "zerofrom", +] + +[[package]] +name = "yoke-derive" +version = "0.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de844c262c8848816172cef550288e7dc6c7b7814b4ee56b3e1553f275f1858e" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.119", + "synstructure", +] + [[package]] name = "zerocopy" version = "0.8.56" @@ -2319,6 +2618,60 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "zerofrom" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ec05a11813ea801ff6d75110ad09cd0824ddba17dfe17128ea0d5f68e6c5272" +dependencies = [ + "zerofrom-derive", +] + +[[package]] +name = "zerofrom-derive" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11532158c46691caf0f2593ea8358fed6bbf68a0315e80aae9bd41fbade684a1" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.119", + "synstructure", +] + +[[package]] +name = "zerotrie" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ea269c3bd32f0a32c321907a2ae912ba6f4649bb0fc764a15627e99a7095a3f" +dependencies = [ + "displaydoc", + "yoke", + "zerofrom", +] + +[[package]] +name = "zerovec" +version = "0.11.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94b5c6b5976d66c1d703c4fd17d3f5e43c8cedaacf604961b171adc7130896d8" +dependencies = [ + "yoke", + "zerofrom", + "zerovec-derive", +] + +[[package]] +name = "zerovec-derive" +version = "0.11.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47402523226a02bfe5230160dc3ccc089aa6f6f19e7fcbb4e6f824bbb1b4aa62" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.3", +] + [[package]] name = "zmij" version = "1.0.23" diff --git a/search/Cargo.toml b/search/Cargo.toml index b9a711a..328df5b 100644 --- a/search/Cargo.toml +++ b/search/Cargo.toml @@ -20,6 +20,14 @@ rskafka = "0.6" serde = { version = "1", features = ["derive"] } serde_json = "1" +# tenants.rs's only outbound HTTP call (GET enterprise-auth's +# /internal/active-tenants) -- default-features = false drops TLS +# support entirely, matching every other internal service-to-service +# call in this repo (plain HTTP, network placement is the trust +# boundary, not TLS -- see ingest/internal/grpcserver. +# HTTPTenantResolver's identical posture against the same service). +reqwest = { version = "0.12", default-features = false, features = ["json"] } + anyhow = "1" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } @@ -31,3 +39,8 @@ tonic-build = "0.12" # Already in the dependency graph transitively (tantivy/tonic-build both # pull it in); promoted to a direct dev-dependency for test use. tempfile = "3" +# tenants.rs's tests spin up a tiny real TCP server to exercise reqwest +# against, rather than pulling in a mocking crate -- needs tokio +# features the main binary doesn't (edition 2021's resolver keeps these +# out of the release build, test-only). +tokio = { version = "1", features = ["net", "io-util"] } diff --git a/search/README.md b/search/README.md index a7630fa..afafd6a 100644 --- a/search/README.md +++ b/search/README.md @@ -62,19 +62,28 @@ binary" needed. The periodic Tantivy commit (`COMMIT_INTERVAL_MS`) now commits every tenant index that's actually seen a write, plus the default index, via `IndexRegistry::commit_all`, not just one index. -**One residual gap, disclosed rather than fixed here**: unlike the read -side (gated by `enterprise/internal/searchclient`'s `TenantChecker`, -which refuses to search a tenant that isn't `active` in `rbacstore`) and -unlike ClickHouse's write side (`chwriter.Registry`, built from an -active-tenants-only snapshot at startup, so an unrecognized tenant has -no writer at all), this consumer's `registry.resolve()` call has no -active-tenant gate — this process has no Postgres access to check -against, the same reason `IndexRegistry` couldn't do the mid-provisioning -check itself before `TenantChecker` was added for the read side. A -still-valid (not yet revoked) ingest credential for a tenant that's no -longer active can cause an index directory to be created for it here. -See `src/registry.rs`'s doc comment on `resolve` for the full writeup, -including why closing it fully isn't scoped yet. +**The active-tenant gap is closed too, the same way the read side closes +it**: `src/tenants.rs`'s `ActiveTenantTracker` polls a new +`GET /internal/active-tenants` endpoint on `enterprise-auth` (this +process has no Postgres access, so unlike `enterprise/internal/ +searchclient`'s `TenantChecker` — a direct `rbacstore.TenantIsActive` +call, since that code runs in `enterprise/` — this needed a network +call instead), and `consumer.rs` refuses (logs and skips, never falls +back to the default or another tenant's index) any tagged record whose +`tenant_id` isn't in the polled allowlist. Off unless +`ENTERPRISE_AUTH_URL`/`ENTERPRISE_AUTH_SERVICE_TOKEN` are both set (see +Configuration below) — when they aren't, write-routing behaves exactly +as it did before this tracker existed, trusting any syntactically-valid +`tenant_id`. When they are, startup blocks on the first fetch succeeding +(fail-closed cold start — see `tenants.rs`'s doc comment for why a +partial/degraded startup isn't the safer choice, and for the +last-known-good behavior periodic refresh failures fall back to). +Verified with real HTTP round trips against a hand-rolled TCP test +server (no mocking crate needed for one endpoint) — the Bearer token +actually sent, the initial-fetch-fails-closed path, and an unreachable +server also failing closed. See `src/registry.rs`'s doc comment on +`resolve` for how the mechanism (index lifecycle) and policy (who gets +gated) responsibilities are split. ## Offset tracking: why this isn't a Kafka consumer group @@ -118,6 +127,8 @@ Environment variables (see `src/config.rs`): | `TENANTS_INDEX_PATH` | `/var/lib/sentry-search/tenants` | Per-tenant index directories live under here, one subdirectory per tenant_id (Phase 4) | | `OFFSETS_PATH` | `/var/lib/sentry-search/offsets.json` | Offset tracking file | | `COMMIT_INTERVAL_MS` | `2000` | How often buffered writes become searchable | +| `ENTERPRISE_AUTH_URL` | (empty) | Enables `tenants::ActiveTenantTracker` -- empty means write-routing has no active-tenant gate, same as every deployment before Phase 4. Must be set together with `ENTERPRISE_AUTH_SERVICE_TOKEN` below, or `Config::load` fails | +| `ENTERPRISE_AUTH_SERVICE_TOKEN` | (empty) | RoleService Bearer credential for `GET /internal/active-tenants`, minted via `enterprise-auth -mint-service-token search` | ## Building & testing @@ -147,7 +158,11 @@ pure `tenant_id_from_headers` header-extraction helper it uses is factored out and unit-tested the same way `ingest/consumer`'s Go equivalent is, including a guard test (`test_tenant_id_header_key_matches_go`) against the header-key literal -drifting from the Go side's. +drifting from the Go side's. `tenants.rs`'s tests genuinely exercise +`reqwest` against a real (if hand-rolled, dependency-free) TCP server — +the actual `Authorization: Bearer` header construction, JSON response +parsing, and both fail-closed paths (a rejected first fetch, an +unreachable server), not a fake HTTP client substituted in. ```sh # from the repo root, not search/ diff --git a/search/src/config.rs b/search/src/config.rs index 41de954..bd59360 100644 --- a/search/src/config.rs +++ b/search/src/config.rs @@ -13,14 +13,35 @@ pub struct Config { pub commit_interval: Duration, /// Phase 4: per-tenant index directories live under here, one /// subdirectory per tenant_id, opened on demand by - /// registry::IndexRegistry -- distinct from `index_path` above, - /// which stays the single shared index every ingest-written record - /// lands in regardless of tenant (see registry.rs's doc comment and - /// /docs/security/threat-model.md's ingest-tenancy caveat). Default + /// registry::IndexRegistry for both the read side (SearchRequest. + /// tenant_id) and the write side (consumer.rs, routing on each + /// record's tenant_id Kafka header) -- `index_path` above stays the + /// single shared index an untagged record, or any deployment that + /// never turned on ingest's TenantResolver, still lands in. Default /// matches the path convention deploy/operator's Tenant controller /// and enterprise/internal/rbacstore's seeded default data source /// already assume (`/var/lib/sentry-search/tenants/`). pub tenants_index_path: PathBuf, + /// Base URL of enterprise-auth's HTTP API, e.g. + /// `http://enterprise-auth:8082` -- same env var name and "empty + /// means off" shape as ingest/internal/config's own + /// ENTERPRISE_AUTH_URL. When set (together with + /// ENTERPRISE_AUTH_SERVICE_TOKEN below), tenants::ActiveTenantTracker + /// gates consumer.rs's write-routing on a polled active-tenant + /// allowlist -- see that module's doc comment for why this needed a + /// network call instead of direct Postgres access. When unset, + /// write-routing behaves exactly as it did before that tracker + /// existed: any syntactically-valid tenant_id is trusted. + pub enterprise_auth_url: Option, + /// RoleService Bearer credential this process presents to + /// enterprise-auth's GET /internal/active-tenants -- minted via the + /// existing `enterprise-auth -mint-service-token search` (the flag + /// is already generic over caller name, no backend change needed to + /// mint one for a new caller). Required together with + /// enterprise_auth_url above; Config::load fails if exactly one of + /// the two is set, rather than silently running with the tracker + /// half-configured. + pub enterprise_auth_service_token: Option, } impl Config { @@ -29,6 +50,14 @@ impl Config { .parse() .context("COMMIT_INTERVAL_MS must be a number")?; + let enterprise_auth_url = getenv_opt("ENTERPRISE_AUTH_URL"); + let enterprise_auth_service_token = getenv_opt("ENTERPRISE_AUTH_SERVICE_TOKEN"); + if enterprise_auth_url.is_some() != enterprise_auth_service_token.is_some() { + anyhow::bail!( + "ENTERPRISE_AUTH_URL and ENTERPRISE_AUTH_SERVICE_TOKEN must be set together, or neither -- got exactly one" + ); + } + Ok(Self { // Rust's SocketAddr parser needs a full address, unlike Go's // net package (ingest/api's ":PORT" convention won't parse @@ -49,6 +78,8 @@ impl Config { "TENANTS_INDEX_PATH", "/var/lib/sentry-search/tenants", )), + enterprise_auth_url, + enterprise_auth_service_token, }) } } @@ -56,3 +87,7 @@ impl Config { fn getenv(key: &str, fallback: &str) -> String { std::env::var(key).unwrap_or_else(|_| fallback.to_string()) } + +fn getenv_opt(key: &str) -> Option { + std::env::var(key).ok().filter(|v| !v.is_empty()) +} diff --git a/search/src/consumer.rs b/search/src/consumer.rs index 174b2c8..29fe122 100644 --- a/search/src/consumer.rs +++ b/search/src/consumer.rs @@ -10,6 +10,7 @@ use crate::config::Config; use crate::logsv1; use crate::offsets::OffsetStore; use crate::registry::IndexRegistry; +use crate::tenants::ActiveTenantTracker; /// Kafka message header a resolved tenant ID rides in, attached by /// ingest's gRPC front end. Mirrors `ingest/internal/grpcserver. @@ -30,7 +31,12 @@ const TENANT_ID_HEADER_KEY: &str = "tenant_id"; /// discovered dynamically, since it has to match what /// /transport/provision-topics.sh actually created anyway (documented /// cross-component contract, same as the topic name already is). -pub async fn run(cfg: Arc, registry: Arc, partition_count: i32) -> Result<()> { +pub async fn run( + cfg: Arc, + registry: Arc, + partition_count: i32, + active_tenants: Option>, +) -> Result<()> { let client = ClientBuilder::new(cfg.redpanda_brokers.clone()) .build() .await @@ -65,9 +71,10 @@ pub async fn run(cfg: Arc, registry: Arc, partition_count let client = Arc::clone(&client); let registry = Arc::clone(®istry); let offsets = Arc::clone(&offsets); + let active_tenants = active_tenants.clone(); let topic = cfg.redpanda_topic.clone(); handles.push(tokio::spawn(async move { - consume_partition(client, topic, partition, start_offset, registry, offsets).await + consume_partition(client, topic, partition, start_offset, registry, offsets, active_tenants).await })); } @@ -100,6 +107,7 @@ async fn consume_partition( start_offset: i64, registry: Arc, offsets: Arc>, + active_tenants: Option>, ) -> Result<()> { let partition_client = client .partition_client(topic.clone(), partition, UnknownTopicHandling::Error) @@ -143,17 +151,33 @@ async fn consume_partition( } let tenant_id = tenant_id_from_headers(&record_and_offset.record.headers); + + // Fail-closed active-tenant gate (see tenants.rs's doc + // comment) -- only applies to tagged records and only when + // a tracker is actually configured, matching resolve()'s + // own "empty tenant_id always means the default index" + // rule and this codebase's "off unless configured" default + // everywhere else. This is the check registry.rs's `resolve` + // doc comment used to name as missing entirely. + if !tenant_id.is_empty() { + if let Some(tracker) = &active_tenants { + if !tracker.is_active(&tenant_id).await { + tracing::warn!(record_id = %rec.record_id, tenant_id, "skipping record: tenant is not active"); + continue; + } + } + } + let index = match registry.resolve(&tenant_id).await { Ok(index) => index, Err(e) => { // Shouldn't normally happen -- ingest/grpcserver only // ever attaches a tenant_id it validated against a - // real credential -- but an unsafe/malformed - // tenant_id is a hard skip, never a silent fall-back - // to the default or any other tenant's index. See - // registry.rs's doc comment for the one residual gap - // this consumer doesn't close (no active-tenant - // check, since this process has no Postgres access). + // real credential, and the active-tenant gate above + // already refused anything not currently active when + // configured -- but an unsafe/malformed tenant_id is + // a hard skip regardless, never a silent fall-back to + // the default or any other tenant's index. tracing::error!(error = %e, record_id = %rec.record_id, tenant_id, "skipping record: failed to resolve tenant index"); continue; } diff --git a/search/src/main.rs b/search/src/main.rs index a3ea7c9..42b6fbb 100644 --- a/search/src/main.rs +++ b/search/src/main.rs @@ -4,6 +4,7 @@ mod grpc; mod index; mod offsets; mod registry; +mod tenants; pub mod logsv1 { tonic::include_proto!("sentry.logs.v1"); @@ -32,6 +33,24 @@ async fn main() -> Result<()> { let cfg = Arc::new(Config::load().context("loading config")?); + // Off unless ENTERPRISE_AUTH_URL/ENTERPRISE_AUTH_SERVICE_TOKEN are + // both set (Config::load already rejects exactly one being set). + // Blocks startup entirely on failure, same "fail hard, let the + // orchestrator restart" posture enterprise-ingest's main.go already + // uses when its own required startup fetch (rbacstore. + // ListProvisionedDataSources) fails -- see tenants.rs's doc comment + // for why a partial/degraded startup isn't the safer choice here. + let active_tenants = match (&cfg.enterprise_auth_url, &cfg.enterprise_auth_service_token) { + (Some(url), Some(token)) => { + tracing::info!(url, "active-tenant write-routing gate enabled"); + Some(tenants::ActiveTenantTracker::start(url, token).await?) + } + _ => { + tracing::info!("ENTERPRISE_AUTH_URL not set -- write-routing has no active-tenant gate"); + None + } + }; + let index = Arc::new( SearchIndex::open_or_create(&cfg.index_path).context("opening tantivy index")?, ); @@ -48,8 +67,9 @@ async fn main() -> Result<()> { let consumer_cfg = Arc::clone(&cfg); let consumer_registry = Arc::clone(®istry); + let consumer_active_tenants = active_tenants.clone(); let consumer_handle = tokio::spawn(async move { - if let Err(e) = consumer::run(consumer_cfg, consumer_registry, partition_count).await { + if let Err(e) = consumer::run(consumer_cfg, consumer_registry, partition_count, consumer_active_tenants).await { tracing::error!(error = %e, "redpanda consumer exited with error"); } }); diff --git a/search/src/registry.rs b/search/src/registry.rs index dad3437..39cb9ea 100644 --- a/search/src/registry.rs +++ b/search/src/registry.rs @@ -22,29 +22,27 @@ use crate::index::SearchIndex; /// path every Phase 0-3 deployment, and every untagged record, still /// uses. /// -/// **Known residual gap, disclosed rather than silently accepted**: -/// unlike the read side (gated by `enterprise/internal/searchclient`'s -/// `TenantChecker`, which refuses to even issue a search for a tenant -/// that isn't `active` in `rbacstore`) and unlike ClickHouse's write -/// side (`enterprise/internal/chwriter.Registry`, built once at startup -/// from `rbacstore.ListProvisionedDataSources` -- `active` tenants -/// only, so an unrecognized `tenant_id` has no writer and the whole -/// batch is refused), this registry's `resolve` has no equivalent gate -/// on the write path: `consumer.rs` calls it directly, with no Postgres -/// access to check tenant status against, the same reason this -/// module's doc comment used to give for the old read-side gap -/// `TenantChecker` was built to close. A syntactically-valid `tenant_id` -/// on an ingest credential that's still valid but should have been -/// revoked (deprovisioning does not yet revoke `ingest_credentials` -/// rows -- see `/CLAUDE.md`'s Phase 4 non-goals) can therefore cause an -/// index directory to be silently created here for a tenant that isn't -/// really active. The blast radius is narrow -- an orphan, isolated, -/// empty-except-for-that-tenant's-own-traffic index directory, not -/// cross-tenant data exposure, and only reachable with a real signed -/// ingest credential, not by an arbitrary caller -- but it is real, not -/// hypothetical. Closing it fully would mean giving `search` (AGPL -/// core, no `enterprise/` import allowed) some way to learn which -/// tenants are actually active; not designed yet. +/// `resolve` itself still has no active-tenant gate of its own -- it +/// will happily open-or-create an index for any syntactically-valid +/// `tenant_id`, active or not. That's deliberate: this struct's job is +/// managing index lifecycles, not policy, the same separation +/// `clickhousewriter.Writer` (mechanism) vs. `chwriter.Registry` +/// (policy: which tenants get a writer at all) draws on the ClickHouse +/// side. The gate lives one layer up, at each caller: +/// `enterprise/internal/searchclient.TenantChecker` for the read side +/// (refuses to even issue a search for a tenant that isn't `active` in +/// `rbacstore`, via a direct Postgres-backed query since that code runs +/// in `enterprise/`), and `consumer.rs`'s `tenants::ActiveTenantTracker` +/// for the write side (a polled allowlist fetched from a new +/// `enterprise-auth` endpoint over HTTP -- `search` is AGPL core with no +/// Postgres access and no `enterprise/` import allowed, so it needed a +/// network boundary instead of an import one, the same shape +/// `ingest/internal/grpcserver.TenantResolver` already uses against the +/// same service). Both gates are optional at this layer -- `resolve` +/// itself works identically whether or not either caller happens to +/// gate it -- so a future caller that forgets to gate would silently +/// reopen this exact class of gap; see `consumer.rs`'s call site for +/// the write side's enforcement. pub struct IndexRegistry { default_index: Arc, tenants_root: PathBuf, diff --git a/search/src/tenants.rs b/search/src/tenants.rs new file mode 100644 index 0000000..224b023 --- /dev/null +++ b/search/src/tenants.rs @@ -0,0 +1,223 @@ +use anyhow::{Context, Result}; +use std::collections::HashSet; +use std::sync::Arc; +use std::time::Duration; +use tokio::sync::RwLock; + +/// How often the tracker re-fetches the active-tenant list after its +/// first successful fetch. Not configurable -- no deployment has needed +/// to tune this yet, and a hardcoded value keeps Config's surface +/// smaller; revisit if that changes. +const REFRESH_INTERVAL: Duration = Duration::from_secs(60); + +/// Tracks which tenant_ids are currently `active` in enterprise-auth's +/// `tenants` table, polled from a new `GET /internal/active-tenants` +/// endpoint -- this closes the one gap registry.rs's `resolve` doc +/// comment used to name: `search` (AGPL core) has no Postgres access, +/// so unlike `chwriter.Registry` (an active-tenants-only snapshot built +/// from `rbacstore.ListProvisionedDataSources` at `enterprise-ingest` +/// startup) or the read side (gated by `enterprise/internal/ +/// searchclient.TenantChecker`, backed by `rbacstore.TenantIsActive` +/// directly), `consumer.rs`'s write-routing had no allowlist at all -- +/// any syntactically-valid `tenant_id` on a still-valid-but-should- +/// have-been-revoked ingest credential could get an index directory +/// created for it. +/// +/// Network boundary, not import boundary -- same shape +/// `ingest/internal/grpcserver`'s `TenantResolver` already uses against +/// this exact service, just Rust calling Go instead of Go calling Go, +/// and authenticated the same way `/alerting` authenticates to `/api`: +/// a long-lived RoleService Bearer credential +/// (`enterprise-auth -mint-service-token search`), not a tenant-scoped +/// one -- this tracker proves "I am the search service," never "I may +/// act as tenant X." +/// +/// Off unless configured: only constructed when both +/// `ENTERPRISE_AUTH_URL` and `ENTERPRISE_AUTH_SERVICE_TOKEN` are set +/// (see config.rs). When they aren't, `consumer.rs` holds `None` and +/// skips the gate entirely -- every tagged write is routed exactly as +/// it was before this tracker existed, the same "off unless configured" +/// default every other optional integration point in this codebase +/// uses. +pub struct ActiveTenantTracker { + tenants: RwLock>, +} + +impl ActiveTenantTracker { + /// Blocks until the first fetch succeeds. A cold start with + /// enterprise-auth unreachable must not silently accept every + /// tenant_id it sees -- that's the exact gap this tracker exists to + /// close -- so there is deliberately no empty-set-and-keep-going + /// fallback here; callers should refuse to start the write-routing + /// consumer at all if this returns an error. Once constructed, + /// periodic refreshes are best-effort: a transient failure logs and + /// keeps serving the last-known-good set rather than clearing it + /// (see the spawned task below) -- only the very first fetch is + /// fail-closed-to-refusing-startup. + pub async fn start(base_url: &str, service_token: &str) -> Result> { + let client = reqwest::Client::new(); + let initial = fetch_active_tenants(&client, base_url, service_token) + .await + .context("fetching initial active-tenant list from enterprise-auth")?; + tracing::info!(count = initial.len(), "loaded initial active-tenant list"); + + let tracker = Arc::new(Self { + tenants: RwLock::new(initial), + }); + + let refresh_tracker = Arc::clone(&tracker); + let base_url = base_url.to_string(); + let service_token = service_token.to_string(); + tokio::spawn(async move { + let mut ticker = tokio::time::interval(REFRESH_INTERVAL); + ticker.tick().await; // fires immediately -- start() already fetched once, skip it + loop { + ticker.tick().await; + match fetch_active_tenants(&client, &base_url, &service_token).await { + Ok(fresh) => { + let count = fresh.len(); + *refresh_tracker.tenants.write().await = fresh; + tracing::debug!(count, "refreshed active-tenant list"); + } + Err(e) => { + // No staleness ceiling: a prolonged enterprise-auth + // outage means the allowlist just doesn't grow or + // shrink until connectivity resumes, disclosed here + // rather than degrading further (e.g. clearing the + // set, which would stop every tenant's indexing on + // one control-plane blip -- a worse blast radius + // than staleness). + tracing::error!(error = %e, "failed to refresh active-tenant list, keeping last-known-good set"); + } + } + } + }); + + Ok(tracker) + } + + pub async fn is_active(&self, tenant_id: &str) -> bool { + self.tenants.read().await.contains(tenant_id) + } +} + +#[derive(serde::Deserialize)] +struct ActiveTenantsResponse { + tenant_ids: Vec, +} + +async fn fetch_active_tenants( + client: &reqwest::Client, + base_url: &str, + service_token: &str, +) -> Result> { + let resp = client + .get(format!("{base_url}/internal/active-tenants")) + .bearer_auth(service_token) + .send() + .await + .context("sending request")? + .error_for_status() + .context("non-2xx response")? + .json::() + .await + .context("parsing response body")?; + Ok(resp.tenant_ids.into_iter().collect()) +} + +#[cfg(test)] +mod tests { + use super::*; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + /// Minimal hand-rolled HTTP/1.1 server -- one dependency-free helper + /// rather than pulling in a mocking crate for the one endpoint this + /// module ever calls. Reads one request, hands it (as raw bytes) to + /// `respond`, writes back exactly what `respond` returns, then + /// closes -- enough to exercise real reqwest request construction + /// (the Bearer header, the URL path) and real response parsing, not + /// a fake client substituted in. + async fn spawn_fake_server( + respond: impl Fn(&str) -> String + Send + Sync + 'static, + ) -> String { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + loop { + let (mut stream, _) = match listener.accept().await { + Ok(v) => v, + Err(_) => return, + }; + let mut buf = vec![0u8; 8192]; + let n = stream.read(&mut buf).await.unwrap_or(0); + let request = String::from_utf8_lossy(&buf[..n]).to_string(); + let response = respond(&request); + let _ = stream.write_all(response.as_bytes()).await; + } + }); + format!("http://{addr}") + } + + fn json_response(status_line: &str, body: &str) -> String { + format!( + "{status_line}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ) + } + + #[tokio::test] + async fn start_fetches_and_serves_the_initial_list() { + let base_url = spawn_fake_server(|_req| { + json_response("HTTP/1.1 200 OK", r#"{"tenant_ids":["acme","globex"]}"#) + }) + .await; + + let tracker = ActiveTenantTracker::start(&base_url, "test-token") + .await + .expect("start should succeed against a healthy fake server"); + + assert!(tracker.is_active("acme").await); + assert!(tracker.is_active("globex").await); + assert!(!tracker.is_active("initech").await); + } + + #[tokio::test] + async fn start_sends_the_bearer_token() { + let base_url = spawn_fake_server(|req| { + if req.contains("authorization: Bearer secret-token") { + json_response("HTTP/1.1 200 OK", r#"{"tenant_ids":["acme"]}"#) + } else { + json_response("HTTP/1.1 401 Unauthorized", r#"{"error":"no credentials presented"}"#) + } + }) + .await; + + let tracker = ActiveTenantTracker::start(&base_url, "secret-token") + .await + .expect("start should succeed once the fake server sees the right token"); + assert!(tracker.is_active("acme").await); + } + + #[tokio::test] + async fn start_fails_closed_when_the_first_fetch_fails() { + let base_url = spawn_fake_server(|_req| { + json_response("HTTP/1.1 401 Unauthorized", r#"{"error":"invalid or expired credentials"}"#) + }) + .await; + + let result = ActiveTenantTracker::start(&base_url, "wrong-token").await; + assert!( + result.is_err(), + "expected start() to fail (not silently start with an empty/permissive allowlist) when the first fetch fails" + ); + } + + #[tokio::test] + async fn start_fails_closed_when_the_server_is_unreachable() { + // Port 1 is (almost certainly) not listening -- connection refused, + // not a slow timeout, so this test stays fast. + let result = ActiveTenantTracker::start("http://127.0.0.1:1", "any-token").await; + assert!(result.is_err()); + } +}