Fixed #19100 - check all companies a user belongs to for asset assignment
This commit is contained in:
@@ -904,11 +904,21 @@ class AssetsController extends Controller
|
||||
|
||||
private function checkoutCompanyMismatchResponse(Asset $asset, User|Asset|Location $target): ?JsonResponse
|
||||
{
|
||||
if ((Setting::getSettings()->full_multiple_companies_support == '1')
|
||||
&& (! is_null($asset->company_id))
|
||||
&& (! is_null($target->company_id))
|
||||
&& ((int) $asset->company_id !== (int) $target->company_id)
|
||||
) {
|
||||
if (Setting::getSettings()->full_multiple_companies_support != '1' || is_null($asset->company_id)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// For users with multiple companies, check all their associated companies,
|
||||
// not just the primary company_id column.
|
||||
if ($target instanceof User) {
|
||||
if (! $target->canReceiveFromCompany((int) $asset->company_id)) {
|
||||
return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_user_company')));
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
if (! is_null($target->company_id) && (int) $asset->company_id !== (int) $target->company_id) {
|
||||
return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_user_company')));
|
||||
}
|
||||
|
||||
@@ -1062,13 +1072,8 @@ class AssetsController extends Controller
|
||||
}
|
||||
|
||||
// In FMCS mode, enforce explicit same-company target checks before mutating checkout state.
|
||||
$targetCompanyId = data_get($target, 'company_id');
|
||||
if ((Setting::getSettings()->full_multiple_companies_support == '1')
|
||||
&& (! is_null($asset->company_id))
|
||||
&& (! is_null($targetCompanyId))
|
||||
&& ((int) $asset->company_id !== (int) $targetCompanyId)
|
||||
) {
|
||||
return response()->json(Helper::formatStandardApiResponse('error', $error_payload, trans('general.error_user_company')));
|
||||
if ($mismatch = $this->checkoutCompanyMismatchResponse($asset, $target)) {
|
||||
return $mismatch;
|
||||
}
|
||||
|
||||
$checkout_at = request('checkout_at', date('Y-m-d H:i:s'));
|
||||
|
||||
@@ -121,9 +121,14 @@ class AssetCheckoutController extends Controller
|
||||
|
||||
$settings = Setting::getSettings();
|
||||
|
||||
// We have to check whether $target->company_id is null here since locations don't have a company yet
|
||||
if (($settings->full_multiple_companies_support) && ((! is_null($target->company_id)) && (! is_null($asset->company_id)))) {
|
||||
if ($target->company_id != $asset->company_id) {
|
||||
// Locations have no company, so we only enforce FMCS when both sides have a company_id.
|
||||
// For users with multiple companies, check all their associated companies via the pivot.
|
||||
if ($settings->full_multiple_companies_support && ! is_null($asset->company_id)) {
|
||||
$mismatch = $target instanceof User
|
||||
? ! $target->canReceiveFromCompany((int) $asset->company_id)
|
||||
: (! is_null($target->company_id) && (int) $target->company_id !== (int) $asset->company_id);
|
||||
|
||||
if ($mismatch) {
|
||||
return redirect()->route('hardware.checkout.create', $asset)->with('error', trans('general.error_user_company'));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ use App\Models\CustomField;
|
||||
use App\Models\LicenseSeat;
|
||||
use App\Models\Setting;
|
||||
use App\Models\Statuslabel;
|
||||
use App\Models\User;
|
||||
use App\View\Label;
|
||||
use Carbon\Carbon;
|
||||
use Illuminate\Contracts\View\View;
|
||||
@@ -687,18 +688,25 @@ class BulkAssetsController extends Controller
|
||||
->with('error', trans('general.error_assets_already_checked_out'));
|
||||
}
|
||||
|
||||
// Prevent checking out assets across companies if FMCS enabled
|
||||
if (Setting::getSettings()->full_multiple_companies_support && $target->company_id) {
|
||||
$company_ids = $assets->pluck('company_id')->unique();
|
||||
// Prevent checking out assets across companies if FMCS enabled.
|
||||
// For users with multiple companies, check all their associated companies via the pivot.
|
||||
if (Setting::getSettings()->full_multiple_companies_support) {
|
||||
$company_ids = $assets->pluck('company_id')->filter()->unique();
|
||||
|
||||
// if there is more than one unique company id or the singular company id does not match
|
||||
// then the checkout is invalid
|
||||
if ($company_ids->count() > 1 || $company_ids->first() != $target->company_id) {
|
||||
// re-add the asset ids so the assets select is re-populated
|
||||
$request->session()->flashInput(['selected_assets' => $asset_ids]);
|
||||
if ($company_ids->isNotEmpty()) {
|
||||
$assetCompanyId = (int) $company_ids->first();
|
||||
|
||||
return redirect(route('hardware.bulkcheckout.show'))
|
||||
->with('error', trans('general.error_user_company_multiple'));
|
||||
$mismatch = $company_ids->count() > 1
|
||||
|| ($target instanceof User
|
||||
? ! $target->canReceiveFromCompany($assetCompanyId)
|
||||
: (! is_null($target->company_id) && (int) $target->company_id !== $assetCompanyId));
|
||||
|
||||
if ($mismatch) {
|
||||
$request->session()->flashInput(['selected_assets' => $asset_ids]);
|
||||
|
||||
return redirect(route('hardware.bulkcheckout.show'))
|
||||
->with('error', trans('general.error_user_company_multiple'));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -625,6 +625,37 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
|
||||
return $this->belongsToMany(Company::class, 'company_user');
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns whether an FMCS company check should block this user from receiving
|
||||
* an asset that belongs to the given company.
|
||||
*
|
||||
* - If the user has no company associations at all: returns true (no restriction).
|
||||
* - If the user has associations: returns true only when $companyId is among them.
|
||||
*
|
||||
* Checks both the primary company_id column and the many-to-many pivot table so
|
||||
* that users assigned to multiple companies can receive assets from any of them.
|
||||
*/
|
||||
public function canReceiveFromCompany(int $companyId): bool
|
||||
{
|
||||
// Primary company matches
|
||||
if (! is_null($this->company_id) && (int) $this->company_id === $companyId) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Pivot company matches
|
||||
if ($this->companies()->where('companies.id', $companyId)->exists()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// User has no company associations — don't enforce (mirrors legacy behaviour
|
||||
// where a null company_id on the user skipped the FMCS check entirely).
|
||||
if (is_null($this->company_id) && ! $this->companies()->exists()) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sync company pivot membership and log the change if the set of companies changed.
|
||||
*
|
||||
|
||||
@@ -277,6 +277,87 @@ class AssetCheckoutTest extends TestCase
|
||||
});
|
||||
}
|
||||
|
||||
public function test_asset_can_be_checked_out_to_user_in_same_company_via_pivot_when_fmcs_enabled()
|
||||
{
|
||||
// Regression: company check used to compare asset company_id to user's primary company_id only.
|
||||
// Users assigned to multiple companies via the pivot table must be able to receive assets
|
||||
// from any of their companies — not just their first/primary one.
|
||||
$this->settings->enableMultipleFullCompanySupport();
|
||||
|
||||
[$companyA, $companyB, $companyC] = Company::factory()->count(3)->create();
|
||||
|
||||
// Actor is in companyC (same as the asset) so FMCS scoping lets them see and checkout it.
|
||||
$actor = User::factory()->checkoutAssets()->for($companyC)->create();
|
||||
$assetInCompanyC = Asset::factory()->for($companyC)->create();
|
||||
|
||||
// Target user's primary company is A, but they also belong to C via pivot.
|
||||
$target = User::factory()->for($companyA)->create();
|
||||
$target->companies()->sync([$companyA->id, $companyB->id, $companyC->id]);
|
||||
|
||||
$this->actingAsForApi($actor)
|
||||
->postJson(route('api.asset.checkout', $assetInCompanyC), [
|
||||
'checkout_to_type' => 'user',
|
||||
'assigned_user' => $target->id,
|
||||
])
|
||||
->assertOk()
|
||||
->assertStatusMessageIs('success');
|
||||
|
||||
$assetInCompanyC->refresh();
|
||||
$this->assertEquals($target->id, $assetInCompanyC->assigned_to);
|
||||
}
|
||||
|
||||
public function test_asset_cannot_be_checked_out_to_user_whose_companies_exclude_asset_company_when_fmcs_enabled()
|
||||
{
|
||||
$this->settings->enableMultipleFullCompanySupport();
|
||||
|
||||
[$companyA, $companyB, $companyC] = Company::factory()->count(3)->create();
|
||||
|
||||
// Actor is in companyC (same as the asset).
|
||||
$actor = User::factory()->checkoutAssets()->for($companyC)->create();
|
||||
$assetInCompanyC = Asset::factory()->for($companyC)->create();
|
||||
|
||||
// Target belongs to A and B — not C. Checkout to them should be blocked.
|
||||
$target = User::factory()->for($companyA)->create();
|
||||
$target->companies()->sync([$companyA->id, $companyB->id]);
|
||||
|
||||
$this->actingAsForApi($actor)
|
||||
->postJson(route('api.asset.checkout', $assetInCompanyC), [
|
||||
'checkout_to_type' => 'user',
|
||||
'assigned_user' => $target->id,
|
||||
])
|
||||
->assertOk()
|
||||
->assertStatusMessageIs('error')
|
||||
->assertMessagesAre(trans('general.error_user_company'));
|
||||
|
||||
$assetInCompanyC->refresh();
|
||||
$this->assertNull($assetInCompanyC->assigned_to);
|
||||
}
|
||||
|
||||
public function test_asset_can_be_checked_out_to_user_with_no_company_when_fmcs_enabled()
|
||||
{
|
||||
// Users with no company associations should not be blocked — they're unrestricted.
|
||||
$this->settings->enableMultipleFullCompanySupport();
|
||||
|
||||
$company = Company::factory()->create();
|
||||
// Actor is in the same company as the asset.
|
||||
$actor = User::factory()->checkoutAssets()->for($company)->create();
|
||||
$assetInCompany = Asset::factory()->for($company)->create();
|
||||
|
||||
$target = User::factory()->create(['company_id' => null]);
|
||||
$target->companies()->sync([]);
|
||||
|
||||
$this->actingAsForApi($actor)
|
||||
->postJson(route('api.asset.checkout', $assetInCompany), [
|
||||
'checkout_to_type' => 'user',
|
||||
'assigned_user' => $target->id,
|
||||
])
|
||||
->assertOk()
|
||||
->assertStatusMessageIs('success');
|
||||
|
||||
$assetInCompany->refresh();
|
||||
$this->assertEquals($target->id, $assetInCompany->assigned_to);
|
||||
}
|
||||
|
||||
public function test_license_seats_are_assigned_to_user_upon_checkout()
|
||||
{
|
||||
$this->markTestIncomplete('This is not implemented');
|
||||
|
||||
Reference in New Issue
Block a user