diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef030e24..3c6e440c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,15 +1,15 @@ name: PHP CI -# Release work lands on a dev-v* branch first and only reaches main via the release +# Release work lands on a release/v* or dev-v* branch before reaching main via the release # PR, so a main-only filter leaves every PR targeting a release branch with no CI at # all — the release is then assembled from unverified commits. on: push: - branches: [ main, 'dev-v*' ] + branches: [ main, 'dev-v*', 'release/v*' ] tags: - 'v*' pull_request: - branches: [ main, 'dev-v*' ] + branches: [ main, 'dev-v*', 'release/v*' ] jobs: build: diff --git a/.github/workflows/postman.yml b/.github/workflows/postman.yml index 42f7dee9..8e6c301a 100644 --- a/.github/workflows/postman.yml +++ b/.github/workflows/postman.yml @@ -16,9 +16,9 @@ name: API Contract (Postman) on: push: - branches: [main, 'dev-v*'] + branches: [main, 'dev-v*', 'release/v*'] pull_request: - branches: [main, 'dev-v*'] + branches: [main, 'dev-v*', 'release/v*'] workflow_dispatch: permissions: contents: read diff --git a/src/Http/Controllers/Internal/v1/UserController.php b/src/Http/Controllers/Internal/v1/UserController.php index fe6a416e..90e88710 100644 --- a/src/Http/Controllers/Internal/v1/UserController.php +++ b/src/Http/Controllers/Internal/v1/UserController.php @@ -114,6 +114,11 @@ public function queryRecord(Request $request) */ public function onQueryRecord($query, Request $request): void { + // Eager-load what the `role` / `roles` / `policies` / `permissions` accessors + // read. CompanyUserRelation matches each user's own company membership, + // so authorization relations are fetched in batches across the page. + $query->with(['companyUser.roles', 'companyUser.policies', 'companyUser.permissions']); + if ($this->canAccessUsersAcrossCompanies($request)) { return; } diff --git a/src/Http/Resources/User.php b/src/Http/Resources/User.php index c8af56cb..f01b108d 100644 --- a/src/Http/Resources/User.php +++ b/src/Http/Resources/User.php @@ -17,6 +17,11 @@ class User extends FleetbaseResource */ public function toArray($request) { + // Read the `role` accessor once. It is not memoised — every read re-queries + // through `companyUser` — and this resource previously evaluated it four times + // per row (twice below, twice for `role_name`). + $role = Http::isInternalRequest() ? $this->role : null; + $data = [ 'id' => $this->when(Http::isInternalRequest(), $this->id, $this->public_id), 'uuid' => $this->when(Http::isInternalRequest(), $this->uuid), @@ -31,10 +36,10 @@ public function toArray($request) 'timezone' => $this->timezone, 'avatar_url' => $this->avatar_url, 'meta' => data_get($this, 'meta', Utils::createObject()), - 'role' => $this->when(Http::isInternalRequest(), $this->role ? new Role($this->role) : null, null), + 'role' => $this->when(Http::isInternalRequest(), $role ? new Role($role) : null, null), 'policies' => $this->when(Http::isInternalRequest(), Policy::collection($this->policies), []), 'permissions' => $this->when(Http::isInternalRequest(), $this->serializePermissions($this->permissions), []), - 'role_name' => $this->when(Http::isInternalRequest(), $this->role ? $this->role->name : null), + 'role_name' => $this->when(Http::isInternalRequest(), $role ? $role->name : null), 'type' => $this->type, 'locale' => $this->getLocale(), 'types' => $this->when(Http::isInternalRequest(), $this->types ?? []), diff --git a/src/Models/User.php b/src/Models/User.php index 0ff6b829..a6fd8301 100644 --- a/src/Models/User.php +++ b/src/Models/User.php @@ -6,6 +6,7 @@ use Fleetbase\Exceptions\InvalidVerificationCodeException; use Fleetbase\Notifications\UserCreated; use Fleetbase\Notifications\UserInvited; +use Fleetbase\Relations\CompanyUserRelation; use Fleetbase\Support\NotificationRegistry; use Fleetbase\Support\Timezone; use Fleetbase\Support\Utils; @@ -349,13 +350,15 @@ public function companies(): HasManyThrough * Defines the relationship between the user and their current company user record. * * This method establishes a `HasOne` relationship, indicating that the user has one associated - * `CompanyUser` record for the current company (determined by the `company_uuid` stored in the session). + * `CompanyUser` record for the company identified by the user's `company_uuid`. * * @return HasOne|Builder the relationship instance between the User and the CompanyUser model */ public function companyUser(): HasOne|Builder { - return $this->hasOne(CompanyUser::class, 'user_uuid', 'uuid')->where('company_uuid', $this->company_uuid); + $related = $this->newRelatedInstance(CompanyUser::class); + + return new CompanyUserRelation($related->newQuery(), $this, $related->qualifyColumn('user_uuid'), 'uuid'); } /** @@ -622,7 +625,13 @@ public function getRoleAttribute(): ?Role return null; } - return $this->companyUser->roles()->first(); + // Prefer the eager-loaded relation when the caller has loaded it, so a list + // query that eager-loads `companyUser.roles` pays no per-row query. Falls back + // to the query for callers that have not, and for a `companyUser` that is not + // an Eloquent model (the suite's UserModelAuthorizationPivotFake is duck-typed). + return $this->companyUser instanceof Model && $this->companyUser->relationLoaded('roles') + ? $this->companyUser->roles->first() + : $this->companyUser->roles()->first(); } /** @@ -639,7 +648,9 @@ public function getRolesAttribute(): Collection return collect(); } - return $this->companyUser->roles()->get(); + return $this->companyUser instanceof Model && $this->companyUser->relationLoaded('roles') + ? $this->companyUser->roles + : $this->companyUser->roles()->get(); } /** @@ -656,7 +667,9 @@ public function getPoliciesAttribute(): Collection return collect(); } - return $this->companyUser->policies()->get(); + return $this->companyUser instanceof Model && $this->companyUser->relationLoaded('policies') + ? $this->companyUser->policies + : $this->companyUser->policies()->get(); } /** @@ -673,7 +686,9 @@ public function getPermissionsAttribute(): Collection return collect(); } - return $this->companyUser->permissions()->get(); + return $this->companyUser instanceof Model && $this->companyUser->relationLoaded('permissions') + ? $this->companyUser->permissions + : $this->companyUser->permissions()->get(); } /** diff --git a/src/Relations/CompanyUserRelation.php b/src/Relations/CompanyUserRelation.php new file mode 100644 index 00000000..56fd8a43 --- /dev/null +++ b/src/Relations/CompanyUserRelation.php @@ -0,0 +1,55 @@ +query->where($this->related->qualifyColumn('company_uuid'), $this->parent->company_uuid); + } + } + + public function addEagerConstraints(array $models) + { + parent::addEagerConstraints($models); + + // Eloquent builds eager relations on an empty parent. Read company UUIDs + // from the actual models instead, retaining the user/company pairs. + $this->query->where(function (Builder $query) use ($models) { + foreach (collect($models)->groupBy('company_uuid') as $companyModels) { + $query->orWhere(function (Builder $query) use ($companyModels) { + $query->where($this->related->qualifyColumn('company_uuid'), $companyModels->first()->company_uuid) + ->whereIn($this->foreignKey, $companyModels->pluck($this->localKey)->all()); + }); + } + }); + } + + public function match(array $models, Collection $results, $relation) + { + $memberships = $results->groupBy('company_uuid'); + + foreach (collect($models)->groupBy('company_uuid') as $companyUuid => $companyModels) { + parent::match($companyModels->all(), $memberships->get($companyUuid, new Collection()), $relation); + } + + return $models; + } + + public function getRelationExistenceQuery(Builder $query, Builder $parentQuery, $columns = ['*']) + { + return parent::getRelationExistenceQuery($query, $parentQuery, $columns) + ->whereColumn($this->related->qualifyColumn('company_uuid'), $parentQuery->getModel()->qualifyColumn('company_uuid')); + } +} diff --git a/tests/Unit/Http/UserControllerTest.php b/tests/Unit/Http/UserControllerTest.php index 0014080f..77ad97ff 100644 --- a/tests/Unit/Http/UserControllerTest.php +++ b/tests/Unit/Http/UserControllerTest.php @@ -693,6 +693,96 @@ function user_controller_assert_created_user_response(mixed $response): object|a ->and($foreign->getData(true))->toBe(['errors' => ['User not found']]); }); +test('user controller batches authorization for each users own company without changing resource values', function () { + $db = user_controller_database()->getConnection('mysql'); + + // The member belongs to two companies; deliberately give its other membership + // different authorization so matching on user_uuid alone cannot pass. + foreach (['pivot-owner-1', 'pivot-member-1', 'pivot-member-2', 'pivot-foreign-1'] as $pivotUuid) { + $db->table('roles')->insert(['id' => $pivotUuid, 'name' => $pivotUuid, 'guard_name' => 'sanctum']); + $db->table('policies')->insert(['id' => $pivotUuid, 'name' => $pivotUuid, 'guard_name' => 'sanctum']); + $db->table('permissions')->insert(['id' => $pivotUuid, 'name' => $pivotUuid, 'guard_name' => 'sanctum']); + foreach (['roles' => 'role_id', 'policies' => 'policy_id', 'permissions' => 'permission_id'] as $relation => $key) { + $db->table('model_has_' . $relation)->insert([ + $key => $pivotUuid, + 'model_type' => Fleetbase\Models\CompanyUser::class, + 'model_uuid' => $pivotUuid, + ]); + } + } + + $expected = ['owner-1' => 'pivot-owner-1', 'member-1' => 'pivot-member-1', 'foreign-1' => 'pivot-foreign-1']; + $controller = user_controller(); + $route = new UserControllerRouteStub('queryRecord'); + $request = user_controller_request('GET', [], user_controller_user('admin-1'), 'queryRecord'); + $request->setRouteResolver(fn () => $route); + session(['user' => 'admin-1']); + $queryCounts = []; + foreach ([['owner-1'], array_keys($expected)] as $userUuids) { + $query = User::whereIn('uuid', $userUuids)->orderBy('uuid'); + $controller->onQueryRecord($query, $request); + $db->enableQueryLog(); + $db->flushQueryLog(); + $users = $query->get(); + $queryCounts[] = count($db->getQueryLog()); + $db->flushQueryLog(); + + foreach ($users as $user) { + expect($user->companyUser->uuid)->toBe($expected[$user->uuid]) + ->and($user->role->id)->toBe($expected[$user->uuid]) + ->and($user->roles->pluck('id')->all())->toBe([$expected[$user->uuid]]) + ->and($user->policies->pluck('id')->all())->toBe([$expected[$user->uuid]]) + ->and($user->permissions->pluck('id')->all())->toBe([$expected[$user->uuid]]); + } + expect($db->getQueryLog())->toBe([]); + $db->disableQueryLog(); + + foreach ($users as $user) { + $lazy = user_controller_user($user->uuid); + $serialize = fn (User $model) => json_decode(json_encode((new Fleetbase\Http\Resources\User($model))->resolve($request)), true); + expect($serialize($user))->toBe($serialize($lazy)); + } + } + expect($queryCounts[0])->toBeGreaterThan(1) + ->and($queryCounts[1])->toBe($queryCounts[0]); + + // The tenant-scoped controller path also loads the correct membership. + $query = User::where('uuid', 'member-1'); + session(['user' => 'owner-1']); + $request->setUserResolver(fn () => user_controller_user('owner-1')); + $controller->onQueryRecord($query, $request); + expect($query->firstOrFail()->role->id)->toBe('pivot-member-1'); +}); + +test('company user relation preserves lazy loading and matches duplicate users in different company contexts', function () { + user_controller_database(); + $member = user_controller_user('member-1'); + expect($member->companyUser()->first()->uuid)->toBe('pivot-member-1'); + $otherCompany = clone $member; + $otherCompany->company_uuid = 'company-2'; + expect($otherCompany->companyUser()->first()->uuid)->toBe('pivot-member-2'); + + $users = new Illuminate\Database\Eloquent\Collection([$member, $otherCompany]); + $users->load('companyUser'); + expect($member->companyUser->uuid)->toBe('pivot-member-1') + ->and($otherCompany->companyUser->uuid)->toBe('pivot-member-2'); +}); + +test('company user relation handles missing memberships and correlates existence queries to the users company', function () { + $db = user_controller_database()->getConnection('mysql'); + $db->table('company_users')->where('uuid', 'pivot-member-1')->update(['deleted_at' => '2026-07-18 10:00:00']); + $users = User::whereIn('uuid', ['admin-1', 'member-1', 'single-1'])->with('companyUser')->get()->keyBy('uuid'); + + expect($users['admin-1']->companyUser)->toBeNull() + ->and($users['member-1']->companyUser)->toBeNull() + ->and($users['single-1']->companyUser->uuid)->toBe('pivot-single-1') + ->and($users['member-1']->role)->toBeNull() + ->and($users['member-1']->roles)->toBeEmpty() + ->and($users['member-1']->policies)->toBeEmpty() + ->and($users['member-1']->permissions)->toBeEmpty() + ->and(User::whereHas('companyUser')->orderBy('uuid')->pluck('uuid')->all())->toBe(['foreign-1', 'owner-1', 'single-1']); +}); + test('user controller restores sandbox connection settings after generic user queries', function () { user_controller_database(); config([ diff --git a/tests/Unit/Models/UserModelTest.php b/tests/Unit/Models/UserModelTest.php index 428fff3d..5e996ea6 100644 --- a/tests/Unit/Models/UserModelTest.php +++ b/tests/Unit/Models/UserModelTest.php @@ -22,6 +22,29 @@ use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Facade; +/** + * A `companyUser` pivot whose relations are already eager-loaded, and whose relation + * QUERY methods throw. If an authorization accessor falls back to querying when the + * relation is present, these throws are what surfaces it. + */ +class UserModelEagerLoadedPivotFake extends Model +{ + public function roles(): object + { + throw new RuntimeException('roles() was queried despite an eager-loaded relation'); + } + + public function policies(): object + { + throw new RuntimeException('policies() was queried despite an eager-loaded relation'); + } + + public function permissions(): object + { + throw new RuntimeException('permissions() was queried despite an eager-loaded relation'); + } +} + class UserModelSaveSpy extends User { public int $saves = 0; @@ -669,6 +692,46 @@ public function save(): bool ->and((new User(['uuid' => 'user-3']))->getCompanyUser($missingCompany))->toBeNull(); }); +it('recovers a company membership created after the relation was loaded as null', function () { + user_model_schema(); + $db = app('db')->connection('mysql'); + $db->table('companies')->insert([ + 'uuid' => 'company-late-membership', + 'name' => 'Late Membership', + 'owner_uuid' => 'user-late-membership', + ]); + + $user = new User(); + $user->setRawAttributes([ + 'uuid' => 'user-late-membership', + 'company_uuid' => 'company-late-membership', + ], true); + $user->loadMissing('companyUser'); + expect($user->relationLoaded('companyUser'))->toBeTrue() + ->and($user->companyUser)->toBeNull(); + + // loadMissing will keep the cached null after a membership is created. + // loadCompanyUser must recover it through the database fallback. + $db->table('company_users')->insert([ + 'uuid' => 'late-membership', + 'company_uuid' => 'company-late-membership', + 'user_uuid' => 'user-late-membership', + 'status' => 'active', + ]); + + expect($user->loadCompanyUser())->toBe($user) + ->and($user->companyUser)->toBeInstanceOf(CompanyUser::class) + ->and($user->companyUser->uuid)->toBe('late-membership') + ->and($user->companyUser->company_uuid)->toBe('company-late-membership'); + + $membership = $user->companyUser; + $db->enableQueryLog(); + $db->flushQueryLog(); + expect($user->loadCompanyUser()->companyUser)->toBe($membership) + ->and($db->getQueryLog())->toBe([]); + $db->disableQueryLog(); +}); + it('falls back to database lookups for company and verification code helpers', function () { user_model_schema(); @@ -825,6 +888,41 @@ public function save(): bool ->and($userWithoutRoles->getRoleName())->toBeNull(); }); +it('reads eager-loaded authorization relations without re-querying them', function () { + user_model_container(); + config([ + 'auth.defaults.guard' => 'web', + 'auth.guards.web' => [ + 'driver' => 'session', + 'provider' => 'users', + ], + ]); + + $role = new Role(); + $role->setRawAttributes(['name' => 'Dispatcher'], true); + + $policy = new Policy(); + $policy->setRawAttributes(['name' => 'Orders Read'], true); + + $permission = new Permission(); + $permission->setRawAttributes(['name' => 'orders.read'], true); + + // The pivot's roles()/policies()/permissions() QUERY methods throw, so this test + // fails loudly if an accessor ignores the loaded relation and queries anyway. + $pivot = new UserModelEagerLoadedPivotFake(); + $pivot->setRelation('roles', collect([$role])); + $pivot->setRelation('policies', collect([$policy])); + $pivot->setRelation('permissions', collect([$permission])); + + $user = new UserModelSaveSpy(); + $user->setRelation('companyUser', $pivot); + + expect($user->role)->toBe($role) + ->and($user->roles)->toEqual(collect([$role])) + ->and($user->policies)->toEqual(collect([$policy])) + ->and($user->permissions)->toEqual(collect([$permission])); +}); + it('enriches new and existing users from request timezone data without calling missing helpers', function () { user_model_container();