Fixing authorization issues (#5807)

* adds permission checks for companies

* adds permission checks for depreciations

* adds permission check for all reports

* fixes permissions for departments

* fixes permission naming (edit -> update)

* fixes authorization checking wrong permission in API

The authorization was checking for the non-existent „edit“ method where it should have checked for the „update“ method.

* adds authorization checks for select2 lists

* adds missing authorization checks for api

* fixes user authorization check for creating users

* adds additional check viewing assets on showing a users assets

* Removes authorization checks for select2 lists

Reference: https://github.com/snipe/snipe-it/pull/5807#pullrequestreview-136018755

app/Http/Controllers/Api/AssetModelsController.php

@@ -145,7 +145,7 @@ class AssetModelsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', AssetModel::class);
+        $this->authorize('update', AssetModel::class);
         $assetmodel = AssetModel::findOrFail($id);
         $assetmodel->fill($request->all());
         $assetmodel->fieldset_id = $request->get("custom_fieldset_id");

app/Http/Controllers/Api/AssetsController.php

@@ -465,7 +465,7 @@ class AssetsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Asset::class);
+        $this->authorize('update', Asset::class);
 
         if ($asset = Asset::find($id)) {
             ($request->has('model_id')) ?
@@ -755,6 +755,7 @@ class AssetsController extends Controller
      */
     public function requestable(Request $request)
     {
+        $this->authorize('viewRequestable', Asset::class);
 
         $assets = Company::scopeCompanyables(Asset::select('assets.*'),"company_id","assets")
             ->with('location', 'assetstatus', 'assetlog', 'company', 'defaultLoc','assignedTo',

app/Http/Controllers/Api/CategoriesController.php

@@ -92,7 +92,7 @@ class CategoriesController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Category::class);
+        $this->authorize('update', Category::class);
         $category = Category::findOrFail($id);
         $category->fill($request->all());
 

app/Http/Controllers/Api/CompaniesController.php

@@ -104,7 +104,7 @@ class CompaniesController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Company::class);
+        $this->authorize('update', Company::class);
         $company = Company::findOrFail($id);
         $company->fill($request->all());
 

app/Http/Controllers/Api/ComponentsController.php

@@ -116,7 +116,7 @@ class ComponentsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Component::class);
+        $this->authorize('update', Component::class);
         $component = Component::findOrFail($id);
         $component->fill($request->all());
 

app/Http/Controllers/Api/ConsumablesController.php

@@ -121,7 +121,7 @@ class ConsumablesController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Consumable::class);
+        $this->authorize('update', Consumable::class);
         $consumable = Consumable::findOrFail($id);
         $consumable->fill($request->all());
 

app/Http/Controllers/Api/CustomFieldsController.php

@@ -57,7 +57,7 @@ class CustomFieldsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', CustomField::class);
+        $this->authorize('update', CustomField::class);
         $field = CustomField::findOrFail($id);
         $data = $request->all();
 
@@ -106,6 +106,9 @@ class CustomFieldsController extends Controller
     public function postReorder(Request $request, $id)
     {
         $fieldset = CustomFieldset::find($id);
+
+        $this->authorize('update', $fieldset);
+
         $fields = array();
         $order_array = array();
 
@@ -125,7 +128,8 @@ class CustomFieldsController extends Controller
 
     public function associate(Request $request, $field_id)
     {
-        $this->authorize('edit', CustomFieldset::class);
+        $this->authorize('update', CustomFieldset::class);
+
         $field = CustomField::findOrFail($field_id);
 
         $fieldset_id = $request->input('fieldset_id');
@@ -142,7 +146,8 @@ class CustomFieldsController extends Controller
 
     public function disassociate(Request $request, $field_id)
     {
-        $this->authorize('edit', CustomFieldset::class);
+        $this->authorize('update', CustomFieldset::class);
+
         $field = CustomField::findOrFail($field_id);
 
         $fieldset_id = $request->input('fieldset_id');
@@ -167,6 +172,8 @@ class CustomFieldsController extends Controller
     {
         $field = CustomField::findOrFail($field_id);
 
+        $this->authorize('delete', $field);
+
         if ($field->fieldset->count() >0) {
             return response()->json(Helper::formatStandardApiResponse('error', null, 'Field is in use.'));
         }

app/Http/Controllers/Api/CustomFieldsetsController.php

@@ -79,7 +79,7 @@ class CustomFieldsetsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', CustomFieldset::class);
+        $this->authorize('update', CustomFieldset::class);
         $fieldset = CustomFieldset::findOrFail($id);
         $fieldset->fill($request->all());
 

app/Http/Controllers/Api/DepartmentsController.php

@@ -114,6 +114,8 @@ class DepartmentsController extends Controller
     {
         $department = Department::findOrFail($id);
 
+        $this->authorize('delete', $department);
+
         if ($department->users->count() > 0) {
             return response()->json(Helper::formatStandardApiResponse('error', null, trans('admin/departments/message.assoc_users')));
         }

app/Http/Controllers/Api/DepreciationsController.php

@@ -88,7 +88,7 @@ class DepreciationsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Depreciation::class);
+        $this->authorize('update', Depreciation::class);
         $depreciation = Depreciation::findOrFail($id);
         $depreciation->fill($request->all());
 

app/Http/Controllers/Api/GroupsController.php

@@ -88,7 +88,7 @@ class GroupsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Group::class);
+        $this->authorize('update', Group::class);
         $group = Group::findOrFail($id);
         $group->fill($request->all());
 

app/Http/Controllers/Api/LicensesController.php

@@ -168,7 +168,7 @@ class LicensesController extends Controller
     public function update(Request $request, $id)
     {
         //
-        $this->authorize('edit', License::class);
+        $this->authorize('update', License::class);
 
         $license = License::findOrFail($id);
         $license->fill($request->all());
@@ -223,6 +223,8 @@ class LicensesController extends Controller
 
         if ($license = License::find($licenseId)) {
 
+            $this->authorize('view', $license);
+
             $seats = LicenseSeat::where('license_id', $licenseId)->with('license', 'user', 'asset');
 
             $offset = request('offset', 0);

app/Http/Controllers/Api/LocationsController.php

@@ -122,7 +122,7 @@ class LocationsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Location::class);
+        $this->authorize('update', Location::class);
         $location = Location::findOrFail($id);
         $location->fill($request->all());
 

app/Http/Controllers/Api/ManufacturersController.php

@@ -99,7 +99,7 @@ class ManufacturersController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Manufacturer::class);
+        $this->authorize('update', Manufacturer::class);
         $manufacturer = Manufacturer::findOrFail($id);
         $manufacturer->fill($request->all());
 

app/Http/Controllers/Api/ReportsController.php

@@ -18,6 +18,7 @@ class ReportsController extends Controller
      */
     public function index(Request $request)
     {
+        $this->authorize('reports.view');
         
         $actionlogs = Actionlog::with('item', 'user', 'target','location');
 

app/Http/Controllers/Api/StatuslabelsController.php

@@ -101,7 +101,7 @@ class StatuslabelsController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Statuslabel::class);
+        $this->authorize('update', Statuslabel::class);
         $statuslabel = Statuslabel::findOrFail($id);
         
         $request->except('deployable', 'pending','archived');
@@ -160,6 +160,7 @@ class StatuslabelsController extends Controller
 
     public function getAssetCountByStatuslabel()
     {
+        $this->authorize('view', Statuslabel::class);
 
         $statuslabels = Statuslabel::with('assets')->groupBy('id')->withCount('assets')->get();
 
@@ -237,6 +238,9 @@ class StatuslabelsController extends Controller
      */
     public function checkIfDeployable($id) {
         $statuslabel = Statuslabel::findOrFail($id);
+
+        $this->authorize('view', $statuslabel);
+
         if ($statuslabel->getStatuslabelType()=='deployable') {
             return '1';
         }

app/Http/Controllers/Api/SuppliersController.php

@@ -93,7 +93,7 @@ class SuppliersController extends Controller
      */
     public function update(Request $request, $id)
     {
-        $this->authorize('edit', Supplier::class);
+        $this->authorize('update', Supplier::class);
         $supplier = Supplier::findOrFail($id);
         $supplier->fill($request->all());
 

app/Http/Controllers/Api/UsersController.php

@@ -191,7 +191,8 @@ class UsersController extends Controller
      */
     public function store(SaveUserRequest $request)
     {
-        $this->authorize('view', User::class);
+        $this->authorize('create', User::class);
+
         $user = new User;
         $user->fill($request->all());
 
@@ -230,7 +231,8 @@ class UsersController extends Controller
      */
     public function update(SaveUserRequest $request, $id)
     {
-        $this->authorize('edit', User::class);
+        $this->authorize('update', User::class);
+
         $user = User::findOrFail($id);
         $user->fill($request->all());
 
@@ -289,6 +291,7 @@ class UsersController extends Controller
     public function assets($id)
     {
         $this->authorize('view', User::class);
+        $this->authorize('view', Asset::class);
         $assets = Asset::where('assigned_to', '=', $id)->with('model')->get();
         return (new AssetsTransformer)->transformAssets($assets, $assets->count());
     }
@@ -304,7 +307,7 @@ class UsersController extends Controller
     public function postTwoFactorReset(Request $request)
     {
 
-        $this->authorize('edit', User::class);
+        $this->authorize('update', User::class);
 
         if ($request->has('id')) {
             try {

app/Http/Controllers/CompaniesController.php

@@ -29,6 +29,8 @@ final class CompaniesController extends Controller
      */
     public function index()
     {
+        $this->authorize('view', Company::class);
+
         return view('companies/index')->with('companies', Company::all());
     }
 
@@ -41,6 +43,8 @@ final class CompaniesController extends Controller
      */
     public function create()
     {
+        $this->authorize('create', Company::class);
+
         return view('companies/edit')->with('item', new Company);
     }
 
@@ -54,6 +58,8 @@ final class CompaniesController extends Controller
      */
     public function store(ImageUploadRequest $request)
     {
+        $this->authorize('create', Company::class);
+
         $company = new Company;
         $company->name = $request->input('name');
 
@@ -90,6 +96,9 @@ final class CompaniesController extends Controller
             return redirect()->route('companies.index')
                 ->with('error', trans('admin/companies/message.does_not_exist'));
         }
+
+        $this->authorize('update', $item);
+
         return view('companies/edit')->with('item', $item);
     }
 
@@ -108,6 +117,8 @@ final class CompaniesController extends Controller
             return redirect()->route('companies.index')->with('error', trans('admin/companies/message.does_not_exist'));
         }
 
+        $this->authorize('update', $company);
+
         $company->name = $request->input('name');
 
         $old_image = $company->image;
@@ -164,6 +175,9 @@ final class CompaniesController extends Controller
             return redirect()->route('companies.index')
                 ->with('error', trans('admin/companies/message.not_found'));
         } else {
+
+            $this->authorize('delete', $company);
+
             try {
                 $company->delete();
                 return redirect()->route('companies.index')

app/Http/Controllers/DepartmentsController.php

@@ -83,6 +83,8 @@ class DepartmentsController extends Controller
     {
         $department = Department::find($id);
 
+        $this->authorize('view', $department);
+
         if (isset($department->id)) {
             return view('departments/view', compact('department'));
         }
@@ -100,6 +102,8 @@ class DepartmentsController extends Controller
      */
     public function create()
     {
+        $this->authorize('create', Department::class);
+
         return view('departments/edit')->with('item', new Department);
     }
 
@@ -118,6 +122,8 @@ class DepartmentsController extends Controller
             return redirect()->to(route('departments.index'))->with('error', trans('admin/departments/message.not_found'));
         }
 
+        $this->authorize('delete', $department);
+
         if ($department->users->count() > 0) {
             return redirect()->to(route('departments.index'))->with('error', trans('admin/departments/message.assoc_users'));
         }
@@ -141,16 +147,20 @@ class DepartmentsController extends Controller
         if (is_null($item = Department::find($id))) {
             return redirect()->back()->with('error', trans('admin/locations/message.does_not_exist'));
         }
+
+        $this->authorize('update', $item);
+
         return view('departments/edit', compact('item'));
     }
 
     public function update(ImageUploadRequest $request, $id) {
 
-        $this->authorize('create', Department::class);
         if (is_null($department = Department::find($id))) {
             return redirect()->route('departments.index')->with('error', trans('admin/departments/message.does_not_exist'));
         }
 
+        $this->authorize('update', $department);
+
         $department->fill($request->all());
         $department->manager_id = ($request->has('manager_id' ) ? $request->input('manager_id') : null);
 

app/Http/Controllers/DepreciationsController.php

@@ -31,6 +31,8 @@ class DepreciationsController extends Controller
      */
     public function index()
     {
+        $this->authorize('view', Depreciation::class);
+
         // Show the page
         return view('depreciations/index', compact('depreciations'));
     }
@@ -46,6 +48,8 @@ class DepreciationsController extends Controller
      */
     public function create()
     {
+        $this->authorize('create', Depreciation::class);
+
         // Show the page
         return view('depreciations/edit')->with('item', new Depreciation);
     }
@@ -62,6 +66,8 @@ class DepreciationsController extends Controller
      */
     public function store(Request $request)
     {
+        $this->authorize('create', Depreciation::class);
+
         // create a new instance
         $depreciation = new Depreciation();
         // Depreciation data
@@ -94,6 +100,8 @@ class DepreciationsController extends Controller
             return redirect()->route('depreciations.index')->with('error', trans('admin/depreciations/message.does_not_exist'));
         }
 
+        $this->authorize('update', $item);
+
         return view('depreciations/edit', compact('item'));
     }
 
@@ -116,6 +124,8 @@ class DepreciationsController extends Controller
             return redirect()->route('depreciations.index')->with('error', trans('admin/depreciations/message.does_not_exist'));
         }
 
+        $this->authorize('update', $depreciation);
+
         // Depreciation data
         $depreciation->name      = $request->input('name');
         $depreciation->months    = $request->input('months');
@@ -145,6 +155,8 @@ class DepreciationsController extends Controller
             return redirect()->route('depreciations.index')->with('error', trans('admin/depreciations/message.not_found'));
         }
 
+        $this->authorize('delete', $depreciation);
+
         if ($depreciation->has_models() > 0) {
             // Redirect to the asset management page
             return redirect()->route('depreciations.index')->with('error', trans('admin/depreciations/message.assoc_users'));
@@ -171,6 +183,8 @@ class DepreciationsController extends Controller
             return redirect()->route('depreciations.index')->with('error', trans('admin/depreciations/message.does_not_exist'));
         }
 
+        $this->authorize('view', $depreciation);
+
         return view('depreciations/view', compact('depreciation'));
     }
 

app/Http/Controllers/ReportsController.php

@@ -26,6 +26,14 @@ use Illuminate\Http\Request;
  */
 class ReportsController extends Controller
 {
+    /**
+     * Checks for correct permissions
+     */
+    public function __construct() {
+        parent::__construct();
+
+        $this->authorize('reports.view');
+    }
 
     /**
     * Returns a view that displays the accessories report.