Merge pull request #18232 from grokability/check-for-valid-json-on-filter

Added filter form request to validate JSON

app/Http/Controllers/Api/AssetsController.php

@@ -3,37 +3,38 @@
 namespace App\Http\Controllers\Api;
 
 use App\Events\CheckoutableCheckedIn;
-use App\Http\Requests\StoreAssetRequest;
-use App\Http\Requests\UpdateAssetRequest;
-use App\Http\Traits\MigratesLegacyAssetLocations;
-use App\Http\Transformers\ComponentsTransformer;
-use App\Models\AccessoryCheckout;
-use App\Models\CheckoutAcceptance;
-use App\Models\LicenseSeat;
-use Illuminate\Database\Eloquent\Builder;
-use Illuminate\Http\JsonResponse;
-use Illuminate\Support\Facades\Crypt;
-use Illuminate\Support\Facades\Gate;
 use App\Helpers\Helper;
 use App\Http\Controllers\Controller;
 use App\Http\Requests\AssetCheckoutRequest;
+use App\Http\Requests\FilterRequest;
+use App\Http\Requests\StoreAssetRequest;
+use App\Http\Requests\UpdateAssetRequest;
+use App\Http\Traits\MigratesLegacyAssetLocations;
 use App\Http\Transformers\AssetsTransformer;
+use App\Http\Transformers\ComponentsTransformer;
 use App\Http\Transformers\LicensesTransformer;
 use App\Http\Transformers\SelectlistTransformer;
+use App\Models\AccessoryCheckout;
 use App\Models\Asset;
 use App\Models\AssetModel;
+use App\Models\CheckoutAcceptance;
 use App\Models\Company;
 use App\Models\CustomField;
 use App\Models\License;
+use App\Models\LicenseSeat;
 use App\Models\Location;
 use App\Models\Setting;
 use App\Models\User;
+use App\View\Label;
 use Carbon\Carbon;
-use Illuminate\Support\Facades\DB;
+use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
+use Illuminate\Support\Facades\Crypt;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Gate;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Route;
-use App\View\Label;
 use Illuminate\Support\Facades\Storage;
 use Illuminate\Support\Facades\Validator;
 
@@ -56,7 +57,7 @@ class AssetsController extends Controller
      * @param int $assetId
      * @since [v4.0]
      */
-    public function index(Request $request, $action = null, $upcoming_status = null) : JsonResponse | array
+    public function index(FilterRequest $request, $action = null, $upcoming_status = null) : JsonResponse | array
     {
 
 

app/Http/Controllers/Api/UsersController.php

@@ -31,6 +31,7 @@ use Illuminate\Support\Facades\Validator;
 use Illuminate\Support\Facades\Log;
 use App\Http\Requests\DeleteUserRequest;
 use Illuminate\Http\JsonResponse;
+use App\Http\Requests\FilterRequest;
 
 class UsersController extends Controller
 {
@@ -42,7 +43,7 @@ class UsersController extends Controller
      *
      * @return array
      */
-    public function index(Request $request) : array
+    public function index(FilterRequest $request) : array
     {
         $this->authorize('view', User::class);
 

app/Http/Requests/FilterRequest.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace App\Http\Requests;
+
+use App\Rules\ValidJson;
+use Illuminate\Foundation\Http\FormRequest;
+
+class FilterRequest extends FormRequest
+{
+    /**
+     * Determine if the user is authorized to make this request.
+     */
+    public function authorize(): bool
+    {
+        return true;
+    }
+
+    /**
+     * Get the validation rules that apply to the request.
+     *
+     * @return array<string, \Illuminate\Contracts\Validation\ValidationRule|array<mixed>|string>
+     */
+    public function rules(): array
+    {
+        return [
+            'filter' => ['nullable', new ValidJson()],
+        ];
+    }
+}

app/Rules/ValidJson.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace App\Rules;
+
+use Closure;
+use Illuminate\Contracts\Validation\ValidationRule;
+
+class ValidJson implements ValidationRule
+{
+    /**
+     * Run the validation rule.
+     *
+     * @param  \Closure(string, ?string=): \Illuminate\Translation\PotentiallyTranslatedString  $fail
+     */
+    public function validate(string $attribute, mixed $value, Closure $fail): void
+    {
+        if (!json_validate($value)) {
+            $fail(trans('validation.json'));
+        }
+    }
+}

tests/Feature/Assets/Api/AssetIndexTest.php

@@ -171,4 +171,33 @@ class AssetIndexTest extends TestCase
             ->assertResponseDoesNotContainInRows($assetA, 'asset_tag')
             ->assertResponseContainsInRows($assetB, 'asset_tag');
     }
+
+    public function test_gracefully_handles_malformed_filter()
+    {
+        $this->actingAsForApi(User::factory()->viewAssets()->create())
+            ->getJson(route('api.assets.index', [
+                // filter should be a json encoded array and not a string
+                'filter' => 'asset_tag:12345',
+            ]))
+            ->assertStatusMessageIs('error')
+            ->assertJson(function (AssertableJson $json) {
+                $json->has('messages.filter')->etc();
+            });
+    }
+
+    public function testReturnsResultViaFilter()
+    {
+
+        Asset::factory()->count(3)->create(['name' => 'MY AWESOME ASSET NAME']);
+        $this->actingAsForApi(User::factory()->viewAssets()->create())
+            ->getJson(route('api.assets.index', [
+                'filter' => '{"name":"MY AWESOME ASSET NAME"}',
+            ]))
+            ->assertOk()
+            ->assertJsonStructure([
+                'total',
+                'rows',
+            ])
+            ->assertJson(fn(AssertableJson $json) => $json->has('rows', 3)->etc());
+    }
 }

tests/Feature/Users/Api/IndexUsersTest.php

@@ -65,6 +65,36 @@ class IndexUsersTest extends TestCase
                 // filter should be a json encoded array and not a string
                 'filter' => 'email:an-email-address@example.com',
             ]))
-            ->assertOk();
+            ->assertStatusMessageIs('error')
+            ->assertJson(function (AssertableJson $json) {
+                $json->has('messages.filter')->etc();
+            });
+    }
+
+    public function testReturnsResultViaFilter()
+    {
+
+        User::factory()->count(3)->create(['first_name' => 'Awesome', 'last_name' => 'Admin', 'email' => 'awesome@example.org']);
+        $this->actingAsForApi(User::factory()->viewUsers()->create())
+            ->getJson(route('api.users.index', [
+                'filter' => '{"first_name":"Awesome","last_name":"Admin","email":"awesome@example.org"}',
+            ]))
+            ->assertOk()
+            ->assertJsonStructure([
+                'total',
+                'rows',
+            ])
+            ->assertJson(fn(AssertableJson $json) => $json->has('rows', 3)->etc());
+
+        $this->actingAsForApi(User::factory()->viewUsers()->create())
+            ->getJson(route('api.users.index', [
+                'filter' => '{"first_name":"Not Awesome"}',
+            ]))
+            ->assertOk()
+            ->assertJsonStructure([
+                'total',
+                'rows',
+            ])
+            ->assertJson(fn(AssertableJson $json) => $json->has('rows', 0)->etc());
     }
 }