Print Views: Fixed FD-56677 - Tighter constraints on print views

2026-08-18 11:15:42 +00:00 · 2026-07-25 16:45:12 +01:00
parent b3f12f974b
commit 7865bc56e3
6 changed files with 273 additions and 71 deletions
--- a/app/Http/Controllers/LocationsController.php
+++ b/app/Http/Controllers/LocationsController.php
@ -4,15 +4,20 @@ namespace App\Http\Controllers;

 use App\Helpers\Helper;
 use App\Http\Requests\ImageUploadRequest;
+use App\Models\Accessory;
 use App\Models\Actionlog;
 use App\Models\Asset;
 use App\Models\Company;
+use App\Models\Component;
+use App\Models\Consumable;
 use App\Models\Location;
 use App\Models\Setting;
 use App\Models\User;
 use Illuminate\Contracts\View\View;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
+use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\Gate;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Storage;

@ -291,17 +296,7 @@ class LocationsController extends Controller
        $this->authorize('view', Location::class);

        if ($location = Location::where('id', $id)->first()) {
-            return view('locations/print')
-                ->with('assigned', false)
-                ->with('assets', $location->assets)
-                ->with('assignedAssets', $location->assignedAssets)
-                ->with('accessories', $location->accessories)
-                ->with('assignedAccessories', $location->assignedAccessories)
-                ->with('users', $location->users()->with('companies')->get())
-                ->with('location', $location)
-                ->with('consumables', $location->consumables)
-                ->with('components', $location->components)
-                ->with('children', $location->children);
+            return view('locations/print', $this->printPayload($location, assigned: false));
        }

        return redirect()->route('locations.index')->with('error', trans('admin/locations/message.does_not_exist'));
@ -310,23 +305,50 @@ class LocationsController extends Controller
    public function print_all_assigned($id): View|RedirectResponse
    {
        $this->authorize('view', Location::class);
+
        if ($location = Location::where('id', $id)->first()) {
-            return view('locations/print')
-                ->with('assigned', true)
-                ->with('assets', $location->assets)
-                ->with('assignedAssets', $location->assignedAssets)
-                ->with('accessories', $location->accessories)
-                ->with('assignedAccessories', $location->assignedAccessories)
-                ->with('users', $location->users()->with('companies')->get())
-                ->with('location', $location)
-                ->with('consumables', $location->consumables)
-                ->with('components', $location->components)
-                ->with('children', $location->children);
+            return view('locations/print', $this->printPayload($location, assigned: true));
        }

        return redirect()->route('locations.index')->with('error', trans('admin/locations/message.does_not_exist'));
    }

+    /**
+     * Build the per-model related collections the print sheet renders.
+     * The route only gates on `view` for Location, but the view rendered
+     * the assigned users / assets / accessories / consumables /
+     * components inline without matching per-model permission checks -
+     * so a caller with locations.view but not users.view could read
+     * assigned users' identity through the printassigned URL, even
+     * though /users/{id} would 403 for them.
+     *
+     * Substitute an empty Collection for each relation the caller can't
+     * view. The existing `@if ($users->count() > 0)` guards in
+     * locations/print.blade.php then naturally skip the entire block,
+     * matching the per-model @can guards used by the standard
+     * locations/view.blade.php page. Instance-context gate checks pass
+     * the model class so FMCS scoping still applies.
+     */
+    private function printPayload(Location $location, bool $assigned): array
+    {
+        $empty = new Collection;
+
+        return [
+            'assigned' => $assigned,
+            'location' => $location,
+            'assets' => Gate::allows('view', Asset::class) ? $location->assets : $empty,
+            'assignedAssets' => Gate::allows('view', Asset::class) ? $location->assignedAssets : $empty,
+            'accessories' => Gate::allows('view', Accessory::class) ? $location->accessories : $empty,
+            'assignedAccessories' => Gate::allows('view', Accessory::class) ? $location->assignedAccessories : $empty,
+            'users' => Gate::allows('view', User::class) ? $location->users()->with('companies')->get() : $empty,
+            'consumables' => Gate::allows('view', Consumable::class) ? $location->consumables : $empty,
+            'components' => Gate::allows('view', Component::class) ? $location->components : $empty,
+            // Child locations key off the same locations.view permission the
+            // outer authorize() already required, so no further gate here.
+            'children' => $location->children,
+        ];
+    }
+
    /**
     * Returns a view that presents a form to clone a location.
     *
--- a/app/Http/Controllers/Users/BulkUsersController.php
+++ b/app/Http/Controllers/Users/BulkUsersController.php
@ -9,6 +9,8 @@ use App\Models\Accessory;
 use App\Models\Actionlog;
 use App\Models\Asset;
 use App\Models\Company;
+use App\Models\Component;
+use App\Models\Consumable;
 use App\Models\ConsumableAssignment;
 use App\Models\Group;
 use App\Models\License;
@ -106,26 +108,23 @@ class BulkUsersController extends Controller
                return redirect()->back()->with('success', trans('admin/users/message.password_resets_sent'));

            } elseif ($request->input('bulk_actions') == 'print') {
-                $users = User::query()
-                    ->with([
-                        'assets.assetlog',
-                        'assets.assignedAssets.assetlog',
-                        'assets.assignedAssets.defaultLoc',
-                        'assets.assignedAssets.location',
-                        'assets.assignedAssets.model.category',
-                        'assets.defaultLoc',
-                        'assets.location',
-                        'assets.model.category',
-                        'accessories.assetlog',
-                        'accessories.category',
-                        'accessories.manufacturer',
-                        'consumables.assetlog',
-                        'consumables.category',
-                        'consumables.manufacturer',
-                        'licenses.category',
-                    ])
-                    ->withTrashed()
-                    ->findMany($request->input('ids'));
+                $actor = auth()->user();
+                $canViewAssets = $actor->can('view', Asset::class);
+                $canViewLicenses = $actor->can('view', License::class);
+                $canViewAccessories = $actor->can('view', Accessory::class);
+                $canViewConsumables = $actor->can('view', Consumable::class);
+                $canViewComponents = $actor->can('view', Component::class);
+
+                $users = collect($request->input('ids'))
+                    ->map(fn ($id) => User::withInventoryRelations(
+                        (int) $id,
+                        $canViewAssets,
+                        $canViewLicenses,
+                        $canViewAccessories,
+                        $canViewConsumables,
+                        $canViewComponents,
+                    )->first())
+                    ->filter();

                $users->each(fn ($user) => $this->authorize('view', $user));

--- a/app/Http/Controllers/Users/UsersController.php
+++ b/app/Http/Controllers/Users/UsersController.php
@ -15,6 +15,7 @@ use App\Models\Actionlog;
 use App\Models\Asset;
 use App\Models\CheckoutAcceptance;
 use App\Models\Company;
+use App\Models\Component;
 use App\Models\Consumable;
 use App\Models\Group;
 use App\Models\License;
@ -714,16 +715,36 @@ class UsersController extends Controller
        $this->authorize('view', User::class);

        $actor = auth()->user();
+        $canViewAssets = $actor->can('view', Asset::class);
        $canViewLicenses = $actor->can('view', License::class);
        $canViewAccessories = $actor->can('view', Accessory::class);
        $canViewConsumables = $actor->can('view', Consumable::class);
+        $canViewComponents = $actor->can('view', Component::class);

-        $user = User::withInventoryRelations($id, $canViewLicenses, $canViewAccessories, $canViewConsumables)->first();
+        $user = User::withInventoryRelations(
+            $id,
+            $canViewAssets,
+            $canViewLicenses,
+            $canViewAccessories,
+            $canViewConsumables,
+            $canViewComponents,
+        )->first();

-        $indirectItemsCount = $user?->assets?->flatMap->assignedAssets->count()
-            + $user?->assets?->flatMap->components->count()
-            + ($canViewLicenses ? $user?->assets?->flatMap->licenses->count() : 0)
-            + ($canViewAccessories ? $user?->assets?->flatMap->assignedAccessories->count() : 0);
+        $indirectItemsCount = 0;
+        if ($canViewAssets && $user?->assets) {
+            foreach ($user->assets as $asset) {
+                $indirectItemsCount += $asset->assignedAssets->count();
+                if ($canViewComponents) {
+                    $indirectItemsCount += $asset->components->count();
+                }
+                if ($canViewLicenses) {
+                    $indirectItemsCount += $asset->licenses->count();
+                }
+                if ($canViewAccessories) {
+                    $indirectItemsCount += $asset->assignedAccessories->count();
+                }
+            }
+        }

        if ($user) {
            $this->authorize('view', $user);
--- a/app/Models/User.php
+++ b/app/Models/User.php
@ -1731,31 +1731,57 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo

    }

-    public function scopeWithInventoryRelations($query, int $id, bool $withLicenses = true, bool $withAccessories = true, bool $withConsumables = true)
-    {
-        $with = [
-            'assets.log' => fn ($query) => $query->withTrashed()
-                ->where('target_type', User::class)
-                ->where('target_id', $id)
-                ->where('action_type', 'accepted'),
-            'assets.defaultLoc',
-            'assets.location',
-            'assets.model.category',
-            'assets.assignedAssets.log' => fn ($query) => $query->withTrashed()
-                ->where('target_type', User::class)
-                ->where('target_id', $id)
-                ->where('action_type', 'accepted'),
-            'assets.assignedAssets.assignedTo',
-            'assets.assignedAssets.defaultLoc',
-            'assets.assignedAssets.location',
-            'assets.assignedAssets.model.category',
-            'assets.components.category',
-        ];
+    public function scopeWithInventoryRelations(
+        $query,
+        int $id,
+        bool $withAssets = true,
+        bool $withLicenses = true,
+        bool $withAccessories = true,
+        bool $withConsumables = true,
+        bool $withComponents = true,
+    ) {
+        $with = [];
+
+        if ($withAssets) {
+            $with = array_merge($with, [
+                'assets.log' => fn ($query) => $query->withTrashed()
+                    ->where('target_type', User::class)
+                    ->where('target_id', $id)
+                    ->where('action_type', 'accepted'),
+                'assets.defaultLoc',
+                'assets.location',
+                'assets.model.category',
+                'assets.assignedAssets.log' => fn ($query) => $query->withTrashed()
+                    ->where('target_type', User::class)
+                    ->where('target_id', $id)
+                    ->where('action_type', 'accepted'),
+                'assets.assignedAssets.assignedTo',
+                'assets.assignedAssets.defaultLoc',
+                'assets.assignedAssets.location',
+                'assets.assignedAssets.model.category',
+            ]);
+
+            if ($withComponents) {
+                $with[] = 'assets.components.category';
+            }
+
+            if ($withLicenses) {
+                $with = array_merge($with, [
+                    'assets.licenses',
+                    'assets.licenses.category',
+                ]);
+            }
+
+            if ($withAccessories) {
+                $with = array_merge($with, [
+                    'assets.assignedAccessories',
+                    'assets.assignedAccessories.accessory.category',
+                ]);
+            }
+        }

        if ($withLicenses) {
            $with = array_merge($with, [
-                'assets.licenses',
-                'assets.licenses.category',
                'directLicenses.category',
                'licenses.category',
            ]);
@ -1763,8 +1789,6 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo

        if ($withAccessories) {
            $with = array_merge($with, [
-                'assets.assignedAccessories',
-                'assets.assignedAccessories.accessory.category',
                'accessories.log' => fn ($query) => $query->withTrashed()
                    ->where('target_type', User::class)
                    ->where('target_id', $id)
--- a/resources/views/users/print.blade.php
+++ b/resources/views/users/print.blade.php
@ -105,6 +105,7 @@
    </h3>
    <p></p>{{ trans('admin/users/general.all_assigned_list_generation')}} {{ Helper::getFormattedDateObject(now(), 'datetime', false) }}

+    @can('view', \App\Models\Asset::class)
    @if ($show_user->assets->count() > 0)
        @php
            $counter = 1;
@ -175,6 +176,7 @@
            </tbody>
        </table>
    @endif
+    @endcan

    @can('view', \App\Models\License::class)
        @if ($show_user->directlicenses->count() > 0)
@ -432,6 +434,7 @@
                    @endphp
                @endforeach
                @endcan
+                @can('view', \App\Models\Component::class)
                @foreach ($asset->components as $component)
                    @if($component)
                        <tr>
@ -446,6 +449,7 @@
                        $indirectAssignmentsCounter ++
                    @endphp
                @endforeach
+                @endcan
                @can('view', \App\Models\Accessory::class)
                @foreach ($asset->assignedAccessories as $indirectAccessory)
                    @if($indirectAccessory)
--- a/tests/Feature/Users/Ui/PrintUserInventoryTest.php
+++ b/tests/Feature/Users/Ui/PrintUserInventoryTest.php
@ -3,7 +3,9 @@
 namespace Tests\Feature\Users\Ui;

 use App\Models\Accessory;
+use App\Models\Asset;
 use App\Models\Company;
+use App\Models\Component;
 use App\Models\Consumable;
 use App\Models\License;
 use App\Models\LicenseSeat;
@ -139,4 +141,134 @@ class PrintUserInventoryTest extends TestCase
            ->assertOk()
            ->assertSee('Unique Consumable DEF456');
    }
+
+    public function test_user_without_assets_view_cannot_see_assigned_assets_in_print()
+    {
+        $subject = User::factory()->create();
+        Asset::factory()->assignedToUser($subject)->create([
+            'name' => 'Unique Asset LEAK111',
+            'asset_tag' => 'LEAKTAG-111',
+        ]);
+
+        $actor = User::factory()->viewUsers()->create();
+
+        $this->actingAs($actor)
+            ->get(route('users.print', $subject))
+            ->assertOk()
+            ->assertDontSee('Unique Asset LEAK111')
+            ->assertDontSee('LEAKTAG-111');
+    }
+
+    public function test_user_with_assets_view_can_see_assigned_assets_in_print()
+    {
+        $subject = User::factory()->create();
+        Asset::factory()->assignedToUser($subject)->create([
+            'name' => 'Unique Asset LEAK111',
+            'asset_tag' => 'LEAKTAG-111',
+        ]);
+
+        $actor = User::factory()->viewUsers()->viewAssets()->create();
+
+        $this->actingAs($actor)
+            ->get(route('users.print', $subject))
+            ->assertOk()
+            ->assertSee('LEAKTAG-111');
+    }
+
+    public function test_user_without_components_view_cannot_see_asset_components_in_print()
+    {
+        $this->settings->set(['show_assigned_assets' => 1]);
+
+        $subject = User::factory()->create();
+        $asset = Asset::factory()->assignedToUser($subject)->create();
+        $component = Component::factory()->create(['name' => 'Unique Component COMP222']);
+        $asset->components()->attach($component->id, [
+            'assigned_qty' => 1,
+            'created_by' => $subject->id,
+            'created_at' => now(),
+        ]);
+
+        $actor = User::factory()->viewUsers()->viewAssets()->create();
+
+        $this->actingAs($actor)
+            ->get(route('users.print', $subject))
+            ->assertOk()
+            ->assertDontSee('Unique Component COMP222');
+    }
+
+    public function test_user_with_components_view_can_see_asset_components_in_print()
+    {
+        $this->settings->set(['show_assigned_assets' => 1]);
+
+        $subject = User::factory()->create();
+        $asset = Asset::factory()->assignedToUser($subject)->create();
+        $component = Component::factory()->create(['name' => 'Unique Component COMP222']);
+        $asset->components()->attach($component->id, [
+            'assigned_qty' => 1,
+            'created_by' => $subject->id,
+            'created_at' => now(),
+        ]);
+
+        $actor = User::factory()->viewUsers()->viewAssets()->viewComponents()->create();
+
+        $this->actingAs($actor)
+            ->get(route('users.print', $subject))
+            ->assertOk()
+            ->assertSee('Unique Component COMP222');
+    }
+
+    public function test_bulk_print_without_assets_view_cannot_see_assigned_assets()
+    {
+        $subject = User::factory()->create();
+        Asset::factory()->assignedToUser($subject)->create([
+            'name' => 'Unique Asset BULK333',
+            'asset_tag' => 'BULKTAG-333',
+        ]);
+
+        $actor = User::factory()->viewUsers()->create();
+
+        $this->actingAs($actor)
+            ->post(route('users/bulkedit'), [
+                'ids' => [$subject->id],
+                'bulk_actions' => 'print',
+            ])
+            ->assertOk()
+            ->assertDontSee('BULKTAG-333');
+    }
+
+    public function test_bulk_print_with_assets_view_can_see_assigned_assets()
+    {
+        $subject = User::factory()->create();
+        Asset::factory()->assignedToUser($subject)->create([
+            'name' => 'Unique Asset BULK333',
+            'asset_tag' => 'BULKTAG-333',
+        ]);
+
+        $actor = User::factory()->viewUsers()->viewAssets()->create();
+
+        $this->actingAs($actor)
+            ->post(route('users/bulkedit'), [
+                'ids' => [$subject->id],
+                'bulk_actions' => 'print',
+            ])
+            ->assertOk()
+            ->assertSee('BULKTAG-333');
+    }
+
+    public function test_bulk_print_without_licenses_view_cannot_see_assigned_licenses()
+    {
+        $subject = User::factory()->create();
+        $license = License::factory()->create(['name' => 'Unique License BULK444']);
+        LicenseSeat::factory()->for($license)->assignedToUser($subject)->create();
+
+        $actor = User::factory()->viewUsers()->create();
+
+        $this->actingAs($actor)
+            ->post(route('users/bulkedit'), [
+                'ids' => [$subject->id],
+                'bulk_actions' => 'print',
+            ])
+            ->assertOk()
+            ->assertDontSee('Unique License BULK444');
+    }
 }