Fixed check for outside assets on user update validation

Signed-off-by: snipe <snipe@snipe.net>
This commit is contained in:
snipe 2024-09-17 15:46:45 +01:00
parent fe5fc6e0ce
commit 9e957baeb5
5 changed files with 105 additions and 8 deletions

View file

@ -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'));
}
}
}
}

View file

@ -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. <br>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. <br>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(

View file

@ -330,7 +330,13 @@
{{ trans('general.company') }}
</div>
<div class="col-md-9">
{{ $user->company->name }}
@can('view', 'App\Models\Company')
<a href="{{ route('companies.show', $user->company->id) }}">
{{ $user->company->name }}
</a>
@else
{{ $user->company->name }}
@endcan
</div>
</div>

View file

@ -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');
}
}

View file

@ -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');
}
}