From 9e957baeb53e49abe4440bc30100b8629b065ae8 Mon Sep 17 00:00:00 2001 From: snipe Date: Tue, 17 Sep 2024 15:46:45 +0100 Subject: [PATCH] Fixed check for outside assets on user update validation Signed-off-by: snipe --- ...erCannotSwitchCompaniesIfItemsAssigned.php | 10 +++- resources/lang/en-US/admin/users/message.php | 2 +- resources/views/users/view.blade.php | 8 ++- tests/Feature/Users/Api/UpdateUserTest.php | 51 ++++++++++++++++++- tests/Feature/Users/Ui/UpdateUserTest.php | 42 ++++++++++++++- 5 files changed, 105 insertions(+), 8 deletions(-) diff --git a/app/Rules/UserCannotSwitchCompaniesIfItemsAssigned.php b/app/Rules/UserCannotSwitchCompaniesIfItemsAssigned.php index c3dd58f424..a433ee9a28 100644 --- a/app/Rules/UserCannotSwitchCompaniesIfItemsAssigned.php +++ b/app/Rules/UserCannotSwitchCompaniesIfItemsAssigned.php @@ -16,8 +16,14 @@ class UserCannotSwitchCompaniesIfItemsAssigned implements ValidationRule public function validate(string $attribute, mixed $value, Closure $fail): void { $user = User::find(request()->route('user')->id); - if (($value) && ($user->allAssignedCount() > 0) && (Setting::getSettings()->full_multiple_companies_support)) { - $fail(trans('admin/users/message.error.multi_company_items_assigned')); + + if (($value) && ($user->allAssignedCount() > 0) && (Setting::getSettings()->full_multiple_companies_support=='1')) { + + // Check for assets with a different company_id than the selected company_id + $user_assets = $user->assets()->where('assets.company_id', '!=', $value)->count(); + if ($user_assets > 0) { + $fail(trans('admin/users/message.error.multi_company_items_assigned')); + } } } } diff --git a/resources/lang/en-US/admin/users/message.php b/resources/lang/en-US/admin/users/message.php index 3f44226335..b6ddad3aac 100644 --- a/resources/lang/en-US/admin/users/message.php +++ b/resources/lang/en-US/admin/users/message.php @@ -53,7 +53,7 @@ return array( 'ldap_could_not_search' => 'Could not search the LDAP server. Please check your LDAP server configuration in the LDAP config file.
Error from LDAP Server:', 'ldap_could_not_get_entries' => 'Could not get entries from the LDAP server. Please check your LDAP server configuration in the LDAP config file.
Error from LDAP Server:', 'password_ldap' => 'The password for this account is managed by LDAP/Active Directory. Please contact your IT department to change your password. ', - 'multi_company_items_assigned' => 'This user has items assigned, please check them in before moving companies.' + 'multi_company_items_assigned' => 'This user has items assigned that belong to a different company. Please check them in or edit their company.' ), 'deletefile' => array( diff --git a/resources/views/users/view.blade.php b/resources/views/users/view.blade.php index 74856e3ace..76a53b37fd 100755 --- a/resources/views/users/view.blade.php +++ b/resources/views/users/view.blade.php @@ -330,7 +330,13 @@ {{ trans('general.company') }}
- {{ $user->company->name }} + @can('view', 'App\Models\Company') + + {{ $user->company->name }} + + @else + {{ $user->company->name }} + @endcan
diff --git a/tests/Feature/Users/Api/UpdateUserTest.php b/tests/Feature/Users/Api/UpdateUserTest.php index 647053d18f..e70471770f 100644 --- a/tests/Feature/Users/Api/UpdateUserTest.php +++ b/tests/Feature/Users/Api/UpdateUserTest.php @@ -422,7 +422,7 @@ class UpdateUserTest extends TestCase $this->assertTrue($user->refresh()->groups->contains($groupB)); } - public function testMultiCompanyUserCannotBeMovedIfHasAsset() + public function testMultiCompanyUserCannotBeMovedIfHasAssetInDifferentCompany() { $this->settings->enableMultipleFullCompanySupport(); @@ -434,7 +434,9 @@ class UpdateUserTest extends TestCase ]); $superUser = User::factory()->superuser()->create(); - $asset = Asset::factory()->create(); + $asset = Asset::factory()->create([ + 'company_id' => $companyA->id, + ]); // no assets assigned, therefore success $this->actingAsForApi($superUser)->patchJson(route('api.users.update', $user), [ @@ -465,4 +467,49 @@ class UpdateUserTest extends TestCase ])->assertStatusMessageIs('error'); } + public function testMultiCompanyUserCanBeUpdatedIfHasAssetInSameCompany() + { + $this->settings->enableMultipleFullCompanySupport(); + + $companyA = Company::factory()->create(); + $companyB = Company::factory()->create(); + + $user = User::factory()->create([ + 'company_id' => $companyA->id, + ]); + $superUser = User::factory()->superuser()->create(); + + $asset = Asset::factory()->create([ + 'company_id' => $companyA->id, + ]); + + // no assets assigned from other company, therefore success + $this->actingAsForApi($superUser)->patchJson(route('api.users.update', $user), [ + 'username' => 'test', + 'company_id' => $companyB->id, + ])->assertStatusMessageIs('success'); + + // same test but PUT + $this->actingAsForApi($superUser)->putJson(route('api.users.update', $user), [ + 'username' => 'test', + 'first_name' => 'Test', + 'company_id' => $companyB->id, + ])->assertStatusMessageIs('success'); + + $asset->checkOut($user, $superUser); + + // asset assigned from other company, therefore error + $this->actingAsForApi($superUser)->patchJson(route('api.users.update', $user), [ + 'username' => 'test', + 'company_id' => $companyB->id, + ])->assertStatusMessageIs('error'); + + // same test but PUT + $this->actingAsForApi($superUser)->putJson(route('api.users.update', $user), [ + 'username' => 'test', + 'first_name' => 'Test', + 'company_id' => $companyB->id, + ])->assertStatusMessageIs('error'); + } + } diff --git a/tests/Feature/Users/Ui/UpdateUserTest.php b/tests/Feature/Users/Ui/UpdateUserTest.php index 3e6867021f..29f6811c77 100644 --- a/tests/Feature/Users/Ui/UpdateUserTest.php +++ b/tests/Feature/Users/Ui/UpdateUserTest.php @@ -82,7 +82,7 @@ class UpdateUserTest extends TestCase $this->assertEquals(1, $admin->refresh()->activated); } - public function testMultiCompanyUserCannotBeMovedIfHasAsset() + public function testMultiCompanyUserCannotBeMovedIfHasAssetInDifferentCompany() { $this->settings->enableMultipleFullCompanySupport(); @@ -94,7 +94,9 @@ class UpdateUserTest extends TestCase ]); $superUser = User::factory()->superuser()->create(); - $asset = Asset::factory()->create(); + $asset = Asset::factory()->create([ + 'company_id' => $companyA->id, + ]); // no assets assigned, therefore success $this->actingAs($superUser)->put(route('users.update', $user), [ @@ -116,4 +118,40 @@ class UpdateUserTest extends TestCase $this->followRedirects($response)->assertSee('error'); } + + public function testMultiCompanyUserCanBeUpdatedIfHasAssetInSameCompany() + { + $this->settings->enableMultipleFullCompanySupport(); + + $companyA = Company::factory()->create(); + + $user = User::factory()->create([ + 'company_id' => $companyA->id, + ]); + $superUser = User::factory()->superuser()->create(); + + $asset = Asset::factory()->create([ + 'company_id' => $companyA->id, + ]); + + // no assets assigned, therefore success + $this->actingAs($superUser)->put(route('users.update', $user), [ + 'first_name' => 'test', + 'username' => 'test', + 'company_id' => $companyA->id, + 'redirect_option' => 'index' + ])->assertRedirect(route('users.index')); + + $asset->checkOut($user, $superUser); + + // asset assigned, therefore error + $response = $this->actingAs($superUser)->patchJson(route('users.update', $user), [ + 'first_name' => 'test', + 'username' => 'test', + 'company_id' => $companyA->id, + 'redirect_option' => 'index' + ]); + + $this->followRedirects($response)->assertSee('success'); + } }