Refactor permissions for datatables. (#2615)

* Add URL link directly to client view in list view

* Implement Form requests for all client routes

* Refactor how permissions are implemented on Datatable row action menus

* fixes for tests

app/Datatables/ClientDatatable.php

@@ -64,42 +64,47 @@ class ClientDatatable extends EntityDatatable
     /**
      * Returns the action dropdown menu 
      * 
-     * @param   $data   Std Class of client datatable rows
+     * @param   $rows   Std Class of client datatable rows
      * @return  object  Rendered action column items
      */
-    private function buildActionColumn($data)
+    private function buildActionColumn($rows)
     {
 
-        $requested_actions = [
+      $requested_actions = [
-            'view_client_client_id', 
+          'view_client_client_id', 
-            'edit_client_client_id', 
+          'edit_client_client_id', 
-            'create_task_client_id', 
+          'create_task_client_id', 
-            'create_invoice_client_id', 
+          'create_invoice_client_id', 
-            'create_payment_client_id', 
+          'create_payment_client_id', 
-            'create_credit_client_id', 
+          'create_credit_client_id', 
-            'create_expense_client_id'
+          'create_expense_client_id'
-        ];
+      ];
 
-        $actions = $this->filterActions($requested_actions, auth()->user()->permissions(), auth()->user()->isAdmin());
+      /*
+       * Build a collection of action
+       */
+      $rows = $this->processActions($requested_actions, $rows, Client::class);
 
-        $data->map(function ($row) use ($actions) {
+      /*
+       * Add a _view_ link directly to the client
+       */
+      $rows->map(function($row){
 
-            $updated_actions = $actions->map(function ($action) use($row){
+        $row->name = '<a href="' . route('clients.show', ['id' => $this->encodePrimaryKey($row->id)]) . '">' . $row->name . '</a>';
+        return $row;
 
-                $action['url'] = route($action['route'], [$action['key'] => $this->encodePrimaryKey($row->id)]);
+      });
-                return $action;
 
-            });
+      return $rows;
-
-            $row->actions = $updated_actions;
-
-            return $row;
-        });
-
-        return $data;
         
     }
 
+    /**
+     * Returns a collection of helper fields
+     * for the Client List Datatable
+     * 
+     * @return Collection collection
+     */
     public function listActions() : Collection
     {
       return collect([
@@ -116,6 +121,11 @@ class ClientDatatable extends EntityDatatable
       ]);
     }
 
+    /**
+     * Returns the Datatable settings including column visibility
+     *     
+     * @return Collection collection
+     */
     public function buildOptions() : Collection
     {
 

app/Datatables/MakesActionMenu.php

@@ -28,6 +28,64 @@ trait MakesActionMenu
 
 	}
 
+    /**
+     * To allow fine grained permissions we need to push the rows through a 
+     * permissions/actions sieve.
+     *
+     * Complicating the calculation is the fact we allow a user who has 
+     * create_entity permissions to also view/edit entities they have created.
+     *
+     * This must persist even if we later remove their create_entity permissions.
+     *
+     * The only clean way is to push each row through the sieve and push in view/edit permissions
+     * onto the users permissions array on a per-row basis. 
+     * 
+     * @param  array $requested_actions - array of requested actions for menu
+     * @param  stdClass $rows - requested $rows for datatable
+     * @param  Class::class - need so we can harvest entity string
+     * @return stdClass
+     */
+    public function processActions(array $requested_actions, $rows, $entity)
+    {
+
+        $rows->map(function ($row) use ($requested_actions, $entity){
+
+            $row->actions = $this->createActionCollection($requested_actions, $row, $entity);
+
+            return $row;
+
+        });
+
+        return $rows;
+
+    }
+
+    /**
+     * Builds the actions for a single row of a datatable
+     * 
+     * @param  array $requested_actions - array of requested actions for menu
+     * @param  stdClass $row - single $row for datatable
+     * @param  Class::class - need so we can harvest entity string
+     * @return Collection
+     */
+    private function createActionCollection($requested_actions, $row, $entity) : Collection
+    {
+        $permissions = auth()->user()->permissions();
+
+        if(auth()->user()->owns($row))
+            array_push($permissions, 'view_' . strtolower(class_basename($entity)), 'edit_' .strtolower(class_basename($entity))); 
+
+        $updated_actions = $this->filterActions($requested_actions, $permissions, auth()->user()->isAdmin())->map(function ($action) use($row){
+
+            $action['url'] = route($action['route'], [$action['key'] => $this->encodePrimaryKey($row->id)]);
+            return $action;
+
+        });
+
+        return $updated_actions;   
+
+    }
+
     /**
      * Filters the main actions collection down to the requested
      * actions for this menu
@@ -46,11 +104,12 @@ trait MakesActionMenu
 
     /**
      * Checks the user permissions against the collection and returns
-     * a Collection of available actions\.
+     * a Collection of available actions.
      * 
      * @param  Collection $actions  collection of possible actions
      * @param  bool       $isAdmin boolean defining if user is an administrator
      * @return Collection collection of filtered actions
+     * 
      */
     private function checkPermissions(Collection $actions, array $permissions, bool $is_admin) :Collection
     {

app/Http/Controllers/ClientController.php

@@ -3,7 +3,9 @@
 namespace App\Http\Controllers;
 
 use App\Datatables\ClientDatatable;
+use App\Http\Requests\Client\CreateClientRequest;
 use App\Http\Requests\Client\EditClientRequest;
+use App\Http\Requests\Client\ShowClientRequest;
 use App\Http\Requests\Client\StoreClientRequest;
 use App\Http\Requests\Client\UpdateClientRequest;
 use App\Jobs\Client\StoreClient;
@@ -57,13 +59,15 @@ class ClientController extends Controller
      *
      * @return \Illuminate\Http\Response
      */
-    public function create()
+    public function create(CreateClientRequest $request)
     {
         $client = new Client;
         $client->name = '';
         $client->company_id = $this->getCurrentCompanyId();
         $client_contact = new ClientContact;
         $client_contact->first_name = "";
+        $client_contact->user_id = auth()->user()->id;
+        $client_contact->company_id = $this->getCurrentCompanyId();
         $client_contact->id = 0;
 
         $client->contacts->add($client_contact);
@@ -106,10 +110,8 @@ class ClientController extends Controller
      * @param  int  $id
      * @return \Illuminate\Http\Response
      */
-    public function show($id)
+    public function show(ShowClientRequest $request, Client $client)
     {
-        $client = Client::find(2);
-        $client->load('contacts', 'primary_contact');
 
         return response()->json($client, 200);
     }
@@ -167,16 +169,24 @@ class ClientController extends Controller
      */
     public function bulk()
     {
+
         $action = request()->input('action');
         $ids = request()->input('ids');
 
         $clients = Client::withTrashed()->find($ids);
 
         $clients->each(function ($client, $key) use($action){
-            ActionEntity::dispatchNow($client, $action);
+
+            if(auth()->user()->can('edit', $client))
+                ActionEntity::dispatchNow($client, $action);
+
         });
-            //todo need to return the updated dataset
+
-            return response()->json('success', 200);
+        //todo need to return the updated dataset
+        return response()->json('success', 200);
+        
     }
 
+
+
 }

app/Http/Requests/Client/CreateClientRequest.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace App\Http\Requests\Client;
+
+use App\Http\Requests\Request;
+use App\Models\Client;
+
+class CreateClientRequest extends Request
+{
+    /**
+     * Determine if the user is authorized to make this request.
+     *
+     * @return bool
+     */
+
+    public function authorize() : bool
+    {
+        return $this->user()->can('create', Client::Class);
+    }
+
+}

app/Http/Requests/Client/EditClientRequest.php

@@ -3,6 +3,7 @@
 namespace App\Http\Requests\Client;
 
 use App\Http\Requests\Request;
+use App\Models\Client;
 
 class EditClientRequest extends Request
 {
@@ -14,7 +15,8 @@ class EditClientRequest extends Request
 
     public function authorize()
     {
-        return true;
+        return $this->user()->can('edit', $this->client);
+        //return true;
        // return ! auth()->user(); //todo permissions
     }
 

app/Http/Requests/Client/ShowClientRequest.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace App\Http\Requests\Client;
+
+use App\Http\Requests\Request;
+use App\Models\Client;
+
+class ShowClientRequest extends Request
+{
+    /**
+     * Determine if the user is authorized to make this request.
+     *
+     * @return bool
+     */
+
+    public function authorize() : bool
+    {
+        return $this->user()->can('view', $this->client);
+    }
+
+}

app/Http/Requests/Client/StoreClientRequest.php

@@ -3,6 +3,7 @@
 namespace App\Http\Requests\Client;
 
 use App\Http\Requests\Request;
+use App\Models\Client;
 
 class StoreClientRequest extends Request
 {
@@ -12,10 +13,9 @@ class StoreClientRequest extends Request
      * @return bool
      */
 
-    public function authorize()
+    public function authorize() : bool
     {
-        return true;
+        return $this->user()->can('create', Client::class);
-       // return ! auth()->user(); //todo permissions
     }
 
     public function rules()

app/Policies/EntityPolicy.php

@@ -38,10 +38,8 @@ class EntityPolicy
 	 */
 	public function edit(User $user, $entity) : bool
 	{
-		$entity = strtolower(class_basename($entity));
-
 			return ($user->isAdmin() && $entity->company_id == $user->company()->pivot->company_id) 
-			|| ($user->hasPermission('edit_' . $entity) && $entity->company_id == $user->company()->pivot->company_id)
+			|| ($user->hasPermission('edit_' . strtolower(class_basename($entity))) && $entity->company_id == $user->company()->pivot->company_id)
 			|| $user->owns($entity);
 	}
 
@@ -56,10 +54,8 @@ class EntityPolicy
 	 */
 	public function view(User $user, $entity) : bool
 	{
-		$entity = strtolower(class_basename($entity));
-
 			return ($user->isAdmin() && $entity->company_id == $user->company()->pivot->company_id) 
-			|| ($user->hasPermission('view_' . $entity) && $entity->company_id == $user->company()->pivot->company_id) 
+			|| ($user->hasPermission('view_' . strtolower(class_basename($entity))) && $entity->company_id == $user->company()->pivot->company_id) 
 			|| $user->owns($entity);
 	}
 

database/migrations/2014_10_13_000000_create_users_table.php

@@ -159,6 +159,8 @@ class CreateUsersTable extends Migration
 
             $table->string('custom_client_label1')->nullable();
             $table->string('custom_client_label2')->nullable();
+            $table->string('custom_client_label3')->nullable();
+            $table->string('custom_client_label4')->nullable();
 
             $table->string('custom_invoice_label1')->nullable();
             $table->string('custom_invoice_label2')->nullable();
@@ -269,6 +271,8 @@ class CreateUsersTable extends Migration
             $table->unsignedInteger('country_id')->nullable();
             $table->string('custom_value1')->nullable();
             $table->string('custom_value2')->nullable();
+            $table->string('custom_value3')->nullable();
+            $table->string('custom_value4')->nullable();
 
             $table->string('shipping_address1')->nullable();
             $table->string('shipping_address2')->nullable();

public/js/client_list.js

@@ -6502,6 +6502,9 @@ exports.default = {
         del: function () {
             this.$events.fire('bulk-action', 'delete');
         },
+        restore: function () {
+            this.$events.fire('bulk-action', 'restore');
+        },
         getBulkCount: function () {
             return this.$store.getters['client_list/getBulkCount'];
         },
@@ -8006,34 +8009,23 @@ var render = function() {
         _c(
           "button",
           {
-            staticClass: "btn btn-primary btn-lg",
+            staticClass: "btn btn-primary btn-lg dropdown-toggle",
-            attrs: { type: "button", disabled: _vm.getBulkCount() == 0 },
+            attrs: {
-            on: { click: _vm.archive }
+              type: "button",
+              disabled: _vm.getBulkCount() == 0,
+              "data-toggle": "dropdown",
+              "aria-haspopup": "true",
+              "aria-expanded": "false"
+            }
           },
           [
-            _vm._v(_vm._s(_vm.trans("texts.archive")) + " "),
+            _vm._v(_vm._s(_vm.trans("texts.action")) + " "),
             _vm.getBulkCount() > 0
               ? _c("span", [_vm._v("(" + _vm._s(_vm.getBulkCount()) + ")")])
               : _vm._e()
           ]
         ),
         _vm._v(" "),
-        _c(
-          "button",
-          {
-            staticClass:
-              "btn btn-primary dropdown-toggle dropdown-toggle-split",
-            attrs: {
-              type: "button",
-              "data-toggle": "dropdown",
-              "aria-haspopup": "true",
-              "aria-expanded": "false",
-              disabled: _vm.getBulkCount() == 0
-            }
-          },
-          [_c("span", { staticClass: "sr-only" }, [_vm._v("Toggle Dropdown")])]
-        ),
-        _vm._v(" "),
         _c(
           "div",
           {
@@ -8043,7 +8035,7 @@ var render = function() {
               "will-change": "transform",
               top: "0px",
               left: "0px",
-              transform: "translate3d(81px, 38px, 0px)"
+              transform: "translate3d(0px, 44px, 0px)"
             },
             attrs: { "x-placement": "bottom-start" }
           },
@@ -8058,6 +8050,16 @@ var render = function() {
               [_vm._v(_vm._s(_vm.trans("texts.archive")))]
             ),
             _vm._v(" "),
+            _c(
+              "a",
+              {
+                staticClass: "dropdown-item",
+                attrs: { href: "#" },
+                on: { click: _vm.restore }
+              },
+              [_vm._v(_vm._s(_vm.trans("texts.restore")))]
+            ),
+            _vm._v(" "),
             _c(
               "a",
               {

public/js/client_list.min.js

@@ -6502,6 +6502,9 @@ exports.default = {
         del: function () {
             this.$events.fire('bulk-action', 'delete');
         },
+        restore: function () {
+            this.$events.fire('bulk-action', 'restore');
+        },
         getBulkCount: function () {
             return this.$store.getters['client_list/getBulkCount'];
         },
@@ -8006,34 +8009,23 @@ var render = function() {
         _c(
           "button",
           {
-            staticClass: "btn btn-primary btn-lg",
+            staticClass: "btn btn-primary btn-lg dropdown-toggle",
-            attrs: { type: "button", disabled: _vm.getBulkCount() == 0 },
+            attrs: {
-            on: { click: _vm.archive }
+              type: "button",
+              disabled: _vm.getBulkCount() == 0,
+              "data-toggle": "dropdown",
+              "aria-haspopup": "true",
+              "aria-expanded": "false"
+            }
           },
           [
-            _vm._v(_vm._s(_vm.trans("texts.archive")) + " "),
+            _vm._v(_vm._s(_vm.trans("texts.action")) + " "),
             _vm.getBulkCount() > 0
               ? _c("span", [_vm._v("(" + _vm._s(_vm.getBulkCount()) + ")")])
               : _vm._e()
           ]
         ),
         _vm._v(" "),
-        _c(
-          "button",
-          {
-            staticClass:
-              "btn btn-primary dropdown-toggle dropdown-toggle-split",
-            attrs: {
-              type: "button",
-              "data-toggle": "dropdown",
-              "aria-haspopup": "true",
-              "aria-expanded": "false",
-              disabled: _vm.getBulkCount() == 0
-            }
-          },
-          [_c("span", { staticClass: "sr-only" }, [_vm._v("Toggle Dropdown")])]
-        ),
-        _vm._v(" "),
         _c(
           "div",
           {
@@ -8043,7 +8035,7 @@ var render = function() {
               "will-change": "transform",
               top: "0px",
               left: "0px",
-              transform: "translate3d(81px, 38px, 0px)"
+              transform: "translate3d(0px, 44px, 0px)"
             },
             attrs: { "x-placement": "bottom-start" }
           },
@@ -8058,6 +8050,16 @@ var render = function() {
               [_vm._v(_vm._s(_vm.trans("texts.archive")))]
             ),
             _vm._v(" "),
+            _c(
+              "a",
+              {
+                staticClass: "dropdown-item",
+                attrs: { href: "#" },
+                on: { click: _vm.restore }
+              },
+              [_vm._v(_vm._s(_vm.trans("texts.restore")))]
+            ),
+            _vm._v(" "),
             _c(
               "a",
               {

resources/js/src/components/util/VueListActions.vue

@@ -4,16 +4,14 @@
 
     <div class="p-2">
       
-    		<div class="btn-group">
+      <div class="btn-group">
-          <button type="button" class="btn btn-primary btn-lg" @click="archive" :disabled="getBulkCount() == 0">{{ trans('texts.archive') }} <span v-if="getBulkCount() > 0">({{ getBulkCount() }})</span></button>
+        <button class="btn btn-primary btn-lg dropdown-toggle" type="button" :disabled="getBulkCount() == 0" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">{{ trans('texts.action') }} <span v-if="getBulkCount() > 0">({{ getBulkCount() }})</span></button>
-          <button type="button" class="btn btn-primary dropdown-toggle dropdown-toggle-split" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false" :disabled="getBulkCount() == 0">
+          <div class="dropdown-menu" x-placement="bottom-start" style="position: absolute; will-change: transform; top: 0px; left: 0px; transform: translate3d(0px, 44px, 0px);">
-            <span class="sr-only">Toggle Dropdown</span>
-          </button>
-          <div class="dropdown-menu" x-placement="bottom-start" style="position: absolute; will-change: transform; top: 0px; left: 0px; transform: translate3d(81px, 38px, 0px);">
             <a class="dropdown-item" @click="archive" href="#">{{ trans('texts.archive') }}</a>
+            <a class="dropdown-item" @click="restore" href="#">{{ trans('texts.restore') }}</a>
             <a class="dropdown-item" @click="del" href="#">{{ trans('texts.delete') }}</a>
           </div>
-        </div>	
+      </div>
             
     </div>
 
@@ -81,6 +79,11 @@
         this.$events.fire('bulk-action', 'delete')
 
       },
+      restore() {
+
+        this.$events.fire('bulk-action', 'restore')
+
+      }
       getBulkCount() {
 
         return this.$store.getters['client_list/getBulkCount']

tests/Unit/DefaultSettingsTest.php

@@ -23,7 +23,7 @@ class DefaultTest extends TestCase
 	{
 		$user_settings = DefaultSettings::userSettings();
 		
-		$this->assertEquals($user_settings->Client->datatable->per_page, 20);
+		$this->assertEquals($user_settings->Client->datatable->per_page, 25);
 	}
 
 	public function testIsObject()