From 88f49a96ae9b08a98f1dc73adf0c8282bb6d60d8 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Sun, 26 Jul 2026 12:28:25 +0100 Subject: [PATCH] docs: phase 1 plan, identity Seven tasks: the index swap, the hq fields, the three scoped lookups, removing admin's unscoped control-plane login, and an end-to-end verification that two users sharing one address sign in to different instances. Also corrects the spec's phase list, which claimed phase 1 projects the creator as owner. Projection needs instance creation, which is phase 2. Co-Authored-By: Claude Opus 5 --- .../2026-07-26-cloud-instance-identity.md | 935 ++++++++++++++++++ ...26-07-26-cloud-instance-creation-design.md | 5 +- 2 files changed, 938 insertions(+), 2 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-26-cloud-instance-identity.md diff --git a/docs/superpowers/plans/2026-07-26-cloud-instance-identity.md b/docs/superpowers/plans/2026-07-26-cloud-instance-identity.md new file mode 100644 index 0000000..608006a --- /dev/null +++ b/docs/superpowers/plans/2026-07-26-cloud-instance-identity.md @@ -0,0 +1,935 @@ +# Cloud Instance Creation — Phase 1: Identity + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the control plane's global unique index on `users.email` with a per-instance one, and scope every lookup that relied on the global index, so one address can belong to several instances. + +**Architecture:** The index change is safe only because the two unscoped `FindOne({email})` lookups are scoped in the same binary that performs the swap. The new compound index is created **before** the old one is dropped, so a failure at any point leaves a working constraint in place. The unscoped helper is deleted rather than left unused, and admin's one unscoped control-plane lookup — which has no instance to scope by — is removed entirely. + +**Tech Stack:** Go 1.26, gin, MongoDB driver v2.8.0, `shared/indexes`, `shared/models`, `shared/provision`. + +## Global Constraints + +- **No automated Go tests.** Verification is by compiler, `grep`, and running built images against scratch databases. Every "confirm" step below is a command with expected output. This matches plans 0a through 4. +- **Never run `go` or `npm` on the host.** Everything runs in a container. The wrapper from earlier plans: + ```sh + # /tmp/gorun.sh + DIR="$1"; shift + MSYS_NO_PATHCONV=1 docker run --rm -v "$(pwd)":/src -v vantage-gomod:/go/pkg/mod \ + -v vantage-gocache:/root/.cache/go-build -w "/src/$DIR" \ + golang:1.26 "$@" + ``` +- **`MSYS_NO_PATHCONV=1` on every `docker` call.** Git Bash rewrites container paths otherwise. +- **Run `go mod tidy` with `GOWORK=off`.** In workspace mode it drops `require` lines and the Docker build then fails with "missing go.sum entry". +- **`shared/` is consumed through `replace` directives** in `server`, `admin` and `sitesvc`. A change to `shared/` reaches all three on their next build; there is no version to bump. +- **All three service images must ship together.** An older image booting after this change would recreate `email_1`. `.gitea/workflows/server-deploy.yml` rebuilds every image on every push to `main`, so this is automatic — the hazard is only a partial manual rollout on the host. +- **This migration is one-way.** Once two users share an address across instances, `email_1` cannot be recreated. There is no rollback; fixes go forward. +- Nothing in this phase projects users, creates instances, or adds UI. Those are phases 2 and 3. + +## Context this plan inherits + +`CLAUDE.md` currently states that the unique index on user email is "a security property, not an optimisation", because `GetUserByEmail` does an unscoped `FindOne`. That statement is true today and stops being true in Task 1. Task 7 updates it in the same series of commits, and the replacement property is stronger: a scoped query cannot be ambiguous, whereas an index merely prevents the ambiguity from arising. + +Spec: [`docs/superpowers/specs/2026-07-26-cloud-instance-creation-design.md`](../specs/2026-07-26-cloud-instance-creation-design.md), phase 1. + +--- + +## File Structure + +**Modified:** + +| Path | Change | +|---|---| +| `shared/indexes/indexes.go` | compound `(instance_id, email)` unique index; idempotent drop of `email_1` | +| `shared/models/user.go` | `HQUserID` field, `AuthLocal`/`AuthOIDC`/`AuthHQ` constants | +| `server/internal/services/users.go` | `GetUserByEmail` deleted, `GetUserInInstanceByEmail` added | +| `server/internal/auth/local.go` | `resolveLoginInstance`, scoped sign-in | +| `server/internal/auth/oidc.go` | scoped lookup, cross-instance guard deleted | +| `admin/internal/auth/cloud.go` | **deleted** | +| `admin/internal/api/routes.go` | `/auth/login` points at `HandleCustomerLogin`; new staff route | +| `admin/internal/api/staff.go` | `staffCreateAccountUser` | +| `CLAUDE.md` | the index security-property paragraph, and the auth section | + +**Created:** none. + +--- + +### Task 1: Compound index and the drop + +**Files:** +- Modify: `shared/indexes/indexes.go` + +**Interfaces:** +- Consumes: nothing new. +- Produces: `indexes.EnsureCoreIndexes(ctx context.Context, db *mongo.Database) error` — unchanged signature, new behaviour. Called at boot by `server`, `sitesvc` and `admin`. + +- [ ] **Step 1: Replace the body of `EnsureCoreIndexes` and add the drop helper** + +Replace the whole file with: + +```go +// Package indexes declares the MongoDB indexes more than one Vantage service +// depends on. +package indexes + +import ( + "context" + "errors" + "fmt" + + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + "go.mongodb.org/mongo-driver/v2/mongo/options" +) + +// legacyUserEmailIndex is the global unique index on users.email that this +// package used to declare. It is dropped on sight. +const legacyUserEmailIndex = "email_1" + +// indexNotFound is MongoDB's IndexNotFound error code. Two services booting at +// once can both decide to drop the legacy index; the loser must not treat that +// as a failure. +const indexNotFound = 27 + +// EnsureCoreIndexes declares the unique indexes on users and instances. +// +// users is unique on (instance_id, email), NOT on email alone. One address is +// one user WITHIN an instance; the same address may hold a user in several +// instances, because an account's people are projected into each instance they +// are granted access to. +// +// This is a security property, not an optimisation, and it is only sufficient +// because every lookup by email is scoped by instance. There is deliberately no +// unscoped lookup by email anywhere in the codebase: an unscoped FindOne would +// return an arbitrary one of several matching users, which on the login path +// means signing someone into a tenant that is not theirs. If you are about to +// add one, you are about to reintroduce that bug. +// +// Creating an index that already exists with the same specification is a no-op, +// so this is safe to call at every boot from every service. +func EnsureCoreIndexes(ctx context.Context, db *mongo.Database) error { + // Create the replacement BEFORE dropping the legacy index. A failure here + // leaves the old constraint in place, which is safe; a failure after the + // drop would leave the collection unconstrained, which is not. + if _, err := db.Collection("users").Indexes().CreateOne(ctx, mongo.IndexModel{ + Keys: bson.D{{Key: "instance_id", Value: 1}, {Key: "email", Value: 1}}, + Options: options.Index().SetUnique(true).SetName("instance_email_unique"), + }); err != nil { + return fmt.Errorf("users.instance_id+email index: %w", err) + } + + if err := dropIndexIfExists(ctx, db.Collection("users"), legacyUserEmailIndex); err != nil { + return fmt.Errorf("drop users.%s: %w", legacyUserEmailIndex, err) + } + + if _, err := db.Collection("instances").Indexes().CreateOne(ctx, mongo.IndexModel{ + Keys: bson.D{{Key: "slug", Value: 1}}, + Options: options.Index().SetUnique(true), + }); err != nil { + return fmt.Errorf("instances.slug index: %w", err) + } + + return nil +} + +// dropIndexIfExists drops name, treating "it was not there" as success whether +// that is discovered by listing or by racing another service to the drop. +func dropIndexIfExists(ctx context.Context, col *mongo.Collection, name string) error { + cur, err := col.Indexes().List(ctx) + if err != nil { + return err + } + var existing []struct { + Name string `bson:"name"` + } + if err := cur.All(ctx, &existing); err != nil { + return err + } + + found := false + for _, i := range existing { + if i.Name == name { + found = true + break + } + } + if !found { + return nil + } + + err = col.Indexes().DropOne(ctx, name) + if err == nil { + return nil + } + var srvErr mongo.ServerError + if errors.As(err, &srvErr) && srvErr.HasErrorCode(indexNotFound) { + return nil + } + return err +} +``` + +- [ ] **Step 2: Confirm it compiles** + +Run: +```sh +sh /tmp/gorun.sh shared go build ./... +``` +Expected: no output. + +- [ ] **Step 3: Confirm the legacy index is not declared anywhere else** + +Run: +```sh +grep -rn '"email"' --include=*.go shared/ server/ sitesvc/ admin/ | grep -i index +``` +Expected: no matches. If sitesvc or the server declares its own `users.email` index, it would recreate what Task 1 drops. + +- [ ] **Step 4: Commit** + +```bash +git add shared/indexes/indexes.go +git commit -m "feat(shared): unique users index is (instance_id, email) + +One address is one user within an instance, not globally, so an account's +people can be projected into every instance they are granted. + +The replacement index is created before email_1 is dropped, so a failure +at any point leaves a working constraint. The drop is idempotent and +tolerates two services racing it. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 2: `hq` fields on the user document + +**Files:** +- Modify: `shared/models/user.go` +- Modify: `server/internal/models/user.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: + - `shared/models.AuthLocal = "local"`, `AuthOIDC = "oidc"`, `AuthHQ = "hq"` + - `shared/models.User.HQUserID string` — bson `hq_user_id,omitempty` + - the same three constants re-exported from `server/internal/models`, which is a thin alias file over `shared/models` and is what server code imports + +Nothing writes `AuthHQ` or `HQUserID` in this phase. They land now so phases 2 and 3 do not have to change the shared module and rebuild every service again. + +- [ ] **Step 1: Add the constants and the field** + +In `shared/models/user.go`, after the `ValidRole` function, add: + +```go +// Auth sources. A user's auth_source says who owns the row. +const ( + AuthLocal = "local" + AuthOIDC = "oidc" + // AuthHQ marks a user projected from a Vantage HQ account. Its role, + // password and existence are owned by HQ, and the instance API refuses to + // change any of them locally — a role editable in two places is a role with + // two answers. + AuthHQ = "hq" +) +``` + +And in the `User` struct, add `HQUserID` immediately after `AuthSource`: + +```go +type User struct { + ID bson.ObjectID `bson:"_id,omitempty" json:"_id,omitempty"` + UserID string `bson:"user_id" json:"user_id"` + InstanceID string `bson:"instance_id" json:"instance_id"` + Email string `bson:"email" json:"email"` + PasswordHash string `bson:"password_hash,omitempty" json:"-"` + Role string `bson:"role" json:"role"` + AuthSource string `bson:"auth_source" json:"auth_source"` + // HQUserID is the customer_users.user_id this row was projected from, + // absent on locally-created users. + HQUserID string `bson:"hq_user_id,omitempty" json:"hq_user_id,omitempty"` + CreatedAt time.Time `bson:"created_at" json:"created_at"` + LastLogin *time.Time `bson:"last_login,omitempty" json:"last_login,omitempty"` +} +``` + +- [ ] **Step 2: Re-export the constants from the server's alias file** + +`server/internal/models/user.go` is a thin alias over `shared/models`, and server code imports that rather than the shared package directly. Add the auth sources alongside the roles it already re-exports: + +```go +package models + +import shared "github.com/mrhid6/vantage/shared/models" + +type User = shared.User + +const ( + RoleOwner = shared.RoleOwner + RoleAdmin = shared.RoleAdmin + RoleMember = shared.RoleMember +) + +const ( + AuthLocal = shared.AuthLocal + AuthOIDC = shared.AuthOIDC + AuthHQ = shared.AuthHQ +) + +func ValidRole(role string) bool { return shared.ValidRole(role) } +``` + +- [ ] **Step 3: Confirm both compile** + +Run: +```sh +sh /tmp/gorun.sh shared go build ./... +sh /tmp/gorun.sh server go build ./... +``` +Expected: no output from either. + +- [ ] **Step 4: Commit** + +```bash +git add shared/models/user.go server/internal/models/user.go +git commit -m "feat(shared): auth_source constants and hq_user_id on User + +Nothing writes them yet. They land now so phases 2 and 3 do not require a +second rebuild of every service that consumes the shared module. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 3: Scoped lookup in the user service + +**Files:** +- Modify: `server/internal/services/users.go:65-75` + +**Interfaces:** +- Consumes: `shared/indexes` from Task 1. +- Produces: `services.GetUserInInstanceByEmail(instanceID, email string) (*models.User, error)`. +- Removes: `services.GetUserByEmail`. Tasks 4 and 5 fix its two callers; the build will be red between this task and Task 5, which is expected and is why they are adjacent. + +- [ ] **Step 1: Replace `GetUserByEmail`** + +In `server/internal/services/users.go`, delete the whole `GetUserByEmail` function and put this in its place: + +```go +// GetUserInInstanceByEmail finds a user by address WITHIN one instance. +// +// There is deliberately no unscoped lookup by email. users is unique on +// (instance_id, email), not on email alone, so an unscoped FindOne would return +// an arbitrary one of several matching users — which on the login path means +// signing someone into a tenant that is not theirs. +func GetUserInInstanceByEmail(instanceID, email string) (*models.User, error) { + email = strings.ToLower(strings.TrimSpace(email)) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + var u models.User + err := db.Col("users").FindOne(ctx, bson.M{ + "instance_id": instanceID, + "email": email, + }).Decode(&u) + if err != nil { + return nil, err + } + return &u, nil +} +``` + +- [ ] **Step 2: Confirm the unscoped helper is gone and the build is red for the expected reason** + +Run: +```sh +grep -rn "GetUserByEmail" --include=*.go . +``` +Expected: exactly two matches, both call sites — `server/internal/auth/local.go` and `server/internal/auth/oidc.go`. No definition. + +Run: +```sh +sh /tmp/gorun.sh server go build ./... +``` +Expected: FAIL with `undefined: services.GetUserByEmail` at those two call sites. Any other error means something else was broken. + +- [ ] **Step 3: Do not commit yet** + +The build is red. Commit at the end of Task 5, when both callers are fixed. A commit that does not build is a commit nobody can bisect through. + +--- + +### Task 4: Scoped local login + +**Files:** +- Modify: `server/internal/auth/local.go:25-49` + +**Interfaces:** +- Consumes: `services.GetUserInInstanceByEmail` from Task 3, `services.CountInstances` and `services.FirstInstance` from `server/internal/services/instances.go:57` and `:63`, `auth.InstanceFromHost` from `server/internal/auth/instancehost.go:53`. +- Produces: `resolveLoginInstance(c *gin.Context) (string, error)`, unexported, used only by this file. + +**Behaviour change worth knowing:** signing in at the bare apex host stops working when more than one instance exists. Cloud sign-in is always on `.vantage.` — `APP_LOGIN_URL` fills `{slug}` in, so every link already points there — and self-hosted has exactly one instance, so both supported paths keep working. A bookmark to the apex login page on a multi-instance deployment will now get a 400 that names the cause. + +- [ ] **Step 1: Add `resolveLoginInstance` and rewrite `HandleLocalLogin`** + +In `server/internal/auth/local.go`, add `"fmt"` to the imports if it is not already there, then add above `HandleLocalLogin`: + +```go +// resolveLoginInstance decides which instance a sign-in attempt belongs to. +// +// Cloud always answers from the host: every instance has its own subdomain, and +// APP_LOGIN_URL fills the slug in, so every sign-in link already points at one. +// Self-hosted has no subdomain and exactly one instance, because a licence +// binds one instance UUID. +// +// Anything else is refused rather than guessed. Picking an instance on someone's +// behalf is how you sign them into the wrong tenant. +func resolveLoginInstance(c *gin.Context) (string, error) { + if inst, ok := InstanceFromHost(c); ok { + return inst.InstanceID, nil + } + n, err := services.CountInstances() + if err != nil { + return "", err + } + if n != 1 { + return "", fmt.Errorf( + "cannot tell which instance this sign-in is for: %d instances exist and the host %q names none of them; sign in at your instance's own address", + n, c.Request.Host) + } + inst, err := services.FirstInstance() + if err != nil { + return "", err + } + return inst.InstanceID, nil +} +``` + +Then replace the body of `HandleLocalLogin` between the JSON bind and `SaveSession` with: + +```go + instanceID, err := resolveLoginInstance(c) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + u, err := services.GetUserInInstanceByEmail(instanceID, body.Email) + if err != nil || !services.VerifyPassword(u, body.Password) { + c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid credentials"}) + return + } +``` + +The `SaveSession` call below it is unchanged: it already reads `u.InstanceID`. + +- [ ] **Step 2: Confirm only the OIDC caller is left broken** + +Run: +```sh +sh /tmp/gorun.sh server go build ./... +``` +Expected: FAIL with `undefined: services.GetUserByEmail` at `internal/auth/oidc.go:130` only. + +--- + +### Task 5: Scoped OIDC callback + +**Files:** +- Modify: `server/internal/auth/oidc.go:129-141` + +**Interfaces:** +- Consumes: `services.GetUserInInstanceByEmail` from Task 3. +- Produces: nothing new. + +The cross-instance guard is deleted because it becomes unreachable: the lookup is now scoped to `instanceID`, so a user belonging to another instance is simply not found, and the OIDC callback provisions a new member — which is correct. OIDC is configured per instance, so only that instance's identity provider can reach this code with that instance's state. + +- [ ] **Step 1: Replace the lookup and delete the guard** + +In `server/internal/auth/oidc.go`, replace: + +```go + email := strings.ToLower(claims.Email) + u, err := services.GetUserByEmail(email) + if err != nil { + + u, err = services.CreateUser(instanceID, email, "", "member", "oidc") + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "provisioning failed"}) + return + } + } else if u.InstanceID != instanceID { + c.JSON(http.StatusForbidden, gin.H{"error": "email belongs to a different organization"}) + return + } +``` + +with: + +```go + email := strings.ToLower(claims.Email) + + // Scoped to the instance the callback state names, so an address that also + // exists in another instance is invisible here. That scoping replaces the + // cross-instance guard this code used to need: there is no longer a way for + // the lookup to return a user belonging to somebody else. + u, err := services.GetUserInInstanceByEmail(instanceID, email) + if err != nil { + u, err = services.CreateUser(instanceID, email, "", models.RoleMember, models.AuthOIDC) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "provisioning failed"}) + return + } + } +``` + +`services.CreateUser`'s signature is `CreateUser(instanceID, email, password, role, authSource string)` — the argument order above matches it, with the two string literals the old code passed replaced by the constants Task 2 added. + +`oidc.go` already imports `github.com/mrhid6/vantage/server/internal/models`; confirm it before relying on the constants: + +```sh +grep -n "server/internal/models" server/internal/auth/oidc.go +``` + +If that returns nothing, add the import rather than reverting to string literals — Task 2 exists so these two values have one spelling. + +- [ ] **Step 2: Confirm the build is green** + +Run: +```sh +sh /tmp/gorun.sh server go build ./... +``` +Expected: no output. + +- [ ] **Step 3: Confirm no unscoped email lookup survives anywhere in the server** + +Run: +```sh +grep -rn "GetUserByEmail" --include=*.go . +``` +Expected: no matches at all. + +Run: +```sh +grep -rn 'FindOne(ctx, bson.M{"email"' --include=*.go server/ +``` +Expected: no matches. + +**Coverage note.** The spec's phase-1 test 6 exercises this path end to end, which needs a working identity provider and is not reproducible in the container harness Task 7 uses. It is verified here by inspection and by the greps in Step 3 instead: the lookup is scoped by `instanceID`, which comes from `ConsumeStateInstance` and not from user input, and the deleted guard was the only other consumer of the unscoped helper. The first real OIDC sign-in after deployment is the confirming evidence — check that an existing SSO user still lands in their own instance before considering this closed. + +- [ ] **Step 4: Commit Tasks 3, 4 and 5 together** + +```bash +git add server/internal/services/users.go server/internal/auth/local.go server/internal/auth/oidc.go +git commit -m "feat(server): scope every user lookup by instance + +users is unique on (instance_id, email) now, so an unscoped FindOne could +return an arbitrary one of several matching users. On the login path that +means signing someone into a tenant that is not theirs. + +GetUserByEmail is deleted rather than left unused. Local sign-in resolves +its instance from the host, falling back to the single instance a +self-hosted deployment has, and refuses to guess otherwise. The OIDC +cross-instance guard goes: a scoped lookup cannot return another +instance's user, which is a stronger guarantee than the check it replaces. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 6: Remove admin's unscoped control-plane login + +**Files:** +- Delete: `admin/internal/auth/cloud.go` +- Modify: `admin/internal/api/routes.go:30`, `admin/internal/api/routes.go:50-52` +- Modify: `admin/internal/api/staff.go` + +**Interfaces:** +- Consumes: `auth.CreateCustomerUser(ctx, accountID, email, password string) error` from `admin/internal/auth/customer.go:32`. +- Produces: `POST /api/staff/accounts/:id/users`. + +`HandleCloudLogin` authenticates against control-plane `users` with an unscoped `FindOne({email})`, and unlike the server's two lookups there is no instance in context to scope it by — HQ sign-in is not per-instance. It already falls through to `HandleCustomerLogin` whenever a `customer_users` row exists, which after phase 2 is every customer. Legacy cloud customers get an HQ login from staff, which is what the new endpoint is for; staff already attach those instances by hand per the spec README. + +- [ ] **Step 1: Delete the file** + +```sh +git rm admin/internal/auth/cloud.go +``` + +- [ ] **Step 2: Point `/auth/login` at the customer handler** + +In `admin/internal/api/routes.go`, replace: + +```go + r.POST("/auth/login", auth.HandleCloudLogin) // falls through to customer login +``` + +with: + +```go + // Every customer authenticates against admin's own customer_users. There is + // deliberately no path that looks a customer up in the control plane by + // email alone: HQ sign-in names no instance, so such a lookup could not be + // scoped, and users.email is no longer globally unique. + r.POST("/auth/login", auth.HandleCustomerLogin) +``` + +- [ ] **Step 3: Add the staff route** + +In `admin/internal/api/routes.go`, inside the `staff` group, immediately after the `staff.GET("/accounts/:id", staffGetAccount)` line, add: + +```go + staff.POST("/accounts/:id/users", staffCreateAccountUser) +``` + +- [ ] **Step 4: Add the handler** + +At the end of `admin/internal/api/staff.go`, add: + +```go +// staffCreateAccountUser gives an account an HQ login. +// +// This is how a legacy cloud customer — one whose instance predates HQ accounts +// — gets into the portal, alongside the manual instance attach the spec README +// describes. It reuses CreateCustomerUser, so the row is unverified until the +// emailed link is opened and is rolled back if that email cannot be sent. +func staffCreateAccountUser(c *gin.Context) { + var body struct { + Email string `json:"email"` + Password string `json:"password"` + } + if err := c.ShouldBindJSON(&body); err != nil || body.Email == "" || len(body.Password) < 12 { + c.JSON(http.StatusBadRequest, gin.H{ + "error": "email and a password of at least 12 characters are required"}) + return + } + ctx := c.Request.Context() + accountID := c.Param("id") + + if n, err := db.Admin("accounts").CountDocuments(ctx, + bson.M{"account_id": accountID}); err != nil || n == 0 { + c.JSON(http.StatusNotFound, gin.H{"error": "no such account"}) + return + } + + email := strings.ToLower(strings.TrimSpace(body.Email)) + if err := auth.CreateCustomerUser(ctx, accountID, email, body.Password); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + + s := auth.Current(c) + audit.Write(ctx, models.AuditEntry{ + Actor: s.Email, Action: "customer_user.created", AccountID: accountID, Target: email}) + c.JSON(http.StatusCreated, gin.H{"pending": true}) +} +``` + +Confirm `strings` is imported in `staff.go`; add it if not: + +```sh +grep -n '"strings"' admin/internal/api/staff.go +``` + +- [ ] **Step 5: Confirm the build is green and nothing still references the deleted handler** + +Run: +```sh +grep -rn "HandleCloudLogin" --include=*.go . +``` +Expected: no matches. + +Run: +```sh +sh /tmp/gorun.sh admin go build ./... +``` +Expected: no output. If `sharedmodels` is now an unused import in some file, remove that import line. + +- [ ] **Step 6: Confirm admin has no unscoped control-plane user lookup left** + +Run: +```sh +grep -rn 'db.Control("users")' --include=*.go admin/ +``` +Expected: no matches. + +- [ ] **Step 7: Commit** + +```bash +git add -A admin/ +git commit -m "feat(admin): drop the unscoped control-plane login branch + +HQ sign-in names no instance, so a lookup of control-plane users by email +alone cannot be scoped — and users.email is no longer globally unique, so +it would return an arbitrary match. Every customer authenticates against +customer_users instead. + +Legacy cloud customers get an HQ login from staff via the new +POST /api/staff/accounts/:id/users, alongside the manual instance attach +the spec README already describes. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 7: Documentation and end-to-end verification + +**Files:** +- Modify: `CLAUDE.md` + +**Interfaces:** +- Consumes: everything above. +- Produces: nothing. + +This is the task that proves the change. With no test suite, this transcript is the only evidence, so run it in full rather than skimming it. + +- [ ] **Step 1: Update `CLAUDE.md`** + +In the **Auth and Orgs** section, replace the paragraph beginning "Unique indexes on user email and org slug are a **security property**" with: + +```markdown +Unique indexes are a **security property**, not an optimisation. `users` is +unique on `(instance_id, email)` — one address is one user *within* an instance, +and the same address may hold a user in several instances, because an account's +people are projected into each instance they are granted. This is sufficient only +because **every lookup by email is scoped by instance**; there is deliberately no +unscoped lookup anywhere, and adding one would let the login path return an +arbitrary one of several matching users. Instance slug, settings instance and ESO +token hash remain globally unique. +``` + +In the **Security** section, replace the "Unique indexes on user email, org slug…" bullet with: + +```markdown +- Unique indexes on `(instance_id, email)`, instance slug, settings instance and the ESO token hash are load-bearing for tenant isolation. So is the absence of any unscoped lookup by email. +``` + +In the **MongoDB Collections** notes, add: + +```markdown +- `users.auth_source` is `local`, `oidc` or `hq`. An `hq` user was projected from a Vantage HQ account and carries `hq_user_id`; HQ owns its role, password and existence. +``` + +- [ ] **Step 2: Build both images** + +```sh +MSYS_NO_PATHCONV=1 docker build -q -f server/Dockerfile -t vantage-server:test . +MSYS_NO_PATHCONV=1 docker build -q -f admin/Dockerfile -t vantage-admin:test . +``` +Expected: two image IDs. A "missing go.sum entry" failure here means `go mod tidy` was run in workspace mode. + +- [ ] **Step 3: Start a scratch Mongo and Redis, and seed the OLD index** + +Redis is not optional here: the server stores sessions in it, so every sign-in below fails without it. + +```sh +MSYS_NO_PATHCONV=1 docker run -d --name vantage-idx-redis -p 6389:6379 redis:7 +MSYS_NO_PATHCONV=1 docker run -d --name vantage-idx-mongo -p 27023:27017 mongo:7 + +MSYS_NO_PATHCONV=1 docker run --rm --add-host host.docker.internal:host-gateway mongo:7 \ + mongosh "mongodb://host.docker.internal:27023/vantage_idx" --quiet --eval \ + 'db.users.createIndex({email:1},{unique:true}); db.getCollection("users").getIndexes().map(i=>i.name)' +``` +Expected: output includes `email_1`. This reproduces a database that predates the change. + +- [ ] **Step 4: Boot the server and confirm the swap** + +```sh +MSYS_NO_PATHCONV=1 docker run -d --name vantage-idx-server -p 8091:8080 \ + -e MONGO_URI=mongodb://host.docker.internal:27023 -e MONGO_DB=vantage_idx \ + -e GRPC_HOST=localhost:9090 -e REDIS_ADDR=host.docker.internal:6389 \ + -e GITEA_HOST=example.invalid \ + --add-host host.docker.internal:host-gateway vantage-server:test + +MSYS_NO_PATHCONV=1 docker run --rm --add-host host.docker.internal:host-gateway mongo:7 \ + mongosh "mongodb://host.docker.internal:27023/vantage_idx" --quiet --eval \ + 'db.getCollection("users").getIndexes().map(i=>({name:i.name,key:i.key,unique:i.unique}))' +``` +Expected: `instance_email_unique` present with key `{instance_id:1, email:1}` and `unique:true`; **no `email_1`**. + +- [ ] **Step 5: Confirm a second boot is a no-op** + +```sh +MSYS_NO_PATHCONV=1 docker restart vantage-idx-server +sleep 5 +MSYS_NO_PATHCONV=1 docker logs vantage-idx-server 2>&1 | grep -i "index\|fatal" | tail -5 +``` +Expected: no index error and no fatal. The drop must tolerate the index already being gone. + +- [ ] **Step 6: Bootstrap instance A and capture its user's password hash** + +```sh +curl -s -X POST http://localhost:8091/auth/bootstrap \ + -H 'Content-Type: application/json' \ + -d '{"instance_name":"Alpha","email":"shared@example.com","password":"hunter2hunter2"}' +``` +Expected: JSON with `instance_id` and `"slug":"alpha"`. + +```sh +MSYS_NO_PATHCONV=1 docker run --rm --add-host host.docker.internal:host-gateway mongo:7 \ + mongosh "mongodb://host.docker.internal:27023/vantage_idx" --quiet --eval \ + 'const u=db.users.findOne({email:"shared@example.com"}); print(u.user_id); print(u.password_hash)' +``` +Expected: a UUID and a bcrypt hash. Keep both. + +- [ ] **Step 7: Create instance B with the SAME address — the case that was impossible before** + +```sh +MSYS_NO_PATHCONV=1 docker run --rm --add-host host.docker.internal:host-gateway mongo:7 \ + mongosh "mongodb://host.docker.internal:27023/vantage_idx" --quiet --eval ' + const a = db.users.findOne({email:"shared@example.com"}); + const bId = UUID().toString().replace(/[{}]/g,""); + db.instances.insertOne({instance_id:bId, name:"Beta", slug:"beta", created_at:new Date()}); + db.users.insertOne({ + user_id: UUID().toString().replace(/[{}]/g,""), + instance_id: bId, + email: "shared@example.com", + password_hash: a.password_hash, + role: "owner", + auth_source: "local", + created_at: new Date() + }); + print("beta instance " + bId); + print("users with that address: " + db.users.countDocuments({email:"shared@example.com"})); + ' +``` +Expected: `users with that address: 2`. Under the old global index this insert would have failed with E11000 — that failure is exactly what this phase removes. + +- [ ] **Step 8: Confirm the compound index still refuses a duplicate WITHIN one instance** + +```sh +MSYS_NO_PATHCONV=1 docker run --rm --add-host host.docker.internal:host-gateway mongo:7 \ + mongosh "mongodb://host.docker.internal:27023/vantage_idx" --quiet --eval ' + const a = db.users.findOne({email:"shared@example.com"}); + try { + db.users.insertOne({user_id:"dup", instance_id:a.instance_id, + email:"shared@example.com", role:"member", auth_source:"local", created_at:new Date()}); + print("FAIL: duplicate accepted"); + } catch (e) { print("refused as expected: " + (e.code === 11000)); } + ' +``` +Expected: `refused as expected: true`. A `FAIL` line means the compound index is missing or not unique. + +- [ ] **Step 9: Confirm each host signs in to its own instance — the whole point of the phase** + +```sh +curl -s -X POST http://localhost:8091/auth/login -H 'Host: alpha.vantage.test' \ + -H 'Content-Type: application/json' -c /tmp/alpha.jar \ + -d '{"email":"shared@example.com","password":"hunter2hunter2"}' +curl -s http://localhost:8091/auth/me -H 'Host: alpha.vantage.test' -b /tmp/alpha.jar +``` +Expected: `{"ok":true}`, then a body whose `instance` is **Alpha**. + +```sh +curl -s -X POST http://localhost:8091/auth/login -H 'Host: beta.vantage.test' \ + -H 'Content-Type: application/json' -c /tmp/beta.jar \ + -d '{"email":"shared@example.com","password":"hunter2hunter2"}' +curl -s http://localhost:8091/auth/me -H 'Host: beta.vantage.test' -b /tmp/beta.jar +``` +Expected: `{"ok":true}`, then a body whose `instance` is **Beta**, with a different `instance_id` from the Alpha response. + +Two sign-ins, one address, one password, two different tenants. If both responses name the same instance, the lookup is not scoped. + +- [ ] **Step 10: Confirm the apex host refuses rather than guesses** + +```sh +curl -s -o /dev/null -w '%{http_code}\n' -X POST http://localhost:8091/auth/login \ + -H 'Host: vantage.test' -H 'Content-Type: application/json' \ + -d '{"email":"shared@example.com","password":"hunter2hunter2"}' +``` +Expected: `400`. Then read the message: + +```sh +curl -s -X POST http://localhost:8091/auth/login -H 'Host: vantage.test' \ + -H 'Content-Type: application/json' \ + -d '{"email":"shared@example.com","password":"hunter2hunter2"}' +``` +Expected: an error naming both the instance count and the host. A `200` here would mean an arbitrary tenant was chosen. + +- [ ] **Step 11: Confirm a wrong password still fails, on the right host** + +```sh +curl -s -o /dev/null -w '%{http_code}\n' -X POST http://localhost:8091/auth/login \ + -H 'Host: alpha.vantage.test' -H 'Content-Type: application/json' \ + -d '{"email":"shared@example.com","password":"wrongwrongwrong"}' +``` +Expected: `401`. + +- [ ] **Step 12: Confirm a single-instance deployment still signs in on a bare host** + +```sh +MSYS_NO_PATHCONV=1 docker run --rm --add-host host.docker.internal:host-gateway mongo:7 \ + mongosh "mongodb://host.docker.internal:27023/vantage_idx" --quiet --eval \ + 'const b=db.instances.findOne({slug:"beta"}); db.users.deleteMany({instance_id:b.instance_id}); db.instances.deleteOne({slug:"beta"}); print(db.instances.countDocuments({}))' +``` +Expected: `1`. + +```sh +curl -s -X POST http://localhost:8091/auth/login -H 'Host: vantage.test' \ + -H 'Content-Type: application/json' \ + -d '{"email":"shared@example.com","password":"hunter2hunter2"}' +``` +Expected: `{"ok":true}`. This is the self-hosted path, and it must keep working. + +- [ ] **Step 13: Confirm admin boots and its login route still works** + +```sh +MSYS_NO_PATHCONV=1 docker run -d --name vantage-idx-admin -p 8093:8083 \ + -e ADMIN_MONGO_URI=mongodb://host.docker.internal:27023/vantage_idx_admin \ + -e CONTROL_MONGO_URI=mongodb://host.docker.internal:27023/vantage_idx \ + -e REDIS_ADDR=host.docker.internal:6389 \ + -e LICENSE_SIGNING_KEY="$LICENSE_SIGNING_KEY" \ + -e PUBLIC_URL=http://localhost:8093 -e ADMIN_ORIGIN=http://localhost:3004 \ + --add-host host.docker.internal:host-gateway vantage-admin:test + +sleep 5 +curl -s http://localhost:8093/healthz +``` +Expected: `{"ok":true}`. A boot failure here most likely means an unused-import error that `go build` caught but the image build did not, or a missing env var. + +```sh +curl -s -o /dev/null -w '%{http_code}\n' -X POST http://localhost:8093/auth/login \ + -H 'Content-Type: application/json' \ + -d '{"email":"nobody@example.com","password":"hunter2hunter2"}' +``` +Expected: `401`, not `500`. This proves `/auth/login` is wired to a live handler after `HandleCloudLogin` was deleted. + +- [ ] **Step 14: Tear the scratch environment down** + +```sh +MSYS_NO_PATHCONV=1 docker rm -f vantage-idx-server vantage-idx-admin vantage-idx-mongo vantage-idx-redis +``` + +- [ ] **Step 15: Commit** + +```bash +git add CLAUDE.md +git commit -m "docs: users is unique per instance, not globally + +The old index was load-bearing because two lookups were unscoped. Both +are scoped now and the unscoped helper is gone, so the property that +matters is the absence of any unscoped lookup by email. Says so, and +documents auth_source hq. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +## Done when + +- `instance_email_unique` exists on `users`, `email_1` does not, and a second boot is a no-op. +- Two users share one address across two instances, and each signs in to their own. +- A duplicate address within one instance is still refused. +- The apex host refuses to guess when several instances exist, and still works when only one does. +- `grep -rn "GetUserByEmail"` and `grep -rn "HandleCloudLogin"` both return nothing. +- `admin` boots and `/auth/login` answers `401` rather than `500`. +- `CLAUDE.md` no longer claims `users.email` is globally unique. + +**Not proven by this plan:** the OIDC sign-in path, which needs a real identity provider. Verify it manually on the first SSO sign-in after deployment — an existing SSO user must still land in their own instance. + +## Not in this phase + +`POST /api/instances`, the Free lifecycle, renewal, the notices, the reaper, the sitesvc cutover, account roles, invitations, instance membership, password propagation, and every UI change. Phases 2 and 3 get their own plans once this one lands. diff --git a/docs/superpowers/specs/2026-07-26-cloud-instance-creation-design.md b/docs/superpowers/specs/2026-07-26-cloud-instance-creation-design.md index 5440dc1..163ec41 100644 --- a/docs/superpowers/specs/2026-07-26-cloud-instance-creation-design.md +++ b/docs/superpowers/specs/2026-07-26-cloud-instance-creation-design.md @@ -60,8 +60,9 @@ empty `PaddleCustomerID`, which spec 5's account model already permits. Three phases, each shippable, in this order. The plan should not interleave them — phase 1 changes an index that everything else then depends on. -1. **Identity** — drop the global email index, scope the lookups, project the - creator as owner. No new UI. +1. **Identity** — drop the global email index, scope the two unscoped lookups, + add the `hq` fields to the user document, and remove admin's unscoped + control-plane login branch. No new UI, and nothing is projected yet. 2. **Instance creation and Free lifecycle** — `POST /api/instances`, renewal, notices, the reaper, the sitesvc cutover. 3. **Membership** — account roles, invitations, per-instance grants, password