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 <noreply@anthropic.com>
This commit is contained in:
@@ -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 <module-dir> <command...>
|
||||
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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 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 `<slug>.vantage.<tld>` — `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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 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.
|
||||
Reference in New Issue
Block a user