Checkouts: Fixed FD-56263 - disallow checkouts to soft-deleted targets

app/Http/Controllers/Api/AssetsController.php

@@ -1252,6 +1252,14 @@ class AssetsController extends Controller
             $error_payload['target_type'] = 'user';
         }
 
+        // withoutGlobalScopes above bypasses the SoftDeletes scope so we
+        // can distinguish "target not found" from "target in another company"
+        // for FMCS error messaging. Trashed targets must not be treated as
+        // valid checkout destinations though, so exclude them here.
+        if (isset($target) && ! empty($target->deleted_at)) {
+            $target = null;
+        }
+
         if ($request->filled('status_id')) {
             $asset->status_id = $request->input('status_id');
         }

app/Http/Controllers/Api/ComponentsController.php

@@ -326,6 +326,13 @@ class ComponentsController extends Controller
             // Scoped lookup can hide cross-company records and lead to partial writes.
             $asset = Asset::withoutGlobalScopes()->find($request->input('assigned_to'));
 
+            // withoutGlobalScopes bypasses SoftDeletes so we can distinguish
+            // "no such asset" from "in another company" for FMCS messaging.
+            // Trashed assets must not be treated as valid checkout targets.
+            if ($asset && ! empty($asset->deleted_at)) {
+                $asset = null;
+            }
+
             if (! $asset) {
                 return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/hardware/message.does_not_exist')));
             }

app/Http/Controllers/Api/ConsumablesController.php

@@ -316,8 +316,16 @@ class ConsumablesController extends Controller
 
         // Resolve the raw target first, then enforce FMCS explicitly.
         // Scoped lookup can hide cross-company users and make failures ambiguous.
-        if (! $user = User::withoutGlobalScopes()->find($request->input('assigned_to'))) {
-            // Return error message
+        $user = User::withoutGlobalScopes()->find($request->input('assigned_to'));
+
+        // withoutGlobalScopes bypasses SoftDeletes so we can tell "no such
+        // user" from "user in another company" for FMCS messaging. Trashed
+        // users must not be treated as valid checkout targets.
+        if ($user && ! empty($user->deleted_at)) {
+            $user = null;
+        }
+
+        if (! $user) {
             return response()->json(Helper::formatStandardApiResponse('error', null, 'No user found'));
         }
 

app/Http/Requests/AccessoryCheckoutRequest.php

@@ -44,9 +44,9 @@ class AccessoryCheckoutRequest extends ImageUploadRequest
 
         return array_merge(
             [
-                'assigned_user' => 'required_without_all:assigned_asset,assigned_location',
-                'assigned_asset' => 'required_without_all:assigned_user,assigned_location',
-                'assigned_location' => 'required_without_all:assigned_user,assigned_asset',
+                'assigned_user' => 'required_without_all:assigned_asset,assigned_location|nullable|exists_undeleted:users,id',
+                'assigned_asset' => 'required_without_all:assigned_user,assigned_location|nullable|exists_undeleted:assets,id',
+                'assigned_location' => 'required_without_all:assigned_user,assigned_asset|nullable|exists_undeleted:locations,id',
 
                 'number_remaining_after_checkout' => [
                     'min:0',

app/Http/Requests/AssetCheckoutRequest.php

@@ -26,9 +26,13 @@ class AssetCheckoutRequest extends Request
         $settings = Setting::getSettings();
 
         $rules = [
-            'assigned_user' => 'numeric|nullable|required_without_all:assigned_asset,assigned_location',
-            'assigned_asset' => 'numeric|nullable|required_without_all:assigned_user,assigned_location',
-            'assigned_location' => 'numeric|nullable|required_without_all:assigned_user,assigned_asset',
+            // exists_undeleted rejects soft-deleted checkout targets so the
+            // controllers below cannot bind live inventory to trashed users,
+            // assets, or locations. Applied at request-validation time so a
+            // bad request bounces with 422 before any controller mutation.
+            'assigned_user' => 'numeric|nullable|required_without_all:assigned_asset,assigned_location|exists_undeleted:users,id',
+            'assigned_asset' => 'numeric|nullable|required_without_all:assigned_user,assigned_location|exists_undeleted:assets,id',
+            'assigned_location' => 'numeric|nullable|required_without_all:assigned_user,assigned_asset|exists_undeleted:locations,id',
             'status_id' => 'nullable|exists:status_labels,id,deployable,1',
             'checkout_to_type' => 'required|in:asset,location,user',
             'checkout_at' => [

tests/Feature/Checkouts/Api/CheckoutToSoftDeletedTargetTest.php

@@ -0,0 +1,205 @@
+<?php
+
+namespace Tests\Feature\Checkouts\Api;
+
+use App\Models\Accessory;
+use App\Models\Asset;
+use App\Models\Component;
+use App\Models\Consumable;
+use App\Models\Location;
+use App\Models\User;
+use Tests\TestCase;
+
+/**
+ * Regression tests confirming the API checkout endpoints refuse to bind
+ * live inventory to soft-deleted targets. The vulnerable resolution used
+ * Model::withoutGlobalScopes()->find(...) (added deliberately for FMCS
+ * error messaging) without a post-lookup deleted_at guard, so trashed
+ * users, assets, and locations were accepted as checkout destinations.
+ *
+ * Fix covers three layers:
+ *   1. exists_undeleted validator on the AssetCheckoutRequest and
+ *      AccessoryCheckoutRequest (422 bounce at request time).
+ *   2. Post-withoutGlobalScopes deleted_at check in each of the four API
+ *      checkout controllers.
+ *   3. This regression suite locks in "trashed target => rejected" for
+ *      every affected endpoint.
+ */
+class CheckoutToSoftDeletedTargetTest extends TestCase
+{
+    // -------------------------------------------------------------------------
+    // Asset checkout
+    // -------------------------------------------------------------------------
+
+    public function test_asset_checkout_rejects_soft_deleted_user_target()
+    {
+        $asset = Asset::factory()->create();
+        $targetUser = User::factory()->create();
+        $targetUser->delete();
+
+        $this->actingAsForApi(User::factory()->superuser()->create())
+            ->postJson(route('api.asset.checkout', $asset), [
+                'checkout_to_type' => 'user',
+                'assigned_user' => $targetUser->id,
+            ])
+            ->assertStatusMessageIs('error');
+
+        $this->assertDatabaseMissing('assets', [
+            'id' => $asset->id,
+            'assigned_to' => $targetUser->id,
+        ]);
+    }
+
+    public function test_asset_checkout_rejects_soft_deleted_asset_target()
+    {
+        $asset = Asset::factory()->create();
+        $targetAsset = Asset::factory()->create();
+        $targetAsset->delete();
+
+        $this->actingAsForApi(User::factory()->superuser()->create())
+            ->postJson(route('api.asset.checkout', $asset), [
+                'checkout_to_type' => 'asset',
+                'assigned_asset' => $targetAsset->id,
+            ])
+            ->assertStatusMessageIs('error');
+
+        $this->assertDatabaseMissing('assets', [
+            'id' => $asset->id,
+            'assigned_to' => $targetAsset->id,
+            'assigned_type' => Asset::class,
+        ]);
+    }
+
+    public function test_asset_checkout_rejects_soft_deleted_location_target()
+    {
+        // Reporter flagged this case as "unverified due to FMCS mismatch" but
+        // the code path is identical to the other two, so the fix must cover it.
+        $asset = Asset::factory()->create();
+        $targetLocation = Location::factory()->create();
+        $targetLocation->delete();
+
+        $this->actingAsForApi(User::factory()->superuser()->create())
+            ->postJson(route('api.asset.checkout', $asset), [
+                'checkout_to_type' => 'location',
+                'assigned_location' => $targetLocation->id,
+            ])
+            ->assertStatusMessageIs('error');
+
+        $this->assertDatabaseMissing('assets', [
+            'id' => $asset->id,
+            'assigned_to' => $targetLocation->id,
+            'assigned_type' => Location::class,
+        ]);
+    }
+
+    // -------------------------------------------------------------------------
+    // Consumable checkout
+    // -------------------------------------------------------------------------
+
+    public function test_consumable_checkout_rejects_soft_deleted_user_target()
+    {
+        $consumable = Consumable::factory()->create();
+        $target = User::factory()->create();
+        $target->delete();
+
+        $this->actingAsForApi(User::factory()->checkoutConsumables()->create())
+            ->postJson(route('api.consumables.checkout', $consumable), [
+                'assigned_to' => $target->id,
+                'checkout_qty' => 1,
+            ])
+            ->assertStatusMessageIs('error');
+
+        $this->assertDatabaseMissing('consumables_users', [
+            'consumable_id' => $consumable->id,
+            'assigned_to' => $target->id,
+        ]);
+    }
+
+    // -------------------------------------------------------------------------
+    // Component checkout
+    // -------------------------------------------------------------------------
+
+    public function test_component_checkout_rejects_soft_deleted_asset_target()
+    {
+        $component = Component::factory()->create();
+        $target = Asset::factory()->create();
+        $target->delete();
+
+        $this->actingAsForApi(User::factory()->superuser()->create())
+            ->postJson(route('api.components.checkout', $component->id), [
+                'assigned_to' => $target->id,
+                'assigned_qty' => 1,
+            ])
+            ->assertStatusMessageIs('error');
+
+        $this->assertDatabaseMissing('components_assets', [
+            'component_id' => $component->id,
+            'asset_id' => $target->id,
+        ]);
+    }
+
+    // -------------------------------------------------------------------------
+    // Accessory checkout (defense-in-depth: not vulnerable to this class of
+    // bug because the accessory API uses the CheckInOutTrait's findOrFail
+    // path which respects the SoftDeletes scope, but the exists_undeleted
+    // rule on AccessoryCheckoutRequest catches the trashed target at request
+    // validation time regardless).
+    // -------------------------------------------------------------------------
+
+    public function test_accessory_checkout_rejects_soft_deleted_user_target()
+    {
+        $accessory = Accessory::factory()->create();
+        $target = User::factory()->create();
+        $target->delete();
+
+        $this->actingAsForApi(User::factory()->superuser()->create())
+            ->postJson(route('api.accessories.checkout', $accessory), [
+                'assigned_user' => $target->id,
+                'checkout_to_type' => 'user',
+                'checkout_qty' => 1,
+            ])
+            ->assertStatusMessageIs('error');
+
+        $this->assertDatabaseMissing('accessories_checkout', [
+            'accessory_id' => $accessory->id,
+            'assigned_to' => $target->id,
+            'assigned_type' => User::class,
+        ]);
+    }
+
+    // -------------------------------------------------------------------------
+    // Happy paths (make sure the fix doesn't over-block)
+    // -------------------------------------------------------------------------
+
+    public function test_asset_checkout_still_works_with_live_user_target()
+    {
+        $asset = Asset::factory()->create();
+        $target = User::factory()->create();
+
+        $this->actingAsForApi(User::factory()->superuser()->create())
+            ->postJson(route('api.asset.checkout', $asset), [
+                'checkout_to_type' => 'user',
+                'assigned_user' => $target->id,
+            ])
+            ->assertStatusMessageIs('success');
+
+        $this->assertDatabaseHas('assets', [
+            'id' => $asset->id,
+            'assigned_to' => $target->id,
+            'assigned_type' => User::class,
+        ]);
+    }
+
+    public function test_consumable_checkout_still_works_with_live_user_target()
+    {
+        $consumable = Consumable::factory()->create();
+        $target = User::factory()->create();
+
+        $this->actingAsForApi(User::factory()->checkoutConsumables()->create())
+            ->postJson(route('api.consumables.checkout', $consumable), [
+                'assigned_to' => $target->id,
+                'checkout_qty' => 1,
+            ])
+            ->assertStatusMessageIs('success');
+    }
+}