Fixed permissions for tasks endpoints

app/Http/Controllers/Api/V1/ProjectController.php

@@ -12,7 +12,6 @@ use App\Models\Organization;
 use App\Models\Project;
 use App\Models\User;
 use Illuminate\Auth\Access\AuthorizationException;
-use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Resources\Json\JsonResource;
 use Illuminate\Support\Facades\Auth;
@@ -47,12 +46,7 @@ class ProjectController extends Controller
             ->whereBelongsTo($organization, 'organization');
 
         if (! $canViewAllProjects) {
-            $projectsQuery->where(function (Builder $builder) use ($user): Builder {
+            $projectsQuery->visibleByUser($user);
-                return $builder->where('is_public', '=', true)
-                    ->orWhereHas('members', function (Builder $builder) use ($user): Builder {
-                        return $builder->whereBelongsTo($user, 'user');
-                    });
-            });
         }
 
         $projects = $projectsQuery->paginate();

app/Http/Controllers/Api/V1/TaskController.php

@@ -10,10 +10,14 @@ use App\Http\Requests\V1\Task\TaskUpdateRequest;
 use App\Http\Resources\V1\Task\TaskCollection;
 use App\Http\Resources\V1\Task\TaskResource;
 use App\Models\Organization;
+use App\Models\Project;
 use App\Models\Task;
+use App\Models\User;
 use Illuminate\Auth\Access\AuthorizationException;
+use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Resources\Json\JsonResource;
+use Illuminate\Support\Facades\Auth;
 
 class TaskController extends Controller
 {
@@ -37,6 +41,9 @@ class TaskController extends Controller
     public function index(Organization $organization, TaskIndexRequest $request): TaskCollection
     {
         $this->checkPermission($organization, 'tasks:view');
+        $canViewAllTasks = $this->hasPermission($organization, 'tasks:view:all');
+        /** @var User $user */
+        $user = Auth::user();
 
         $projectId = $request->input('project_id');
 
@@ -47,6 +54,13 @@ class TaskController extends Controller
             $query->where('project_id', '=', $projectId);
         }
 
+        if (! $canViewAllTasks) {
+            $query->whereHas('project', function (Builder $builder) use ($user): void {
+                /** @var Builder<Project> $builder */
+                $builder->visibleByUser($user);
+            });
+        }
+
         $tasks = $query->paginate();
 
         return new TaskCollection($tasks);

app/Http/Requests/V1/Task/TaskIndexRequest.php

@@ -6,9 +6,11 @@ namespace App\Http\Requests\V1\Task;
 
 use App\Models\Organization;
 use App\Models\Project;
+use App\Service\PermissionStore;
 use Illuminate\Contracts\Validation\ValidationRule;
 use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Foundation\Http\FormRequest;
+use Illuminate\Support\Facades\Auth;
 use Korridor\LaravelModelValidationRules\Rules\ExistsEloquent;
 
 /**
@@ -28,7 +30,13 @@ class TaskIndexRequest extends FormRequest
                 'uuid',
                 new ExistsEloquent(Project::class, null, function (Builder $builder): Builder {
                     /** @var Builder<Project> $builder */
-                    return $builder->whereBelongsTo($this->organization, 'organization');
+                    $builder = $builder->whereBelongsTo($this->organization, 'organization');
+
+                    if (! app(PermissionStore::class)->has($this->organization, 'tasks:view:all')) {
+                        $builder = $builder->visibleByUser(Auth::user());
+                    }
+
+                    return $builder;
                 }),
             ],
         ];

app/Models/Project.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace App\Models;
 
 use Database\Factories\ProjectFactory;
+use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Concerns\HasUuids;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
@@ -23,6 +24,7 @@ use Illuminate\Database\Eloquent\Relations\HasMany;
  * @property-read Client|null $client
  * @property-read Collection<Task> $tasks
  *
+ * @method Builder<Project> visibleByUser(User $user)
  * @method static ProjectFactory factory()
  */
 class Project extends Model
@@ -71,4 +73,17 @@ class Project extends Model
     {
         return $this->hasMany(Task::class);
     }
+
+    /**
+     * @param  Builder<Project>  $builder
+     */
+    public function scopeVisibleByUser(Builder $builder, User $user): void
+    {
+        $builder->where(function (Builder $builder) use ($user): Builder {
+            return $builder->where('is_public', '=', true)
+                ->orWhereHas('members', function (Builder $builder) use ($user): Builder {
+                    return $builder->whereBelongsTo($user, 'user');
+                });
+        });
+    }
 }

app/Providers/JetstreamServiceProvider.php

@@ -119,6 +119,10 @@ class JetstreamServiceProvider extends ServiceProvider
             'tags:create',
             'tags:update',
             'tags:delete',
+            'clients:view',
+            'clients:create',
+            'clients:update',
+            'clients:delete',
             'organizations:view',
             'members:view',
         ])->description('Managers have the ability to read, create, and update their own time entries as well as those of their team.');

tests/TestCase.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Tests;
 
+use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
 use TiMacDonald\Log\LogFake;
 
@@ -16,4 +17,9 @@ abstract class TestCase extends BaseTestCase
         parent::setUp();
         LogFake::bind();
     }
+
+    protected function assertEqualsIdsOfEloquentCollection(array $ids, Collection $models): void
+    {
+        $this->assertEqualsCanonicalizing($ids, $models->pluck('id')->toArray());
+    }
 }

tests/Unit/Endpoint/Api/V1/TaskEndpointTest.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Tests\Unit\Endpoint\Api\V1;
 
 use App\Models\Project;
+use App\Models\ProjectMember;
 use App\Models\Task;
 use Laravel\Passport\Passport;
 
@@ -25,11 +26,27 @@ class TaskEndpointTest extends ApiEndpointTestAbstract
         $response->assertForbidden();
     }
 
+    public function test_index_endpoint_validation_fails_if_project_id_is_not_pat(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+        ]);
+        Task::factory()->forOrganization($data->organization)->createMany(4);
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->getJson(route('api.v1.tasks.index', [$data->organization->getKey()]));
+
+        // Assert
+        $response->assertForbidden();
+    }
+
     public function test_index_endpoint_returns_list_of_all_tasks_of_organization(): void
     {
         // Arrange
         $data = $this->createUserWithPermission([
             'tasks:view',
+            'tasks:view:all',
         ]);
         $tasks = Task::factory()->forOrganization($data->organization)->createMany(4);
         Passport::actingAs($data->user);
@@ -42,11 +59,35 @@ class TaskEndpointTest extends ApiEndpointTestAbstract
         $response->assertJsonCount(4, 'data');
     }
 
+    public function test_index_endpoint_returns_list_of_all_tasks_with_access_of_organization_if_user_has_no_all_permission(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+            'tasks:view',
+        ]);
+        $otherProject = Project::factory()->create();
+        Task::factory()->forOrganization($data->organization)->forProject($otherProject)->createMany(4);
+        $projectPublic = Project::factory()->isPublic()->create();
+        Task::factory()->forOrganization($data->organization)->forProject($projectPublic)->createMany(2);
+        $projectAsMember = Project::factory()->isPrivate()->create();
+        ProjectMember::factory()->forProject($projectAsMember)->forUser($data->user)->create();
+        Task::factory()->forOrganization($data->organization)->forProject($projectAsMember)->createMany(2);
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->getJson(route('api.v1.tasks.index', [$data->organization->getKey()]));
+
+        // Assert
+        $response->assertStatus(200);
+        $response->assertJsonCount(4, 'data');
+    }
+
     public function test_index_endpoint_returns_list_of_all_tasks_of_organization_filtered_by_project(): void
     {
         // Arrange
         $data = $this->createUserWithPermission([
             'tasks:view',
+            'tasks:view:all',
         ]);
         $project = Project::factory()->forOrganization($data->organization)->create();
         Task::factory()->forOrganization($data->organization)->createMany(4);
@@ -90,7 +131,53 @@ class TaskEndpointTest extends ApiEndpointTestAbstract
         ]);
     }
 
-    public function test_store_endpoint_fails_if_user_has_no_permission_to_create_tags()
+    public function test_index_endpoint_validation_fails_if_project_is_not_visible_by_user_and_user_does_not_have_tasks_all_permission(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+            'tasks:view',
+        ]);
+        $project = Project::factory()->forOrganization($data->organization)->create();
+        Task::factory()->forOrganization($data->organization)->createMany(4);
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->getJson(route('api.v1.tasks.index', [
+            $data->organization->getKey(),
+            'project_id' => $project->getKey(),
+        ]));
+
+        // Assert
+        $response->assertStatus(422);
+        $response->assertInvalid([
+            'project_id',
+        ]);
+    }
+
+    public function test_index_endpoint_returns_list_of_all_tasks_of_organization_filtered_by_project_if_user_has_access_to_project(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+            'tasks:view',
+        ]);
+        $project = Project::factory()->forOrganization($data->organization)->create();
+        ProjectMember::factory()->forProject($project)->forUser($data->user)->create();
+        Task::factory()->forOrganization($data->organization)->createMany(4);
+        Task::factory()->forOrganization($data->organization)->forProject($project)->createMany(2);
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->getJson(route('api.v1.tasks.index', [
+            $data->organization->getKey(),
+            'project_id' => $project->getKey(),
+        ]));
+
+        // Assert
+        $response->assertStatus(200);
+        $response->assertJsonCount(2, 'data');
+    }
+
+    public function test_store_endpoint_fails_if_user_has_no_permission_to_create_tasks()
     {
         // Arrange
         $data = $this->createUserWithPermission([

tests/Unit/Model/ProjectModelTest.php

@@ -9,6 +9,7 @@ use App\Models\Organization;
 use App\Models\Project;
 use App\Models\ProjectMember;
 use App\Models\Task;
+use App\Models\User;
 
 class ProjectModelTest extends ModelTestAbstract
 {
@@ -86,4 +87,29 @@ class ProjectModelTest extends ModelTestAbstract
         $this->assertCount(3, $membersRel);
         $this->assertTrue($membersRel->first()->is($members->first()));
     }
+
+    public function test_scope_visible_by_user_filters_so_that_only_public_projects_or_projects_where_the_user_is_member_are_shown(): void
+    {
+        // Arrange
+        $user = User::factory()->create();
+        $projectPrivate = Project::factory()->isPrivate()->create();
+        $projectPublic = Project::factory()->isPublic()->create();
+        $projectPrivateButMember = Project::factory()->isPrivate()->create();
+        ProjectMember::factory()->forProject($projectPrivateButMember)->forUser($user)->create();
+
+        // Act
+        $projectsVisible = Project::query()->visibleByUser($user)->get();
+        $allProjects = Project::query()->get();
+
+        // Assert
+        $this->assertEqualsIdsOfEloquentCollection([
+            $projectPublic->getKey(),
+            $projectPrivateButMember->getKey(),
+        ], $projectsVisible);
+        $this->assertEqualsIdsOfEloquentCollection([
+            $projectPrivate->getKey(),
+            $projectPublic->getKey(),
+            $projectPrivateButMember->getKey(),
+        ], $allProjects);
+    }
 }