diff --git a/docs/phase-4-runbook.md b/docs/phase-4-runbook.md index a8966ed..5ec8093 100644 --- a/docs/phase-4-runbook.md +++ b/docs/phase-4-runbook.md @@ -132,32 +132,32 @@ curl -s http://localhost:8082/auth/features ``` Before a login can succeed, the logging-in identity needs a -`tenant_memberships` row -- there's no admin UI for this yet, so insert -one directly: +`tenant_memberships` row. There's still no admin UI for this, but as of +this runbook revision there's no manual SQL either -- +`enterprise-auth`'s `-create-tenant`/`-grant-membership-*` operator +flags replace the old psql dance: ```sh -docker run --rm --network sentry_default postgres:16-alpine psql \ - "postgres://sentry:sentry-dev-only@metadata-postgres:5432/sentry_metadata" -c \ - "INSERT INTO tenants (id, display_name, status) VALUES ('acme', 'Acme Corp', 'active') ON CONFLICT DO NOTHING;" -# The users row is created automatically on first login (UpsertUserBySSO) -# -- but tenant_memberships needs the user's ID, which doesn't exist -# until after a first login attempt fails with 403. Log in once (it'll -# fail with "no tenant membership"), then: -docker run --rm --network sentry_default postgres:16-alpine psql \ - "postgres://sentry:sentry-dev-only@metadata-postgres:5432/sentry_metadata" -c \ - "SELECT id, email FROM users;" -docker run --rm --network sentry_default postgres:16-alpine psql \ - "postgres://sentry:sentry-dev-only@metadata-postgres:5432/sentry_metadata" -c \ - "INSERT INTO tenant_memberships (id, tenant_id, user_id, role) VALUES (gen_random_uuid(), 'acme', '', 'viewer');" +docker compose run --rm enterprise-auth -create-tenant=acme -display-name="Acme Corp" +# Log in once at http://localhost:8082/auth/oidc/login -- it'll fail +# with "no tenant membership" (403), but UpsertUserBySSO already created +# the users row by that point, which -grant-membership-user-email needs. +docker compose run --rm enterprise-auth \ + -grant-membership-tenant=acme -grant-membership-user-email= -grant-membership-role=viewer ``` -Then visit `http://localhost:8082/auth/oidc/login` in a real browser, -complete the IdP's login, and confirm you land on +Then visit `http://localhost:8082/auth/oidc/login` again in a real +browser, complete the IdP's login, and confirm you land on `POST_LOGIN_REDIRECT_URL` (`http://localhost:3000` by default) with a -`sentry_session` cookie set. This whole bootstrap sequence (manual SQL -to create the first tenant membership) is exactly the kind of rough -edge an admin UI would smooth over -- named as real future work, not -hidden. +`sentry_session` cookie set. `-create-tenant` only touches `rbacstore` +(control-plane/RBAC) -- it's independent of `enterprise-api +-provision-tenant`'s ClickHouse/Tantivy data-plane provisioning (§8), +so a tenant created this way can log users in immediately but can't yet +serve their queries until that's run too, the same "two separate +operator actions" gap named in "Known gaps" below. Not yet built: an +equivalent for revoking/listing memberships, or anything for +`dashboard_permissions` grants (§5a) beyond calling the HTTP endpoints +directly. ## 3b. `enterprise-auth`: human login via SAML (new -- same "verified live in this session, not against a real running container or a real @@ -185,9 +185,10 @@ curl -s http://localhost:8082/auth/features # expect: {"sso_configured":true,"oidc_enabled":false,"saml_enabled":true} ``` -Bootstrapping the first `tenant_memberships` row is the same manual-SQL -dance as §3a (log in once, it fails with 403, insert the membership -using the `users` row that got created, log in again). Then visit +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 @@ -448,8 +449,13 @@ Full accounting: `/docs/security/threat-model.md`. Headline items: verification), not yet a real external IdP or a running `enterprise-auth` container. No tenant-picker UI for a multi-membership identity either (refused outright) for either protocol. -- No admin UI to create a `tenant_memberships` row -- §3a's manual SQL - bootstrap is the only way to grant a logged-in identity access today. +- No admin UI to create a `tenant_memberships` row, but §3a/§3b's manual + SQL bootstrap is gone -- `enterprise-auth -create-tenant`/ + `-grant-membership-*` (offline operator flags, same shape as + `-mint-service-token`) replace it. Nothing yet for revoking a + membership, listing a tenant's members, or changing a role after the + fact (SetMembership's upsert supports it at the storage layer; there's + just no flag exposing it). - **Per-resource dashboard grants are now enforced** (`api/dashboards`' handler reads `dashboard_permissions` via `enterprise/internal/rbacstore.DashboardPermissions`, only when diff --git a/enterprise/README.md b/enterprise/README.md index d576055..81d9a7a 100644 --- a/enterprise/README.md +++ b/enterprise/README.md @@ -150,7 +150,7 @@ silently left out: ## Package layout ``` -cmd/enterprise-auth/ config loading, OIDC discovery at startup, health/authorize/features endpoints, -mint-service-token +cmd/enterprise-auth/ config loading, OIDC discovery at startup, health/authorize/features endpoints, -mint-service-token, -create-tenant, -grant-membership-* cmd/enterprise-api/ multi-tenant-aware alternative to api/cmd/api -- see its own doc comment internal/tenant/ the ID type -- see its package doc comment before touching it internal/oidc/ coreos/go-oidc wiring: discovery, login redirect, code exchange + ID token verification @@ -232,8 +232,8 @@ docker run --rm --network sentry_default -v $(pwd)/..:/src -w /src/enterprise \ ## Turning on auth enforcement for manual testing -Off by default (see "Status" above -- there's no login flow to issue a -human session yet). To exercise the `RoleService` path end to end: +Off by default (see "Status" above). To exercise the `RoleService` path +end to end: ```sh docker compose up -d enterprise-auth @@ -246,6 +246,36 @@ TOKEN=$(docker compose run --rm enterprise-auth -mint-service-token=alerting) docker build -f Dockerfile -t sentry-enterprise-auth . # context is enterprise/, not the repo root ``` +## Bootstrapping a tenant and its first human user + +`-create-tenant`/`-grant-membership-*` are `enterprise-auth` operator +flags, same "offline action gated by access to enterprise-auth's own +environment, not a network-reachable endpoint" shape as +`-mint-service-token` -- deliberately not an authenticated HTTP admin +API, which would have to solve "who's allowed to create the very first +tenant/membership" itself. Replaces what used to be a manual `psql` +dance (see `/docs/phase-4-runbook.md` §3a's history if you're wondering +why old references to it still show up in git blame): + +```sh +docker compose run --rm enterprise-auth -create-tenant=acme -display-name="Acme Corp" +# Log in once via /auth/oidc/login or /auth/saml/login -- it fails with +# "no tenant membership" (403), but UpsertUserBySSO already created the +# users row by then, which -grant-membership-user-email needs. +docker compose run --rm enterprise-auth \ + -grant-membership-tenant=acme -grant-membership-user-email=you@example.com -grant-membership-role=owner +``` + +`-create-tenant` only touches `rbacstore` -- pair with `enterprise-api +-provision-tenant` (below) for a tenant to actually be able to run +queries, not just log in. `role=owner` also calls `SetOwner`, since a +tenant's Owner is a dedicated `tenants.owner_user_id` column, not just +the highest `tenant_memberships` role. Not yet built: revoking a +membership, listing a tenant's members, or a flag for +`dashboard_permissions` grants (those go through the HTTP endpoints +`api/dashboards`' handler now exposes -- `PUT`/`DELETE +/dashboards/{id}/permissions/{userId}`, `GET .../permissions`). + ## Provisioning a tenant and running `enterprise-api` ```sh diff --git a/enterprise/cmd/enterprise-auth/main.go b/enterprise/cmd/enterprise-auth/main.go index 9d04f04..e01822b 100644 --- a/enterprise/cmd/enterprise-auth/main.go +++ b/enterprise/cmd/enterprise-auth/main.go @@ -13,7 +13,11 @@ // (crewjam/saml's samlsp.FetchMetadata; a trusted operator-supplied URL, // same trust level as OIDC_ISSUER_URL's discovery fetch, not // end-user-controlled input). Also fully wired: -mint-service-token -// (the RoleService credential /alerting presents). +// (the RoleService credential /alerting presents), and +// -create-tenant/-grant-membership-* -- the operator actions that +// replace phase-4-runbook.md's old "log in once so UpsertUserBySSO +// creates a users row, then hand-write a psql INSERT into +// tenant_memberships" bootstrap dance with a real command. package main import ( @@ -58,6 +62,19 @@ func main() { // operator action gated by access to enterprise-auth's own // environment/secrets, not a network-reachable endpoint. mintServiceToken := flag.String("mint-service-token", "", "mint a RoleService credential for the named caller (e.g. \"alerting\") and exit") + // -create-tenant/-grant-membership-* are offline operator actions, + // same "not a network-reachable endpoint" shape as + // -mint-service-token and enterprise-api's -provision-tenant -- + // deliberately not an authenticated HTTP admin API, which would + // have to solve "who's allowed to create the very first tenant/ + // membership" (a real bootstrap problem an offline flag sidesteps + // entirely: access to enterprise-auth's own environment/secrets is + // the trust boundary, same as every other operator flag here). + createTenant := flag.String("create-tenant", "", "create a tenant row (id) in 'provisioning' status and exit -- pair with -display-name; use enterprise-api -provision-tenant separately for ClickHouse/Tantivy provisioning") + createTenantDisplayName := flag.String("display-name", "", "display name for -create-tenant (defaults to the tenant id if unset)") + grantTenant := flag.String("grant-membership-tenant", "", "tenant id to grant a membership in -- all three -grant-membership-* flags are required together") + grantUserEmail := flag.String("grant-membership-user-email", "", "email of an existing user to grant a tenant_memberships row to -- the user must have attempted an SSO login at least once already (UpsertUserBySSO creates the users row on first login, even one that then fails with \"no tenant membership\")") + grantRole := flag.String("grant-membership-role", "", "role to grant: viewer, editor, admin, or owner") // -healthcheck: same self-check mode as api/-healthcheck (see that // binary's doc comment) -- enterprise-auth's image is distroless too. healthcheck := flag.Bool("healthcheck", false, "self-check mode for Docker's HEALTHCHECK") @@ -99,6 +116,13 @@ func main() { } rbac := rbacstore.NewStore(pgPool) + if *createTenant != "" { + os.Exit(runCreateTenant(ctx, logger, rbac, *createTenant, *createTenantDisplayName)) + } + if *grantTenant != "" || *grantUserEmail != "" || *grantRole != "" { + os.Exit(runGrantMembership(ctx, logger, rbac, *grantTenant, *grantUserEmail, *grantRole)) + } + // oidcProvider stays nil (loginhandler.RegisterRoutes then registers // nothing) unless OIDC is actually configured -- matches every other // optional-config path in this codebase. @@ -179,6 +203,91 @@ func main() { } } +// runCreateTenant creates a tenant row in rbacstore, refusing if one +// already exists (idempotency-refusal, same reasoning as +// enterprise-api's runProvisionTenant refusing an already-active +// tenant: a second run for the same id is far more likely to be an +// operator mistake than an intentional retry, and CreateTenant's +// generated columns -- id is the only stable identity here, there's no +// credential to accidentally rotate -- so this guards against duplicate +// setup steps, not a security property). Only touches rbacstore -- data- +// plane provisioning (ClickHouse/Tantivy) is enterprise-api +// -provision-tenant's separate job, per +// /docs/phase-4-isolation-design.md's ordered provisioning gate; running +// this alone leaves the tenant able to log users in but not yet able to +// serve their queries, same "two separate operator actions" reality +// enterprise/README.md already discloses. +func runCreateTenant(ctx context.Context, logger *slog.Logger, rbac *rbacstore.Store, tenantID, displayName string) int { + if _, err := rbac.GetTenant(ctx, tenantID); err == nil { + logger.Error("tenant already exists -- refusing to create it again", "tenant_id", tenantID) + return 1 + } else if err != rbacstore.ErrNotFound { + logger.Error("checking for existing tenant", "error", err) + return 1 + } + name := displayName + if name == "" { + name = tenantID + } + if _, err := rbac.CreateTenant(ctx, tenantID, name); err != nil { + logger.Error("creating tenant", "error", err) + return 1 + } + logger.Info("created tenant", "tenant_id", tenantID, "display_name", name, "status", "provisioning") + return 0 +} + +// runGrantMembership replaces phase-4-runbook.md's old manual-SQL +// bootstrap: an operator who knows a user's email (the user must have +// attempted an SSO login at least once already, so UpsertUserBySSO's +// created the users row -- this flag never creates a user itself, since +// that identity has to come from a real IdP round trip, not an operator +// guess) can grant them a tenant_memberships row without touching SQL +// directly. role="owner" also calls SetOwner, since a tenant's Owner is +// a tenant-level column, not just the highest tenant_memberships role +// (see SetOwner's doc comment) -- a real Owner assignment via this flag +// needs both to agree, same as any other owner-assignment call site. +func runGrantMembership(ctx context.Context, logger *slog.Logger, rbac *rbacstore.Store, tenantID, userEmail, role string) int { + if tenantID == "" || userEmail == "" || role == "" { + logger.Error("-grant-membership-tenant, -grant-membership-user-email, and -grant-membership-role are all required together") + return 1 + } + rbacRole := rbacstore.Role(role) + switch rbacRole { + case rbacstore.RoleViewer, rbacstore.RoleEditor, rbacstore.RoleAdmin, rbacstore.RoleOwner: + default: + logger.Error("invalid -grant-membership-role -- must be viewer, editor, admin, or owner", "role", role) + return 1 + } + + if _, err := rbac.GetTenant(ctx, tenantID); err != nil { + logger.Error("looking up tenant", "tenant_id", tenantID, "error", err) + return 1 + } + user, err := rbac.GetUserByEmail(ctx, userEmail) + if err != nil { + if err == rbacstore.ErrNotFound { + logger.Error("no user with this email exists yet -- they must attempt an SSO login at least once first (it will fail with \"no tenant membership\", but UpsertUserBySSO creates the users row before that check runs)", "email", userEmail) + } else { + logger.Error("looking up user by email", "email", userEmail, "error", err) + } + return 1 + } + + if err := rbac.SetMembership(ctx, tenantID, user.ID, rbacRole); err != nil { + logger.Error("setting membership", "error", err) + return 1 + } + if rbacRole == rbacstore.RoleOwner { + if err := rbac.SetOwner(ctx, tenantID, user.ID); err != nil { + logger.Error("setting tenant owner", "error", err) + return 1 + } + } + logger.Info("granted membership", "tenant_id", tenantID, "user_id", user.ID, "email", userEmail, "role", role) + return 0 +} + // runHealthcheck mirrors api/cmd/api/main.go's runHealthcheck exactly -- // see that function's doc comment for why this execs the binary against // itself rather than using an external tool. diff --git a/enterprise/internal/rbacstore/rbacstore.go b/enterprise/internal/rbacstore/rbacstore.go index 00f50bd..f703fe3 100644 --- a/enterprise/internal/rbacstore/rbacstore.go +++ b/enterprise/internal/rbacstore/rbacstore.go @@ -111,6 +111,25 @@ func (s *Store) UpsertUserBySSO(ctx context.Context, ssoSubject, email, displayN return &u, nil } +// GetUserByEmail supports enterprise-auth's -grant-membership-* operator +// flags (cmd/enterprise-auth/main.go): granting a tenant_memberships row +// by email is friendlier than requiring the operator to already know a +// user's generated UUID, and email is the same natural key +// UpsertUserBySSO already upserts on. +func (s *Store) GetUserByEmail(ctx context.Context, email string) (*User, error) { + var u User + row := s.pool.QueryRow(ctx, ` + SELECT id, email, display_name, sso_subject, created_at, updated_at + FROM users WHERE email = $1`, email) + if err := row.Scan(&u.ID, &u.Email, &u.DisplayName, &u.SSOSubject, &u.CreatedAt, &u.UpdatedAt); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, ErrNotFound + } + return nil, fmt.Errorf("rbacstore: getting user by email: %w", err) + } + return &u, nil +} + func (s *Store) GetUser(ctx context.Context, id string) (*User, error) { var u User row := s.pool.QueryRow(ctx, ` diff --git a/enterprise/internal/rbacstore/rbacstore_test.go b/enterprise/internal/rbacstore/rbacstore_test.go index 74d8d42..607977a 100644 --- a/enterprise/internal/rbacstore/rbacstore_test.go +++ b/enterprise/internal/rbacstore/rbacstore_test.go @@ -577,3 +577,33 @@ func TestDashboardPermissionsAdapterRejectsAdminRole(t *testing.T) { t.Fatal("expected an error granting role=admin via a dashboard permission") } } + +// TestGetUserByEmail is the regression test for +// cmd/enterprise-auth/main.go's -grant-membership-user-email flag, +// which looks up an existing user by email rather than requiring the +// operator to already know their generated UUID. +func TestGetUserByEmail(t *testing.T) { + s := testStore(t) + ctx := context.Background() + email := "lookup-" + uniqueSuffix() + "@example.com" + + created, err := s.UpsertUserBySSO(ctx, "sub-"+uniqueSuffix(), email, "Lookup Me") + if err != nil { + t.Fatalf("UpsertUserBySSO: %v", err) + } + + got, err := s.GetUserByEmail(ctx, email) + if err != nil { + t.Fatalf("GetUserByEmail: %v", err) + } + if got.ID != created.ID { + t.Fatalf("GetUserByEmail returned ID %q, want %q", got.ID, created.ID) + } +} + +func TestGetUserByEmailNotFound(t *testing.T) { + s := testStore(t) + if _, err := s.GetUserByEmail(context.Background(), "does-not-exist-"+uniqueSuffix()+"@example.com"); err != ErrNotFound { + t.Fatalf("GetUserByEmail error = %v, want ErrNotFound", err) + } +}