Merge remote-tracking branch 'origin/develop'

.env.docker

@@ -45,6 +45,7 @@ DB_SSL_KEY_PATH=null
 DB_SSL_CERT_PATH=null
 DB_SSL_CA_PATH=null
 DB_SSL_CIPHER=null
+DB_SSL_VERIFY_SERVER=null
 
 # --------------------------------------------
 # REQUIRED: OUTGOING MAIL SERVER SETTINGS

.env.dusk.example

@@ -36,6 +36,7 @@ DB_SSL_KEY_PATH=null
 DB_SSL_CERT_PATH=null
 DB_SSL_CA_PATH=null
 DB_SSL_CIPHER=null
+DB_SSL_VERIFY_SERVER=null
 
 # --------------------------------------------
 # REQUIRED: OUTGOING MAIL SERVER SETTINGS

.env.example

@@ -42,6 +42,7 @@ DB_SSL_KEY_PATH=null
 DB_SSL_CERT_PATH=null
 DB_SSL_CA_PATH=null
 DB_SSL_CIPHER=null
+DB_SSL_VERIFY_SERVER=null
 
 # --------------------------------------------
 # REQUIRED: OUTGOING MAIL SERVER SETTINGS

app/Http/Controllers/Api/UsersController.php

@@ -273,6 +273,7 @@ class UsersController extends Controller
             $users = $users->withTrashed();
         }
 
+        // Apply companyable scope
         $users = Company::scopeCompanyables($users);
 
 
@@ -403,7 +404,10 @@ class UsersController extends Controller
     public function show($id)
     {
         $this->authorize('view', User::class);
+
         $user = User::withCount('assets as assets_count', 'licenses as licenses_count', 'accessories as accessories_count', 'consumables as consumables_count')->findOrFail($id);
+        $user = Company::scopeCompanyables($user)->find($id);
+        $this->authorize('update', $user);
 
         return (new UsersTransformer)->transformUser($user);
     }
@@ -423,6 +427,8 @@ class UsersController extends Controller
         $this->authorize('update', User::class);
 
         $user = User::findOrFail($id);
+        $user = Company::scopeCompanyables($user)->find($id);
+        $this->authorize('update', $user);
 
         /**
          * This is a janky hack to prevent people from changing admin demo user data on the public demo.
@@ -459,6 +465,7 @@ class UsersController extends Controller
             if (! Auth::user()->isSuperUser()) {
                 unset($permissions_array['superuser']);
             }
+
             $user->permissions = $permissions_array;
         }
 
@@ -481,6 +488,7 @@ class UsersController extends Controller
 
             // Check if the request has groups passed and has a value
             if ($request->filled('groups')) {
+
                 $validator = Validator::make($request->all(), [
                     'groups.*' => 'integer|exists:permission_groups,id',
                 ]);
@@ -488,10 +496,19 @@ class UsersController extends Controller
                 if ($validator->fails()){
                     return response()->json(Helper::formatStandardApiResponse('error', null, $user->getErrors()));
                 }
+
+                // Only save groups if the user is a superuser
+                if (Auth::user()->isSuperUser()) {
                     $user->groups()->sync($request->input('groups'));
+                }
+
             // The groups field has been passed but it is null, so we should blank it out
             } elseif ($request->has('groups')) {
-                $user->groups()->sync([]);
+                
+                // Only save groups if the user is a superuser
+                if (Auth::user()->isSuperUser()) {
+                    $user->groups()->sync($request->input('groups'));
+                }
             }
 
 
@@ -512,7 +529,12 @@ class UsersController extends Controller
     public function destroy($id)
     {
         $this->authorize('delete', User::class);
-        $user = User::findOrFail($id);
+        $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
+        $user = Company::scopeCompanyables($user)->find($id);
+        $this->authorize('delete', $user);
+
+        if ($user) {
+
             $this->authorize('delete', $user);
             
             if (($user->assets) && ($user->assets->count() > 0)) {
@@ -520,23 +542,23 @@ class UsersController extends Controller
             }
 
             if (($user->licenses) && ($user->licenses->count() > 0)) {
-            return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has '.$user->licenses->count().' license(s) associated with them and cannot be deleted.'));
+                return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has ' . $user->licenses->count() . ' license(s) associated with them and cannot be deleted.'));
             }
 
             if (($user->accessories) && ($user->accessories->count() > 0)) {
-            return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has '.$user->accessories->count().' accessories associated with them.'));
+                return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has ' . $user->accessories->count() . ' accessories associated with them.'));
             }
 
             if (($user->managedLocations()) && ($user->managedLocations()->count() > 0)) {
-            return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has '.$user->managedLocations()->count().' locations that they manage.'));
+                return response()->json(Helper::formatStandardApiResponse('error', null, 'This user still has ' . $user->managedLocations()->count() . ' locations that they manage.'));
             }
 
             if ($user->delete()) {
 
                 // Remove the user's avatar if they have one
-            if (Storage::disk('public')->exists('avatars/'.$user->avatar)) {
+                if (Storage::disk('public')->exists('avatars/' . $user->avatar)) {
                     try {
-                    Storage::disk('public')->delete('avatars/'.$user->avatar);
+                        Storage::disk('public')->delete('avatars/' . $user->avatar);
                     } catch (\Exception $e) {
                         \Log::debug($e);
                     }
@@ -544,6 +566,7 @@ class UsersController extends Controller
 
                 return response()->json(Helper::formatStandardApiResponse('success', null, trans('admin/users/message.success.delete')));
             }
+        }
 
         return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.error.delete')));
     }
@@ -560,6 +583,11 @@ class UsersController extends Controller
     {
         $this->authorize('view', User::class);
         $this->authorize('view', Asset::class);
+
+        $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
+        $user = Company::scopeCompanyables($user)->find($id);
+        $this->authorize('view', $user);
+
         $assets = Asset::where('assigned_to', '=', $id)->where('assigned_type', '=', User::class)->with('model');
 
 
@@ -595,7 +623,10 @@ class UsersController extends Controller
      */
     public function emailAssetList(Request $request, $id)
     {
+        $this->authorize('update', User::class);
         $user = User::findOrFail($id);
+        $user = Company::scopeCompanyables($user)->find($id);
+        $this->authorize('update', $user);
 
         if (empty($user->email)) {
             return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/users/message.inventorynotification.error')));
@@ -619,6 +650,7 @@ class UsersController extends Controller
         $this->authorize('view', User::class);
         $this->authorize('view', Consumable::class);
         $user = User::findOrFail($id);
+        $this->authorize('update', $user);
         $consumables = $user->consumables;
         return (new ConsumablesTransformer)->transformConsumables($consumables, $consumables->count(), $request);
     }
@@ -635,6 +667,7 @@ class UsersController extends Controller
     {
         $this->authorize('view', User::class);
         $user = User::findOrFail($id);
+        $this->authorize('view', $user);
         $this->authorize('view', Accessory::class);
         $accessories = $user->accessories;
 
@@ -655,6 +688,7 @@ class UsersController extends Controller
         $this->authorize('view', License::class);
         
         if ($user = User::where('id', $id)->withTrashed()->first()) {
+            $this->authorize('update', $user);
             $licenses = $user->licenses()->get();
             return (new LicensesTransformer())->transformLicenses($licenses, $licenses->count());
         }
@@ -678,6 +712,7 @@ class UsersController extends Controller
         if ($request->filled('id')) {
             try {
                 $user = User::find($request->get('id'));
+                $this->authorize('update', $user);
                 $user->two_factor_secret = null;
                 $user->two_factor_enrolled = 0;
                 $user->saveQuietly();

app/Http/Controllers/Users/UsersController.php

@@ -182,8 +182,13 @@ class UsersController extends Controller
      */
     public function edit($id)
     {
-        if ($user = 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');
 
@@ -210,27 +215,30 @@ class UsersController extends Controller
      */
     public function update(SaveUserRequest $request, $id = null)
     {
-        // We need to reverse the UI specific logic for our
+        $this->authorize('update', User::class);
-        // permissions here before we update the user.
-        $permissions = $request->input('permissions', []);
-        app('request')->request->set('permissions', $permissions);
 
         // 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')));
-        }
 
+        // 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';
@@ -240,15 +248,13 @@ class UsersController extends Controller
                 }
             }
 
-        // Only save groups if the user is a super user
+            // Only save groups if the user is a superuser
             if (Auth::user()->isSuperUser()) {
                 $user->groups()->sync($request->input('groups'));
             }
 
-        // Update the user
+            // Update the user fields
-        if ($request->filled('username')) {
             $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');
@@ -300,9 +306,6 @@ class UsersController extends Controller
             // 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')
@@ -310,6 +313,11 @@ class UsersController extends Controller
             }
 
             return redirect()->back()->withInput()->withErrors($user->getErrors());
+
+
+        }
+
+        return redirect()->route('users.index')->with('error', trans('admin/users/message.user_not_found', compact('id')));
     }
 
     /**
@@ -323,12 +331,13 @@ class UsersController extends Controller
      */
     public function destroy($id = null)
     {
-        try {
-            // Get user information
-            $user = User::findOrFail($id);
-            // Authorize takes care of many of our logic checks now.
-            $this->authorize('delete', User::class);
 
+        $this->authorize('delete', User::class);
+        $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
+        $user = Company::scopeCompanyables($user)->find($id);
+
+
+        if ($user) {
             // Check if we are not trying to delete ourselves
             if ($user->id === Auth::id()) {
                 // Redirect to the user management page
@@ -362,16 +371,12 @@ class UsersController extends Controller
 
             // Delete the user
             $user->delete();
-
-            // Prepare the success message
-            // Redirect to the user management page
             return redirect()->route('users.index')->with('success', trans('admin/users/message.success.delete'));
-        } catch (ModelNotFoundException $e) {
+        }
-            // Prepare the error message
+
-            // Redirect to the user management page
         return redirect()->route('users.index')
             ->with('error', trans('admin/users/message.user_not_found', compact('id')));
-        }
+
     }
 
     /**
@@ -427,58 +432,24 @@ class UsersController extends Controller
      */
     public function show($userId = null)
     {
-        if (! $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed()->find($userId)) {
+        // Make sure the user can view users at all
-            // Redirect to the user management page
+        $this->authorize('view', User::class);
-            return redirect()->route('users.index')
-                ->with('error', trans('admin/users/message.user_not_found', ['id' => $userId]));
-        }
 
-        $userlog = $user->userlog->load('item');
+        $user = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
+        $user = Company::scopeCompanyables($user)->find($userId);
 
+        // Make sure they can view this particular user
         $this->authorize('view', $user);
 
-        return view('users/view', compact('user', 'userlog'))
+        if ($user) {
-            ->with('settings', Setting::getSettings());
+            $userlog = $user->userlog->load('item');
+            return view('users/view', compact('user', 'userlog'))->with('settings', Setting::getSettings());
         }
 
-    /**
+        return redirect()->route('users.index')->with('error', trans('admin/users/message.user_not_found', ['id' => $userId]));
-     * Unsuspend a user.
-     *
-     * @author [A. Gianotto] [<snipe@snipe.net>]
-     * @since [v1.0]
-     * @param  int $id
-     * @return Redirect
-     * @throws \Illuminate\Auth\Access\AuthorizationException
-     */
-    public function getUnsuspend($id = null)
-    {
-        try {
-            // Get user information
-            $user = User::findOrFail($id);
-            $this->authorize('update', $user);
 
-            // Check if we are not trying to unsuspend ourselves
-            if ($user->id === Auth::id()) {
-                // Prepare the error message
-                $error = trans('admin/users/message.error.unsuspend');
-                // Redirect to the user management page
-                return redirect()->route('users.index')->with('error', $error);
     }
 
-            // Do we have permission to unsuspend this user?
-            if ($user->isSuperUser() && ! Auth::user()->isSuperUser()) {
-                // Redirect to the user management page
-                return redirect()->route('users.index')->with('error', 'Insufficient permissions!');
-            }
-
-            // Redirect to the user management page
-            return redirect()->route('users.index')->with('success', trans('admin/users/message.success.unsuspend'));
-        } catch (ModelNotFoundException $e) {
-            // Redirect to the user management page
-            return redirect()->route('users.index')
-                ->with('error', trans('admin/users/message.user_not_found', compact('id')));
-        }
-    }
 
     /**
      * Return a view containing a pre-populated new user form,
@@ -493,22 +464,34 @@ class UsersController extends Controller
     public function getClone(Request $request, $id = null)
     {
         $this->authorize('create', User::class);
+
         // 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);
 
-        try {
+
-            // Get the user information
+        $user_to_clone = User::with('assets', 'assets.model', 'consumables', 'accessories', 'licenses', 'userloc')->withTrashed();
-            $user_to_clone = User::withTrashed()->find($id);
+        $user_to_clone = Company::scopeCompanyables($user_to_clone)->find($id);
+
+        // Make sure they can view this particular user
+        $this->authorize('view', $user_to_clone);
+
+
+        if ($user_to_clone) {
+
+
             $user = clone $user_to_clone;
+
+            // Blank out some fields
             $user->first_name = '';
             $user->last_name = '';
             $user->email = substr($user->email, ($pos = strpos($user->email, '@')) !== false ? $pos : 0);
             $user->id = null;
 
-            // Get this user groups
+            // Get this user's groups
             $userGroups = $user_to_clone->groups()->pluck('name', 'id');
+
             // Get all the available permissions
             $permissions = config('permissions');
             $clonedPermissions = $user_to_clone->decodePermissions();
@@ -521,12 +504,10 @@ class UsersController extends Controller
                 ->with('groups', Group::pluck('name', 'id'))
                 ->with('userGroups', $userGroups)
                 ->with('clone_user', $user_to_clone);
-        } catch (ModelNotFoundException $e) {
-            // Prepare the error message
-            // Redirect to the user management page
-            return redirect()->route('users.index')
-                ->with('error', trans('admin/users/message.user_not_found', compact('id')));
         }
+
+        return redirect()->route('users.index')->with('error', trans('admin/users/message.user_not_found', compact('id')));
+
     }
 
     /**
@@ -546,8 +527,20 @@ class UsersController extends Controller
             // Open output stream
             $handle = fopen('php://output', 'w');
 
-            User::with('assets', 'accessories', 'consumables', 'department', 'licenses', 'manager', 'groups', 'userloc', 'company')
+            $users = User::with(
-                ->orderBy('created_at', 'DESC')
+                '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
@@ -565,7 +558,7 @@ class UsersController extends Controller
                         trans('general.licenses'),
                         trans('general.accessories'),
                         trans('general.consumables'),
-                        trans('admin/users/table.groups'),
+                        trans('general.groups'),
                         trans('general.notes'),
                         trans('admin/users/table.activated'),
                         trans('general.created_at'),
@@ -626,7 +619,11 @@ class UsersController extends Controller
     public function printInventory($id)
     {
         $this->authorize('view', User::class);
-        $show_user = User::where('id', $id)->withTrashed()->first();
+        $show_user = Company::scopeCompanyables(User::where('id', $id)->withTrashed()->first());
+
+        // Make sure they can view this particular user
+        $this->authorize('view', $show_user);
+
         $assets = Asset::where('assigned_to', $id)->where('assigned_type', User::class)->with('model', 'model.category')->get();
         $accessories = $show_user->accessories()->get();
         $consumables = $show_user->consumables()->get();
@@ -651,10 +648,13 @@ class UsersController extends Controller
     {
         $this->authorize('view', User::class);
 
-        if (!$user = User::find($id)) {
+        $user = Company::scopeCompanyables(User::find($id));
-            return redirect()->back()
+
-                ->with('error', trans('admin/users/message.user_not_found', ['id' => $id]));
+        // Make sure they can view this particular user
-        }
+        $this->authorize('view', $user);
+
+        if ($user) {
+
             if (empty($user->email)) {
                 return redirect()->back()->with('error', trans('admin/users/message.user_has_no_email'));
             }
@@ -663,6 +663,10 @@ class UsersController extends Controller
             return redirect()->back()->with('success', trans('admin/users/general.user_notified'));
         }
 
+        return redirect()->back()->with('error', trans('admin/users/message.user_not_found', ['id' => $id]));
+
+    }
+
     /**
      * Send individual password reset email
      *
@@ -672,19 +676,19 @@ class UsersController extends Controller
      */
     public function sendPasswordReset($id)
     {
-        if (($user = User::find($id)) && ($user->activated == '1') && ($user->email != '') && ($user->ldap_import == '0')) {
+        if (($user = Company::scopeCompanyables(User::find($id))) && ($user->activated == '1') && ($user->email != '') && ($user->ldap_import == '0')) {
             $credentials = ['email' => trim($user->email)];
 
             try {
 
                 Password::sendResetLink($credentials);
-
                 return redirect()->back()->with('success', trans('admin/users/message.password_reset_sent', ['email' => $user->email]));
+
             } catch (\Exception $e) {
-                return redirect()->back()->with('error', ' Error sending email. :( ');
+                return redirect()->back()->with('error', trans('general.error_sending_email'));
             }
         }
 
-        return redirect()->back()->with('error', 'User is not activated, is LDAP synced, or does not have an email address ');
+        return redirect()->back()->with('error', trans('general.pwd_reset_not_sent'));
     }
 }

app/Models/Company.php

@@ -81,26 +81,6 @@ final class Company extends SnipeModel
         }
     }
 
-    /**
-     * Scoping table queries, determining if a logged in user is part of a company, and only allows
-     * that user to see items associated with that company
-     */
-    private static function scopeCompanyablesDirectly($query, $column = 'company_id', $table_name = null)
-    {
-        if (Auth::user()) {
-            $company_id = Auth::user()->company_id;
-        } else {
-            $company_id = null;
-        }
-
-        $table = ($table_name) ? $table_name."." : $query->getModel()->getTable().".";
-
-        if (\Schema::hasColumn($query->getModel()->getTable(), $column)) {
-             return $query->where($table.$column, '=', $company_id);
-        } else {
-            return $query->join('users as users_comp', 'users_comp.id', 'user_id')->where('users_comp.company_id', '=', $company_id);
-        }
-    }
 
     public static function getIdFromInput($unescaped_input)
     {
@@ -141,25 +121,49 @@ final class Company extends SnipeModel
         }
     }
 
+    /**
+     * Check to see if the current user should have access to the model.
+     * I hate this method and I think it should be refactored.
+     *
+     * @param $companyable
+     * @return bool|void
+     */
     public static function isCurrentUserHasAccess($companyable)
     {
+        // When would this even happen tho??
         if (is_null($companyable)) {
             return false;
-        } elseif (! static::isFullMultipleCompanySupportEnabled()) {
+        }
+
+        // If FMCS is not enabled, everyone has access, return true
+        if (! static::isFullMultipleCompanySupportEnabled()) {
             return true;
-        } elseif (!$companyable instanceof Company && !\Schema::hasColumn($companyable->getModel()->getTable(), 'company_id')) {
+        }
+
+        // Again, where would this happen? But check that $companyable is not a string
+        if (!is_string($companyable)) {
+            $company_table = $companyable->getModel()->getTable();
+            try {
                 // This is primary for the gate:allows-check in location->isDeletable()
                 // Locations don't have a company_id so without this it isn't possible to delete locations with FullMultipleCompanySupport enabled
                 // because this function is called by SnipePermissionsPolicy->before()
+                if (!$companyable instanceof Company && !\Schema::hasColumn($company_table, 'company_id')) {
                     return true;
-        } else {
+                }
+
+            } catch (\Exception $e) {
+                \Log::warning($e);
+            }
+        }
+
+
         if (Auth::user()) {
+            \Log::warning('Companyable is '.$companyable);
             $current_user_company_id = Auth::user()->company_id;
             $companyable_company_id = $companyable->company_id;
-
             return $current_user_company_id == null || $current_user_company_id == $companyable_company_id || Auth::user()->isSuperUser();
         }
-        }
+
     }
 
     public static function isCurrentUserAuthorized()
@@ -190,6 +194,10 @@ final class Company extends SnipeModel
                 && ($this->users()->count() === 0);
     }
 
+    /**
+     * @param $unescaped_input
+     * @return int|mixed|string|null
+     */
     public static function getIdForUser($unescaped_input)
     {
         if (! static::isFullMultipleCompanySupportEnabled() || Auth::user()->isSuperUser()) {
@@ -199,38 +207,6 @@ final class Company extends SnipeModel
         }
     }
 
-    public static function scopeCompanyables($query, $column = 'company_id', $table_name = null)
-    {
-        // If not logged in and hitting this, assume we are on the command line and don't scope?'
-        if (! static::isFullMultipleCompanySupportEnabled() || (Auth::check() && Auth::user()->isSuperUser()) || (! Auth::check())) {
-            return $query;
-        } else {
-            return static::scopeCompanyablesDirectly($query, $column, $table_name);
-        }
-    }
-
-    public static function scopeCompanyableChildren(array $companyable_names, $query)
-    {
-        if (count($companyable_names) == 0) {
-            throw new Exception('No Companyable Children to scope');
-        } elseif (! static::isFullMultipleCompanySupportEnabled() || (Auth::check() && Auth::user()->isSuperUser())) {
-            return $query;
-        } else {
-            $f = function ($q) {
-                static::scopeCompanyablesDirectly($q);
-            };
-
-            $q = $query->where(function ($q) use ($companyable_names, $f) {
-                $q2 = $q->whereHas($companyable_names[0], $f);
-
-                for ($i = 1; $i < count($companyable_names); $i++) {
-                    $q2 = $q2->orWhereHas($companyable_names[$i], $f);
-                }
-            });
-
-            return $q;
-        }
-    }
 
     public function users()
     {
@@ -261,4 +237,98 @@ final class Company extends SnipeModel
     {
         return $this->hasMany(Component::class, 'company_id');
     }
+
+    /**
+     * START COMPANY SCOPING FOR FMCS
+     */
+
+    /**
+     * Scoping table queries, determining if a logged in user is part of a company, and only allows the user to access items associated with that company if FMCS is enabled.
+     *
+     * This method is the one that the CompanyableTrait uses to contrain queries automatically, however that trait CANNOT be
+     * applied to the user's model, since it causes an infinite loop against the authenticated user.
+     *
+     * @todo - refactor that trait to handle the user's model as well.
+     *
+     * @author [A. Gianotto] <snipe@snipe.net>
+     * @param $query
+     * @param $column
+     * @param $table_name
+     * @return mixed
+     */
+    public static function scopeCompanyables($query, $column = 'company_id', $table_name = null)
+    {
+        // If not logged in and hitting this, assume we are on the command line and don't scope?'
+        if (! static::isFullMultipleCompanySupportEnabled() || (Auth::check() && Auth::user()->isSuperUser()) || (! Auth::check())) {
+            return $query;
+        } else {
+            \Log::debug('Fire scopeCompanyablesDirectly.');
+            return static::scopeCompanyablesDirectly($query, $column, $table_name);
+        }
+    }
+
+    /**
+     * Scoping table queries, determining if a logged in user is part of a company, and only allows
+     * that user to see items associated with that company
+     */
+    private static function scopeCompanyablesDirectly($query, $column = 'company_id', $table_name = null)
+    {
+        // Get the company ID of the logged in user, or set it to null if there is no company assicoated with the user
+        if (Auth::user()) {
+            \Log::debug('Admin company is: '.Auth::user()->company_id);
+            $company_id = Auth::user()->company_id;
+        } else {
+            $company_id = null;
+        }
+
+        // Dynamically get the table name if it's not passed in, based on the model we're querying against
+        $table = ($table_name) ? $table_name."." : $query->getModel()->getTable().".";
+         \Log::debug('Model is: '.$query->getModel());
+
+        \Log::debug('Table is: '.$table);
+
+        // If the column exists in the table, use it to scope the query
+        if (\Schema::hasColumn($query->getModel()->getTable(), $column)) {
+            return $query->where($table.$column, '=', $company_id);
+        } else {
+            return $query->join('users as users_comp', 'users_comp.id', 'user_id')->where('users_comp.company_id', '=', $company_id);
+        }
+    }
+
+    /**
+     * I legit do not know what this method does, but we can't remove it (yet).
+     *
+     * This gets invoked by CompanyableChildScope, but I'm not sure what it does.
+     *
+     * @author [A. Gianotto] <snipe@snipe.net>
+     * @param array $companyable_names
+     * @param $query
+     * @return mixed
+     */
+    public static function scopeCompanyableChildren(array $companyable_names, $query)
+    {
+        \Log::debug('Company Names in scopeCompanyableChildren: '.print_r($companyable_names, true));
+
+        if (count($companyable_names) == 0) {
+            throw new Exception('No Companyable Children to scope');
+        } elseif (! static::isFullMultipleCompanySupportEnabled() || (Auth::check() && Auth::user()->isSuperUser())) {
+            return $query;
+        } else {
+            $f = function ($q) {
+                \Log::debug('scopeCompanyablesDirectly firing ');
+                static::scopeCompanyablesDirectly($q);
+            };
+
+            $q = $query->where(function ($q) use ($companyable_names, $f) {
+                $q2 = $q->whereHas($companyable_names[0], $f);
+
+                for ($i = 1; $i < count($companyable_names); $i++) {
+                    $q2 = $q2->orWhereHas($companyable_names[$i], $f);
+                }
+            });
+
+            return $q;
+        }
+    }
+
 }

app/Models/CompanyableTrait.php

@@ -5,8 +5,13 @@ namespace App\Models;
 trait CompanyableTrait
 {
     /**
-     * Boot the companyable trait for a model.
+     * This trait is used to scope models to the current company. To use this scope on companyable models,
+     * we use the "use Companyable;" statement at the top of the mode.
      *
+     * We CANNOT USE THIS ON USERS, as it causes an infinite loop and prevents users from logging in, since this scope will be
+     * applied to the currently logged in (or logging in) user in addition to the user model for viewing lists of users.
+     *
+     * @see \App\Models\Company\Company::scopeCompanyables()
      * @return void
      */
     public static function bootCompanyableTrait()

app/Policies/SnipePermissionsPolicy.php

@@ -35,16 +35,50 @@ abstract class SnipePermissionsPolicy
 
     public function before(User $user, $ability, $item)
     {
-        // Lets move all company related checks here.
+        /**
-        if ($item instanceof \App\Models\SnipeModel && ! Company::isCurrentUserHasAccess($item)) {
+         * If an admin, they can do all item related tasks, but ARE constrained by FMCSA company access.
-            return false;
+         * That scoping happens on the model level (except for the Users model) via the Companyable trait.
-        }
+         *
-        // If an admin, they can do all asset related tasks.
+         * This does lead to some inconsistencies in the responses, since attempting to edit assets,
+         * accessories, etc (anything other than users) will result in a Forbidden error, whereas the users
+         * area will redirect with "That user doesn't exist" since the scoping is handled directly on those queries.
+         *
+         * The *superuser* global permission gets handled in the AuthServiceProvider before() method.
+         *
+         * @see https://snipe-it.readme.io/docs/permissions
+         */
+
         if ($user->hasAccess('admin')) {
             return true;
         }
+
+        /**
+         * If we got here by $this→authorize('something', $actualModel) then we can continue on Il but if we got here
+         * via $this→authorize('something', Model::class) then calling Company:: isCurrentUserHasAccess($item) gets weird.
+         * Bail out here by returning "nothing" and allow the relevant method lower in this class to be called and handle authorization.
+         */
+        if (!$item instanceof Model){
+            return;
         }
 
+
+        /**
+         * The Company::isCurrentUserHasAccess() method from the company model handles the check for FMCS already so we
+         * don't have to do that here.
+         */
+        if (!Company::isCurrentUserHasAccess($item)) {
+            return false;
+        }
+
+    }
+
+
+    /**
+     * These methods handle the generic view/create/edit/delete permissions for the model.
+     *
+     * @param User $user
+     * @return bool
+     */
     public function index(User $user)
     {
         return $user->hasAccess($this->columnName().'.view');

app/Providers/AuthServiceProvider.php

@@ -93,21 +93,28 @@ class AuthServiceProvider extends ServiceProvider
         Passport::personalAccessTokensExpireIn(Carbon::now()->addYears(config('passport.expiration_years')));
         Passport::withCookieSerialization();
 
-        // --------------------------------
+
-        // BEFORE ANYTHING ELSE
+        /**
-        // --------------------------------
+         * BEFORE ANYTHING ELSE
-        // If this condition is true, ANYTHING else below will be assumed
+         *
-        // to be true. This can cause weird blade behavior.
+         * If this condition is true, ANYTHING else below will be assumed to be true.
+         * This is where we set the superadmin permission to allow superadmins to be able to do everything within the system.
+         *
+         */
         Gate::before(function ($user) {
             if ($user->isSuperUser()) {
                 return true;
             }
         });
 
-        // --------------------------------
+
-        // GENERAL GATES
+        /**
-        // These control general sections of the admin
+         * GENERAL GATES
-        // --------------------------------
+         *
+         * These control general sections of the admin. These definitions are used in our blades via @can('blah) and also
+         * use in our controllers to determine if a user has access to a certain area.
+         */
+
         Gate::define('admin', function ($user) {
             if ($user->hasAccess('admin')) {
                 return true;

app/Providers/ValidationServiceProvider.php

@@ -279,7 +279,24 @@ class ValidationServiceProvider extends ServiceProvider
 
         Validator::extend('is_unique_department', function ($attribute, $value, $parameters, $validator) {
             $data = $validator->getData();
-            if ((array_key_exists('location_id', $data) && $data['location_id'] != null) && (array_key_exists('company_id', $data) && $data['company_id'] != null)) {
+
+            if (
+                array_key_exists('location_id', $data) && $data['location_id'] !== null &&
+                array_key_exists('company_id', $data) && $data['company_id'] !== null
+            ) {
+                //for updating existing departments
+                if(array_key_exists('id', $data) && $data['id'] !== null){
+                    $count = Department::where('name', $data['name'])
+                        ->where('location_id', $data['location_id'])
+                        ->where('company_id', $data['company_id'])
+                        ->whereNotNull('company_id')
+                        ->whereNotNull('location_id')
+                        ->where('id', '!=', $data['id'])
+                        ->count('name');
+
+                    return $count < 1;
+                }else // for entering in new departments
+                {
                 $count = Department::where('name', $data['name'])
                     ->where('location_id', $data['location_id'])
                     ->where('company_id', $data['company_id'])
@@ -289,6 +306,7 @@ class ValidationServiceProvider extends ServiceProvider
 
                 return $count < 1;
             }
+        }
             else {
                 return true;
         }

app/View/Label.php

@@ -142,7 +142,32 @@ class Label implements View
                         // Remove Duplicates
                         $toAdd = $field
                             ->filter(fn($o) => !$myFields->contains('dataSource', $o['dataSource']))
-                            ->first();
+                            // For fields that have multiple options, we need to combine them
+                            // into a single field so all values are displayed.
+                            ->reduce(function ($previous, $current) {
+                                // On the first iteration we simply return the item.
+                                // If there is only one item to be processed for the row
+                                // then this effectively skips everything below this if block.
+                                if (is_null($previous)) {
+                                    return $current;
+                                }
+
+                                // At this point we are dealing with a row with multiple items being displayed.
+                                // We need to combine the label and value of the current item with the previous item.
+
+                                // The end result of this will be in this format:
+                                // {labelOne} {valueOne} | {labelTwo} {valueTwo} | {labelThree} {valueThree}
+                                $previous['value'] = trim(implode(' | ', [
+                                    implode(' ', [$previous['label'], $previous['value']]),
+                                    implode(' ', [$current['label'], $current['value']]),
+                                ]));
+
+                                // We'll set the label to an empty string since we
+                                // injected the label into the value field above.
+                                $previous['label'] = '';
+
+                                return $previous;
+                            });
 
                         return $toAdd ? $myFields->push($toAdd) : $myFields;
                     }, new Collection());

config/database.php

@@ -96,6 +96,7 @@ return [
                 PDO::MYSQL_ATTR_SSL_CERT                => env('DB_SSL_CERT_PATH'), // /path/to/cert.pem
                 PDO::MYSQL_ATTR_SSL_CA                  => env('DB_SSL_CA_PATH'),   // /path/to/ca.pem
                 PDO::MYSQL_ATTR_SSL_CIPHER              => env('DB_SSL_CIPHER'),
+                PDO::MYSQL_ATTR_SSL_VERIFY_SERVER_CERT  => env('DB_SSL_VERIFY_SERVER'), //true/false
             ]) : [],
         ],
 

resources/lang/en-US/admin/licenses/message.php

@@ -3,7 +3,7 @@
 return array(
 
     'does_not_exist' => 'License does not exist or you do not have permission to view it.',
-    'user_does_not_exist' => 'User does not exist.',
+    'user_does_not_exist' => 'User does not exist or you do not have permission to view them.',
     'asset_does_not_exist' 	=> 'The asset you are trying to associate with this license does not exist.',
     'owner_doesnt_match_asset' => 'The asset you are trying to associate with this license is owned by somene other than the person selected in the assigned to dropdown.',
     'assoc_users'	 => 'This license is currently checked out to a user and cannot be deleted. Please check the license in first, and then try deleting again. ',

resources/lang/en-US/general.php

@@ -507,6 +507,9 @@ return [
     'or' => 'or',
     'url'                   => 'URL',
     'edit_fieldset' => 'Edit fieldset fields and options',
+    'permission_denied_superuser_demo' => 'Permission denied. You cannot update user information for superadmins on the demo.',
+    'pwd_reset_not_sent' => 'User is not activated, is LDAP synced, or does not have an email address',
+    'error_sending_email' => 'Error sending email',
     'bulk' => [
             'delete' =>
                 [

tests/Feature/Api/Users/UpdateUserApiTest.php

@@ -2,6 +2,8 @@
 
 namespace Tests\Feature\Api\Users;
 
+use App\Models\Company;
+use App\Models\Group;
 use App\Models\User;
 use Tests\TestCase;
 
@@ -58,4 +60,119 @@ class UpdateUserApiTest extends TestCase
 
         $this->assertEquals(0, $user->refresh()->activated);
     }
+
+    public function testUsersScopedToCompanyDuringUpdateWhenMultipleFullCompanySupportEnabled()
+    {
+        $this->settings->enableMultipleFullCompanySupport();
+
+        $companyA = Company::factory()->create(['name'=>'Company A']);
+        $companyB = Company::factory()->create(['name'=>'Company B']);
+
+        $adminA = User::factory(['company_id' => $companyA->id])->admin()->create();
+        $adminB = User::factory(['company_id' => $companyB->id])->admin()->create();
+        $adminNoCompany = User::factory(['company_id' => null])->admin()->create();
+
+        // Create users that belongs to company A and B and one that is unscoped
+        $scoped_user_in_companyA = User::factory()->create(['company_id' => $companyA->id]);
+        $scoped_user_in_companyB = User::factory()->create(['company_id' => $companyB->id]);
+        $scoped_user_in_no_company = User::factory()->create(['company_id' => null]);
+
+        // Admin for Company A should allow updating user from Company A
+        $this->actingAsForApi($adminA)
+            ->patchJson(route('api.users.update', $scoped_user_in_companyA))
+            ->assertStatus(200);
+
+        // Admin for Company A should get denied updating user from Company B
+        $this->actingAsForApi($adminA)
+            ->patchJson(route('api.users.update', $scoped_user_in_companyB))
+            ->assertStatus(403);
+
+        // Admin for Company A should get denied updating user without a company
+        $this->actingAsForApi($adminA)
+            ->patchJson(route('api.users.update', $scoped_user_in_no_company))
+            ->assertStatus(403);
+
+        // Admin for Company B should allow updating user from Company B
+        $this->actingAsForApi($adminB)
+            ->patchJson(route('api.users.update', $scoped_user_in_companyB))
+            ->assertStatus(200);
+
+        // Admin for Company B should get denied updating user from Company A
+        $this->actingAsForApi($adminB)
+            ->patchJson(route('api.users.update', $scoped_user_in_companyA))
+            ->assertStatus(403);
+
+        // Admin for Company B should get denied updating user without a company
+        $this->actingAsForApi($adminB)
+            ->patchJson(route('api.users.update', $scoped_user_in_no_company))
+            ->assertStatus(403);
+
+        // Admin without a company should allow updating user without a company
+        $this->actingAsForApi($adminNoCompany)
+            ->patchJson(route('api.users.update', $scoped_user_in_no_company))
+            ->assertStatus(200);
+
+        // Admin without a company should get denied updating user from Company A
+        $this->actingAsForApi($adminNoCompany)
+            ->patchJson(route('api.users.update', $scoped_user_in_companyA))
+            ->assertStatus(403);
+
+        // Admin without a company should get denied updating user from Company B
+        $this->actingAsForApi($adminNoCompany)
+            ->patchJson(route('api.users.update', $scoped_user_in_companyB))
+            ->assertStatus(403);
+    }
+
+    public function testUserGroupsAreOnlyUpdatedIfAuthenticatedUserIsSuperUser()
+    {
+        $groupToJoin = Group::factory()->create();
+
+        $normalUser = User::factory()->editUsers()->create();
+        $superUser = User::factory()->superuser()->create();
+
+        $oneUserToUpdate = User::factory()->create();
+        $anotherUserToUpdate = User::factory()->create();
+
+        $this->actingAsForApi($normalUser)
+            ->patchJson(route('api.users.update', $oneUserToUpdate), [
+                'groups' => [$groupToJoin->id],
+            ]);
+
+        $this->actingAsForApi($superUser)
+            ->patchJson(route('api.users.update', $anotherUserToUpdate), [
+                'groups' => [$groupToJoin->id],
+            ]);
+
+        $this->assertFalse(
+            $oneUserToUpdate->refresh()->groups->contains($groupToJoin),
+            'Non-super-user was able to modify user group'
+        );
+        $this->assertTrue($anotherUserToUpdate->refresh()->groups->contains($groupToJoin));
+    }
+
+    public function testUserGroupsCanBeClearedBySuperUser()
+    {
+        $normalUser = User::factory()->editUsers()->create();
+        $superUser = User::factory()->superuser()->create();
+
+        $oneUserToUpdate = User::factory()->create();
+        $anotherUserToUpdate = User::factory()->create();
+
+        $joinedGroup = Group::factory()->create();
+        $oneUserToUpdate->groups()->sync([$joinedGroup->id]);
+        $anotherUserToUpdate->groups()->sync([$joinedGroup->id]);
+
+        $this->actingAsForApi($normalUser)
+            ->patchJson(route('api.users.update', $oneUserToUpdate), [
+                'groups' => null,
+            ]);
+
+        $this->actingAsForApi($superUser)
+            ->patchJson(route('api.users.update', $anotherUserToUpdate), [
+                'groups' => null,
+            ]);
+
+        $this->assertTrue($oneUserToUpdate->refresh()->groups->contains($joinedGroup));
+        $this->assertFalse($anotherUserToUpdate->refresh()->groups->contains($joinedGroup));
+    }
 }

upgrade.php

@@ -19,6 +19,7 @@ $app_environment = 'develop';
 $skip_php_checks = false;
 $branch = 'master';
 $branch_override = false;
+$no_interactive = false;
 
 // Check for branch or other overrides
 if ($argc > 1){
@@ -32,6 +33,9 @@ if ($argc > 1){
                 $branch = $argv[$arg];
                 $branch_override = true;
                 break;
+            case '--no-interactive':
+                $no_interactive = true;
+                break;
             default: // for legacy support from before we started using --branch
                 $branch = $argv[$arg];
                 $branch_override = true;
@@ -81,7 +85,13 @@ if($upgrade_requirements){
     echo "Found PHP requirements, will check for PHP > $php_min_works and < $php_max_wontwork\n";
 }
 // done fetching requirements
-$yesno = readline("\nProceed with upgrade? [Y/n]: ");
+
+if (!$no_interactive) {
+    $yesno = readline("\nProceed with upgrade? [Y/n]: ");
+} else {
+    $yesno = "yes";
+}
+
 if ($yesno == "yes" || $yesno == "YES" ||$yesno == "y" ||$yesno == "Y"){
     # don't do anything
 } else {