Merge remote-tracking branch 'origin/develop'

Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	public/js/build/app.js
#	public/js/build/vendor.js
#	public/js/dist/all.js
#	public/mix-manifest.json
This commit is contained in:
snipe 2024-08-29 15:04:32 +01:00
commit e5b9d9a28b
20 changed files with 286 additions and 123 deletions

View file

@ -427,13 +427,10 @@ class UsersController extends Controller
* @param \Illuminate\Http\Request $request
* @param int $id
*/
public function update(SaveUserRequest $request, $id) : JsonResponse
public function update(SaveUserRequest $request, User $user): JsonResponse
{
$this->authorize('update', User::class);
if ($user = User::find($id)) {
$this->authorize('update', $user);
/**
@ -443,12 +440,10 @@ class UsersController extends Controller
*
*/
if ((($id == 1) || ($id == 2)) && (config('app.lock_passwords'))) {
if ((($user->id == 1) || ($user->id == 2)) && (config('app.lock_passwords'))) {
return response()->json(Helper::formatStandardApiResponse('error', null, 'Permission denied. You cannot update user information via API on the demo.'));
}
$user->fill($request->all());
if ($user->id == $request->input('manager_id')) {
@ -473,16 +468,13 @@ class UsersController extends Controller
$user->permissions = $permissions_array;
}
// Update the location of any assets checked out to this user
Asset::where('assigned_type', User::class)
->where('assigned_to', $user->id)->update(['location_id' => $request->input('location_id', null)]);
app('App\Http\Requests\ImageUploadRequest')->handleImages($user, 600, 'image', 'avatars', 'avatar');
if ($user->save()) {
// Check if the request has groups passed and has a value, AND that the user us a superuser
if (($request->has('groups')) && (auth()->user()->isSuperUser())) {
@ -496,20 +488,12 @@ class UsersController extends Controller
// Sync the groups since the user is a superuser and the groups pass validation
$user->groups()->sync($request->input('groups'));
}
return response()->json(Helper::formatStandardApiResponse('success', (new UsersTransformer)->transformUser($user), trans('admin/users/message.success.update')));
}
return response()->json(Helper::formatStandardApiResponse('error', null, $user->getErrors()));
}
return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.user_not_found', compact('id'))));
}
/**
* Remove the specified resource from storage.
*

View file

@ -186,7 +186,7 @@ class UsersController extends Controller
{
$this->authorize('update', User::class);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id);
$user = User::with(['assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc'])->withTrashed()->find($id);
if ($user) {
@ -214,28 +214,24 @@ class UsersController extends Controller
* @return \Illuminate\Http\RedirectResponse
* @throws \Illuminate\Auth\Access\AuthorizationException
*/
public function update(SaveUserRequest $request, $id = null)
public function update(SaveUserRequest $request, User $user)
{
$this->authorize('update', User::class);
// This is a janky hack to prevent people from changing admin demo user data on the public demo.
// The $ids 1 and 2 are special since they are seeded as superadmins in the demo seeder.
// Thanks, jerks. You are why we can't have nice things. - snipe
if ((($id == 1) || ($id == 2)) && (config('app.lock_passwords'))) {
if ((($user->id == 1) || ($user->id == 2)) && (config('app.lock_passwords'))) {
return redirect()->route('users.index')->with('error', trans('general.permission_denied_superuser_demo'));
}
// We need to reverse the UI specific logic for our
// permissions here before we update the user.
$permissions = $request->input('permissions', []);
app('request')->request->set('permissions', $permissions);
$user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($id);
$user->load(['assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc'])->withTrashed();
// User is valid - continue...
if ($user) {
$this->authorize('update', $user);
// Figure out of this user was an admin before this edit
@ -318,13 +314,7 @@ class UsersController extends Controller
return redirect()->to(Helper::getRedirectOption($request, $user->id, 'Users'))
->with('success', trans('admin/users/message.success.update'));
}
return redirect()->back()->withInput()->withErrors($user->getErrors());
}
return redirect()->route('users.index')->with('error', trans('admin/users/message.user_not_found', compact('id')));
}
/**

View file

@ -34,6 +34,14 @@ class SaveUserRequest extends FormRequest
$rules = [
'department_id' => 'nullable|exists:departments,id',
'manager_id' => 'nullable|exists:users,id',
'company_id' => [
// determines if the user is being moved between companies and checks to see if they have any items assigned
function ($attribute, $value, $fail) {
if (($this->has('company_id')) && ($this->user?->allAssignedCount() > 0) && (Setting::getSettings()->full_multiple_companies_support)) {
$fail(trans('admin/users/message.error.multi_company_items_assigned'));
}
}
]
];
switch ($this->method()) {

View file

@ -4,6 +4,7 @@ namespace App\Http\Requests;
use App\Http\Requests\Traits\MayContainCustomFields;
use App\Models\Asset;
use App\Models\Setting;
use Illuminate\Support\Facades\Gate;
use Illuminate\Validation\Rule;
@ -41,6 +42,12 @@ class UpdateAssetRequest extends ImageUploadRequest
],
);
// if the purchase cost is passed in as a string **and** the digit_separator is ',' (as is common in the EU)
// then we tweak the purchase_cost rule to make it a string
if (Setting::getSettings()->digit_separator === '1.234,56' && is_string($this->input('purchase_cost'))) {
$rules['purchase_cost'] = ['nullable', 'string'];
}
return $rules;
}
}

View file

@ -21,6 +21,11 @@ class SnipeModel extends Model
*/
public function setPurchaseCostAttribute($value)
{
if (is_float($value)) {
//value is *already* a floating-point number. Just assign it directly
$this->attributes['purchase_cost'] = $value;
return;
}
$value = Helper::ParseCurrency($value);
if ($value == 0) {

45
package-lock.json generated
View file

@ -23,7 +23,7 @@
"ekko-lightbox": "^5.1.1",
"imagemin": "^8.0.1",
"jquery-slimscroll": "^1.3.8",
"jquery-ui": "^1.13.3",
"jquery-ui": "^1.14.0",
"jquery-validation": "^1.21.0",
"jquery.iframe-transport": "^1.0.0",
"jspdf-autotable": "^3.8.2",
@ -37,7 +37,7 @@
"signature_pad": "^4.2.0",
"tableexport.jquery.plugin": "1.30.0",
"tether": "^1.4.0",
"webpack": "^5.92.0"
"webpack": "^5.94.0"
},
"devDependencies": {
"all-contributors-cli": "^6.26.1",
@ -2105,25 +2105,10 @@
"@types/node": "*"
}
},
"node_modules/@types/eslint": {
"version": "8.56.10",
"license": "MIT",
"dependencies": {
"@types/estree": "*",
"@types/json-schema": "*"
}
},
"node_modules/@types/eslint-scope": {
"version": "3.7.7",
"license": "MIT",
"dependencies": {
"@types/eslint": "*",
"@types/estree": "*"
}
},
"node_modules/@types/estree": {
"version": "1.0.5",
"license": "MIT"
"resolved": "https://registry.npmjs.org/@types/estree/-/estree-1.0.5.tgz",
"integrity": "sha512-/kYRxGDLWzHOB7q+wtSUQlFrtcdUccpfy+X+9iMBpHK8QLLhx2wIPYuS5DYtR9Wa/YlZAbIovy7qVdB1Aq6Lyw=="
},
"node_modules/@types/express": {
"version": "4.17.21",
@ -5310,9 +5295,9 @@
}
},
"node_modules/enhanced-resolve": {
"version": "5.17.0",
"resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-5.17.0.tgz",
"integrity": "sha512-dwDPwZL0dmye8Txp2gzFmA6sxALaSvdRDjPH0viLcKrtlOL3tw62nWWweVD1SdILDTJrbrL6tdWVN58Wo6U3eA==",
"version": "5.17.1",
"resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-5.17.1.tgz",
"integrity": "sha512-LMHl3dXhTcfv8gM4kEzIUeTQ+7fpdA0l2tUf34BddXPkz2A5xJ5L/Pchd5BL6rdccM9QGvu0sWZzK1Z1t4wwyg==",
"dependencies": {
"graceful-fs": "^4.2.4",
"tapable": "^2.2.0"
@ -7067,10 +7052,11 @@
"license": "BSD-2-Clause"
},
"node_modules/jquery-ui": {
"version": "1.13.3",
"license": "MIT",
"version": "1.14.0",
"resolved": "https://registry.npmjs.org/jquery-ui/-/jquery-ui-1.14.0.tgz",
"integrity": "sha512-mPfYKBoRCf0MzaT2cyW5i3IuZ7PfTITaasO5OFLAQxrHuI+ZxruPa+4/K1OMNT8oElLWGtIxc9aRbyw20BKr8g==",
"dependencies": {
"jquery": ">=1.8.0 <4.0.0"
"jquery": ">=1.12.0 <5.0.0"
}
},
"node_modules/jquery-validation": {
@ -10879,11 +10865,10 @@
"license": "BSD-2-Clause"
},
"node_modules/webpack": {
"version": "5.92.1",
"resolved": "https://registry.npmjs.org/webpack/-/webpack-5.92.1.tgz",
"integrity": "sha512-JECQ7IwJb+7fgUFBlrJzbyu3GEuNBcdqr1LD7IbSzwkSmIevTm8PF+wej3Oxuz/JFBUZ6O1o43zsPkwm1C4TmA==",
"version": "5.94.0",
"resolved": "https://registry.npmjs.org/webpack/-/webpack-5.94.0.tgz",
"integrity": "sha512-KcsGn50VT+06JH/iunZJedYGUJS5FGjow8wb9c0v5n1Om8O1g4L6LjtfxwlXIATopoQu+vOXXa7gYisWxCoPyg==",
"dependencies": {
"@types/eslint-scope": "^3.7.3",
"@types/estree": "^1.0.5",
"@webassemblyjs/ast": "^1.12.1",
"@webassemblyjs/wasm-edit": "^1.12.1",
@ -10892,7 +10877,7 @@
"acorn-import-attributes": "^1.9.5",
"browserslist": "^4.21.10",
"chrome-trace-event": "^1.0.2",
"enhanced-resolve": "^5.17.0",
"enhanced-resolve": "^5.17.1",
"es-module-lexer": "^1.2.1",
"eslint-scope": "5.1.1",
"events": "^3.2.0",

View file

@ -43,7 +43,7 @@
"ekko-lightbox": "^5.1.1",
"imagemin": "^8.0.1",
"jquery-slimscroll": "^1.3.8",
"jquery-ui": "^1.13.3",
"jquery-ui": "^1.14.0",
"jquery-validation": "^1.21.0",
"jquery.iframe-transport": "^1.0.0",
"jspdf-autotable": "^3.8.2",
@ -57,6 +57,6 @@
"signature_pad": "^4.2.0",
"tableexport.jquery.plugin": "1.30.0",
"tether": "^1.4.0",
"webpack": "^5.92.0"
"webpack": "^5.94.0"
}
}

Binary file not shown.

Binary file not shown.

Binary file not shown.

BIN
public/js/dist/all.js vendored

Binary file not shown.

View file

@ -1,5 +1,5 @@
{
"/js/build/app.js": "/js/build/app.js?id=35438e10820b4461fafe3db72c901c69",
"/js/build/app.js": "/js/build/app.js?id=6d4d575774a1be993efe0598cc6b1c20",
"/css/dist/skins/skin-black-dark.css": "/css/dist/skins/skin-black-dark.css?id=5f3abb12a286d6cb8aa523322d7f053b",
"/css/dist/skins/_all-skins.css": "/css/dist/skins/_all-skins.css?id=89b7dcd91db033fb19ebbd7f7dcc0c35",
"/css/build/overrides.css": "/css/build/overrides.css?id=863d3083406a65c0fd94cf9ecda6a6ae",
@ -109,7 +109,7 @@
"/css/dist/skins/skin-yellow-dark.min.css": "/css/dist/skins/skin-yellow-dark.min.css?id=dd5eb6c76770bacaa2e960849d275516",
"/css/dist/skins/skin-yellow.min.css": "/css/dist/skins/skin-yellow.min.css?id=fc7adb943668ac69fe4b646625a7571f",
"/css/dist/bootstrap-table.css": "/css/dist/bootstrap-table.css?id=393d720a0f9aba560094fbc8d3b0c0f0",
"/js/build/vendor.js": "/js/build/vendor.js?id=c1c24b883f48dc3d16b817aed0b457cc",
"/js/build/vendor.js": "/js/build/vendor.js?id=5269eb5a6beb74f03387c78938cf17b2",
"/js/dist/bootstrap-table.js": "/js/dist/bootstrap-table.js?id=6660df122e24940d42d03c06775fec7b",
"/js/dist/all.js": "/js/dist/all.js?id=63efb984ecc1cf51bcf19086b3e09ffd"
"/js/dist/all.js": "/js/dist/all.js?id=262c933ac5d4c02c006d9bd531896c7b"
}

View file

@ -6,7 +6,7 @@ return array(
'declined' => 'You have successfully declined this asset.',
'bulk_manager_warn' => 'Your users have been successfully updated, however your manager entry was not saved because the manager you selected was also in the user list to be edited, and users may not be their own manager. Please select your users again, excluding the manager.',
'user_exists' => 'User already exists!',
'user_not_found' => 'User does not exist.',
'user_not_found' => 'User does not exist or you do not have permission view them.',
'user_login_required' => 'The login field is required',
'user_has_no_assets_assigned' => 'No assets currently assigned to user.',
'user_password_required' => 'The password is required.',
@ -53,6 +53,7 @@ return array(
'ldap_could_not_search' => 'Could not search the LDAP server. Please check your LDAP server configuration in the LDAP config file. <br>Error from LDAP Server:',
'ldap_could_not_get_entries' => 'Could not get entries from the LDAP server. Please check your LDAP server configuration in the LDAP config file. <br>Error from LDAP Server:',
'password_ldap' => 'The password for this account is managed by LDAP/Active Directory. Please contact your IT department to change your password. ',
'multi_company_items_assigned' => 'This user has items assigned, please check them in before moving companies.'
),
'deletefile' => array(

View file

@ -66,7 +66,9 @@
<div class="row">
<div class="col-md-8 col-md-offset-2">
<form class="form-horizontal" method="post" autocomplete="off" action="{{ (isset($user->id)) ? route('users.update', ['user' => $user->id]) : route('users.store') }}" enctype="multipart/form-data" id="userForm">
<form class="form-horizontal" method="post" autocomplete="off"
action="{{ (isset($user->id)) ? route('users.update', ['user' => $user->id]) : route('users.store') }}"
enctype="multipart/form-data" id="userForm">
{{csrf_field()}}
@if($user->id)

View file

@ -1099,16 +1099,18 @@ Route::group(['prefix' => 'v1', 'middleware' => ['api', 'throttle:api']], functi
});
Route::match(['put', 'patch'], '{user}/update', [Api\UsersController::class, 'update'])
->name('api.users.update');
Route::resource('users',
Api\UsersController::class,
['names' => [
'index' => 'api.users.index',
'show' => 'api.users.show',
'update' => 'api.users.update',
'store' => 'api.users.store',
'destroy' => 'api.users.destroy',
],
'except' => ['create', 'edit'],
'except' => ['create', 'edit', 'update'],
'parameters' => ['user' => 'user_id'],
]
); // end users API routes

View file

@ -145,10 +145,12 @@ Route::group(['prefix' => 'users', 'middleware' => ['auth']], function () {
]
)->name('users/bulkeditsave');
// pulling this out of the resource because I need route model binding in the request
Route::match(['put', 'patch'], '/{user}', [Users\UsersController::class, 'update'])->name('users.update');
});
Route::resource('users', Users\UsersController::class, [
'middleware' => ['auth'],
'parameters' => ['user' => 'user_id'],
'except' => ['update']
]);

View file

@ -103,6 +103,102 @@ class UpdateAssetTest extends TestCase
$this->assertEquals('2023-09-03 00:00:00', $updatedAsset->last_audit_date);
}
public function testUpdatesPeriodAsCommaSeparatorForPurchaseCost()
{
$this->settings->set([
'default_currency' => 'EUR',
'digit_separator' => '1.234,56',
]);
$original_asset = Asset::factory()->create();
$response = $this->actingAsForApi(User::factory()->superuser()->create())
->patchJson(route('api.assets.update', $original_asset->id), [
'asset_tag' => 'random-string',
'model_id' => AssetModel::factory()->create()->id,
'status_id' => Statuslabel::factory()->create()->id,
// API also accepts string for comma separated values
'purchase_cost' => '1.112,34',
])
->assertStatusMessageIs('success');
$asset = Asset::find($response['payload']['id']);
$this->assertEquals(1112.34, $asset->purchase_cost);
}
public function testUpdatesFloatForPurchaseCost()
{
$this->settings->set([
'default_currency' => 'EUR',
'digit_separator' => '1.234,56',
]);
$original_asset = Asset::factory()->create();
$response = $this->actingAsForApi(User::factory()->superuser()->create())
->patchJson(route('api.assets.update', $original_asset->id), [
'asset_tag' => 'random-string',
'model_id' => AssetModel::factory()->create()->id,
'status_id' => Statuslabel::factory()->create()->id,
// API also accepts string for comma separated values
'purchase_cost' => 12.34,
])
->assertStatusMessageIs('success');
$asset = Asset::find($response['payload']['id']);
$this->assertEquals(12.34, $asset->purchase_cost);
}
public function testUpdatesUSDecimalForPurchaseCost()
{
$this->settings->set([
'default_currency' => 'EUR',
'digit_separator' => '1,234.56',
]);
$original_asset = Asset::factory()->create();
$response = $this->actingAsForApi(User::factory()->superuser()->create())
->patchJson(route('api.assets.update', $original_asset->id), [
'asset_tag' => 'random-string',
'model_id' => AssetModel::factory()->create()->id,
'status_id' => Statuslabel::factory()->create()->id,
// API also accepts string for comma separated values
'purchase_cost' => '5412.34', //NOTE - you cannot use thousands-separator here!!!!
])
->assertStatusMessageIs('success');
$asset = Asset::find($response['payload']['id']);
$this->assertEquals(5412.34, $asset->purchase_cost);
}
public function testUpdatesFloatUSDecimalForPurchaseCost()
{
$this->settings->set([
'default_currency' => 'EUR',
'digit_separator' => '1,234.56',
]);
$original_asset = Asset::factory()->create();
$response = $this->actingAsForApi(User::factory()->superuser()->create())
->patchJson(route('api.assets.update', $original_asset->id), [
'asset_tag' => 'random-string',
'model_id' => AssetModel::factory()->create()->id,
'status_id' => Statuslabel::factory()->create()->id,
// API also accepts string for comma separated values
'purchase_cost' => 12.34,
])
->assertStatusMessageIs('success');
$asset = Asset::find($response['payload']['id']);
$this->assertEquals(12.34, $asset->purchase_cost);
}
public function testAssetEolDateIsCalculatedIfPurchaseDateUpdated()
{
$asset = Asset::factory()->laptopMbp()->noPurchaseOrEolDate()->create();

View file

@ -2,6 +2,7 @@
namespace Tests\Feature\Users\Api;
use App\Models\Asset;
use App\Models\Company;
use App\Models\Department;
use App\Models\Group;
@ -344,4 +345,47 @@ class UpdateUserTest extends TestCase
$this->assertTrue($user->refresh()->groups->contains($groupB));
}
public function testMultiCompanyUserCannotBeMovedIfHasAsset()
{
$this->settings->enableMultipleFullCompanySupport();
$companyA = Company::factory()->create();
$companyB = Company::factory()->create();
$user = User::factory()->create([
'company_id' => $companyA->id,
]);
$superUser = User::factory()->superuser()->create();
$asset = Asset::factory()->create();
// no assets assigned, therefore success
$this->actingAsForApi($superUser)->patchJson(route('api.users.update', $user), [
'username' => 'test',
'company_id' => $companyB->id,
])->assertStatusMessageIs('success');
// same test but PUT
$this->actingAsForApi($superUser)->putJson(route('api.users.update', $user), [
'username' => 'test',
'first_name' => 'Test',
'company_id' => $companyB->id,
])->assertStatusMessageIs('success');
$asset->checkOut($user, $superUser);
// asset assigned, therefore error
$this->actingAsForApi($superUser)->patchJson(route('api.users.update', $user), [
'username' => 'test',
'company_id' => $companyB->id,
])->assertStatusMessageIs('error');
// same test but PUT
$this->actingAsForApi($superUser)->putJson(route('api.users.update', $user), [
'username' => 'test',
'first_name' => 'Test',
'company_id' => $companyB->id,
])->assertStatusMessageIs('error');
}
}

View file

@ -2,6 +2,8 @@
namespace Tests\Feature\Users\Ui;
use App\Models\Asset;
use App\Models\Company;
use App\Models\User;
use Tests\TestCase;
@ -79,4 +81,39 @@ class UpdateUserTest extends TestCase
$this->assertEquals(1, $admin->refresh()->activated);
}
public function testMultiCompanyUserCannotBeMovedIfHasAsset()
{
$this->settings->enableMultipleFullCompanySupport();
$companyA = Company::factory()->create();
$companyB = Company::factory()->create();
$user = User::factory()->create([
'company_id' => $companyA->id,
]);
$superUser = User::factory()->superuser()->create();
$asset = Asset::factory()->create();
// no assets assigned, therefore success
$this->actingAs($superUser)->put(route('users.update', $user), [
'first_name' => 'test',
'username' => 'test',
'company_id' => $companyB->id,
'redirect_option' => 'index'
])->assertRedirect(route('users.index'));
$asset->checkOut($user, $superUser);
// asset assigned, therefore error
$response = $this->actingAs($superUser)->patchJson(route('users.update', $user), [
'first_name' => 'test',
'username' => 'test',
'company_id' => $companyB->id,
'redirect_option' => 'index'
]);
$this->followRedirects($response)->assertSee('error');
}
}

View file

@ -40,7 +40,7 @@ class ViewUserTest extends TestCase
$this->actingAs(User::factory()->viewUsers()->for($companyA)->create())
->get(route('users.print', ['userId' => $user->id]))
->assertStatus(403);
->assertStatus(302);
$this->actingAs(User::factory()->viewUsers()->for($companyB)->create())
->get(route('users.print', ['userId' => $user->id]))