From 3d5e3fe475030b7af00d8965e45f1d33339cf65f Mon Sep 17 00:00:00 2001 From: luNunezProcessmaker Date: Mon, 4 Sep 2023 09:22:00 -0400 Subject: [PATCH 1/3] feature/FOUR-10253 --- .../Http/Controllers/Api/UserController.php | 6 ++ .../views/shared/users/sidebar.blade.php | 63 +++++++++++-------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/ProcessMaker/Http/Controllers/Api/UserController.php b/ProcessMaker/Http/Controllers/Api/UserController.php index 9ed4e57563..cfd852d7b5 100644 --- a/ProcessMaker/Http/Controllers/Api/UserController.php +++ b/ProcessMaker/Http/Controllers/Api/UserController.php @@ -293,6 +293,12 @@ public function update(User $user, Request $request) $request->validate(User::rules($user)); $fields = $request->json()->all(); + if (($fields['username'] !== $user->getAttribute('username') || isset($fields['password'])) && + (!Auth::user()->hasPermission('edit-user-and-password') || !Auth::user()->is_administrator)) { + var_dump($fields['username'], $user->getAttribute('username')); + throw new AuthorizationException(__('Not authorized to update the username and password.')); + + } if (isset($fields['password'])) { $fields['password'] = Hash::make($fields['password']); } diff --git a/resources/views/shared/users/sidebar.blade.php b/resources/views/shared/users/sidebar.blade.php index f189b4258a..a79e41c2f5 100644 --- a/resources/views/shared/users/sidebar.blade.php +++ b/resources/views/shared/users/sidebar.blade.php @@ -23,34 +23,45 @@
{{__('Login Information')}}
-
- {!! Form::label('username', __('Username') . '*', [], false) !!} - {!! Form::text('username', null, ['id' => 'username', 'rows' => 4, 'class'=> 'form-control', 'v-model' - => 'formData.username', 'autocomplete' => 'off', 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.username}', 'required', 'aria-required' => 'true']) !!} - -
-
- - {{__('Leave the password blank to keep the current password:')}} - -
-
- {!! Form::label('password', __('New Password')) !!} - -
- {!! Form::password('password', ['id' => 'password', 'rows' => 4, 'class'=> 'form-control', 'v-model' - => 'formData.password', 'autocomplete' => 'new-password', '@input' => 'props.updatePassword($event.target.value)', - 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.password}']) !!} +
+
+ {!! Form::label('username', __('Username') . '*', [], false) !!} + {!! Form::text('username', null, ['id' => 'username', 'rows' => 4, 'class'=> 'form-control', 'v-model' + => 'formData.username', 'autocomplete' => 'off', 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.username}', 'required', 'aria-required' => 'true']) !!} + +
+ @can('edit-user-and-password') +
+ + {{__('Leave the password blank to keep the current password:')}} +
- -
+ @endcan +
+ {!! Form::label('password', __('New Password')) !!} + +
+ {!! Form::password('password', ['id' => 'password', 'rows' => 4, 'class'=> 'form-control', 'v-model' + => 'formData.password', 'autocomplete' => 'new-password', '@input' => 'props.updatePassword($event.target.value)', + 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.password}']) !!} +
+ +
-
- {!! Form::label('confPassword', __('Confirm Password')) !!} - {!! Form::password('confPassword', ['id' => 'confPassword', 'rows' => 4, 'class'=> 'form-control', 'v-model' - => 'formData.confPassword', 'autocomplete' => 'new-password', 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.password}']) !!} - -
+
+ {!! Form::label('confPassword', __('Confirm Password')) !!} + {!! Form::password('confPassword', ['id' => 'confPassword', 'rows' => 4, 'class'=> 'form-control', 'v-model' + => 'formData.confPassword', 'autocomplete' => 'new-password', 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.password}']) !!} + +
+ @cannot('edit-user-and-password') +
+ + {{__('To change the current username and password please contact your administrator.')}} + +
+ @endcannot + @if (!\Request::is('profile/edit'))
From d9bcca4a8273d7e1eb5897744c07c62d6dc64ccb Mon Sep 17 00:00:00 2001 From: luNunezProcessmaker Date: Mon, 4 Sep 2023 09:30:56 -0400 Subject: [PATCH 2/3] feature/FOUR-10253 --- resources/views/shared/users/sidebar.blade.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/resources/views/shared/users/sidebar.blade.php b/resources/views/shared/users/sidebar.blade.php index a79e41c2f5..cb97917c33 100644 --- a/resources/views/shared/users/sidebar.blade.php +++ b/resources/views/shared/users/sidebar.blade.php @@ -22,8 +22,10 @@
-
{{__('Login Information')}}
+ +
{{__('Login Information')}}
+
{!! Form::label('username', __('Username') . '*', [], false) !!} {!! Form::text('username', null, ['id' => 'username', 'rows' => 4, 'class'=> 'form-control', 'v-model' From 4682d7146d308b0d0b214a60e2aca732475da96e Mon Sep 17 00:00:00 2001 From: luNunezProcessmaker Date: Mon, 4 Sep 2023 14:58:47 -0400 Subject: [PATCH 3/3] feature/FOUR-10253 --- .../Http/Controllers/Api/UserController.php | 6 - ProcessMaker/Http/Kernel.php | 1 + .../ValidateEditUserAndPasswordPermission.php | 26 ++++ resources/lang/en.json | 2 + routes/api.php | 3 +- tests/Feature/Api/UsersTest.php | 121 +++++++++++++++--- 6 files changed, 133 insertions(+), 26 deletions(-) create mode 100644 ProcessMaker/Http/Middleware/ValidateEditUserAndPasswordPermission.php diff --git a/ProcessMaker/Http/Controllers/Api/UserController.php b/ProcessMaker/Http/Controllers/Api/UserController.php index cfd852d7b5..9ed4e57563 100644 --- a/ProcessMaker/Http/Controllers/Api/UserController.php +++ b/ProcessMaker/Http/Controllers/Api/UserController.php @@ -293,12 +293,6 @@ public function update(User $user, Request $request) $request->validate(User::rules($user)); $fields = $request->json()->all(); - if (($fields['username'] !== $user->getAttribute('username') || isset($fields['password'])) && - (!Auth::user()->hasPermission('edit-user-and-password') || !Auth::user()->is_administrator)) { - var_dump($fields['username'], $user->getAttribute('username')); - throw new AuthorizationException(__('Not authorized to update the username and password.')); - - } if (isset($fields['password'])) { $fields['password'] = Hash::make($fields['password']); } diff --git a/ProcessMaker/Http/Kernel.php b/ProcessMaker/Http/Kernel.php index 9865b838a5..06db381335 100644 --- a/ProcessMaker/Http/Kernel.php +++ b/ProcessMaker/Http/Kernel.php @@ -75,6 +75,7 @@ class Kernel extends HttpKernel 'external.connection' => \ProcessMaker\Http\Middleware\ValidateExternalConnection::class, 'client' => \Laravel\Passport\Http\Middleware\CheckClientCredentials::class, 'template-authorization' => \ProcessMaker\Http\Middleware\TemplateAuthorization::class, + 'edit_username_password' => \ProcessMaker\Http\Middleware\ValidateEditUserAndPasswordPermission::class, ]; /** diff --git a/ProcessMaker/Http/Middleware/ValidateEditUserAndPasswordPermission.php b/ProcessMaker/Http/Middleware/ValidateEditUserAndPasswordPermission.php new file mode 100644 index 0000000000..7c37508492 --- /dev/null +++ b/ProcessMaker/Http/Middleware/ValidateEditUserAndPasswordPermission.php @@ -0,0 +1,26 @@ +route('user'); + $fields = $request->json()->all(); + if (($fields['username'] !== $user->getAttribute('username') || in_array('password', $fields)) && + !Auth::user()->hasPermission('edit-user-and-password') && !Auth::user()->is_administrator) { + throw new AuthorizationException(__('Not authorized to update the username and password.')); + + } + + return $next($request); + } +} diff --git a/resources/lang/en.json b/resources/lang/en.json index f95bfefc40..c100ff85fb 100644 --- a/resources/lang/en.json +++ b/resources/lang/en.json @@ -583,6 +583,7 @@ "None": "None", "Normal": "Normal", "Not Authorized": "Not Authorized", + "Not authorized to update the username and password.": "Not authorized to update the username and password.", "Notifications (API)": "Notifications (API)", "Notifications Inbox": "Notifications Inbox", "Notifications": "Notifications", @@ -983,6 +984,7 @@ "Time": "Time", "Timeout": "Timeout", "Timing Control": "Timing Control", + "To change the current username and password please contact your administrator.": "To change the current username and password please contact your administrator.", "To Do Tasks": "To Do Tasks", "To Do": "To Do", "to": "to", diff --git a/routes/api.php b/routes/api.php index 49d3cbaa9a..3fa7a6e8b3 100644 --- a/routes/api.php +++ b/routes/api.php @@ -34,6 +34,7 @@ use ProcessMaker\Http\Controllers\Api\UserTokenController; use ProcessMaker\Http\Controllers\Process\ModelerController; use ProcessMaker\Http\Controllers\TestStatusController; +use ProcessMaker\Http\Middleware\ValidateEditUserAndPasswordPermission; Route::middleware('auth:api', 'setlocale', 'bindings', 'sanitize')->prefix('api/1.0')->name('api.')->group(function () { // Users @@ -43,7 +44,7 @@ Route::get('users/{user}/get_pinnned_controls', [UserController::class, 'getPinnnedControls'])->name('users.getPinnnedControls'); // Permissions handled in the controller Route::post('users', [UserController::class, 'store'])->name('users.store')->middleware('can:create-users'); Route::put('users/restore', [UserController::class, 'restore'])->name('users.restore')->middleware('can:create-users'); - Route::put('users/{user}', [UserController::class, 'update'])->name('users.update'); // Permissions handled in the controller + Route::put('users/{user}', [UserController::class, 'update'])->name('users.update')->middleware('edit_username_password'); // Permissions handled in the controller Route::put('users/{user}/update_pinned_controls', [UserController::class, 'updatePinnedControls'])->name('users.updatePinnnedControls'); // Permissions handled in the controller Route::delete('users/{user}', [UserController::class, 'destroy'])->name('users.destroy')->middleware('can:delete-users'); Route::put('password/change', [ChangePasswordController::class, 'update'])->name('password.update'); diff --git a/tests/Feature/Api/UsersTest.php b/tests/Feature/Api/UsersTest.php index ad64f0d6f9..001ee115fa 100644 --- a/tests/Feature/Api/UsersTest.php +++ b/tests/Feature/Api/UsersTest.php @@ -2,6 +2,7 @@ namespace Tests\Feature\Api; +use Database\Seeders\PermissionSeeder; use Faker\Factory as Faker; use Illuminate\Http\UploadedFile; use ProcessMaker\Models\Setting; @@ -40,6 +41,39 @@ class UsersTest extends TestCase 'created_at', ]; + public function getUpdatedData() + { + $faker = Faker::create(); + + return [ + 'username' => 'newusername', + 'email' => $faker->email(), + 'firstname' => $faker->firstName(), + 'lastname' => $faker->lastName(), + 'phone' => $faker->phoneNumber(), + 'cell' => $faker->phoneNumber(), + 'fax' => $faker->phoneNumber(), + 'address' => $faker->streetAddress(), + 'city' => $faker->city(), + 'state' => $faker->stateAbbr(), + 'postal' => $faker->postcode(), + 'country' => $faker->country(), + 'timezone' => $faker->timezone(), + 'status' => $faker->randomElement(['ACTIVE', 'INACTIVE']), + 'birthdate' => $faker->dateTimeThisCentury()->format('Y-m-d'), + 'password' => $faker->sentence(10), + ]; + } + + protected function withUserSetup() + { + $this->user->is_administrator = true; + $this->user->save(); + + (new PermissionSeeder)->run($this->user); + } + + /** * Test verify the parameter required for create form */ @@ -343,25 +377,7 @@ public function testUpdateUser() $verify = $this->apiCall('GET', $url); // Post saved success - $response = $this->apiCall('PUT', $url, [ - 'username' => 'updatemytestusername', - 'email' => $faker->email(), - 'firstname' => $faker->firstName(), - 'lastname' => $faker->lastName(), - 'phone' => $faker->phoneNumber(), - 'cell' => $faker->phoneNumber(), - 'fax' => $faker->phoneNumber(), - 'address' => $faker->streetAddress(), - 'city' => $faker->city(), - 'state' => $faker->stateAbbr(), - 'postal' => $faker->postcode(), - 'country' => $faker->country(), - 'timezone' => $faker->timezone(), - 'status' => $faker->randomElement(['ACTIVE', 'INACTIVE']), - 'birthdate' => $faker->dateTimeThisCentury()->format('Y-m-d'), - 'password' => $faker->password(8) . 'A' . '1', - 'force_change_password' => $faker->boolean(), - ]); + $response = $this->apiCall('PUT', $url, $this->getUpdatedData()); // Validate the header status code $response->assertStatus(204); @@ -694,4 +710,71 @@ public function testCreateUserValidateUsername() $response->assertStatus(422); } } + + /** + * Update username and password + * If is an admin user can edit username and password himself + */ + public function testUpdateUserAdmin() + { + $url = self::API_TEST_URL . '/' . $this->user->id; + + // Load the starting user data + $verify = $this->apiCall('GET', $url); + + // Post saved success + $response = $this->apiCall('PUT', $url, $this->getUpdatedData()); + + // Validate the header status code + $response->assertStatus(204); + + // Load the updated user data + $verifyNew = $this->apiCall('GET', $url); + + // Check that it has changed + $this->assertNotEquals($verify, $verifyNew); + } + + /** + * Update username and password + * If is a user without permission can not edit and a user with permission can edit himself + */ + public function testUpdateUserNotAdmin() + { + // Without permission + $this->user = User::factory()->create(['status' => 'ACTIVE']); + $this->user->is_administrator = false; + $this->user->save(); + $this->user->refresh(); + $this->flushSession(); + + $url = self::API_TEST_URL . '/' . $this->user->id; + + // Load the starting user data + $verify = $this->apiCall('GET', $url); + + $response = $this->apiCall('PUT', $url, $this->getUpdatedData()); + + // Validate the header status code + $response->assertStatus(403); + + // With permission + $this->user->giveDirectPermission('edit-user-and-password'); + $this->user->save(); + $this->user->refresh(); + $this->flushSession(); + + // Post saved success + $response = $this->apiCall('PUT', $url, $this->getUpdatedData()); + + // Validate the header status code + $response->assertStatus(204); + + // Load the updated user data + $verifyNew = $this->apiCall('GET', $url); + + // Check that it has changed + $this->assertNotEquals($verify, $verifyNew); + } + }