diff --git a/app/Models/Customer.php b/app/Models/Customer.php index 47eb2d65..876fb756 100644 --- a/app/Models/Customer.php +++ b/app/Models/Customer.php @@ -3,6 +3,7 @@ namespace App\Models; use App\Notifications\CustomerMailResetPasswordNotification; +use App\Support\SafeOrderBy; use App\Traits\HasCustomFieldsTrait; use Carbon\Carbon; use Illuminate\Database\Eloquent\Factories\HasFactory; @@ -269,7 +270,7 @@ class Customer extends Authenticatable implements HasMedia public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeWhereSearch($query, $search) diff --git a/app/Models/Estimate.php b/app/Models/Estimate.php index 353996fc..8ebe1f15 100644 --- a/app/Models/Estimate.php +++ b/app/Models/Estimate.php @@ -8,6 +8,7 @@ use App\Facades\PDF; use App\Mail\SendEstimateMail; use App\Services\SerialNumberFormatter; use App\Space\PdfTemplateUtils; +use App\Support\SafeOrderBy; use App\Traits\GeneratesPdfTrait; use App\Traits\HasCustomFieldsTrait; use Carbon\Carbon; @@ -194,7 +195,7 @@ class Estimate extends Model implements HasMedia public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeWhereCompany($query) diff --git a/app/Models/Expense.php b/app/Models/Expense.php index dfaa0da8..d7de1613 100644 --- a/app/Models/Expense.php +++ b/app/Models/Expense.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Support\SafeOrderBy; use App\Traits\HasCustomFieldsTrait; use Carbon\Carbon; use Illuminate\Database\Eloquent\Factories\HasFactory; @@ -200,7 +201,7 @@ class Expense extends Model implements HasMedia public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeWhereCompany($query) diff --git a/app/Models/FileDisk.php b/app/Models/FileDisk.php index 0755dcce..5a5b8e33 100644 --- a/app/Models/FileDisk.php +++ b/app/Models/FileDisk.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Support\SafeOrderBy; use Carbon\Carbon; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; @@ -32,7 +33,7 @@ class FileDisk extends Model public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeFileDisksBetween($query, $start, $end) diff --git a/app/Models/Invoice.php b/app/Models/Invoice.php index fb13ce52..e92f24ec 100644 --- a/app/Models/Invoice.php +++ b/app/Models/Invoice.php @@ -8,6 +8,7 @@ use App\Facades\PDF; use App\Mail\SendInvoiceMail; use App\Services\SerialNumberFormatter; use App\Space\PdfTemplateUtils; +use App\Support\SafeOrderBy; use App\Traits\GeneratesPdfTrait; use App\Traits\HasCustomFieldsTrait; use Carbon\Carbon; @@ -259,7 +260,7 @@ class Invoice extends Model implements HasMedia public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeApplyFilters($query, array $filters) @@ -288,7 +289,8 @@ class Invoice extends Model implements HasMedia $query->where('customer_id', $customerId); })->when($filters['orderByField'] ?? null, function ($query, $orderByField) use ($filters) { $orderBy = $filters['orderBy'] ?? 'desc'; - $query->orderBy($orderByField, $orderBy); + + return SafeOrderBy::apply($query, $orderByField, $orderBy); }, function ($query) { $query->orderBy('sequence_number', 'desc'); }); diff --git a/app/Models/Item.php b/app/Models/Item.php index b53b9a18..dda3bfd6 100644 --- a/app/Models/Item.php +++ b/app/Models/Item.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Support\SafeOrderBy; use Carbon\Carbon; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; @@ -63,7 +64,7 @@ class Item extends Model public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeWhereItem($query, $item_id) diff --git a/app/Models/Payment.php b/app/Models/Payment.php index b52a1e58..cf019f91 100644 --- a/app/Models/Payment.php +++ b/app/Models/Payment.php @@ -6,6 +6,7 @@ use App\Facades\Hashids; use App\Jobs\GeneratePaymentPdfJob; use App\Mail\SendPaymentMail; use App\Services\SerialNumberFormatter; +use App\Support\SafeOrderBy; use App\Traits\GeneratesPdfTrait; use App\Traits\HasCustomFieldsTrait; use Barryvdh\DomPDF\Facade\Pdf as PDF; @@ -356,7 +357,7 @@ class Payment extends Model implements HasMedia public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeWherePayment($query, $payment_id) diff --git a/app/Models/RecurringInvoice.php b/app/Models/RecurringInvoice.php index 5d910425..d44f5e36 100644 --- a/app/Models/RecurringInvoice.php +++ b/app/Models/RecurringInvoice.php @@ -5,6 +5,7 @@ namespace App\Models; use App\Facades\Hashids; use App\Http\Requests\RecurringInvoiceRequest; use App\Services\SerialNumberFormatter; +use App\Support\SafeOrderBy; use App\Traits\HasCustomFieldsTrait; use Carbon\Carbon; use Cron; @@ -132,7 +133,7 @@ class RecurringInvoice extends Model public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeWhereStatus($query, $status) diff --git a/app/Models/TaxType.php b/app/Models/TaxType.php index 85480f0e..cd12bf43 100644 --- a/app/Models/TaxType.php +++ b/app/Models/TaxType.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\Support\SafeOrderBy; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -73,7 +74,7 @@ class TaxType extends Model public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeWhereSearch($query, $search) diff --git a/app/Models/User.php b/app/Models/User.php index c778ab93..28294080 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -4,6 +4,7 @@ namespace App\Models; use App\Http\Requests\UserRequest; use App\Notifications\MailResetPasswordNotification; +use App\Support\SafeOrderBy; use App\Traits\HasCustomFieldsTrait; use Carbon\Carbon; use Illuminate\Database\Eloquent\Factories\HasFactory; @@ -179,7 +180,7 @@ class User extends Authenticatable implements HasMedia public function scopeWhereOrder($query, $orderByField, $orderBy) { - $query->orderBy($orderByField, $orderBy); + return SafeOrderBy::apply($query, $orderByField, $orderBy); } public function scopeWhereSearch($query, $search) diff --git a/app/Support/SafeOrderBy.php b/app/Support/SafeOrderBy.php new file mode 100644 index 00000000..f0793e0d --- /dev/null +++ b/app/Support/SafeOrderBy.php @@ -0,0 +1,31 @@ +orderBy($field, $direction); + } +} diff --git a/tests/Unit/SafeOrderByTest.php b/tests/Unit/SafeOrderByTest.php new file mode 100644 index 00000000..8734dc9e --- /dev/null +++ b/tests/Unit/SafeOrderByTest.php @@ -0,0 +1,32 @@ +toSql() + ); + + expect($sql)->toContain('order by') + ->and($sql)->not->toContain('case') + ->and($sql)->not->toContain('select 1'); +}); + +test('allows a plain column', function () { + $sql = strtolower(SafeOrderBy::apply(DB::table('invoices'), 'invoice_date', 'asc')->toSql()); + + expect($sql)->toContain('order by')->and($sql)->toContain('invoice_date'); +}); + +test('allows a table-qualified column so joined/aliased sorts keep working', function () { + $sql = strtolower(SafeOrderBy::apply(DB::table('estimates'), 'customers.name', 'asc')->toSql()); + + expect($sql)->toContain('customers')->and($sql)->toContain('name'); +}); + +test('clamps the direction to asc/desc', function () { + $sql = strtolower(SafeOrderBy::apply(DB::table('invoices'), 'id', 'asc); drop table users; --')->toSql()); + + expect($sql)->toContain('order by')->and($sql)->not->toContain('drop table'); +});