Merge pull request #15691 from marcusmoore/fixes/get-id-for-current-user

Updated `Company::getIdForCurrentUser()` to return null in certain scenarios

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,
         ]);
     }

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;
                 }
             }
         }

tests/Feature/Accessories/Ui/CreateAccessoryWithFullMultipleCompanySupportTest.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Tests\Feature\Accessories\Ui;
+
+use App\Models\Accessory;
+use App\Models\Category;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
+use Tests\TestCase;
+
+class CreateAccessoryWithFullMultipleCompanySupportTest extends TestCase
+{
+    use ProvidesDataForFullMultipleCompanySupportTesting;
+
+    #[DataProvider('dataForFullMultipleCompanySupportTesting')]
+    public function testAdheresToFullMultipleCompaniesSupportScoping($data)
+    {
+        ['actor' => $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);
+    }
+}

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())

tests/Feature/Assets/Api/StoreAssetWithFullMultipleCompanySupportTest.php

@@ -0,0 +1,58 @@
+<?php
+
+namespace Tests\Feature\Assets\Api;
+
+use App\Models\Asset;
+use App\Models\AssetModel;
+use App\Models\Statuslabel;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
+use Tests\TestCase;
+
+class StoreAssetWithFullMultipleCompanySupportTest extends TestCase
+{
+    use ProvidesDataForFullMultipleCompanySupportTesting;
+
+    /**
+     * @link https://github.com/snipe/snipe-it/issues/15654
+     */
+    #[DataProvider('dataForFullMultipleCompanySupportTesting')]
+    public function testAdheresToFullMultipleCompaniesSupportScoping($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' => $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);
+    }
+}

tests/Feature/Assets/Ui/StoreAssetWithFullMultipleCompanySupportTest.php

@@ -0,0 +1,35 @@
+<?php
+
+namespace Tests\Feature\Assets\Ui;
+
+use App\Models\Asset;
+use App\Models\AssetModel;
+use App\Models\Statuslabel;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
+use Tests\TestCase;
+
+class StoreAssetWithFullMultipleCompanySupportTest extends TestCase
+{
+    use ProvidesDataForFullMultipleCompanySupportTesting;
+
+    #[DataProvider('dataForFullMultipleCompanySupportTesting')]
+    public function testAdheresToFullMultipleCompaniesSupportScoping($data)
+    {
+        ['actor' => $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);
+    }
+}

tests/Feature/Components/Ui/StoreComponentWithFullMultipleCompanySupportTest.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Tests\Feature\Components\Ui;
+
+use App\Models\Category;
+use App\Models\Component;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
+use Tests\TestCase;
+
+class StoreComponentWithFullMultipleCompanySupportTest extends TestCase
+{
+    use ProvidesDataForFullMultipleCompanySupportTesting;
+
+    #[DataProvider('dataForFullMultipleCompanySupportTesting')]
+    public function testAdheresToFullMultipleCompaniesSupportScoping($data)
+    {
+        ['actor' => $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);
+    }
+}

tests/Feature/Consumables/Ui/StoreConsumableWithFullMultipleCompanySupportTest.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace Tests\Feature\Consumables\Ui;
+
+use App\Models\Category;
+use App\Models\Consumable;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
+use Tests\TestCase;
+
+class StoreConsumableWithFullMultipleCompanySupportTest extends TestCase
+{
+    use ProvidesDataForFullMultipleCompanySupportTesting;
+
+    #[DataProvider('dataForFullMultipleCompanySupportTesting')]
+    public function testAdheresToFullMultipleCompaniesSupportScoping($data)
+    {
+        ['actor' => $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);
+    }
+}

tests/Feature/Licenses/Ui/StoreLicenseWithFullMultipleCompanySupportTest.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Tests\Feature\Licenses\Ui;
+
+use App\Models\Category;
+use App\Models\License;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Tests\Support\ProvidesDataForFullMultipleCompanySupportTesting;
+use Tests\TestCase;
+
+class StoreLicenseWithFullMultipleCompanySupportTest extends TestCase
+{
+    use ProvidesDataForFullMultipleCompanySupportTesting;
+
+    #[DataProvider('dataForFullMultipleCompanySupportTesting')]
+    public function testAdheresToFullMultipleCompaniesSupportScoping($data)
+    {
+        ['actor' => $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);
+    }
+}

tests/Support/ProvidesDataForFullMultipleCompanySupportTesting.php

@@ -0,0 +1,70 @@
+<?php
+
+namespace Tests\Support;
+
+use App\Models\Company;
+use App\Models\User;
+use Generator;
+
+trait ProvidesDataForFullMultipleCompanySupportTesting
+{
+    public static function dataForFullMultipleCompanySupportTesting(): Generator
+    {
+        yield "User in a company should result in user's company_id being used" => [
+            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);
+                    },
+                ];
+            }
+        ];
+    }
+}

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