[ADVAPP-1064]: Refactor Task access to not use individual permissions to manage access

app-modules/task/database/migrations/2024_12_18_104302_delete_task_individual_update_permissions.php

@@ -0,0 +1,52 @@
+<?php
+
+/*
+<COPYRIGHT>
+
+    Copyright © 2016-2024, Canyon GBS LLC. All rights reserved.
+
+    Advising App™ is licensed under the Elastic License 2.0. For more details,
+    see https://github.com/canyongbs/advisingapp/blob/main/LICENSE.
+
+    Notice:
+
+    - You may not provide the software to third parties as a hosted or managed
+      service, where the service provides users with access to any substantial set of
+      the features or functionality of the software.
+    - You may not move, change, disable, or circumvent the license key functionality
+      in the software, and you may not remove or obscure any functionality in the
+      software that is protected by the license key.
+    - You may not alter, remove, or obscure any licensing, copyright, or other notices
+      of the licensor in the software. Any use of the licensor’s trademarks is subject
+      to applicable law.
+    - Canyon GBS LLC respects the intellectual property rights of others and expects the
+      same in return. Canyon GBS™ and Advising App™ are registered trademarks of
+      Canyon GBS LLC, and we are committed to enforcing and protecting our trademarks
+      vigorously.
+    - The software solution, including services, infrastructure, and code, is offered as a
+      Software as a Service (SaaS) by Canyon GBS LLC.
+    - Use of this software implies agreement to the license terms and conditions as stated
+      in the Elastic License 2.0.
+
+    For more information or inquiries please visit our website at
+    https://www.canyongbs.com or contact us via email at legal@canyongbs.com.
+
+</COPYRIGHT>
+*/
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Support\Facades\DB;
+
+return new class () extends Migration {
+    public function up(): void
+    {
+        DB::table('permissions')
+            ->select('id')
+            ->whereRaw("name ~ '^task\\.[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}\\.update$'")
+            ->orderBy('id')
+            ->chunkById(100, function ($rows) {
+                $ids = $rows->pluck('id')->toArray();
+                DB::table('permissions')->whereIn('id', $ids)->delete();
+            });
+    }
+};

app-modules/task/src/Observers/TaskObserver.php

@@ -36,12 +36,9 @@
 
 namespace AdvisingApp\Task\Observers;
 
-use AdvisingApp\Authorization\Models\Permission;
-use AdvisingApp\Authorization\Models\PermissionGroup;
 use AdvisingApp\Notification\Events\TriggeredAutoSubscription;
 use AdvisingApp\Task\Models\Task;
 use AdvisingApp\Task\Notifications\TaskAssignedToUserNotification;
-use App\Models\User;
 use Exception;
 use Illuminate\Support\Facades\DB;
 
@@ -65,27 +62,11 @@ public function creating(Task $task): void
                 $task->assignedTo()->associate($user);
             }
         }
-
-        Permission::create([
-            'name' => "task.{$task->getKey()}.update",
-            'guard_name' => 'web',
-            'group_id' => PermissionGroup::query()
-                ->where('name', 'Task')
-                ->value('id'),
-        ]);
     }
 
     public function created(Task $task): void
     {
         try {
-            // Add permissions to creator
-            $task->createdBy?->givePermissionTo("task.{$task->getKey()}.update");
-
-            // Add permissions to assigned User unless they are the creator
-            if ($task->assigned_to !== $task->created_by) {
-                $task->assignedTo?->givePermissionTo("task.{$task->getKey()}.update");
-            }
-
             TriggeredAutoSubscription::dispatchIf(! empty($task->createdBy), $task->createdBy, $task);
         } catch (Exception $e) {
             DB::rollBack();
@@ -94,24 +75,6 @@ public function created(Task $task): void
         }
     }
 
-    public function updated(Task $task): void
-    {
-        try {
-            if ($task->isDirty('assigned_to') && $task->assigned_to !== $task->created_by) {
-                if ($task->getOriginal('assigned_to') !== $task->created_by) {
-                    User::find($task->getOriginal('assigned_to'))?->revokePermissionTo("task.{$task->getKey()}.update");
-                }
-
-                // Add permissions to newly assigned User
-                $task->assignedTo?->givePermissionTo("task.{$task->getKey()}.update");
-            }
-        } catch (Exception $e) {
-            DB::rollBack();
-
-            throw $e;
-        }
-    }
-
     public function saved(Task $task): void
     {
         DB::commit();

app-modules/task/src/Policies/TaskPolicy.php

@@ -99,8 +99,12 @@ public function update(Authenticatable $authenticatable, Task $task): Response
             return Response::deny('You do not have permission to update this task.');
         }
 
+        if ($authenticatable->getKey() !== $task->assigned_to && $authenticatable->getKey() !== $task->created_by) {
+            return Response::deny('You do not have permission to update this task.');
+        }
+
         return $authenticatable->canOrElse(
-            abilities: ["task.{$task->getKey()}.update"],
+            abilities: ['task.*.update'],
             denyResponse: 'You do not have permission to update this task.'
         );
     }

app-modules/task/tests/EditTaskTest.php

@@ -36,6 +36,7 @@
 
 use AdvisingApp\Authorization\Enums\LicenseType;
 use AdvisingApp\Task\Filament\Resources\TaskResource;
+use AdvisingApp\Task\Filament\Resources\TaskResource\Pages\EditTask;
 use AdvisingApp\Task\Models\Task;
 use AdvisingApp\Task\Tests\RequestFactories\EditTaskRequestFactory;
 use App\Models\User;
@@ -62,13 +63,24 @@
             ])
         )->assertForbidden();
 
-    livewire(TaskResource\Pages\EditTask::class, [
+    livewire(EditTask::class, [
         'record' => $task->getRouteKey(),
     ])
         ->assertForbidden();
 
     $user->givePermissionTo('task.view-any');
-    $user->givePermissionTo("task.{$task->getKey()}.update");
+    $user->givePermissionTo('task.*.update');
+
+    actingAs($user)
+        ->get(
+            TaskResource::getUrl('edit', [
+                'record' => $task,
+            ])
+        )->assertForbidden();
+
+    $task->assignedTo()->associate($user)->save();
+
+    $task->refresh();
 
     actingAs($user)
         ->get(
@@ -80,7 +92,7 @@
     // TODO: Finish these tests to ensure changes are allowed
     $request = collect(EditTaskRequestFactory::new()->create());
 
-    livewire(TaskResource\Pages\EditTask::class, [
+    livewire(EditTask::class, [
         'record' => $task->getRouteKey(),
     ])
         ->fillForm($request->toArray())
@@ -89,3 +101,136 @@
 
     // TODO: Check for changes
 });
+
+test('A user without proper permissions and that is not associated to the Task cannot access', function () {
+    $user = User::factory()->licensed(LicenseType::cases())->create();
+
+    $task = Task::factory()->create();
+
+    actingAs($user)
+        ->get(
+            TaskResource::getUrl('edit', [
+                'record' => $task,
+            ])
+        )->assertForbidden();
+
+    livewire(EditTask::class, [
+        'record' => $task->getRouteKey(),
+    ])
+        ->assertForbidden();
+});
+
+test('A User with proper permissions and that is not associated to the Task cannot access', function () {
+    $user = User::factory()->licensed(LicenseType::cases())->create();
+
+    $task = Task::factory()->create();
+
+    $user->givePermissionTo('task.view-any');
+    $user->givePermissionTo('task.*.update');
+
+    actingAs($user)
+        ->get(
+            TaskResource::getUrl('edit', [
+                'record' => $task,
+            ])
+        )->assertForbidden();
+
+    livewire(EditTask::class, [
+        'record' => $task->getRouteKey(),
+    ])
+        ->assertForbidden();
+});
+
+test('A User without proper permissions that is the assigned user cannot access', function () {
+    $user = User::factory()->licensed(LicenseType::cases())->create();
+
+    $task = Task::factory()->create();
+
+    $task->assignedTo()->associate($user)->save();
+
+    $task->refresh();
+
+    actingAs($user)
+        ->get(
+            TaskResource::getUrl('edit', [
+                'record' => $task,
+            ])
+        )->assertForbidden();
+
+    livewire(EditTask::class, [
+        'record' => $task->getRouteKey(),
+    ])
+        ->assertForbidden();
+});
+
+test('A User without proper permissions that is the created by user cannot access', function () {
+    $user = User::factory()->licensed(LicenseType::cases())->create();
+
+    $task = Task::factory()->create();
+
+    $task->createdBy()->associate($user)->save();
+
+    $task->refresh();
+
+    actingAs($user)
+        ->get(
+            TaskResource::getUrl('edit', [
+                'record' => $task,
+            ])
+        )->assertForbidden();
+
+    livewire(EditTask::class, [
+        'record' => $task->getRouteKey(),
+    ])
+        ->assertForbidden();
+});
+
+test('A User with proper permissions that is the assigned user can access', function () {
+    $user = User::factory()->licensed(LicenseType::cases())->create();
+
+    $task = Task::factory()->create();
+
+    $user->givePermissionTo('task.view-any');
+    $user->givePermissionTo('task.*.update');
+
+    $task->assignedTo()->associate($user)->save();
+
+    $task->refresh();
+
+    actingAs($user)
+        ->get(
+            TaskResource::getUrl('edit', [
+                'record' => $task,
+            ])
+        )->assertSuccessful();
+
+    livewire(EditTask::class, [
+        'record' => $task->getRouteKey(),
+    ])
+        ->assertSuccessful();
+});
+
+test('A User with proper permissions that is the created by user can access.', function () {
+    $user = User::factory()->licensed(LicenseType::cases())->create();
+
+    $task = Task::factory()->create();
+
+    $user->givePermissionTo('task.view-any');
+    $user->givePermissionTo('task.*.update');
+
+    $task->createdBy()->associate($user)->save();
+
+    $task->refresh();
+
+    actingAs($user)
+        ->get(
+            TaskResource::getUrl('edit', [
+                'record' => $task,
+            ])
+        )->assertSuccessful();
+
+    livewire(EditTask::class, [
+        'record' => $task->getRouteKey(),
+    ])
+        ->assertSuccessful();
+});

app-modules/task/tests/TaskAssignmentTest.php

@@ -34,7 +34,6 @@
 </COPYRIGHT>
 */
 
-use AdvisingApp\Authorization\Models\Permission;
 use AdvisingApp\Task\Models\Task;
 use AdvisingApp\Task\Notifications\TaskAssignedToUserNotification;
 use App\Models\User;
@@ -44,41 +43,6 @@
     Notification::fake();
 });
 
-it('creates the proper permissions record when a Task is created', function () {
-    $task = Task::factory()->create();
-
-    expect(Permission::where('name', "task.{$task->getKey()}.update")->exists())->toBeTrue();
-});
-
-it('gives the proper permission to the creator of a Task', function () {
-    /** @var Task $task */
-    $task = Task::factory()->create();
-
-    expect($task->createdBy->can("task.{$task->getKey()}.update"))->toBeTrue();
-});
-
-it('gives the proper permission to the assigned User of a Task on create and update', function () {
-    /** @var Task $task */
-    $task = Task::factory()->assigned()->create();
-
-    expect($task->createdBy->can("task.{$task->getKey()}.update"))->toBeTrue()
-        ->and($task->assignedTo->can("task.{$task->getKey()}.update"))->toBeTrue();
-
-    $originalAssignedUser = $task->assignedTo;
-
-    $newAssignedUser = User::factory()->create();
-
-    $task->assignedTo()->associate($newAssignedUser)->save();
-
-    $task->refresh();
-    $originalAssignedUser->refresh();
-    $newAssignedUser->refresh();
-
-    expect($task->createdBy->can("task.{$task->getKey()}.update"))->toBeTrue()
-        ->and($newAssignedUser->can("task.{$task->getKey()}.update"))->toBeTrue()
-        ->and($originalAssignedUser->can("task.{$task->getKey()}.update"))->toBeFalse();
-});
-
 it('sends the proper notification to the assigned User', function () {
     $task = Task::factory()->assigned()->create();