Furhter validation for scoped locations

There is a new validator introduced that checks on object update (assets, users, etc.) if the company matches the locations company.
In case of the creation of a new location it must be checked that the parent matches the own company.
On updating a location a check for every related object must be made to see if the company matches the location.

Signed-off-by: Tobias Regnery <tobias.regnery@gmail.com>

app/Console/Commands/TestLocationsFMCS.php

@@ -12,7 +12,7 @@ class TestLocationsFMCS extends Command
      *
      * @var string
      */
-    protected $signature = 'snipeit:test-locations-fmcs';
+    protected $signature = 'snipeit:test-locations-fmcs {--location_id=}';
 
     /**
      * The console command description.
@@ -28,7 +28,13 @@ class TestLocationsFMCS extends Command
     {
         $this->info('Test for inconsistencies if FullMultipleCompanySupport with scoped locations will be used');
         $this->info('Depending on the database size this will take a while, output will be displayed after the complete test is over');
-        $ret = Helper::test_locations_fmcs(true);
+
+        // if parameter location_id is set, only test this location
+        $location_id = null;
+        if ($this->option('location_id')) {
+            $location_id = $this->option('location_id');
+        }
+        $ret = Helper::test_locations_fmcs(true, $location_id);
 
         foreach($ret as $output) {
             $this->info($output);

app/Helpers/Helper.php

@@ -1538,32 +1538,52 @@ class Helper
      * @author T. Regnery <tobias.regnery@gmail.com>
      * @since 7.0
      *
-     * @param $artisan  when false, bail out on first inconsistent entry
+     * @param $artisan          when false, bail out on first inconsistent entry
+     * @param $location_id      when set, only test this specific location
+     * @param $new_company_id   in case of updating a location, this is the newly requested company_id
      * @return string []
      */
-    static public function test_locations_fmcs($artisan) {
+    static public function test_locations_fmcs($artisan, $location_id = null, $new_company_id = null) {
         $ret = [];
-        $locations = Location::all();
+
+        if ($location_id) {
+            $location = Location::find($location_id);
+            if ($location) {
+                $locations = collect([])->push(Location::find($location_id));
+            }
+        } else {
+            $locations = Location::all();
+        }
 
         foreach($locations as $location) {
-            $location_company = $location->company_id;
+            // in case of an update of a single location use the newly requested company_id
+            if ($new_company_id) {
+                $location_company = $new_company_id;
+            } else {
+                $location_company = $location->company_id;
+            }
 
             // depending on the relationship we must use different operations to retrieve the objects
             $keywords_relation = ['many' => ['users', 'assets', 'rtd_assets', 'consumables', 'components', 'accessories', 'assignedAssets', 'assignedAccessories'],
                                   'one'  => ['parent', 'manager']];
 
+            // In case of a single location the children must be checked either, becuase we don't walk every location
+            if ($location_id) {
+                $keywords_relation['many'][] = 'children';
+            }
+
             foreach ($keywords_relation as $relation => $keywords) {
                 foreach($keywords as $keyword) {
                     if ($relation == 'many') {
                         $items = $location->$keyword->all();
                     } else {
-                        $items = array($location->$keyword);
+                        $items = collect([])->push($location->$keyword);
                     }
-                
+
                     foreach ($items as $item) {
                         if ($item && $item->company_id != $location_company) {
                             $ret[] = 'type: ' . get_class($item) . ', id: ' . $item->id . ', company_id: ' . $item->company_id . ', location company_id: ' . $location_company;
-                            // when called from SettingsController we bail out on the first error
+                            // when not called from artisan command we bail out on the first error
                             if (!$artisan) {
                                 return $ret;
                             }

app/Http/Controllers/Api/LocationsController.php

@@ -177,6 +177,10 @@ class LocationsController extends Controller
         // Only scope location if the setting is enabled
         if (Setting::getSettings()->scope_locations_fmcs) {
             $location->company_id = Company::getIdForCurrentUser($request->get('company_id'));
+            // check if parent is set and has a different company
+            if ($location->parent_id && Location::find($location->parent_id)->company_id != $location->company_id) {
+                response()->json(Helper::formatStandardApiResponse('error', null, 'different company than parent'));
+            }    
         }
 
         if ($location->save()) {
@@ -243,6 +247,10 @@ class LocationsController extends Controller
             // Only scope location if the setting is enabled
             if (Setting::getSettings()->scope_locations_fmcs) {
                 $location->company_id = Company::getIdForCurrentUser($request->get('company_id'));
+                // check if there are related objects with different company
+                if (Helper::test_locations_fmcs(false, $id, $location->company_id)) {
+                    return response()->json(Helper::formatStandardApiResponse('error', null, 'error scoped locations'));
+                }                
             } else {
                 $location->company_id = $request->get('company_id');
             }

app/Http/Controllers/LocationsController.php

@@ -2,6 +2,7 @@
 
 namespace App\Http\Controllers;
 
+use App\Helpers\Helper;
 use App\Http\Requests\ImageUploadRequest;
 use App\Models\Actionlog;
 use App\Models\Asset;
@@ -85,6 +86,10 @@ class LocationsController extends Controller
         // Only scope the location if the setting is enabled
         if (Setting::getSettings()->scope_locations_fmcs) {
             $location->company_id = Company::getIdForCurrentUser($request->input('company_id'));
+            // check if parent is set and has a different company
+            if ($location->parent_id && Location::find($location->parent_id)->company_id != $location->company_id) {
+                return redirect()->back()->withInput()->withInput()->with('error', 'different company than parent');
+            }                
         } else {
             $location->company_id = $request->input('company_id');
         }
@@ -152,6 +157,10 @@ class LocationsController extends Controller
         // Only scope the location if the setting is enabled
         if (Setting::getSettings()->scope_locations_fmcs) {
             $location->company_id = Company::getIdForCurrentUser($request->input('company_id'));
+            // check if there are related objects with different company
+            if (Helper::test_locations_fmcs(false, $locationId, $location->company_id)) {
+                return redirect()->back()->withInput()->withInput()->with('error', 'error scoped locations');
+            }            
         } else {
             $location->company_id = $request->input('company_id');
         }

app/Models/Accessory.php

@@ -61,6 +61,7 @@ class Accessory extends SnipeModel
         'qty'               => 'required|integer|min:1',
         'category_id'       => 'required|integer|exists:categories,id',
         'company_id'        => 'integer|nullable',
+        'location_id'       => 'exists:locations,id|nullable|fmcs_location',
         'min_amt'           => 'integer|min:0|nullable',
         'purchase_cost'     => 'numeric|nullable|gte:0|max:9999999999999',
         'purchase_date'     => 'date_format:Y-m-d|nullable',

app/Models/Asset.php

@@ -108,8 +108,8 @@ class Asset extends Depreciable
         'expected_checkin'  => ['nullable', 'date'],
         'last_audit_date'   => ['nullable', 'date_format:Y-m-d H:i:s'],
         'next_audit_date'   => ['nullable', 'date'],
-        'location_id'       => ['nullable', 'exists:locations,id'],
+        'location_id'       => ['nullable', 'exists:locations,id', 'fmcs_location'],
-        'rtd_location_id'   => ['nullable', 'exists:locations,id'],
+        'rtd_location_id'   => ['nullable', 'exists:locations,id', 'fmcs_location'],
         'purchase_date'     => ['nullable', 'date', 'date_format:Y-m-d'],
         'serial'            => ['nullable', 'unique_undeleted:assets,serial'],
         'purchase_cost'     => ['nullable', 'numeric', 'gte:0', 'max:9999999999999'],
@@ -122,7 +122,7 @@ class Asset extends Depreciable
         'assigned_to'       => ['nullable', 'integer'],
         'requestable'       => ['nullable', 'boolean'],
         'assigned_user'     => ['nullable', 'exists:users,id,deleted_at,NULL'],
-        'assigned_location' => ['nullable', 'exists:locations,id,deleted_at,NULL'],
+        'assigned_location' => ['nullable', 'exists:locations,id,deleted_at,NULL', 'fmcs_location'],
         'assigned_asset'    => ['nullable', 'exists:assets,id,deleted_at,NULL']
     ];
 

app/Models/Component.php

@@ -35,6 +35,7 @@ class Component extends SnipeModel
         'category_id'    => 'required|integer|exists:categories,id',
         'supplier_id'    => 'nullable|integer|exists:suppliers,id',
         'company_id'     => 'integer|nullable|exists:companies,id',
+        'location_id'    => 'exists:locations,id|nullable|fmcs_location',
         'min_amt'        => 'integer|min:0|nullable',
         'purchase_date'   => 'date_format:Y-m-d|nullable',
         'purchase_cost'  => 'numeric|nullable|gte:0|max:9999999999999',

app/Models/Consumable.php

@@ -49,6 +49,7 @@ class Consumable extends SnipeModel
         'qty'         => 'required|integer|min:0|max:99999',
         'category_id' => 'required|integer',
         'company_id'  => 'integer|nullable',
+        'location_id' => 'exists:locations,id|nullable|fmcs_location',
         'min_amt'     => 'integer|min:0|max:99999|nullable',
         'purchase_cost'   => 'numeric|nullable|gte:0|max:9999999999999',
         'purchase_date'   => 'date_format:Y-m-d|nullable',

app/Models/User.php

@@ -94,7 +94,7 @@ class User extends SnipeModel implements AuthenticatableContract, AuthorizableCo
         'locale'                  => 'max:10|nullable',
         'website'                 => 'url|nullable|max:191',
         'manager_id'              => 'nullable|exists:users,id|cant_manage_self',
-        'location_id'             => 'exists:locations,id|nullable',
+        'location_id'             => 'exists:locations,id|nullable|fmcs_location',
         'start_date'              => 'nullable|date_format:Y-m-d',
         'end_date'                => 'nullable|date_format:Y-m-d|after_or_equal:start_date',
         'autoassign_licenses'     => 'boolean',

app/Providers/ValidationServiceProvider.php

@@ -4,6 +4,7 @@ namespace App\Providers;
 
 use App\Models\CustomField;
 use App\Models\Department;
+use App\Models\Location;
 use App\Models\Setting;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\ServiceProvider;
@@ -353,6 +354,20 @@ class ValidationServiceProvider extends ServiceProvider
 
             return in_array($value, $options);
         });
+
+        // Validates that the company of the validated object matches the company of the location in case of scoped locations
+        Validator::extend('fmcs_location', function ($attribute, $value, $parameters, $validator){
+            $settings = Setting::getSettings();
+            if ($settings->full_multiple_companies_support == '1' && $settings->scope_locations_fmcs == '1') {
+                $company_id = array_get($validator->getData(), 'company_id');
+                $location = Location::find($value);
+
+                if ($company_id != $location->company_id) {
+                    return false;
+                }
+            }
+            return true;
+        });
     }
 
     /**