diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index 26d01051b..fb7469ac8 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 171d55954..8886da77f 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/tests/Feature/Accessories/Ui/CreateAccessoryWithFullMultipleCompanySupportTest.php b/tests/Feature/Accessories/Ui/CreateAccessoryWithFullMultipleCompanySupportTest.php new file mode 100644 index 000000000..a606db3fe --- /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 a14750451..ea5cfb617 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 000000000..60676f39c --- /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 000000000..344325603 --- /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 000000000..bb36a6fa8 --- /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 000000000..6f53d2298 --- /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 000000000..de6ffbaae --- /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 000000000..d247b9816 --- /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 6d77c8873..c21c4f36a 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)); } }