mirror of
https://github.com/snipe/snipe-it.git
synced 2026-08-18 11:15:42 +00:00
Gate tweaked
This commit is contained in:
@ -13,13 +13,14 @@ class AssetModelPolicy extends SnipePermissionsPolicy
|
||||
|
||||
/**
|
||||
* READ ability for model files (index / show / download). Cascades from
|
||||
* assets.files because a model's file attachments (user manuals, spec
|
||||
* sheets, etc.) show up on the asset detail page and are legitimately
|
||||
* useful to anyone who can see the asset itself.
|
||||
* asset visibility: model file attachments (user manuals, spec sheets,
|
||||
* etc.) apply to every asset of a given model, so anyone who can view
|
||||
* assets can see them. Managing (upload/delete) still requires the
|
||||
* dedicated `models.files` grant via manageFiles() below.
|
||||
*/
|
||||
public function files(User $user, $item = null)
|
||||
{
|
||||
if ($user->hasAccess('assets.files')) {
|
||||
if ($user->hasAccess('assets.view')) {
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
@ -8,11 +8,16 @@ namespace App\Presenters;
|
||||
class UploadedFilesPresenter extends Presenter
|
||||
{
|
||||
/**
|
||||
* Json Column Layout for bootstrap table
|
||||
* Json Column Layout for bootstrap table.
|
||||
*
|
||||
* @param array $hide_fields Column field names to omit from the layout.
|
||||
* Callers pass `['available_actions']` for
|
||||
* read-only views (e.g. the model files tab
|
||||
* for users without models.files) so the
|
||||
* delete button doesn't render at all.
|
||||
* @return string
|
||||
*/
|
||||
public static function dataTableLayout()
|
||||
public static function dataTableLayout($hide_fields = [])
|
||||
{
|
||||
|
||||
$layout = [
|
||||
@ -94,7 +99,11 @@ class UploadedFilesPresenter extends Presenter
|
||||
'title' => trans('general.created_at'),
|
||||
'visible' => true,
|
||||
'formatter' => 'dateDisplayFormatter',
|
||||
], [
|
||||
],
|
||||
];
|
||||
|
||||
if (! in_array('available_actions', $hide_fields)) {
|
||||
$layout[] = [
|
||||
'field' => 'available_actions',
|
||||
'scope' => 'col',
|
||||
'searchable' => false,
|
||||
@ -105,8 +114,8 @@ class UploadedFilesPresenter extends Presenter
|
||||
'formatter' => 'deleteUploadFormatter',
|
||||
'printIgnore' => true,
|
||||
'class' => 'hidden-print',
|
||||
],
|
||||
];
|
||||
];
|
||||
}
|
||||
|
||||
return json_encode($layout);
|
||||
}
|
||||
|
||||
@ -12,7 +12,7 @@
|
||||
|
||||
@if(isset($object))
|
||||
<table
|
||||
data-columns="{{ \App\Presenters\UploadedFilesPresenter::dataTableLayout() }}"
|
||||
data-columns="{{ \App\Presenters\UploadedFilesPresenter::dataTableLayout(Gate::allows('manageFiles', $object) ? [] : ['available_actions']) }}"
|
||||
data-cookie-id-table="{{ $object_type }}-FileUploadsTable"
|
||||
data-id-table="{{ $object_type }}-FileUploadsTable"
|
||||
id="{{ $object_type }}-FileUploadsTable"
|
||||
|
||||
@ -1,13 +1,16 @@
|
||||
@props([
|
||||
'count' => null,
|
||||
'class' => false,
|
||||
'item' => null,
|
||||
])
|
||||
|
||||
<x-tabs.nav-item
|
||||
:$class
|
||||
name="model-files"
|
||||
icon_type="more-files"
|
||||
label="{{ trans('general.additional_files') }}"
|
||||
count="{{ $count }}"
|
||||
tooltip="{{ trans('general.additional_files') }}"
|
||||
/>
|
||||
@can('files', $item)
|
||||
<x-tabs.nav-item
|
||||
:$class
|
||||
name="model-files"
|
||||
icon_type="more-files"
|
||||
label="{{ trans('general.additional_files') }}"
|
||||
count="{{ $count }}"
|
||||
tooltip="{{ trans('general.additional_files') }}"
|
||||
/>
|
||||
@endcan
|
||||
@ -69,7 +69,7 @@
|
||||
/>
|
||||
<x-tabs.note-tab :item="$asset" count="{{ $asset->journal->count() }}"/>
|
||||
<x-tabs.files-tab :item="$asset" count="{{ $asset->uploads()->count() }}"/>
|
||||
<x-tabs.model-files-tab count="{{ $asset->model?->uploads()->count() }}"/>
|
||||
<x-tabs.model-files-tab :item="$asset->model" count="{{ $asset->model?->uploads()->count() }}"/>
|
||||
<x-tabs.history-tab count="{{ $asset->history()->count() }}" :model="$asset"/>
|
||||
<x-tabs.upload-tab :item="$asset"/>
|
||||
</x-slot:tabnav>
|
||||
|
||||
@ -22,16 +22,16 @@ use Tests\TestCase;
|
||||
*/
|
||||
class AssetModelFilesPolicyBypassTest extends TestCase
|
||||
{
|
||||
public function test_read_cascade_from_assets_files_is_preserved_for_index()
|
||||
public function test_asset_viewer_can_list_model_files()
|
||||
{
|
||||
// Seed one file on the model as a superuser so there is something to see.
|
||||
$model = AssetModel::factory()->create();
|
||||
$this->uploadFileAs(User::factory()->superuser()->create(), $model);
|
||||
|
||||
// A user with only assets.files (no models.files) can still see model files
|
||||
// because the model's files show up on the asset detail page and blocking
|
||||
// that read would be a UX regression.
|
||||
$reader = User::factory()->manageAssetFiles()->create();
|
||||
// Anyone who can view assets can see the model's file attachments
|
||||
// (user manuals, spec sheets) because they show up on the asset
|
||||
// detail page and apply to every asset of that model.
|
||||
$reader = User::factory()->viewAssets()->create();
|
||||
|
||||
$this->actingAsForApi($reader)
|
||||
->getJson(route('api.files.index', ['object_type' => 'models', 'id' => $model->id]))
|
||||
@ -39,21 +39,49 @@ class AssetModelFilesPolicyBypassTest extends TestCase
|
||||
->assertJsonPath('total', 1);
|
||||
}
|
||||
|
||||
public function test_read_cascade_from_assets_files_is_preserved_for_show()
|
||||
public function test_asset_viewer_can_download_model_file()
|
||||
{
|
||||
// Seed one file as superuser.
|
||||
$model = AssetModel::factory()->create();
|
||||
$fileId = $this->uploadFileAndReturnId(User::factory()->superuser()->create(), $model);
|
||||
|
||||
$reader = User::factory()->manageAssetFiles()->create();
|
||||
$reader = User::factory()->viewAssets()->create();
|
||||
|
||||
$this->actingAsForApi($reader)
|
||||
->get(route('api.files.show', ['object_type' => 'models', 'id' => $model->id, 'file_id' => $fileId]))
|
||||
->assertOk();
|
||||
}
|
||||
|
||||
public function test_asset_viewer_cannot_upload_to_model()
|
||||
{
|
||||
$model = AssetModel::factory()->create();
|
||||
$writer = User::factory()->viewAssets()->create();
|
||||
|
||||
$this->actingAsForApi($writer)
|
||||
->post(
|
||||
route('api.files.store', ['object_type' => 'models', 'id' => $model->id]),
|
||||
['file' => [UploadedFile::fake()->create('test.jpg', 100)]]
|
||||
)
|
||||
->assertForbidden();
|
||||
}
|
||||
|
||||
public function test_asset_viewer_cannot_delete_model_file()
|
||||
{
|
||||
// Seed a file as superuser so there is something for the low-priv user to try to delete.
|
||||
$model = AssetModel::factory()->create();
|
||||
$fileId = $this->uploadFileAndReturnId(User::factory()->superuser()->create(), $model);
|
||||
|
||||
$writer = User::factory()->viewAssets()->create();
|
||||
|
||||
$this->actingAsForApi($writer)
|
||||
->delete(route('api.files.destroy', ['object_type' => 'models', 'id' => $model->id, 'file_id' => $fileId]))
|
||||
->assertForbidden();
|
||||
}
|
||||
|
||||
public function test_user_with_only_assets_files_cannot_upload_to_model()
|
||||
{
|
||||
// The originally-reported bypass scenario: assets.files alone used to
|
||||
// grant model file upload/delete. It must not anymore.
|
||||
$model = AssetModel::factory()->create();
|
||||
$writer = User::factory()->manageAssetFiles()->create();
|
||||
|
||||
@ -67,7 +95,6 @@ class AssetModelFilesPolicyBypassTest extends TestCase
|
||||
|
||||
public function test_user_with_only_assets_files_cannot_delete_model_file()
|
||||
{
|
||||
// Seed a file as superuser so there is something for the low-priv user to try to delete.
|
||||
$model = AssetModel::factory()->create();
|
||||
$fileId = $this->uploadFileAndReturnId(User::factory()->superuser()->create(), $model);
|
||||
|
||||
|
||||
26
tests/Unit/Presenters/UploadedFilesPresenterTest.php
Normal file
26
tests/Unit/Presenters/UploadedFilesPresenterTest.php
Normal file
@ -0,0 +1,26 @@
|
||||
<?php
|
||||
|
||||
namespace Tests\Unit\Presenters;
|
||||
|
||||
use App\Presenters\UploadedFilesPresenter;
|
||||
use Tests\TestCase;
|
||||
|
||||
class UploadedFilesPresenterTest extends TestCase
|
||||
{
|
||||
public function test_layout_includes_actions_column_by_default()
|
||||
{
|
||||
$layout = json_decode(UploadedFilesPresenter::dataTableLayout(), true);
|
||||
|
||||
$this->assertContains('available_actions', array_column($layout, 'field'));
|
||||
}
|
||||
|
||||
public function test_layout_omits_actions_column_when_hidden()
|
||||
{
|
||||
$layout = json_decode(
|
||||
UploadedFilesPresenter::dataTableLayout(['available_actions']),
|
||||
true
|
||||
);
|
||||
|
||||
$this->assertNotContains('available_actions', array_column($layout, 'field'));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user