diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index 26d01051b4..fb7469ac88 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -26,11 +26,19 @@ class StoreAssetRequest extends ImageUploadRequest public function prepareForValidation(): void { + // Guard against users passing in an array for company_id instead of an integer. + // If the company_id is not an integer then we simply use what was + // provided to be caught by model level validation later. + // The use of is_numeric accounts for 1 and '1'. + $idForCurrentUser = is_numeric($this->company_id) + ? Company::getIdForCurrentUser($this->company_id) + : $this->company_id; + $this->parseLastAuditDate(); $this->merge([ 'asset_tag' => $this->asset_tag ?? Asset::autoincrement_asset(), - 'company_id' => Company::getIdForCurrentUser($this->company_id), + 'company_id' => $idForCurrentUser, 'assigned_to' => $assigned_to ?? null, ]); } diff --git a/app/Models/Company.php b/app/Models/Company.php index 171d559542..8886da77f6 100644 --- a/app/Models/Company.php +++ b/app/Models/Company.php @@ -116,7 +116,7 @@ final class Company extends SnipeModel if ($current_user->company_id != null) { return $current_user->company_id; } else { - return static::getIdFromInput($unescaped_input); + return null; } } } diff --git a/resources/views/accessories/view.blade.php b/resources/views/accessories/view.blade.php index fb2904e2cf..1e3568f0ef 100644 --- a/resources/views/accessories/view.blade.php +++ b/resources/views/accessories/view.blade.php @@ -179,7 +179,8 @@ @if ($accessory->image!='')
- {{ $accessory->name }} + + {{ $accessory->name }}
@endif diff --git a/resources/views/components/view.blade.php b/resources/views/components/view.blade.php index 5bd185bedf..ba3a6d6e24 100644 --- a/resources/views/components/view.blade.php +++ b/resources/views/components/view.blade.php @@ -161,7 +161,7 @@
@if ($component->image!='')
- + {{ $component->name }}
diff --git a/resources/views/consumables/view.blade.php b/resources/views/consumables/view.blade.php index 87650ac9eb..ac95e6391b 100644 --- a/resources/views/consumables/view.blade.php +++ b/resources/views/consumables/view.blade.php @@ -89,7 +89,7 @@ @if ($consumable->image!='')
- + {{ $consumable->name }}
@endif diff --git a/resources/views/hardware/view.blade.php b/resources/views/hardware/view.blade.php index f93c26cf8e..6a260cf254 100755 --- a/resources/views/hardware/view.blade.php +++ b/resources/views/hardware/view.blade.php @@ -168,7 +168,7 @@
@if (($asset->image) || (($asset->model) && ($asset->model->image!=''))) diff --git a/tests/Feature/Accessories/Ui/CreateAccessoryWithFullMultipleCompanySupportTest.php b/tests/Feature/Accessories/Ui/CreateAccessoryWithFullMultipleCompanySupportTest.php new file mode 100644 index 0000000000..a606db3fe2 --- /dev/null +++ b/tests/Feature/Accessories/Ui/CreateAccessoryWithFullMultipleCompanySupportTest.php @@ -0,0 +1,37 @@ + $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data(); + + $this->settings->enableMultipleFullCompanySupport(); + + $this->actingAs($actor) + ->post(route('accessories.store'), [ + 'redirect_option' => 'index', + 'name' => 'My Cool Accessory', + 'qty' => '1', + 'category_id' => Category::factory()->create()->id, + 'company_id' => $company->id, + ]); + + $accessory = Accessory::withoutGlobalScopes()->where([ + 'name' => 'My Cool Accessory', + ])->sole(); + + $assertions($accessory); + } +} diff --git a/tests/Feature/Assets/Api/StoreAssetTest.php b/tests/Feature/Assets/Api/StoreAssetTest.php index a147504519..ea5cfb6178 100644 --- a/tests/Feature/Assets/Api/StoreAssetTest.php +++ b/tests/Feature/Assets/Api/StoreAssetTest.php @@ -560,6 +560,9 @@ class StoreAssetTest extends TestCase $this->assertTrue($asset->assignedAssets()->find($response['payload']['id'])->is($apiAsset)); } + /** + * @link https://app.shortcut.com/grokability/story/24475 + */ public function testCompanyIdNeedsToBeInteger() { $this->actingAsForApi(User::factory()->createAssets()->create()) diff --git a/tests/Feature/Assets/Api/StoreAssetWithFullMultipleCompanySupportTest.php b/tests/Feature/Assets/Api/StoreAssetWithFullMultipleCompanySupportTest.php new file mode 100644 index 0000000000..60676f39c1 --- /dev/null +++ b/tests/Feature/Assets/Api/StoreAssetWithFullMultipleCompanySupportTest.php @@ -0,0 +1,58 @@ + $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data(); + + $this->settings->enableMultipleFullCompanySupport(); + + $response = $this->actingAsForApi($actor) + ->postJson(route('api.assets.store'), [ + 'asset_tag' => 'random_string', + 'company_id' => $company->id, + 'model_id' => AssetModel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, + ]); + + $asset = Asset::withoutGlobalScopes()->findOrFail($response['payload']['id']); + + $assertions($asset); + } + + #[DataProvider('dataForFullMultipleCompanySupportTesting')] + public function testHandlesCompanyIdBeingString($data) + { + ['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data(); + + $this->settings->enableMultipleFullCompanySupport(); + + $response = $this->actingAsForApi($actor) + ->postJson(route('api.assets.store'), [ + 'asset_tag' => 'random_string', + 'company_id' => (string) $company->id, + 'model_id' => AssetModel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, + ]); + + $asset = Asset::withoutGlobalScopes()->findOrFail($response['payload']['id']); + + $assertions($asset); + } +} diff --git a/tests/Feature/Assets/Ui/StoreAssetWithFullMultipleCompanySupportTest.php b/tests/Feature/Assets/Ui/StoreAssetWithFullMultipleCompanySupportTest.php new file mode 100644 index 0000000000..3443256030 --- /dev/null +++ b/tests/Feature/Assets/Ui/StoreAssetWithFullMultipleCompanySupportTest.php @@ -0,0 +1,35 @@ + $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data(); + + $this->settings->enableMultipleFullCompanySupport(); + + $this->actingAs($actor) + ->post(route('hardware.store'), [ + 'asset_tags' => ['1' => '1234'], + 'model_id' => AssetModel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->create()->id, + 'company_id' => $company->id, + ]); + + $asset = Asset::where('asset_tag', '1234')->sole(); + + $assertions($asset); + } +} diff --git a/tests/Feature/Components/Ui/StoreComponentWithFullMultipleCompanySupportTest.php b/tests/Feature/Components/Ui/StoreComponentWithFullMultipleCompanySupportTest.php new file mode 100644 index 0000000000..bb36a6fa82 --- /dev/null +++ b/tests/Feature/Components/Ui/StoreComponentWithFullMultipleCompanySupportTest.php @@ -0,0 +1,34 @@ + $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data(); + + $this->settings->enableMultipleFullCompanySupport(); + + $this->actingAs($actor) + ->post(route('components.store'), [ + 'name' => 'My Cool Component', + 'qty' => '1', + 'category_id' => Category::factory()->create()->id, + 'company_id' => $company->id, + ]); + + $component = Component::where('name', 'My Cool Component')->sole(); + + $assertions($component); + } +} diff --git a/tests/Feature/Consumables/Ui/StoreConsumableWithFullMultipleCompanySupportTest.php b/tests/Feature/Consumables/Ui/StoreConsumableWithFullMultipleCompanySupportTest.php new file mode 100644 index 0000000000..6f53d2298c --- /dev/null +++ b/tests/Feature/Consumables/Ui/StoreConsumableWithFullMultipleCompanySupportTest.php @@ -0,0 +1,33 @@ + $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data(); + + $this->settings->enableMultipleFullCompanySupport(); + + $this->actingAs($actor) + ->post(route('consumables.store'), [ + 'name' => 'My Cool Consumable', + 'category_id' => Category::factory()->forConsumables()->create()->id, + 'company_id' => $company->id, + ]); + + $consumable = Consumable::where('name', 'My Cool Consumable')->sole(); + + $assertions($consumable); + } +} diff --git a/tests/Feature/Licenses/Ui/StoreLicenseWithFullMultipleCompanySupportTest.php b/tests/Feature/Licenses/Ui/StoreLicenseWithFullMultipleCompanySupportTest.php new file mode 100644 index 0000000000..de6ffbaaee --- /dev/null +++ b/tests/Feature/Licenses/Ui/StoreLicenseWithFullMultipleCompanySupportTest.php @@ -0,0 +1,34 @@ + $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data(); + + $this->settings->enableMultipleFullCompanySupport(); + + $this->actingAs($actor) + ->post(route('licenses.store'), [ + 'name' => 'My Cool License', + 'seats' => '1', + 'category_id' => Category::factory()->forLicenses()->create()->id, + 'company_id' => $company->id, + ]); + + $license = License::where('name', 'My Cool License')->sole(); + + $assertions($license); + } +} diff --git a/tests/Support/ProvidesDataForFullMultipleCompanySupportTesting.php b/tests/Support/ProvidesDataForFullMultipleCompanySupportTesting.php new file mode 100644 index 0000000000..d247b98161 --- /dev/null +++ b/tests/Support/ProvidesDataForFullMultipleCompanySupportTesting.php @@ -0,0 +1,70 @@ + [ + function () { + $jedi = Company::factory()->create(); + $sith = Company::factory()->create(); + $luke = User::factory()->for($jedi) + ->createAccessories() + ->createAssets() + ->createComponents() + ->createConsumables() + ->createLicenses() + ->create(); + + return [ + 'actor' => $luke, + 'company_attempting_to_associate' => $sith, + 'assertions' => function ($model) use ($jedi) { + self::assertEquals($jedi->id, $model->company_id); + }, + ]; + } + ]; + + yield "User without a company should result in company_id being null" => [ + function () { + $userInNoCompany = User::factory() + ->createAccessories() + ->createAssets() + ->createComponents() + ->createConsumables() + ->createLicenses() + ->create(['company_id' => null]); + + return [ + 'actor' => $userInNoCompany, + 'company_attempting_to_associate' => Company::factory()->create(), + 'assertions' => function ($model) { + self::assertNull($model->company_id); + }, + ]; + } + ]; + + yield "Super-User assigning across companies should result in company_id being set to what was provided" => [ + function () { + $superUser = User::factory()->superuser()->create(['company_id' => null]); + $company = Company::factory()->create(); + + return [ + 'actor' => $superUser, + 'company_attempting_to_associate' => $company, + 'assertions' => function ($model) use ($company) { + self::assertEquals($model->company_id, $company->id); + }, + ]; + } + ]; + } +} diff --git a/tests/Unit/Models/Company/GetIdForCurrentUserTest.php b/tests/Unit/Models/Company/GetIdForCurrentUserTest.php index 6d77c88731..c21c4f36a3 100644 --- a/tests/Unit/Models/Company/GetIdForCurrentUserTest.php +++ b/tests/Unit/Models/Company/GetIdForCurrentUserTest.php @@ -32,11 +32,11 @@ class GetIdForCurrentUserTest extends TestCase $this->assertEquals(2000, Company::getIdForCurrentUser(1000)); } - public function testReturnsProvidedValueForNonSuperUserWithoutCompanyIdWhenFullCompanySupportEnabled() + public function testReturnsNullForNonSuperUserWithoutCompanyIdWhenFullCompanySupportEnabled() { $this->settings->enableMultipleFullCompanySupport(); $this->actingAs(User::factory()->create(['company_id' => null])); - $this->assertEquals(1000, Company::getIdForCurrentUser(1000)); + $this->assertNull(Company::getIdForCurrentUser(1000)); } }