From a4d8e23a24798ce15e80cb2f17035aa53ff33890 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 20 Mar 2021 08:48:04 +1100 Subject: [PATCH 01/10] Update readme --- README.md | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/README.md b/README.md index 77af4773cb0b..aa2a4db850a6 100644 --- a/README.md +++ b/README.md @@ -7,15 +7,7 @@ [![Codacy Badge](https://api.codacy.com/project/badge/Grade/d39acb4bf0f74a0698dc77f382769ba5)](https://www.codacy.com/app/turbo124/invoiceninja?utm_source=github.com&utm_medium=referral&utm_content=invoiceninja/invoiceninja&utm_campaign=Badge_Grade) -# Invoice Ninja version 5.1 RC2! - -Invoice Ninja version 5.1 has now reached Release Candidate 2! - -What does this mean exactly? We consider this version _almost_ stable. There may be some remaining small issues which we would love to get feedback on. We would really appreciate the community booting up this version and attempting the migration from their Invoice Ninja V4 application and inspect the migrated data. - -We'd also like feedback on any issues that you can see, and help us nail down the few remaining issues before Version 5 graduates to Stable Gold Release. - -Please note we do not consider this version ready for production use, please stick with your V4 installation for your production clients! +# Invoice Ninja version 5! ## Quick Start From b841fe7000b8a83a632a1e4ee487d9962bd4cd9e Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 20 Mar 2021 09:29:20 +1100 Subject: [PATCH 02/10] Working on unique rules for numbers --- .../Requests/Client/UpdateClientRequest.php | 11 +++- .../Requests/Payment/UpdatePaymentRequest.php | 5 +- ...add_unique_constraints_on_all_entities.php | 65 +++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 database/migrations/2021_03_19_221024_add_unique_constraints_on_all_entities.php diff --git a/app/Http/Requests/Client/UpdateClientRequest.php b/app/Http/Requests/Client/UpdateClientRequest.php index e322ad0a29f3..c230597960ae 100644 --- a/app/Http/Requests/Client/UpdateClientRequest.php +++ b/app/Http/Requests/Client/UpdateClientRequest.php @@ -16,6 +16,7 @@ use App\Http\Requests\Request; use App\Http\ValidationRules\ValidClientGroupSettingsRule; use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\MakesHash; +use Illuminate\Validation\Rule; class UpdateClientRequest extends Request { @@ -52,7 +53,14 @@ class UpdateClientRequest extends Request $rules['country_id'] = 'integer|nullable'; $rules['shipping_country_id'] = 'integer|nullable'; //$rules['id_number'] = 'unique:clients,id_number,,id,company_id,' . auth()->user()->company()->id; - $rules['id_number'] = 'unique:clients,id_number,'.$this->id.',id,company_id,'.$this->company_id; + //$rules['id_number'] = 'unique:clients,id_number,'.$this->id.',id,company_id,'.$this->company_id; + + if($this->id_number) + $rules['id_number'] = Rule::unique('clients')->where('company_id', auth()->user()->company()->id)->ignore($this->client->id); + + if($this->number) + $rules['number'] = Rule::unique('clients')->where('company_id', auth()->user()->company()->id)->ignore($this->client->id); + $rules['settings'] = new ValidClientGroupSettingsRule(); $rules['contacts.*.email'] = 'bail|nullable|distinct|sometimes|email'; $rules['contacts.*.password'] = [ @@ -72,7 +80,6 @@ class UpdateClientRequest extends Request public function messages() { return [ - 'unique' => ctrans('validation.unique', ['attribute' => 'email']), 'email' => ctrans('validation.email', ['attribute' => 'email']), 'name.required' => ctrans('validation.required', ['attribute' => 'name']), 'required' => ctrans('validation.required', ['attribute' => 'email']), diff --git a/app/Http/Requests/Payment/UpdatePaymentRequest.php b/app/Http/Requests/Payment/UpdatePaymentRequest.php index e5436b55bb12..e576b182a71f 100644 --- a/app/Http/Requests/Payment/UpdatePaymentRequest.php +++ b/app/Http/Requests/Payment/UpdatePaymentRequest.php @@ -35,12 +35,15 @@ class UpdatePaymentRequest extends Request public function rules() { $rules = [ - 'number' => 'nullable|unique:payments,number,'.$this->id.',id,company_id,'.$this->payment->company_id, 'invoices' => ['array', new PaymentAppliedValidAmount, new ValidCreditsPresentRule], 'invoices.*.invoice_id' => 'distinct', 'documents' => 'mimes:png,ai,svg,jpeg,tiff,pdf,gif,psd,txt,doc,xls,ppt,xlsx,docx,pptx', ]; + if ($this->input('number')) { + $rules['number'] = 'nullable|unique:payments,number,'.$this->id.',id,company_id,'.$this->payment->company_id; + } + if ($this->input('documents') && is_array($this->input('documents'))) { $documents = count($this->input('documents')); diff --git a/database/migrations/2021_03_19_221024_add_unique_constraints_on_all_entities.php b/database/migrations/2021_03_19_221024_add_unique_constraints_on_all_entities.php new file mode 100644 index 000000000000..a1a1d2d01921 --- /dev/null +++ b/database/migrations/2021_03_19_221024_add_unique_constraints_on_all_entities.php @@ -0,0 +1,65 @@ +unique(['company_id', 'number']); + }); + + Schema::table('tasks', function (Blueprint $table) { + $table->unique(['company_id', 'number']); + }); + + Schema::table('vendors', function (Blueprint $table) { + $table->unique(['company_id', 'number']); + }); + + Schema::table('payments', function (Blueprint $table) { + $table->unique(['company_id', 'number']); + }); + + Schema::table('projects', function (Blueprint $table) { + $table->unique(['company_id', 'number']); + }); + + Schema::table('clients', function (Blueprint $table) { + $table->unique(['company_id', 'number']); + }); + + Schema::table('payment_hashes', function (Blueprint $table) { + $table->unique(['hash']); + }); + + Schema::table('recurring_invoices', function (Blueprint $table) { + $table->string('number')->change(); + $table->unique(['company_id', 'number']); + }); + + Schema::table('recurring_invoice_invitations', function (Blueprint $table) { + $table->unique(['client_contact_id', 'recurring_invoice_id'],'recur_invoice_client_unique'); + }); + + + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + // + } +} From 451e4e1bbeff6ec5de774e802fc1b1977791f828 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 20 Mar 2021 09:51:52 +1100 Subject: [PATCH 03/10] Ensure better unique rules --- .../Requests/Credit/UpdateCreditRequest.php | 6 +++--- .../Requests/Invoice/UpdateInvoiceRequest.php | 7 +++---- .../Requests/Payment/UpdatePaymentRequest.php | 6 +++--- .../Requests/Quote/UpdateQuoteRequest.php | 6 +++--- .../UpdateRecurringInvoiceRequest.php | 7 ++++--- .../UpdateRecurringQuoteRequest.php | 4 ++++ .../Requests/TaxRate/UpdateTaxRateRequest.php | 15 ++++++++++----- .../Requests/Vendor/UpdateVendorRequest.php | 19 ++++++++----------- 8 files changed, 38 insertions(+), 32 deletions(-) diff --git a/app/Http/Requests/Credit/UpdateCreditRequest.php b/app/Http/Requests/Credit/UpdateCreditRequest.php index 426301ef39b4..2c7c43d39bf9 100644 --- a/app/Http/Requests/Credit/UpdateCreditRequest.php +++ b/app/Http/Requests/Credit/UpdateCreditRequest.php @@ -16,6 +16,7 @@ use App\Http\Requests\Request; use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\CleanLineItems; use App\Utils\Traits\MakesHash; +use Illuminate\Validation\Rule; class UpdateCreditRequest extends Request { @@ -52,9 +53,8 @@ class UpdateCreditRequest extends Request $rules['documents'] = 'file|mimes:png,ai,svg,jpeg,tiff,pdf,gif,psd,txt,doc,xls,ppt,xlsx,docx,pptx|max:20000'; } - if ($this->input('number')) { - $rules['number'] = 'unique:credits,number,'.$this->id.',id,company_id,'.$this->credit->company_id; - } + if($this->number) + $rules['number'] = Rule::unique('credits')->where('company_id', auth()->user()->company()->id)->ignore($this->credit->id); $rules['line_items'] = 'array'; diff --git a/app/Http/Requests/Invoice/UpdateInvoiceRequest.php b/app/Http/Requests/Invoice/UpdateInvoiceRequest.php index 7234211d1884..0fb69a64e3f6 100644 --- a/app/Http/Requests/Invoice/UpdateInvoiceRequest.php +++ b/app/Http/Requests/Invoice/UpdateInvoiceRequest.php @@ -16,6 +16,7 @@ use App\Http\ValidationRules\Invoice\LockedInvoiceRule; use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\CleanLineItems; use App\Utils\Traits\MakesHash; +use Illuminate\Validation\Rule; class UpdateInvoiceRequest extends Request { @@ -49,10 +50,8 @@ class UpdateInvoiceRequest extends Request $rules['id'] = new LockedInvoiceRule($this->invoice); - // if ($this->input('number') && strlen($this->input('number')) >= 1) { - if ($this->input('number')) { - $rules['number'] = 'unique:invoices,number,'.$this->id.',id,company_id,'.$this->invoice->company_id; - } + if($this->number) + $rules['number'] = Rule::unique('invoices')->where('company_id', auth()->user()->company()->id)->ignore($this->invoice->id); $rules['line_items'] = 'array'; diff --git a/app/Http/Requests/Payment/UpdatePaymentRequest.php b/app/Http/Requests/Payment/UpdatePaymentRequest.php index e576b182a71f..9f921821dc45 100644 --- a/app/Http/Requests/Payment/UpdatePaymentRequest.php +++ b/app/Http/Requests/Payment/UpdatePaymentRequest.php @@ -16,6 +16,7 @@ use App\Http\ValidationRules\PaymentAppliedValidAmount; use App\Http\ValidationRules\ValidCreditsPresentRule; use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\MakesHash; +use Illuminate\Validation\Rule; class UpdatePaymentRequest extends Request { @@ -40,9 +41,8 @@ class UpdatePaymentRequest extends Request 'documents' => 'mimes:png,ai,svg,jpeg,tiff,pdf,gif,psd,txt,doc,xls,ppt,xlsx,docx,pptx', ]; - if ($this->input('number')) { - $rules['number'] = 'nullable|unique:payments,number,'.$this->id.',id,company_id,'.$this->payment->company_id; - } + if($this->number) + $rules['number'] = Rule::unique('payments')->where('company_id', auth()->user()->company()->id)->ignore($this->payment->id); if ($this->input('documents') && is_array($this->input('documents'))) { $documents = count($this->input('documents')); diff --git a/app/Http/Requests/Quote/UpdateQuoteRequest.php b/app/Http/Requests/Quote/UpdateQuoteRequest.php index 019c630aac3d..6ce240c6bae1 100644 --- a/app/Http/Requests/Quote/UpdateQuoteRequest.php +++ b/app/Http/Requests/Quote/UpdateQuoteRequest.php @@ -15,6 +15,7 @@ use App\Http\Requests\Request; use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\CleanLineItems; use App\Utils\Traits\MakesHash; +use Illuminate\Validation\Rule; class UpdateQuoteRequest extends Request { @@ -46,9 +47,8 @@ class UpdateQuoteRequest extends Request $rules['documents'] = 'file|mimes:png,ai,svg,jpeg,tiff,pdf,gif,psd,txt,doc,xls,ppt,xlsx,docx,pptx|max:20000'; } - if ($this->input('number')) { - $rules['number'] = 'unique:quotes,number,'.$this->id.',id,company_id,'.$this->quote->company_id; - } + if($this->number) + $rules['number'] = Rule::unique('quotes')->where('company_id', auth()->user()->company()->id)->ignore($this->quote->id); $rules['line_items'] = 'array'; diff --git a/app/Http/Requests/RecurringInvoice/UpdateRecurringInvoiceRequest.php b/app/Http/Requests/RecurringInvoice/UpdateRecurringInvoiceRequest.php index 6d6609534247..7156974c233c 100644 --- a/app/Http/Requests/RecurringInvoice/UpdateRecurringInvoiceRequest.php +++ b/app/Http/Requests/RecurringInvoice/UpdateRecurringInvoiceRequest.php @@ -16,6 +16,7 @@ use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\CleanLineItems; use App\Utils\Traits\MakesHash; use Illuminate\Http\UploadedFile; +use Illuminate\Validation\Rule; class UpdateRecurringInvoiceRequest extends Request { @@ -47,9 +48,9 @@ class UpdateRecurringInvoiceRequest extends Request $rules['documents'] = 'file|mimes:png,ai,svg,jpeg,tiff,pdf,gif,psd,txt,doc,xls,ppt,xlsx,docx,pptx|max:20000'; } - if ($this->input('number')) { - $rules['number'] = 'unique:recurring_invoices,number,'.$this->recurring_invoice->id.',id,company_id,'.$this->recurring_invoice->company_id; - } + if($this->number) + $rules['number'] = Rule::unique('recurring_invoices')->where('company_id', auth()->user()->company()->id)->ignore($this->recurring_invoice->id); + return $rules; } diff --git a/app/Http/Requests/RecurringQuote/UpdateRecurringQuoteRequest.php b/app/Http/Requests/RecurringQuote/UpdateRecurringQuoteRequest.php index 7a05426a1568..8527cba80c2a 100644 --- a/app/Http/Requests/RecurringQuote/UpdateRecurringQuoteRequest.php +++ b/app/Http/Requests/RecurringQuote/UpdateRecurringQuoteRequest.php @@ -14,6 +14,7 @@ namespace App\Http\Requests\RecurringQuote; use App\Http\Requests\Request; use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\CleanLineItems; +use Illuminate\Validation\Rule; class UpdateRecurringQuoteRequest extends Request { @@ -47,6 +48,9 @@ class UpdateRecurringQuoteRequest extends Request $input['line_items'] = isset($input['line_items']) ? $this->cleanItems($input['line_items']) : []; + if($this->number) + $rules['number'] = Rule::unique('recurring_quotes')->where('company_id', auth()->user()->company()->id)->ignore($this->recurring_quote->id); + $this->replace($input); } } diff --git a/app/Http/Requests/TaxRate/UpdateTaxRateRequest.php b/app/Http/Requests/TaxRate/UpdateTaxRateRequest.php index 949630e58ab7..ea11b59f37e5 100644 --- a/app/Http/Requests/TaxRate/UpdateTaxRateRequest.php +++ b/app/Http/Requests/TaxRate/UpdateTaxRateRequest.php @@ -12,6 +12,7 @@ namespace App\Http\Requests\TaxRate; use App\Http\Requests\Request; +use Illuminate\Validation\Rule; class UpdateTaxRateRequest extends Request { @@ -27,10 +28,14 @@ class UpdateTaxRateRequest extends Request public function rules() { - return [ - // 'name' => 'unique:tax_rates,name,'.$this->tax_rate->name.',id,company_id,'.auth()->user()->companyId(), - 'name' => 'unique:tax_rates,name,'.$this->id.',id,company_id,'.$this->company_id, - 'rate' => 'numeric', - ]; + $rules = []; + + $rules['rate'] = 'numeric'; + + if($this->number) + $rules['number'] = Rule::unique('tax_rates')->where('company_id', auth()->user()->company()->id)->ignore($this->tax_rate->id); + + return $rules; + } } diff --git a/app/Http/Requests/Vendor/UpdateVendorRequest.php b/app/Http/Requests/Vendor/UpdateVendorRequest.php index 2c7038492d93..14f334e278d0 100644 --- a/app/Http/Requests/Vendor/UpdateVendorRequest.php +++ b/app/Http/Requests/Vendor/UpdateVendorRequest.php @@ -14,6 +14,7 @@ namespace App\Http\Requests\Vendor; use App\Http\Requests\Request; use App\Utils\Traits\ChecksEntityStatus; use App\Utils\Traits\MakesHash; +use Illuminate\Validation\Rule; class UpdateVendorRequest extends Request { @@ -35,18 +36,15 @@ class UpdateVendorRequest extends Request /* Ensure we have a client name, and that all emails are unique*/ $rules['country_id'] = 'integer|nullable'; - //$rules['id_number'] = 'unique:clients,id_number,,id,company_id,' . auth()->user()->company()->id; - $rules['id_number'] = 'unique:clients,id_number,'.$this->id.',id,company_id,'.$this->company_id; + + if($this->number) + $rules['number'] = Rule::unique('vendors')->where('company_id', auth()->user()->company()->id)->ignore($this->vendor->id); + + if($this->id_number) + $rules['id_number'] = Rule::unique('vendors')->where('company_id', auth()->user()->company()->id)->ignore($this->vendor->id); + $rules['contacts.*.email'] = 'nullable|distinct'; - $contacts = request('contacts'); - - if (is_array($contacts)) { - // for ($i = 0; $i < count($contacts); $i++) { - // // $rules['contacts.' . $i . '.email'] = 'nullable|email|unique:client_contacts,email,' . isset($contacts[$i]['id'].',company_id,'.$this->company_id); - // //$rules['contacts.' . $i . '.email'] = 'nullable|email'; - // } - } return $rules; } @@ -54,7 +52,6 @@ class UpdateVendorRequest extends Request public function messages() { return [ - 'unique' => ctrans('validation.unique', ['attribute' => 'email']), 'email' => ctrans('validation.email', ['attribute' => 'email']), 'name.required' => ctrans('validation.required', ['attribute' => 'name']), 'required' => ctrans('validation.required', ['attribute' => 'email']), From 9e0328757a3ab85bf833bd85122bd382910ec492 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 20 Mar 2021 10:06:44 +1100 Subject: [PATCH 04/10] Tests for number validation across entities --- .../Requests/Client/StoreClientRequest.php | 8 +- .../Requests/Expense/StoreExpenseRequest.php | 5 +- tests/Feature/ClientApiTest.php | 36 +++++++++ tests/Feature/CreditTest.php | 76 +++++++++++++++++++ tests/Feature/ExpenseApiTest.php | 41 +++++++++- tests/Feature/InvoiceTest.php | 11 ++- 6 files changed, 169 insertions(+), 8 deletions(-) diff --git a/app/Http/Requests/Client/StoreClientRequest.php b/app/Http/Requests/Client/StoreClientRequest.php index b9cb4f1a15d6..313ee11eb6ae 100644 --- a/app/Http/Requests/Client/StoreClientRequest.php +++ b/app/Http/Requests/Client/StoreClientRequest.php @@ -19,6 +19,7 @@ use App\Models\Client; use App\Models\GroupSetting; use App\Utils\Traits\MakesHash; use Illuminate\Support\Facades\Cache; +use Illuminate\Validation\Rule; class StoreClientRequest extends Request { @@ -48,7 +49,6 @@ class StoreClientRequest extends Request /* Ensure we have a client name, and that all emails are unique*/ //$rules['name'] = 'required|min:1'; - $rules['id_number'] = 'unique:clients,id_number,'.$this->id.',id,company_id,'.$this->company_id; $rules['settings'] = new ValidClientGroupSettingsRule(); $rules['contacts.*.email'] = 'bail|nullable|distinct|sometimes|email'; $rules['contacts.*.password'] = [ @@ -66,6 +66,10 @@ class StoreClientRequest extends Request $rules['hosted_clients'] = new CanStoreClientsRule($this->company_id); } + $rules['number'] = ['nullable',Rule::unique('clients')->where('company_id', auth()->user()->company()->id)]; + $rules['id_number'] = ['nullable',Rule::unique('clients')->where('company_id', auth()->user()->company()->id)]; + + return $rules; } @@ -122,7 +126,7 @@ class StoreClientRequest extends Request public function messages() { return [ - 'unique' => ctrans('validation.unique', ['attribute' => 'email']), + // 'unique' => ctrans('validation.unique', ['attribute' => ['email','number']), //'required' => trans('validation.required', ['attribute' => 'email']), 'contacts.*.email.required' => ctrans('validation.email', ['attribute' => 'email']), ]; diff --git a/app/Http/Requests/Expense/StoreExpenseRequest.php b/app/Http/Requests/Expense/StoreExpenseRequest.php index 828217b4ee63..3a4726867d97 100644 --- a/app/Http/Requests/Expense/StoreExpenseRequest.php +++ b/app/Http/Requests/Expense/StoreExpenseRequest.php @@ -35,10 +35,9 @@ class StoreExpenseRequest extends Request { $rules = []; - if (isset($this->number)) { + if ($this->number) $rules['number'] = Rule::unique('expenses')->where('company_id', auth()->user()->company()->id); - } - + if(!empty($this->client_id)) $rules['client_id'] = 'bail|sometimes|exists:clients,id,company_id,'.auth()->user()->company()->id; diff --git a/tests/Feature/ClientApiTest.php b/tests/Feature/ClientApiTest.php index 3c87a5e07633..affd7461d3c6 100644 --- a/tests/Feature/ClientApiTest.php +++ b/tests/Feature/ClientApiTest.php @@ -54,6 +54,28 @@ class ClientApiTest extends TestCase $response->assertStatus(200); } + public function testDuplicateNumberCatch() + { + $data = [ + 'name' => $this->faker->firstName, + 'number' => 'iamaduplicate', + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/clients', $data); + + $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/clients', $data); + + $response->assertStatus(302); + } + public function testClientPut() { $data = [ @@ -67,6 +89,20 @@ class ClientApiTest extends TestCase ])->put('/api/v1/clients/'.$this->encodePrimaryKey($this->client->id), $data); $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/clients/'.$this->encodePrimaryKey($this->client->id), $data); + + $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/clients/', $data); + + $response->assertStatus(302); } public function testClientGet() diff --git a/tests/Feature/CreditTest.php b/tests/Feature/CreditTest.php index 936a10b6978d..0562f2fd16a7 100644 --- a/tests/Feature/CreditTest.php +++ b/tests/Feature/CreditTest.php @@ -131,4 +131,80 @@ class CreditTest extends TestCase $response->assertStatus(200); } + + public function testDuplicateNumberCatch() + { + $data = [ + 'status_id' => 1, + 'number' => 'dfdfd', + 'discount' => 0, + 'is_amount_discount' => 1, + 'number' => '3434343', + 'public_notes' => 'notes', + 'is_deleted' => 0, + 'custom_value1' => 0, + 'custom_value2' => 0, + 'custom_value3' => 0, + 'custom_value4' => 0, + 'status' => 1, + 'client_id' => $this->encodePrimaryKey($this->client->id), + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/credits', $data); + + $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/credits', $data); + + $response->assertStatus(302); + } + + public function testCreditPut() + { + $data = [ + 'status_id' => 1, + 'number' => 'dfdfd', + 'discount' => 0, + 'is_amount_discount' => 1, + 'number' => '3434343', + 'public_notes' => 'notes', + 'is_deleted' => 0, + 'custom_value1' => 0, + 'custom_value2' => 0, + 'custom_value3' => 0, + 'custom_value4' => 0, + 'status' => 1, + 'client_id' => $this->encodePrimaryKey($this->client->id), + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/credits/'.$this->encodePrimaryKey($this->credit->id), $data); + + $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/credits/'.$this->encodePrimaryKey($this->credit->id), $data); + + $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/credits/', $data); + + $response->assertStatus(302); + } + + + } diff --git a/tests/Feature/ExpenseApiTest.php b/tests/Feature/ExpenseApiTest.php index f17aeb5362b7..70254f13b67a 100644 --- a/tests/Feature/ExpenseApiTest.php +++ b/tests/Feature/ExpenseApiTest.php @@ -57,11 +57,34 @@ class ExpenseApiTest extends TestCase $this->assertNotEmpty($arr['data']['number']); } + public function testDuplicateNumberCatch() + { + $data = [ + 'public_notes' => $this->faker->firstName, + 'number' => 'iamaduplicate', + ]; + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/expenses', $data); + + $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/expenses', $data); + + $response->assertStatus(302); + } + + public function testExpensePut() { $data = [ 'public_notes' => $this->faker->firstName, - 'id_number' => 'Coolio', + 'number' => 'Coolio', ]; $response = $this->withHeaders([ @@ -70,6 +93,22 @@ class ExpenseApiTest extends TestCase ])->put('/api/v1/expenses/'.$this->encodePrimaryKey($this->expense->id), $data); $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/expenses/'.$this->encodePrimaryKey($this->expense->id), $data); + + $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/expenses/', $data); + + $response->assertStatus(302); + + } public function testExpenseGet() diff --git a/tests/Feature/InvoiceTest.php b/tests/Feature/InvoiceTest.php index 8ad15adfc832..68341e10b631 100644 --- a/tests/Feature/InvoiceTest.php +++ b/tests/Feature/InvoiceTest.php @@ -125,8 +125,15 @@ class InvoiceTest extends TestCase ])->post('/api/v1/invoices/', $invoice) ->assertStatus(200); - //test that the same request should produce a validation error due - //to duplicate number being used. + + $arr = $response->json(); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/invoices/'.$arr['data']['id'], $invoice) + ->assertStatus(200); + $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->token, From a9233ba62fc05fa224eb32dcf8a1db51310f4c4e Mon Sep 17 00:00:00 2001 From: David Bomba Date: Sat, 20 Mar 2021 10:10:45 +1100 Subject: [PATCH 05/10] Tests for number validation across entities --- tests/Feature/PaymentTest.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/Feature/PaymentTest.php b/tests/Feature/PaymentTest.php index 74e89dbdc5a2..11a1387256af 100644 --- a/tests/Feature/PaymentTest.php +++ b/tests/Feature/PaymentTest.php @@ -104,6 +104,15 @@ class PaymentTest extends TestCase $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/payments/'.$this->encodePrimaryKey($Payment->id), $Payment->toArray()); + + $response->assertStatus(200); + + $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->token, From 7173ba2931d5ba1ef8698e52d8debe0273e10773 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 20 Mar 2021 11:16:29 +1100 Subject: [PATCH 06/10] catch project exceptions --- app/Http/Middleware/QueryLogging.php | 2 +- app/Models/Project.php | 1 + tests/Feature/ProjectApiTest.php | 22 ++++++++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/Http/Middleware/QueryLogging.php b/app/Http/Middleware/QueryLogging.php index 92aee1a2046b..b898814f6855 100644 --- a/app/Http/Middleware/QueryLogging.php +++ b/app/Http/Middleware/QueryLogging.php @@ -54,7 +54,7 @@ class QueryLogging nlog($request->method().' - '.$request->url().": $count queries - ".$time); // if($count > 50) - // Log::nlog($queries); + //nlog($queries); } } diff --git a/app/Models/Project.php b/app/Models/Project.php index 42b84cbbaa29..daf67ad2d1e8 100644 --- a/app/Models/Project.php +++ b/app/Models/Project.php @@ -37,6 +37,7 @@ class Project extends BaseModel 'custom_value4', 'assigned_user_id', 'color', + 'number', ]; public function getEntityType() diff --git a/tests/Feature/ProjectApiTest.php b/tests/Feature/ProjectApiTest.php index d843329482d7..a9062e742cab 100644 --- a/tests/Feature/ProjectApiTest.php +++ b/tests/Feature/ProjectApiTest.php @@ -16,6 +16,7 @@ use Illuminate\Foundation\Testing\DatabaseTransactions; use Illuminate\Support\Facades\Session; use Tests\MockAccountData; use Tests\TestCase; +use Illuminate\Validation\ValidationException; /** * @test @@ -56,6 +57,7 @@ class ProjectApiTest extends TestCase $data = [ 'name' => $this->faker->firstName, 'client_id' => $this->client->hashed_id, + 'number' => 'duplicate', ]; $response = $this->withHeaders([ @@ -64,6 +66,26 @@ class ProjectApiTest extends TestCase ])->post('/api/v1/projects', $data); $response->assertStatus(200); + + $arr = $response->json(); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/projects/'.$arr['data']['id'], $data)->assertStatus(200); + + try{ + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/projects', $data); + } + catch(ValidationException $e){ + $response->assertStatus(302); + } + + + } public function testProjectPut() From b74062b1cab8daee2c10b54452487d3d9eae8df2 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 20 Mar 2021 11:21:50 +1100 Subject: [PATCH 07/10] Number tests for quotes and recurring invoices --- tests/Feature/QuoteTest.php | 19 ++++++++++++++++++- tests/Feature/RecurringInvoiceTest.php | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/tests/Feature/QuoteTest.php b/tests/Feature/QuoteTest.php index a01e66bdca50..c452c57b19a2 100644 --- a/tests/Feature/QuoteTest.php +++ b/tests/Feature/QuoteTest.php @@ -86,7 +86,8 @@ class QuoteTest extends TestCase $quote_update = [ 'status_id' => Quote::STATUS_APPROVED, - // 'client_id' => $this->encodePrimaryKey($quote->client_id), + 'client_id' => $this->encodePrimaryKey($this->quote->client_id), + 'number' => 'Rando', ]; $this->assertNotNull($this->quote); @@ -98,6 +99,22 @@ class QuoteTest extends TestCase $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/quotes/'.$this->encodePrimaryKey($this->quote->id), $quote_update); + + $response->assertStatus(200); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/quotes/', $quote_update); + + $response->assertStatus(302); + + $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->token, diff --git a/tests/Feature/RecurringInvoiceTest.php b/tests/Feature/RecurringInvoiceTest.php index 47cbd5415223..ca2e84d439de 100644 --- a/tests/Feature/RecurringInvoiceTest.php +++ b/tests/Feature/RecurringInvoiceTest.php @@ -117,6 +117,7 @@ class RecurringInvoiceTest extends TestCase $RecurringInvoice_update = [ 'status_id' => RecurringInvoice::STATUS_DRAFT, 'client_id' => $this->encodePrimaryKey($RecurringInvoice->client_id), + 'number' => 'customnumber' ]; $this->assertNotNull($RecurringInvoice); @@ -127,6 +128,26 @@ class RecurringInvoiceTest extends TestCase ])->put('/api/v1/recurring_invoices/'.$this->encodePrimaryKey($RecurringInvoice->id), $RecurringInvoice_update) ->assertStatus(200); + $arr = $response->json(); + + $this->assertEquals('customnumber', $arr['data']['number']); + + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/recurring_invoices/'.$this->encodePrimaryKey($RecurringInvoice->id), $RecurringInvoice_update) + ->assertStatus(200); + + + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/recurring_invoices/', $RecurringInvoice_update) + ->assertStatus(302); + + $response = $this->withHeaders([ 'X-API-SECRET' => config('ninja.api_secret'), 'X-API-TOKEN' => $this->token, From 0b1edab660453b65c68c3a1d9cb5b37ab5f53c27 Mon Sep 17 00:00:00 2001 From: = Date: Sat, 20 Mar 2021 11:25:44 +1100 Subject: [PATCH 08/10] Task tests for numbers --- tests/Feature/TaskApiTest.php | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Feature/TaskApiTest.php b/tests/Feature/TaskApiTest.php index e9d07ae477d7..e401ed05e4f4 100644 --- a/tests/Feature/TaskApiTest.php +++ b/tests/Feature/TaskApiTest.php @@ -14,6 +14,7 @@ use App\Utils\Traits\MakesHash; use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Testing\DatabaseTransactions; use Illuminate\Support\Facades\Session; +use Illuminate\Validation\ValidationException; use Tests\MockAccountData; use Tests\TestCase; @@ -44,6 +45,7 @@ class TaskApiTest extends TestCase { $data = [ 'description' => $this->faker->firstName, + 'number' => 'taskynumber' ]; $response = $this->withHeaders([ @@ -54,6 +56,27 @@ class TaskApiTest extends TestCase $arr = $response->json(); $response->assertStatus(200); + $this->assertEquals('taskynumber', $arr['data']['number']); + + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/tasks/'.$arr['data']['id'], $data); + + $response->assertStatus(200); + + try{ + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/tasks', $data); + + $arr = $response->json(); + }catch(ValidationException $e){ + $response->assertStatus(302); + } + $this->assertNotEmpty($arr['data']['number']); } From 32d9c4109eebc4d12d1c9d7d4588a1a272ae5cff Mon Sep 17 00:00:00 2001 From: = Date: Sat, 20 Mar 2021 11:28:39 +1100 Subject: [PATCH 09/10] Vendor number tests --- tests/Feature/VendorApiTest.php | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/Feature/VendorApiTest.php b/tests/Feature/VendorApiTest.php index cd8983413200..d4288c8b5f3c 100644 --- a/tests/Feature/VendorApiTest.php +++ b/tests/Feature/VendorApiTest.php @@ -14,6 +14,7 @@ use App\Utils\Traits\MakesHash; use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Testing\DatabaseTransactions; use Illuminate\Support\Facades\Session; +use Illuminate\Validation\ValidationException; use Tests\MockAccountData; use Tests\TestCase; @@ -59,6 +60,7 @@ class VendorApiTest extends TestCase $data = [ 'name' => $this->faker->firstName, 'id_number' => 'Coolio', + 'number' => 'wiggles' ]; $response = $this->withHeaders([ @@ -67,6 +69,33 @@ class VendorApiTest extends TestCase ])->put('/api/v1/vendors/'.$this->encodePrimaryKey($this->vendor->id), $data); $response->assertStatus(200); + + $arr = $response->json(); + + $this->assertEquals('Coolio', $arr['data']['id_number']); + $this->assertEquals('wiggles', $arr['data']['number']); + + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->put('/api/v1/vendors/'.$this->encodePrimaryKey($this->vendor->id), $data); + + $response->assertStatus(200); + + + try{ + $response = $this->withHeaders([ + 'X-API-SECRET' => config('ninja.api_secret'), + 'X-API-TOKEN' => $this->token, + ])->post('/api/v1/vendors/', $data); + + + }catch(ValidationException $e){ + + $response->assertStatus(302); + + } + } public function testVendorGet() From b158fb430f9ad660dd2fb6b4c65730aee7918c1c Mon Sep 17 00:00:00 2001 From: = Date: Sat, 20 Mar 2021 11:41:41 +1100 Subject: [PATCH 10/10] Small fixes for 2FA --- app/Http/Controllers/Auth/LoginController.php | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 7ac6dcee34d3..1a98f29dd0b7 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -171,7 +171,7 @@ class LoginController extends BaseController //if user has 2fa enabled - lets check this now: - if($user->google_2fa_secret && $request->has('one_time_password')) + if($user->google_2fa_secret && $request->has('one_time_password') && strlen($request->input('one_time_password')) >= 1) { $google2fa = new Google2FA(); @@ -194,6 +194,7 @@ class LoginController extends BaseController $user->setCompany($user->account->default_company); $timeout = auth()->user()->company()->default_password_timeout; + Cache::put(auth()->user()->hashed_id.'_logged_in', Str::random(64), $timeout); $cu = CompanyUser::query() @@ -322,33 +323,34 @@ class LoginController extends BaseController if ($user) { - $client = new Google_Client(); - $client->setClientId(config('ninja.auth.google.client_id')); - $client->setClientSecret(config('ninja.auth.google.client_secret')); - $client->setRedirectUri(config('ninja.app_url')); + // we are no longer accessing the permissions for gmail - email permissions here - $token = false; + // $client = new Google_Client(); + // $client->setClientId(config('ninja.auth.google.client_id')); + // $client->setClientSecret(config('ninja.auth.google.client_secret')); + // $client->setRedirectUri(config('ninja.app_url')); - try{ - $token = $client->authenticate(request()->input('server_auth_code')); - } - catch(\Exception $e) { + // $token = false; - return response() - ->json(['message' => ctrans('texts.invalid_credentials')], 401) - ->header('X-App-Version', config('ninja.app_version')) - ->header('X-Api-Version', config('ninja.minimum_client_version')); + // try{ + // $token = $client->authenticate(request()->input('server_auth_code')); + // } + // catch(\Exception $e) { - } + // return response() + // ->json(['message' => ctrans('texts.invalid_credentials')], 401) + // ->header('X-App-Version', config('ninja.app_version')) + // ->header('X-Api-Version', config('ninja.minimum_client_version')); - $refresh_token = ''; + // } - if (array_key_exists('refresh_token', $token)) { - $refresh_token = $token['refresh_token']; - } + // $refresh_token = ''; - //$access_token = $token['access_token']; + // if (array_key_exists('refresh_token', $token)) { + // $refresh_token = $token['refresh_token']; + // } + $name = OAuth::splitName($google->harvestName($user)); $new_account = [