From 3f74ff25d2e0ddc259c6d397150ff6d5ac42d6ed Mon Sep 17 00:00:00 2001 From: Godfrey M Date: Wed, 25 Sep 2024 16:19:09 -0700 Subject: [PATCH 01/54] fixed error message --- app/Http/Controllers/Api/StatuslabelsController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Api/StatuslabelsController.php b/app/Http/Controllers/Api/StatuslabelsController.php index 754ebf7323..7e4851ff5a 100644 --- a/app/Http/Controllers/Api/StatuslabelsController.php +++ b/app/Http/Controllers/Api/StatuslabelsController.php @@ -95,7 +95,8 @@ class StatuslabelsController extends Controller $request->except('deployable', 'pending', 'archived'); if (! $request->filled('type')) { - return response()->json(Helper::formatStandardApiResponse('error', null, ['type' => ['Status label type is required.']]), 500); + + return response()->json(Helper::formatStandardApiResponse('error', null, ['type' => ['Status label type is required.']])); } $statuslabel = new Statuslabel; From 3153bbb13fb2244a457135e7550f656a51a6ea20 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 1 Oct 2024 17:04:18 -0500 Subject: [PATCH 02/54] dumb fix --- app/Http/Requests/Traits/MayContainCustomFields.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/Requests/Traits/MayContainCustomFields.php b/app/Http/Requests/Traits/MayContainCustomFields.php index 9a7f85e3a2..bbdf62893d 100644 --- a/app/Http/Requests/Traits/MayContainCustomFields.php +++ b/app/Http/Requests/Traits/MayContainCustomFields.php @@ -23,7 +23,7 @@ trait MayContainCustomFields return str_starts_with($attributes, '_snipeit_'); }); // if there are custom fields, find the one's that don't exist on the model's fieldset and add an error to the validator's error bag - if (count($request_fields) > 0) { + if (count($request_fields) > 0 && $validator->errors()->isEmpty()) { $request_fields->diff($asset_model?->fieldset?->fields?->pluck('db_column')) ->each(function ($request_field_name) use ($request_fields, $validator) { if (CustomField::where('db_column', $request_field_name)->exists()) { From 524a44272452d701e9da4d2e77ca10780f019f96 Mon Sep 17 00:00:00 2001 From: bryanlopezinc Date: Thu, 10 Oct 2024 23:32:07 +0100 Subject: [PATCH 03/54] Improved import performance --- app/Console/Commands/ObjectImportCommand.php | 49 +++++++------------- app/Importer/Importer.php | 33 +++++-------- 2 files changed, 28 insertions(+), 54 deletions(-) diff --git a/app/Console/Commands/ObjectImportCommand.php b/app/Console/Commands/ObjectImportCommand.php index 8370e7c050..a1202ded89 100644 --- a/app/Console/Commands/ObjectImportCommand.php +++ b/app/Console/Commands/ObjectImportCommand.php @@ -6,6 +6,7 @@ use Illuminate\Console\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputOption; use Illuminate\Support\Facades\Log; +use Symfony\Component\Console\Helper\ProgressIndicator; ini_set('max_execution_time', env('IMPORT_TIME_LIMIT', 600)); //600 seconds = 10 minutes ini_set('memory_limit', env('IMPORT_MEMORY_LIMIT', '500M')); @@ -29,6 +30,11 @@ class ObjectImportCommand extends Command */ protected $description = 'Import Items from CSV'; + /** + * The progress indicator instance. + */ + protected ProgressIndicator $progressIndicator; + /** * Create a new command instance. * @@ -39,8 +45,6 @@ class ObjectImportCommand extends Command parent::__construct(); } - private $bar; - /** * Execute the console command. * @@ -48,6 +52,8 @@ class ObjectImportCommand extends Command */ public function handle() { + $this->progressIndicator = new ProgressIndicator($this->output); + $filename = $this->argument('filename'); $class = title_case($this->option('item-type')); $classString = "App\\Importer\\{$class}Importer"; @@ -61,46 +67,25 @@ class ObjectImportCommand extends Command // This $logFile/useFiles() bit is currently broken, so commenting it out for now // $logFile = $this->option('logfile'); // Log::useFiles($logFile); - $this->comment('======= Importing Items from '.$filename.' ========='); + $this->progressIndicator->start('======= Importing Items from '.$filename.' ========='); + $importer->import(); - $this->bar = null; - - if (! empty($this->errors)) { - $this->comment('The following Errors were encountered.'); - foreach ($this->errors as $asset => $error) { - $this->comment('Error: Item: '.$asset.' failed validation: '.json_encode($error)); - } - } else { - $this->comment('All Items imported successfully!'); - } - $this->comment(''); + $this->progressIndicator->finish('Import finished.'); } - public function errorCallback($item, $field, $errorString) + public function errorCallback($item, $field, $error) { - $this->errors[$item->name][$field] = $errorString; + $this->output->write("\x0D\x1B[2K"); + + $this->warn('Error: Item: '.$item->name.' failed validation: '.json_encode($error)); } - public function progress($count) + public function progress($importedItemsCount) { - if (! $this->bar) { - $this->bar = $this->output->createProgressBar($count); - } - static $index = 0; - $index++; - if ($index < $count) { - $this->bar->advance(); - } else { - $this->bar->finish(); - } + $this->progressIndicator->advance(); } - // Tracks the current item for error messages - private $updating; - // An array of errors encountered while parsing - private $errors; - /** * Log a message to file, configurable by the --log-file parameter. * If a warning message is passed, we'll spit it to the console as well. diff --git a/app/Importer/Importer.php b/app/Importer/Importer.php index 47de5add4c..6f2816c7af 100644 --- a/app/Importer/Importer.php +++ b/app/Importer/Importer.php @@ -21,7 +21,6 @@ abstract class Importer * Id of User performing import * @var */ - protected $created_by; /** * Are we updating items in the import @@ -149,17 +148,23 @@ abstract class Importer { $headerRow = $this->csv->fetchOne(); $this->csv->setHeaderOffset(0); //explicitly sets the CSV document header record - $results = $this->normalizeInputArray($this->csv->getRecords($headerRow)); $this->populateCustomFields($headerRow); - DB::transaction(function () use (&$results) { + DB::transaction(function () use ($headerRow) { + $importedItemsCount = 0; Model::unguard(); - $resultsCount = count($results); - foreach ($results as $row) { + + foreach ($this->csv->getRecords($headerRow) as $row) { + //Lowercase header values to ensure we're comparing values properly. + $row = array_change_key_case($row, CASE_LOWER); + $this->handle($row); + + $importedItemsCount++; + if ($this->progressCallback) { - call_user_func($this->progressCallback, $resultsCount); + call_user_func($this->progressCallback, $importedItemsCount); } $this->log('------------- Action Summary ----------------'); @@ -237,22 +242,6 @@ abstract class Importer return $key; } - /** - * Used to lowercase header values to ensure we're comparing values properly. - * - * @param $results - * @return array - */ - public function normalizeInputArray($results) - { - $newArray = []; - foreach ($results as $index => $arrayToNormalize) { - $newArray[$index] = array_change_key_case($arrayToNormalize); - } - - return $newArray; - } - /** * Figure out the fieldname of the custom field * From 42095c0167b661d0f7a996a3bd17e0c0cfac5750 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 15 Oct 2024 13:02:22 -0700 Subject: [PATCH 04/54] Add reference link --- tests/Feature/Assets/Api/StoreAssetTest.php | 3 +++ 1 file changed, 3 insertions(+) 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()) From d9afde4610dd0527fb19fd52093f2d654ce36430 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 15 Oct 2024 17:00:22 -0700 Subject: [PATCH 05/54] Write failing test --- tests/Feature/Assets/Api/StoreAssetTest.php | 47 +++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/Feature/Assets/Api/StoreAssetTest.php b/tests/Feature/Assets/Api/StoreAssetTest.php index ea5cfb6178..12c6d5e858 100644 --- a/tests/Feature/Assets/Api/StoreAssetTest.php +++ b/tests/Feature/Assets/Api/StoreAssetTest.php @@ -23,6 +23,53 @@ class StoreAssetTest extends TestCase ->assertForbidden(); } + /** + * @link https://github.com/snipe/snipe-it/issues/15654 + */ + public function testAdheresToFullMultipleCompaniesSupportScoping() + { + [$companyA, $companyB] = Company::factory()->count(2)->create(); + $model = AssetModel::factory()->create(); + $status = Statuslabel::factory()->readyToDeploy()->create(); + + $userInNoCompany = User::factory() + ->createAssets() + ->create(['company_id' => null]); + + $userInCompanyA = User::factory() + ->for($companyA) + ->createAssets() + ->create(); + + $this->assertNull($userInNoCompany->company_id); + $this->assertEquals($companyA->id, $userInCompanyA->company_id); + + $this->settings->enableMultipleFullCompanySupport(); + + $responseForUserWithNoCompany = $this->actingAsForApi($userInNoCompany) + ->postJson(route('api.assets.store'), [ + 'asset_tag' => 'random_string', + 'company_id' => $companyB->id, + 'model_id' => $model->id, + 'status_id' => $status->id, + ]); + + $responseForUserInCompanyA = $this->actingAsForApi($userInCompanyA) + ->postJson(route('api.assets.store'), [ + 'asset_tag' => 'another_string', + 'company_id' => $companyB->id, + 'model_id' => $model->id, + 'status_id' => $status->id, + ]); + + $assetForUserWithNoCompany = Asset::withoutGlobalScopes()->find($responseForUserWithNoCompany['payload']['id']); + $assetForUserInCompanyA = Asset::withoutGlobalScopes()->find($responseForUserInCompanyA['payload']['id']); + + // company_id should be the company_id of the user that performed the action + $this->assertNull($assetForUserWithNoCompany->company_id); + $this->assertEquals($userInCompanyA->company_id, $assetForUserInCompanyA->company_id); + } + public function testAllAssetAttributesAreStored() { $company = Company::factory()->create(); From cba1a560408c821c4112a2548a6df20755536efd Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 15 Oct 2024 17:38:11 -0700 Subject: [PATCH 06/54] Improve readability? --- tests/Feature/Assets/Api/StoreAssetTest.php | 77 ++++++++++++--------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/tests/Feature/Assets/Api/StoreAssetTest.php b/tests/Feature/Assets/Api/StoreAssetTest.php index 12c6d5e858..8d648b835d 100644 --- a/tests/Feature/Assets/Api/StoreAssetTest.php +++ b/tests/Feature/Assets/Api/StoreAssetTest.php @@ -10,8 +10,10 @@ use App\Models\Location; use App\Models\Statuslabel; use App\Models\Supplier; use App\Models\User; +use Generator; use Illuminate\Support\Facades\Crypt; use Illuminate\Testing\Fluent\AssertableJson; +use PHPUnit\Framework\Attributes\DataProvider; use Tests\TestCase; class StoreAssetTest extends TestCase @@ -23,51 +25,60 @@ class StoreAssetTest extends TestCase ->assertForbidden(); } + public static function userProvider(): Generator + { + yield 'User in a company' => [ + function () { + $jedi = Company::factory()->create(); + $sith = Company::factory()->create(); + $luke = User::factory()->for($jedi)->createAssets()->create(); + + return [ + 'actor' => $luke, + 'company_attempting_to_associate' => $sith, + 'assertions' => function ($asset) use ($jedi) { + self::assertEquals($jedi->id, $asset->company_id); + }, + ]; + } + ]; + + yield 'User without a company' => [ + function () { + $userInNoCompany = User::factory()->createAssets()->create(['company_id' => null]); + + return [ + 'actor' => $userInNoCompany, + 'company_attempting_to_associate' => Company::factory()->create(), + 'assertions' => function ($asset) { + self::assertNull($asset->company_id); + }, + ]; + } + ]; + } + /** * @link https://github.com/snipe/snipe-it/issues/15654 */ - public function testAdheresToFullMultipleCompaniesSupportScoping() + #[DataProvider('userProvider')] + public function testAdheresToFullMultipleCompaniesSupportScoping($data) { - [$companyA, $companyB] = Company::factory()->count(2)->create(); - $model = AssetModel::factory()->create(); - $status = Statuslabel::factory()->readyToDeploy()->create(); - - $userInNoCompany = User::factory() - ->createAssets() - ->create(['company_id' => null]); - - $userInCompanyA = User::factory() - ->for($companyA) - ->createAssets() - ->create(); - - $this->assertNull($userInNoCompany->company_id); - $this->assertEquals($companyA->id, $userInCompanyA->company_id); + ['actor' => $actor, 'company_attempting_to_associate' => $company, 'assertions' => $assertions] = $data(); $this->settings->enableMultipleFullCompanySupport(); - $responseForUserWithNoCompany = $this->actingAsForApi($userInNoCompany) + $response = $this->actingAsForApi($actor) ->postJson(route('api.assets.store'), [ 'asset_tag' => 'random_string', - 'company_id' => $companyB->id, - 'model_id' => $model->id, - 'status_id' => $status->id, + 'company_id' => $company->id, + 'model_id' => AssetModel::factory()->create()->id, + 'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id, ]); - $responseForUserInCompanyA = $this->actingAsForApi($userInCompanyA) - ->postJson(route('api.assets.store'), [ - 'asset_tag' => 'another_string', - 'company_id' => $companyB->id, - 'model_id' => $model->id, - 'status_id' => $status->id, - ]); + $asset = Asset::withoutGlobalScopes()->findOrFail($response['payload']['id']); - $assetForUserWithNoCompany = Asset::withoutGlobalScopes()->find($responseForUserWithNoCompany['payload']['id']); - $assetForUserInCompanyA = Asset::withoutGlobalScopes()->find($responseForUserInCompanyA['payload']['id']); - - // company_id should be the company_id of the user that performed the action - $this->assertNull($assetForUserWithNoCompany->company_id); - $this->assertEquals($userInCompanyA->company_id, $assetForUserInCompanyA->company_id); + $assertions($asset); } public function testAllAssetAttributesAreStored() From fdcc17ca2c33d38a7af505c99d9547e014f5f783 Mon Sep 17 00:00:00 2001 From: Tobias Regnery Date: Wed, 16 Oct 2024 11:18:24 +0200 Subject: [PATCH 07/54] Fix user creation with FullMultipleCompanySupport enabled over API It is currently possible as a non-superuser to create a new user or patch an existing user with arbitrary company over the API if FullMultipleCompanySupport is enabled. Altough a highly unlikely scenario as the user needs permission to create API keys and new users, it is a bug that should get fixed. Add a call to getIdForCurrentUser() to normalize the company_id if FullMultipleCompanySupport is enabled. --- app/Http/Controllers/Api/UsersController.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/Http/Controllers/Api/UsersController.php b/app/Http/Controllers/Api/UsersController.php index 4714b29ea3..a9c8c26f14 100644 --- a/app/Http/Controllers/Api/UsersController.php +++ b/app/Http/Controllers/Api/UsersController.php @@ -14,6 +14,7 @@ use App\Http\Transformers\UsersTransformer; use App\Models\Actionlog; use App\Models\Asset; use App\Models\Accessory; +use App\Models\Company; use App\Models\Consumable; use App\Models\License; use App\Models\User; @@ -371,6 +372,7 @@ class UsersController extends Controller $user = new User; $user->fill($request->all()); + $user->company_id = Company::getIdForCurrentUser($request->input('company_id')); $user->created_by = auth()->id(); if ($request->has('permissions')) { @@ -452,6 +454,10 @@ class UsersController extends Controller $user->fill($request->all()); + if ($request->filled('company_id')) { + $user->company_id = Company::getIdForCurrentUser($request->input('company_id')); + } + if ($user->id == $request->input('manager_id')) { return response()->json(Helper::formatStandardApiResponse('error', null, 'You cannot be your own manager')); } From 2f72c66614676efd205585e25d909424b136d869 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 16 Oct 2024 11:30:06 -0700 Subject: [PATCH 08/54] Add additional case --- tests/Feature/Assets/Api/StoreAssetTest.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/Feature/Assets/Api/StoreAssetTest.php b/tests/Feature/Assets/Api/StoreAssetTest.php index 8d648b835d..118ccee406 100644 --- a/tests/Feature/Assets/Api/StoreAssetTest.php +++ b/tests/Feature/Assets/Api/StoreAssetTest.php @@ -56,6 +56,21 @@ class StoreAssetTest extends TestCase ]; } ]; + + yield 'Super-User assigning across companies' => [ + function () { + $superUser = User::factory()->superuser()->create(); + $company = Company::factory()->create(); + + return [ + 'actor' => $superUser, + 'company_attempting_to_associate' => $company, + 'assertions' => function ($asset) use ($company) { + self::assertEquals($asset->company_id, $company->id); + }, + ]; + } + ]; } /** From 604a9644624a52cd1aa965d92d36aa936840265d Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Wed, 16 Oct 2024 11:52:24 -0700 Subject: [PATCH 09/54] Improve scenario descriptions --- tests/Feature/Assets/Api/StoreAssetTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Feature/Assets/Api/StoreAssetTest.php b/tests/Feature/Assets/Api/StoreAssetTest.php index 118ccee406..95b73520bf 100644 --- a/tests/Feature/Assets/Api/StoreAssetTest.php +++ b/tests/Feature/Assets/Api/StoreAssetTest.php @@ -27,7 +27,7 @@ class StoreAssetTest extends TestCase public static function userProvider(): Generator { - yield 'User in a company' => [ + yield "User in a company should result in user's company_id being used" => [ function () { $jedi = Company::factory()->create(); $sith = Company::factory()->create(); @@ -43,7 +43,7 @@ class StoreAssetTest extends TestCase } ]; - yield 'User without a company' => [ + yield "User without a company should result in asset's company_id being null" => [ function () { $userInNoCompany = User::factory()->createAssets()->create(['company_id' => null]); @@ -57,7 +57,7 @@ class StoreAssetTest extends TestCase } ]; - yield 'Super-User assigning across companies' => [ + yield "Super-User assigning across companies should result in asset's company_id being set to what was provided" => [ function () { $superUser = User::factory()->superuser()->create(); $company = Company::factory()->create(); From 6b7af802af41c92a36e77605415869c9e72ec192 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 10 Oct 2024 13:28:23 +0100 Subject: [PATCH 10/54] Add 'bulk checkout' as one of the bulk actions in the bulk actions toolbar --- .../Assets/BulkAssetsController.php | 13 +++++++---- app/Models/Asset.php | 4 ++++ .../lang/en-US/admin/hardware/message.php | 5 ++++ .../partials/asset-bulk-actions.blade.php | 23 +++++++++++-------- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index 1ce08e65e9..b11c36469e 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -52,6 +52,10 @@ class BulkAssetsController extends Controller } $asset_ids = $request->input('ids'); + if ($request->input('bulk_actions') === 'checkout') { + $request->session()->flashInput(['selected_assets' => $asset_ids]); + return redirect()->route('hardware.bulkcheckout.show'); + } // Figure out where we need to send the user after the update is complete, and store that in the session $bulk_back_url = request()->headers->get('referer'); @@ -580,8 +584,9 @@ class BulkAssetsController extends Controller if ($target->location_id != '') { $asset->location_id = $target->location_id; - $asset->unsetEventDispatcher(); - $asset->save(); + $asset::withoutEvents(function () use ($asset) { // TODO - I don't know why this is being saved without events + $asset->save(); + }); } if ($error) { @@ -592,10 +597,10 @@ class BulkAssetsController extends Controller if (! $errors) { // Redirect to the new asset page - return redirect()->to('hardware')->with('success', trans('admin/hardware/message.checkout.success')); + return redirect()->to('hardware')->with('success', trans_choice('admin/hardware/message.multi-checkout.success', $asset_ids)); } // Redirect to the asset management page with error - return redirect()->route('hardware.bulkcheckout.show')->with('error', trans('admin/hardware/message.checkout.error'))->withErrors($errors); + return redirect()->route('hardware.bulkcheckout.show')->with('error', trans_choice('admin/hardware/message.multi-checkout.error', $asset_ids))->withErrors($errors); } catch (ModelNotFoundException $e) { return redirect()->route('hardware.bulkcheckout.show')->with('error', $e->getErrors()); } diff --git a/app/Models/Asset.php b/app/Models/Asset.php index ce8b870eb2..54257ed240 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -2,6 +2,7 @@ namespace App\Models; +use Watson\Validating\ValidationException; use App\Events\CheckoutableCheckedOut; use App\Exceptions\CheckoutNotAllowed; use App\Helpers\Helper; @@ -379,6 +380,9 @@ class Asset extends Depreciable return true; } + $validator = $this->makeValidator($this->getRules()); + + throw new ValidationException($validator, $this); return false; } diff --git a/resources/lang/en-US/admin/hardware/message.php b/resources/lang/en-US/admin/hardware/message.php index 041d32f56c..27877e02f9 100644 --- a/resources/lang/en-US/admin/hardware/message.php +++ b/resources/lang/en-US/admin/hardware/message.php @@ -79,6 +79,11 @@ return [ 'no_assets_selected' => 'You must select at least one asset from the list', ], + 'multi-checkout' => [ + 'error' => 'Asset was not checked out, please try again|Assets were not checked out, please try again', + 'success' => 'Asset checked out successfully.|Assets checked out successfully.', + ], + 'checkin' => [ 'error' => 'Asset was not checked in, please try again', 'success' => 'Asset checked in successfully.', diff --git a/resources/views/partials/asset-bulk-actions.blade.php b/resources/views/partials/asset-bulk-actions.blade.php index b597ad647f..992fb52bba 100644 --- a/resources/views/partials/asset-bulk-actions.blade.php +++ b/resources/views/partials/asset-bulk-actions.blade.php @@ -16,17 +16,20 @@ From 3cf746d7df83ef3e7cfa45c602fc182ebe8f11e3 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Wed, 16 Oct 2024 23:13:32 +0100 Subject: [PATCH 11/54] Rework the bulk checkout to not change how all checkouts work --- .../Controllers/Assets/BulkAssetsController.php | 14 ++++++++------ app/Models/Asset.php | 4 ---- resources/views/hardware/bulk-checkout.blade.php | 7 +++++++ 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/app/Http/Controllers/Assets/BulkAssetsController.php b/app/Http/Controllers/Assets/BulkAssetsController.php index b11c36469e..c27cfe3e0c 100644 --- a/app/Http/Controllers/Assets/BulkAssetsController.php +++ b/app/Http/Controllers/Assets/BulkAssetsController.php @@ -575,22 +575,24 @@ class BulkAssetsController extends Controller } $errors = []; - DB::transaction(function () use ($target, $admin, $checkout_at, $expected_checkin, $errors, $asset_ids, $request) { + DB::transaction(function () use ($target, $admin, $checkout_at, $expected_checkin, &$errors, $asset_ids, $request) { //NOTE: $errors is passsed by reference! foreach ($asset_ids as $asset_id) { $asset = Asset::findOrFail($asset_id); $this->authorize('checkout', $asset); - $error = $asset->checkOut($target, $admin, $checkout_at, $expected_checkin, e($request->get('note')), $asset->name, null); + $checkout_success = $asset->checkOut($target, $admin, $checkout_at, $expected_checkin, e($request->get('note')), $asset->name, null); + //TODO - I think this logic is duplicated in the checkOut method? if ($target->location_id != '') { $asset->location_id = $target->location_id; - $asset::withoutEvents(function () use ($asset) { // TODO - I don't know why this is being saved without events + // TODO - I don't know why this is being saved without events + $asset::withoutEvents(function () use ($asset) { $asset->save(); }); } - if ($error) { - array_merge_recursive($errors, $asset->getErrors()->toArray()); + if (!$checkout_success) { + $errors = array_merge_recursive($errors, $asset->getErrors()->toArray()); } } }); @@ -600,7 +602,7 @@ class BulkAssetsController extends Controller return redirect()->to('hardware')->with('success', trans_choice('admin/hardware/message.multi-checkout.success', $asset_ids)); } // Redirect to the asset management page with error - return redirect()->route('hardware.bulkcheckout.show')->with('error', trans_choice('admin/hardware/message.multi-checkout.error', $asset_ids))->withErrors($errors); + return redirect()->route('hardware.bulkcheckout.show')->withInput()->with('error', trans_choice('admin/hardware/message.multi-checkout.error', $asset_ids))->withErrors($errors); } catch (ModelNotFoundException $e) { return redirect()->route('hardware.bulkcheckout.show')->with('error', $e->getErrors()); } diff --git a/app/Models/Asset.php b/app/Models/Asset.php index 54257ed240..ce8b870eb2 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -2,7 +2,6 @@ namespace App\Models; -use Watson\Validating\ValidationException; use App\Events\CheckoutableCheckedOut; use App\Exceptions\CheckoutNotAllowed; use App\Helpers\Helper; @@ -380,9 +379,6 @@ class Asset extends Depreciable return true; } - $validator = $this->makeValidator($this->getRules()); - - throw new ValidationException($validator, $this); return false; } diff --git a/resources/views/hardware/bulk-checkout.blade.php b/resources/views/hardware/bulk-checkout.blade.php index 405e5e47cd..39e2cdf10c 100644 --- a/resources/views/hardware/bulk-checkout.blade.php +++ b/resources/views/hardware/bulk-checkout.blade.php @@ -115,5 +115,12 @@ @section('moar_scripts') @include('partials/assets-assigned') + @stop From c56affd66345a92f64fb8c7451ced58b5340dd70 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:07:37 +0100 Subject: [PATCH 12/54] Added SVG icon Signed-off-by: snipe --- app/Helpers/Helper.php | 37 ++----------------------------------- 1 file changed, 2 insertions(+), 35 deletions(-) diff --git a/app/Helpers/Helper.php b/app/Helpers/Helper.php index 18e149b57d..6c43fec05a 100644 --- a/app/Helpers/Helper.php +++ b/app/Helpers/Helper.php @@ -1123,6 +1123,7 @@ class Helper 'png' => 'far fa-image', 'webp' => 'far fa-image', 'avif' => 'far fa-image', + 'svg' => 'fas fa-vector-square', // word 'doc' => 'far fa-file-word', 'docx' => 'far fa-file-word', @@ -1135,7 +1136,7 @@ class Helper //Text 'txt' => 'far fa-file-alt', 'rtf' => 'far fa-file-alt', - 'xml' => 'far fa-file-alt', + 'xml' => 'fas fa-code', // Misc 'pdf' => 'far fa-file-pdf', 'lic' => 'far fa-save', @@ -1148,41 +1149,7 @@ class Helper return 'far fa-file'; } - public static function show_file_inline($filename) - { - $extension = substr(strrchr($filename, '.'), 1); - if ($extension) { - switch ($extension) { - case 'jpg': - case 'jpeg': - case 'gif': - case 'png': - case 'webp': - case 'avif': - return true; - break; - default: - return false; - } - } - - return false; - } - - /** - * Generate a random encrypted password. - * - * @author Wes Hulette - * - * @since 5.0.0 - * - * @return string - */ - public static function generateEncyrptedPassword(): string - { - return bcrypt(self::generateUnencryptedPassword()); - } /** * Get a random unencrypted password. From 96191a5e9391e86701095b74fefc9064e4bf74d3 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:07:54 +0100 Subject: [PATCH 13/54] Added method to decide if the file should be inlinable Signed-off-by: snipe --- app/Helpers/StorageHelper.php | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/app/Helpers/StorageHelper.php b/app/Helpers/StorageHelper.php index 2cdab1d66c..0b56aa97cd 100644 --- a/app/Helpers/StorageHelper.php +++ b/app/Helpers/StorageHelper.php @@ -25,4 +25,25 @@ class StorageHelper return Storage::disk($disk)->download($filename); } } + + public static function allowSafeInline($file_with_path) { + + $allowed_inline = [ + 'pdf', + 'svg', + 'jpg', + 'gif', + 'svg', + 'avif', + 'webp', + 'png', + ]; + + if (in_array(pathinfo($file_with_path, PATHINFO_EXTENSION), $allowed_inline)) { + return true; + } + + return false; + + } } From ccd20194484313c6e53bf357687ef8a29a4fffe6 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:08:04 +0100 Subject: [PATCH 14/54] Removed unusded use statements Signed-off-by: snipe --- app/Http/Controllers/Users/UserFilesController.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/Http/Controllers/Users/UserFilesController.php b/app/Http/Controllers/Users/UserFilesController.php index 9e5f322c03..c155570513 100644 --- a/app/Http/Controllers/Users/UserFilesController.php +++ b/app/Http/Controllers/Users/UserFilesController.php @@ -7,9 +7,6 @@ use App\Http\Controllers\Controller; use App\Http\Requests\UploadFileRequest; use App\Models\Actionlog; use App\Models\User; -use Illuminate\Support\Facades\Auth; -use Illuminate\Support\Facades\Input; -use Illuminate\Support\Facades\Response; use Symfony\Component\HttpFoundation\JsonResponse; use Illuminate\Support\Facades\Storage; From c49abb6aeafbfbf43ebcb7e1a9c565288b72b9d9 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:08:38 +0100 Subject: [PATCH 15/54] Refactor the UserFilesController show method for simpler inlining Signed-off-by: snipe --- .../Controllers/Users/UserFilesController.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Users/UserFilesController.php b/app/Http/Controllers/Users/UserFilesController.php index c155570513..377692965b 100644 --- a/app/Http/Controllers/Users/UserFilesController.php +++ b/app/Http/Controllers/Users/UserFilesController.php @@ -113,6 +113,9 @@ class UserFilesController extends Controller public function show($userId = null, $fileId = null) { + + + if (empty($fileId)) { return redirect()->route('users.show')->with('error', 'Invalid file request'); } @@ -126,15 +129,21 @@ class UserFilesController extends Controller if ($log = Actionlog::whereNotNull('filename')->where('item_id', $user->id)->find($fileId)) { - // Display the file inline - if (request('inline') == 'true') { + $file = 'private_uploads/users/'.$log->filename; + + + if ((request('inline') == 'true') && (StorageHelper::allowSafeInline($file) === false)) { + + // Display the file as text is not allowed for security reasons $headers = [ 'Content-Disposition' => 'inline', + 'Content-Type' => 'text/plain', ]; - return Storage::download('private_uploads/users/'.$log->filename, $log->filename, $headers); + return Storage::download($file, $log->filename, $headers); + } - return Storage::download('private_uploads/users/'.$log->filename); + return Storage::download($file); } return redirect()->route('users.index')->with('error', trans('admin/users/message.log_record_not_found')); From c49921f50fc54785b87d59e6265b29088bba48fe Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:08:54 +0100 Subject: [PATCH 16/54] Removed unused (maybe?) API endpoint Signed-off-by: snipe --- app/Http/Transformers/ActionlogsTransformer.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Http/Transformers/ActionlogsTransformer.php b/app/Http/Transformers/ActionlogsTransformer.php index d0605c747b..49eee42415 100644 --- a/app/Http/Transformers/ActionlogsTransformer.php +++ b/app/Http/Transformers/ActionlogsTransformer.php @@ -158,7 +158,6 @@ class ActionlogsTransformer [ 'url' => $file_url, 'filename' => $actionlog->filename, - 'inlineable' => (bool) Helper::show_file_inline($actionlog->filename), ] : null, 'item' => ($actionlog->item) ? [ From 017884f8432abd3afed7397a7967bb2813df0262 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:09:09 +0100 Subject: [PATCH 17/54] Added checks and filetype display Signed-off-by: snipe --- resources/views/users/view.blade.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/resources/views/users/view.blade.php b/resources/views/users/view.blade.php index 4ef3a0c24f..d33b40d948 100755 --- a/resources/views/users/view.blade.php +++ b/resources/views/users/view.blade.php @@ -995,7 +995,8 @@ - {{trans('general.file_type')}} + {{trans('general.icon')}} + {{trans('general.file_type')}} {{ trans('general.image') }} {{ trans('general.file_name') }} {{ trans('general.filesize') }} @@ -1009,9 +1010,11 @@ @foreach ($user->uploads as $file) - + {{ Helper::filetype_icon($file->filename) }} - + + + {{ pathinfo('private_uploads/users/'.$file->filename, PATHINFO_EXTENSION) }} @if (($file->filename) && (Storage::exists('private_uploads/users/'.$file->filename))) @@ -1045,7 +1048,7 @@ {{ trans('general.download') }} - + @endif From c01190fac279b38d8ee74a636d8cf9c2fd7a7285 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:18:34 +0100 Subject: [PATCH 18/54] Conditionally add content-type Signed-off-by: snipe --- app/Http/Controllers/Users/UserFilesController.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/Http/Controllers/Users/UserFilesController.php b/app/Http/Controllers/Users/UserFilesController.php index 377692965b..15b8018b78 100644 --- a/app/Http/Controllers/Users/UserFilesController.php +++ b/app/Http/Controllers/Users/UserFilesController.php @@ -132,15 +132,19 @@ class UserFilesController extends Controller $file = 'private_uploads/users/'.$log->filename; - if ((request('inline') == 'true') && (StorageHelper::allowSafeInline($file) === false)) { - // Display the file as text is not allowed for security reasons + if (request('inline') == 'true') { + $headers = [ 'Content-Disposition' => 'inline', - 'Content-Type' => 'text/plain', ]; - return Storage::download($file, $log->filename, $headers); + // This is NOT allowed as inline - force it to be displayed as text + if (StorageHelper::allowSafeInline($file) === false) { + array_push($headers, ['Content-Type' => 'text/plain']); + } + + return Storage::download($file, $log->filename, $headers); } return Storage::download($file); From 02c80ff18af48c016433807b4bdc0f384ffde41a Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:18:40 +0100 Subject: [PATCH 19/54] Added comment Signed-off-by: snipe --- app/Helpers/StorageHelper.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/Helpers/StorageHelper.php b/app/Helpers/StorageHelper.php index 0b56aa97cd..ef0facc27b 100644 --- a/app/Helpers/StorageHelper.php +++ b/app/Helpers/StorageHelper.php @@ -26,6 +26,15 @@ class StorageHelper } } + + /** + * This determines the file types that should be allowed inline and checks their fileinfo extension + * + * @author [ + * @since v7.0.14 + * @param $file_with_path + * @return bool + */ public static function allowSafeInline($file_with_path) { $allowed_inline = [ From d67addc69ee2fde2fdc9d90e1eefa13f1c1e4f26 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:21:43 +0100 Subject: [PATCH 20/54] =?UTF-8?q?Removed=20filetype=20column=20-=20it?= =?UTF-8?q?=E2=80=99s=20dumb?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: snipe --- resources/views/users/view.blade.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/resources/views/users/view.blade.php b/resources/views/users/view.blade.php index d33b40d948..223d525f1d 100755 --- a/resources/views/users/view.blade.php +++ b/resources/views/users/view.blade.php @@ -995,7 +995,6 @@ - {{trans('general.icon')}} {{trans('general.file_type')}} {{ trans('general.image') }} {{ trans('general.file_name') }} @@ -1013,9 +1012,6 @@ {{ Helper::filetype_icon($file->filename) }} - - {{ pathinfo('private_uploads/users/'.$file->filename, PATHINFO_EXTENSION) }} - @if (($file->filename) && (Storage::exists('private_uploads/users/'.$file->filename))) @if (Helper::checkUploadIsImage($file->get_src('users'))) From 4933aa5784e3a86021136775055d81eed7b3c966 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:27:04 +0100 Subject: [PATCH 21/54] Add StorageHelper to app config Signed-off-by: snipe --- config/app.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/app.php b/config/app.php index bc74b4dd05..39898ff437 100755 --- a/config/app.php +++ b/config/app.php @@ -280,7 +280,6 @@ return [ Illuminate\Redis\RedisServiceProvider::class, Illuminate\Auth\Passwords\PasswordResetServiceProvider::class, Illuminate\Session\SessionServiceProvider::class, -// Illuminate\Translation\TranslationServiceProvider::class, //replaced on next line App\Providers\SnipeTranslationServiceProvider::class, //we REPLACE the default Laravel translator with our own Illuminate\Validation\ValidationServiceProvider::class, Illuminate\View\ViewServiceProvider::class, @@ -373,7 +372,7 @@ return [ 'Image' => Intervention\Image\ImageServiceProvider::class, 'Carbon' => Carbon\Carbon::class, 'Helper' => App\Helpers\Helper::class, - // makes it much easier to use 'Helper::blah' in blades (which is where we usually use this) + 'StorageHelper' => App\Helpers\StorageHelper::class, 'Icon' => App\Helpers\IconHelper::class, 'Socialite' => Laravel\Socialite\Facades\Socialite::class, From 0e9b3c9119e77259ed2f8740ae98e9149936bd51 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 17 Oct 2024 00:27:39 +0100 Subject: [PATCH 22/54] Check for existence before trying to get the icon Signed-off-by: snipe --- resources/views/users/view.blade.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/resources/views/users/view.blade.php b/resources/views/users/view.blade.php index 223d525f1d..7821b87402 100755 --- a/resources/views/users/view.blade.php +++ b/resources/views/users/view.blade.php @@ -1009,13 +1009,17 @@ @foreach ($user->uploads as $file) - - {{ Helper::filetype_icon($file->filename) }} + @if (Storage::exists('private_uploads/users/'.$file->filename)) + + {{ Helper::filetype_icon($file->filename) }} + @endif @if (($file->filename) && (Storage::exists('private_uploads/users/'.$file->filename))) @if (Helper::checkUploadIsImage($file->get_src('users'))) - + + + @else {{ trans('general.preview_not_available') }} @endif @@ -1044,7 +1048,7 @@ {{ trans('general.download') }} - + @endif From f50ccbcc492db6c98cabf6dc6752dd99ab82bce7 Mon Sep 17 00:00:00 2001 From: Tobias Regnery Date: Thu, 17 Oct 2024 11:07:28 +0200 Subject: [PATCH 23/54] Fix outdated comment in CompanyableTrait As of commit 5800e8d the user model uses CompanyableTrait so remove this clearly outdated comment --- app/Models/CompanyableTrait.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/Models/CompanyableTrait.php b/app/Models/CompanyableTrait.php index df67f2be4f..04a620d8e3 100644 --- a/app/Models/CompanyableTrait.php +++ b/app/Models/CompanyableTrait.php @@ -8,9 +8,6 @@ trait CompanyableTrait * This trait is used to scope models to the current company. To use this scope on companyable models, * we use the "use Companyable;" statement at the top of the mode. * - * We CANNOT USE THIS ON USERS, as it causes an infinite loop and prevents users from logging in, since this scope will be - * applied to the currently logged in (or logging in) user in addition to the user model for viewing lists of users. - * * @see \App\Models\Company\Company::scopeCompanyables() * @return void */ From 780ed91a1098ed9af2a5f8f95d6ac87a14e55b20 Mon Sep 17 00:00:00 2001 From: NebelKreis Date: Thu, 17 Oct 2024 16:09:25 +0200 Subject: [PATCH 24/54] Fix: Removed strtolower() from dashboard titles This fix ensures the correct capitalization in different languages. --- resources/views/dashboard.blade.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/resources/views/dashboard.blade.php b/resources/views/dashboard.blade.php index 39a5d8b751..9eb2eb5386 100755 --- a/resources/views/dashboard.blade.php +++ b/resources/views/dashboard.blade.php @@ -34,7 +34,7 @@

{{ number_format(\App\Models\Asset::AssetsForShow()->count()) }}

-

{{ strtolower(trans('general.assets')) }}

+

{{ trans('general.assets') }}