From 180f00b4ca123daca78c08a3da30bb5046cacd5b Mon Sep 17 00:00:00 2001 From: Kilian Boute Date: Tue, 18 Aug 2026 14:03:39 +0000 Subject: [PATCH 1/2] feat(tenant): share device ownership resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every pipeline stamps the organisation and project that own a source device onto the resources it derives from that device's media, and Hub API later reads them back with a single selector. Monitor had the only implementation; the remaining pipelines were about to copy it. Copying is the wrong shape for this. Two services that resolve the same device differently do not fail loudly: they write resources that are silently invisible to the tenant, or visible to the wrong one. Independently deployed services can only agree if there is exactly one implementation, so it moves here before the duplication spreads. The two halves of the resolution are deliberately asymmetric. The organisation is verified against persisted records, because it is the tenant boundary, a wrong value crosses tenants, and a populated collection exists to check against; ambiguous, orphaned, and conflicting ownership all fail closed rather than guess. The project is assigned from models/pkg/projectscope, because during the hidden rollout no authoritative project record exists and any query would let two services disagree. An empty organisation collection means organisations-bootstrap has not run on that instance rather than that the device is unowned, so a primary organisation is derived from the master user's id — the value the migration will later persist. Erroring there would drop every message on an un-migrated deployment. This lands beside pkg/database rather than inside it: resolution needs models.Device, and pkg/database stays a dependency-light leaf with no models import. Collection names are configuration because services do not all spell them the same way. Co-Authored-By: Claude Opus 5 --- go.mod | 11 +- go.sum | 14 ++ pkg/tenancy/device.go | 305 +++++++++++++++++++++++++++++++++++++ pkg/tenancy/device_test.go | 297 ++++++++++++++++++++++++++++++++++++ 4 files changed, 622 insertions(+), 5 deletions(-) create mode 100644 pkg/tenancy/device.go create mode 100644 pkg/tenancy/device_test.go diff --git a/go.mod b/go.mod index aa6c449..dc2ac8b 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.25.8 require ( github.com/go-playground/validator/v10 v10.30.2 - github.com/uug-ai/models v1.4.43 + github.com/uug-ai/models v1.7.14 go.mongodb.org/mongo-driver v1.17.9 go.opentelemetry.io/contrib/instrumentation/go.mongodb.org/mongo-driver/mongo/otelmongo v0.68.0 ) @@ -22,6 +22,7 @@ require ( github.com/leodido/go-urn v1.4.0 // indirect github.com/montanaflynn/stats v0.9.0 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect + github.com/sirupsen/logrus v1.9.4 // indirect github.com/xdg-go/pbkdf2 v1.0.0 // indirect github.com/xdg-go/scram v1.2.0 // indirect github.com/xdg-go/stringprep v1.0.4 // indirect @@ -30,8 +31,8 @@ require ( go.opentelemetry.io/otel v1.43.0 // indirect go.opentelemetry.io/otel/metric v1.43.0 // indirect go.opentelemetry.io/otel/trace v1.43.0 // indirect - golang.org/x/crypto v0.49.0 // indirect - golang.org/x/sync v0.20.0 // indirect - golang.org/x/sys v0.42.0 // indirect - golang.org/x/text v0.35.0 // indirect + golang.org/x/crypto v0.51.0 // indirect + golang.org/x/sync v0.21.0 // indirect + golang.org/x/sys v0.44.0 // indirect + golang.org/x/text v0.38.0 // indirect ) diff --git a/go.sum b/go.sum index eaf3ddf..7a0e313 100644 --- a/go.sum +++ b/go.sum @@ -29,10 +29,16 @@ github.com/montanaflynn/stats v0.9.0 h1:tsBJ0RXwph9BmAuFoCmqGv6e8xa0MENQ8m0ptKq2 github.com/montanaflynn/stats v0.9.0/go.mod h1:etXPPgVO6n31NxCd9KQUMvCM+ve0ruNzt6R8Bnaayow= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/sirupsen/logrus v1.9.4 h1:TsZE7l11zFCLZnZ+teH4Umoq5BhEIfIzfRDZ1Uzql2w= +github.com/sirupsen/logrus v1.9.4/go.mod h1:ftWc9WdOfJ0a92nsE2jF5u5ZwH8Bv2zdeOC42RjbV2g= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/uug-ai/models v1.4.43 h1:lqQbLyj4IblFWK6zgaLM3/yyqEL6ovN4hcH6bwGgctY= github.com/uug-ai/models v1.4.43/go.mod h1:yb67RVwPaNQmo8bLTyOTtze03h6eB01MLtUKTBzqZ2U= +github.com/uug-ai/models v1.7.13 h1:5NxDKkn4YCmxenoo3i+5STqCSorS+ooJSDfrj8HMK9A= +github.com/uug-ai/models v1.7.13/go.mod h1:Ts/6PtFhsFOkZQRuC/tntFBG8Fl1BGICC6dmubXULbk= +github.com/uug-ai/models v1.7.14 h1:Ho87KFu+aOpSmtZ9OoPnLNBGgDEMQCO1K0OIKFgdcuQ= +github.com/uug-ai/models v1.7.14/go.mod h1:Ts/6PtFhsFOkZQRuC/tntFBG8Fl1BGICC6dmubXULbk= github.com/xdg-go/pbkdf2 v1.0.0 h1:Su7DPu48wXMwC3bs7MCNG+z4FhcyEuz5dlvchbq0B0c= github.com/xdg-go/pbkdf2 v1.0.0/go.mod h1:jrpuAogTd400dnrH08LKmI/xc1MbPOebTwRqcT5RDeI= github.com/xdg-go/scram v1.2.0 h1:bYKF2AEwG5rqd1BumT4gAnvwU/M9nBp2pTSxeZw7Wvs= @@ -58,6 +64,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.49.0 h1:+Ng2ULVvLHnJ/ZFEq4KdcDd/cfjrrjjNSXNzxg0Y4U4= golang.org/x/crypto v0.49.0/go.mod h1:ErX4dUh2UM+CFYiXZRTcMpEcN8b/1gxEuv3nODoYtCA= +golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= +golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= @@ -66,6 +74,8 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= +golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -73,6 +83,8 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= +golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -81,6 +93,8 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= +golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE= +golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/pkg/tenancy/device.go b/pkg/tenancy/device.go new file mode 100644 index 0000000..87081b8 --- /dev/null +++ b/pkg/tenancy/device.go @@ -0,0 +1,305 @@ +// Package tenancy resolves the immutable organisation and project that own a +// source device, and therefore own every resource a pipeline derives from that +// device's media. +// +// This lives in a shared module on purpose. Ownership resolution is not a +// per-service concern that each pipeline may answer its own way: the answer is +// stamped onto media, markers, counts, and notifications that a single Hub API +// reader later scopes with one selector. Two services that resolve the same +// device differently do not produce a visible error — they produce resources +// that are silently invisible to the tenant, or visible to the wrong one. The +// only way to prevent that across independently deployed services is to have +// exactly one implementation. +// +// Two rules govern the resolution, and they are deliberately asymmetric: +// +// - The organisation is verified against persisted records. It is the tenant +// boundary, a wrong value crosses tenants, and a populated organisation +// collection exists to check against. Ambiguity fails closed. +// - The project is assigned from a definition, never looked up. During the +// hidden single-project rollout no authoritative project record exists, so +// any query would let two services disagree. See models/pkg/projectscope. +package tenancy + +import ( + "context" + "errors" + "fmt" + + "github.com/uug-ai/database/pkg/database" + "github.com/uug-ai/models/pkg/models" + "github.com/uug-ai/models/pkg/projectscope" + "github.com/uug-ai/models/pkg/properties" + "go.mongodb.org/mongo-driver/bson/primitive" + "go.mongodb.org/mongo-driver/mongo" + "go.mongodb.org/mongo-driver/mongo/options" +) + +// Collections names the database and collections the resolver reads. Services +// historically declared these constants themselves and do not all spell them +// the same way, so they are configuration rather than package constants. +type Collections struct { + Database string + Devices string + Users string + Organisations string +} + +// DefaultCollections is the Kerberos Hub layout every current pipeline service +// uses. Services with a different layout override the fields they need. +func DefaultCollections() Collections { + return Collections{ + Database: "Kerberos", + Devices: "devices", + Users: "users", + Organisations: "organisation", + } +} + +func (c Collections) withDefaults() Collections { + defaults := DefaultCollections() + if c.Database == "" { + c.Database = defaults.Database + } + if c.Devices == "" { + c.Devices = defaults.Devices + } + if c.Users == "" { + c.Users = defaults.Users + } + if c.Organisations == "" { + c.Organisations = defaults.Organisations + } + return c +} + +// Ownership is the resolved tenant placement of a device. Both fields are +// always set on success: ProjectId is never the zero value, because an +// unassigned device falls back to the organisation default rather than to +// "no project". +type Ownership struct { + OrganisationId primitive.ObjectID + ProjectId primitive.ObjectID +} + +// DeviceResolver resolves device ownership against one database. +type DeviceResolver struct { + db *database.Database + collections Collections +} + +// NewDeviceResolver binds a resolver to a database. Zero-valued Collections +// fields fall back to DefaultCollections. +func NewDeviceResolver(db *database.Database, collections Collections) *DeviceResolver { + return &DeviceResolver{db: db, collections: collections.withDefaults()} +} + +type persistedOrganisationOwnership struct { + Id primitive.ObjectID `bson:"_id"` + OwnerId primitive.ObjectID `bson:"ownerId"` + LegacyOwnerId string `bson:"owner_id"` +} + +type persistedUserOwnership struct { + Id primitive.ObjectID `bson:"_id"` + MasterAccount string `bson:"user_id"` +} + +// DeviceProjection lists the fields ResolveDevice needs. Callers that load +// devices themselves must include all of them: omitting the project field +// yields a nil value that is indistinguishable from "unassigned", so the +// resolver would silently default a device that actually has a project. +func DeviceProjection() *database.Projection { + return database.NewProjection().Include( + properties.DeviceId, + properties.DeviceKey, + properties.DeviceOrganisationId, + properties.DeviceProjectId, + properties.DeviceUserId, + ) +} + +// LoadDevice reads the ownership fields of a source device by its key. +func (r *DeviceResolver) LoadDevice(ctx context.Context, deviceKey string) (models.Device, error) { + if r == nil || r.db == nil || r.db.Client == nil { + return models.Device{}, fmt.Errorf("source device %q lookup requires a database", deviceKey) + } + + databaseCtx, cancel := context.WithTimeout(ctx, r.db.Client.GetTimeout()) + defer cancel() + + var device models.Device + err := r.db.Client.FindOne( + databaseCtx, + r.collections.Database, + r.collections.Devices, + map[string]string{properties.DeviceKey: deviceKey}, + DeviceProjection(), + ).Into(&device) + if err != nil { + return models.Device{}, fmt.Errorf("find source device %q: %w", deviceKey, err) + } + if device.Id.IsZero() { + return models.Device{}, fmt.Errorf("source device %q has no persisted identity", deviceKey) + } + return device, nil +} + +// ResolveDeviceByKey loads a device and resolves its ownership in one step. +func (r *DeviceResolver) ResolveDeviceByKey(ctx context.Context, deviceKey string) (models.Device, Ownership, error) { + device, err := r.LoadDevice(ctx, deviceKey) + if err != nil { + return models.Device{}, Ownership{}, err + } + ownership, err := r.ResolveDevice(ctx, device) + if err != nil { + return models.Device{}, Ownership{}, err + } + return device, ownership, nil +} + +// ResolveDevice returns the organisation and project owning a device. +func (r *DeviceResolver) ResolveDevice(ctx context.Context, device models.Device) (Ownership, error) { + organisationId, err := r.resolveOrganisation(ctx, device) + if err != nil { + return Ownership{}, err + } + return Ownership{ + OrganisationId: organisationId, + ProjectId: projectscope.ResolveProjectId(organisationId, device.ProjectId), + }, nil +} + +func (r *DeviceResolver) resolveOrganisation(ctx context.Context, device models.Device) (primitive.ObjectID, error) { + if device.OrganisationId != "" { + organisationId, err := primitive.ObjectIDFromHex(device.OrganisationId) + if err != nil { + return primitive.NilObjectID, fmt.Errorf("source device %q has invalid canonical organisation ownership", device.Key) + } + return organisationId, nil + } + + if device.UserId == "" { + return primitive.NilObjectID, fmt.Errorf("source device %q has no canonical or legacy organisation ownership", device.Key) + } + legacyId, err := primitive.ObjectIDFromHex(device.UserId) + if err != nil { + return primitive.NilObjectID, fmt.Errorf("source device %q has no valid organisation ownership", device.Key) + } + if r == nil || r.db == nil || r.db.Client == nil { + return primitive.NilObjectID, fmt.Errorf("source device %q legacy ownership requires database resolution", device.Key) + } + + // Newer compatibility writers mirror the organisation id into user_id. An + // exact organisation identity is therefore the strongest legacy candidate. + if _, found, err := r.findOrganisationById(ctx, legacyId); err != nil { + return primitive.NilObjectID, err + } else if found { + return legacyId, nil + } + + // Older heartbeat documents may contain the concrete actor in user_id. Use + // the stable master relationship, never the actor's mutable active selection. + ownerId := legacyId + user, userExists, err := r.findUserOwnership(ctx, legacyId) + if err != nil { + return primitive.NilObjectID, err + } + if userExists && user.MasterAccount != "" { + ownerId, err = primitive.ObjectIDFromHex(user.MasterAccount) + if err != nil { + return primitive.NilObjectID, fmt.Errorf("source device %q legacy user has invalid master ownership", device.Key) + } + if _, found, err := r.findOrganisationById(ctx, ownerId); err != nil { + return primitive.NilObjectID, err + } else if found { + return ownerId, nil + } + } + + organisations, err := r.findOrganisationsByOwner(ctx, ownerId) + if err != nil { + return primitive.NilObjectID, err + } + if len(organisations) == 1 { + return organisations[0].Id, nil + } + if len(organisations) > 1 { + return primitive.NilObjectID, fmt.Errorf("source device %q legacy owner resolves to %d organisations", device.Key, len(organisations)) + } + + // No organisation record exists at all, so organisations-bootstrap has not + // run on this instance. A primary organisation deterministically reuses the + // master user's id, so deriving it here produces exactly the value the + // migration will later persist. Failing instead would drop every message on + // an un-migrated deployment. An owner with no user record is orphaned and + // must not be invented. + if !userExists { + return primitive.NilObjectID, fmt.Errorf("source device %q legacy owner %s has no user or organisation record", device.Key, ownerId.Hex()) + } + return ownerId, nil +} + +func (r *DeviceResolver) findOrganisationById(ctx context.Context, id primitive.ObjectID) (persistedOrganisationOwnership, bool, error) { + databaseCtx, cancel := context.WithTimeout(ctx, r.db.Client.GetTimeout()) + defer cancel() + + var organisation persistedOrganisationOwnership + err := r.db.Client.FindOne(databaseCtx, r.collections.Database, r.collections.Organisations, map[string]any{ + properties.OrganisationId: id, + }).Into(&organisation) + if errors.Is(err, mongo.ErrNoDocuments) { + return persistedOrganisationOwnership{}, false, nil + } + if err != nil { + return persistedOrganisationOwnership{}, false, fmt.Errorf("find organisation %s: %w", id.Hex(), err) + } + if organisation.Id.IsZero() { + return persistedOrganisationOwnership{}, false, fmt.Errorf("organisation %s has no persisted identity", id.Hex()) + } + return organisation, true, nil +} + +func (r *DeviceResolver) findUserOwnership(ctx context.Context, id primitive.ObjectID) (persistedUserOwnership, bool, error) { + databaseCtx, cancel := context.WithTimeout(ctx, r.db.Client.GetTimeout()) + defer cancel() + + projection := database.NewProjection().Include(properties.UserId, properties.UserMasterAccount) + var user persistedUserOwnership + err := r.db.Client.FindOne(databaseCtx, r.collections.Database, r.collections.Users, map[string]any{ + properties.UserId: id, + }, projection).Into(&user) + if errors.Is(err, mongo.ErrNoDocuments) { + return persistedUserOwnership{}, false, nil + } + if err != nil { + return persistedUserOwnership{}, false, fmt.Errorf("find legacy device user %s: %w", id.Hex(), err) + } + return user, true, nil +} + +func (r *DeviceResolver) findOrganisationsByOwner(ctx context.Context, ownerId primitive.ObjectID) ([]persistedOrganisationOwnership, error) { + databaseCtx, cancel := context.WithTimeout(ctx, r.db.Client.GetTimeout()) + defer cancel() + + filter := map[string]any{"$or": []any{ + map[string]any{properties.OrganisationOwnerId: ownerId}, + map[string]any{"owner_id": ownerId.Hex()}, + }} + var organisations []persistedOrganisationOwnership + if err := r.db.Client.Find(databaseCtx, r.collections.Database, r.collections.Organisations, filter, options.Find().SetLimit(2)).All(&organisations); err != nil { + return nil, fmt.Errorf("find organisations owned by %s: %w", ownerId.Hex(), err) + } + for _, organisation := range organisations { + if organisation.Id.IsZero() { + return nil, fmt.Errorf("organisation owned by %s has no persisted identity", ownerId.Hex()) + } + if !organisation.OwnerId.IsZero() && organisation.OwnerId != ownerId { + return nil, fmt.Errorf("organisation %s has conflicting canonical owner %s and legacy owner %s", organisation.Id.Hex(), organisation.OwnerId.Hex(), ownerId.Hex()) + } + if organisation.LegacyOwnerId != "" && organisation.LegacyOwnerId != ownerId.Hex() { + return nil, fmt.Errorf("organisation %s has conflicting legacy owner %s and resolved owner %s", organisation.Id.Hex(), organisation.LegacyOwnerId, ownerId.Hex()) + } + } + return organisations, nil +} diff --git a/pkg/tenancy/device_test.go b/pkg/tenancy/device_test.go new file mode 100644 index 0000000..a358040 --- /dev/null +++ b/pkg/tenancy/device_test.go @@ -0,0 +1,297 @@ +package tenancy + +import ( + "context" + "testing" + + "github.com/uug-ai/database/pkg/database" + "github.com/uug-ai/models/pkg/models" + "go.mongodb.org/mongo-driver/bson/primitive" + "go.mongodb.org/mongo-driver/mongo" +) + +func testResolver(collections Collections) (*DeviceResolver, *database.MockDatabase) { + mock := database.NewMockDatabase() + return NewDeviceResolver(&database.Database{Client: mock}, collections), mock +} + +func assertOwnership(t *testing.T, resolver *DeviceResolver, device models.Device, wantOrganisationId, wantProjectId primitive.ObjectID) { + t.Helper() + + ownership, err := resolver.ResolveDevice(context.Background(), device) + if err != nil { + t.Fatalf("ResolveDevice() error = %v", err) + } + if ownership.OrganisationId != wantOrganisationId { + t.Fatalf("organisationId = %s, want %s", ownership.OrganisationId.Hex(), wantOrganisationId.Hex()) + } + if ownership.ProjectId != wantProjectId { + t.Fatalf("projectId = %s, want %s", ownership.ProjectId.Hex(), wantProjectId.Hex()) + } +} + +func assertFailsClosed(t *testing.T, resolver *DeviceResolver, device models.Device, why string) { + t.Helper() + + if _, err := resolver.ResolveDevice(context.Background(), device); err == nil { + t.Fatalf("%s must fail closed", why) + } +} + +func TestDefaultCollectionsFillMissingFields(t *testing.T) { + // A service that overrides only its database name must still read the + // standard collections rather than empty strings. + collections := Collections{Database: "Custom"}.withDefaults() + want := DefaultCollections() + want.Database = "Custom" + if collections != want { + t.Fatalf("withDefaults() = %+v, want %+v", collections, want) + } +} + +func TestResolveDeviceUsesCanonicalSnapshotWithoutLegacyQueries(t *testing.T) { + resolver, mock := testResolver(Collections{}) + organisationId := primitive.NewObjectID() + projectId := primitive.NewObjectID() + + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + OrganisationId: organisationId.Hex(), + ProjectId: &projectId, + UserId: primitive.NewObjectID().Hex(), + }, organisationId, projectId) + + if len(mock.FindOneCalls) != 0 || len(mock.FindCalls) != 0 { + t.Fatal("canonical device ownership must not consult mutable legacy records") + } +} + +func TestResolveDeviceDefaultsProjectToOrganisationWithoutQueries(t *testing.T) { + resolver, mock := testResolver(Collections{}) + organisationId := primitive.NewObjectID() + + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + OrganisationId: organisationId.Hex(), + }, organisationId, organisationId) + + // The hidden default is a definition, not a lookup. Consulting a project + // record here would let two services disagree for the same organisation. + if len(mock.FindOneCalls) != 0 || len(mock.FindCalls) != 0 { + t.Fatal("default project resolution must not query persisted records") + } +} + +func TestResolveDeviceIgnoresZeroStoredProject(t *testing.T) { + resolver, _ := testResolver(Collections{}) + organisationId := primitive.NewObjectID() + zero := primitive.NilObjectID + + // A stored zero means "unassigned", not "assigned to nothing"; stamping it + // would hide every derived resource from project-scoped reads. + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + OrganisationId: organisationId.Hex(), + ProjectId: &zero, + }, organisationId, organisationId) +} + +func TestResolveDeviceTreatsLegacyOrganisationIdAsCanonicalCandidate(t *testing.T) { + resolver, mock := testResolver(Collections{}) + organisationId := primitive.NewObjectID() + mock.QueueFindOne(map[string]any{"_id": organisationId}, nil) + + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + UserId: organisationId.Hex(), + }, organisationId, organisationId) +} + +func TestResolveDeviceResolvesLegacySubUserThroughMaster(t *testing.T) { + resolver, mock := testResolver(Collections{}) + userId := primitive.NewObjectID() + masterId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": userId, "user_id": masterId.Hex()}, nil) + mock.QueueFindOne(map[string]any{"_id": masterId, "ownerId": masterId}, nil) + + // A sub-user's own id is never a tenant identity; ownership follows the + // stable master relationship, not the actor's mutable active selection. + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + UserId: userId.Hex(), + }, masterId, masterId) +} + +func TestResolveDeviceResolvesLegacyOwnerToSingleSecondaryOrganisation(t *testing.T) { + resolver, mock := testResolver(Collections{}) + ownerId := primitive.NewObjectID() + organisationId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": ownerId}, nil) + mock.QueueFind([]persistedOrganisationOwnership{{Id: organisationId, OwnerId: ownerId}}, nil) + + // A secondary organisation has an independent id, so the project default + // follows the organisation rather than the owning user. + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + UserId: ownerId.Hex(), + }, organisationId, organisationId) +} + +func TestResolveDeviceDerivesPrimaryOrganisationBeforeBootstrap(t *testing.T) { + // An instance that has never run organisations-bootstrap has an empty + // organisation collection. A primary organisation reuses the master user's + // id, so deriving it produces the value the migration will later persist. + // Failing here would drop every message on an un-migrated deployment. + resolver, mock := testResolver(Collections{}) + masterId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": masterId}, nil) + mock.QueueFind([]persistedOrganisationOwnership{}, nil) + + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + UserId: masterId.Hex(), + }, masterId, masterId) +} + +func TestResolveDeviceDerivesSubUserMasterOrganisationBeforeBootstrap(t *testing.T) { + resolver, mock := testResolver(Collections{}) + userId := primitive.NewObjectID() + masterId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": userId, "user_id": masterId.Hex()}, nil) + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFind([]persistedOrganisationOwnership{}, nil) + + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + UserId: userId.Hex(), + }, masterId, masterId) +} + +func TestResolveDeviceFailsOnAmbiguousLegacyOwner(t *testing.T) { + resolver, mock := testResolver(Collections{}) + ownerId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": ownerId}, nil) + mock.QueueFind([]persistedOrganisationOwnership{ + {Id: primitive.NewObjectID(), OwnerId: ownerId}, + {Id: primitive.NewObjectID(), OwnerId: ownerId}, + }, nil) + + assertFailsClosed(t, resolver, models.Device{Key: "device-1", UserId: ownerId.Hex()}, "ambiguous legacy ownership") +} + +func TestResolveDeviceRejectsOrphanedLegacyOwner(t *testing.T) { + resolver, mock := testResolver(Collections{}) + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFind([]persistedOrganisationOwnership{}, nil) + + assertFailsClosed(t, resolver, models.Device{ + Key: "device-1", + UserId: primitive.NewObjectID().Hex(), + }, "an owner with neither a user nor an organisation record") +} + +func TestResolveDeviceRejectsInvalidMasterRelationship(t *testing.T) { + resolver, mock := testResolver(Collections{}) + userId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": userId, "user_id": "invalid"}, nil) + + assertFailsClosed(t, resolver, models.Device{Key: "device-1", UserId: userId.Hex()}, "invalid master ownership") +} + +func TestResolveDeviceRejectsConflictingOwnerFields(t *testing.T) { + resolver, mock := testResolver(Collections{}) + ownerId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": ownerId}, nil) + mock.QueueFind([]persistedOrganisationOwnership{{ + Id: primitive.NewObjectID(), + OwnerId: primitive.NewObjectID(), + LegacyOwnerId: ownerId.Hex(), + }}, nil) + + assertFailsClosed(t, resolver, models.Device{Key: "device-1", UserId: ownerId.Hex()}, "conflicting canonical and legacy organisation owners") +} + +func TestResolveDeviceRejectsMissingOrInvalidOwnership(t *testing.T) { + resolver, _ := testResolver(Collections{}) + + for _, device := range []models.Device{ + {Key: "missing"}, + {Key: "invalid-organisation", OrganisationId: "invalid", UserId: primitive.NewObjectID().Hex()}, + {Key: "invalid-user", UserId: "invalid"}, + } { + assertFailsClosed(t, resolver, device, "device "+device.Key) + } +} + +func TestResolveDeviceRequiresDatabaseForLegacyOwnership(t *testing.T) { + resolver := NewDeviceResolver(nil, Collections{}) + + assertFailsClosed(t, resolver, models.Device{ + Key: "device-1", + UserId: primitive.NewObjectID().Hex(), + }, "legacy ownership without a database") +} + +func TestLoadDeviceReadsConfiguredCollection(t *testing.T) { + resolver, mock := testResolver(Collections{Database: "Custom", Devices: "sources"}) + deviceId := primitive.NewObjectID() + organisationId := primitive.NewObjectID() + mock.QueueFindOne(map[string]any{ + "_id": deviceId, + "key": "device-1", + "organisationId": organisationId.Hex(), + }, nil) + + device, err := resolver.LoadDevice(context.Background(), "device-1") + if err != nil { + t.Fatalf("LoadDevice() error = %v", err) + } + if device.Id != deviceId { + t.Fatalf("device id = %s, want %s", device.Id.Hex(), deviceId.Hex()) + } + if len(mock.FindOneCalls) != 1 { + t.Fatalf("FindOne calls = %d, want 1", len(mock.FindOneCalls)) + } + if mock.FindOneCalls[0].Db != "Custom" || mock.FindOneCalls[0].Collection != "sources" { + t.Fatalf("read %s/%s, want Custom/sources", mock.FindOneCalls[0].Db, mock.FindOneCalls[0].Collection) + } +} + +func TestLoadDeviceRejectsDocumentWithoutIdentity(t *testing.T) { + resolver, mock := testResolver(Collections{}) + mock.QueueFindOne(map[string]any{"key": "device-1"}, nil) + + if _, err := resolver.LoadDevice(context.Background(), "device-1"); err == nil { + t.Fatal("a device document with no persisted identity must fail closed") + } +} + +func TestResolveDeviceByKeyReturnsDeviceAndOwnership(t *testing.T) { + resolver, mock := testResolver(Collections{}) + deviceId := primitive.NewObjectID() + organisationId := primitive.NewObjectID() + mock.QueueFindOne(map[string]any{ + "_id": deviceId, + "key": "device-1", + "organisationId": organisationId.Hex(), + }, nil) + + device, ownership, err := resolver.ResolveDeviceByKey(context.Background(), "device-1") + if err != nil { + t.Fatalf("ResolveDeviceByKey() error = %v", err) + } + if device.Id != deviceId { + t.Fatalf("device id = %s, want %s", device.Id.Hex(), deviceId.Hex()) + } + if ownership.OrganisationId != organisationId || ownership.ProjectId != organisationId { + t.Fatalf("ownership = %+v, want organisation and project %s", ownership, organisationId.Hex()) + } +} From d24d73e19f3d7ebd8dca9f139ffecbe0be6bac41 Mon Sep 17 00:00:00 2001 From: Kilian Boute Date: Tue, 18 Aug 2026 14:11:38 +0000 Subject: [PATCH 2/2] fix(tenant): require a user record for the derived master organisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before bootstrap has run the organisation collection is empty, so ownership is derived rather than read: a primary organisation reuses the master user's id. That derivation is only meaningful because bootstrap mints one organisation per master user. When ownership followed a sub-user's user_id, the existence check proved only that the sub-user was real. A dangling master link therefore produced an organisation id for an account that no longer exists, and bootstrap will never create it. The media would be stamped with a tenant no reader selects and disappear — the silent failure this resolver exists to prevent, arrived at by a different route. Confirm the owner has a user record before deriving from it. The lookup runs only when a master link was followed and only on the pre-bootstrap path, so the common case pays nothing: when the owner is the queried user itself its existence is already proven, and when an organisation record exists it is returned before this point. Co-Authored-By: Claude Opus 5 --- pkg/tenancy/device.go | 19 +++++++++++++++--- pkg/tenancy/device_test.go | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/pkg/tenancy/device.go b/pkg/tenancy/device.go index 87081b8..cb1137a 100644 --- a/pkg/tenancy/device.go +++ b/pkg/tenancy/device.go @@ -232,9 +232,22 @@ func (r *DeviceResolver) resolveOrganisation(ctx context.Context, device models. // run on this instance. A primary organisation deterministically reuses the // master user's id, so deriving it here produces exactly the value the // migration will later persist. Failing instead would drop every message on - // an un-migrated deployment. An owner with no user record is orphaned and - // must not be invented. - if !userExists { + // an un-migrated deployment. + // + // The derivation is only meaningful if the owner has a user record, because + // bootstrap mints one organisation per master user. An owner with no user + // record is orphaned: deriving an id for it would stamp media with a tenant + // the migration never creates, which no reader would ever select. + ownerExists := userExists + if ownerId != legacyId { + // Ownership followed a master link, so userExists only proves the + // sub-user is real. The derived id is the master's, and a dangling + // user_id must not be turned into an organisation. + if _, ownerExists, err = r.findUserOwnership(ctx, ownerId); err != nil { + return primitive.NilObjectID, err + } + } + if !ownerExists { return primitive.NilObjectID, fmt.Errorf("source device %q legacy owner %s has no user or organisation record", device.Key, ownerId.Hex()) } return ownerId, nil diff --git a/pkg/tenancy/device_test.go b/pkg/tenancy/device_test.go index a358040..660b7fc 100644 --- a/pkg/tenancy/device_test.go +++ b/pkg/tenancy/device_test.go @@ -163,6 +163,7 @@ func TestResolveDeviceDerivesSubUserMasterOrganisationBeforeBootstrap(t *testing mock.QueueFindOne(nil, mongo.ErrNoDocuments) mock.QueueFindOne(map[string]any{"_id": userId, "user_id": masterId.Hex()}, nil) mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": masterId}, nil) mock.QueueFind([]persistedOrganisationOwnership{}, nil) assertOwnership(t, resolver, models.Device{ @@ -171,6 +172,45 @@ func TestResolveDeviceDerivesSubUserMasterOrganisationBeforeBootstrap(t *testing }, masterId, masterId) } +func TestResolveDeviceRejectsDanglingMasterBeforeBootstrap(t *testing.T) { + // The sub-user is real but its user_id points at a master that no longer + // exists. Bootstrap mints one organisation per master user, so deriving an + // id here would stamp media with a tenant the migration never creates — + // invisible to every reader rather than merely wrong. + resolver, mock := testResolver(Collections{}) + userId := primitive.NewObjectID() + masterId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": userId, "user_id": masterId.Hex()}, nil) + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFind([]persistedOrganisationOwnership{}, nil) + + assertFailsClosed(t, resolver, models.Device{ + Key: "device-1", + UserId: userId.Hex(), + }, "a legacy sub-user whose master account no longer exists") +} + +func TestResolveDeviceDoesNotRecheckDirectOwnerBeforeBootstrap(t *testing.T) { + // The owner is the queried user itself, so its existence is already proven. + // A second lookup would be a wasted round trip on every un-migrated message. + resolver, mock := testResolver(Collections{}) + masterId := primitive.NewObjectID() + mock.QueueFindOne(nil, mongo.ErrNoDocuments) + mock.QueueFindOne(map[string]any{"_id": masterId}, nil) + mock.QueueFind([]persistedOrganisationOwnership{}, nil) + + assertOwnership(t, resolver, models.Device{ + Key: "device-1", + UserId: masterId.Hex(), + }, masterId, masterId) + + if len(mock.FindOneCalls) != 2 { + t.Fatalf("FindOne calls = %d, want 2", len(mock.FindOneCalls)) + } +} + func TestResolveDeviceFailsOnAmbiguousLegacyOwner(t *testing.T) { resolver, mock := testResolver(Collections{}) ownerId := primitive.NewObjectID()