From 64f71a00c91eafce68df8102a47e9a7893898068 Mon Sep 17 00:00:00 2001 From: Nolan Ehrstrom Date: Thu, 26 Sep 2024 17:35:35 -0700 Subject: [PATCH] Allow admin users to reassign any user task --- ProcessMaker/Models/ProcessRequestToken.php | 22 +----- .../Policies/ProcessRequestTokenPolicy.php | 35 +++++++--- .../ProcessMaker/Models/UserFactory.php | 4 +- .../Api/ProcessRequestTokenPolicyTest.php | 70 +++++++++++++++++++ 4 files changed, 99 insertions(+), 32 deletions(-) diff --git a/ProcessMaker/Models/ProcessRequestToken.php b/ProcessMaker/Models/ProcessRequestToken.php index 1b88fe7ca6..18de925842 100644 --- a/ProcessMaker/Models/ProcessRequestToken.php +++ b/ProcessMaker/Models/ProcessRequestToken.php @@ -7,6 +7,7 @@ use Exception; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Support\Arr; +use Illuminate\Support\Facades\Gate; use Illuminate\Support\Facades\Notification; use Laravel\Scout\Searchable; use Log; @@ -496,25 +497,6 @@ public function getAdvanceStatusAttribute() return $result; } - /** - * Check if the user has access to reassign this task - * - * @param User $user - */ - public function authorizeReassignment(User $user) - { - if ($user->can('update', $this)) { - $definitions = $this->getDefinition(); - if (empty($definitions['allowReassignment']) || $definitions['allowReassignment'] === 'false') { - throw new AuthorizationException('Not authorized to reassign this task'); - } - - return true; - } else { - throw new AuthorizationException('Not authorized to view this task'); - } - } - /** * Check if this task can be escalated to the manager by the assignee * @@ -1168,7 +1150,7 @@ public function reassign($toUserId, User $requestingUser) $reassingAction = true; } else { // Validate if user can reassign - $this->authorizeReassignment($requestingUser); + Gate::forUser($requestingUser)->authorize('reassign', $this); // Reassign user $this->reassignTo($toUserId); $this->persistUserData($toUserId); diff --git a/ProcessMaker/Policies/ProcessRequestTokenPolicy.php b/ProcessMaker/Policies/ProcessRequestTokenPolicy.php index 43893e3bd6..4f4612d651 100644 --- a/ProcessMaker/Policies/ProcessRequestTokenPolicy.php +++ b/ProcessMaker/Policies/ProcessRequestTokenPolicy.php @@ -3,6 +3,7 @@ namespace ProcessMaker\Policies; use Illuminate\Auth\Access\HandlesAuthorization; +use Illuminate\Auth\Access\Response; use Illuminate\Support\Facades\Request; use ProcessMaker\Models\AnonymousUser; use ProcessMaker\Models\ProcessRequestToken; @@ -17,7 +18,7 @@ class ProcessRequestTokenPolicy * Run before all methods to determine if the * user is an admin and can do everything. * - * @param \ProcessMaker\Models\User $user + * @param User $user * @return mixed */ public function before(User $user) @@ -30,8 +31,8 @@ public function before(User $user) /** * Determine whether the user can view the process request token. * - * @param \ProcessMaker\Models\User $user - * @param \ProcessMaker\Models\ProcessRequestToken $processRequestToken + * @param User $user + * @param ProcessRequestToken $processRequestToken * @return mixed */ public function view(User $user, ProcessRequestToken $processRequestToken) @@ -49,8 +50,8 @@ public function view(User $user, ProcessRequestToken $processRequestToken) /** * Determine whether the user can update the process request token. * - * @param \ProcessMaker\Models\User $user - * @param \ProcessMaker\Models\ProcessRequestToken $processRequestToken + * @param User $user + * @param ProcessRequestToken $processRequestToken * @return mixed */ public function update(User $user, ProcessRequestToken $processRequestToken) @@ -70,9 +71,9 @@ public function update(User $user, ProcessRequestToken $processRequestToken) /** * Determine if the user can view a screen associated with the task * - * @param \ProcessMaker\Models\User $user - * @param \ProcessMaker\Models\ProcessRequestToken $processRequestToken - * @param \ProcessMaker\Models\Screen $screen + * @param User $user + * @param ProcessRequestToken $processRequestToken + * @param Screen $screen * @return mixed */ public function viewScreen(User $user, ProcessRequestToken $task, Screen $screen) @@ -92,8 +93,8 @@ public function viewScreen(User $user, ProcessRequestToken $task, Screen $screen /** * Determine if a user can rollback the process request. * - * @param \ProcessMaker\Models\User $user - * @param \ProcessMaker\Models\ProcessRequestToken $processRequestToken + * @param User $user + * @param ProcessRequestToken $processRequestToken * * @return bool */ @@ -102,4 +103,18 @@ public function rollback(User $user, ProcessRequestToken $task) // For now, only the process manager can rollback the request return $user->id === $task->process->managerId; } + + public function reassign(User $user, ProcessRequestToken $task) + { + if ($user->can('update', $task)) { + $definitions = $task->getDefinition(); + if (empty($definitions['allowReassignment']) || $definitions['allowReassignment'] === 'false') { + return Response::deny('Not authorized to reassign this task'); + } + + return true; + } else { + return Response::deny('Not authorized to update this task'); + } + } } diff --git a/database/factories/ProcessMaker/Models/UserFactory.php b/database/factories/ProcessMaker/Models/UserFactory.php index b838effb7a..9a4380b193 100644 --- a/database/factories/ProcessMaker/Models/UserFactory.php +++ b/database/factories/ProcessMaker/Models/UserFactory.php @@ -20,10 +20,10 @@ public function definition(): array } return [ - 'username' => $this->faker->unique()->userName().'.'.$this->faker->word(), + 'username' => $this->faker->unique()->userName() . '.' . $this->faker->word(), 'email' => $this->faker->unique()->email(), 'password' => $GLOBALS['testPassword'], - 'status' => $this->faker->randomElement(['ACTIVE', 'INACTIVE']), + 'status' => 'ACTIVE', 'firstname' => $this->faker->firstName(), 'lastname' => $this->faker->lastName(), 'address' => $this->faker->streetAddress(), diff --git a/tests/Feature/Api/ProcessRequestTokenPolicyTest.php b/tests/Feature/Api/ProcessRequestTokenPolicyTest.php index f80c800682..ff2d9d63db 100644 --- a/tests/Feature/Api/ProcessRequestTokenPolicyTest.php +++ b/tests/Feature/Api/ProcessRequestTokenPolicyTest.php @@ -6,6 +6,8 @@ use Faker\Factory as Faker; use Illuminate\Support\Facades\Hash; use ProcessMaker\Models\Process; +use ProcessMaker\Models\ProcessCategory; +use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Models\ProcessRequestToken; use ProcessMaker\Models\ProcessTaskAssignment; use ProcessMaker\Models\Screen; @@ -124,4 +126,72 @@ public function testGetInterstitialNestedScreen() $response->assertStatus(200); $this->assertEquals('Screen Interstitial', $response->json()['config'][0]['name']); } + + public function createTaskHelper($allowReassignment, $assignedTo) + { + ProcessCategory::factory()->create(['is_system' => true]); + $this->seed(\ProcessMaker\Package\AdvancedUserManager\Database\Seeds\AssignmentProcessSeeder::class); + + $bpmn = file_get_contents(base_path('tests/Feature/Api/bpmnPatterns/SimpleTaskProcess.bpmn')); + $value = $allowReassignment ? 'true' : 'false'; + $bpmn = str_replace('pm:allowReassignment="false"', 'pm:allowReassignment="' . $value . '"', $bpmn); + $process = Process::factory()->create(['bpmn' => $bpmn]); + + $url = route('api.process_events.trigger', [$process->id, 'event' => 'node_1']); + $this->apiCall('POST', $url); + + $task = ProcessRequestToken::where('status', 'ACTIVE')->first(); + $task->user_id = $assignedTo->id; + $task->save(); + + return $task; + } + + public function testReassignTask() + { + $assignedUser = User::factory()->create(); + $reassignToUser = User::factory()->create(); + $task = $this->createTaskHelper(true, $assignedUser); + + $this->user = $assignedUser; + $response = $this->apiCall('PUT', route('api.tasks.update', $task), [ + 'user_id' => $reassignToUser->id, + ]); + $response->assertStatus(200); + + $this->assertEquals($reassignToUser->id, $task->refresh()->user_id); + } + + public function testReassignWithoutTaskSetting() + { + $assignedUser = User::factory()->create(); + $reassignToUser = User::factory()->create(); + $task = $this->createTaskHelper(false, $assignedUser); + + $this->user = $assignedUser; + $response = $this->apiCall('PUT', route('api.tasks.update', $task), [ + 'user_id' => $reassignToUser->id, + ]); + $response->assertStatus(403); + + $this->assertEquals('Not authorized to reassign this task', $response->json()['message']); + } + + public function testAdminReassignWithoutTaskSetting() + { + $assignedUser = User::factory()->create(); + $reassignToUser = User::factory()->create(); + $adminUser = User::factory()->create([ + 'is_administrator' => true, + ]); + $task = $this->createTaskHelper(false, $assignedUser); + + $this->user = $adminUser; + $response = $this->apiCall('PUT', route('api.tasks.update', $task), [ + 'user_id' => $reassignToUser->id, + ]); + $response->assertStatus(200); + + $this->assertEquals($reassignToUser->id, $task->refresh()->user_id); + } }