From da3afca7fab05b7be4b0efabb262a2258e77c79d Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Sun, 26 Jul 2026 12:48:27 +0100 Subject: [PATCH] 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 --- shared/indexes/indexes.go | 76 ++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 8 deletions(-) diff --git a/shared/indexes/indexes.go b/shared/indexes/indexes.go index ef11974..cadd84b 100644 --- a/shared/indexes/indexes.go +++ b/shared/indexes/indexes.go @@ -4,6 +4,7 @@ package indexes import ( "context" + "errors" "fmt" "go.mongodb.org/mongo-driver/v2/bson" @@ -11,21 +12,44 @@ import ( "go.mongodb.org/mongo-driver/v2/mongo/options" ) -// EnsureCoreIndexes declares the unique indexes on users.email and orgs.slug. +// 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. // -// These are a security property, not an optimisation. GetUserByEmail does an -// unscoped FindOne, so a duplicate email would let the OIDC cross-org guard -// compare against an arbitrary user. Every caller must treat a failure here as -// fatal. +// 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: "email", Value: 1}}, - Options: options.Index().SetUnique(true), + 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.email index: %w", err) + 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{ @@ -37,3 +61,39 @@ func EnsureCoreIndexes(ctx context.Context, db *mongo.Database) error { 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 +}