Acceptances: Fixed FD-55978 - Cross-company deletion of pending checkout acceptances via unscoped report endpoint

2026-06-12 19:15:36 +01:00
parent b40e227ad3
commit 802067f398
2 changed files with 139 additions and 2 deletions
@@ -803,7 +803,7 @@ class ReportsController extends Controller
                    ->where('item_type', '=', Asset::class)
                    ->whereBetween('action_date', [$checkout_start, $checkout_end]); // we are *not* doing ->get()...

-                $assets->whereIn('id', $actionlogassets); //...because this _should_ act as a 'subquery'
+                $assets->whereIn('id', $actionlogassets); // ...because this _should_ act as a 'subquery'
            }

            if (($request->filled('checkin_date_start'))) {
@@ -1324,6 +1324,11 @@ class ReportsController extends Controller
            // Redirect to the unaccepted items report page with error
            return redirect()->route('reports/unaccepted_assets')->with('error', trans('general.bad_data'));
        }
+
+        if (! $this->currentUserCanAccessAcceptance($acceptance)) {
+            return redirect()->route('reports/unaccepted_assets')->with('error', trans('general.insufficient_permissions'));
+        }
+
        $item = $acceptance->checkoutable;
        $assignee = $acceptance->assignedTo ?? $item->assignedTo ?? null;
        $email = $assignee?->email;
@@ -1358,6 +1363,33 @@ class ReportsController extends Controller
        return redirect()->route('reports/unaccepted_assets')->with('success', trans('admin/reports/general.reminder_sent'));
    }

+    private function currentUserCanAccessAcceptance(CheckoutAcceptance $acceptance): bool
+    {
+        if (Setting::getSettings()->full_multiple_companies_support != '1') {
+            return true;
+        }
+
+        $user = auth()->user();
+
+        if (! $user->company_id || $user->isSuperUser()) {
+            return true;
+        }
+
+        // Bypass Eloquent global scopes so cross-company items are still found
+        $checkoutableType = $acceptance->checkoutable_type;
+        $checkoutable = $checkoutableType::withoutGlobalScopes()->find($acceptance->checkoutable_id);
+
+        if ($checkoutable instanceof LicenseSeat) {
+            $itemCompanyId = License::withoutGlobalScopes()
+                ->where('id', $checkoutable->license_id)
+                ->value('company_id');
+        } else {
+            $itemCompanyId = $checkoutable?->company_id;
+        }
+
+        return $itemCompanyId === null || (int) $itemCompanyId === (int) $user->company_id;
+    }
+
    private function getCheckoutMailType(CheckoutAcceptance $acceptance, $logItem): Mailable
    {
        $lookup = [
@@ -1390,11 +1422,21 @@ class ReportsController extends Controller
    {
        $this->authorize('reports.view');

-        if (! $acceptance = CheckoutAcceptance::pending()->find($acceptanceId)) {
+        $acceptance = CheckoutAcceptance::pending()
+            ->with(['checkoutable' => function (MorphTo $morphTo) {
+                $morphTo->morphWith([LicenseSeat::class => ['license']]);
+            }])
+            ->find($acceptanceId);
+
+        if (! $acceptance) {
            // Redirect to the unaccepted assets report page with error
            return redirect()->route('reports/unaccepted_assets')->with('error', trans('general.bad_data'));
        }

+        if (! $this->currentUserCanAccessAcceptance($acceptance)) {
+            return redirect()->route('reports/unaccepted_assets')->with('error', trans('general.insufficient_permissions'));
+        }
+
        if ($acceptance->delete()) {
            return redirect()->route('reports/unaccepted_assets')->with('success', trans('admin/reports/general.acceptance_deleted'));
        } else {
@@ -0,0 +1,95 @@
+<?php
+
+namespace Tests\Feature\Reporting;
+
+use App\Models\Asset;
+use App\Models\CheckoutAcceptance;
+use App\Models\Company;
+use App\Models\User;
+use Tests\TestCase;
+
+class DeleteAcceptanceAuthorizationTest extends TestCase
+{
+    public function test_user_without_reports_view_cannot_delete_acceptance()
+    {
+        $acceptance = CheckoutAcceptance::factory()->pending()->create();
+
+        $this->actingAs(User::factory()->create())
+            ->delete(route('reports/unaccepted_assets_delete', $acceptance->id))
+            ->assertForbidden();
+
+        $this->assertNull($acceptance->fresh()->deleted_at);
+    }
+
+    public function test_reports_user_can_delete_acceptance_for_their_own_company()
+    {
+        $this->settings->enableMultipleFullCompanySupport();
+
+        [$companyA] = Company::factory()->count(2)->create();
+
+        $asset = Asset::factory()->create(['company_id' => $companyA->id]);
+        $reporter = User::factory()->canViewReports()->create(['company_id' => $companyA->id]);
+        $acceptance = CheckoutAcceptance::factory()->pending()->for($asset, 'checkoutable')->create();
+
+        $this->actingAs($reporter)
+            ->delete(route('reports/unaccepted_assets_delete', $acceptance->id))
+            ->assertRedirectToRoute('reports/unaccepted_assets')
+            ->assertSessionHas('success');
+
+        $this->assertNotNull($acceptance->fresh()->deleted_at);
+    }
+
+    public function test_reports_user_cannot_delete_acceptance_belonging_to_another_company()
+    {
+        $this->settings->enableMultipleFullCompanySupport();
+
+        [$companyA, $companyB] = Company::factory()->count(2)->create();
+
+        $assetB = Asset::factory()->create(['company_id' => $companyB->id]);
+        $reporter = User::factory()->canViewReports()->create(['company_id' => $companyA->id]);
+        $acceptance = CheckoutAcceptance::factory()->pending()->for($assetB, 'checkoutable')->create();
+
+        $this->actingAs($reporter)
+            ->delete(route('reports/unaccepted_assets_delete', $acceptance->id))
+            ->assertRedirectToRoute('reports/unaccepted_assets')
+            ->assertSessionHas('error');
+
+        $this->assertNull($acceptance->fresh()->deleted_at);
+    }
+
+    public function test_superuser_can_delete_acceptance_from_any_company()
+    {
+        $this->settings->enableMultipleFullCompanySupport();
+
+        [$companyA, $companyB] = Company::factory()->count(2)->create();
+
+        $assetB = Asset::factory()->create(['company_id' => $companyB->id]);
+        $superuser = User::factory()->superuser()->create(['company_id' => $companyA->id]);
+        $acceptance = CheckoutAcceptance::factory()->pending()->for($assetB, 'checkoutable')->create();
+
+        $this->actingAs($superuser)
+            ->delete(route('reports/unaccepted_assets_delete', $acceptance->id))
+            ->assertRedirectToRoute('reports/unaccepted_assets')
+            ->assertSessionHas('success');
+
+        $this->assertNotNull($acceptance->fresh()->deleted_at);
+    }
+
+    public function test_company_scoping_not_enforced_when_fmcs_disabled()
+    {
+        $this->settings->disableMultipleFullCompanySupport();
+
+        [$companyA, $companyB] = Company::factory()->count(2)->create();
+
+        $assetB = Asset::factory()->create(['company_id' => $companyB->id]);
+        $reporter = User::factory()->canViewReports()->create(['company_id' => $companyA->id]);
+        $acceptance = CheckoutAcceptance::factory()->pending()->for($assetB, 'checkoutable')->create();
+
+        $this->actingAs($reporter)
+            ->delete(route('reports/unaccepted_assets_delete', $acceptance->id))
+            ->assertRedirectToRoute('reports/unaccepted_assets')
+            ->assertSessionHas('success');
+
+        $this->assertNotNull($acceptance->fresh()->deleted_at);
+    }
+}