ouyang/snipe-it
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #15907 from uberbrady/protect_assigned_to_assigned_type_rebased

Browse Source 
Rebased version of #15629 - prevent setting assigned_to without setting assigned_type
This commit is contained in:
snipe
2025-05-05 20:42:46 +01:00
committed by GitHub
parent 9ef20997a5 eccdcc373e
commit a67b320cae
8 changed files with 167 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
app/Http/Controllers/Api/AssetsController.php
View File

@@ -701,7 +701,9 @@ class AssetsController extends Controller
return response()->json(Helper::formatStandardApiResponse('success', $asset, trans('admin/hardware/message.create.success')));
return response()->json(Helper::formatStandardApiResponse('success', (new AssetsTransformer)->transformAsset($asset), trans('admin/hardware/message.create.success')));
// below is what we want the _eventual_ return to look like - in a more standardized format.
// return response()->json(Helper::formatStandardApiResponse('success', (new AssetsTransformer)->transformAsset($asset), trans('admin/hardware/message.create.success')));
}
return response()->json(Helper::formatStandardApiResponse('error', null, $asset->getErrors()), 200);

1
app/Http/Requests/StoreAssetRequest.php
View File

@@ -39,7 +39,6 @@ class StoreAssetRequest extends ImageUploadRequest
$this->merge([
'asset_tag' => $this->asset_tag ?? Asset::autoincrement_asset(),
'company_id' => $idForCurrentUser,
'assigned_to' => $assigned_to ?? null,
]);
}

3
app/Models/Asset.php
View File

@@ -119,7 +119,8 @@ class Asset extends Depreciable
'byod' => ['nullable', 'boolean'],
'order_number' => ['nullable', 'string', 'max:191'],
'notes' => ['nullable', 'string', 'max:65535'],
'assigned_to' => ['nullable', 'integer'],
'assigned_to' => ['nullable', 'integer', 'required_with:assigned_type'],
'assigned_type' => ['nullable', 'required_with:assigned_to', 'in:'.User::class.",".Location::class.",".Asset::class],
'requestable' => ['nullable', 'boolean'],
'assigned_user' => ['nullable', 'exists:users,id,deleted_at,NULL'],
'assigned_location' => ['nullable', 'exists:locations,id,deleted_at,NULL', 'fmcs_location'],

5
app/Observers/AssetObserver.php
View File

@@ -40,8 +40,9 @@ class AssetObserver
// If the asset isn't being checked out or audited, log the update.
// (Those other actions already create log entries.)
if (($attributes['assigned_to'] == $attributesOriginal['assigned_to'])
&& ($same_checkout_counter) && ($same_checkin_counter)
if (array_key_exists('assigned_to', $attributes) && array_key_exists('assigned_to', $attributesOriginal)
&& ($attributes['assigned_to'] == $attributesOriginal['assigned_to'])
&& ($same_checkout_counter) && ($same_checkin_counter)
&& ((isset( $attributes['next_audit_date']) ? $attributes['next_audit_date'] : null) == (isset($attributesOriginal['next_audit_date']) ? $attributesOriginal['next_audit_date']: null))
&& ($attributes['last_checkout'] == $attributesOriginal['last_checkout']) && (!$restoring_or_deleting))
{

8
tests/Feature/Assets/Api/AssetIndexTest.php
View File

@@ -83,7 +83,7 @@ class AssetIndexTest extends TestCase
public function testAssetApiIndexReturnsDueForExpectedCheckin()
{
Asset::factory()->count(3)->create(['assigned_to' => '1', 'expected_checkin' => Carbon::now()->format('Y-m-d')]);
Asset::factory()->count(3)->create(['assigned_to' => '1', 'assigned_type' => User::class, 'expected_checkin' => Carbon::now()->format('Y-m-d')]);
$this->actingAsForApi(User::factory()->superuser()->create())
->getJson(
@@ -99,7 +99,7 @@ class AssetIndexTest extends TestCase
public function testAssetApiIndexReturnsOverdueForExpectedCheckin()
{
Asset::factory()->count(3)->create(['assigned_to' => '1', 'expected_checkin' => Carbon::now()->subDays(1)->format('Y-m-d')]);
Asset::factory()->count(3)->create(['assigned_to' => '1', 'assigned_type' => User::class, 'expected_checkin' => Carbon::now()->subDays(1)->format('Y-m-d')]);
$this->actingAsForApi(User::factory()->superuser()->create())
->getJson(route('api.assets.list-upcoming', ['action' => 'checkins', 'upcoming_status' => 'overdue']))
@@ -113,8 +113,8 @@ class AssetIndexTest extends TestCase
public function testAssetApiIndexReturnsDueOrOverdueForExpectedCheckin()
{
Asset::factory()->count(3)->create(['assigned_to' => '1', 'expected_checkin' => Carbon::now()->subDays(1)->format('Y-m-d')]);
Asset::factory()->count(2)->create(['assigned_to' => '1', 'expected_checkin' => Carbon::now()->format('Y-m-d')]);
Asset::factory()->count(3)->create(['assigned_to' => '1', 'assigned_type' => User::class, 'expected_checkin' => Carbon::now()->subDays(1)->format('Y-m-d')]);
Asset::factory()->count(2)->create(['assigned_to' => '1', 'assigned_type' => User::class, 'expected_checkin' => Carbon::now()->format('Y-m-d')]);
$this->actingAsForApi(User::factory()->superuser()->create())
->getJson(route('api.assets.list-upcoming', ['action' => 'checkins', 'upcoming_status' => 'due-or-overdue']))

65
tests/Feature/Assets/Api/StoreAssetTest.php
View File

@@ -162,6 +162,71 @@ class StoreAssetTest extends TestCase
$this->assertNotNull($response->json('messages.status_id'));
}
public function testSaveWithAssignedToChecksOut()
{
$user = User::factory()->create();
$response = $this->actingAsForApi(User::factory()->superuser()->create())
->postJson(route('api.assets.store'), [
'asset_tag' => '1235',
'assigned_to' => $user->id,
'assigned_type' => User::class,
'model_id' => AssetModel::factory()->create()->id,
'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id,
])
->assertOk()
->assertStatusMessageIs('success');
$asset = Asset::find($response->json()['payload']['id']);
$this->assertEquals($user->id, $asset->assigned_to);
$this->assertEquals('Asset created successfully. :)', $response->json('messages'));
}
public function testSaveWithNoAssignedTypeReturnsValidationError()
{
$response = $this->actingAsForApi(User::factory()->superuser()->create())
->postJson(route('api.assets.store'), [
'asset_tag' => '1235',
'assigned_to' => '1',
// 'assigned_type' => User::class, //deliberately omit assigned_type
'model_id' => AssetModel::factory()->create()->id,
'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id,
])
->assertOk()
->assertStatusMessageIs('error');
$this->assertNotNull($response->json('messages.assigned_type'));
}
public function testSaveWithBadAssignedTypeReturnsValidationError()
{
$response = $this->actingAsForApi(User::factory()->superuser()->create())
->postJson(route('api.assets.store'), [
'asset_tag' => '1235',
'assigned_to' => '1',
'assigned_type' => 'nonsense_string', //deliberately bad assigned_type
'model_id' => AssetModel::factory()->create()->id,
'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id,
])
->assertOk()
->assertStatusMessageIs('error');
$this->assertNotNull($response->json('messages.assigned_type'));
}
public function testSaveWithAssignedTypeAndNoAssignedToReturnsValidationError()
{
$response = $this->actingAsForApi(User::factory()->superuser()->create())
->postJson(route('api.assets.store'), [
'asset_tag' => '1235',
//'assigned_to' => '1', //deliberately omit assigned_to
'assigned_type' => User::class,
'model_id' => AssetModel::factory()->create()->id,
'status_id' => Statuslabel::factory()->readyToDeploy()->create()->id,
])
->assertOk()
->assertStatusMessageIs('error');
$this->assertNotNull($response->json('messages.assigned_to'));
}
public function testSaveWithPendingStatusWithoutUserIsSuccessful()
{
$response = $this->actingAsForApi(User::factory()->superuser()->create())

80
tests/Feature/Assets/Api/UpdateAssetTest.php
View File

@@ -423,6 +423,86 @@ class UpdateAssetTest extends TestCase
$this->assertEquals($asset->assigned_type, 'App\Models\User');
}
public function testCheckoutToUserWithAssignedToAndAssignedType()
{
$asset = Asset::factory()->create();
$user = User::factory()->editAssets()->create();
$assigned_user = User::factory()->create();
$response = $this->actingAsForApi($user)
->patchJson(route('api.assets.update', $asset->id), [
'assigned_to' => $assigned_user->id,
'assigned_type' => User::class
])
->assertOk()
->assertStatusMessageIs('success')
->json();
$asset->refresh();
$this->assertEquals($assigned_user->id, $asset->assigned_to);
$this->assertEquals($asset->assigned_type, 'App\Models\User');
}
public function testCheckoutToUserWithAssignedToWithoutAssignedType()
{
$asset = Asset::factory()->create();
$user = User::factory()->editAssets()->create();
$assigned_user = User::factory()->create();
$response = $this->actingAsForApi($user)
->patchJson(route('api.assets.update', $asset->id), [
'assigned_to' => $assigned_user->id,
// 'assigned_type' => User::class //deliberately omit assigned_type
])
->assertOk()
->assertStatusMessageIs('error');
$asset->refresh();
$this->assertNotEquals($assigned_user->id, $asset->assigned_to);
$this->assertNotEquals($asset->assigned_type, 'App\Models\User');
$this->assertNotNull($response->json('messages.assigned_type'));
}
public function testCheckoutToUserWithAssignedToWithBadAssignedType()
{
$asset = Asset::factory()->create();
$user = User::factory()->editAssets()->create();
$assigned_user = User::factory()->create();
$response = $this->actingAsForApi($user)
->patchJson(route('api.assets.update', $asset->id), [
'assigned_to' => $assigned_user->id,
'assigned_type' => 'more_deliberate_nonsense' //deliberately bad assigned_type
])
->assertOk()
->assertStatusMessageIs('error');
$asset->refresh();
$this->assertNotEquals($assigned_user->id, $asset->assigned_to);
$this->assertNotEquals($asset->assigned_type, 'App\Models\User');
$this->assertNotNull($response->json('messages.assigned_type'));
}
public function testCheckoutToUserWithoutAssignedToWithAssignedType()
{
$asset = Asset::factory()->create();
$user = User::factory()->editAssets()->create();
$assigned_user = User::factory()->create();
$response = $this->actingAsForApi($user)
->patchJson(route('api.assets.update', $asset->id), [
//'assigned_to' => $assigned_user->id, // deliberately omit assigned_to
'assigned_type' => User::class
])
->assertOk()
->assertStatusMessageIs('error');
$asset->refresh();
$this->assertNotEquals($assigned_user->id, $asset->assigned_to);
$this->assertNotEquals($asset->assigned_type, 'App\Models\User');
$this->assertNotNull($response->json('messages.assigned_to'));
}
public function testCheckoutToDeletedUserFailsOnAssetUpdate()
{
$asset = Asset::factory()->create();

10
tests/Unit/AssetTest.php
View File

@@ -4,6 +4,7 @@ namespace Tests\Unit;
use App\Models\Asset;
use App\Models\AssetModel;
use App\Models\Category;
use App\Models\User;
use Carbon\Carbon;
use Tests\TestCase;
use App\Models\Setting;
@@ -189,4 +190,13 @@ class AssetTest extends TestCase
$this->assertEquals(Carbon::createFromDate(2019, 1, 1)->format('Y-m-d'), $asset->warranty_expires->format('Y-m-d'));
}
public function testAssignedTypeWithoutAssignTo()
{
$user = User::factory()->create();
$asset = Asset::factory()->create([
'assigned_to' => $user->id
]);
$this->assertModelMissing($asset);
}
}