diff --git a/app/Http/Controllers/V1/Admin/Company/CompaniesController.php b/app/Http/Controllers/V1/Admin/Company/CompaniesController.php index 05163008..77964d5d 100644 --- a/app/Http/Controllers/V1/Admin/Company/CompaniesController.php +++ b/app/Http/Controllers/V1/Admin/Company/CompaniesController.php @@ -61,10 +61,10 @@ class CompaniesController extends Controller $company = Company::find($request->header('company')); $this->authorize('transfer company ownership', $company); - if ($user->hasCompany($company->id)) { + if (! $user->hasCompany($company->id)) { return response()->json([ 'success' => false, - 'message' => 'User does not belongs to this company.', + 'message' => 'User does not belong to this company.', ]); } diff --git a/app/Http/Controllers/V1/Admin/Customer/CustomersController.php b/app/Http/Controllers/V1/Admin/Customer/CustomersController.php index d0a73186..3dbe5846 100644 --- a/app/Http/Controllers/V1/Admin/Customer/CustomersController.php +++ b/app/Http/Controllers/V1/Admin/Customer/CustomersController.php @@ -92,7 +92,11 @@ class CustomersController extends Controller { $this->authorize('delete multiple customers'); - Customer::deleteCustomers($request->ids); + $ids = Customer::whereCompany() + ->whereIn('id', $request->ids) + ->pluck('id'); + + Customer::deleteCustomers($ids); return response()->json([ 'success' => true, diff --git a/app/Http/Controllers/V1/Admin/Estimate/CloneEstimateController.php b/app/Http/Controllers/V1/Admin/Estimate/CloneEstimateController.php index 546b4fcf..64a57f64 100644 --- a/app/Http/Controllers/V1/Admin/Estimate/CloneEstimateController.php +++ b/app/Http/Controllers/V1/Admin/Estimate/CloneEstimateController.php @@ -21,6 +21,7 @@ class CloneEstimateController extends Controller */ public function __invoke(Request $request, Estimate $estimate) { + $this->authorize('view', $estimate); $this->authorize('create', Estimate::class); $date = Carbon::now(); diff --git a/app/Http/Controllers/V1/Admin/Estimate/EstimatesController.php b/app/Http/Controllers/V1/Admin/Estimate/EstimatesController.php index 31a415da..fabd2a17 100644 --- a/app/Http/Controllers/V1/Admin/Estimate/EstimatesController.php +++ b/app/Http/Controllers/V1/Admin/Estimate/EstimatesController.php @@ -68,7 +68,11 @@ class EstimatesController extends Controller { $this->authorize('delete multiple estimates'); - Estimate::destroy($request->ids); + $ids = Estimate::whereCompany() + ->whereIn('id', $request->ids) + ->pluck('id'); + + Estimate::destroy($ids); return response()->json([ 'success' => true, diff --git a/app/Http/Controllers/V1/Admin/Expense/ExpensesController.php b/app/Http/Controllers/V1/Admin/Expense/ExpensesController.php index dd42f123..c62e8c1d 100644 --- a/app/Http/Controllers/V1/Admin/Expense/ExpensesController.php +++ b/app/Http/Controllers/V1/Admin/Expense/ExpensesController.php @@ -81,7 +81,11 @@ class ExpensesController extends Controller { $this->authorize('delete multiple expenses'); - Expense::destroy($request->ids); + $ids = Expense::whereCompany() + ->whereIn('id', $request->ids) + ->pluck('id'); + + Expense::destroy($ids); return response()->json([ 'success' => true, diff --git a/app/Http/Controllers/V1/Admin/General/SearchController.php b/app/Http/Controllers/V1/Admin/General/SearchController.php index f5179e44..a81b5c89 100644 --- a/app/Http/Controllers/V1/Admin/General/SearchController.php +++ b/app/Http/Controllers/V1/Admin/General/SearchController.php @@ -25,7 +25,8 @@ class SearchController extends Controller ->paginate(10); if ($user->isOwner()) { - $users = User::applyFilters($request->only(['search'])) + $users = User::whereCompany() + ->applyFilters($request->only(['search'])) ->latest() ->paginate(10); } diff --git a/app/Http/Controllers/V1/Admin/Invoice/CloneInvoiceController.php b/app/Http/Controllers/V1/Admin/Invoice/CloneInvoiceController.php index 77596d64..e3775781 100644 --- a/app/Http/Controllers/V1/Admin/Invoice/CloneInvoiceController.php +++ b/app/Http/Controllers/V1/Admin/Invoice/CloneInvoiceController.php @@ -21,6 +21,7 @@ class CloneInvoiceController extends Controller */ public function __invoke(Request $request, Invoice $invoice) { + $this->authorize('view', $invoice); $this->authorize('create', Invoice::class); $date = Carbon::now(); diff --git a/app/Http/Controllers/V1/Admin/Invoice/InvoicesController.php b/app/Http/Controllers/V1/Admin/Invoice/InvoicesController.php index 9c3c1629..630d4736 100644 --- a/app/Http/Controllers/V1/Admin/Invoice/InvoicesController.php +++ b/app/Http/Controllers/V1/Admin/Invoice/InvoicesController.php @@ -100,7 +100,11 @@ class InvoicesController extends Controller { $this->authorize('delete multiple invoices'); - Invoice::deleteInvoices($request->ids); + $ids = Invoice::whereCompany() + ->whereIn('id', $request->ids) + ->pluck('id'); + + Invoice::deleteInvoices($ids); return response()->json([ 'success' => true, diff --git a/app/Http/Controllers/V1/Admin/Item/ItemsController.php b/app/Http/Controllers/V1/Admin/Item/ItemsController.php index 93e21bec..7c63a61a 100644 --- a/app/Http/Controllers/V1/Admin/Item/ItemsController.php +++ b/app/Http/Controllers/V1/Admin/Item/ItemsController.php @@ -90,7 +90,11 @@ class ItemsController extends Controller { $this->authorize('delete multiple items'); - Item::destroy($request->ids); + $ids = Item::whereCompany() + ->whereIn('id', $request->ids) + ->pluck('id'); + + Item::destroy($ids); return response()->json([ 'success' => true, diff --git a/app/Http/Controllers/V1/Admin/Payment/PaymentsController.php b/app/Http/Controllers/V1/Admin/Payment/PaymentsController.php index 6f7c2dc3..ee6d0ff7 100644 --- a/app/Http/Controllers/V1/Admin/Payment/PaymentsController.php +++ b/app/Http/Controllers/V1/Admin/Payment/PaymentsController.php @@ -73,7 +73,11 @@ class PaymentsController extends Controller { $this->authorize('delete multiple payments'); - Payment::deletePayments($request->ids); + $ids = Payment::whereCompany() + ->whereIn('id', $request->ids) + ->pluck('id'); + + Payment::deletePayments($ids); return response()->json([ 'success' => true, diff --git a/app/Http/Controllers/V1/Admin/RecurringInvoice/RecurringInvoiceController.php b/app/Http/Controllers/V1/Admin/RecurringInvoice/RecurringInvoiceController.php index b859392c..e3becd7b 100644 --- a/app/Http/Controllers/V1/Admin/RecurringInvoice/RecurringInvoiceController.php +++ b/app/Http/Controllers/V1/Admin/RecurringInvoice/RecurringInvoiceController.php @@ -84,7 +84,11 @@ class RecurringInvoiceController extends Controller { $this->authorize('delete multiple recurring invoices'); - RecurringInvoice::deleteRecurringInvoice($request->ids); + $ids = RecurringInvoice::whereCompany() + ->whereIn('id', $request->ids) + ->pluck('id'); + + RecurringInvoice::deleteRecurringInvoice($ids); return response()->json([ 'success' => true, diff --git a/app/Http/Controllers/V1/Admin/Users/UsersController.php b/app/Http/Controllers/V1/Admin/Users/UsersController.php index bf0df019..03eb3628 100644 --- a/app/Http/Controllers/V1/Admin/Users/UsersController.php +++ b/app/Http/Controllers/V1/Admin/Users/UsersController.php @@ -25,14 +25,15 @@ class UsersController extends Controller $user = $request->user(); - $users = User::applyFilters($request->all()) + $users = User::whereCompany() + ->applyFilters($request->all()) ->where('id', '<>', $user->id) ->latest() ->paginate($limit); return UserResource::collection($users) ->additional(['meta' => [ - 'user_total_count' => User::count(), + 'user_total_count' => User::whereCompany()->count(), ]]); } diff --git a/app/Models/User.php b/app/Models/User.php index 62484a2c..7367cb4d 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -218,6 +218,13 @@ class User extends Authenticatable implements HasMedia return $query->where('email', 'LIKE', '%'.$email.'%'); } + public function scopeWhereCompany($query) + { + return $query->whereHas('companies', function ($q) { + $q->where('company_id', request()->header('company')); + }); + } + public function scopePaginateData($query, $limit) { if ($limit == 'all') { diff --git a/app/Policies/CustomerPolicy.php b/app/Policies/CustomerPolicy.php index aa2feccb..02ad2fa3 100644 --- a/app/Policies/CustomerPolicy.php +++ b/app/Policies/CustomerPolicy.php @@ -32,7 +32,7 @@ class CustomerPolicy */ public function view(User $user, Customer $customer): bool { - if (BouncerFacade::can('view-customer', $customer)) { + if (BouncerFacade::can('view-customer', $customer) && $user->hasCompany($customer->company_id)) { return true; } @@ -60,7 +60,7 @@ class CustomerPolicy */ public function update(User $user, Customer $customer): bool { - if (BouncerFacade::can('edit-customer', $customer)) { + if (BouncerFacade::can('edit-customer', $customer) && $user->hasCompany($customer->company_id)) { return true; } @@ -74,7 +74,7 @@ class CustomerPolicy */ public function delete(User $user, Customer $customer): bool { - if (BouncerFacade::can('delete-customer', $customer)) { + if (BouncerFacade::can('delete-customer', $customer) && $user->hasCompany($customer->company_id)) { return true; } @@ -88,7 +88,7 @@ class CustomerPolicy */ public function restore(User $user, Customer $customer): bool { - if (BouncerFacade::can('delete-customer', $customer)) { + if (BouncerFacade::can('delete-customer', $customer) && $user->hasCompany($customer->company_id)) { return true; } @@ -102,7 +102,7 @@ class CustomerPolicy */ public function forceDelete(User $user, Customer $customer): bool { - if (BouncerFacade::can('delete-customer', $customer)) { + if (BouncerFacade::can('delete-customer', $customer) && $user->hasCompany($customer->company_id)) { return true; } diff --git a/tests/Feature/Admin/CustomerTest.php b/tests/Feature/Admin/CustomerTest.php index 33f3ec09..731a4e34 100644 --- a/tests/Feature/Admin/CustomerTest.php +++ b/tests/Feature/Admin/CustomerTest.php @@ -2,6 +2,7 @@ use App\Http\Controllers\V1\Admin\Customer\CustomersController; use App\Http\Requests\CustomerRequest; +use App\Models\Company; use App\Models\Customer; use App\Models\Invoice; use App\Models\User; @@ -157,3 +158,40 @@ test('delete multiple customer', function () { 'success' => true, ]); }); + +test('cannot view customer from another company', function () { + $otherCompany = Company::factory()->create(); + $otherCustomer = Customer::factory()->create([ + 'company_id' => $otherCompany->id, + ]); + + getJson("api/v1/customers/{$otherCustomer->id}") + ->assertForbidden(); +}); + +test('cannot update customer from another company', function () { + $otherCompany = Company::factory()->create(); + $otherCustomer = Customer::factory()->create([ + 'company_id' => $otherCompany->id, + ]); + + putJson("api/v1/customers/{$otherCustomer->id}", [ + 'name' => 'Hacked Name', + 'email' => 'hacked@example.com', + ])->assertForbidden(); +}); + +test('cannot bulk delete customer from another company', function () { + $otherCompany = Company::factory()->create(); + $otherCustomer = Customer::factory()->create([ + 'company_id' => $otherCompany->id, + ]); + + postJson('api/v1/customers/delete', [ + 'ids' => [$otherCustomer->id], + ])->assertOk(); + + $this->assertDatabaseHas('customers', [ + 'id' => $otherCustomer->id, + ]); +});