Discussion: Moving to policies for controller based authorization (sn…

…ipe#3080) * Make delete routes work. We put a little form in the modal that spoofs the delete field. * Fix route on creating a user. * Fix redundant id parameter. * Port acceptance tests to new urls. * Initial work on migrating to model based policies instead of global gates. Will allow for much more detailed permissions bits in the future. * This needs to stay for the dashboard checks. * Add user states for permissions to build tests. * Build up unit tests for gates/permissions. Move accessories/consumables/assets to policies instead of in authserviceprovider * Migrate various locations to new syntax. Update test to be more specific * Fix functional tests. Add an artisan command for installing a settings setup on travis-ci * Try a different id... Need to come up with a better way of passing the id for tests that need an existing one. * Try to fix travis * Update urls to use routes and not hardcode old paths. Also fix some migration errors found along the way.: * Add a environment for travis functional tests. * Adjust config file to make travis use it. * Use redirect()->route instead of redirect()-to * Dump all failures in the output directory if travis fails. * Cleanups and minor fixes. * Adjust the supplier modelfactory to comply with new validation restrictions. * Some test fixes. * Locales can be longer than 5 characters according to faker... fex gez_ET. Increase lenght in mysql and add a validation * Update test database dump to latest migrations.
dmeltzer · Dec 19, 2016 · cd8c585 · cd8c585
1 parent ae2cb5f
commit cd8c585
Show file tree

Hide file tree

Showing 83 changed files with 2,439 additions and 1,281 deletions.
diff --git a/.env.testing b/.env.testing
@@ -3,7 +3,7 @@
 # --------------------------------------------
 APP_ENV=testing
 APP_DEBUG=true
-APP_KEY=ChangeMe
+APP_KEY=base64:glJpcM7BYwWiBggp3SQ/+NlRkqsBQMaGEOjemXqJzOU=
 APP_URL=http://localhost:8000
 APP_TIMEZONE='US/Pacific'
 APP_LOCALE=en

diff --git a/.travis.yml b/.travis.yml
@@ -14,18 +14,18 @@ php:
 # execute any number of scripts before the test run, custom env's are available as variables
 before_script:
   - phantomjs --webdriver=4444  &
-  - sleep 10
+  - sleep 4
   - mysql -e "create database IF NOT EXISTS snipeit_unit;" -utravis
   - composer self-update
   - composer install -n --prefer-source
-  - cp .env.testing-ci .env
   - chmod -R 777 storage
-  - php artisan migrate --database=mysql --force
+  - php artisan migrate --env=testing-ci --database=mysql --force
   - ./vendor/bin/codecept build
-  - php artisan key:generate
-  - php artisan db:seed --database=mysql --force
-  - php artisan snipeit:create-admin --first_name=Alison --last_name=Foobar --email=me@example.com --username=snipe --password=password
-  - php artisan serve --port=8000 --host=localhost &
+  - php artisan key:generate --env=testing-ci
+  - php artisan db:seed --env=testing-ci --database=mysql --force
+  - php artisan --env=testing-ci snipeit:create-admin --first_name=Alison --last_name=Foobar --email=me@example.com --username=snipe --password=password
+  - php artisan --env=testing-ci snipeit:travisci-install
+  - php artisan serve --env=testing-ci --port=8000 --host=localhost &
   - sleep 5
   - pip install --user codecov
   - sleep 5
@@ -34,16 +34,17 @@ before_script:
 
 # omitting "script:" will default to phpunit
 # use the $DB env variable to determine the phpunit.xml to use
-# script: ./vendor/bin/codecept run --env testing-ci - broken :(
-script: ./vendor/bin/codecept run unit --env testing-ci
+# script: ./vendor/bin/codecept run --env testing-ci
+script: ./vendor/bin/codecept run unit --env testing-ci && ./vendor/bin/codecept run functional --env=functional-travis
 #script: ./vendor/bin/codecept run
 
 after_success:
   - codecov
 
 after_failure:
-  - cat tests/_output/AccessoriesCept.fail.html
+  - cat tests/_output/*.fail.html
   - curl http://localhost:8000/login
+  - cat storage/logs/laravel.log
 
 # configure notifications (email, IRC, campfire etc)
 notifications:

diff --git a/app/Console/Commands/SendExpirationAlerts.php b/app/Console/Commands/SendExpirationAlerts.php
@@ -89,7 +89,7 @@ public function fire()
             } else {
                 $license_data['email_content'] .= '<tr style="background-color:#d9534f;">';
             }
-                $license_data['email_content'] .= '<td><a href="'.config('app.url').'/admin/licenses/'.$license->id.'/view">';
+                $license_data['email_content'] .= '<td><a href="'.route('licenses.show', $license->id).'">';
                 $license_data['email_content'] .= $license->name.'</a></td>';
                 $license_data['email_content'] .= '<td>'.$license->expiration_date.'</td>';
                 $license_data['email_content'] .= '<td>'.$difference.' days</td>';

diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php
@@ -40,4 +40,9 @@ protected function schedule(Schedule $schedule)
         $schedule->command('snipeit:backup')->weekly();
         $schedule->command('backup:clean')->daily();
     }
+
+    protected function commands()
+    {
+        require base_path('routes/console.php');
+    }
 }
diff --git a/app/Http/Controllers/AccessoriesController.php b/app/Http/Controllers/AccessoriesController.php
@@ -40,6 +40,7 @@ class AccessoriesController extends Controller
     */
     public function index(Request $request)
     {
+        $this->authorize('index', Accessory::class);
         return View::make('accessories/index');
     }
 
@@ -52,6 +53,7 @@ public function index(Request $request)
    */
     public function create(Request $request)
     {
+        $this->authorize('create', Accessory::class);
         // Show the page
         return View::make('accessories/edit')
           ->with('item', new Accessory)
@@ -70,7 +72,7 @@ public function create(Request $request)
    */
     public function store(Request $request)
     {
-
+        $this->authorize(Accessory::class);
         // create a new model instance
         $accessory = new Accessory();
 
@@ -123,10 +125,10 @@ public function edit(Request $request, $accessoryId = null)
         if (is_null($item = Accessory::find($accessoryId))) {
             // Redirect to the blogs management page
             return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.does_not_exist'));
-        } elseif (!Company::isCurrentUserHasAccess($item)) {
-            return redirect()->route('accessories.index')->with('error', trans('general.insufficient_permissions'));
         }
 
+        $this->authorize($item);
+
         return View::make('accessories/edit', compact('item'))
           ->with('category_list', Helper::categoryList('accessory'))
           ->with('company_list', Helper::companyList())
@@ -148,10 +150,10 @@ public function update(Request $request, $accessoryId = null)
         if (is_null($accessory = Accessory::find($accessoryId))) {
             // Redirect to the accessory index page
             return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.does_not_exist'));
-        } elseif (!Company::isCurrentUserHasAccess($accessory)) {
-            return redirect()->route('accessories.index')->with('error', trans('general.insufficient_permissions'));
         }
 
+        $this->authorize($accessory);
+
       // Update the accessory data
         $accessory->name                    = e(Input::get('name'));
 
@@ -205,10 +207,10 @@ public function destroy(Request $request, $accessoryId)
         if (is_null($accessory = Accessory::find($accessoryId))) {
             // Redirect to the blogs management page
             return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.not_found'));
-        } elseif (!Company::isCurrentUserHasAccess($accessory)) {
-            return redirect()->route('accessories.index')->with('error', trans('general.insufficient_permissions'));
         }
 
+        $this->authorize($accessory);
+
 
         if ($accessory->hasUsers() > 0) {
              return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.assoc_users', array('count'=> $accessory->hasUsers())));
@@ -236,14 +238,9 @@ public function destroy(Request $request, $accessoryId)
     public function show(Request $request, $accessoryID = null)
     {
         $accessory = Accessory::find($accessoryID);
-
+        $this->authorize('view', $accessory);
         if (isset($accessory->id)) {
-
-            if (!Company::isCurrentUserHasAccess($accessory)) {
-                return redirect()->route('accessories.index')->with('error', trans('general.insufficient_permissions'));
-            } else {
-                return View::make('accessories/view', compact('accessory'));
-            }
+            return View::make('accessories/view', compact('accessory'));
         } else {
             // Prepare the error message
             $error = trans('admin/accessories/message.does_not_exist', compact('id'));
@@ -267,11 +264,11 @@ public function getCheckout(Request $request, $accessoryId)
         // Check if the accessory exists
         if (is_null($accessory = Accessory::find($accessoryId))) {
             // Redirect to the accessory management page with error
-            return redirect()->to('accessories.index')->with('error', trans('admin/accessories/message.not_found'));
-        } elseif (!Company::isCurrentUserHasAccess($accessory)) {
-            return redirect()->route('accessories.index')->with('error', trans('general.insufficient_permissions'));
+            return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.not_found'));
         }
 
+        $this->authorize('checkout', $accessory);
+
         // Get the dropdown of users and then pass it to the checkout view
         $users_list = Helper::usersList();
 
@@ -295,10 +292,10 @@ public function postCheckout(Request $request, $accessoryId)
         if (is_null($accessory = Accessory::find($accessoryId))) {
             // Redirect to the accessory management page with error
             return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.user_not_found'));
-        } elseif (!Company::isCurrentUserHasAccess($accessory)) {
-            return redirect()->route('accessories.index')->with('error', trans('general.insufficient_permissions'));
         }
 
+        $this->authorize('checkout', $accessory);
+
         if (!$user = User::find(Input::get('assigned_to'))) {
             return redirect()->route('accessories.index')->with('error', trans('admin/accessories/message.not_found'));
         }
@@ -336,7 +333,7 @@ public function postCheckout(Request $request, $accessoryId)
                 'fields' => [
                   [
                   'title' => 'Checked Out:',
-                  'value' => 'Accessory <'.url('/').'/admin/accessories/'.$accessory->id.'/view'.'|'.$accessory->name.'> checked out to <'.url('/').'/admin/users/'.$user->id.'/view|'.$user->fullName().'> by <'.url('/').'/admin/users/'.$admin_user->id.'/view'.'|'.$admin_user->fullName().'>.'
+                  'value' => 'Accessory <'.route('accessories.show', $accessory->id).'|'.$accessory->name.'> checked out to <'.route('users.show', $user->id).'|'.$user->fullName().'> by <'.route('users.show', $admin_user->id).'|'.$admin_user->fullName().'>.'
                   ],
                   [
                       'title' => 'Note:',
@@ -397,12 +394,8 @@ public function getCheckin(Request $request, $accessoryUserId = null, $backto =
         }
 
         $accessory = Accessory::find($accessory_user->accessory_id);
-
-        if (!Company::isCurrentUserHasAccess($accessory)) {
-            return redirect()->route('accessories.index')->with('error', trans('general.insufficient_permissions'));
-        } else {
-            return View::make('accessories/checkin', compact('accessory'))->with('backto', $backto);
-        }
+        $this->authorize('checkin', $accessory);
+        return View::make('accessories/checkin', compact('accessory'))->with('backto', $backto);
     }
 
 
@@ -425,9 +418,7 @@ public function postCheckin(Request $request, $accessoryUserId = null, $backto =
 
         $accessory = Accessory::find($accessory_user->accessory_id);
 
-        if (!Company::isCurrentUserHasAccess($accessory)) {
-            return redirect()->route('accessories.index')->with('error', trans('general.insufficient_permissions'));
-        }
+        $this->authorize('checkin', $accessory);
 
         $return_to = e($accessory_user->assigned_to);
         $logaction = $accessory->logCheckin(User::find($return_to), e(Input::get('note')));
@@ -456,7 +447,7 @@ public function postCheckin(Request $request, $accessoryUserId = null, $backto =
                         'fields' => [
                             [
                                 'title' => 'Checked In:',
-                                'value' => class_basename(strtoupper($logaction->item_type)).' <'.url('/').'/admin/accessories/'.e($accessory->id).'/view'.'|'.e($accessory->name).'> checked in by <'.url('/').'/admin/users/'.e($admin_user->id).'/view'.'|'.e($admin_user->fullName()).'>.'
+                                'value' => class_basename(strtoupper($logaction->item_type)).' <'.route('accessories.show', $accessory->id).'|'.e($accessory->name).'> checked in by <'.route('users.show', $admin_user->id).'|'.e($admin_user->fullName()).'>.'
                             ],
                             [
                                 'title' => 'Note:',
@@ -493,9 +484,9 @@ public function postCheckin(Request $request, $accessoryUserId = null, $backto =
             }
 
             if ($backto=='user') {
-                return redirect()->to("admin/users/".$return_to.'/view')->with('success', trans('admin/accessories/message.checkin.success'));
+                return redirect()->route("users.show", $return_to)->with('success', trans('admin/accessories/message.checkin.success'));
             } else {
-                return redirect()->to("admin/accessories/".$accessory->id."/view")->with('success', trans('admin/accessories/message.checkin.success'));
+                return redirect()->route("accessories.show", $accessory->id)->with('success', trans('admin/accessories/message.checkin.success'));
             }
         }
 
@@ -532,6 +523,7 @@ public function postCheckin(Request $request, $accessoryUserId = null, $backto =
     **/
     public function getDatatable(Request $request)
     {
+        $this->authorize('index', Accessory::class);
         $accessories = Company::scopeCompanyables(
             Accessory::select('accessories.*')
             ->whereNull('accessories.deleted_at')
@@ -578,23 +570,23 @@ public function getDatatable(Request $request)
         foreach ($accessories as $accessory) {
 
             $actions = '<nobr>';
-            if (Gate::allows('accessories.checkout')) {
+            if (Gate::allows('checkout', $accessory)) {
                 $actions .= '<a href="' . route('checkout/accessory',
                         $accessory->id) . '" style="margin-right:5px;" class="btn btn-info btn-sm" ' . (($accessory->numRemaining() > 0) ? '' : ' disabled') . '>' . trans('general.checkout') . '</a>';
             }
-            if (Gate::allows('accessories.edit')) {
+            if (Gate::allows('update', $accessory)) {
                 $actions .= '<a href="' . route('accessories.update',
                         $accessory->id) . '" class="btn btn-warning btn-sm" style="margin-right:5px;"><i class="fa fa-pencil icon-white"></i></a>';
             }
-            if (Gate::allows('accessories.delete')) {
+            if (Gate::allows('delete', $accessory)) {
                 $actions .= '<a data-html="false" class="btn delete-asset btn-danger btn-sm" data-toggle="modal" href="' . route('accessories.destroy',
                         $accessory->id) . '" data-content="' . trans('admin/accessories/message.delete.confirm') . '" data-title="' . trans('general.delete') . ' ' . htmlspecialchars($accessory->name) . '?" onClick="return false;"><i class="fa fa-trash icon-white"></i></a>';
             }
             $actions .= '</nobr>';
             $company = $accessory->company;
 
             $rows[] = array(
-            'name'          => '<a href="'.url('admin/accessories/'.$accessory->id).'/view">'. $accessory->name.'</a>',
+            'name'          => '<a href="'.route('accessories.show',$accessory->id).'">'. $accessory->name.'</a>',
             'category'      => ($accessory->category) ? (string)link_to('admin/settings/categories/'.$accessory->category->id.'/view', $accessory->category->name) : '',
             'model_number'      =>  e($accessory->model_number),
             'qty'           => e($accessory->qty),
@@ -606,7 +598,7 @@ public function getDatatable(Request $request)
             'numRemaining'  => $accessory->numRemaining(),
             'actions'       => $actions,
             'companyName'   => is_null($company) ? '' : e($company->name),
-            'manufacturer'      => $accessory->manufacturer ? (string) link_to('/admin/settings/manufacturers/'.$accessory->manufacturer_id.'/view', $accessory->manufacturer->name) : ''
+            'manufacturer'      => $accessory->manufacturer ? (string) link_to(route('manufacturers.show', $accessory->manufacturer_id), $accessory->manufacturer->name) : ''
 
             );
         }
@@ -657,13 +649,13 @@ public function getDataView(Request $request, $accessoryID)
 
         foreach ($accessory_users as $user) {
             $actions = '';
-            if (Gate::allows('accessories.checkin')) {
+            if (Gate::allows('checkin', $accessory)) {
                 $actions .= '<a href="' . route('checkin/accessory',
                         $user->pivot->id) . '" class="btn btn-info btn-sm">Checkin</a>';
             }
 
-            if (Gate::allows('users.view')) {
-                $name = (string) link_to('/admin/users/'.$user->id.'/view', e($user->fullName()));
+            if (Gate::allows('view', $user)) {
+                $name = (string) link_to_route('users.show', e($user->fullName()), [$user->id]);
             } else {
                 $name = e($user->fullName());
             }

diff --git a/app/Http/Controllers/ActionlogController.php b/app/Http/Controllers/ActionlogController.php
@@ -9,7 +9,7 @@ class ActionlogController extends Controller
 {
     public function displaySig($filename)
     {
-
+        $this->authorize('view', \App\Models\Asset::class);
         $file = config('app.private_uploads') . '/signatures/' . $filename;
         $filetype = Helper::checkUploadIsImage($file);
         $contents = file_get_contents($file);