From 740a20899cc03f2db851249e2f9ede0ee1ca6737 Mon Sep 17 00:00:00 2001 From: Koda Date: Mon, 20 Apr 2026 23:54:37 +0000 Subject: [PATCH 1/4] Scope project user payloads to visible users --- .../Projects/BuildProjectBacklogPayload.php | 15 ++++++++ .../BuildProjectTicketDetailPayload.php | 15 ++++++++ tests/Feature/Api/SettingsApiTest.php | 38 +++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/app/Actions/Projects/BuildProjectBacklogPayload.php b/app/Actions/Projects/BuildProjectBacklogPayload.php index eb71b01..e7b8622 100644 --- a/app/Actions/Projects/BuildProjectBacklogPayload.php +++ b/app/Actions/Projects/BuildProjectBacklogPayload.php @@ -59,9 +59,24 @@ class BuildProjectBacklogPayload )->resolve(), 'users' => UserResource::collection( User::query() + ->whereIn('id', $this->visibleUserIds($project)) ->orderBy('name') ->get(), )->resolve(), ]; } + + /** + * @return array + */ + private function visibleUserIds(Project $project): array + { + return User::query() + ->get() + ->filter(fn (User $candidate): bool => $this->permissionService->allows($candidate, Permission::ProjectRead, $project)) + ->pluck('id') + ->map(fn (mixed $userId): int => (int) $userId) + ->values() + ->all(); + } } diff --git a/app/Actions/Projects/BuildProjectTicketDetailPayload.php b/app/Actions/Projects/BuildProjectTicketDetailPayload.php index 26b4511..56a014b 100644 --- a/app/Actions/Projects/BuildProjectTicketDetailPayload.php +++ b/app/Actions/Projects/BuildProjectTicketDetailPayload.php @@ -116,9 +116,24 @@ class BuildProjectTicketDetailPayload 'ticketTypes' => TicketTypeResource::collection($project->ticketTypes()->get())->resolve(), 'users' => UserResource::collection( User::query() + ->whereIn('id', $this->visibleUserIds($project)) ->orderBy('name') ->get(), )->resolve(), ]; } + + /** + * @return array + */ + private function visibleUserIds(Project $project): array + { + return User::query() + ->get() + ->filter(fn (User $candidate): bool => $this->permissionService->allows($candidate, Permission::ProjectRead, $project)) + ->pluck('id') + ->map(fn (mixed $userId): int => (int) $userId) + ->values() + ->all(); + } } diff --git a/tests/Feature/Api/SettingsApiTest.php b/tests/Feature/Api/SettingsApiTest.php index b5ec766..686338d 100644 --- a/tests/Feature/Api/SettingsApiTest.php +++ b/tests/Feature/Api/SettingsApiTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Api; use App\Actions\ProjectTicketMetadata\InitializeProjectTicketMetadata; use App\Models\Project; +use App\Models\Team; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Http; @@ -149,6 +150,43 @@ class SettingsApiTest extends TestCase ->assertNoContent(); } + public function test_project_payloads_only_return_users_who_can_view_the_project(): void + { + [$user, $project] = $this->projectContext(); + $viewer = User::factory()->create(['name' => 'Visible User']); + User::factory()->create(['name' => 'Hidden User']); + $teamViewer = User::factory()->create(['name' => 'Team Viewer']); + $team = Team::factory()->create(); + + $this->syncProjectPermissions($project, $viewer, ['view']); + $team->users()->attach($teamViewer); + $this->syncProjectPermissions($project, $team, ['view']); + + $ticket = $project->tickets()->firstOrCreate([ + 'sequence_number' => 1, + ], [ + 'ticket_key' => 'CORE-1', + 'ticket_priority_id' => $project->ticketPriorities()->firstOrFail()->id, + 'ticket_status_id' => $project->ticketStatuses()->firstOrFail()->id, + 'ticket_type_id' => $project->ticketTypes()->where('is_parent', false)->firstOrFail()->id, + 'title' => 'Scoped users ticket', + ]); + + $this->actingAs($user) + ->getJson(route('api.projects.backlog', $project)) + ->assertOk() + ->assertJsonPath('data.users.0.name', 'Team Viewer') + ->assertJsonPath('data.users.1.name', 'Visible User') + ->assertJsonCount(2, 'data.users'); + + $this->actingAs($user) + ->getJson(route('api.projects.tickets.show', [$project, $ticket])) + ->assertOk() + ->assertJsonPath('data.users.0.name', 'Team Viewer') + ->assertJsonPath('data.users.1.name', 'Visible User') + ->assertJsonCount(2, 'data.users'); + } + public function test_project_repositories_and_user_git_connections_are_available(): void { Http::preventStrayRequests(); -- 2.54.0 From f412e4ec19b23bcb053c52e77f44fe87e9ada440 Mon Sep 17 00:00:00 2001 From: Koda Date: Tue, 21 Apr 2026 00:08:12 +0000 Subject: [PATCH 2/4] Refactor visible project users into query scope --- .../Projects/BuildProjectBacklogPayload.php | 16 +--------------- .../Projects/BuildProjectTicketDetailPayload.php | 16 +--------------- app/Models/User.php | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/app/Actions/Projects/BuildProjectBacklogPayload.php b/app/Actions/Projects/BuildProjectBacklogPayload.php index e7b8622..bff6315 100644 --- a/app/Actions/Projects/BuildProjectBacklogPayload.php +++ b/app/Actions/Projects/BuildProjectBacklogPayload.php @@ -59,24 +59,10 @@ class BuildProjectBacklogPayload )->resolve(), 'users' => UserResource::collection( User::query() - ->whereIn('id', $this->visibleUserIds($project)) + ->visibleToProject($project) ->orderBy('name') ->get(), )->resolve(), ]; } - - /** - * @return array - */ - private function visibleUserIds(Project $project): array - { - return User::query() - ->get() - ->filter(fn (User $candidate): bool => $this->permissionService->allows($candidate, Permission::ProjectRead, $project)) - ->pluck('id') - ->map(fn (mixed $userId): int => (int) $userId) - ->values() - ->all(); - } } diff --git a/app/Actions/Projects/BuildProjectTicketDetailPayload.php b/app/Actions/Projects/BuildProjectTicketDetailPayload.php index 56a014b..25257cb 100644 --- a/app/Actions/Projects/BuildProjectTicketDetailPayload.php +++ b/app/Actions/Projects/BuildProjectTicketDetailPayload.php @@ -116,24 +116,10 @@ class BuildProjectTicketDetailPayload 'ticketTypes' => TicketTypeResource::collection($project->ticketTypes()->get())->resolve(), 'users' => UserResource::collection( User::query() - ->whereIn('id', $this->visibleUserIds($project)) + ->visibleToProject($project) ->orderBy('name') ->get(), )->resolve(), ]; } - - /** - * @return array - */ - private function visibleUserIds(Project $project): array - { - return User::query() - ->get() - ->filter(fn (User $candidate): bool => $this->permissionService->allows($candidate, Permission::ProjectRead, $project)) - ->pluck('id') - ->map(fn (mixed $userId): int => (int) $userId) - ->values() - ->all(); - } } diff --git a/app/Models/User.php b/app/Models/User.php index ef51f81..b84a873 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -3,12 +3,14 @@ namespace App\Models; // use Illuminate\Contracts\Auth\MustVerifyEmail; +use App\Actions\Permissions\PermissionService; use Database\Factories\UserFactory; use Illuminate\Database\Eloquent\Attributes\Fillable; use Illuminate\Database\Eloquent\Attributes\Hidden; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasMany; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\MorphMany; use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Notifications\Notifiable; @@ -69,4 +71,17 @@ class User extends Authenticatable { return $this->hasMany(Ticket::class, 'author_user_id'); } + + public function scopeVisibleToProject(Builder $query, Project $project): Builder + { + $permissionService = app(PermissionService::class); + + return $query->where(function (Builder $builder) use ($permissionService, $project): void { + foreach (User::query()->select('id')->cursor() as $candidate) { + if ($permissionService->allows($candidate, \App\Enums\Permission::ProjectRead, $project)) { + $builder->orWhereKey($candidate->id); + } + } + }); + } } -- 2.54.0 From c9a3a7f58d2b5651c4bc55eaa75e3ff6332aa5c1 Mon Sep 17 00:00:00 2001 From: Twyco Date: Wed, 22 Apr 2026 10:39:59 +0200 Subject: [PATCH 3/4] fixed scope --- app/Models/User.php | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/app/Models/User.php b/app/Models/User.php index b84a873..22fdb24 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -4,13 +4,14 @@ namespace App\Models; // use Illuminate\Contracts\Auth\MustVerifyEmail; use App\Actions\Permissions\PermissionService; +use App\Enums\Permission; use Database\Factories\UserFactory; use Illuminate\Database\Eloquent\Attributes\Fillable; use Illuminate\Database\Eloquent\Attributes\Hidden; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasMany; -use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Relations\MorphMany; use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Notifications\Notifiable; @@ -76,12 +77,17 @@ class User extends Authenticatable { $permissionService = app(PermissionService::class); - return $query->where(function (Builder $builder) use ($permissionService, $project): void { - foreach (User::query()->select('id')->cursor() as $candidate) { - if ($permissionService->allows($candidate, \App\Enums\Permission::ProjectRead, $project)) { - $builder->orWhereKey($candidate->id); - } - } - }); + $allowedUserIds = User::query() + ->select('id') + ->cursor() + ->filter(fn (User $candidate) => $permissionService->allows( + $candidate, + Permission::ProjectRead, + $project + )) + ->pluck('id') + ->all(); + + return $query->whereKey($allowedUserIds); } } -- 2.54.0 From 3f4c8f49d9147244c6e1659b4680f4007ccd6cad Mon Sep 17 00:00:00 2001 From: Twyco Date: Wed, 22 Apr 2026 10:57:31 +0200 Subject: [PATCH 4/4] fixed test scope --- tests/Feature/Api/SettingsApiTest.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/Feature/Api/SettingsApiTest.php b/tests/Feature/Api/SettingsApiTest.php index 686338d..5f88734 100644 --- a/tests/Feature/Api/SettingsApiTest.php +++ b/tests/Feature/Api/SettingsApiTest.php @@ -175,16 +175,18 @@ class SettingsApiTest extends TestCase $this->actingAs($user) ->getJson(route('api.projects.backlog', $project)) ->assertOk() - ->assertJsonPath('data.users.0.name', 'Team Viewer') - ->assertJsonPath('data.users.1.name', 'Visible User') - ->assertJsonCount(2, 'data.users'); + ->assertJsonPath('data.users.0.name', $user->name) + ->assertJsonPath('data.users.1.name', 'Team Viewer') + ->assertJsonPath('data.users.2.name', 'Visible User') + ->assertJsonCount(3, 'data.users'); $this->actingAs($user) ->getJson(route('api.projects.tickets.show', [$project, $ticket])) ->assertOk() - ->assertJsonPath('data.users.0.name', 'Team Viewer') - ->assertJsonPath('data.users.1.name', 'Visible User') - ->assertJsonCount(2, 'data.users'); + ->assertJsonPath('data.users.0.name', $user->name) + ->assertJsonPath('data.users.1.name', 'Team Viewer') + ->assertJsonPath('data.users.2.name', 'Visible User') + ->assertJsonCount(3, 'data.users'); } public function test_project_repositories_and_user_git_connections_are_available(): void -- 2.54.0