From f54a94bd4cf097d92c1ac8eb16ef36f7b073f9c5 Mon Sep 17 00:00:00 2001 From: snipe Date: Thu, 11 Apr 2024 14:40:00 +0100 Subject: [PATCH] Refactorered methods Signed-off-by: snipe --- .../Controllers/Users/UsersController.php | 221 ++++++++++-------- 1 file changed, 124 insertions(+), 97 deletions(-) diff --git a/app/Http/Controllers/Users/UsersController.php b/app/Http/Controllers/Users/UsersController.php index 937cba551a..9239180ff3 100755 --- a/app/Http/Controllers/Users/UsersController.php +++ b/app/Http/Controllers/Users/UsersController.php @@ -183,8 +183,12 @@ class UsersController extends Controller public function edit($id) { - if ($user = Company::scopeCompanyables(User::find($id))) { - $this->authorize('update', $user); + $this->authorize('update', User::class); + $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed(); + $user = Company::scopeCompanyables($user)->find($id); + + if ($user) { + $permissions = config('permissions'); $groups = Group::pluck('name', 'id'); @@ -211,106 +215,109 @@ class UsersController extends Controller */ public function update(SaveUserRequest $request, $id = null) { - // 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); + $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'))) { - return redirect()->route('users.index')->with('error', 'Permission denied. You cannot update user information for superadmins on the demo.'); + return redirect()->route('users.index')->with('error', trans('general.permission_denied_superuser_demo')); } - try { - $user = User::findOrFail($id); - } catch (ModelNotFoundException $e) { - return redirect()->route('users.index') - ->with('error', trans('admin/users/message.user_not_found', compact('id'))); - } - $this->authorize('update', $user); - // Figure out of this user was an admin before this edit - $orig_permissions_array = $user->decodePermissions(); - $orig_superuser = '0'; - if (is_array($orig_permissions_array)) { - if (array_key_exists('superuser', $orig_permissions_array)) { - $orig_superuser = $orig_permissions_array['superuser']; + // 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(); + $user = Company::scopeCompanyables($user)->find($id); + + // User is valid - continue... + if ($user) { + $this->authorize('update', $user); + + // Figure out of this user was an admin before this edit + $orig_permissions_array = $user->decodePermissions(); + $orig_superuser = '0'; + if (is_array($orig_permissions_array)) { + if (array_key_exists('superuser', $orig_permissions_array)) { + $orig_superuser = $orig_permissions_array['superuser']; + } } - } - // Only save groups if the user is a super user - if (Auth::user()->isSuperUser()) { - $user->groups()->sync($request->input('groups')); - } + // Only save groups if the user is a superuser + if (Auth::user()->isSuperUser()) { + $user->groups()->sync($request->input('groups')); + } - // Update the user - if ($request->filled('username')) { + // Update the user fields $user->username = trim($request->input('username')); - } - $user->email = trim($request->input('email')); - $user->first_name = $request->input('first_name'); - $user->last_name = $request->input('last_name'); - $user->two_factor_optin = $request->input('two_factor_optin') ?: 0; - $user->locale = $request->input('locale'); - $user->employee_num = $request->input('employee_num'); - $user->activated = $request->input('activated', 0); - $user->jobtitle = $request->input('jobtitle', null); - $user->phone = $request->input('phone'); - $user->location_id = $request->input('location_id', null); - $user->company_id = Company::getIdForUser($request->input('company_id', null)); - $user->manager_id = $request->input('manager_id', null); - $user->notes = $request->input('notes'); - $user->department_id = $request->input('department_id', null); - $user->address = $request->input('address', null); - $user->city = $request->input('city', null); - $user->state = $request->input('state', null); - $user->country = $request->input('country', null); - // if a user is editing themselves we should always keep activated true - $user->activated = $request->input('activated', $request->user()->is($user) ? 1 : 0); - $user->zip = $request->input('zip', null); - $user->remote = $request->input('remote', 0); - $user->vip = $request->input('vip', 0); - $user->website = $request->input('website', null); - $user->start_date = $request->input('start_date', null); - $user->end_date = $request->input('end_date', null); - $user->autoassign_licenses = $request->input('autoassign_licenses', 0); + $user->email = trim($request->input('email')); + $user->first_name = $request->input('first_name'); + $user->last_name = $request->input('last_name'); + $user->two_factor_optin = $request->input('two_factor_optin') ?: 0; + $user->locale = $request->input('locale'); + $user->employee_num = $request->input('employee_num'); + $user->activated = $request->input('activated', 0); + $user->jobtitle = $request->input('jobtitle', null); + $user->phone = $request->input('phone'); + $user->location_id = $request->input('location_id', null); + $user->company_id = Company::getIdForUser($request->input('company_id', null)); + $user->manager_id = $request->input('manager_id', null); + $user->notes = $request->input('notes'); + $user->department_id = $request->input('department_id', null); + $user->address = $request->input('address', null); + $user->city = $request->input('city', null); + $user->state = $request->input('state', null); + $user->country = $request->input('country', null); + // if a user is editing themselves we should always keep activated true + $user->activated = $request->input('activated', $request->user()->is($user) ? 1 : 0); + $user->zip = $request->input('zip', null); + $user->remote = $request->input('remote', 0); + $user->vip = $request->input('vip', 0); + $user->website = $request->input('website', null); + $user->start_date = $request->input('start_date', null); + $user->end_date = $request->input('end_date', null); + $user->autoassign_licenses = $request->input('autoassign_licenses', 0); + + // 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)]); + + // Do we want to update the user password? + if ($request->filled('password')) { + $user->password = bcrypt($request->input('password')); + } + + $permissions_array = $request->input('permission'); + + // Strip out the superuser permission if the user isn't a superadmin + if (! Auth::user()->isSuperUser()) { + unset($permissions_array['superuser']); + $permissions_array['superuser'] = $orig_superuser; + } + + $user->permissions = json_encode($permissions_array); + + // Handle uploaded avatar + app(ImageUploadRequest::class)->handleImages($user, 600, 'avatar', 'avatars', 'avatar'); + + if ($user->save()) { + // Redirect to the user page + return redirect()->route('users.index') + ->with('success', trans('admin/users/message.success.update')); + } + + return redirect()->back()->withInput()->withErrors($user->getErrors()); - // 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)]); - // Do we want to update the user password? - if ($request->filled('password')) { - $user->password = bcrypt($request->input('password')); } - $permissions_array = $request->input('permission'); - - // Strip out the superuser permission if the user isn't a superadmin - if (! Auth::user()->isSuperUser()) { - unset($permissions_array['superuser']); - $permissions_array['superuser'] = $orig_superuser; - } - - $user->permissions = json_encode($permissions_array); - - // Handle uploaded avatar - app(ImageUploadRequest::class)->handleImages($user, 600, 'avatar', 'avatars', 'avatar'); - - //\Log::debug(print_r($user, true)); - - // Was the user updated? - if ($user->save()) { - // Redirect to the user page - return redirect()->route('users.index') - ->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'))); } /** @@ -326,7 +333,7 @@ class UsersController extends Controller { try { // Get user information - $user = User::findOrFail($id); + $user = Company::scopeCompanyables(User::findOrFail($id)); // Authorize takes care of many of our logic checks now. $this->authorize('delete', User::class); @@ -429,17 +436,25 @@ class UsersController extends Controller public function show($userId = null) { - if (! $user = Company::scopeCompanyables(User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($userId))) { - // Redirect to the user management page - return redirect()->route('users.index') - ->with('error', trans('admin/users/message.user_not_found', ['id' => $userId])); + // We can use the more generic auth check here since the company scoping is happening at the query level + $this->authorize('view', User::class); + + $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed(); + $user = Company::scopeCompanyables($user)->find($userId); + + //dd($user); + + if ($user) { + \Log::debug('User '.$user->username.' is found - checking permission'); + + $userlog = $user->userlog->load('item'); + + return view('users/view', compact('user', 'userlog'))->with('settings', Setting::getSettings()); } - $this->authorize('view', $user); - $userlog = $user->userlog->load('item'); + return redirect()->route('users.index') + ->with('error', trans('admin/users/message.user_not_found', ['id' => $userId])); - return view('users/view', compact('user', 'userlog')) - ->with('settings', Setting::getSettings()); } /** @@ -547,8 +562,20 @@ class UsersController extends Controller // Open output stream $handle = fopen('php://output', 'w'); - Company::scopeCompanyables(User::with('assets', 'accessories', 'consumables', 'department', 'licenses', 'manager', 'groups', 'userloc', 'company') - ->orderBy('created_at', 'DESC') + $users = User::with( + 'assets', + 'accessories', + 'consumables', + 'department', + 'licenses', + 'manager', + 'groups', + 'userloc', + 'company' + )->orderBy('created_at', 'DESC'); + + // FMCS scoping + Company::scopeCompanyables($users) ->chunk(500, function ($users) use ($handle) { $headers = [ // strtolower to prevent Excel from trying to open it as a SYLK file @@ -605,7 +632,7 @@ class UsersController extends Controller fputcsv($handle, $values); } - })); + }); // Close the output stream fclose($handle);