diff --git a/app/Http/Controllers/BaseController.php b/app/Http/Controllers/BaseController.php index c7f6db9b8615..7cf98c9388ca 100644 --- a/app/Http/Controllers/BaseController.php +++ b/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); } }, diff --git a/app/Listeners/Activity/RestoreClientActivity.php b/app/Listeners/Activity/RestoreClientActivity.php index 5eed7edb9aa8..7f739585ac1c 100644 --- a/app/Listeners/Activity/RestoreClientActivity.php +++ b/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; } } diff --git a/app/Models/User.php b/app/Models/User.php index d97c4ae8aee8..d0c09fb1270e 100644 --- a/app/Models/User.php +++ b/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); - - // return $this->isOwner() || - // $this->isAdmin() || - // (is_int(stripos($this->token()->cu->permissions, $all_permission))) || - // (is_int(stripos($this->token()->cu->permissions, $permission))); + (stripos($this->token()->cu->permissions, $edit_all) !== false) || + (stripos($this->token()->cu->permissions, $edit_entity) !== false); } diff --git a/app/Observers/ClientContactObserver.php b/app/Observers/ClientContactObserver.php index c7b6c5b0b110..ecc6fd67677e 100644 --- a/app/Observers/ClientContactObserver.php +++ b/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(); } /** diff --git a/app/Observers/ClientObserver.php b/app/Observers/ClientObserver.php index dcd2d61c2bad..d5ad6b4e82a1 100644 --- a/app/Observers/ClientObserver.php +++ b/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) - { - // + } } diff --git a/app/Policies/CompanyPolicy.php b/app/Policies/CompanyPolicy.php index 5a8a1945a08e..53a3406b3555 100644 --- a/app/Policies/CompanyPolicy.php +++ b/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); } } diff --git a/app/Policies/EntityPolicy.php b/app/Policies/EntityPolicy.php index a1882e7c91b6..621f8668380a 100644 --- a/app/Policies/EntityPolicy.php +++ b/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()); } diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index cdf0b1c2d158..156cff42d096 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/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; diff --git a/app/Repositories/BaseRepository.php b/app/Repositories/BaseRepository.php index 072c8e617f42..9c3b779a65ba 100644 --- a/app/Repositories/BaseRepository.php +++ b/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)) { diff --git a/tests/Unit/PermissionsTest.php b/tests/Unit/PermissionsTest.php index 5226178ec037..6638967a23ab 100644 --- a/tests/Unit/PermissionsTest.php +++ b/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()