From 7865bc56e372447631b6c0d6eb6774faf896553a Mon Sep 17 00:00:00 2001 From: snipe Date: Sat, 25 Jul 2026 16:45:12 +0100 Subject: [PATCH] Print Views: Fixed FD-56677 - Tighter constraints on print views --- app/Http/Controllers/LocationsController.php | 66 ++++++--- .../Controllers/Users/BulkUsersController.php | 39 +++--- .../Controllers/Users/UsersController.php | 31 +++- app/Models/User.php | 72 ++++++---- resources/views/users/print.blade.php | 4 + .../Users/Ui/PrintUserInventoryTest.php | 132 ++++++++++++++++++ 6 files changed, 273 insertions(+), 71 deletions(-) diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index b03816fe03..bfe2a109b5 100755 --- 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. * diff --git a/app/Http/Controllers/Users/BulkUsersController.php b/app/Http/Controllers/Users/BulkUsersController.php index 0eed29b406..572a70af95 100644 --- 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)); diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index 6bcd0b47df..2deecf4ec8 100755 --- 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); diff --git a/app/Models/User.php b/app/Models/User.php index 1999d68718..815db37b9b 100644 --- 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) diff --git a/resources/views/users/print.blade.php b/resources/views/users/print.blade.php index 866b0319e9..df29f04fe6 100644 --- a/resources/views/users/print.blade.php +++ b/resources/views/users/print.blade.php @@ -105,6 +105,7 @@

{{ 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 @@ @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) @@ -446,6 +449,7 @@ $indirectAssignmentsCounter ++ @endphp @endforeach + @endcan @can('view', \App\Models\Accessory::class) @foreach ($asset->assignedAccessories as $indirectAccessory) @if($indirectAccessory) diff --git a/tests/Feature/Users/Ui/PrintUserInventoryTest.php b/tests/Feature/Users/Ui/PrintUserInventoryTest.php index 545adb841c..6511654ec8 100644 --- 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'); + } }