From 883c8f22893ef95d81eaadca4128525569553112 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Fri, 20 Jan 2023 23:45:29 +1100 Subject: [PATCH 01/13] Tests for refactors of API permissions --- app/Http/Controllers/BaseController.php | 18 ++- tests/Feature/BaseApiTest.php | 178 ++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 tests/Feature/BaseApiTest.php diff --git a/app/Http/Controllers/BaseController.php b/app/Http/Controllers/BaseController.php index 3403d0e2128c..5925ae8b3078 100644 --- a/app/Http/Controllers/BaseController.php +++ b/app/Http/Controllers/BaseController.php @@ -12,9 +12,20 @@ namespace App\Http\Controllers; use App\Models\Account; +use App\Models\BankIntegration; use App\Models\BankTransaction; +use App\Models\BankTransactionRule; +use App\Models\ClientGatewayToken; use App\Models\Company; +use App\Models\CompanyGateway; +use App\Models\Design; +use App\Models\ExpenseCategory; +use App\Models\GroupSetting; +use App\Models\PaymentTerm; +use App\Models\Scheduler; +use App\Models\TaxRate; use App\Models\User; +use App\Models\Webhook; use App\Transformers\ArraySerializer; use App\Transformers\EntityTransformer; use App\Utils\Ninja; @@ -858,12 +869,13 @@ class BaseController extends Controller // 28-03-2022 this is definitely correct here, do not append _ to the view, it resolved correctly when snake cased if (auth()->user() && ! auth()->user()->hasPermission('view'.lcfirst(class_basename(Str::snake($this->entity_type))))) { //06-10-2022 - some entities do not have assigned_user_id - this becomes an issue when we have a large company and low permission users - if(lcfirst(class_basename(Str::snake($this->entity_type))) == 'user') + if(in_array($this->entity_type, [User::class])){ $query->where('id', auth()->user()->id); - elseif($this->entity_type == BankTransaction::class){ //table without assigned_user_id + } + elseif(in_array($this->entity_type, [BankTransactionRule::class,CompanyGateway::class, TaxRate::class, BankIntegration::class, Scheduler::class, BankTransaction::class, Webhook::class, ExpenseCategory::class])){ //table without assigned_user_id $query->where('user_id', '=', auth()->user()->id); } - elseif(in_array(lcfirst(class_basename(Str::snake($this->entity_type))),['design','group_setting','payment_term'])){ + elseif(in_array($this->entity_type,[ ClientGatewayToken::class,Design::class,GroupSetting::class,PaymentTerm::class])){ //need to pass these back regardless nlog($this->entity_type); } diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php new file mode 100644 index 000000000000..725d444c02c1 --- /dev/null +++ b/tests/Feature/BaseApiTest.php @@ -0,0 +1,178 @@ +makeTestData(); + + $this->withoutMiddleware( + ThrottleRequests::class + ); + + $lower_permission_user = User::factory()->create([ + 'account_id' => $this->account->id, + 'confirmation_code' => $this->createDbHash(config('database.default')), + 'email' => $this->faker->safeEmail(), + ]); + + $this->low_cu = CompanyUserFactory::create($lower_permission_user->id, $this->company->id, $this->account->id); + $this->low_cu->is_owner = false; + $this->low_cu->is_admin = false; + $this->low_cu->is_locked = false; + $this->low_cu->permissions = '["view_task"]'; + $this->low_cu->save(); + + $this->low_token = \Illuminate\Support\Str::random(64); + + $company_token = new CompanyToken; + $company_token->user_id = $lower_permission_user->id; + $company_token->company_id = $this->company->id; + $company_token->account_id = $this->account->id; + $company_token->name = 'test token'; + $company_token->token = $this->low_token; + $company_token->is_system = true; + $company_token->save(); + + } + + // public function testGeneratingClassName() + // { + + // $this->assertEquals('user', Str::snake(User::class)); + + // $this->assertEquals('user',lcfirst(class_basename(Str::snake(User::class)))); + + + // } + + public function testRestrictedRoute() + { + // $permissions = ["view_invoice","view_client","edit_client","edit_invoice","create_invoice","create_client"]; + + // $response = $this->withHeaders([ + // 'X-API-SECRET' => config('ninja.api_secret'), + // 'X-API-TOKEN' => $this->token, + // ])->get('/api/v1/clients/') + // ->assertStatus(200) + // ->assertJson(fn (AssertableJson $json) => $json->has('data',1)->etc()); + + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->get('/api/v1/tasks/') + ->assertStatus(200) + ->assertJson(fn (AssertableJson $json) => $json->has('data',1)->etc()); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->get('/api/v1/group_settings/') + ->assertStatus(200) + ->assertJson(fn (AssertableJson $json) => $json->has('data',2)->etc()); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->get('/api/v1/designs/') + ->assertStatus(200) + ->assertJson(fn (AssertableJson $json) => $json->has('data',11)->etc()); + + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->low_token, + ])->get('/api/v1/users/'); + + + $response->assertStatus(200) + ->assertJson(fn (AssertableJson $json) => $json->has('data',1)->etc()); + + + collect($this->list_routes)->filter(function ($route){ + return !in_array($route, ['tasks','users','group_settings','designs']); + })->each(function($route){ + nlog($route); + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->low_token, + ])->get("/api/v1/{$route}/") + ->assertJson(fn (AssertableJson $json) => + $json->has('meta') + ->has('data',0) + ); + + }); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->low_token, + ])->get('/api/v1/companies/'.$this->company->hashed_id) + ->assertStatus(401); + + + + } +} From ca225846c2a178e3d5f37a8f4c464b8834a9b537 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 08:09:39 +1100 Subject: [PATCH 02/13] base api testS --- app/Providers/AppServiceProvider.php | 1 - tests/Feature/BaseApiTest.php | 35 +++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 14a1c388c3d8..97174d02c2d8 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -52,7 +52,6 @@ class AppServiceProvider extends ServiceProvider // ); // }); - Relation::morphMap([ 'invoices' => Invoice::class, 'proposals' => Proposal::class, diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php index 725d444c02c1..d4cb5a17922e 100644 --- a/tests/Feature/BaseApiTest.php +++ b/tests/Feature/BaseApiTest.php @@ -14,6 +14,7 @@ namespace Tests\Feature; use App\Factory\CompanyUserFactory; use App\Models\CompanyToken; use App\Models\CompanyUser; +use App\Models\Product; use App\Models\User; use Illuminate\Routing\Middleware\ThrottleRequests; use Illuminate\Support\Str; @@ -106,8 +107,40 @@ class BaseApiTest extends TestCase // } + + public function testOwnerRoutes() + { - public function testRestrictedRoute() + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->get('/api/v1/users/'); + + $response->assertStatus(200) + ->assertJson(fn (AssertableJson $json) => $json->has('data',2)->etc()); + + /*does not test the number of records however*/ + collect($this->list_routes)->each(function($route){ + nlog($route); + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->low_token, + ])->get("/api/v1/{$route}/") + ->assertJson(fn (AssertableJson $json) => + $json->has('meta') + ->has('data') + ); + }); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->low_token, + ])->get('/api/v1/companies/'.$this->company->hashed_id) + ->assertStatus(401); + + } + + public function testRestrictedUserRoute() { // $permissions = ["view_invoice","view_client","edit_client","edit_invoice","create_invoice","create_client"]; From 810e997dfce09d3ca2731e1bfbb6eb6d7f14ac1e Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 09:18:09 +1100 Subject: [PATCH 03/13] Remove logging --- tests/Feature/BaseApiTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php index d4cb5a17922e..03a1425814c1 100644 --- a/tests/Feature/BaseApiTest.php +++ b/tests/Feature/BaseApiTest.php @@ -187,7 +187,7 @@ class BaseApiTest extends TestCase collect($this->list_routes)->filter(function ($route){ return !in_array($route, ['tasks','users','group_settings','designs']); })->each(function($route){ - nlog($route); + // nlog($route); $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->low_token, From 805b0f4674050ab2c4e3ddf94bb59eda788f58b9 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 09:20:39 +1100 Subject: [PATCH 04/13] Checks for settings in template --- resources/views/email/template/client.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/views/email/template/client.blade.php b/resources/views/email/template/client.blade.php index e0c07b128710..2e3a20080956 100644 --- a/resources/views/email/template/client.blade.php +++ b/resources/views/email/template/client.blade.php @@ -1,6 +1,6 @@ @php $primary_color = isset($settings) ? $settings->primary_color : '#4caf50'; - $email_alignment = isset($settings) ? $settings->email_alignment : 'center'; + $email_alignment = isset($settings->email_alignment) ? $settings->email_alignment : 'center'; @endphp From f7281e4310f34299f73412ed2f96e5fe25d82590 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 09:23:56 +1100 Subject: [PATCH 05/13] ListResponse permissions --- app/Http/Controllers/BaseController.php | 29 ++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/BaseController.php b/app/Http/Controllers/BaseController.php index 5925ae8b3078..2a5772c0bd8b 100644 --- a/app/Http/Controllers/BaseController.php +++ b/app/Http/Controllers/BaseController.php @@ -865,6 +865,29 @@ class BaseController extends Controller $query->with($includes); + + + /*Restore here if refactor produces unexpected edge cases*/ +/* + if (auth()->user() && ! auth()->user()->hasPermission('view'.lcfirst(class_basename(Str::snake($this->entity_type))))) { + //06-10-2022 - some entities do not have assigned_user_id - this becomes an issue when we have a large company and low permission users + if(lcfirst(class_basename(Str::snake($this->entity_type))) == 'user') + $query->where('id', auth()->user()->id); + elseif($this->entity_type == BankTransaction::class){ //table without assigned_user_id + $query->where('user_id', '=', auth()->user()->id); + } + elseif(in_array(lcfirst(class_basename(Str::snake($this->entity_type))),['design','group_setting','payment_term'])){ + //need to pass these back regardless + nlog($this->entity_type); + } + else + $query->where('user_id', '=', auth()->user()->id)->orWhere('assigned_user_id', auth()->user()->id); + + } +*/ + + +/**/ // 10-01-2022 need to ensure we snake case properly here to ensure permissions work as expected // 28-03-2022 this is definitely correct here, do not append _ to the view, it resolved correctly when snake cased if (auth()->user() && ! auth()->user()->hasPermission('view'.lcfirst(class_basename(Str::snake($this->entity_type))))) { @@ -875,7 +898,7 @@ class BaseController extends Controller elseif(in_array($this->entity_type, [BankTransactionRule::class,CompanyGateway::class, TaxRate::class, BankIntegration::class, Scheduler::class, BankTransaction::class, Webhook::class, ExpenseCategory::class])){ //table without assigned_user_id $query->where('user_id', '=', auth()->user()->id); } - elseif(in_array($this->entity_type,[ ClientGatewayToken::class,Design::class,GroupSetting::class,PaymentTerm::class])){ + elseif(in_array($this->entity_type,[ClientGatewayToken::class,Design::class,GroupSetting::class,PaymentTerm::class])){ //need to pass these back regardless nlog($this->entity_type); } @@ -883,6 +906,10 @@ class BaseController extends Controller $query->where('user_id', '=', auth()->user()->id)->orWhere('assigned_user_id', auth()->user()->id); } +/**/ + + + if (request()->has('updated_at') && request()->input('updated_at') > 0) { $query->where('updated_at', '>=', date('Y-m-d H:i:s', intval(request()->input('updated_at')))); From fc5d6a99fe0f7704e041f065a51882a2db28d79c Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 09:59:00 +1100 Subject: [PATCH 06/13] add guard to client gateway tokens --- app/Http/Controllers/BaseController.php | 3 +- .../ClientGatewayTokenController.php | 3 +- .../ListClientGatewayTokenRequest.php | 28 ++ tests/Feature/BaseApiTest.php | 314 +++++++++++++++++- 4 files changed, 338 insertions(+), 10 deletions(-) create mode 100644 app/Http/Requests/ClientGatewayToken/ListClientGatewayTokenRequest.php diff --git a/app/Http/Controllers/BaseController.php b/app/Http/Controllers/BaseController.php index 2a5772c0bd8b..3d687333ce81 100644 --- a/app/Http/Controllers/BaseController.php +++ b/app/Http/Controllers/BaseController.php @@ -898,8 +898,7 @@ class BaseController extends Controller elseif(in_array($this->entity_type, [BankTransactionRule::class,CompanyGateway::class, TaxRate::class, BankIntegration::class, Scheduler::class, BankTransaction::class, Webhook::class, ExpenseCategory::class])){ //table without assigned_user_id $query->where('user_id', '=', auth()->user()->id); } - elseif(in_array($this->entity_type,[ClientGatewayToken::class,Design::class,GroupSetting::class,PaymentTerm::class])){ - //need to pass these back regardless + elseif(in_array($this->entity_type,[Design::class, GroupSetting::class, PaymentTerm::class])){ nlog($this->entity_type); } else diff --git a/app/Http/Controllers/ClientGatewayTokenController.php b/app/Http/Controllers/ClientGatewayTokenController.php index 44912c5647a4..84b918c09a49 100644 --- a/app/Http/Controllers/ClientGatewayTokenController.php +++ b/app/Http/Controllers/ClientGatewayTokenController.php @@ -18,6 +18,7 @@ use App\Filters\ClientGatewayTokenFilters; use App\Http\Requests\ClientGatewayToken\CreateClientGatewayTokenRequest; use App\Http\Requests\ClientGatewayToken\DestroyClientGatewayTokenRequest; use App\Http\Requests\ClientGatewayToken\EditClientGatewayTokenRequest; +use App\Http\Requests\ClientGatewayToken\ListClientGatewayTokenRequest; use App\Http\Requests\ClientGatewayToken\ShowClientGatewayTokenRequest; use App\Http\Requests\ClientGatewayToken\StoreClientGatewayTokenRequest; use App\Http\Requests\ClientGatewayToken\UpdateClientGatewayTokenRequest; @@ -103,7 +104,7 @@ class ClientGatewayTokenController extends BaseController * @param ClientGatewayTokenFilters $filters * @return Response|mixed */ - public function index(Request $request) + public function index(ListClientGatewayTokenRequest $request) { $client_gateway_token_gateway_tokens = ClientGatewayToken::scope(); diff --git a/app/Http/Requests/ClientGatewayToken/ListClientGatewayTokenRequest.php b/app/Http/Requests/ClientGatewayToken/ListClientGatewayTokenRequest.php new file mode 100644 index 000000000000..afbfe279e95c --- /dev/null +++ b/app/Http/Requests/ClientGatewayToken/ListClientGatewayTokenRequest.php @@ -0,0 +1,28 @@ +user()->isAdmin(); + } +} diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php index 03a1425814c1..0323f0f693db 100644 --- a/tests/Feature/BaseApiTest.php +++ b/tests/Feature/BaseApiTest.php @@ -11,11 +11,45 @@ namespace Tests\Feature; +use App\DataMapper\ClientRegistrationFields; +use App\DataMapper\ClientSettings; +use App\DataMapper\CompanySettings; +use App\Factory\ClientGatewayTokenFactory; use App\Factory\CompanyUserFactory; +use App\Factory\WebhookFactory; +use App\Models\BankIntegration; +use App\Models\BankTransaction; +use App\Models\BankTransactionRule; +use App\Models\Client; +use App\Models\ClientContact; +use App\Models\ClientGatewayToken; +use App\Models\Company; +use App\Models\CompanyGateway; use App\Models\CompanyToken; use App\Models\CompanyUser; +use App\Models\Credit; +use App\Models\Document; +use App\Models\Expense; +use App\Models\ExpenseCategory; +use App\Models\GroupSetting; +use App\Models\Invoice; +use App\Models\Payment; use App\Models\Product; +use App\Models\Project; +use App\Models\PurchaseOrder; +use App\Models\Quote; +use App\Models\RecurringExpense; +use App\Models\RecurringInvoice; +use App\Models\RecurringQuote; +use App\Models\Scheduler; +use App\Models\Subscription; +use App\Models\Task; +use App\Models\TaskStatus; +use App\Models\TaxRate; use App\Models\User; +use App\Models\Vendor; +use App\Models\VendorContact; +use App\Models\Webhook; use Illuminate\Routing\Middleware\ThrottleRequests; use Illuminate\Support\Str; use Illuminate\Testing\Fluent\AssertableJson; @@ -62,6 +96,10 @@ class BaseApiTest extends TestCase 'bank_transaction_rules', ]; + private string $low_token; + + private string $owner_token; + protected function setUp() :void { parent::setUp(); @@ -72,13 +110,70 @@ class BaseApiTest extends TestCase ThrottleRequests::class ); + $company = Company::factory()->create([ + 'account_id' => $this->account->id, + ]); + + $this->company = $company; + + $company->client_registration_fields = ClientRegistrationFields::generate(); + $settings = CompanySettings::defaults(); + $settings->company_logo = 'https://pdf.invoicing.co/favicon-v2.png'; + $settings->website = 'www.invoiceninja.com'; + $settings->address1 = 'Address 1'; + $settings->address2 = 'Address 2'; + $settings->city = 'City'; + $settings->state = 'State'; + $settings->postal_code = 'Postal Code'; + $settings->phone = '555-343-2323'; + $settings->email = 'test@example.com'; + $settings->country_id = '840'; + $settings->vat_number = 'vat number'; + $settings->id_number = 'id number'; + $settings->use_credits_payment = 'always'; + $settings->timezone_id = '1'; + $settings->entity_send_time = 0; + $company->track_inventory = true; + $company->settings = $settings; + $company->save(); + + $this->account->default_company_id = $company->id; + $this->account->save(); + + $owner_user = User::factory()->create([ + 'account_id' => $this->account->id, + 'confirmation_code' => $this->createDbHash(config('database.default')), + 'email' => $this->faker->safeEmail(), + ]); + + $this->owner_cu = CompanyUserFactory::create($owner_user->id, $company->id, $this->account->id); + $this->owner_cu->is_owner = true; + $this->owner_cu->is_admin = true; + $this->owner_cu->is_locked = false; + $this->owner_cu->permissions = '[]'; + $this->owner_cu->save(); + + $this->owner_token = \Illuminate\Support\Str::random(64); + + $user_id = $owner_user->id; + + $company_token = new CompanyToken; + $company_token->user_id = $owner_user->id; + $company_token->company_id = $company->id; + $company_token->account_id = $this->account->id; + $company_token->name = 'test token'; + $company_token->token = $this->owner_token; + $company_token->is_system = true; + $company_token->save(); + + $lower_permission_user = User::factory()->create([ 'account_id' => $this->account->id, 'confirmation_code' => $this->createDbHash(config('database.default')), 'email' => $this->faker->safeEmail(), ]); - $this->low_cu = CompanyUserFactory::create($lower_permission_user->id, $this->company->id, $this->account->id); + $this->low_cu = CompanyUserFactory::create($lower_permission_user->id, $company->id, $this->account->id); $this->low_cu->is_owner = false; $this->low_cu->is_admin = false; $this->low_cu->is_locked = false; @@ -96,6 +191,204 @@ class BaseApiTest extends TestCase $company_token->is_system = true; $company_token->save(); + + + Product::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + $client = Client::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + + $contact = ClientContact::factory()->create([ + 'user_id' => $user_id, + 'client_id' => $client->id, + 'company_id' => $company->id, + 'is_primary' => 1, + 'send_email' => true, + ]); + + $payment = Payment::factory()->create([ + 'user_id' => $user_id, + 'client_id' => $client->id, + 'company_id' => $company->id, + 'amount' => 10, + ]); + + $contact2 = ClientContact::factory()->create([ + 'user_id' => $user_id, + 'client_id' => $client->id, + 'company_id' => $company->id, + 'send_email' => true, + ]); + + $vendor = Vendor::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'currency_id' => 1, + ]); + + $vendor_contact = VendorContact::factory()->create([ + 'user_id' => $user_id, + 'vendor_id' => $this->vendor->id, + 'company_id' => $company->id, + 'is_primary' => 1, + 'send_email' => true, + ]); + + $vendor_contact2 = VendorContact::factory()->create([ + 'user_id' => $user_id, + 'vendor_id' => $this->vendor->id, + 'company_id' => $company->id, + 'send_email' => true, + ]); + + $project = Project::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'client_id' => $client->id, + ]); + + $expense = Expense::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + $recurring_expense = RecurringExpense::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'frequency_id' => 5, + 'remaining_cycles' => 5, + ]); + + $recurring_quote = RecurringQuote::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'client_id' => $client->id, + ]); + + $task = Task::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + $invoice = Invoice::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'client_id' => $client->id, + ]); + + $quote = Quote::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'client_id' => $client->id, + ]); + + $credit = Credit::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'client_id' => $client->id, + ]); + + $po = PurchaseOrder::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'vendor_id' => $vendor->id, + ]); + + + $recurring_invoice = RecurringInvoice::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'client_id' => $client->id, + ]); + + + $task_status = TaskStatus::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + $task->status_id = TaskStatus::where('company_id', $company->id)->first()->id; + $task->save(); + + $expense_category = ExpenseCategory::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + + $tax_rate = TaxRate::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + $gs = new GroupSetting; + $gs->name = 'Test'; + $gs->company_id = $client->company_id; + $gs->settings = ClientSettings::buildClientSettings($company->settings, $client->settings); + + $gs_settings = $gs->settings; + $gs_settings->website = 'http://staging.invoicing.co'; + $gs->settings = $gs_settings; + $gs->save(); + + $scheduler = Scheduler::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + $bank_integration = BankIntegration::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'account_id' => $this->account->id, + ]); + + $bank_transaction = BankTransaction::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + 'bank_integration_id' => $bank_integration->id, + ]); + + $bank_transaction_rule = BankTransactionRule::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + + $subscription = Subscription::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + $webhook = WebhookFactory::create($company->id, $user_id); + $webhook->save(); + + $document = Document::factory()->create([ + 'user_id' => $user_id, + 'company_id' => $company->id, + ]); + + $cg = new CompanyGateway; + $cg->company_id = $company->id; + $cg->user_id = $user_id; + $cg->gateway_key = 'd14dd26a37cecc30fdd65700bfb55b23'; + $cg->require_cvv = true; + $cg->require_billing_address = true; + $cg->require_shipping_address = true; + $cg->update_details = true; + $cg->config = encrypt(config('ninja.testvars.stripe')); + $cg->fees_and_limits = []; + $cg->save(); + + $cgt = ClientGatewayTokenFactory::create($company->id); + $cgt->save(); + + } // public function testGeneratingClassName() @@ -113,22 +406,24 @@ class BaseApiTest extends TestCase $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), - 'X-API-TOKEN' => $this->token, + 'X-API-TOKEN' => $this->owner_token, ])->get('/api/v1/users/'); $response->assertStatus(200) ->assertJson(fn (AssertableJson $json) => $json->has('data',2)->etc()); /*does not test the number of records however*/ - collect($this->list_routes)->each(function($route){ + collect($this->list_routes)->filter(function ($route){ + return !in_array($route, ['users','designs','payment_terms']); + })->each(function($route){ nlog($route); $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), - 'X-API-TOKEN' => $this->low_token, + 'X-API-TOKEN' => $this->owner_token, ])->get("/api/v1/{$route}/") ->assertJson(fn (AssertableJson $json) => $json->has('meta') - ->has('data') + ->has('data',1) ); }); @@ -185,7 +480,7 @@ class BaseApiTest extends TestCase collect($this->list_routes)->filter(function ($route){ - return !in_array($route, ['tasks','users','group_settings','designs']); + return !in_array($route, ['tasks', 'users', 'group_settings','designs','client_gateway_tokens']); })->each(function($route){ // nlog($route); $response = $this->withHeaders([ @@ -205,7 +500,12 @@ class BaseApiTest extends TestCase ])->get('/api/v1/companies/'.$this->company->hashed_id) ->assertStatus(401); - + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->low_token, + ])->get('/api/v1/client_gateway_tokens/') + ->assertStatus(401); + } } From a55cee5a68c390dbfb0fab5c1dc8e8c049acf574 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 10:04:12 +1100 Subject: [PATCH 07/13] tests for admin --- tests/Feature/BaseApiTest.php | 53 ++++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php index 0323f0f693db..8ea082ed735e 100644 --- a/tests/Feature/BaseApiTest.php +++ b/tests/Feature/BaseApiTest.php @@ -401,6 +401,9 @@ class BaseApiTest extends TestCase // } + /** + * Tests admin/owner facing routes respond with the correct status and/or data set + */ public function testOwnerRoutes() { @@ -435,6 +438,51 @@ class BaseApiTest extends TestCase } + public function testAdminRoutes() + { + $this->owner_cu = CompanyUser::where('user_id', $this->owner_cu->user_id)->where('company_id', $this->owner_cu->company_id)->first(); + $this->owner_cu->is_owner = false; + $this->owner_cu->is_admin = true; + $this->owner_cu->is_locked = false; + $this->owner_cu->permissions = '[]'; + $this->owner_cu->save(); + + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->owner_token, + ])->get('/api/v1/users/'); + + $response->assertStatus(200) + ->assertJson(fn (AssertableJson $json) => $json->has('data',2)->etc()); + + /*does not test the number of records however*/ + collect($this->list_routes)->filter(function ($route){ + return !in_array($route, ['users','designs','payment_terms']); + })->each(function($route){ + nlog($route); + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->owner_token, + ])->get("/api/v1/{$route}/") + ->assertJson(fn (AssertableJson $json) => + $json->has('meta') + ->has('data',1) + ); + }); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->low_token, + ])->get('/api/v1/companies/'.$this->company->hashed_id) + ->assertStatus(401); + + } + + + /** + * Tests user facing routes respond with the correct status and/or data set + */ public function testRestrictedUserRoute() { // $permissions = ["view_invoice","view_client","edit_client","edit_invoice","create_invoice","create_client"]; @@ -474,15 +522,12 @@ class BaseApiTest extends TestCase 'X-API-TOKEN' => $this->low_token, ])->get('/api/v1/users/'); - $response->assertStatus(200) ->assertJson(fn (AssertableJson $json) => $json->has('data',1)->etc()); - collect($this->list_routes)->filter(function ($route){ return !in_array($route, ['tasks', 'users', 'group_settings','designs','client_gateway_tokens']); })->each(function($route){ - // nlog($route); $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->low_token, @@ -506,6 +551,6 @@ class BaseApiTest extends TestCase ])->get('/api/v1/client_gateway_tokens/') ->assertStatus(401); - } + } From d97f80ecd07d2d75ff40bdfae8b4e4620afc84bb Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 10:08:56 +1100 Subject: [PATCH 08/13] Tests for locked user --- tests/Feature/BaseApiTest.php | 42 +++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php index 8ea082ed735e..40e3c2e1edbc 100644 --- a/tests/Feature/BaseApiTest.php +++ b/tests/Feature/BaseApiTest.php @@ -465,6 +465,7 @@ class BaseApiTest extends TestCase 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->owner_token, ])->get("/api/v1/{$route}/") + ->assertStatus(200) ->assertJson(fn (AssertableJson $json) => $json->has('meta') ->has('data',1) @@ -473,12 +474,49 @@ class BaseApiTest extends TestCase $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), - 'X-API-TOKEN' => $this->low_token, + 'X-API-TOKEN' => $this->owner_token, ])->get('/api/v1/companies/'.$this->company->hashed_id) - ->assertStatus(401); + ->assertStatus(200); } + public function testAdminLockedRoutes() + { + $this->owner_cu = CompanyUser::where('user_id', $this->owner_cu->user_id)->where('company_id', $this->owner_cu->company_id)->first(); + $this->owner_cu->is_owner = false; + $this->owner_cu->is_admin = true; + $this->owner_cu->is_locked = true; + $this->owner_cu->permissions = '[]'; + $this->owner_cu->save(); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->owner_token, + ])->get('/api/v1/users/'); + + $response->assertStatus(403); + + /*does not test the number of records however*/ + collect($this->list_routes)->filter(function ($route){ + return !in_array($route, ['users','designs','payment_terms']); + })->each(function($route){ + nlog($route); + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->owner_token, + ])->get("/api/v1/{$route}/") + ->assertStatus(403); + }); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->owner_token, + ])->get('/api/v1/companies/'.$this->company->hashed_id) + ->assertStatus(403); + + } + + /** * Tests user facing routes respond with the correct status and/or data set From 580868767cf6bee10d98c75db0a4fd3d513934ac Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 11:33:41 +1100 Subject: [PATCH 09/13] Add additional checks into check data script --- app/Console/Commands/CheckData.php | 48 ++++++++++++++---- app/Http/Controllers/BaseController.php | 4 +- tests/Feature/BaseApiTest.php | 66 +++++++++++++++---------- 3 files changed, 79 insertions(+), 39 deletions(-) diff --git a/app/Console/Commands/CheckData.php b/app/Console/Commands/CheckData.php index fd934399fe43..7ffba27472b4 100644 --- a/app/Console/Commands/CheckData.php +++ b/app/Console/Commands/CheckData.php @@ -15,6 +15,7 @@ use App; use App\DataMapper\ClientSettings; use App\Factory\ClientContactFactory; use App\Factory\VendorContactFactory; +use App\Jobs\Company\CreateCompanyToken; use App\Models\Account; use App\Models\Client; use App\Models\ClientContact; @@ -124,7 +125,8 @@ class CheckData extends Command $this->checkOauthSanity(); $this->checkVendorSettings(); $this->checkClientSettings(); - + $this->checkCompanyTokens(); + if(Ninja::isHosted()){ $this->checkAccountStatuses(); $this->checkNinjaPortalUrls(); @@ -157,6 +159,25 @@ class CheckData extends Command $this->log .= $str."\n"; } + private function checkCompanyTokens() + { + + CompanyUser::doesnthave('token')->cursor()->each(function ($cu){ + + if($cu->user){ + $this->logMessage("Creating missing company token for user # {$cu->user->id} for company id # {$cu->company->id}"); + (new CreateCompanyToken($cu->company, $cu->user, 'System'))->handle(); + } + else { + $this->logMessage("Dangling User ID # {$cu->id}"); + } + + }); + + + + } + private function checkOauthSanity() { User::where('oauth_provider_id', '1')->cursor()->each(function ($user){ @@ -422,17 +443,26 @@ class CheckData extends Command $contact_class = VendorContact::class; } - $invitation = new $entity_obj(); - $invitation->company_id = $entity->company_id; - $invitation->user_id = $entity->user_id; - $invitation->{$entity_key} = $entity->id; - $invitation->{$contact_id} = $contact_class::where('company_id', $entity->company_id)->where($client_vendor_key,$entity->{$client_vendor_key})->first()->id; - $invitation->key = Str::random(config('ninja.key_length')); + $invitation = false; - $this->logMessage("Add invitation for {$entity_key} - {$entity->id}"); + //check contact exists! + if($contact_class::where('company_id', $entity->company_id)->where($client_vendor_key,$entity->{$client_vendor_key})->exists()) + { + $invitation = new $entity_obj(); + $invitation->company_id = $entity->company_id; + $invitation->user_id = $entity->user_id; + $invitation->{$entity_key} = $entity->id; + $invitation->{$contact_id} = $contact_class::where('company_id', $entity->company_id)->where($client_vendor_key,$entity->{$client_vendor_key})->first()->id; + $invitation->key = Str::random(config('ninja.key_length')); + $this->logMessage("Add invitation for {$entity_key} - {$entity->id}"); + } + else + $this->logMessage("No contact present, so cannot add invitation for {$entity_key} - {$entity->id}"); try{ - $invitation->save(); + + if($invitation) + $invitation->save(); } catch(\Exception $e){ $this->logMessage($e->getMessage()); diff --git a/app/Http/Controllers/BaseController.php b/app/Http/Controllers/BaseController.php index 3d687333ce81..7727fef4a4f0 100644 --- a/app/Http/Controllers/BaseController.php +++ b/app/Http/Controllers/BaseController.php @@ -886,7 +886,7 @@ class BaseController extends Controller } */ - + /*21-01-2023*/ /**/ // 10-01-2022 need to ensure we snake case properly here to ensure permissions work as expected // 28-03-2022 this is definitely correct here, do not append _ to the view, it resolved correctly when snake cased @@ -908,8 +908,6 @@ class BaseController extends Controller /**/ - - if (request()->has('updated_at') && request()->input('updated_at') > 0) { $query->where('updated_at', '>=', date('Y-m-d H:i:s', intval(request()->input('updated_at')))); } diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php index 40e3c2e1edbc..7a29402defb4 100644 --- a/tests/Feature/BaseApiTest.php +++ b/tests/Feature/BaseApiTest.php @@ -191,8 +191,6 @@ class BaseApiTest extends TestCase $company_token->is_system = true; $company_token->save(); - - Product::factory()->create([ 'user_id' => $user_id, 'company_id' => $company->id, @@ -203,7 +201,6 @@ class BaseApiTest extends TestCase 'company_id' => $company->id, ]); - $contact = ClientContact::factory()->create([ 'user_id' => $user_id, 'client_id' => $client->id, @@ -307,7 +304,6 @@ class BaseApiTest extends TestCase 'client_id' => $client->id, ]); - $task_status = TaskStatus::factory()->create([ 'user_id' => $user_id, 'company_id' => $company->id, @@ -321,7 +317,6 @@ class BaseApiTest extends TestCase 'company_id' => $company->id, ]); - $tax_rate = TaxRate::factory()->create([ 'user_id' => $user_id, 'company_id' => $company->id, @@ -388,7 +383,6 @@ class BaseApiTest extends TestCase $cgt = ClientGatewayTokenFactory::create($company->id); $cgt->save(); - } // public function testGeneratingClassName() @@ -430,14 +424,20 @@ class BaseApiTest extends TestCase ); }); - $response = $this->withHeaders([ - 'X-API-SECRET' => config('ninja.api_secret'), - 'X-API-TOKEN' => $this->low_token, - ])->get('/api/v1/companies/'.$this->company->hashed_id) - ->assertStatus(401); + } + + public function testOwnerAccessCompany() + { + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->low_token, + ])->get('/api/v1/companies/'.$this->company->hashed_id) + ->assertStatus(401); } + public function testAdminRoutes() { $this->owner_cu = CompanyUser::where('user_id', $this->owner_cu->user_id)->where('company_id', $this->owner_cu->company_id)->first(); @@ -447,7 +447,6 @@ class BaseApiTest extends TestCase $this->owner_cu->permissions = '[]'; $this->owner_cu->save(); - $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->owner_token, @@ -456,7 +455,6 @@ class BaseApiTest extends TestCase $response->assertStatus(200) ->assertJson(fn (AssertableJson $json) => $json->has('data',2)->etc()); - /*does not test the number of records however*/ collect($this->list_routes)->filter(function ($route){ return !in_array($route, ['users','designs','payment_terms']); })->each(function($route){ @@ -472,11 +470,16 @@ class BaseApiTest extends TestCase ); }); + } + + public function testAdminAccessCompany() + { + $response = $this->withHeaders([ - 'X-API-SECRET' => config('ninja.api_secret'), - 'X-API-TOKEN' => $this->owner_token, - ])->get('/api/v1/companies/'.$this->company->hashed_id) - ->assertStatus(200); + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->owner_token, + ])->get('/api/v1/companies/'.$this->company->hashed_id) + ->assertStatus(200); } @@ -492,11 +495,9 @@ class BaseApiTest extends TestCase $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->owner_token, - ])->get('/api/v1/users/'); + ])->get('/api/v1/users/') + ->assertStatus(403); - $response->assertStatus(403); - - /*does not test the number of records however*/ collect($this->list_routes)->filter(function ($route){ return !in_array($route, ['users','designs','payment_terms']); })->each(function($route){ @@ -508,14 +509,25 @@ class BaseApiTest extends TestCase ->assertStatus(403); }); - $response = $this->withHeaders([ - 'X-API-SECRET' => config('ninja.api_secret'), - 'X-API-TOKEN' => $this->owner_token, - ])->get('/api/v1/companies/'.$this->company->hashed_id) - ->assertStatus(403); - } + public function testAdminLockedCompany() + { + + $this->owner_cu = CompanyUser::where('user_id', $this->owner_cu->user_id)->where('company_id', $this->owner_cu->company_id)->first(); + $this->owner_cu->is_owner = false; + $this->owner_cu->is_admin = true; + $this->owner_cu->is_locked = true; + $this->owner_cu->permissions = '[]'; + $this->owner_cu->save(); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->owner_token, + ])->get('/api/v1/companies/'.$this->company->hashed_id) + ->assertStatus(403); + + } /** From 29bab35ed12956476b8f9d9d0053b8ad85a4f9ac Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 14:41:21 +1100 Subject: [PATCH 10/13] add stubs for stripe data --- tests/Feature/BaseApiTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php index 7a29402defb4..b980d8c1cdc6 100644 --- a/tests/Feature/BaseApiTest.php +++ b/tests/Feature/BaseApiTest.php @@ -376,7 +376,7 @@ class BaseApiTest extends TestCase $cg->require_billing_address = true; $cg->require_shipping_address = true; $cg->update_details = true; - $cg->config = encrypt(config('ninja.testvars.stripe')); + $cg->config = encrypt('{"publishableKey":"pk_test_P1riKDKD0p","apiKey":"sk_test_Yorqvz45"}'); $cg->fees_and_limits = []; $cg->save(); From c1563c571d31f08aa339b3a9aebd09ab0b4a027a Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 16:52:24 +1100 Subject: [PATCH 11/13] Permissions tests --- app/Http/Controllers/BaseController.php | 38 +------ app/Models/Client.php | 12 ++- app/Models/Traits/Excludable.php | 38 +++++++ app/Models/User.php | 25 +++++ tests/Feature/BaseApiTest.php | 6 +- tests/Unit/PermissionsTest.php | 125 ++++++++++++++++++++++++ 6 files changed, 206 insertions(+), 38 deletions(-) create mode 100644 app/Models/Traits/Excludable.php create mode 100644 tests/Unit/PermissionsTest.php diff --git a/app/Http/Controllers/BaseController.php b/app/Http/Controllers/BaseController.php index 7727fef4a4f0..45fa77bf03b8 100644 --- a/app/Http/Controllers/BaseController.php +++ b/app/Http/Controllers/BaseController.php @@ -234,6 +234,7 @@ class BaseController extends Controller } $transformer = new $this->entity_transformer($this->serializer); + $updated_at = request()->has('updated_at') ? request()->input('updated_at') : 0; if ($user->getCompany()->is_large && $updated_at == 0) { @@ -251,7 +252,6 @@ class BaseController extends Controller $query->where('clients.updated_at', '>=', $updated_at)->with('contacts.company', 'gateway_tokens', 'documents'); if (! $user->hasPermission('view_client')) { - // $query->where('clients.user_id', $user->id)->orWhere('clients.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('clients.user_id', $user->id)->orWhere('clients.assigned_user_id', $user->id); @@ -270,7 +270,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('invitations', 'documents'); if (! $user->hasPermission('view_credit')) { - // $query->where('credits.user_id', $user->id)->orWhere('credits.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('credits.user_id', $user->id)->orWhere('credits.assigned_user_id', $user->id); @@ -291,7 +290,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('documents'); if (! $user->hasPermission('view_expense')) { - // $query->where('expenses.user_id', $user->id)->orWhere('expenses.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('expenses.user_id', $user->id)->orWhere('expenses.assigned_user_id', $user->id); @@ -301,14 +299,11 @@ class BaseController extends Controller 'company.groups' => function ($query) use ($updated_at, $user) { $query->whereNotNull('updated_at')->with('documents'); - // if(!$user->isAdmin()) - // $query->where('group_settings.user_id', $user->id); }, 'company.invoices'=> function ($query) use ($updated_at, $user) { $query->where('updated_at', '>=', $updated_at)->with('invitations', 'documents'); if (! $user->hasPermission('view_invoice')) { - // $query->where('invoices.user_id', $user->id)->orWhere('invoices.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { @@ -321,7 +316,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('paymentables', 'documents'); if (! $user->hasPermission('view_payment')) { - // $query->where('payments.user_id', $user->id)->orWhere('payments.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('payments.user_id', $user->id)->orWhere('payments.assigned_user_id', $user->id); @@ -340,13 +334,11 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('documents'); if (! $user->hasPermission('view_product')) { - // $query->where('products.user_id', $user->id)->orWhere('products.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('products.user_id', $user->id)->orWhere('products.assigned_user_id', $user->id); }); - } }, @@ -354,7 +346,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('documents'); if (! $user->hasPermission('view_project')) { - // $query->where('projects.user_id', $user->id)->orWhere('projects.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('projects.user_id', $user->id)->orWhere('projects.assigned_user_id', $user->id); @@ -366,8 +357,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('documents'); if (! $user->hasPermission('view_purchase_order')) { - // $query->where('purchase_orders.user_id', $user->id)->orWhere('purchase_orders.assigned_user_id', $user->id); - $query->whereNested(function($query) use ($user) { $query->where('purchase_orders.user_id', $user->id)->orWhere('purchase_orders.assigned_user_id', $user->id); @@ -379,8 +368,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('invitations', 'documents'); if (! $user->hasPermission('view_quote')) { - // $query->where('quotes.user_id', $user->id)->orWhere('quotes.assigned_user_id', $user->id); - $query->whereNested(function($query) use ($user) { $query->where('quotes.user_id', $user->id)->orWhere('quotes.assigned_user_id', $user->id); @@ -392,7 +379,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('invitations', 'documents', 'client.gateway_tokens', 'client.group_settings', 'client.company'); if (! $user->hasPermission('view_recurring_invoice')) { - // $query->where('recurring_invoices.user_id', $user->id)->orWhere('recurring_invoices.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('recurring_invoices.user_id', $user->id)->orWhere('recurring_invoices.assigned_user_id', $user->id); @@ -404,7 +390,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('documents'); if (! $user->hasPermission('view_recurring_expense')) { - // $query->where('recurring_expenses.user_id', $user->id)->orWhere('recurring_expenses.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('recurring_expenses.user_id', $user->id)->orWhere('recurring_expenses.assigned_user_id', $user->id); @@ -416,7 +401,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('documents'); if (! $user->hasPermission('view_task')) { - // $query->where('tasks.user_id', $user->id)->orWhere('tasks.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('tasks.user_id', $user->id)->orWhere('tasks.assigned_user_id', $user->id); @@ -431,7 +415,6 @@ class BaseController extends Controller $query->where('updated_at', '>=', $updated_at)->with('contacts', 'documents'); if (! $user->hasPermission('view_vendor')) { - // $query->where('vendors.user_id', $user->id)->orWhere('vendors.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('vendors.user_id', $user->id)->orWhere('vendors.assigned_user_id', $user->id); @@ -622,7 +605,6 @@ class BaseController extends Controller $query->where('clients.created_at', '>=', $created_at)->with('contacts.company', 'gateway_tokens', 'documents'); if (! $user->hasPermission('view_client')) { - // $query->where('clients.user_id', $user->id)->orWhere('clients.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('clients.user_id', $user->id)->orWhere('clients.assigned_user_id', $user->id); @@ -641,7 +623,6 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('invitations', 'documents'); if (! $user->hasPermission('view_credit')) { - // $query->where('credits.user_id', $user->id)->orWhere('credits.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('credits.user_id', $user->id)->orWhere('credits.assigned_user_id', $user->id); @@ -655,13 +636,10 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('documents'); if (! $user->hasPermission('view_expense')) { - // $query->where('expenses.user_id', $user->id)->orWhere('expenses.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('expenses.user_id', $user->id)->orWhere('expenses.assigned_user_id', $user->id); }); - - } }, 'company.groups' => function ($query) use ($created_at, $user) { @@ -671,7 +649,6 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('invitations', 'documents'); if (! $user->hasPermission('view_invoice')) { - // $query->where('invoices.user_id', $user->id)->orWhere('invoices.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('invoices.user_id', $user->id)->orWhere('invoices.assigned_user_id', $user->id); @@ -683,7 +660,6 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('paymentables', 'documents'); if (! $user->hasPermission('view_payment')) { - // $query->where('payments.user_id', $user->id)->orWhere('payments.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('payments.user_id', $user->id)->orWhere('payments.assigned_user_id', $user->id); @@ -698,7 +674,7 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('documents'); if (! $user->hasPermission('view_product')) { - // $query->where('products.user_id', $user->id)->orWhere('products.assigned_user_id', $user->id); + $query->whereNested(function($query) use ($user) { $query->where('products.user_id', $user->id)->orWhere('products.assigned_user_id', $user->id); }); @@ -708,7 +684,7 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('documents'); if (! $user->hasPermission('view_project')) { - // $query->where('projects.user_id', $user->id)->orWhere('projects.assigned_user_id', $user->id); + $query->whereNested(function($query) use ($user) { $query->where('projects.user_id', $user->id)->orWhere('projects.assigned_user_id', $user->id); }); @@ -718,7 +694,7 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('documents'); if (! $user->hasPermission('view_purchase_order')) { - // $query->where('purchase_orders.user_id', $user->id)->orWhere('purchase_orders.assigned_user_id', $user->id); + $query->whereNested(function($query) use ($user) { $query->where('purchase_orders.user_id', $user->id)->orWhere('purchase_orders.assigned_user_id', $user->id); }); @@ -729,7 +705,6 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('invitations', 'documents'); if (! $user->hasPermission('view_quote')) { - // $query->where('quotes.user_id', $user->id)->orWhere('quotes.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('quotes.user_id', $user->id)->orWhere('quotes.assigned_user_id', $user->id); @@ -741,7 +716,6 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('invitations', 'documents', 'client.gateway_tokens', 'client.group_settings', 'client.company'); if (! $user->hasPermission('view_recurring_invoice')) { - // $query->where('recurring_invoices.user_id', $user->id)->orWhere('recurring_invoices.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('recurring_invoices.user_id', $user->id)->orWhere('recurring_invoices.assigned_user_id', $user->id); @@ -753,7 +727,6 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('documents'); if (! $user->hasPermission('view_task')) { -// $query->where('tasks.user_id', $user->id)->orWhere('tasks.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('tasks.user_id', $user->id)->orWhere('tasks.assigned_user_id', $user->id); @@ -768,7 +741,6 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('contacts', 'documents'); if (! $user->hasPermission('view_vendor')) { - // $query->where('vendors.user_id', $user->id)->orWhere('vendors.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('vendors.user_id', $user->id)->orWhere('vendors.assigned_user_id', $user->id); @@ -807,7 +779,6 @@ class BaseController extends Controller $query->where('created_at', '>=', $created_at)->with('documents'); if (! $user->hasPermission('view_recurring_expense')) { - // $query->where('recurring_expenses.user_id', $user->id)->orWhere('recurring_expenses.assigned_user_id', $user->id); $query->whereNested(function($query) use ($user) { $query->where('recurring_expenses.user_id', $user->id)->orWhere('recurring_expenses.assigned_user_id', $user->id); @@ -907,6 +878,7 @@ class BaseController extends Controller } /**/ + // $query->exclude(['balance','credit_balance','paid_to_date']); if (request()->has('updated_at') && request()->input('updated_at') > 0) { $query->where('updated_at', '>=', date('Y-m-d H:i:s', intval(request()->input('updated_at')))); diff --git a/app/Models/Client.php b/app/Models/Client.php index 7b09fa68f80e..24dd322b942d 100644 --- a/app/Models/Client.php +++ b/app/Models/Client.php @@ -21,12 +21,12 @@ use App\Models\Project; use App\Models\Quote; use App\Models\Task; use App\Services\Client\ClientService; +use App\Models\Traits\Excludable; use App\Utils\Traits\AppSetup; use App\Utils\Traits\ClientGroupSettingsSaver; use App\Utils\Traits\GeneratesCounter; use App\Utils\Traits\MakesDates; use App\Utils\Traits\MakesHash; -use Exception; use Illuminate\Contracts\Translation\HasLocalePreference; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Support\Facades\Cache; @@ -42,6 +42,7 @@ class Client extends BaseModel implements HasLocalePreference use GeneratesCounter; use AppSetup; use ClientGroupSettingsSaver; + use Excludable; protected $presenter = ClientPresenter::class; @@ -132,6 +133,13 @@ class Client extends BaseModel implements HasLocalePreference 'phone', ]; + // public function scopeExclude($query) + // { + // $query->makeHidden(['balance','paid_to_date']); + + // return $query; + // } + public function getEntityType() { return self::class; @@ -417,7 +425,7 @@ class Client extends BaseModel implements HasLocalePreference return $this->company; } - throw new Exception('Could not find a settings object', 1); + throw new \Exception('Could not find a settings object', 1); } public function documents() diff --git a/app/Models/Traits/Excludable.php b/app/Models/Traits/Excludable.php new file mode 100644 index 000000000000..06b494dd2808 --- /dev/null +++ b/app/Models/Traits/Excludable.php @@ -0,0 +1,38 @@ +getConnection()->getSchemaBuilder()->getColumnListing($this->getTable()); + } + + /** + * Exclude an array of elements from the result. + * @param $query + * @param $columns + * + * @return mixed + */ + public function scopeExclude($query, $columns) + { + return $query->select(array_diff($this->getTableColumns(), (array) $columns)); + } + +} + diff --git a/app/Models/User.php b/app/Models/User.php index b27f81f043f7..2b802e5e1e43 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -372,6 +372,31 @@ class User extends Authenticatable implements MustVerifyEmail } + /** + * Used when we need to match exactly what permission + * the user has, and not aggregate owner and admins. + * + * This method is used when we need to scope down the query + * and display a limited subset. + * + * @param string $permission '["view_all"]' + * @return boolean + */ + public function hasExactPermission(string $permission = ''): bool + { + + $parts = explode('_', $permission); + $all_permission = ''; + + if (count($parts) > 1) { + $all_permission = $parts[0].'_all'; + } + + return (is_int(stripos($this->token()->cu->permissions, $all_permission))) || + (is_int(stripos($this->token()->cu->permissions, $permission))); + + } + public function documents() { return $this->morphMany(Document::class, 'documentable'); diff --git a/tests/Feature/BaseApiTest.php b/tests/Feature/BaseApiTest.php index b980d8c1cdc6..b624387a3963 100644 --- a/tests/Feature/BaseApiTest.php +++ b/tests/Feature/BaseApiTest.php @@ -413,7 +413,7 @@ class BaseApiTest extends TestCase collect($this->list_routes)->filter(function ($route){ return !in_array($route, ['users','designs','payment_terms']); })->each(function($route){ - nlog($route); + // nlog($route); $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->owner_token, @@ -458,7 +458,7 @@ class BaseApiTest extends TestCase collect($this->list_routes)->filter(function ($route){ return !in_array($route, ['users','designs','payment_terms']); })->each(function($route){ - nlog($route); + // nlog($route); $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->owner_token, @@ -501,7 +501,7 @@ class BaseApiTest extends TestCase collect($this->list_routes)->filter(function ($route){ return !in_array($route, ['users','designs','payment_terms']); })->each(function($route){ - nlog($route); + // nlog($route); $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->owner_token, diff --git a/tests/Unit/PermissionsTest.php b/tests/Unit/PermissionsTest.php new file mode 100644 index 000000000000..91fa3e9854a0 --- /dev/null +++ b/tests/Unit/PermissionsTest.php @@ -0,0 +1,125 @@ +faker = \Faker\Factory::create(); + + $account = Account::factory()->create([ + 'hosted_client_count' => 1000, + 'hosted_company_count' => 1000, + ]); + + $account->num_users = 3; + $account->save(); + + $this->company = Company::factory()->create([ + 'account_id' => $account->id, + ]); + + $this->user = User::factory()->create([ + 'account_id' => $account->id, + 'confirmation_code' => '123', + 'email' => $this->faker->safeEmail(), + ]); + + $this->cu = CompanyUserFactory::create($this->user->id, $this->company->id, $account->id); + $this->cu->is_owner = false; + $this->cu->is_admin = false; + $this->cu->is_locked = false; + $this->cu->permissions = '["view_client"]'; + $this->cu->save(); + + $this->token = \Illuminate\Support\Str::random(64); + + $company_token = new CompanyToken; + $company_token->user_id = $this->user->id; + $company_token->company_id = $this->company->id; + $company_token->account_id = $account->id; + $company_token->name = 'test token'; + $company_token->token = $this->token; + $company_token->is_system = true; + $company_token->save(); + + } + + public function testExactPermissions() + { + + $this->assertTrue($this->user->hasExactPermission("view_client")); + $this->assertFalse($this->user->hasExactPermission("view_all")); + + } + + public function testMissingPermissions() + { + + $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); + $low_cu->permissions = '[""]'; + $low_cu->save(); + + $this->assertFalse($this->user->hasExactPermission("view_client")); + $this->assertFalse($this->user->hasExactPermission("view_all")); + + } + + public function testViewAllValidPermissions() + { + + $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); + $low_cu->permissions = '["view_all"]'; + $low_cu->save(); + + $this->assertTrue($this->user->hasExactPermission("view_client")); + $this->assertTrue($this->user->hasExactPermission("view_all")); + + } + + public function testViewClientPermission() + { + + $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); + $low_cu->permissions = '["view_client"]'; + $low_cu->save(); + + //this is aberrant + $this->assertTrue($this->user->hasPermission("viewclient")); + + } + +} + From 0ae2260951e8d72d9a8b32f999a2b110bd59be88 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 16:59:51 +1100 Subject: [PATCH 12/13] Tests around hasPermission --- app/Models/User.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/Models/User.php b/app/Models/User.php index 2b802e5e1e43..c8ebf0692297 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -365,6 +365,8 @@ class User extends Authenticatable implements MustVerifyEmail $all_permission = $parts[0].'_all'; } +//empty $all_permissions leads to stripos returning true; + return $this->isOwner() || $this->isAdmin() || (is_int(stripos($this->token()->cu->permissions, $all_permission))) || From 1944d8214e7781374c777d4477606db3fb8087b0 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 21 Jan 2023 19:19:08 +1100 Subject: [PATCH 13/13] Clean up for logging. --- app/Http/Controllers/BaseController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Http/Controllers/BaseController.php b/app/Http/Controllers/BaseController.php index 45fa77bf03b8..76e85b80ce6b 100644 --- a/app/Http/Controllers/BaseController.php +++ b/app/Http/Controllers/BaseController.php @@ -870,7 +870,7 @@ class BaseController extends Controller $query->where('user_id', '=', auth()->user()->id); } elseif(in_array($this->entity_type,[Design::class, GroupSetting::class, PaymentTerm::class])){ - nlog($this->entity_type); + // nlog($this->entity_type); } else $query->where('user_id', '=', auth()->user()->id)->orWhere('assigned_user_id', auth()->user()->id);