From d4b9f6a2a47a3a7e9dc6310c25062493764aaeda Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 31 Oct 2023 21:00:22 -0500 Subject: [PATCH 01/10] add not_array (not ready, doesn't catch for specific exception) --- app/Models/Asset.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Models/Asset.php b/app/Models/Asset.php index 9f15101d33..7f4f691628 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -91,7 +91,7 @@ class Asset extends Depreciable protected $rules = [ 'name' => 'max:255|nullable', - 'model_id' => 'required|integer|exists:models,id,deleted_at,NULL', + 'model_id' => 'required|integer|exists:models,id,deleted_at,NULL|not_array', 'status_id' => 'required|integer|exists:status_labels,id', 'company_id' => 'integer|nullable', 'warranty_months' => 'numeric|nullable|digits_between:0,240', From b67b00dd82af6c32636d645d88990b5d5d19b9ab Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Tue, 31 Oct 2023 21:06:44 -0500 Subject: [PATCH 02/10] this is a start, something up with asset_tag unique rule --- app/Http/Controllers/Api/AssetsController.php | 4 +- app/Http/Requests/StoreAssetRequest.php | 49 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 app/Http/Requests/StoreAssetRequest.php diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index bbdc982599..c32c3c4429 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -3,6 +3,7 @@ namespace App\Http\Controllers\Api; use App\Events\CheckoutableCheckedIn; +use App\Http\Requests\StoreAssetRequest; use Illuminate\Support\Facades\Gate; use App\Helpers\Helper; use App\Http\Controllers\Controller; @@ -33,6 +34,7 @@ use TCPDF; use Validator; use Route; + /** * This class controls all actions related to assets for * the Snipe-IT Asset Management application. @@ -532,7 +534,7 @@ class AssetsController extends Controller * @since [v4.0] * @return \Illuminate\Http\JsonResponse */ - public function store(ImageUploadRequest $request) + public function store(StoreAssetRequest $request) { $this->authorize('create', Asset::class); diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php new file mode 100644 index 0000000000..992d67e25b --- /dev/null +++ b/app/Http/Requests/StoreAssetRequest.php @@ -0,0 +1,49 @@ +getRules(), + parent::rules(), + ); + + if(!$this->expectsJson()) { + //accepts an array for the gui form + $rules['asset_tags.*'] = $rules['asset_tag']; + unset($rules['asset_tag']); + $rules['serials.*'] = $rules['serial']; + unset($rules['serial']); + } + + return $rules; + } +} From 53bd5626c9e3fda94f0394993d8384941d9a7f20 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 1 Nov 2023 11:33:29 -0500 Subject: [PATCH 03/10] this works, need to write up pr --- app/Exceptions/Handler.php | 6 ++++++ app/Http/Requests/StoreAssetRequest.php | 8 +++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 3d4db93452..b3317ad2df 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -4,6 +4,7 @@ namespace App\Exceptions; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; use App\Helpers\Helper; +use Illuminate\Http\JsonResponse; use Illuminate\Validation\ValidationException; use Illuminate\Auth\AuthenticationException; use ArieTimmerman\Laravel\SCIMServer\Exceptions\SCIMException; @@ -150,6 +151,11 @@ class Handler extends ExceptionHandler return redirect()->guest('login'); } + protected function invalidJson($request, ValidationException $exception) + { + return response()->json(Helper::formatStandardApiResponse('error', null, $exception->errors()), 200); + } + /** * A list of the inputs that are never flashed for validation exceptions. diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index 992d67e25b..a9a06baed4 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -15,8 +15,8 @@ class StoreAssetRequest extends ImageUploadRequest */ public function authorize(): bool { - //TODO: make sure this works - //return Gate::allows('create', new Asset); + // TODO: make sure this works + return Gate::allows('create', new Asset); } public function prepareForValidation(): void @@ -36,13 +36,11 @@ class StoreAssetRequest extends ImageUploadRequest parent::rules(), ); - if(!$this->expectsJson()) { - //accepts an array for the gui form + // unsets unique check here, that check cannot run twice. $rules['asset_tags.*'] = $rules['asset_tag']; unset($rules['asset_tag']); $rules['serials.*'] = $rules['serial']; unset($rules['serial']); - } return $rules; } From d971172cf35ddd96cf2f8b3526341284c14193d5 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 1 Nov 2023 11:34:15 -0500 Subject: [PATCH 04/10] rm unnecessary import --- app/Exceptions/Handler.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index b3317ad2df..e76d8e5dae 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -4,7 +4,6 @@ namespace App\Exceptions; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; use App\Helpers\Helper; -use Illuminate\Http\JsonResponse; use Illuminate\Validation\ValidationException; use Illuminate\Auth\AuthenticationException; use ArieTimmerman\Laravel\SCIMServer\Exceptions\SCIMException; From 3f834cb88fdc99cf7fe3b6a55a80f0f2da226aa7 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 1 Nov 2023 11:43:53 -0500 Subject: [PATCH 05/10] authorization tested in request --- app/Http/Controllers/Api/AssetsController.php | 2 -- app/Http/Requests/StoreAssetRequest.php | 1 - 2 files changed, 3 deletions(-) diff --git a/app/Http/Controllers/Api/AssetsController.php b/app/Http/Controllers/Api/AssetsController.php index c32c3c4429..eb2be64442 100644 --- a/app/Http/Controllers/Api/AssetsController.php +++ b/app/Http/Controllers/Api/AssetsController.php @@ -536,8 +536,6 @@ class AssetsController extends Controller */ public function store(StoreAssetRequest $request) { - $this->authorize('create', Asset::class); - $asset = new Asset(); $asset->model()->associate(AssetModel::find((int) $request->get('model_id'))); diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index a9a06baed4..d42ead35d5 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -15,7 +15,6 @@ class StoreAssetRequest extends ImageUploadRequest */ public function authorize(): bool { - // TODO: make sure this works return Gate::allows('create', new Asset); } From e05af5216e3c8474533e3e573e29db863876d23c Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 1 Nov 2023 12:14:46 -0500 Subject: [PATCH 06/10] formatting --- app/Http/Requests/StoreAssetRequest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index d42ead35d5..2fb2544160 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -35,11 +35,11 @@ class StoreAssetRequest extends ImageUploadRequest parent::rules(), ); - // unsets unique check here, that check cannot run twice. - $rules['asset_tags.*'] = $rules['asset_tag']; - unset($rules['asset_tag']); - $rules['serials.*'] = $rules['serial']; - unset($rules['serial']); + // unsets unique check here, that check cannot run twice. + $rules['asset_tags.*'] = $rules['asset_tag']; + unset($rules['asset_tag']); + $rules['serials.*'] = $rules['serial']; + unset($rules['serial']); return $rules; } From d167ec6dc0ec8b87fbb7522c53ba37848d030e51 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 1 Nov 2023 13:36:35 -0500 Subject: [PATCH 07/10] unique undeleted recreated with laravel rule --- app/Http/Requests/StoreAssetRequest.php | 6 ------ app/Models/Asset.php | 5 ++++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/app/Http/Requests/StoreAssetRequest.php b/app/Http/Requests/StoreAssetRequest.php index 2fb2544160..254895f134 100644 --- a/app/Http/Requests/StoreAssetRequest.php +++ b/app/Http/Requests/StoreAssetRequest.php @@ -35,12 +35,6 @@ class StoreAssetRequest extends ImageUploadRequest parent::rules(), ); - // unsets unique check here, that check cannot run twice. - $rules['asset_tags.*'] = $rules['asset_tag']; - unset($rules['asset_tag']); - $rules['serials.*'] = $rules['serial']; - unset($rules['serial']); - return $rules; } } diff --git a/app/Models/Asset.php b/app/Models/Asset.php index 7f4f691628..fac914dcb0 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -100,7 +100,10 @@ class Asset extends Depreciable 'expected_checkin' => 'date|nullable', 'location_id' => 'exists:locations,id|nullable', 'rtd_location_id' => 'exists:locations,id|nullable', - 'asset_tag' => 'required|min:1|max:255|unique_undeleted', + // okay, i know this looks scary - but it's not. it checks to see if the asset tag is unique, but only if the asset is not deleted - `NULL,NULL` says to ignore the unique rule if following conditions are met + // https://laracasts.com/discuss/channels/laravel/unique-validation-with-soft-delete + // this is tested and verified + 'asset_tag' => 'required|min:1|max:255|unique:assets,asset_tag,NULL,NULL,deleted_at,NULL', 'purchase_date' => 'date|date_format:Y-m-d|nullable', 'serial' => 'unique_serial|nullable', 'purchase_cost' => 'numeric|nullable|gte:0', From c9604b896a0c777cb180dfea55e731867e914932 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 1 Nov 2023 13:46:32 -0500 Subject: [PATCH 08/10] nevermind --- app/Models/Asset.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/Models/Asset.php b/app/Models/Asset.php index fac914dcb0..a7d4d7442a 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -100,10 +100,7 @@ class Asset extends Depreciable 'expected_checkin' => 'date|nullable', 'location_id' => 'exists:locations,id|nullable', 'rtd_location_id' => 'exists:locations,id|nullable', - // okay, i know this looks scary - but it's not. it checks to see if the asset tag is unique, but only if the asset is not deleted - `NULL,NULL` says to ignore the unique rule if following conditions are met - // https://laracasts.com/discuss/channels/laravel/unique-validation-with-soft-delete - // this is tested and verified - 'asset_tag' => 'required|min:1|max:255|unique:assets,asset_tag,NULL,NULL,deleted_at,NULL', + 'asset_tag' => 'required|min:1|max:255|unique_undeleted:assets,asset_tag', 'purchase_date' => 'date|date_format:Y-m-d|nullable', 'serial' => 'unique_serial|nullable', 'purchase_cost' => 'numeric|nullable|gte:0', From 938ec75aa70b7e9da3c264b2a57e378283fce389 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 1 Nov 2023 14:09:03 -0500 Subject: [PATCH 09/10] clarifying note --- app/Providers/ValidationServiceProvider.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/Providers/ValidationServiceProvider.php b/app/Providers/ValidationServiceProvider.php index 70fa64702e..30c5a0197e 100644 --- a/app/Providers/ValidationServiceProvider.php +++ b/app/Providers/ValidationServiceProvider.php @@ -48,6 +48,8 @@ class ValidationServiceProvider extends ServiceProvider // Unique only if undeleted // This works around the use case where multiple deleted items have the same unique attribute. // (I think this is a bug in Laravel's validator?) + // $parameters is the rule parameters, like `unique_undeleted:users,id` - $parameters[0] is users, $parameters[1] is id + // the UniqueUndeletedTrait prefills these so you can just use `unique_undeleted` in your rules (but this would only work directly in the model) Validator::extend('unique_undeleted', function ($attribute, $value, $parameters, $validator) { if (count($parameters)) { $count = DB::table($parameters[0])->select('id')->where($attribute, '=', $value)->whereNull('deleted_at')->where('id', '!=', $parameters[1])->count(); From 75532d96622f77324be8fb6a3a3b4fac3ecb7130 Mon Sep 17 00:00:00 2001 From: spencerrlongg Date: Wed, 1 Nov 2023 16:49:59 -0500 Subject: [PATCH 10/10] adds permission test --- tests/Feature/Api/Assets/AssetStoreTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 tests/Feature/Api/Assets/AssetStoreTest.php diff --git a/tests/Feature/Api/Assets/AssetStoreTest.php b/tests/Feature/Api/Assets/AssetStoreTest.php new file mode 100644 index 0000000000..5a68aebccb --- /dev/null +++ b/tests/Feature/Api/Assets/AssetStoreTest.php @@ -0,0 +1,19 @@ +actingAsForApi(User::factory()->create()) + ->postJson(route('api.assets.store')) + ->assertForbidden(); + } +}