From 07e70cf7a9c3d9a770872da34575b1cdef93837f Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:19:16 +0100 Subject: [PATCH 01/24] Added test --- tests/Unit/CompanyScopingTest.php | 60 +++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/Unit/CompanyScopingTest.php b/tests/Unit/CompanyScopingTest.php index 894f15a97d..dea7588644 100644 --- a/tests/Unit/CompanyScopingTest.php +++ b/tests/Unit/CompanyScopingTest.php @@ -149,6 +149,66 @@ class CompanyScopingTest extends TestCase $this->assertCanSee($licenseSeatB); } + #[DataProvider('models')] + public function test_company_user_cannot_see_null_company_items_in_strict_mode($model) + { + $company = Company::factory()->create(); + $nullCompanyItem = $model::factory()->create(['company_id' => null]); + $companyItem = $model::factory()->for($company)->create(); + $companyUser = $company->users()->save(User::factory()->make()); + + $this->settings->enableMultipleFullCompanySupport(); + + $this->actingAs($companyUser); + $this->assertCannotSee($nullCompanyItem); + $this->assertCanSee($companyItem); + } + + #[DataProvider('models')] + public function test_company_user_can_see_null_company_items_in_floater_mode($model) + { + $company = Company::factory()->create(); + $nullCompanyItem = $model::factory()->create(['company_id' => null]); + $companyItem = $model::factory()->for($company)->create(); + $companyUser = $company->users()->save(User::factory()->make()); + + $this->settings->enableFloaterMode(); + + $this->actingAs($companyUser); + $this->assertCanSee($nullCompanyItem); + $this->assertCanSee($companyItem); + } + + #[DataProvider('models')] + public function test_null_company_user_cannot_see_company_items_in_strict_mode($model) + { + $company = Company::factory()->create(); + $nullCompanyItem = $model::factory()->create(['company_id' => null]); + $companyItem = $model::factory()->for($company)->create(); + $nullCompanyUser = User::factory()->create(['company_id' => null]); + + $this->settings->enableMultipleFullCompanySupport(); + + $this->actingAs($nullCompanyUser); + $this->assertCanSee($nullCompanyItem); + $this->assertCannotSee($companyItem); + } + + #[DataProvider('models')] + public function test_null_company_user_can_see_all_items_in_floater_mode($model) + { + $company = Company::factory()->create(); + $nullCompanyItem = $model::factory()->create(['company_id' => null]); + $companyItem = $model::factory()->for($company)->create(); + $nullCompanyUser = User::factory()->create(['company_id' => null]); + + $this->settings->enableFloaterMode(); + + $this->actingAs($nullCompanyUser); + $this->assertCanSee($nullCompanyItem); + $this->assertCanSee($companyItem); + } + private function assertCanSee(Model $model) { $this->assertTrue( From cd1f6b8e73b5345a7967182c6cf6e6ed08901fe8 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:19:39 +0100 Subject: [PATCH 02/24] FMCS: Added floater option in admin settings --- ...dd_null_company_is_floater_to_settings.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 database/migrations/2026_06_10_000001_add_null_company_is_floater_to_settings.php diff --git a/database/migrations/2026_06_10_000001_add_null_company_is_floater_to_settings.php b/database/migrations/2026_06_10_000001_add_null_company_is_floater_to_settings.php new file mode 100644 index 0000000000..a106020236 --- /dev/null +++ b/database/migrations/2026_06_10_000001_add_null_company_is_floater_to_settings.php @@ -0,0 +1,22 @@ +boolean('null_company_is_floater')->default(false)->after('scope_locations_fmcs'); + }); + } + + public function down(): void + { + Schema::table('settings', function (Blueprint $table) { + $table->dropColumn('null_company_is_floater'); + }); + } +}; From 8cc4ad27c9e5f72c9e546ff6e03e6d0b39746371 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:20:05 +0100 Subject: [PATCH 03/24] FMCS: Added floater option checkbox to general settings --- resources/views/settings/general.blade.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/resources/views/settings/general.blade.php b/resources/views/settings/general.blade.php index cd84969794..d807b521be 100644 --- a/resources/views/settings/general.blade.php +++ b/resources/views/settings/general.blade.php @@ -61,6 +61,21 @@ + +
+
+ + {!! $errors->first('null_company_is_floater', '') !!} +

+ {{ trans('admin/settings/general.null_company_is_floater_help_text') }} +

+
+
+ +
From 3dd5358e738db62a6ea96219470fc14f2dc1b6d2 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:20:33 +0100 Subject: [PATCH 04/24] FMCS: updated/added tests --- .../Accessories/Ui/CheckoutAccessoryTest.php | 50 +++++++++++++++++++ .../Checkouts/Api/AssetCheckoutTest.php | 4 +- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php b/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php index 7c41d3aa49..132e57375a 100644 --- a/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php +++ b/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php @@ -114,6 +114,56 @@ class CheckoutAccessoryTest extends TestCase ]); } + public function test_checkout_to_null_company_user_blocked_in_strict_mode() + { + [$companyA] = Company::factory()->count(1)->create(); + $accessory = Accessory::factory()->for($companyA)->create(['qty' => 5]); + $nullCompanyUser = User::factory()->create(['company_id' => null]); + + $this->settings->enableMultipleFullCompanySupport(); + + $actor = User::factory()->superuser()->create(); + + $this->actingAs($actor) + ->post(route('accessories.checkout.store', $accessory), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $nullCompanyUser->id, + 'checkout_qty' => 1, + 'redirect_option' => 'index', + ]) + ->assertRedirect(); + + $this->assertDatabaseMissing('accessories_checkout', [ + 'accessory_id' => $accessory->id, + 'assigned_to' => $nullCompanyUser->id, + ]); + } + + public function test_checkout_to_null_company_user_succeeds_in_floater_mode() + { + [$companyA] = Company::factory()->count(1)->create(); + $accessory = Accessory::factory()->for($companyA)->create(['qty' => 5]); + $nullCompanyUser = User::factory()->create(['company_id' => null]); + + $this->settings->enableFloaterMode(); + + $actor = User::factory()->superuser()->create(); + + $this->actingAs($actor) + ->post(route('accessories.checkout.store', $accessory), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $nullCompanyUser->id, + 'checkout_qty' => 1, + 'redirect_option' => 'index', + ]) + ->assertRedirect(); + + $this->assertDatabaseHas('accessories_checkout', [ + 'accessory_id' => $accessory->id, + 'assigned_to' => $nullCompanyUser->id, + ]); + } + public function test_checkout_to_location_does_not_throw_when_fmcs_enabled() { [$companyA] = Company::factory()->count(1)->create(); diff --git a/tests/Feature/Checkouts/Api/AssetCheckoutTest.php b/tests/Feature/Checkouts/Api/AssetCheckoutTest.php index 6777e34e50..a5089a8cf1 100644 --- a/tests/Feature/Checkouts/Api/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/Api/AssetCheckoutTest.php @@ -335,8 +335,8 @@ class AssetCheckoutTest extends TestCase 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(); + // In floater mode, users with no company associations can receive items from any company. + $this->settings->enableFloaterMode(); $company = Company::factory()->create(); // Actor is in the same company as the asset. From 758c1cabc5a7afe9a43601aa5407030a506a7874 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:20:55 +0100 Subject: [PATCH 05/24] FMSC Tests: added enableFloaterMode for setup --- tests/Support/Settings.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/Support/Settings.php b/tests/Support/Settings.php index fff2a79f63..35e91b40f5 100644 --- a/tests/Support/Settings.php +++ b/tests/Support/Settings.php @@ -102,6 +102,19 @@ class Settings return $this; } + public function enableFloaterMode(): Settings + { + return $this->update([ + 'full_multiple_companies_support' => 1, + 'null_company_is_floater' => 1, + ]); + } + + public function disableFloaterMode(): Settings + { + return $this->update(['null_company_is_floater' => 0]); + } + public function enableSlackWebhook(): Settings { return $this->update([ From ab1a5c02417b6c5321d7cc76d28ca9e2402a1a94 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:22:50 +0100 Subject: [PATCH 06/24] FMCS: check for floater mode in user and company --- app/Models/Company.php | 31 +++++++++++++++++++++++++++++++ app/Models/User.php | 14 +++++++------- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/app/Models/Company.php b/app/Models/Company.php index 9c7a48878b..2b2c01da2e 100644 --- a/app/Models/Company.php +++ b/app/Models/Company.php @@ -398,10 +398,17 @@ final class Company extends SnipeModel return $query->whereIn('companies.id', $companyIds); } + $floater = Setting::getSettings()->null_company_is_floater; + // Users are scoped by pivot membership (company_user), not by company_id column, // since a user may belong to multiple companies and company_id alone is insufficient. if ($query->getModel()->getTable() == 'users') { if (empty($companyIds)) { + // Floater: actor has no company and is unrestricted — see everyone. + if ($floater) { + return $query; + } + // No pivot memberships: mirror old null-company behavior — show only users // who are also not in any company via the pivot. return $query->whereNotIn('users.id', function ($sub) { @@ -409,6 +416,17 @@ final class Company extends SnipeModel }); } + // Floater: also include users with no company associations (they float). + if ($floater) { + return $query->where(function ($q) use ($companyIds) { + $q->whereIn('users.id', function ($sub) use ($companyIds) { + $sub->select('user_id')->from('company_user')->whereIn('company_id', $companyIds); + })->orWhereNotIn('users.id', function ($sub) { + $sub->select('user_id')->from('company_user'); + }); + }); + } + return $query->whereIn('users.id', function ($sub) use ($companyIds) { $sub->select('user_id')->from('company_user')->whereIn('company_id', $companyIds); }); @@ -419,6 +437,11 @@ final class Company extends SnipeModel $table = ($table_name) ? $table_name.'.' : $query->getModel()->getTable().'.'; if (empty($companyIds)) { + // Floater: actor has no company and is unrestricted — see everything. + if ($floater) { + return $query; + } + return $query->whereNull($table.$column); } @@ -432,6 +455,14 @@ final class Company extends SnipeModel }); } + // Floater: null-company items are visible to users from any company. + if ($floater) { + return $query->where(function ($q) use ($table, $column, $companyIds) { + $q->whereIn($table.$column, $companyIds) + ->orWhereNull($table.$column); + }); + } + return $query->whereIn($table.$column, $companyIds); } } diff --git a/app/Models/User.php b/app/Models/User.php index 61a4714369..e43f45cc23 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -634,22 +634,22 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo */ public function canReceiveFromCompany(int $companyId): bool { + // Items with no company association are unrestricted — anyone can receive them. + if (! $companyId) { + return true; + } + // Query the pivot directly to avoid the Company model's FMCS global scope, // which would restrict results to the current actor's visible companies. $userCompanyIds = DB::table('company_user') ->where('user_id', $this->id) ->pluck('company_id'); - if ($userCompanyIds->contains($companyId)) { - return true; - } - - // User has no company associations — don't enforce. if ($userCompanyIds->isEmpty()) { - return true; + return (bool) Setting::getSettings()->null_company_is_floater; } - return false; + return $userCompanyIds->contains($companyId); } /** From 9f89dffaae7f0ecd30485da7101557b0373da550 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:23:14 +0100 Subject: [PATCH 07/24] FMCS: update the helper that checks for location-scoping --- app/Helpers/Helper.php | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/app/Helpers/Helper.php b/app/Helpers/Helper.php index c770210293..3a06fea177 100644 --- a/app/Helpers/Helper.php +++ b/app/Helpers/Helper.php @@ -14,6 +14,7 @@ use App\Models\License; use App\Models\Location; use App\Models\Setting; use App\Models\Statuslabel; +use App\Models\User; use Carbon\Carbon; use Illuminate\Contracts\Encryption\DecryptException; use Illuminate\Http\RedirectResponse; @@ -1745,20 +1746,36 @@ class Helper $count = 0; foreach ($items as $item) { + if (! $item) { + continue; + } - if ($item && $item->company_id != $location_company) { + // Users belong to companies via the many-to-many pivot (company_user). + // canReceiveFromCompany() returns true only when the user's pivot + // contains the location's company, so !canReceiveFromCompany() is + // the correct mismatch signal. + if ($item instanceof User) { + $isMismatch = ! $item->canReceiveFromCompany((int) $location_company); + } else { + $isMismatch = ($item->company_id != $location_company); + } + + if ($isMismatch) { + if ($item instanceof User) { + $itemCompanyIds = $item->companies->pluck('id')->implode(', '); + $itemCompanyNames = $item->companies->pluck('name')->implode(', '); + } else { + $itemCompanyIds = $item->company_id ?? null; + $itemCompanyNames = $item->company->name ?? null; + } $mismatched[] = [ class_basename(get_class($item)), $item->id, $item->name ?? $item->asset_tag ?? $item->serial ?? $item->username, $item->assigned_type ? str_replace('App\\Models\\', '', $item->assigned_type) : null, - $item->company_id ?? null, - $item->company->name ?? null, - // $item->defaultLoc->id ?? null, - // $item->defaultLoc->name ?? null, - // $item->defaultLoc->company->id ?? null, - // $item->defaultLoc->company->name ?? null, + $itemCompanyIds, + $itemCompanyNames, $item->location->name ?? null, $item->location->company->name ?? null, $location_company ?? null, From 87bc834885fa71f8cf949ef0ac2a0968c51e1137 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:25:13 +0100 Subject: [PATCH 08/24] FMCS: updated language strings (may tweak) --- resources/lang/en-US/admin/settings/general.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/resources/lang/en-US/admin/settings/general.php b/resources/lang/en-US/admin/settings/general.php index d0db5f1c25..274cede659 100644 --- a/resources/lang/en-US/admin/settings/general.php +++ b/resources/lang/en-US/admin/settings/general.php @@ -180,6 +180,8 @@ return [ 'scope_locations_fmcs_check_button' => 'Check Compatibility', 'scope_locations_fmcs_test_needed' => 'Please Check Compatibility to enable this', 'scope_locations_fmcs_support_disabled_text' => 'This option is disabled because you have conflicting locations set for :count or more items.', + 'null_company_is_floater_text' => 'Treat No-Company Users/Locations as Floaters', + 'null_company_is_floater_help_text' => 'Controls what can be checked out TO users or locations that have no company assigned. When disabled (default, matching v8.5 behavior), items can only be checked out to no-company targets if the item also has no company — null is treated as its own pseudo-company. When enabled, items from any company can be checked out to targets with no company assignment.', 'show_in_model_list' => 'Show in Model Dropdowns', 'optional' => 'optional', 'per_page' => 'Results Per Page', From d03f68ae3417484a1a3094c9a478ad86e1d40b0e Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:26:12 +0100 Subject: [PATCH 09/24] FMCS: Updated floater value in controller --- app/Http/Controllers/SettingsController.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/SettingsController.php b/app/Http/Controllers/SettingsController.php index a3d653eaa1..5420152a41 100644 --- a/app/Http/Controllers/SettingsController.php +++ b/app/Http/Controllers/SettingsController.php @@ -93,10 +93,12 @@ class SettingsController extends Controller $old_locations_fmcs = $setting->scope_locations_fmcs; $setting->full_multiple_companies_support = $request->input('full_multiple_companies_support', '0'); $setting->scope_locations_fmcs = $request->input('scope_locations_fmcs', '0'); + $setting->null_company_is_floater = $request->input('null_company_is_floater', '0'); - // Backward compatibility for locations makes no sense without FullMultipleCompanySupport + // These options make no sense without FullMultipleCompanySupport if (! $setting->full_multiple_companies_support) { $setting->scope_locations_fmcs = '0'; + $setting->null_company_is_floater = '0'; } // check for inconsistencies when activating scoped locations From 53628d6ae3988187e2b5b32fcb71900df13d642e Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:26:41 +0100 Subject: [PATCH 10/24] FMCS: Users API - Check for floater in results --- app/Http/Controllers/Api/UsersController.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 5512247dbb..6fb22ef6ea 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -404,7 +404,16 @@ class UsersController extends Controller if ((Setting::getSettings()->full_multiple_companies_support == '1') && $request->filled('companyId')) { $companyIds = array_values(array_filter(array_map('intval', explode(',', $request->input('companyId'))))); if (! empty($companyIds)) { - $users->whereHas('companies', fn ($q) => $q->whereIn('companies.id', $companyIds)); + if (Setting::getSettings()->null_company_is_floater) { + // Floater mode: users with no company associations can receive items from + // any company, so include them alongside exact-company matches. + $users->where(function ($q) use ($companyIds) { + $q->whereHas('companies', fn ($q2) => $q2->whereIn('companies.id', $companyIds)) + ->orWhereDoesntHave('companies'); + }); + } else { + $users->whereHas('companies', fn ($q) => $q->whereIn('companies.id', $companyIds)); + } } } From e3190c3922478aacd5838f333cb12ffe5d34d21d Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:41:02 +0100 Subject: [PATCH 11/24] FMCS Admin Settings: updated language --- resources/lang/en-US/admin/settings/general.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/lang/en-US/admin/settings/general.php b/resources/lang/en-US/admin/settings/general.php index 274cede659..54e76cca54 100644 --- a/resources/lang/en-US/admin/settings/general.php +++ b/resources/lang/en-US/admin/settings/general.php @@ -180,8 +180,8 @@ return [ 'scope_locations_fmcs_check_button' => 'Check Compatibility', 'scope_locations_fmcs_test_needed' => 'Please Check Compatibility to enable this', 'scope_locations_fmcs_support_disabled_text' => 'This option is disabled because you have conflicting locations set for :count or more items.', - 'null_company_is_floater_text' => 'Treat No-Company Users/Locations as Floaters', - 'null_company_is_floater_help_text' => 'Controls what can be checked out TO users or locations that have no company assigned. When disabled (default, matching v8.5 behavior), items can only be checked out to no-company targets if the item also has no company — null is treated as its own pseudo-company. When enabled, items from any company can be checked out to targets with no company assignment.', + 'null_company_is_floater_text' => 'Treat items and users without company associations as floaters', + 'null_company_is_floater_help_text' => 'When disabled, items can only be checked out to targets without a company association if the item also has no company - null is treated as its own pseudo-company. When enabled, items from any company can be checked out to targets with no company assignment.', 'show_in_model_list' => 'Show in Model Dropdowns', 'optional' => 'optional', 'per_page' => 'Results Per Page', From 0f6367bb17156c06ae73de5a16dd35c315472cb2 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 11:47:54 +0100 Subject: [PATCH 12/24] FMCS: Extended checks to accessories, bulk controllers, etc --- .../AccessoryCheckoutController.php | 19 ++++++++++++------- app/Http/Controllers/Api/AssetsController.php | 6 +++++- .../Assets/AssetCheckoutController.php | 4 +++- .../Assets/BulkAssetsController.php | 4 +++- .../Licenses/LicenseCheckoutController.php | 9 +++++++-- .../Accessories/Ui/CheckoutAccessoryTest.php | 2 +- 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/Accessories/AccessoryCheckoutController.php b/app/Http/Controllers/Accessories/AccessoryCheckoutController.php index a00e072cb7..0159481824 100644 --- a/app/Http/Controllers/Accessories/AccessoryCheckoutController.php +++ b/app/Http/Controllers/Accessories/AccessoryCheckoutController.php @@ -67,13 +67,18 @@ class AccessoryCheckoutController extends Controller $target = $this->determineCheckoutTarget(); session()->put(['checkout_to_type' => $target]); - if ( - Setting::getSettings()->full_multiple_companies_support == '1' - && $accessory->company_id - && $target instanceof User - && ! $target->canReceiveFromCompany($accessory->company_id) - ) { - return redirect()->back()->with('error', trans('general.error_user_company')); + if (Setting::getSettings()->full_multiple_companies_support == '1' && $accessory->company_id) { + if ($target instanceof User) { + $mismatch = ! $target->canReceiveFromCompany($accessory->company_id); + } else { + $mismatch = is_null($target->company_id) + ? ! Setting::getSettings()->null_company_is_floater + : (int) $target->company_id !== (int) $accessory->company_id; + } + + if ($mismatch) { + return redirect()->back()->with('error', trans('general.error_user_company')); + } } $accessory->checkout_qty = $request->input('checkout_qty', 1); diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 0581847bf0..550bd29074 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -927,7 +927,11 @@ class AssetsController extends Controller return null; } - if (! is_null($target->company_id) && (int) $asset->company_id !== (int) $target->company_id) { + $nonUserMismatch = is_null($target->company_id) + ? ! Setting::getSettings()->null_company_is_floater + : (int) $asset->company_id !== (int) $target->company_id; + + if ($nonUserMismatch) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_user_company'))); } diff --git a/app/Http/Controllers/Assets/AssetCheckoutController.php b/app/Http/Controllers/Assets/AssetCheckoutController.php index 9c0221984a..2400467276 100644 --- a/app/Http/Controllers/Assets/AssetCheckoutController.php +++ b/app/Http/Controllers/Assets/AssetCheckoutController.php @@ -126,7 +126,9 @@ class AssetCheckoutController extends Controller 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); + : (is_null($target->company_id) + ? ! $settings->null_company_is_floater + : (int) $target->company_id !== (int) $asset->company_id); if ($mismatch) { return redirect()->route('hardware.checkout.create', $asset)->with('error', trans('general.error_user_company')); diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 7bca83d2ec..889ed68891 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -699,7 +699,9 @@ class BulkAssetsController extends Controller $mismatch = $company_ids->count() > 1 || ($target instanceof User ? ! $target->canReceiveFromCompany($assetCompanyId) - : (! is_null($target->company_id) && (int) $target->company_id !== $assetCompanyId)); + : (is_null($target->company_id) + ? ! Setting::getSettings()->null_company_is_floater + : (int) $target->company_id !== $assetCompanyId)); if ($mismatch) { $request->session()->flashInput(['selected_assets' => $asset_ids]); diff --git a/app/Http/Controllers/Licenses/LicenseCheckoutController.php b/app/Http/Controllers/Licenses/LicenseCheckoutController.php index f30acbf2a7..afc209a936 100644 --- a/app/Http/Controllers/Licenses/LicenseCheckoutController.php +++ b/app/Http/Controllers/Licenses/LicenseCheckoutController.php @@ -99,8 +99,13 @@ class LicenseCheckoutController extends Controller if (Setting::getSettings()->full_multiple_companies_support == '1') { if ($request->filled('asset_id')) { $fmcsTarget = Asset::find($request->input('asset_id')); - if ($fmcsTarget && $license->company_id && $license->company_id !== $fmcsTarget->company_id) { - return redirect()->route('licenses.index')->with('error', trans('general.error_user_company')); + if ($fmcsTarget && $license->company_id) { + $mismatch = is_null($fmcsTarget->company_id) + ? ! Setting::getSettings()->null_company_is_floater + : ($license->company_id !== $fmcsTarget->company_id); + if ($mismatch) { + return redirect()->route('licenses.index')->with('error', trans('general.error_user_company')); + } } } elseif ($request->filled('assigned_to')) { $fmcsTarget = User::find($request->input('assigned_to')); diff --git a/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php b/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php index 132e57375a..a46ffe1bce 100644 --- a/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php +++ b/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php @@ -170,7 +170,7 @@ class CheckoutAccessoryTest extends TestCase $accessory = Accessory::factory()->for($companyA)->create(['qty' => 5]); $location = Location::factory()->create(); - $this->settings->enableMultipleFullCompanySupport(); + $this->settings->enableFloaterMode(); $actor = User::factory()->superuser()->create(); From 6fc2ff72520f13a4e90d219d9c6a1a6eb3c82c7d Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 12:15:28 +0100 Subject: [PATCH 13/24] FMCS console checker: added test --- app/Helpers/Helper.php | 8 +- .../Feature/Console/TestLocationsFmcsTest.php | 190 ++++++++++++++++++ 2 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/Console/TestLocationsFmcsTest.php diff --git a/app/Helpers/Helper.php b/app/Helpers/Helper.php index 3a06fea177..9f89d9db8f 100644 --- a/app/Helpers/Helper.php +++ b/app/Helpers/Helper.php @@ -1701,6 +1701,8 @@ class Helper return []; } + $floater = (bool) Setting::getSettings()->null_company_is_floater; + foreach ($locations as $location) { // in case of an update of a single location, use the newly requested company_id if ($new_company_id) { @@ -1756,8 +1758,12 @@ class Helper // the correct mismatch signal. if ($item instanceof User) { $isMismatch = ! $item->canReceiveFromCompany((int) $location_company); + } elseif ($item->company_id == $location_company) { + $isMismatch = false; + } elseif (is_null($item->company_id) || is_null($location_company)) { + $isMismatch = ! $floater; } else { - $isMismatch = ($item->company_id != $location_company); + $isMismatch = true; } if ($isMismatch) { diff --git a/tests/Feature/Console/TestLocationsFmcsTest.php b/tests/Feature/Console/TestLocationsFmcsTest.php new file mode 100644 index 0000000000..250ed21f6f --- /dev/null +++ b/tests/Feature/Console/TestLocationsFmcsTest.php @@ -0,0 +1,190 @@ +create(array_merge([ + 'location_id' => $location->id, + 'rtd_location_id' => $location->id, + ], $attrs)); + } + + public function test_item_at_location_with_same_company_is_not_flagged() + { + $company = Company::factory()->create(); + $location = Location::factory()->for($company)->create(); + $asset = $this->assetAt($location, ['company_id' => $company->id]); + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertNotContains($asset->id, $this->mismatchedIds($result)); + } + + public function test_item_at_location_with_different_company_is_flagged() + { + [$companyA, $companyB] = Company::factory()->count(2)->create(); + $location = Location::factory()->for($companyA)->create(); + $asset = $this->assetAt($location, ['company_id' => $companyB->id]); + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertContains($asset->id, $this->mismatchedIds($result)); + } + + public function test_null_company_item_at_company_location_is_flagged_in_strict_mode() + { + $company = Company::factory()->create(); + $location = Location::factory()->for($company)->create(); + $asset = $this->assetAt($location, ['company_id' => null]); + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertContains($asset->id, $this->mismatchedIds($result)); + } + + public function test_null_company_item_at_company_location_is_not_flagged_in_floater_mode() + { + $company = Company::factory()->create(); + $location = Location::factory()->for($company)->create(); + $asset = $this->assetAt($location, ['company_id' => null]); + + $this->settings->enableFloaterMode(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertNotContains($asset->id, $this->mismatchedIds($result)); + } + + public function test_company_item_at_null_company_location_is_flagged_in_strict_mode() + { + $company = Company::factory()->create(); + $location = Location::factory()->create(['company_id' => null]); + $asset = $this->assetAt($location, ['company_id' => $company->id]); + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertContains($asset->id, $this->mismatchedIds($result)); + } + + public function test_company_item_at_null_company_location_is_not_flagged_in_floater_mode() + { + $company = Company::factory()->create(); + $location = Location::factory()->create(['company_id' => null]); + $asset = $this->assetAt($location, ['company_id' => $company->id]); + + $this->settings->enableFloaterMode(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertNotContains($asset->id, $this->mismatchedIds($result)); + } + + public function test_null_company_item_at_null_company_location_is_never_flagged() + { + $location = Location::factory()->create(['company_id' => null]); + $asset = $this->assetAt($location, ['company_id' => null]); + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertNotContains($asset->id, $this->mismatchedIds($result)); + } + + public function test_user_at_location_with_matching_company_is_not_flagged() + { + $company = Company::factory()->create(); + $location = Location::factory()->for($company)->create(); + $user = User::factory()->create(['location_id' => $location->id]); + $user->companies()->sync([$company->id]); + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertNotContains($user->id, $this->mismatchedIds($result)); + } + + public function test_user_at_location_with_different_company_is_flagged() + { + [$companyA, $companyB] = Company::factory()->count(2)->create(); + $location = Location::factory()->for($companyA)->create(); + $user = User::factory()->create(['location_id' => $location->id]); + $user->companies()->sync([$companyB->id]); + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertContains($user->id, $this->mismatchedIds($result)); + } + + public function test_null_company_user_at_company_location_is_flagged_in_strict_mode() + { + $company = Company::factory()->create(); + $location = Location::factory()->for($company)->create(); + $user = User::factory()->create(['company_id' => null, 'location_id' => $location->id]); + $user->companies()->sync([]); + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertContains($user->id, $this->mismatchedIds($result)); + } + + public function test_null_company_user_at_company_location_is_not_flagged_in_floater_mode() + { + $company = Company::factory()->create(); + $location = Location::factory()->for($company)->create(); + $user = User::factory()->create(['company_id' => null, 'location_id' => $location->id]); + $user->companies()->sync([]); + + $this->settings->enableFloaterMode(); + + $result = Helper::test_locations_fmcs(true); + + $this->assertNotContains($user->id, $this->mismatchedIds($result)); + } + + public function test_location_id_option_scopes_check_to_single_location() + { + [$companyA, $companyB] = Company::factory()->count(2)->create(); + $locationA = Location::factory()->for($companyA)->create(); + $locationB = Location::factory()->for($companyB)->create(); + $assetA = $this->assetAt($locationA, ['company_id' => $companyB->id]); // mismatch at A + $assetB = $this->assetAt($locationB, ['company_id' => $companyA->id]); // mismatch at B + + $this->settings->enableMultipleFullCompanySupport(); + + $result = Helper::test_locations_fmcs(true, $locationA->id); + + $this->assertContains($assetA->id, $this->mismatchedIds($result)); + $this->assertNotContains($assetB->id, $this->mismatchedIds($result)); + } +} From 6a0ec6945126a79fc25c0990c99abe632db370c3 Mon Sep 17 00:00:00 2001 From: snipe Date: Wed, 10 Jun 2026 12:44:46 +0100 Subject: [PATCH 14/24] FMCS/Validation: Fixed #19166 - translate error messages on FMCS fail --- .../AccessoryCheckoutController.php | 12 ++++++++- .../Controllers/Api/LocationsController.php | 26 ++++++++++++++----- .../Assets/AssetCheckoutController.php | 12 ++++++++- .../ComponentCheckoutController.php | 6 ++++- .../ConsumableCheckoutController.php | 6 ++++- .../Licenses/LicenseCheckoutController.php | 12 +++++++-- app/Http/Controllers/LocationsController.php | 26 ++++++++++++++----- app/Providers/ValidationServiceProvider.php | 14 ++++++++++ resources/lang/en-US/general.php | 3 +++ resources/lang/en-US/validation.php | 2 +- 10 files changed, 100 insertions(+), 19 deletions(-) diff --git a/app/Http/Controllers/Accessories/AccessoryCheckoutController.php b/app/Http/Controllers/Accessories/AccessoryCheckoutController.php index 0159481824..331325d2e1 100644 --- a/app/Http/Controllers/Accessories/AccessoryCheckoutController.php +++ b/app/Http/Controllers/Accessories/AccessoryCheckoutController.php @@ -77,7 +77,17 @@ class AccessoryCheckoutController extends Controller } if ($mismatch) { - return redirect()->back()->with('error', trans('general.error_user_company')); + $targetType = match (class_basename($target)) { + 'User' => trans('general.user'), + 'Location' => trans('general.location'), + default => trans('general.asset'), + }; + + return redirect()->back()->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.accessory').' "'.$accessory->name.'"', + 'item_company' => $accessory->company?->name ?? trans('general.unassigned'), + 'target' => $targetType.' "'.($target->name ?? $target->username ?? $target->id).'"', + ])); } } diff --git a/app/Http/Controllers/Api/LocationsController.php b/app/Http/Controllers/Api/LocationsController.php index 64f706287d..fe1448b3c2 100644 --- a/app/Http/Controllers/Api/LocationsController.php +++ b/app/Http/Controllers/Api/LocationsController.php @@ -215,8 +215,12 @@ class LocationsController extends Controller if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); // check if parent is set and has a different company - if ($location->parent_id && Location::find($location->parent_id)->company_id != $location->company_id) { - return response()->json(Helper::formatStandardApiResponse('error', null, 'different company than parent')); + if ($location->parent_id && ($parent = Location::find($location->parent_id)) && $parent->company_id != $location->company_id) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_location_parent_company', [ + 'parent' => $parent->name, + 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), + 'location_company' => $location->company?->name ?? trans('general.unassigned'), + ]))); } } @@ -307,12 +311,22 @@ class LocationsController extends Controller if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); // check if there are related objects with different company - if (Helper::test_locations_fmcs(false, $id, $location->company_id)) { - return response()->json(Helper::formatStandardApiResponse('error', null, 'error scoped locations')); + if ($mismatched = Helper::test_locations_fmcs(false, $id, $location->company_id)) { + $first = $mismatched[0]; + + return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_location_scoped_items', [ + 'item_type' => trans('general.'.strtolower($first[0])), + 'item_name' => $first[2], + 'item_company' => $first[5] ?? trans('general.unassigned'), + ]))); } // check if parent is set and has a different company - if ($location->parent_id && Location::find($location->parent_id)->company_id != $location->company_id) { - return response()->json(Helper::formatStandardApiResponse('error', null, 'different company than parent')); + if ($location->parent_id && ($parent = Location::find($location->parent_id)) && $parent->company_id != $location->company_id) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_location_parent_company', [ + 'parent' => $parent->name, + 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), + 'location_company' => $location->company?->name ?? trans('general.unassigned'), + ]))); } } else { $location->company_id = $request->input('company_id'); diff --git a/app/Http/Controllers/Assets/AssetCheckoutController.php b/app/Http/Controllers/Assets/AssetCheckoutController.php index 2400467276..0e6e7cda0d 100644 --- a/app/Http/Controllers/Assets/AssetCheckoutController.php +++ b/app/Http/Controllers/Assets/AssetCheckoutController.php @@ -131,7 +131,17 @@ class AssetCheckoutController extends Controller : (int) $target->company_id !== (int) $asset->company_id); if ($mismatch) { - return redirect()->route('hardware.checkout.create', $asset)->with('error', trans('general.error_user_company')); + $targetType = match (class_basename($target)) { + 'User' => trans('general.user'), + 'Location' => trans('general.location'), + default => trans('general.asset'), + }; + + return redirect()->route('hardware.checkout.create', $asset)->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.asset').' "'.($asset->name ?? $asset->asset_tag).'"', + 'item_company' => $asset->company?->name ?? trans('general.unassigned'), + 'target' => $targetType.' "'.($target->name ?? $target->username ?? $target->id).'"', + ])); } } diff --git a/app/Http/Controllers/Components/ComponentCheckoutController.php b/app/Http/Controllers/Components/ComponentCheckoutController.php index 949e9a4f1a..83986cd497 100644 --- a/app/Http/Controllers/Components/ComponentCheckoutController.php +++ b/app/Http/Controllers/Components/ComponentCheckoutController.php @@ -105,7 +105,11 @@ class ComponentCheckoutController extends Controller $asset = Asset::find($request->input('asset_id')); if ((Setting::getSettings()->full_multiple_companies_support) && $component->company_id !== $asset->company_id) { - return redirect()->route('components.checkout.show', $componentId)->with('error', trans('general.error_user_company')); + return redirect()->route('components.checkout.show', $componentId)->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.component').' "'.$component->name.'"', + 'item_company' => $component->company?->name ?? trans('general.unassigned'), + 'target' => trans('general.asset').' "'.($asset->name ?? $asset->asset_tag).'"', + ])); } $component->checkout_qty = $request->input('assigned_qty'); diff --git a/app/Http/Controllers/Consumables/ConsumableCheckoutController.php b/app/Http/Controllers/Consumables/ConsumableCheckoutController.php index fddd821c17..9540a8b0ac 100644 --- a/app/Http/Controllers/Consumables/ConsumableCheckoutController.php +++ b/app/Http/Controllers/Consumables/ConsumableCheckoutController.php @@ -102,7 +102,11 @@ class ConsumableCheckoutController extends Controller && $consumable->company_id && ! $user->canReceiveFromCompany($consumable->company_id) ) { - return redirect()->back()->with('error', trans('general.error_user_company')); + return redirect()->back()->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.consumable').' "'.$consumable->name.'"', + 'item_company' => $consumable->company?->name ?? trans('general.unassigned'), + 'target' => trans('general.user').' "'.$user->username.'"', + ])); } // Update the consumable data diff --git a/app/Http/Controllers/Licenses/LicenseCheckoutController.php b/app/Http/Controllers/Licenses/LicenseCheckoutController.php index afc209a936..fc96b9f74b 100644 --- a/app/Http/Controllers/Licenses/LicenseCheckoutController.php +++ b/app/Http/Controllers/Licenses/LicenseCheckoutController.php @@ -104,13 +104,21 @@ class LicenseCheckoutController extends Controller ? ! Setting::getSettings()->null_company_is_floater : ($license->company_id !== $fmcsTarget->company_id); if ($mismatch) { - return redirect()->route('licenses.index')->with('error', trans('general.error_user_company')); + return redirect()->route('licenses.index')->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.license').' "'.$license->name.'"', + 'item_company' => $license->company?->name ?? trans('general.unassigned'), + 'target' => trans('general.asset').' "'.($fmcsTarget->name ?? $fmcsTarget->asset_tag).'"', + ])); } } } elseif ($request->filled('assigned_to')) { $fmcsTarget = User::find($request->input('assigned_to')); if ($fmcsTarget && $license->company_id && ! $fmcsTarget->canReceiveFromCompany($license->company_id)) { - return redirect()->route('licenses.index')->with('error', trans('general.error_user_company')); + return redirect()->route('licenses.index')->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.license').' "'.$license->name.'"', + 'item_company' => $license->company?->name ?? trans('general.unassigned'), + 'target' => trans('general.user').' "'.$fmcsTarget->username.'"', + ])); } } } diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index cad42a7c5b..da149d91ca 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -95,8 +95,12 @@ class LocationsController extends Controller if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); // check if parent is set and has a different company - if ($location->parent_id && Location::find($location->parent_id)->company_id != $location->company_id) { - return redirect()->back()->withInput()->withInput()->with('error', 'different company than parent'); + if ($location->parent_id && ($parent = Location::find($location->parent_id)) && $parent->company_id != $location->company_id) { + return redirect()->back()->withInput()->with('error', trans('general.error_location_parent_company', [ + 'parent' => $parent->name, + 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), + 'location_company' => $location->company?->name ?? trans('general.unassigned'), + ])); } } else { $location->company_id = $request->input('company_id'); @@ -175,12 +179,22 @@ class LocationsController extends Controller if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); // check if there are related objects with different company - if (Helper::test_locations_fmcs(false, $location->id, $location->company_id)) { - return redirect()->back()->withInput()->withInput()->with('error', 'error scoped locations'); + if ($mismatched = Helper::test_locations_fmcs(false, $location->id, $location->company_id)) { + $first = $mismatched[0]; + + return redirect()->back()->withInput()->with('error', trans('general.error_location_scoped_items', [ + 'item_type' => trans('general.'.strtolower($first[0])), + 'item_name' => $first[2], + 'item_company' => $first[5] ?? trans('general.unassigned'), + ])); } // check if parent is set and has a different company - if ($location->parent_id && Location::find($location->parent_id)->company_id != $location->company_id) { - return redirect()->back()->withInput()->withInput()->with('error', 'different company than parent'); + if ($location->parent_id && ($parent = Location::find($location->parent_id)) && $parent->company_id != $location->company_id) { + return redirect()->back()->withInput()->with('error', trans('general.error_location_parent_company', [ + 'parent' => $parent->name, + 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), + 'location_company' => $location->company?->name ?? trans('general.unassigned'), + ])); } } else { $location->company_id = $request->input('company_id'); diff --git a/app/Providers/ValidationServiceProvider.php b/app/Providers/ValidationServiceProvider.php index 50d8082af9..26befdc8fa 100644 --- a/app/Providers/ValidationServiceProvider.php +++ b/app/Providers/ValidationServiceProvider.php @@ -349,6 +349,20 @@ class ValidationServiceProvider extends ServiceProvider return in_array($value, $options); }); + Validator::replacer('fmcs_location', function ($message, $attribute, $rule, $parameters, $validator) { + $locationId = $validator->getData()[$attribute] ?? null; + $location = $locationId ? Location::find($locationId) : null; + + return str_replace( + [':location', ':location_company'], + [ + $location?->name ?? '?', + $location?->company?->name ?? trans('general.unassigned'), + ], + $message + ); + }); + // Validates that the company of the validated object matches the company of the location in case of scoped locations Validator::extend('fmcs_location', function ($attribute, $value, $parameters, $validator) { $settings = Setting::getSettings(); diff --git a/resources/lang/en-US/general.php b/resources/lang/en-US/general.php index 0a48ab53a7..7604eca137 100644 --- a/resources/lang/en-US/general.php +++ b/resources/lang/en-US/general.php @@ -587,6 +587,9 @@ return [ 'error_user_company' => 'Checkout target company and asset company do not match', 'error_user_company_multiple' => 'One or more of the checkout target company and asset company do not match', 'error_user_company_accept_view' => 'An Asset assigned to you belongs to a different company so you can\'t accept nor deny it, please check with your manager', + 'error_checkout_company_mismatch' => ':item cannot be checked out to :target — :item belongs to :item_company', + 'error_location_scoped_items' => 'This location cannot be saved. :item_type ":item_name" (Company: :item_company) does not match this location\'s company.', + 'error_location_parent_company' => 'Parent location ":parent" belongs to :parent_company, but this location belongs to :location_company.', 'error_assets_already_checked_out' => 'One or more of the assets are already checked out', 'assigned_assets_removed' => 'The following were removed from the selected assets because they are already checked out', 'unassigned_assets_removed' => 'The following were removed from the selected assets because they are not currently checked out', diff --git a/resources/lang/en-US/validation.php b/resources/lang/en-US/validation.php index eccd1abb08..7ae4cb26d3 100644 --- a/resources/lang/en-US/validation.php +++ b/resources/lang/en-US/validation.php @@ -174,7 +174,7 @@ return [ 'ulid' => 'The :attribute field must be a valid ULID.', 'uuid' => 'The :attribute field must be a valid UUID.', 'valid_css_color' => 'The :attribute field must be a valid CSS color (hex, rgb, rgba, hsl, or hsla).', - 'fmcs_location' => 'Full multiple company support and location scoping is enabled in the Admin Settings, and the selected location and selected company are not compatible.', + 'fmcs_location' => 'Location ":location" belongs to :location_company, which does not match the selected company.', 'is_unique_across_company_and_location' => 'The :attribute must be unique within the selected company and location.', /* From a27c551f64a309268fb5753e910ac9af7c41b554 Mon Sep 17 00:00:00 2001 From: snipe Date: Fri, 12 Jun 2026 16:10:17 +0100 Subject: [PATCH 15/24] Style changes requested --- .../AccessoryCheckoutController.php | 9 ++++--- app/Http/Controllers/Api/AssetsController.php | 10 +++++--- .../Controllers/Api/LocationsController.php | 6 +++-- app/Http/Controllers/Api/UsersController.php | 11 +-------- .../Assets/AssetCheckoutController.php | 17 ++++++++----- .../Assets/BulkAssetsController.php | 19 ++++++++++----- .../Licenses/LicenseCheckoutController.php | 10 +++++--- app/Http/Controllers/LocationsController.php | 6 +++-- app/Models/Company.php | 24 +++++++++++++++---- 9 files changed, 73 insertions(+), 39 deletions(-) diff --git a/app/Http/Controllers/Accessories/AccessoryCheckoutController.php b/app/Http/Controllers/Accessories/AccessoryCheckoutController.php index 331325d2e1..3f0bc8054a 100644 --- a/app/Http/Controllers/Accessories/AccessoryCheckoutController.php +++ b/app/Http/Controllers/Accessories/AccessoryCheckoutController.php @@ -69,11 +69,14 @@ class AccessoryCheckoutController extends Controller if (Setting::getSettings()->full_multiple_companies_support == '1' && $accessory->company_id) { if ($target instanceof User) { + // Users may belong to multiple companies; canReceiveFromCompany() checks the pivot. $mismatch = ! $target->canReceiveFromCompany($accessory->company_id); + } elseif (is_null($target->company_id)) { + // Target has no company — only a mismatch when floater mode is off. + $mismatch = ! Setting::getSettings()->null_company_is_floater; } else { - $mismatch = is_null($target->company_id) - ? ! Setting::getSettings()->null_company_is_floater - : (int) $target->company_id !== (int) $accessory->company_id; + // Both sides have a company; require an exact match. + $mismatch = (int) $target->company_id !== (int) $accessory->company_id; } if ($mismatch) { diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 550bd29074..8930f376a1 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -927,9 +927,13 @@ class AssetsController extends Controller return null; } - $nonUserMismatch = is_null($target->company_id) - ? ! Setting::getSettings()->null_company_is_floater - : (int) $asset->company_id !== (int) $target->company_id; + if (is_null($target->company_id)) { + // Target has no company — only a mismatch when floater mode is off. + $nonUserMismatch = ! Setting::getSettings()->null_company_is_floater; + } else { + // Both sides have a company; require an exact match. + $nonUserMismatch = (int) $asset->company_id !== (int) $target->company_id; + } if ($nonUserMismatch) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_user_company'))); diff --git a/app/Http/Controllers/Api/LocationsController.php b/app/Http/Controllers/Api/LocationsController.php index fe1448b3c2..bfa27a5a28 100644 --- a/app/Http/Controllers/Api/LocationsController.php +++ b/app/Http/Controllers/Api/LocationsController.php @@ -215,7 +215,8 @@ class LocationsController extends Controller if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); // check if parent is set and has a different company - if ($location->parent_id && ($parent = Location::find($location->parent_id)) && $parent->company_id != $location->company_id) { + $parent = $location->parent_id ? Location::find($location->parent_id) : null; + if ($parent && $parent->company_id != $location->company_id) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_location_parent_company', [ 'parent' => $parent->name, 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), @@ -321,7 +322,8 @@ class LocationsController extends Controller ]))); } // check if parent is set and has a different company - if ($location->parent_id && ($parent = Location::find($location->parent_id)) && $parent->company_id != $location->company_id) { + $parent = $location->parent_id ? Location::find($location->parent_id) : null; + if ($parent && $parent->company_id != $location->company_id) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_location_parent_company', [ 'parent' => $parent->name, 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 6fb22ef6ea..49c3960055 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -404,16 +404,7 @@ class UsersController extends Controller if ((Setting::getSettings()->full_multiple_companies_support == '1') && $request->filled('companyId')) { $companyIds = array_values(array_filter(array_map('intval', explode(',', $request->input('companyId'))))); if (! empty($companyIds)) { - if (Setting::getSettings()->null_company_is_floater) { - // Floater mode: users with no company associations can receive items from - // any company, so include them alongside exact-company matches. - $users->where(function ($q) use ($companyIds) { - $q->whereHas('companies', fn ($q2) => $q2->whereIn('companies.id', $companyIds)) - ->orWhereDoesntHave('companies'); - }); - } else { - $users->whereHas('companies', fn ($q) => $q->whereIn('companies.id', $companyIds)); - } + $users = Company::scopeUsersByCompanyIds($users, $companyIds); } } diff --git a/app/Http/Controllers/Assets/AssetCheckoutController.php b/app/Http/Controllers/Assets/AssetCheckoutController.php index 0e6e7cda0d..86b9b9897e 100644 --- a/app/Http/Controllers/Assets/AssetCheckoutController.php +++ b/app/Http/Controllers/Assets/AssetCheckoutController.php @@ -124,11 +124,16 @@ class AssetCheckoutController extends Controller // 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) - ? ! $settings->null_company_is_floater - : (int) $target->company_id !== (int) $asset->company_id); + if ($target instanceof User) { + // Users may belong to multiple companies; canReceiveFromCompany() checks the pivot. + $mismatch = ! $target->canReceiveFromCompany((int) $asset->company_id); + } elseif (is_null($target->company_id)) { + // Target has no company — only a mismatch when floater mode is off. + $mismatch = ! $settings->null_company_is_floater; + } else { + // Both sides have a company; require an exact match. + $mismatch = (int) $target->company_id !== (int) $asset->company_id; + } if ($mismatch) { $targetType = match (class_basename($target)) { @@ -138,7 +143,7 @@ class AssetCheckoutController extends Controller }; return redirect()->route('hardware.checkout.create', $asset)->with('error', trans('general.error_checkout_company_mismatch', [ - 'item' => trans('general.asset').' "'.($asset->name ?? $asset->asset_tag).'"', + 'item' => trans('general.asset').' "'.$asset->display_name.'"', 'item_company' => $asset->company?->name ?? trans('general.unassigned'), 'target' => $targetType.' "'.($target->name ?? $target->username ?? $target->id).'"', ])); diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 889ed68891..7c6e161da2 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -696,12 +696,19 @@ class BulkAssetsController extends Controller if ($company_ids->isNotEmpty()) { $assetCompanyId = (int) $company_ids->first(); - $mismatch = $company_ids->count() > 1 - || ($target instanceof User - ? ! $target->canReceiveFromCompany($assetCompanyId) - : (is_null($target->company_id) - ? ! Setting::getSettings()->null_company_is_floater - : (int) $target->company_id !== $assetCompanyId)); + if ($company_ids->count() > 1) { + // Selected assets span multiple companies; bulk checkout can't satisfy all of them. + $mismatch = true; + } elseif ($target instanceof User) { + // Users may belong to multiple companies; canReceiveFromCompany() checks the pivot. + $mismatch = ! $target->canReceiveFromCompany($assetCompanyId); + } elseif (is_null($target->company_id)) { + // Target has no company — only a mismatch when floater mode is off. + $mismatch = ! Setting::getSettings()->null_company_is_floater; + } else { + // Both sides have a company; require an exact match. + $mismatch = (int) $target->company_id !== $assetCompanyId; + } if ($mismatch) { $request->session()->flashInput(['selected_assets' => $asset_ids]); diff --git a/app/Http/Controllers/Licenses/LicenseCheckoutController.php b/app/Http/Controllers/Licenses/LicenseCheckoutController.php index fc96b9f74b..edcbde1744 100644 --- a/app/Http/Controllers/Licenses/LicenseCheckoutController.php +++ b/app/Http/Controllers/Licenses/LicenseCheckoutController.php @@ -100,9 +100,13 @@ class LicenseCheckoutController extends Controller if ($request->filled('asset_id')) { $fmcsTarget = Asset::find($request->input('asset_id')); if ($fmcsTarget && $license->company_id) { - $mismatch = is_null($fmcsTarget->company_id) - ? ! Setting::getSettings()->null_company_is_floater - : ($license->company_id !== $fmcsTarget->company_id); + if (is_null($fmcsTarget->company_id)) { + // Target asset has no company — only a mismatch when floater mode is off. + $mismatch = ! Setting::getSettings()->null_company_is_floater; + } else { + // Both sides have a company; require an exact match. + $mismatch = $license->company_id !== $fmcsTarget->company_id; + } if ($mismatch) { return redirect()->route('licenses.index')->with('error', trans('general.error_checkout_company_mismatch', [ 'item' => trans('general.license').' "'.$license->name.'"', diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index da149d91ca..8b25784f38 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -95,7 +95,8 @@ class LocationsController extends Controller if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); // check if parent is set and has a different company - if ($location->parent_id && ($parent = Location::find($location->parent_id)) && $parent->company_id != $location->company_id) { + $parent = $location->parent_id ? Location::find($location->parent_id) : null; + if ($parent && $parent->company_id != $location->company_id) { return redirect()->back()->withInput()->with('error', trans('general.error_location_parent_company', [ 'parent' => $parent->name, 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), @@ -189,7 +190,8 @@ class LocationsController extends Controller ])); } // check if parent is set and has a different company - if ($location->parent_id && ($parent = Location::find($location->parent_id)) && $parent->company_id != $location->company_id) { + $parent = $location->parent_id ? Location::find($location->parent_id) : null; + if ($parent && $parent->company_id != $location->company_id) { return redirect()->back()->withInput()->with('error', trans('general.error_location_parent_company', [ 'parent' => $parent->name, 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), diff --git a/app/Models/Company.php b/app/Models/Company.php index 2b2c01da2e..1466571a81 100644 --- a/app/Models/Company.php +++ b/app/Models/Company.php @@ -416,14 +416,12 @@ final class Company extends SnipeModel }); } - // Floater: also include users with no company associations (they float). + // Floater: also include users with no company associations (they float). They all float down here, Georgie.). if ($floater) { return $query->where(function ($q) use ($companyIds) { $q->whereIn('users.id', function ($sub) use ($companyIds) { $sub->select('user_id')->from('company_user')->whereIn('company_id', $companyIds); - })->orWhereNotIn('users.id', function ($sub) { - $sub->select('user_id')->from('company_user'); - }); + })->orWhereDoesntHave('companies'); }); } @@ -467,6 +465,24 @@ final class Company extends SnipeModel } } + /** + * Scope a users query to those belonging to the given company IDs, respecting floater mode. + * + * Extracted from controller-level inline logic so the same rule is enforced consistently + * everywhere users are filtered by a specific set of company IDs (e.g. select2 dropdowns). + */ + public static function scopeUsersByCompanyIds($query, array $companyIds): mixed + { + if (Setting::getSettings()->null_company_is_floater) { + return $query->where(function ($q) use ($companyIds) { + $q->whereHas('companies', fn($q2) => $q2->whereIn('companies.id', $companyIds)) + ->orWhereDoesntHave('companies'); + }); + } + + return $query->whereHas('companies', fn($q) => $q->whereIn('companies.id', $companyIds)); + } + /** * I legit do not know what this method does, but we can't remove it (yet). * From c14880dfcaa7d95804fe6eba8a0fe95198ac3efa Mon Sep 17 00:00:00 2001 From: snipe Date: Fri, 12 Jun 2026 16:10:29 +0100 Subject: [PATCH 16/24] Oh, pint --- app/Models/Company.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Models/Company.php b/app/Models/Company.php index 1466571a81..c32dd3d222 100644 --- a/app/Models/Company.php +++ b/app/Models/Company.php @@ -475,12 +475,12 @@ final class Company extends SnipeModel { if (Setting::getSettings()->null_company_is_floater) { return $query->where(function ($q) use ($companyIds) { - $q->whereHas('companies', fn($q2) => $q2->whereIn('companies.id', $companyIds)) + $q->whereHas('companies', fn ($q2) => $q2->whereIn('companies.id', $companyIds)) ->orWhereDoesntHave('companies'); }); } - return $query->whereHas('companies', fn($q) => $q->whereIn('companies.id', $companyIds)); + return $query->whereHas('companies', fn ($q) => $q->whereIn('companies.id', $companyIds)); } /** From 8ebddd95ffa823374daa81e7757fb2a779c6463f Mon Sep 17 00:00:00 2001 From: snipe Date: Fri, 12 Jun 2026 16:46:23 +0100 Subject: [PATCH 17/24] FMCS+location scoping - Fixed scope boundaries --- .../Controllers/Api/LocationsController.php | 29 +++++++++++-------- app/Http/Controllers/LocationsController.php | 22 +++++++------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/app/Http/Controllers/Api/LocationsController.php b/app/Http/Controllers/Api/LocationsController.php index bfa27a5a28..8353e00aec 100644 --- a/app/Http/Controllers/Api/LocationsController.php +++ b/app/Http/Controllers/Api/LocationsController.php @@ -211,10 +211,12 @@ class LocationsController extends Controller $location->fill($request->all()); $location = $request->handleImages($location); - // Only scope location if the setting is enabled if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); - // check if parent is set and has a different company + } + + // Parent company check applies whenever FMCS is on, independent of scope_locations_fmcs. + if (Setting::getSettings()->full_multiple_companies_support) { $parent = $location->parent_id ? Location::find($location->parent_id) : null; if ($parent && $parent->company_id != $location->company_id) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_location_parent_company', [ @@ -308,7 +310,6 @@ class LocationsController extends Controller $location = $request->handleImages($location); if ($request->filled('company_id')) { - // Only scope location if the setting is enabled if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); // check if there are related objects with different company @@ -321,20 +322,24 @@ class LocationsController extends Controller 'item_company' => $first[5] ?? trans('general.unassigned'), ]))); } - // check if parent is set and has a different company - $parent = $location->parent_id ? Location::find($location->parent_id) : null; - if ($parent && $parent->company_id != $location->company_id) { - return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_location_parent_company', [ - 'parent' => $parent->name, - 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), - 'location_company' => $location->company?->name ?? trans('general.unassigned'), - ]))); - } } else { $location->company_id = $request->input('company_id'); } } + // Parent company check applies whenever FMCS is on, independent of scope_locations_fmcs. + // Runs outside the company_id gate so a parent_id-only update is also validated. + if (Setting::getSettings()->full_multiple_companies_support) { + $parent = $location->parent_id ? Location::find($location->parent_id) : null; + if ($parent && $parent->company_id != $location->company_id) { + return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_location_parent_company', [ + 'parent' => $parent->name, + 'parent_company' => $parent->company?->name ?? trans('general.unassigned'), + 'location_company' => $location->company?->name ?? trans('general.unassigned'), + ]))); + } + } + if ($location->isValid()) { $location->save(); diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index 8b25784f38..1180a738bd 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -89,12 +89,14 @@ class LocationsController extends Controller $location->fax = request('fax'); $location->tag_color = $request->input('tag_color'); $location->notes = $request->input('notes'); - $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); - - // Only scope the location if the setting is enabled if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); - // check if parent is set and has a different company + } else { + $location->company_id = $request->input('company_id'); + } + + // Parent company check applies whenever FMCS is on, independent of scope_locations_fmcs. + if (Setting::getSettings()->full_multiple_companies_support) { $parent = $location->parent_id ? Location::find($location->parent_id) : null; if ($parent && $parent->company_id != $location->company_id) { return redirect()->back()->withInput()->with('error', trans('general.error_location_parent_company', [ @@ -103,8 +105,6 @@ class LocationsController extends Controller 'location_company' => $location->company?->name ?? trans('general.unassigned'), ])); } - } else { - $location->company_id = $request->input('company_id'); } if ($request->has('use_cloned_image')) { @@ -176,7 +176,6 @@ class LocationsController extends Controller $location->tag_color = $request->input('tag_color'); $location->notes = $request->input('notes'); - // Only scope the location if the setting is enabled if (Setting::getSettings()->scope_locations_fmcs) { $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); // check if there are related objects with different company @@ -189,7 +188,12 @@ class LocationsController extends Controller 'item_company' => $first[5] ?? trans('general.unassigned'), ])); } - // check if parent is set and has a different company + } else { + $location->company_id = $request->input('company_id'); + } + + // Parent company check applies whenever FMCS is on, independent of scope_locations_fmcs. + if (Setting::getSettings()->full_multiple_companies_support) { $parent = $location->parent_id ? Location::find($location->parent_id) : null; if ($parent && $parent->company_id != $location->company_id) { return redirect()->back()->withInput()->with('error', trans('general.error_location_parent_company', [ @@ -198,8 +202,6 @@ class LocationsController extends Controller 'location_company' => $location->company?->name ?? trans('general.unassigned'), ])); } - } else { - $location->company_id = $request->input('company_id'); } $location = $request->handleImages($location); From c8ae09cd43199f2bcf940065751b0c6fe11f8d25 Mon Sep 17 00:00:00 2001 From: snipe Date: Fri, 12 Jun 2026 17:26:57 +0100 Subject: [PATCH 18/24] FMCS location scoping - Added tests --- .../Api/LocationParentCompanyTest.php | 174 ++++++++++++++++++ .../Ui/LocationParentCompanyTest.php | 152 +++++++++++++++ 2 files changed, 326 insertions(+) create mode 100644 tests/Feature/Locations/Api/LocationParentCompanyTest.php create mode 100644 tests/Feature/Locations/Ui/LocationParentCompanyTest.php diff --git a/tests/Feature/Locations/Api/LocationParentCompanyTest.php b/tests/Feature/Locations/Api/LocationParentCompanyTest.php new file mode 100644 index 0000000000..94db08c976 --- /dev/null +++ b/tests/Feature/Locations/Api/LocationParentCompanyTest.php @@ -0,0 +1,174 @@ +settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + + $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.locations.store'), [ + 'name' => 'Location B', + 'company_id' => $globex->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertOk() + ->assertStatusMessageIs('error'); + + $this->assertFalse(Location::where('name', 'Location B')->exists()); + } + + public function test_can_create_location_with_same_company_parent_when_fmcs_enabled() + { + $this->settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + + $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.locations.store'), [ + 'name' => 'Location B', + 'company_id' => $acme->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertOk() + ->assertStatusMessageIs('success'); + + $this->assertTrue(Location::where('name', 'Location B')->exists()); + } + + public function test_can_create_location_with_cross_company_parent_when_fmcs_disabled() + { + $this->settings->disableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + + $this->actingAsForApi(User::factory()->superuser()->create()) + ->postJson(route('api.locations.store'), [ + 'name' => 'Location B', + 'company_id' => $globex->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertOk() + ->assertStatusMessageIs('success'); + + $this->assertTrue(Location::where('name', 'Location B')->exists()); + } + + // ----------------------------------------------------------------------- + // update + // ----------------------------------------------------------------------- + + public function test_cannot_update_location_to_cross_company_parent_when_fmcs_enabled() + { + $this->settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + $location = Location::factory()->create(['name' => 'Location B', 'company_id' => $globex->id]); + + $this->actingAsForApi(User::factory()->superuser()->create()) + ->patchJson(route('api.locations.update', $location), [ + 'name' => 'Location B', + 'company_id' => $globex->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertOk() + ->assertStatusMessageIs('error'); + + $this->assertNull($location->fresh()->parent_id); + } + + public function test_cannot_update_location_parent_id_only_to_cross_company_parent_when_fmcs_enabled() + { + // Ensures the check fires even when company_id is not included in the request. + $this->settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + $location = Location::factory()->create(['name' => 'Location B', 'company_id' => $globex->id]); + + $this->actingAsForApi(User::factory()->superuser()->create()) + ->patchJson(route('api.locations.update', $location), [ + 'parent_id' => $parentLocation->id, + ]) + ->assertOk() + ->assertStatusMessageIs('error'); + + $this->assertNull($location->fresh()->parent_id); + } + + public function test_can_update_location_to_same_company_parent_when_fmcs_enabled() + { + $this->settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + $location = Location::factory()->create(['name' => 'Location B', 'company_id' => $acme->id]); + + $this->actingAsForApi(User::factory()->superuser()->create()) + ->patchJson(route('api.locations.update', $location), [ + 'name' => 'Location B', + 'company_id' => $acme->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertOk() + ->assertStatusMessageIs('success'); + + $this->assertEquals($parentLocation->id, $location->fresh()->parent_id); + } + + public function test_can_update_location_to_cross_company_parent_when_fmcs_disabled() + { + $this->settings->disableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + $location = Location::factory()->create(['name' => 'Location B', 'company_id' => $globex->id]); + + $this->actingAsForApi(User::factory()->superuser()->create()) + ->patchJson(route('api.locations.update', $location), [ + 'name' => 'Location B', + 'company_id' => $globex->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertOk() + ->assertStatusMessageIs('success'); + + $this->assertEquals($parentLocation->id, $location->fresh()->parent_id); + } +} diff --git a/tests/Feature/Locations/Ui/LocationParentCompanyTest.php b/tests/Feature/Locations/Ui/LocationParentCompanyTest.php new file mode 100644 index 0000000000..351e36a243 --- /dev/null +++ b/tests/Feature/Locations/Ui/LocationParentCompanyTest.php @@ -0,0 +1,152 @@ +settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + + $this->actingAs(User::factory()->superuser()->create()) + ->from(route('locations.create')) + ->post(route('locations.store'), [ + 'name' => 'Location B', + 'company_id' => $globex->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertRedirect(route('locations.create')) + ->assertSessionHas('error'); + + $this->assertFalse(Location::where('name', 'Location B')->exists()); + } + + public function test_can_create_location_with_same_company_parent_when_fmcs_enabled() + { + $this->settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('locations.store'), [ + 'name' => 'Location B', + 'company_id' => $acme->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertRedirect(route('locations.index')) + ->assertSessionHasNoErrors(); + + $this->assertTrue(Location::where('name', 'Location B')->exists()); + } + + public function test_can_create_location_with_cross_company_parent_when_fmcs_disabled() + { + $this->settings->disableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + + $this->actingAs(User::factory()->superuser()->create()) + ->post(route('locations.store'), [ + 'name' => 'Location B', + 'company_id' => $globex->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertRedirect(route('locations.index')) + ->assertSessionHasNoErrors(); + + $this->assertTrue(Location::where('name', 'Location B')->exists()); + } + + // ----------------------------------------------------------------------- + // update (edit) + // ----------------------------------------------------------------------- + + public function test_cannot_update_location_to_cross_company_parent_when_fmcs_enabled() + { + $this->settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + $location = Location::factory()->create(['name' => 'Location B', 'company_id' => $globex->id]); + + $this->actingAs(User::factory()->superuser()->create()) + ->from(route('locations.edit', $location)) + ->put(route('locations.update', $location), [ + 'name' => 'Location B', + 'company_id' => $globex->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertRedirect(route('locations.edit', $location)) + ->assertSessionHas('error'); + + $this->assertNull($location->fresh()->parent_id); + } + + public function test_can_update_location_to_same_company_parent_when_fmcs_enabled() + { + $this->settings->enableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + $location = Location::factory()->create(['name' => 'Location B', 'company_id' => $acme->id]); + + $this->actingAs(User::factory()->superuser()->create()) + ->put(route('locations.update', $location), [ + 'name' => 'Location B', + 'company_id' => $acme->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertRedirect(route('locations.index')) + ->assertSessionHasNoErrors(); + + $this->assertEquals($parentLocation->id, $location->fresh()->parent_id); + } + + public function test_can_update_location_to_cross_company_parent_when_fmcs_disabled() + { + $this->settings->disableMultipleFullCompanySupport(); + + $acme = Company::factory()->create(['name' => 'Acme']); + $globex = Company::factory()->create(['name' => 'Globex']); + + $parentLocation = Location::factory()->create(['company_id' => $acme->id]); + $location = Location::factory()->create(['name' => 'Location B', 'company_id' => $globex->id]); + + $this->actingAs(User::factory()->superuser()->create()) + ->put(route('locations.update', $location), [ + 'name' => 'Location B', + 'company_id' => $globex->id, + 'parent_id' => $parentLocation->id, + ]) + ->assertRedirect(route('locations.index')) + ->assertSessionHasNoErrors(); + + $this->assertEquals($parentLocation->id, $location->fresh()->parent_id); + } +} From 8d0a6af2aa57d794b574c83dd3e0ef592c167ec5 Mon Sep 17 00:00:00 2001 From: snipe Date: Sat, 13 Jun 2026 12:35:48 +0100 Subject: [PATCH 19/24] Refactor into the Companyable trait --- .../Controllers/Components/ComponentCheckoutController.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Components/ComponentCheckoutController.php b/app/Http/Controllers/Components/ComponentCheckoutController.php index 83986cd497..90c0bc309b 100644 --- a/app/Http/Controllers/Components/ComponentCheckoutController.php +++ b/app/Http/Controllers/Components/ComponentCheckoutController.php @@ -7,7 +7,6 @@ use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Models\Asset; use App\Models\Component; -use App\Models\Setting; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Contracts\View\View; use Illuminate\Http\RedirectResponse; @@ -104,11 +103,11 @@ class ComponentCheckoutController extends Controller // Check if the asset exists $asset = Asset::find($request->input('asset_id')); - if ((Setting::getSettings()->full_multiple_companies_support) && $component->company_id !== $asset->company_id) { + if (! $component->canCheckoutTo($asset)) { return redirect()->route('components.checkout.show', $componentId)->with('error', trans('general.error_checkout_company_mismatch', [ 'item' => trans('general.component').' "'.$component->name.'"', 'item_company' => $component->company?->name ?? trans('general.unassigned'), - 'target' => trans('general.asset').' "'.($asset->name ?? $asset->asset_tag).'"', + 'target' => trans('general.asset').' "'.$asset->display_name.'"', ])); } From 2033f2538675ea01ae319517507a61735b87efae Mon Sep 17 00:00:00 2001 From: snipe Date: Sat, 13 Jun 2026 12:36:15 +0100 Subject: [PATCH 20/24] FMCS/Floater: Refactor logic into the companyable trait --- .../AccessoryCheckoutController.php | 36 +++++------------ .../Assets/AssetCheckoutController.php | 40 +++++-------------- .../ConsumableCheckoutController.php | 7 +--- .../Licenses/LicenseCheckoutController.php | 23 ++++------- app/Models/Traits/CompanyableTrait.php | 36 +++++++++++++++++ 5 files changed, 66 insertions(+), 76 deletions(-) diff --git a/app/Http/Controllers/Accessories/AccessoryCheckoutController.php b/app/Http/Controllers/Accessories/AccessoryCheckoutController.php index 3f0bc8054a..f1e3502f11 100644 --- a/app/Http/Controllers/Accessories/AccessoryCheckoutController.php +++ b/app/Http/Controllers/Accessories/AccessoryCheckoutController.php @@ -10,7 +10,6 @@ use App\Http\Traits\CheckInOutTrait; use App\Models\Accessory; use App\Models\AccessoryCheckout; use App\Models\CheckoutAcceptance; -use App\Models\Setting; use App\Models\User; use Carbon\Carbon; use Illuminate\Contracts\View\View; @@ -67,31 +66,18 @@ class AccessoryCheckoutController extends Controller $target = $this->determineCheckoutTarget(); session()->put(['checkout_to_type' => $target]); - if (Setting::getSettings()->full_multiple_companies_support == '1' && $accessory->company_id) { - if ($target instanceof User) { - // Users may belong to multiple companies; canReceiveFromCompany() checks the pivot. - $mismatch = ! $target->canReceiveFromCompany($accessory->company_id); - } elseif (is_null($target->company_id)) { - // Target has no company — only a mismatch when floater mode is off. - $mismatch = ! Setting::getSettings()->null_company_is_floater; - } else { - // Both sides have a company; require an exact match. - $mismatch = (int) $target->company_id !== (int) $accessory->company_id; - } + if (! $accessory->canCheckoutTo($target)) { + $targetType = match (class_basename($target)) { + 'User' => trans('general.user'), + 'Location' => trans('general.location'), + default => trans('general.asset'), + }; - if ($mismatch) { - $targetType = match (class_basename($target)) { - 'User' => trans('general.user'), - 'Location' => trans('general.location'), - default => trans('general.asset'), - }; - - return redirect()->back()->with('error', trans('general.error_checkout_company_mismatch', [ - 'item' => trans('general.accessory').' "'.$accessory->name.'"', - 'item_company' => $accessory->company?->name ?? trans('general.unassigned'), - 'target' => $targetType.' "'.($target->name ?? $target->username ?? $target->id).'"', - ])); - } + return redirect()->back()->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.accessory').' "'.$accessory->name.'"', + 'item_company' => $accessory->company?->name ?? trans('general.unassigned'), + 'target' => $targetType.' "'.($target->name ?? $target->username ?? $target->id).'"', + ])); } $accessory->checkout_qty = $request->input('checkout_qty', 1); diff --git a/app/Http/Controllers/Assets/AssetCheckoutController.php b/app/Http/Controllers/Assets/AssetCheckoutController.php index 86b9b9897e..0c8e0d7f2e 100644 --- a/app/Http/Controllers/Assets/AssetCheckoutController.php +++ b/app/Http/Controllers/Assets/AssetCheckoutController.php @@ -9,7 +9,6 @@ use App\Http\Requests\AssetCheckoutRequest; use App\Http\Traits\CheckInOutTrait; use App\Models\Asset; use App\Models\CheckoutAcceptance; -use App\Models\Setting; use App\Models\User; use Illuminate\Contracts\View\View; use Illuminate\Database\Eloquent\ModelNotFoundException; @@ -119,35 +118,18 @@ class AssetCheckoutController extends Controller // Add any custom fields that should be included in the checkout $asset->customFieldsForCheckinCheckout('display_checkout'); - $settings = Setting::getSettings(); + if (! $asset->canCheckoutTo($target)) { + $targetType = match (class_basename($target)) { + 'User' => trans('general.user'), + 'Location' => trans('general.location'), + default => trans('general.asset'), + }; - // 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)) { - if ($target instanceof User) { - // Users may belong to multiple companies; canReceiveFromCompany() checks the pivot. - $mismatch = ! $target->canReceiveFromCompany((int) $asset->company_id); - } elseif (is_null($target->company_id)) { - // Target has no company — only a mismatch when floater mode is off. - $mismatch = ! $settings->null_company_is_floater; - } else { - // Both sides have a company; require an exact match. - $mismatch = (int) $target->company_id !== (int) $asset->company_id; - } - - if ($mismatch) { - $targetType = match (class_basename($target)) { - 'User' => trans('general.user'), - 'Location' => trans('general.location'), - default => trans('general.asset'), - }; - - return redirect()->route('hardware.checkout.create', $asset)->with('error', trans('general.error_checkout_company_mismatch', [ - 'item' => trans('general.asset').' "'.$asset->display_name.'"', - 'item_company' => $asset->company?->name ?? trans('general.unassigned'), - 'target' => $targetType.' "'.($target->name ?? $target->username ?? $target->id).'"', - ])); - } + return redirect()->route('hardware.checkout.create', $asset)->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.asset').' "'.$asset->display_name.'"', + 'item_company' => $asset->company?->name ?? trans('general.unassigned'), + 'target' => $targetType.' "'.($target->name ?? $target->username ?? $target->id).'"', + ])); } session()->put([ diff --git a/app/Http/Controllers/Consumables/ConsumableCheckoutController.php b/app/Http/Controllers/Consumables/ConsumableCheckoutController.php index 9540a8b0ac..018655fe7c 100644 --- a/app/Http/Controllers/Consumables/ConsumableCheckoutController.php +++ b/app/Http/Controllers/Consumables/ConsumableCheckoutController.php @@ -7,7 +7,6 @@ use App\Helpers\Helper; use App\Http\Controllers\Controller; use App\Models\CheckoutAcceptance; use App\Models\Consumable; -use App\Models\Setting; use App\Models\User; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Contracts\View\View; @@ -97,11 +96,7 @@ class ConsumableCheckoutController extends Controller return redirect()->route('consumables.checkout.show', $consumable)->with('error', trans('admin/consumables/message.checkout.user_does_not_exist'))->withInput(); } - if ( - Setting::getSettings()->full_multiple_companies_support == '1' - && $consumable->company_id - && ! $user->canReceiveFromCompany($consumable->company_id) - ) { + if (! $consumable->canCheckoutTo($user)) { return redirect()->back()->with('error', trans('general.error_checkout_company_mismatch', [ 'item' => trans('general.consumable').' "'.$consumable->name.'"', 'item_company' => $consumable->company?->name ?? trans('general.unassigned'), diff --git a/app/Http/Controllers/Licenses/LicenseCheckoutController.php b/app/Http/Controllers/Licenses/LicenseCheckoutController.php index edcbde1744..54bcef42f8 100644 --- a/app/Http/Controllers/Licenses/LicenseCheckoutController.php +++ b/app/Http/Controllers/Licenses/LicenseCheckoutController.php @@ -99,25 +99,16 @@ class LicenseCheckoutController extends Controller if (Setting::getSettings()->full_multiple_companies_support == '1') { if ($request->filled('asset_id')) { $fmcsTarget = Asset::find($request->input('asset_id')); - if ($fmcsTarget && $license->company_id) { - if (is_null($fmcsTarget->company_id)) { - // Target asset has no company — only a mismatch when floater mode is off. - $mismatch = ! Setting::getSettings()->null_company_is_floater; - } else { - // Both sides have a company; require an exact match. - $mismatch = $license->company_id !== $fmcsTarget->company_id; - } - if ($mismatch) { - return redirect()->route('licenses.index')->with('error', trans('general.error_checkout_company_mismatch', [ - 'item' => trans('general.license').' "'.$license->name.'"', - 'item_company' => $license->company?->name ?? trans('general.unassigned'), - 'target' => trans('general.asset').' "'.($fmcsTarget->name ?? $fmcsTarget->asset_tag).'"', - ])); - } + if ($fmcsTarget && ! $license->canCheckoutTo($fmcsTarget)) { + return redirect()->route('licenses.index')->with('error', trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.license').' "'.$license->name.'"', + 'item_company' => $license->company?->name ?? trans('general.unassigned'), + 'target' => trans('general.asset').' "'.$fmcsTarget->display_name.'"', + ])); } } elseif ($request->filled('assigned_to')) { $fmcsTarget = User::find($request->input('assigned_to')); - if ($fmcsTarget && $license->company_id && ! $fmcsTarget->canReceiveFromCompany($license->company_id)) { + if ($fmcsTarget && ! $license->canCheckoutTo($fmcsTarget)) { return redirect()->route('licenses.index')->with('error', trans('general.error_checkout_company_mismatch', [ 'item' => trans('general.license').' "'.$license->name.'"', 'item_company' => $license->company?->name ?? trans('general.unassigned'), diff --git a/app/Models/Traits/CompanyableTrait.php b/app/Models/Traits/CompanyableTrait.php index 4d891d05d3..c2da82ce54 100644 --- a/app/Models/Traits/CompanyableTrait.php +++ b/app/Models/Traits/CompanyableTrait.php @@ -3,6 +3,9 @@ namespace App\Models\Traits; use App\Models\CompanyableScope; +use App\Models\Setting; +use App\Models\User; +use Illuminate\Database\Eloquent\Model; trait CompanyableTrait { @@ -18,4 +21,37 @@ trait CompanyableTrait { static::addGlobalScope(new CompanyableScope); } + + /** + * Whether this item may be checked out to the given target under FMCS rules. + * + * Returns true when: + * - FMCS is disabled, OR + * - this item has no company (uncompanied items are unrestricted), OR + * - target is a User whose company pivot includes this item's company, OR + * - target has no company and null_company_is_floater is enabled, OR + * - target's company_id exactly matches this item's company_id. + */ + public function canCheckoutTo(Model $target): bool + { + $settings = Setting::getSettings(); + + if (! $settings->full_multiple_companies_support) { + return true; + } + + if (! $this->company_id) { + return true; + } + + if ($target instanceof User) { + return $target->canReceiveFromCompany((int) $this->company_id); + } + + if (is_null($target->company_id)) { + return (bool) $settings->null_company_is_floater; + } + + return (int) $target->company_id === (int) $this->company_id; + } } From 73f72cbbb0df36b876b4f98de2a412f206ec34f2 Mon Sep 17 00:00:00 2001 From: snipe Date: Sat, 13 Jun 2026 12:42:48 +0100 Subject: [PATCH 21/24] Use the new companyable trait in the bulk assets controller --- .../Controllers/Assets/BulkAssetsController.php | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 7c6e161da2..33649e8feb 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -689,25 +689,16 @@ class BulkAssetsController extends Controller } // 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 ($company_ids->isNotEmpty()) { - $assetCompanyId = (int) $company_ids->first(); - if ($company_ids->count() > 1) { // Selected assets span multiple companies; bulk checkout can't satisfy all of them. $mismatch = true; - } elseif ($target instanceof User) { - // Users may belong to multiple companies; canReceiveFromCompany() checks the pivot. - $mismatch = ! $target->canReceiveFromCompany($assetCompanyId); - } elseif (is_null($target->company_id)) { - // Target has no company — only a mismatch when floater mode is off. - $mismatch = ! Setting::getSettings()->null_company_is_floater; } else { - // Both sides have a company; require an exact match. - $mismatch = (int) $target->company_id !== $assetCompanyId; + // All assets share the same company; let the model enforce the checkout rules. + $mismatch = ! $assets->first()->canCheckoutTo($target); } if ($mismatch) { From 75f86cd669c199e22f1f014124780f395d9740e6 Mon Sep 17 00:00:00 2001 From: snipe Date: Sat, 13 Jun 2026 12:55:10 +0100 Subject: [PATCH 22/24] FMCS+location+floater+importer: handle the importer as well --- app/Importer/AssetImporter.php | 25 +++++++++++++++-------- app/Importer/ComponentImporter.php | 22 +++++++++++++------- app/Importer/LicenseImporter.php | 32 ++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/app/Importer/AssetImporter.php b/app/Importer/AssetImporter.php index e56ebde78a..bd8cce65a7 100644 --- a/app/Importer/AssetImporter.php +++ b/app/Importer/AssetImporter.php @@ -214,16 +214,25 @@ class AssetImporter extends ItemImporter // -- the class that needs to use it (command importer or GUI importer inside the project). if (isset($target) && ($target !== false)) { $asset = $asset->fresh(); - $targetType = get_class($target); - $alreadyCheckedOutToTarget = ($asset->assigned_to == $target->id) && ($asset->assigned_type === $targetType); - // Skip duplicate checkout noise when update mode keeps the same assignment target. - if (! $alreadyCheckedOutToTarget) { - if (! is_null($asset->assigned_to)) { - event(new CheckoutableCheckedIn($asset, $asset->assigned, auth()->user(), 'Checkin from CSV Importer', $checkin_date)); + if (! $asset->canCheckoutTo($target)) { + $this->log(trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.asset').' "'.$asset->display_name.'"', + 'item_company' => $asset->company?->name ?? trans('general.unassigned'), + 'target' => ($target->name ?? $target->username ?? $target->id), + ])); + } else { + $targetType = get_class($target); + $alreadyCheckedOutToTarget = ($asset->assigned_to == $target->id) && ($asset->assigned_type === $targetType); + + // Skip duplicate checkout noise when update mode keeps the same assignment target. + if (! $alreadyCheckedOutToTarget) { + if (! is_null($asset->assigned_to)) { + event(new CheckoutableCheckedIn($asset, $asset->assigned, auth()->user(), 'Checkin from CSV Importer', $checkin_date)); + } + + $asset->checkOut($target, $this->created_by, $checkout_date, null, 'Checkout from CSV Importer', $asset->name); } - - $asset->checkOut($target, $this->created_by, $checkout_date, null, 'Checkout from CSV Importer', $asset->name); } } diff --git a/app/Importer/ComponentImporter.php b/app/Importer/ComponentImporter.php index 5066c8eb2b..0145997745 100644 --- a/app/Importer/ComponentImporter.php +++ b/app/Importer/ComponentImporter.php @@ -59,13 +59,21 @@ class ComponentImporter extends ItemImporter // If we have an asset tag, checkout to that asset. if (isset($this->item['asset_tag']) && ($asset = Asset::where('asset_tag', $this->item['asset_tag'])->first())) { - $component->assets()->attach($component->id, [ - 'component_id' => $component->id, - 'created_by' => auth()->id(), - 'created_at' => date('Y-m-d H:i:s'), - 'assigned_qty' => 1, // Only assign the first one to the asset - 'asset_id' => $asset->id, - ]); + if (! $component->canCheckoutTo($asset)) { + $this->log(trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.component').' "'.$component->name.'"', + 'item_company' => $component->company?->name ?? trans('general.unassigned'), + 'target' => trans('general.asset').' "'.$asset->display_name.'"', + ])); + } else { + $component->assets()->attach($component->id, [ + 'component_id' => $component->id, + 'created_by' => auth()->id(), + 'created_at' => date('Y-m-d H:i:s'), + 'assigned_qty' => 1, // Only assign the first one to the asset + 'asset_id' => $asset->id, + ]); + } } return; diff --git a/app/Importer/LicenseImporter.php b/app/Importer/LicenseImporter.php index 101adc3b41..d637d9084e 100644 --- a/app/Importer/LicenseImporter.php +++ b/app/Importer/LicenseImporter.php @@ -106,16 +106,32 @@ class LicenseImporter extends ItemImporter } if ($checkout_target) { - $targetLicense->assigned_to = $checkout_target->id; - $targetLicense->created_by = auth()->id(); - if ($asset) { - $targetLicense->asset_id = $asset->id; + if (! $license->canCheckoutTo($checkout_target)) { + $this->log(trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.license').' "'.$license->name.'"', + 'item_company' => $license->company?->name ?? trans('general.unassigned'), + 'target' => ($checkout_target->name ?? $checkout_target->username ?? $checkout_target->id), + ])); + } else { + $targetLicense->assigned_to = $checkout_target->id; + $targetLicense->created_by = auth()->id(); + if ($asset) { + $targetLicense->asset_id = $asset->id; + } + $targetLicense->save(); } - $targetLicense->save(); } elseif ($asset) { - $targetLicense->created_by = auth()->id(); - $targetLicense->asset_id = $asset->id; - $targetLicense->save(); + if (! $license->canCheckoutTo($asset)) { + $this->log(trans('general.error_checkout_company_mismatch', [ + 'item' => trans('general.license').' "'.$license->name.'"', + 'item_company' => $license->company?->name ?? trans('general.unassigned'), + 'target' => trans('general.asset').' "'.$asset->display_name.'"', + ])); + } else { + $targetLicense->created_by = auth()->id(); + $targetLicense->asset_id = $asset->id; + $targetLicense->save(); + } } } From e3a9872d286651181c34e8e9eeb1ca58b4085c8e Mon Sep 17 00:00:00 2001 From: snipe Date: Sat, 13 Jun 2026 13:18:37 +0100 Subject: [PATCH 23/24] Updated tests --- app/Importer/ItemImporter.php | 1 + .../Importing/Api/ImportAssetsTest.php | 85 +++++++++++++++++ .../Importing/Api/ImportComponentsTest.php | 86 +++++++++++++++++ .../Importing/Api/ImportLicenseTest.php | 94 +++++++++++++++++++ .../Importing/ComponentsImportFileBuilder.php | 2 + .../Importing/LicensesImportFileBuilder.php | 6 ++ 6 files changed, 274 insertions(+) diff --git a/app/Importer/ItemImporter.php b/app/Importer/ItemImporter.php index 5cfa8973ca..dc19cf076f 100644 --- a/app/Importer/ItemImporter.php +++ b/app/Importer/ItemImporter.php @@ -82,6 +82,7 @@ class ItemImporter extends Importer $this->item['qty'] = $this->findCsvMatch($row, 'quantity'); $this->item['requestable'] = $this->findCsvMatch($row, 'requestable'); $this->item['created_by'] = auth()->id(); + $this->item['asset_tag'] = $this->findCsvMatch($row, 'asset_tag'); $this->item['serial'] = $this->findCsvMatch($row, 'serial'); $this->item['item_no'] = trim($this->findCsvMatch($row, 'item_no')); diff --git a/tests/Feature/Importing/Api/ImportAssetsTest.php b/tests/Feature/Importing/Api/ImportAssetsTest.php index 3f88f33e3c..54e95293e2 100644 --- a/tests/Feature/Importing/Api/ImportAssetsTest.php +++ b/tests/Feature/Importing/Api/ImportAssetsTest.php @@ -4,6 +4,7 @@ namespace Tests\Feature\Importing\Api; use App\Models\Actionlog as ActionLog; use App\Models\Asset; +use App\Models\Company; use App\Models\CustomField; use App\Models\Import; use App\Models\User; @@ -641,4 +642,88 @@ class ImportAssetsTest extends ImportDataTestCase implements TestsPermissionsReq $this->assertNotEquals($encryptedMacAddress, $macAddress); } + + #[Test] + public function import_asset_checkout_is_blocked_when_fmcs_companies_differ(): void + { + [$companyA, $companyB] = Company::factory()->count(2)->create(); + $user = User::factory()->for($companyB)->create(); + $this->settings->enableMultipleFullCompanySupport(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $companyA->name, + 'assigneeUsername' => $user->username, + ]); + + $import = Import::factory()->asset()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $newAsset = Asset::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $this->assertNull($newAsset->assigned_to, 'Asset should not be checked out when item and user companies differ under FMCS'); + } + + #[Test] + public function import_asset_checkout_is_allowed_when_fmcs_companies_match(): void + { + $company = Company::factory()->create(); + $user = User::factory()->for($company)->create(); + $this->settings->enableMultipleFullCompanySupport(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'assigneeUsername' => $user->username, + ]); + + $import = Import::factory()->asset()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $newAsset = Asset::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $this->assertEquals($user->id, $newAsset->assigned_to, 'Asset should be checked out when companies match under FMCS'); + } + + #[Test] + public function import_asset_checkout_is_blocked_when_floater_disabled_and_user_has_no_company(): void + { + $company = Company::factory()->create(); + $user = User::factory()->create(['company_id' => null]); + $this->settings->enableMultipleFullCompanySupport()->disableFloaterMode(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'assigneeUsername' => $user->username, + ]); + + $import = Import::factory()->asset()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $newAsset = Asset::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $this->assertNull($newAsset->assigned_to, 'Asset should not be checked out to a no-company user when floater mode is off'); + } + + #[Test] + public function import_asset_checkout_is_allowed_when_floater_enabled_and_user_has_no_company(): void + { + $company = Company::factory()->create(); + $user = User::factory()->create(['company_id' => null]); + $this->settings->enableFloaterMode(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'assigneeUsername' => $user->username, + ]); + + $import = Import::factory()->asset()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $newAsset = Asset::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $this->assertEquals($user->id, $newAsset->assigned_to, 'Asset should be checked out to a no-company user when floater mode is on'); + } } diff --git a/tests/Feature/Importing/Api/ImportComponentsTest.php b/tests/Feature/Importing/Api/ImportComponentsTest.php index 9364ab3ca8..8c481f55a9 100644 --- a/tests/Feature/Importing/Api/ImportComponentsTest.php +++ b/tests/Feature/Importing/Api/ImportComponentsTest.php @@ -3,6 +3,8 @@ namespace Tests\Feature\Importing\Api; use App\Models\Actionlog as ActionLog; +use App\Models\Asset; +use App\Models\Company; use App\Models\Component; use App\Models\Import; use App\Models\User; @@ -348,4 +350,88 @@ class ImportComponentsTest extends ImportDataTestCase implements TestsPermission $this->assertNull($newComponent->image); $this->assertNull($newComponent->notes); } + + #[Test] + public function import_component_checkout_to_asset_is_blocked_when_fmcs_companies_differ(): void + { + [$companyA, $companyB] = Company::factory()->count(2)->create(); + $asset = Asset::factory()->for($companyB)->create(); + $this->settings->enableMultipleFullCompanySupport(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $companyA->name, + 'assetTag' => $asset->asset_tag, + ]); + + $import = Import::factory()->component()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $newComponent = Component::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $this->assertEquals(0, $newComponent->assets()->count(), 'Component should not be checked out when item and asset companies differ under FMCS'); + } + + #[Test] + public function import_component_checkout_to_asset_is_allowed_when_fmcs_companies_match(): void + { + $company = Company::factory()->create(); + $asset = Asset::factory()->for($company)->create(); + $this->settings->enableMultipleFullCompanySupport(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'assetTag' => $asset->asset_tag, + ]); + + $import = Import::factory()->component()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $newComponent = Component::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $this->assertEquals(1, $newComponent->assets()->count(), 'Component should be checked out when companies match under FMCS'); + } + + #[Test] + public function import_component_checkout_to_asset_is_blocked_when_floater_disabled_and_asset_has_no_company(): void + { + $company = Company::factory()->create(); + $asset = Asset::factory()->create(['company_id' => null]); + $this->settings->enableMultipleFullCompanySupport()->disableFloaterMode(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'assetTag' => $asset->asset_tag, + ]); + + $import = Import::factory()->component()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $newComponent = Component::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $this->assertEquals(0, $newComponent->assets()->count(), 'Component should not be checked out to a no-company asset when floater mode is off'); + } + + #[Test] + public function import_component_checkout_to_asset_is_allowed_when_floater_enabled_and_asset_has_no_company(): void + { + $company = Company::factory()->create(); + $asset = Asset::factory()->create(['company_id' => null]); + $this->settings->enableFloaterMode(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'assetTag' => $asset->asset_tag, + ]); + + $import = Import::factory()->component()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $newComponent = Component::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $this->assertEquals(1, $newComponent->assets()->count(), 'Component should be checked out to a no-company asset when floater mode is on'); + } } diff --git a/tests/Feature/Importing/Api/ImportLicenseTest.php b/tests/Feature/Importing/Api/ImportLicenseTest.php index 8699689660..8dac1de093 100644 --- a/tests/Feature/Importing/Api/ImportLicenseTest.php +++ b/tests/Feature/Importing/Api/ImportLicenseTest.php @@ -3,8 +3,10 @@ namespace Tests\Feature\Importing\Api; use App\Models\Actionlog as ActivityLog; +use App\Models\Company; use App\Models\Import; use App\Models\License; +use App\Models\LicenseSeat; use App\Models\User; use Illuminate\Foundation\Testing\WithFaker; use Illuminate\Support\Str; @@ -399,4 +401,96 @@ class ImportLicenseTest extends ImportDataTestCase implements TestsPermissionsRe $this->assertNull($newLicense->deprecate); $this->assertNull($newLicense->min_amt); } + + #[Test] + public function import_license_checkout_is_blocked_when_fmcs_companies_differ(): void + { + [$companyA, $companyB] = Company::factory()->count(2)->create(); + $user = User::factory()->for($companyB)->create(); + $this->settings->enableMultipleFullCompanySupport(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $companyA->name, + 'checkoutUsername' => $user->username, + 'seats' => 5, + ]); + + $import = Import::factory()->license()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $license = License::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $checkedOutSeat = LicenseSeat::where('license_id', $license->id)->whereNotNull('assigned_to')->first(); + $this->assertNull($checkedOutSeat, 'License seat should not be checked out when item and user companies differ under FMCS'); + } + + #[Test] + public function import_license_checkout_is_allowed_when_fmcs_companies_match(): void + { + $company = Company::factory()->create(); + $user = User::factory()->for($company)->create(); + $this->settings->enableMultipleFullCompanySupport(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'checkoutUsername' => $user->username, + 'seats' => 5, + ]); + + $import = Import::factory()->license()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $license = License::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $checkedOutSeat = LicenseSeat::where('license_id', $license->id)->where('assigned_to', $user->id)->first(); + $this->assertNotNull($checkedOutSeat, 'License seat should be checked out when companies match under FMCS'); + } + + #[Test] + public function import_license_checkout_is_blocked_when_floater_disabled_and_user_has_no_company(): void + { + $company = Company::factory()->create(); + $user = User::factory()->create(['company_id' => null]); + $this->settings->enableMultipleFullCompanySupport()->disableFloaterMode(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'checkoutUsername' => $user->username, + 'seats' => 5, + ]); + + $import = Import::factory()->license()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $license = License::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $checkedOutSeat = LicenseSeat::where('license_id', $license->id)->whereNotNull('assigned_to')->first(); + $this->assertNull($checkedOutSeat, 'License seat should not be checked out to a no-company user when floater mode is off'); + } + + #[Test] + public function import_license_checkout_is_allowed_when_floater_enabled_and_user_has_no_company(): void + { + $company = Company::factory()->create(); + $user = User::factory()->create(['company_id' => null]); + $this->settings->enableFloaterMode(); + + $importFileBuilder = ImportFileBuilder::new([ + 'companyName' => $company->name, + 'checkoutUsername' => $user->username, + 'seats' => 5, + ]); + + $import = Import::factory()->license()->create(['file_path' => $importFileBuilder->saveToImportsDirectory()]); + + $this->actingAsForApi(User::factory()->superuser()->create()); + $this->importFileResponse(['import' => $import->id])->assertOk(); + + $license = License::where('serial', $importFileBuilder->firstRow()['serialNumber'])->sole(); + $checkedOutSeat = LicenseSeat::where('license_id', $license->id)->where('assigned_to', $user->id)->first(); + $this->assertNotNull($checkedOutSeat, 'License seat should be checked out to a no-company user when floater mode is on'); + } } diff --git a/tests/Support/Importing/ComponentsImportFileBuilder.php b/tests/Support/Importing/ComponentsImportFileBuilder.php index afc4c0e8a5..68ad054162 100644 --- a/tests/Support/Importing/ComponentsImportFileBuilder.php +++ b/tests/Support/Importing/ComponentsImportFileBuilder.php @@ -31,6 +31,7 @@ class ComponentsImportFileBuilder extends FileBuilder protected function getDictionary(): array { return [ + 'assetTag' => 'Asset Tag', 'category' => 'Category', 'companyName' => 'Company', 'itemName' => 'item Name', @@ -51,6 +52,7 @@ class ComponentsImportFileBuilder extends FileBuilder $faker = fake(); return [ + 'assetTag' => '', 'category' => Str::random(), 'companyName' => Str::random()." {$faker->companySuffix}", 'itemName' => Str::random(), diff --git a/tests/Support/Importing/LicensesImportFileBuilder.php b/tests/Support/Importing/LicensesImportFileBuilder.php index e7c6ca3ee0..33cd09a38e 100644 --- a/tests/Support/Importing/LicensesImportFileBuilder.php +++ b/tests/Support/Importing/LicensesImportFileBuilder.php @@ -39,6 +39,9 @@ class LicensesImportFileBuilder extends FileBuilder { return [ 'category' => 'Category', + 'checkoutEmail' => 'Email', + 'checkoutFullName' => 'Full Name', + 'checkoutUsername' => 'Username', 'companyName' => 'Company', 'expirationDate' => 'expiration date', 'isMaintained' => 'maintained', @@ -66,6 +69,9 @@ class LicensesImportFileBuilder extends FileBuilder return [ 'category' => Str::random(), + 'checkoutEmail' => '', + 'checkoutFullName' => '', + 'checkoutUsername' => '', 'companyName' => Str::random()." {$faker->companySuffix}", 'expirationDate' => $faker->date, 'isMaintained' => $faker->randomElement(['TRUE', 'FALSE']), From 43a32071f1211b442a90c4885f8972d415a116cc Mon Sep 17 00:00:00 2001 From: snipe Date: Sat, 13 Jun 2026 14:38:32 +0100 Subject: [PATCH 24/24] FMCS/Companyable Trait: refactor API call to use canCheckoutTo --- app/Http/Controllers/Api/AssetsController.php | 24 +---------- app/Models/Traits/CompanyableTrait.php | 6 ++- .../Accessories/Ui/CheckoutAccessoryTest.php | 29 ++++++++++++- .../Checkouts/Api/AssetCheckoutTest.php | 42 +++++++++++++++++++ 4 files changed, 76 insertions(+), 25 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index 8930f376a1..b2b1a4042c 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -913,29 +913,7 @@ 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)) { - 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)) { - // Target has no company — only a mismatch when floater mode is off. - $nonUserMismatch = ! Setting::getSettings()->null_company_is_floater; - } else { - // Both sides have a company; require an exact match. - $nonUserMismatch = (int) $asset->company_id !== (int) $target->company_id; - } - - if ($nonUserMismatch) { + if (! $asset->canCheckoutTo($target)) { return response()->json(Helper::formatStandardApiResponse('error', null, trans('general.error_user_company'))); } diff --git a/app/Models/Traits/CompanyableTrait.php b/app/Models/Traits/CompanyableTrait.php index c2da82ce54..2c00fc5698 100644 --- a/app/Models/Traits/CompanyableTrait.php +++ b/app/Models/Traits/CompanyableTrait.php @@ -41,7 +41,11 @@ trait CompanyableTrait } if (! $this->company_id) { - return true; + if (is_null($target->company_id)) { + return true; + } + + return (bool) $settings->null_company_is_floater; } if ($target instanceof User) { diff --git a/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php b/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php index a46ffe1bce..97881ce1a7 100644 --- a/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php +++ b/tests/Feature/Accessories/Ui/CheckoutAccessoryTest.php @@ -63,7 +63,7 @@ class CheckoutAccessoryTest extends TestCase ]); } - public function test_checkout_to_user_succeeds_when_accessory_has_no_company_with_fmcs_enabled() + public function test_checkout_to_user_is_blocked_when_accessory_has_no_company_with_fmcs_enabled_without_floater() { $accessory = Accessory::factory()->create(['qty' => 5, 'company_id' => null]); [$companyA] = Company::factory()->count(1)->create(); @@ -71,6 +71,33 @@ class CheckoutAccessoryTest extends TestCase $user->companies()->sync([$companyA->id]); $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $actor = User::factory()->superuser()->create(); + + $this->actingAs($actor) + ->post(route('accessories.checkout.store', $accessory), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $user->id, + 'checkout_qty' => 1, + 'redirect_option' => 'index', + ]) + ->assertRedirect(); + + $this->assertDatabaseMissing('accessories_checkout', [ + 'accessory_id' => $accessory->id, + 'assigned_to' => $user->id, + ]); + } + + public function test_checkout_to_user_succeeds_when_accessory_has_no_company_with_floater_enabled() + { + $accessory = Accessory::factory()->create(['qty' => 5, 'company_id' => null]); + [$companyA] = Company::factory()->count(1)->create(); + $user = User::factory()->for($companyA)->create(); + $user->companies()->sync([$companyA->id]); + + $this->settings->enableFloaterMode(); $actor = User::factory()->superuser()->create(); diff --git a/tests/Feature/Checkouts/Api/AssetCheckoutTest.php b/tests/Feature/Checkouts/Api/AssetCheckoutTest.php index a5089a8cf1..cbba06287e 100644 --- a/tests/Feature/Checkouts/Api/AssetCheckoutTest.php +++ b/tests/Feature/Checkouts/Api/AssetCheckoutTest.php @@ -408,4 +408,46 @@ class AssetCheckoutTest extends TestCase $this->assertTrue((bool) $asset->fresh()->requestable); } + + public function test_null_company_asset_cannot_be_checked_out_to_companied_user_when_fmcs_enabled_without_floater() + { + $this->settings->enableMultipleFullCompanySupport(); + $this->settings->disableFloaterMode(); + + $company = Company::factory()->create(); + $actor = User::factory()->superuser()->create(); + $nullCompanyAsset = Asset::factory()->create(['company_id' => null]); + $companiedUser = User::factory()->for($company)->create(); + + $this->actingAsForApi($actor) + ->postJson(route('api.asset.checkout', $nullCompanyAsset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $companiedUser->id, + ]) + ->assertOk() + ->assertStatusMessageIs('error') + ->assertMessagesAre(trans('general.error_user_company')); + + $this->assertNull($nullCompanyAsset->fresh()->assigned_to); + } + + public function test_null_company_asset_can_be_checked_out_to_companied_user_when_floater_enabled() + { + $this->settings->enableFloaterMode(); + + $company = Company::factory()->create(); + $actor = User::factory()->superuser()->create(); + $nullCompanyAsset = Asset::factory()->create(['company_id' => null]); + $companiedUser = User::factory()->for($company)->create(); + + $this->actingAsForApi($actor) + ->postJson(route('api.asset.checkout', $nullCompanyAsset), [ + 'checkout_to_type' => 'user', + 'assigned_user' => $companiedUser->id, + ]) + ->assertOk() + ->assertStatusMessageIs('success'); + + $this->assertEquals($companiedUser->id, $nullCompanyAsset->fresh()->assigned_to); + } }