From 1ccbf8942c87d528a6ea47f41f7cf0bad6eab024 Mon Sep 17 00:00:00 2001 From: Tobias Regnery Date: Thu, 10 Feb 2022 11:36:36 +0100 Subject: [PATCH 1/5] Add ability to tie locations to companies Locations are the last big part of the application that can't be tied to companies. This can be a problem with FullMultipleCompanySupport, because you can't restrict the visibility of locations to the company of the users. In order to change this, add a company_id to the locations table and wire everything up in the views and controllers. Aditionally add a new formatter to filter the locations to a specific company, like it is done for assets. Locations are properly scoped to the users company if FullMultipleCompanySupport is enabled. If a parent location of a location has a different company than the user, the location does not show up. --- .../Controllers/Api/LocationsController.php | 21 ++++++++++- app/Http/Controllers/LocationsController.php | 27 +++++++++----- .../Transformers/LocationsTransformer.php | 4 +++ app/Models/Location.php | 31 +++++++++++++++- app/Presenters/LocationPresenter.php | 9 +++++ ..._10_110210_add_company_id_to_locations.php | 35 +++++++++++++++++++ resources/views/locations/edit.blade.php | 3 ++ resources/views/locations/index.blade.php | 2 +- resources/views/locations/print.blade.php | 6 +++- resources/views/locations/view.blade.php | 3 ++ resources/views/modals/location.blade.php | 10 ++++++ .../views/partials/bootstrap-table.blade.php | 8 +++++ 12 files changed, 147 insertions(+), 12 deletions(-) create mode 100644 database/migrations/2022_02_10_110210_add_company_id_to_locations.php diff --git a/app/Http/Controllers/Api/LocationsController.php b/app/Http/Controllers/Api/LocationsController.php index 10744bee2764..93ba287d4d5d 100644 --- a/app/Http/Controllers/Api/LocationsController.php +++ b/app/Http/Controllers/Api/LocationsController.php @@ -9,6 +9,7 @@ use App\Http\Transformers\LocationsTransformer; use App\Http\Transformers\SelectlistTransformer; use App\Models\Asset; +use App\Models\Company; use App\Models\Location; use Illuminate\Http\Request; use Illuminate\Pagination\LengthAwarePaginator; @@ -48,6 +49,7 @@ public function index(Request $request) : JsonResponse | array 'rtd_assets_count', 'currency', 'ldap_ou', + 'company_id', ]; $locations = Location::with('parent', 'manager', 'children')->select([ @@ -68,12 +70,15 @@ public function index(Request $request) : JsonResponse | array 'locations.image', 'locations.ldap_ou', 'locations.currency', + 'locations.company_id', ])->withCount('assignedAssets as assigned_assets_count') ->withCount('assets as assets_count') ->withCount('rtd_assets as rtd_assets_count') ->withCount('children as children_count') ->withCount('users as users_count'); + $locations = Company::scopeCompanyables($locations); + if ($request->filled('search')) { $locations = $locations->TextSearch($request->input('search')); } @@ -106,6 +111,10 @@ public function index(Request $request) : JsonResponse | array $locations->where('locations.manager_id', '=', $request->input('manager_id')); } + if ($request->filled('company_id')) { + $locations->where('locations.company_id', '=', $request->input('company_id')); + } + // Make sure the offset and limit are actually integers and do not exceed system limits $offset = ($request->input('offset') > $locations->count()) ? $locations->count() : app('api_offset_value'); $limit = app('api_limit_value'); @@ -122,6 +131,9 @@ public function index(Request $request) : JsonResponse | array case 'manager': $locations->OrderManager($order); break; + case 'company': + $locations->OrderCompany($order); + break; default: $locations->orderBy($sort, $order); break; @@ -147,6 +159,7 @@ public function store(ImageUploadRequest $request) : JsonResponse $this->authorize('create', Location::class); $location = new Location; $location->fill($request->all()); + $location->company_id = Company::getIdForCurrentUser($request->get('company_id')); $location = $request->handleImages($location); if ($location->save()) { @@ -166,7 +179,7 @@ public function store(ImageUploadRequest $request) : JsonResponse public function show($id) : JsonResponse | array { $this->authorize('view', Location::class); - $location = Location::with('parent', 'manager', 'children') + $location = Location::with('parent', 'manager', 'children', 'company') ->select([ 'locations.id', 'locations.name', @@ -209,6 +222,10 @@ public function update(ImageUploadRequest $request, $id) : JsonResponse $location->fill($request->all()); $location = $request->handleImages($location); + if ($request->filled('company_id')) { + $location->company_id = Company::getIdForCurrentUser($request->get('company_id')); + } + if ($location->isValid()) { $location->save(); @@ -305,6 +322,8 @@ public function selectlist(Request $request) : array 'locations.image', ]); + $locations = Company::scopeCompanyables($locations); + $page = 1; if ($request->filled('page')) { $page = $request->input('page'); diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index 75abce97ed5b..7784ad1825c3 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -5,6 +5,7 @@ use App\Http\Requests\ImageUploadRequest; use App\Models\Actionlog; use App\Models\Asset; +use App\Models\Company; use App\Models\Location; use App\Models\User; use Illuminate\Support\Facades\Storage; @@ -78,6 +79,7 @@ public function store(ImageUploadRequest $request) : RedirectResponse $location->created_by = auth()->id(); $location->phone = request('phone'); $location->fax = request('fax'); + $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); $location = $request->handleImages($location); @@ -138,6 +140,7 @@ public function update(ImageUploadRequest $request, $locationId = null) : Redire $location->fax = request('fax'); $location->ldap_ou = $request->input('ldap_ou'); $location->manager_id = $request->input('manager_id'); + $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); $location = $request->handleImages($location); @@ -211,20 +214,22 @@ public function show($id = null) : View | RedirectResponse public function print_assigned($id) : View | RedirectResponse { - if ($location = Location::where('id', $id)->first()) { $parent = Location::where('id', $location->parent_id)->first(); $manager = User::where('id', $location->manager_id)->first(); + $company = Company::where('id', $location->company_id)->first(); $users = User::where('location_id', $id)->with('company', 'department', 'location')->get(); $assets = Asset::where('assigned_to', $id)->where('assigned_type', Location::class)->with('model', 'model.category')->get(); - return view('locations/print')->with('assets', $assets)->with('users', $users)->with('location', $location)->with('parent', $parent)->with('manager', $manager); - + return view('locations/print') + ->with('assets', $assets) + ->with('users',$users) + ->with('location', $location) + ->with('parent', $parent) + ->with('manager', $manager) + ->with('company', $company); } return redirect()->route('locations.index')->with('error', trans('admin/locations/message.does_not_exist')); - - - } @@ -296,10 +301,16 @@ public function print_all_assigned($id) : View | RedirectResponse if ($location = Location::where('id', $id)->first()) { $parent = Location::where('id', $location->parent_id)->first(); $manager = User::where('id', $location->manager_id)->first(); + $company = Company::where('id', $location->company_id)->first(); $users = User::where('location_id', $id)->with('company', 'department', 'location')->get(); $assets = Asset::where('location_id', $id)->with('model', 'model.category')->get(); - return view('locations/print')->with('assets', $assets)->with('users', $users)->with('location', $location)->with('parent', $parent)->with('manager', $manager); - + return view('locations/print') + ->with('assets', $assets) + ->with('users',$users) + ->with('location', $location) + ->with('parent', $parent) + ->with('manager', $manager) + ->with('company', $company); } return redirect()->route('locations.index')->with('error', trans('admin/locations/message.does_not_exist')); } diff --git a/app/Http/Transformers/LocationsTransformer.php b/app/Http/Transformers/LocationsTransformer.php index 513b967f421d..0f39eea18d39 100644 --- a/app/Http/Transformers/LocationsTransformer.php +++ b/app/Http/Transformers/LocationsTransformer.php @@ -58,6 +58,10 @@ public function transformLocation(Location $location = null) 'name'=> e($location->parent->name), ] : null, 'manager' => ($location->manager) ? (new UsersTransformer)->transformUser($location->manager) : null, + 'company' => ($location->company) ? [ + 'id' => (int) $location->company->id, + 'name'=> e($location->company->name) + ] : null, 'children' => $children_arr, ]; diff --git a/app/Models/Location.php b/app/Models/Location.php index 014db3053e21..7f99c785310b 100755 --- a/app/Models/Location.php +++ b/app/Models/Location.php @@ -22,6 +22,7 @@ class Location extends SnipeModel protected $presenter = \App\Presenters\LocationPresenter::class; use Presentable; use SoftDeletes; + use CompanyableTrait; protected $table = 'locations'; protected $rules = [ @@ -34,11 +35,13 @@ class Location extends SnipeModel 'zip' => 'max:10|nullable', 'manager_id' => 'exists:users,id|nullable', 'parent_id' => 'nullable|exists:locations,id|non_circular:locations,id', + 'company_id' => 'integer|nullable|exists:companies,id', ]; protected $casts = [ 'parent_id' => 'integer', 'manager_id' => 'integer', + 'company_id' => 'integer', ]; /** @@ -72,6 +75,7 @@ class Location extends SnipeModel 'currency', 'manager_id', 'image', + 'company_id', ]; protected $hidden = ['user_id']; @@ -90,7 +94,8 @@ class Location extends SnipeModel * @var array */ protected $searchableRelations = [ - 'parent' => ['name'], + 'parent' => ['name'], + 'company' => ['name'] ]; @@ -214,6 +219,17 @@ public function parent() ->with('parent'); } + /** + * Establishes the locations -> company relationship + * + * @author [T. Regnery] [] + * @since [v7.0] + * @return \Illuminate\Database\Eloquent\Relations\Relation + */ + public function company() + { + return $this->belongsTo(\App\Models\Company::class, 'company_id'); + } /** * Find the manager of a location @@ -313,4 +329,17 @@ public function scopeOrderManager($query, $order) { return $query->leftJoin('users as location_user', 'locations.manager_id', '=', 'location_user.id')->orderBy('location_user.first_name', $order)->orderBy('location_user.last_name', $order); } + + /** + * Query builder scope to order on company + * + * @param \Illuminate\Database\Query\Builder $query Query builder instance + * @param text $order Order + * + * @return \Illuminate\Database\Query\Builder Modified query builder + */ + public function scopeOrderCompany($query, $order) + { + return $query->leftJoin('companies as company_sort', 'locations.company_id', '=', 'company_sort.id')->orderBy('company_sort.name', $order); + } } diff --git a/app/Presenters/LocationPresenter.php b/app/Presenters/LocationPresenter.php index d6bbe0db11db..7185aa04fcab 100644 --- a/app/Presenters/LocationPresenter.php +++ b/app/Presenters/LocationPresenter.php @@ -27,6 +27,15 @@ public static function dataTableLayout() 'title' => trans('general.id'), 'visible' => false, ], + [ + 'field' => 'company', + 'searchable' => true, + 'sortable' => true, + 'switchable' => true, + 'title' => trans('general.company'), + 'visible' => false, + 'formatter' => 'locationCompanyObjFilterFormatter' + ], [ 'field' => 'name', 'searchable' => true, diff --git a/database/migrations/2022_02_10_110210_add_company_id_to_locations.php b/database/migrations/2022_02_10_110210_add_company_id_to_locations.php new file mode 100644 index 000000000000..8ef5822812fd --- /dev/null +++ b/database/migrations/2022_02_10_110210_add_company_id_to_locations.php @@ -0,0 +1,35 @@ +integer('company_id')->unsigned()->nullable(); + $table->index(['company_id']); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('locations', function (Blueprint $table) { + $table->dropIndex(['company_id']); + $table->dropColumn('company_id'); + }); + } +} + diff --git a/resources/views/locations/edit.blade.php b/resources/views/locations/edit.blade.php index 4b4e655a5288..1d12b1cda98e 100755 --- a/resources/views/locations/edit.blade.php +++ b/resources/views/locations/edit.blade.php @@ -17,6 +17,9 @@ @include ('partials.forms.edit.user-select', ['translated_name' => trans('admin/users/table.manager'), 'fieldname' => 'manager_id']) + +@include ('partials.forms.edit.company-select', ['translated_name' => trans('general.company'), 'fieldname' => 'company_id']) + @include ('partials.forms.edit.phone') @include ('partials.forms.edit.fax') diff --git a/resources/views/locations/index.blade.php b/resources/views/locations/index.blade.php index 2d020cb0262e..ba0abc545c6b 100755 --- a/resources/views/locations/index.blade.php +++ b/resources/views/locations/index.blade.php @@ -41,7 +41,7 @@ data-sort-order="asc" id="locationTable" class="table table-striped snipe-table" - data-url="{{ route('api.locations.index') }}" + data-url="{{ route('api.locations.index', array('company_id'=>e(Request::get('company_id')))) }}" data-export-options='{ "fileName": "export-locations-{{ date('Y-m-d') }}", "ignoreColumn": ["actions","image","change","checkbox","checkincheckout","icon"] diff --git a/resources/views/locations/print.blade.php b/resources/views/locations/print.blade.php index 9f54978ff5e3..b7ee71cee2ff 100644 --- a/resources/views/locations/print.blade.php +++ b/resources/views/locations/print.blade.php @@ -53,7 +53,11 @@ @if ($parent) {{ $parent->present()->fullName() }} @endif - +
+@if ($company) + {{ trans('admin/companies/table.name') }}: {{ $company->present()->Name() }} +
+@endif @if ($manager) {{ trans('general.manager') }} {{ $manager->present()->fullName() }}
@endif diff --git a/resources/views/locations/view.blade.php b/resources/views/locations/view.blade.php index a0d3b8c742f7..d9f83020b733 100644 --- a/resources/views/locations/view.blade.php +++ b/resources/views/locations/view.blade.php @@ -412,6 +412,9 @@ class="table table-striped snipe-table" @if ($location->manager)
  • {{ trans('admin/users/table.manager') }}: {!! $location->manager->present()->nameUrl() !!}
    • @endif + @if ($location->company) +
  • {{ trans('admin/companies/table.name') }}: {!! $location->company->present()->nameUrl() !!}
    • + @endif @if ($location->parent)
  • {{ trans('admin/locations/table.parent') }}: {!! $location->parent->present()->nameUrl() !!}
    • @endif diff --git a/resources/views/modals/location.blade.php b/resources/views/modals/location.blade.php index 9580f4bbf936..59c4516d9814 100644 --- a/resources/views/modals/location.blade.php +++ b/resources/views/modals/location.blade.php @@ -11,6 +11,16 @@ @include('modals.partials.name', ['item' => new \App\Models\Location(), 'required' => 'true']) + + @if ($user->company) + + @endif + + +
    + @include ('partials.forms.edit.company-select', ['translated_name' => trans('general.company'), 'fieldname' => 'company_id']) +
    +
    diff --git a/resources/views/partials/bootstrap-table.blade.php b/resources/views/partials/bootstrap-table.blade.php index f24552d75366..42bcc41b32c7 100644 --- a/resources/views/partials/bootstrap-table.blade.php +++ b/resources/views/partials/bootstrap-table.blade.php @@ -748,6 +748,14 @@ function usersCompanyObjFilterFormatter(value, row) { } } + function locationCompanyObjFilterFormatter(value, row) { + if (value) { + return '' + row.company.name + ''; + } else { + return value; + } + } + function employeeNumFormatter(value, row) { if ((row) && (row.assigned_to) && ((row.assigned_to.employee_number))) { From 1318dc611150bc83975dc5d6aec8ad756d26168d Mon Sep 17 00:00:00 2001 From: Tobias Regnery Date: Mon, 27 Feb 2023 13:54:41 +0100 Subject: [PATCH 2/5] Add a backward compatibility setting for locations with companies Now that locations have a company_id they get restricted to the users company with FullMultipleCompanySupport. This breaks backward compatibility, because before everyone can handle locations without restrictions. Add a setting right below FullMultipleCompanySupport so that everyone can switch to the desired behaviour. The default is off and the existing behaviour is preserved. --- .../Controllers/Api/LocationsController.php | 24 +++++++++++--- app/Http/Controllers/LocationsController.php | 16 +++++++++- app/Http/Controllers/SettingsController.php | 7 ++++ app/Models/Location.php | 12 ++++++- ..._27_092130_add_scope_locations_setting.php | 32 +++++++++++++++++++ .../lang/de-DE/admin/settings/general.php | 2 ++ .../lang/en-US/admin/settings/general.php | 2 ++ resources/views/modals/location.blade.php | 4 +-- resources/views/settings/general.blade.php | 18 ++++++++++- 9 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 database/migrations/2023_02_27_092130_add_scope_locations_setting.php diff --git a/app/Http/Controllers/Api/LocationsController.php b/app/Http/Controllers/Api/LocationsController.php index 93ba287d4d5d..eb6d720f1a96 100644 --- a/app/Http/Controllers/Api/LocationsController.php +++ b/app/Http/Controllers/Api/LocationsController.php @@ -11,6 +11,7 @@ use App\Models\Asset; use App\Models\Company; use App\Models\Location; +use App\Models\Setting; use Illuminate\Http\Request; use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Support\Collection; @@ -77,7 +78,10 @@ public function index(Request $request) : JsonResponse | array ->withCount('children as children_count') ->withCount('users as users_count'); - $locations = Company::scopeCompanyables($locations); + // Only scope locations if the setting is enabled + if (Setting::getSettings()->scope_locations_fmcs) { + $locations = Company::scopeCompanyables($locations); + } if ($request->filled('search')) { $locations = $locations->TextSearch($request->input('search')); @@ -159,9 +163,13 @@ public function store(ImageUploadRequest $request) : JsonResponse $this->authorize('create', Location::class); $location = new Location; $location->fill($request->all()); - $location->company_id = Company::getIdForCurrentUser($request->get('company_id')); $location = $request->handleImages($location); + // Only scope location if the setting is enabled + if (Setting::getSettings()->scope_locations_fmcs) { + $location->company_id = Company::getIdForCurrentUser($request->get('company_id')); + } + if ($location->save()) { return response()->json(Helper::formatStandardApiResponse('success', (new LocationsTransformer)->transformLocation($location), trans('admin/locations/message.create.success'))); } @@ -223,7 +231,12 @@ public function update(ImageUploadRequest $request, $id) : JsonResponse $location = $request->handleImages($location); if ($request->filled('company_id')) { - $location->company_id = Company::getIdForCurrentUser($request->get('company_id')); + // Only scope location if the setting is enabled + if (Setting::getSettings()->scope_locations_fmcs) { + $location->company_id = Company::getIdForCurrentUser($request->get('company_id')); + } else { + $location->company_id = $request->get('company_id'); + } } if ($location->isValid()) { @@ -322,7 +335,10 @@ public function selectlist(Request $request) : array 'locations.image', ]); - $locations = Company::scopeCompanyables($locations); + // Only scope locations if the setting is enabled + if (Setting::getSettings()->scope_locations_fmcs) { + $locations = Company::scopeCompanyables($locations); + } $page = 1; if ($request->filled('page')) { diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index 7784ad1825c3..e21f00b507ee 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/app/Http/Controllers/LocationsController.php @@ -7,6 +7,7 @@ use App\Models\Asset; use App\Models\Company; use App\Models\Location; +use App\Models\Setting; use App\Models\User; use Illuminate\Support\Facades\Storage; use Illuminate\Http\Request; @@ -81,6 +82,13 @@ public function store(ImageUploadRequest $request) : RedirectResponse $location->fax = request('fax'); $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); + // Only scope the location if the setting is enabled + if (Setting::getSettings()->scope_locations_fmcs) { + $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); + } else { + $location->company_id = $request->input('company_id'); + } + $location = $request->handleImages($location); if ($location->save()) { @@ -140,7 +148,13 @@ public function update(ImageUploadRequest $request, $locationId = null) : Redire $location->fax = request('fax'); $location->ldap_ou = $request->input('ldap_ou'); $location->manager_id = $request->input('manager_id'); - $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); + + // Only scope the location if the setting is enabled + if (Setting::getSettings()->scope_locations_fmcs) { + $location->company_id = Company::getIdForCurrentUser($request->input('company_id')); + } else { + $location->company_id = $request->input('company_id'); + } $location = $request->handleImages($location); diff --git a/app/Http/Controllers/SettingsController.php b/app/Http/Controllers/SettingsController.php index aa773d9eea5f..5a3a35abb2f9 100755 --- a/app/Http/Controllers/SettingsController.php +++ b/app/Http/Controllers/SettingsController.php @@ -314,6 +314,13 @@ public function postSettings(Request $request) : RedirectResponse } $setting->full_multiple_companies_support = $request->input('full_multiple_companies_support', '0'); + $setting->scope_locations_fmcs = $request->input('scope_locations_fmcs', '0'); + + // Backward compatibility for locations makes no sense without FullMultipleCompanySupport + if (!$setting->full_multiple_companies_support) { + $setting->scope_locations_fmcs = '0'; + } + $setting->unique_serial = $request->input('unique_serial', '0'); $setting->shortcuts_enabled = $request->input('shortcuts_enabled', '0'); $setting->show_images_in_email = $request->input('show_images_in_email', '0'); diff --git a/app/Models/Location.php b/app/Models/Location.php index 7f99c785310b..dee2fbd27195 100755 --- a/app/Models/Location.php +++ b/app/Models/Location.php @@ -4,6 +4,7 @@ use App\Http\Traits\UniqueUndeletedTrait; use App\Models\Asset; +use App\Models\Setting; use App\Models\SnipeModel; use App\Models\Traits\Searchable; use App\Models\User; @@ -17,12 +18,21 @@ class Location extends SnipeModel { + function __construct() { + parent::__construct(); + // This is a workaround for backward compatibility with older versions where locations doesn't get scoped. + // Normaly we would only add 'use CompanyableTrait;', but this has to be conditional on the setting. + // So instead of using the trait, add the scope directly if no backward compatibility is used + if (Setting::getSettings()->scope_locations_fmcs) { + static::addGlobalScope(new CompanyableScope); + } + } + use HasFactory; protected $presenter = \App\Presenters\LocationPresenter::class; use Presentable; use SoftDeletes; - use CompanyableTrait; protected $table = 'locations'; protected $rules = [ diff --git a/database/migrations/2023_02_27_092130_add_scope_locations_setting.php b/database/migrations/2023_02_27_092130_add_scope_locations_setting.php new file mode 100644 index 000000000000..c1e2ff83e9fe --- /dev/null +++ b/database/migrations/2023_02_27_092130_add_scope_locations_setting.php @@ -0,0 +1,32 @@ +boolean('scope_locations_fmcs')->default('0')->after('full_multiple_companies_support'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table('settings', function (Blueprint $table) { + $table->dropColumn('scope_locations_fmcs'); + }); + } +} \ No newline at end of file diff --git a/resources/lang/de-DE/admin/settings/general.php b/resources/lang/de-DE/admin/settings/general.php index 6d01f0d10a9a..ebce8536c54e 100644 --- a/resources/lang/de-DE/admin/settings/general.php +++ b/resources/lang/de-DE/admin/settings/general.php @@ -148,6 +148,8 @@ 'logo_print_assets_help' => 'Firmenlogo anzeigen beim Drucken der Asset-Liste ', 'full_multiple_companies_support_help_text' => 'Beschränkung von Benutzern (inklusive Administratoren) die einer Firma zugewiesen sind zu den Assets der Firma.', 'full_multiple_companies_support_text' => 'Volle Mehrmandanten-Unterstützung für Firmen', + 'scope_locations_fmcs_support_text' => 'Beschränke Standorte mit voller Mehrmandanten-Unterstützung für Firmen', + 'scope_locations_fmcs_support_help_text' => 'Bis zu Version 7.0 waren Standorte nicht auf die Firma des Benutzers beschränkt. Wenn diese Einstellung deaktiviert ist, wird die Kompatibilität zu älteren Versionen gewahrt und die Standorte nicht beschränkt. Wenn diese Einstellung aktiviert ist, werden Standorte ebenfalls auf die Firma des Benutzers beschränkt.', 'show_in_model_list' => 'In Modell-Dropdown-Liste anzeigen', 'optional' => 'optional', 'per_page' => 'Ergebnisse pro Seite', diff --git a/resources/lang/en-US/admin/settings/general.php b/resources/lang/en-US/admin/settings/general.php index d656391edd5a..010dd3d5ecf3 100644 --- a/resources/lang/en-US/admin/settings/general.php +++ b/resources/lang/en-US/admin/settings/general.php @@ -148,6 +148,8 @@ 'logo_print_assets_help' => 'Use branding on printable asset lists ', 'full_multiple_companies_support_help_text' => 'Restricting users (including admins) assigned to companies to their company\'s assets.', 'full_multiple_companies_support_text' => 'Full Multiple Companies Support', + 'scope_locations_fmcs_support_text' => 'Scope Locations with Full Multiple Companies Support', + 'scope_locations_fmcs_support_help_text' => 'Up until Version 7.0 locations were not restricted to the users company. If this setting is disabled, this preserves backward compatibility with older versions and locations are not restricted. If this setting is enabled, locations are also restricted to the users company', 'show_in_model_list' => 'Show in Model Dropdowns', 'optional' => 'optional', 'per_page' => 'Results Per Page', diff --git a/resources/views/modals/location.blade.php b/resources/views/modals/location.blade.php index 59c4516d9814..6816f32869d4 100644 --- a/resources/views/modals/location.blade.php +++ b/resources/views/modals/location.blade.php @@ -11,8 +11,8 @@
    @include('modals.partials.name', ['item' => new \App\Models\Location(), 'required' => 'true']) - - @if ($user->company) + + @if (($snipeSettings->scope_locations_fmcs == '1') && ($user->company)) @endif diff --git a/resources/views/settings/general.blade.php b/resources/views/settings/general.blade.php index 17c0a8ec81db..74c0b27264a6 100644 --- a/resources/views/settings/general.blade.php +++ b/resources/views/settings/general.blade.php @@ -54,7 +54,24 @@

    + + +
    +
    + {{ Form::label('scope_locations_fmcs', trans('admin/settings/general.scope_locations_fmcs_support_text')) }} +
    +
    + + {!! $errors->first('scope_locations_fmcs', '') !!} +

    + {{ trans('admin/settings/general.scope_locations_fmcs_support_help_text') }} +

    +
    +
    @@ -479,6 +496,5 @@ }); }); - @stop From 651d1c735b83cf01fed2ba8cb0607680284a5ba4 Mon Sep 17 00:00:00 2001 From: Tobias Regnery Date: Wed, 13 Sep 2023 10:54:23 +0200 Subject: [PATCH 3/5] Another slightly less ugly way for backward compatibility Instead of using a constructor, add a special check in the boot-method for locations. This seems to fit better in the system and does hopefully not break the existing tests. Signed-off-by: Tobias Regnery --- app/Models/CompanyableTrait.php | 9 ++++++++- app/Models/Location.php | 11 +---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/app/Models/CompanyableTrait.php b/app/Models/CompanyableTrait.php index 04a620d8e3d5..c4b9b89fbff6 100644 --- a/app/Models/CompanyableTrait.php +++ b/app/Models/CompanyableTrait.php @@ -13,6 +13,13 @@ trait CompanyableTrait */ public static function bootCompanyableTrait() { - static::addGlobalScope(new CompanyableScope); + // In Version 7.0 and before locations weren't scoped by companies, so add a check for the backward compatibility setting + if (__CLASS__ != 'App\Models\Location') { + static::addGlobalScope(new CompanyableScope); + } else { + if (Setting::getSettings()->scope_locations_fmcs == 1) { + static::addGlobalScope(new CompanyableScope); + } + } } } diff --git a/app/Models/Location.php b/app/Models/Location.php index dee2fbd27195..6d92e55b4478 100755 --- a/app/Models/Location.php +++ b/app/Models/Location.php @@ -18,17 +18,8 @@ class Location extends SnipeModel { - function __construct() { - parent::__construct(); - // This is a workaround for backward compatibility with older versions where locations doesn't get scoped. - // Normaly we would only add 'use CompanyableTrait;', but this has to be conditional on the setting. - // So instead of using the trait, add the scope directly if no backward compatibility is used - if (Setting::getSettings()->scope_locations_fmcs) { - static::addGlobalScope(new CompanyableScope); - } - } - use HasFactory; + use CompanyableTrait; protected $presenter = \App\Presenters\LocationPresenter::class; use Presentable; From 6921df933479399af9774f8ce90c339781d35a05 Mon Sep 17 00:00:00 2001 From: Tobias Regnery Date: Thu, 23 Jan 2025 15:26:04 +0100 Subject: [PATCH 4/5] Check for inconsistencies before activating scoped locations Before activating scoped location all locations and their related objects will be checked. If there are locations with different companies than the related objects error out. Because this operation is quite slow, bail out on the first inconsistent entry. There is a new artisan command introduced that checks every location. Depending on the size of the database, this will take very long. Signed-off-by: Tobias Regnery --- app/Console/Commands/TestLocationsFMCS.php | 37 +++++++++++++++++ app/Helpers/Helper.php | 45 +++++++++++++++++++++ app/Http/Controllers/SettingsController.php | 9 +++++ 3 files changed, 91 insertions(+) create mode 100644 app/Console/Commands/TestLocationsFMCS.php diff --git a/app/Console/Commands/TestLocationsFMCS.php b/app/Console/Commands/TestLocationsFMCS.php new file mode 100644 index 000000000000..e7562e23cf4e --- /dev/null +++ b/app/Console/Commands/TestLocationsFMCS.php @@ -0,0 +1,37 @@ +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); + + foreach($ret as $output) { + $this->info($output); + } + } +} diff --git a/app/Helpers/Helper.php b/app/Helpers/Helper.php index 95a344dce9e7..dcfb5e995dd1 100644 --- a/app/Helpers/Helper.php +++ b/app/Helpers/Helper.php @@ -12,6 +12,7 @@ use App\Models\Setting; use App\Models\Statuslabel; use App\Models\License; +use App\Models\Location; use Illuminate\Support\Facades\Crypt; use Illuminate\Contracts\Encryption\DecryptException; use Carbon\Carbon; @@ -1529,4 +1530,48 @@ static public function getRedirectOption($request, $id, $table, $item_id = null) } return redirect()->back()->with('error', trans('admin/hardware/message.checkout.error')); } + + /** + * Check for inconsistencies before activating scoped locations with FullMultipleCompanySupport + * If there are locations with different companies than related objects unforseen problems could arise + * + * @author T. Regnery + * @since 7.0 + * + * @param $artisan when false, bail out on first inconsistent entry + * @return string [] + */ + static public function test_locations_fmcs($artisan) { + $ret = []; + $locations = Location::all(); + + foreach($locations as $location) { + $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']]; + + foreach ($keywords_relation as $relation => $keywords) { + foreach($keywords as $keyword) { + if ($relation == 'many') { + $items = $location->$keyword->all(); + } else { + $items = array($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 + if (!$artisan) { + return $ret; + } + } + } + } + } + } + return $ret; + } } diff --git a/app/Http/Controllers/SettingsController.php b/app/Http/Controllers/SettingsController.php index 2784a0bf184c..6f69de8bdab0 100755 --- a/app/Http/Controllers/SettingsController.php +++ b/app/Http/Controllers/SettingsController.php @@ -314,6 +314,7 @@ public function postSettings(Request $request) : RedirectResponse $setting->modellist_displays = implode(',', $request->input('show_in_model_list')); } + $old_locations_fmcs = $setting->scope_locations_fmcs; $setting->full_multiple_companies_support = $request->input('full_multiple_companies_support', '0'); $setting->scope_locations_fmcs = $request->input('scope_locations_fmcs', '0'); @@ -322,6 +323,14 @@ public function postSettings(Request $request) : RedirectResponse $setting->scope_locations_fmcs = '0'; } + // check for inconsistencies when activating scoped locations + if ($old_locations_fmcs == '0' && $setting->scope_locations_fmcs == '1') { + $ret = Helper::test_locations_fmcs(false); + if (count($ret) != 0) { + return redirect()->back()->withInput()->with('error', 'Inconsistencies with scoped locations found, please use php artisan snipeit:test-locations-fmcs for details'); + } + } + $setting->unique_serial = $request->input('unique_serial', '0'); $setting->shortcuts_enabled = $request->input('shortcuts_enabled', '0'); $setting->show_images_in_email = $request->input('show_images_in_email', '0'); From 4e0bcac1a1c2cc24ced481de92703876062c5fbc Mon Sep 17 00:00:00 2001 From: Tobias Regnery Date: Fri, 24 Jan 2025 11:12:11 +0100 Subject: [PATCH 5/5] 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 --- app/Console/Commands/TestLocationsFMCS.php | 10 ++++-- app/Helpers/Helper.php | 34 +++++++++++++++---- .../Controllers/Api/LocationsController.php | 8 +++++ app/Http/Controllers/LocationsController.php | 9 +++++ app/Models/Accessory.php | 1 + app/Models/Asset.php | 6 ++-- app/Models/Component.php | 1 + app/Models/Consumable.php | 1 + app/Models/User.php | 2 +- app/Providers/ValidationServiceProvider.php | 15 ++++++++ 10 files changed, 74 insertions(+), 13 deletions(-) diff --git a/app/Console/Commands/TestLocationsFMCS.php b/app/Console/Commands/TestLocationsFMCS.php index e7562e23cf4e..fb606e6db43a 100644 --- a/app/Console/Commands/TestLocationsFMCS.php +++ b/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 @@ public function handle() { $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); diff --git a/app/Helpers/Helper.php b/app/Helpers/Helper.php index dcfb5e995dd1..14b8fcba5fe2 100644 --- a/app/Helpers/Helper.php +++ b/app/Helpers/Helper.php @@ -1538,32 +1538,52 @@ static public function getRedirectOption($request, $id, $table, $item_id = null) * @author T. Regnery * @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; } diff --git a/app/Http/Controllers/Api/LocationsController.php b/app/Http/Controllers/Api/LocationsController.php index 40f1ebc3cdee..be96dc4c98f8 100644 --- a/app/Http/Controllers/Api/LocationsController.php +++ b/app/Http/Controllers/Api/LocationsController.php @@ -177,6 +177,10 @@ public function store(ImageUploadRequest $request) : JsonResponse // 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 @@ public function update(ImageUploadRequest $request, $id) : JsonResponse // 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'); } diff --git a/app/Http/Controllers/LocationsController.php b/app/Http/Controllers/LocationsController.php index e21f00b507ee..c7a8476c4628 100755 --- a/app/Http/Controllers/LocationsController.php +++ b/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 @@ public function store(ImageUploadRequest $request) : RedirectResponse // 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 @@ public function update(ImageUploadRequest $request, $locationId = null) : Redire // 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'); } diff --git a/app/Models/Accessory.php b/app/Models/Accessory.php index fc1bb36ab40d..039f8692f6b4 100755 --- a/app/Models/Accessory.php +++ b/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', diff --git a/app/Models/Asset.php b/app/Models/Asset.php index ce8b870eb2e0..2fe5b299dff6 100644 --- a/app/Models/Asset.php +++ b/app/Models/Asset.php @@ -108,8 +108,8 @@ public function declinedCheckout(User $declinedBy, $signature) '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'], - 'rtd_location_id' => ['nullable', 'exists:locations,id'], + 'location_id' => ['nullable', 'exists:locations,id', 'fmcs_location'], + '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 @@ public function declinedCheckout(User $declinedBy, $signature) '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'] ]; diff --git a/app/Models/Component.php b/app/Models/Component.php index fb77bf082412..7c5aa15d1ad8 100644 --- a/app/Models/Component.php +++ b/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', diff --git a/app/Models/Consumable.php b/app/Models/Consumable.php index 30161e84296a..45dd67e1c711 100644 --- a/app/Models/Consumable.php +++ b/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', diff --git a/app/Models/User.php b/app/Models/User.php index e48b8bf074f8..029bda04613d 100644 --- a/app/Models/User.php +++ b/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', diff --git a/app/Providers/ValidationServiceProvider.php b/app/Providers/ValidationServiceProvider.php index 76ba1b629aa9..a08a035fbbf0 100644 --- a/app/Providers/ValidationServiceProvider.php +++ b/app/Providers/ValidationServiceProvider.php @@ -4,6 +4,7 @@ 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 @@ public function boot() 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; + }); } /**