Refactor for permissions to include Edit permissions when testing view permissions

app/Http/Controllers/BaseController.php

@@ -859,7 +859,7 @@ class BaseController extends Controller
                 'company.bank_transactions'=> function ($query) use ($created_at, $user) {
                     $query->where('created_at', '>=', $created_at);
 
-                    if (! $user->hasPermission('bank_transactions')) {
+                    if (! $user->hasPermission('bank_transaction')) {
                         $query->where('bank_transactions.user_id', $user->id);
                     }
                 },

app/Listeners/Activity/RestoreClientActivity.php

@@ -52,6 +52,5 @@ class RestoreClientActivity implements ShouldQueue
 
         $this->activity_repo->save($fields, $event->client, $event->event_vars);
 
-        return false;
     }
 }

app/Models/User.php

@@ -367,22 +367,41 @@ class User extends Authenticatable implements MustVerifyEmail
      */
     public function hasPermission($permission) : bool
     {
-        $parts = explode('_', $permission);
-        $all_permission = '____';
 
+       /**
+        * We use the limit parameter here to ensure we don't split on permissions that have multiple underscores.
+        *
+        * For example view_recurring_invoice without the limit would split to view bank recurring invoice
+        *
+        * Using only part 0 and 1 would search for permission view_recurring / edit_recurring so this would
+        * leak permissions for other recurring_* entities
+        *
+        * The solution here will split the word - consistently - into view _ {entity} and edit _ {entity}
+        * 
+        */
+        $parts = explode('_', $permission, 2);
+        $all_permission = '____';
+        $edit_all = '____';
+        $edit_entity = '____';
+
+        /* If we have multiple parts, then make sure we search for the _all permission */
         if (count($parts) > 1) {
             $all_permission = $parts[0].'_all';
+
+            /*If this is a view search, make sure we add in the edit_{entity} AND edit_all permission into the checks*/
+            if($parts[0] == 'view') {
+                $edit_all = 'edit_all';
+                $edit_entity = "edit_{$parts[1]}";
+            }
+
         }
 
         return  $this->isOwner() ||
                 $this->isAdmin() ||
+                (stripos($this->token()->cu->permissions, $permission) !== false) ||
                 (stripos($this->token()->cu->permissions, $all_permission) !== false) ||
-                (stripos($this->token()->cu->permissions, $permission) !== false);
+                (stripos($this->token()->cu->permissions, $edit_all) !== false) ||
-
+                (stripos($this->token()->cu->permissions, $edit_entity) !== false);
-        // return  $this->isOwner() ||
-        //         $this->isAdmin() ||
-        //         (is_int(stripos($this->token()->cu->permissions, $all_permission))) ||
-        //         (is_int(stripos($this->token()->cu->permissions, $permission)));
 
     }
 

app/Observers/ClientContactObserver.php

@@ -12,6 +12,7 @@
 namespace App\Observers;
 
 use App\Models\ClientContact;
+use App\Models\CreditInvitation;
 use App\Models\InvoiceInvitation;
 use App\Models\QuoteInvitation;
 use App\Models\RecurringInvoiceInvitation;
@@ -79,7 +80,12 @@ class ClientContactObserver
 
         });
 
+        CreditInvitation::withTrashed()->where('client_contact_id', $client_contact_id)->cursor()->each(function ($invite){
+
+          if($invite->credits()->doesnthave('invitations'))
+            $invite->credit->service()->createInvitations();
         
+        });
     }
 
     /**
@@ -90,9 +96,6 @@ class ClientContactObserver
      */
     public function restored(ClientContact $clientContact)
     {
-        // $clientContact->invoice_invitations()->restore();
-        // $clientContact->quote_invitations()->restore();
-        // $clientContact->credit_invitations()->restore();
     }
 
     /**

app/Observers/ClientObserver.php

@@ -18,8 +18,6 @@ use App\Models\Webhook;
 class ClientObserver
 {
 
-    public $afterCommit = true;
-
     /**
      * Handle the client "created" event.
      *
@@ -32,9 +30,9 @@ class ClientObserver
                                     ->where('event_id', Webhook::EVENT_CREATE_CLIENT)
                                     ->exists();
 
-        if ($subscriptions) {
+        if ($subscriptions) 
             WebhookHandler::dispatch(Webhook::EVENT_CREATE_CLIENT, $client, $client->company)->delay(now()->addSeconds(rand(1,5)));
-        }
+        
     }
 
     /**
@@ -45,21 +43,23 @@ class ClientObserver
      */
     public function updated(Client $client)
     {
-
+        
-        nlog("updated event {$client->id}");
-
         $event = Webhook::EVENT_UPDATE_CLIENT;
 
+        if($client->getOriginal('deleted_at') && !$client->deleted_at)
+            $event = Webhook::EVENT_RESTORE_CLIENT;
+        
         if($client->is_deleted)
-            $event = Webhook::EVENT_DELETE_CLIENT; //this event works correctly.
+            $event = Webhook::EVENT_DELETE_CLIENT; 
+        
         
         $subscriptions = Webhook::where('company_id', $client->company->id)
                                     ->where('event_id', $event)
                                     ->exists();
 
-        if ($subscriptions) {
+        if ($subscriptions) 
             WebhookHandler::dispatch($event, $client, $client->company)->delay(now()->addSeconds(rand(1,5)));
-        }
+        
 
     }
 
@@ -73,49 +73,14 @@ class ClientObserver
     {
         if($client->is_deleted)
             return;
-
-        nlog("deleted event {$client->id}");
         
         $subscriptions = Webhook::where('company_id', $client->company->id)
                                     ->where('event_id', Webhook::EVENT_ARCHIVE_CLIENT)
                                     ->exists();
 
-        if ($subscriptions) {
+        if ($subscriptions) 
             WebhookHandler::dispatch(Webhook::EVENT_ARCHIVE_CLIENT, $client, $client->company)->delay(now()->addSeconds(rand(1,5)));
-        }
+        
-    }
-
-    /**
-     * Handle the client "restored" event.
-     *
-     * @param Client $client
-     * @return void
-     */
-    public function restored(Client $client)
-    {
-        nlog("Restored {$client->id}");
-
-        $subscriptions = Webhook::where('company_id', $client->company->id)
-                                    ->where('event_id', Webhook::EVENT_RESTORE_CLIENT)
-                                    ->exists();
-
-        if ($subscriptions) {
-            WebhookHandler::dispatch(Webhook::EVENT_RESTORE_CLIENT, $client, $client->company)->delay(now()->addSeconds(rand(1,5)));
-        }
-
-
-        return false;
-    }
-
-    /**
-     * Handle the client "force deleted" event.
-     *
-     * @param Client $client
-     * @return void
-     */
-    public function forceDeleted(Client $client)
-    {
-        //
     }
 
 }

app/Policies/CompanyPolicy.php

@@ -42,7 +42,7 @@ class CompanyPolicy extends EntityPolicy
     {
         return ($user->isAdmin() && $entity->id == $user->companyId())
             || ($user->hasPermission('view_'.strtolower(class_basename($entity))) && $entity->id == $user->companyId())
-            || ($user->hasPermission('view_all') && $entity->id == $user->companyId())
+            // || ($user->hasPermission('view_all') && $entity->id == $user->companyId())
             || $user->owns($entity);
     }
 
@@ -59,7 +59,7 @@ class CompanyPolicy extends EntityPolicy
     {
         return ($user->isAdmin() && $entity->id == $user->companyId())
             || ($user->hasPermission('edit_'.strtolower(class_basename($entity))) && $entity->id == $user->companyId())
-            || ($user->hasPermission('edit_all') && $entity->id == $user->companyId())
+            // || ($user->hasPermission('edit_all') && $entity->id == $user->companyId())
             || $user->owns($entity);
     }
 }

app/Policies/EntityPolicy.php

@@ -48,7 +48,7 @@ class EntityPolicy
     {
         return ($user->isAdmin() && $entity->company_id == $user->companyId())
             || ($user->hasPermission('edit_'.\Illuminate\Support\Str::snake(class_basename($entity))) && $entity->company_id == $user->companyId())
-            || ($user->hasPermission('edit_all') && $entity->company_id == $user->companyId())
+            // || ($user->hasPermission('edit_all') && $entity->company_id == $user->companyId()) //this is redundant as the edit_ check covers the _all check
             || ($user->owns($entity) && $entity->company_id == $user->companyId())
             || ($user->assigned($entity) && $entity->company_id == $user->companyId());
     }
@@ -65,7 +65,7 @@ class EntityPolicy
     {
         return ($user->isAdmin() && $entity->company_id == $user->companyId())
             || ($user->hasPermission('view_'.\Illuminate\Support\Str::snake(class_basename($entity))) && $entity->company_id == $user->companyId())
-            || ($user->hasPermission('view_all') && $entity->company_id == $user->companyId())
+            // || ($user->hasPermission('view_all') && $entity->company_id == $user->companyId()) //this is redundant as the edit_ check covers the _all check
             || ($user->owns($entity) && $entity->company_id == $user->companyId())
             || ($user->assigned($entity) && $entity->company_id == $user->companyId());
     }

app/Providers/AuthServiceProvider.php

@@ -12,7 +12,6 @@
 namespace App\Providers;
 
 use App\Models\Activity;
-use App\Models\Bank;
 use App\Models\BankIntegration;
 use App\Models\BankTransaction;
 use App\Models\BankTransactionRule;

app/Repositories/BaseRepository.php

@@ -83,14 +83,14 @@ class BaseRepository
 
         $fromDeleted = false;
 
-        $entity->restore();
-
         if ($entity->is_deleted) {
             $fromDeleted = true;
             $entity->is_deleted = false;
             $entity->saveQuietly();
         }
 
+        $entity->restore();
+
         $className = $this->getEventClass($entity, 'Restored');
 
         if (class_exists($className)) {

tests/Unit/PermissionsTest.php

@@ -125,6 +125,50 @@ class PermissionsTest extends TestCase
         // this is aberrant
         $this->assertFalse($this->user->hasPermission("view____client"));
 
+        $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first();
+        $low_cu->permissions = '["edit_client"]';
+        $low_cu->save();
+
+        $this->assertTrue($this->user->hasPermission("view_client"));
+
+
+        $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first();
+        $low_cu->permissions = '["edit_all"]';
+        $low_cu->save();
+
+        $this->assertTrue($this->user->hasPermission("view_client"));
+
+        $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first();
+        $low_cu->permissions = '["view_all"]';
+        $low_cu->save();
+
+        $this->assertFalse($this->user->hasPermission("edit_client"));
+
+        $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first();
+        $low_cu->permissions = '["edit_all"]';
+        $low_cu->save();
+
+        $this->assertTrue($this->user->hasPermission('view_invoice'));
+        $this->assertTrue($this->user->hasPermission('view_client'));
+        $this->assertTrue($this->user->hasPermission('view_recurring_invoice'));
+        $this->assertTrue($this->user->hasPermission('view_product'));
+        $this->assertTrue($this->user->hasPermission('view_payment'));
+        $this->assertTrue($this->user->hasPermission('view_quote'));
+        $this->assertTrue($this->user->hasPermission('view_credit'));
+        $this->assertTrue($this->user->hasPermission('view_project'));
+        $this->assertTrue($this->user->hasPermission('view_task'));
+        $this->assertTrue($this->user->hasPermission('view_vendor'));
+        $this->assertTrue($this->user->hasPermission('view_purchase_order'));
+        $this->assertTrue($this->user->hasPermission('view_expense'));
+        $this->assertTrue($this->user->hasPermission('view_bank_transaction'));
+
+
+        $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first();
+        $low_cu->permissions = '["edit_recurring_invoice"]';
+        $low_cu->save();
+
+        $this->assertTrue($this->user->hasPermission('view_recurring_invoice'));
+
     }
 
     public function testPermissionResolution()