From 5839d8385debf379644f55e773639e1c7e673163 Mon Sep 17 00:00:00 2001 From: Darko Gjorgjijoski <5760249+gdarko@users.noreply.github.com> Date: Fri, 12 Jun 2026 09:18:16 +0200 Subject: [PATCH] fix(security): harden public EmailLog token endpoints (GHSA-73q7) (#662) Customer PDF controllers resolved the target document by raw mailable_id, ignoring mailable_type, and skipped the expiry check on the JSON endpoints. - Resolve via the $emailLog->mailable morph relation and assert the expected type (abort 404) so a token issued for one document type can't disclose another whose numeric id collides. - Enforce isExpired() (abort 403) on every public path, including the JSON getInvoice/getEstimate/getPayment endpoints. - Harden EmailLog::isExpired() to treat a null/unresolvable mailable as expired instead of throwing. Adds tests for cross-type 404, JSON-path expiry 403, and the valid path. --- .../V1/Customer/EstimatePdfController.php | 42 ++++++------- .../V1/Customer/InvoicePdfController.php | 61 ++++++++++--------- .../V1/Customer/PaymentPdfController.php | 12 ++-- app/Models/EmailLog.php | 12 +++- tests/Feature/Customer/EmailLogTokenTest.php | 61 +++++++++++++++++++ 5 files changed, 131 insertions(+), 57 deletions(-) create mode 100644 tests/Feature/Customer/EmailLogTokenTest.php diff --git a/app/Http/Controllers/V1/Customer/EstimatePdfController.php b/app/Http/Controllers/V1/Customer/EstimatePdfController.php index be2b59ed..ccb54e15 100644 --- a/app/Http/Controllers/V1/Customer/EstimatePdfController.php +++ b/app/Http/Controllers/V1/Customer/EstimatePdfController.php @@ -15,38 +15,38 @@ class EstimatePdfController extends Controller { public function getPdf(EmailLog $emailLog, Request $request) { - $estimate = Estimate::find($emailLog->mailable_id); + $estimate = $emailLog->mailable; + abort_unless($estimate instanceof Estimate, 404); + abort_if($emailLog->isExpired(), 403, 'Link Expired.'); - if (! $emailLog->isExpired()) { - if ($estimate && ($estimate->status == Estimate::STATUS_SENT || $estimate->status == Estimate::STATUS_DRAFT)) { - $estimate->status = Estimate::STATUS_VIEWED; - $estimate->save(); - $notifyEstimateViewed = CompanySetting::getSetting( - 'notify_estimate_viewed', + if ($estimate->status == Estimate::STATUS_SENT || $estimate->status == Estimate::STATUS_DRAFT) { + $estimate->status = Estimate::STATUS_VIEWED; + $estimate->save(); + $notifyEstimateViewed = CompanySetting::getSetting( + 'notify_estimate_viewed', + $estimate->company_id + ); + + if ($notifyEstimateViewed == 'YES') { + $data['estimate'] = Estimate::findOrFail($estimate->id)->toArray(); + $data['user'] = Customer::find($estimate->customer_id)->toArray(); + $notificationEmail = CompanySetting::getSetting( + 'notification_email', $estimate->company_id ); - if ($notifyEstimateViewed == 'YES') { - $data['estimate'] = Estimate::findOrFail($estimate->id)->toArray(); - $data['user'] = Customer::find($estimate->customer_id)->toArray(); - $notificationEmail = CompanySetting::getSetting( - 'notification_email', - $estimate->company_id - ); - - \Mail::to($notificationEmail)->send(new EstimateViewedMail($data)); - } + \Mail::to($notificationEmail)->send(new EstimateViewedMail($data)); } - - return $estimate->getGeneratedPDFOrStream('estimate'); } - abort(403, 'Link Expired.'); + return $estimate->getGeneratedPDFOrStream('estimate'); } public function getEstimate(EmailLog $emailLog) { - $estimate = Estimate::find($emailLog->mailable_id); + $estimate = $emailLog->mailable; + abort_unless($estimate instanceof Estimate, 404); + abort_if($emailLog->isExpired(), 403, 'Link Expired.'); return new EstimateResource($estimate); } diff --git a/app/Http/Controllers/V1/Customer/InvoicePdfController.php b/app/Http/Controllers/V1/Customer/InvoicePdfController.php index 329d3565..84cf3b1d 100644 --- a/app/Http/Controllers/V1/Customer/InvoicePdfController.php +++ b/app/Http/Controllers/V1/Customer/InvoicePdfController.php @@ -15,46 +15,49 @@ class InvoicePdfController extends Controller { public function getPdf(EmailLog $emailLog, Request $request) { - $invoice = Invoice::find($emailLog->mailable_id); + // Resolve the document through the morph relation and enforce the + // expected type — a token issued for another mailable type (or whose + // numeric id collides) must not disclose an invoice. + $invoice = $emailLog->mailable; + abort_unless($invoice instanceof Invoice, 404); + abort_if($emailLog->isExpired(), 403, 'Link Expired.'); - if (! $emailLog->isExpired()) { - if ($invoice && ($invoice->status == Invoice::STATUS_SENT || $invoice->status == Invoice::STATUS_DRAFT)) { - $invoice->status = Invoice::STATUS_VIEWED; - $invoice->viewed = true; - $invoice->save(); - $notifyInvoiceViewed = CompanySetting::getSetting( - 'notify_invoice_viewed', + if ($invoice->status == Invoice::STATUS_SENT || $invoice->status == Invoice::STATUS_DRAFT) { + $invoice->status = Invoice::STATUS_VIEWED; + $invoice->viewed = true; + $invoice->save(); + $notifyInvoiceViewed = CompanySetting::getSetting( + 'notify_invoice_viewed', + $invoice->company_id + ); + + if ($notifyInvoiceViewed == 'YES') { + $data['invoice'] = Invoice::findOrFail($invoice->id)->toArray(); + $data['user'] = Customer::find($invoice->customer_id)->toArray(); + $notificationEmail = CompanySetting::getSetting( + 'notification_email', $invoice->company_id ); - if ($notifyInvoiceViewed == 'YES') { - $data['invoice'] = Invoice::findOrFail($invoice->id)->toArray(); - $data['user'] = Customer::find($invoice->customer_id)->toArray(); - $notificationEmail = CompanySetting::getSetting( - 'notification_email', - $invoice->company_id - ); - - \Mail::to($notificationEmail)->send(new InvoiceViewedMail($data)); - } + \Mail::to($notificationEmail)->send(new InvoiceViewedMail($data)); } - - if ($request->has('pdf')) { - return $invoice->getGeneratedPDFOrStream('invoice'); - } - - return view('app')->with([ - 'customer_logo' => get_company_setting('customer_portal_logo', $invoice->company_id), - 'current_theme' => get_company_setting('customer_portal_theme', $invoice->company_id), - ]); } - abort(403, 'Link Expired.'); + if ($request->has('pdf')) { + return $invoice->getGeneratedPDFOrStream('invoice'); + } + + return view('app')->with([ + 'customer_logo' => get_company_setting('customer_portal_logo', $invoice->company_id), + 'current_theme' => get_company_setting('customer_portal_theme', $invoice->company_id), + ]); } public function getInvoice(EmailLog $emailLog) { - $invoice = Invoice::find($emailLog->mailable_id); + $invoice = $emailLog->mailable; + abort_unless($invoice instanceof Invoice, 404); + abort_if($emailLog->isExpired(), 403, 'Link Expired.'); return new CustomerInvoiceResource($invoice); } diff --git a/app/Http/Controllers/V1/Customer/PaymentPdfController.php b/app/Http/Controllers/V1/Customer/PaymentPdfController.php index bf6ea7d5..eed03e01 100644 --- a/app/Http/Controllers/V1/Customer/PaymentPdfController.php +++ b/app/Http/Controllers/V1/Customer/PaymentPdfController.php @@ -12,16 +12,18 @@ class PaymentPdfController extends Controller { public function getPdf(EmailLog $emailLog, Request $request) { - if (! $emailLog->isExpired()) { - return $emailLog->mailable->getGeneratedPDFOrStream('payment'); - } + $payment = $emailLog->mailable; + abort_unless($payment instanceof Payment, 404); + abort_if($emailLog->isExpired(), 403, 'Link Expired.'); - abort(403, 'Link Expired.'); + return $payment->getGeneratedPDFOrStream('payment'); } public function getPayment(EmailLog $emailLog) { - $payment = Payment::find($emailLog->mailable_id); + $payment = $emailLog->mailable; + abort_unless($payment instanceof Payment, 404); + abort_if($emailLog->isExpired(), 403, 'Link Expired.'); return new PaymentResource($payment); } diff --git a/app/Models/EmailLog.php b/app/Models/EmailLog.php index 4a456013..d650381e 100644 --- a/app/Models/EmailLog.php +++ b/app/Models/EmailLog.php @@ -20,8 +20,16 @@ class EmailLog extends Model public function isExpired() { - $linkExpiryDays = (int) CompanySetting::getSetting('link_expiry_days', $this->mailable()->get()->toArray()[0]['company_id']); - $checkExpiryLinks = CompanySetting::getSetting('automatically_expire_public_links', $this->mailable()->get()->toArray()[0]['company_id']); + $mailable = $this->mailable; + + // A token whose target document no longer resolves is treated as + // expired/invalid rather than throwing. + if (! $mailable) { + return true; + } + + $linkExpiryDays = (int) CompanySetting::getSetting('link_expiry_days', $mailable->company_id); + $checkExpiryLinks = CompanySetting::getSetting('automatically_expire_public_links', $mailable->company_id); $expiryDate = $this->created_at->addDays($linkExpiryDays); diff --git a/tests/Feature/Customer/EmailLogTokenTest.php b/tests/Feature/Customer/EmailLogTokenTest.php new file mode 100644 index 00000000..84cd4ece --- /dev/null +++ b/tests/Feature/Customer/EmailLogTokenTest.php @@ -0,0 +1,61 @@ + 'DatabaseSeeder', '--force' => true]); + Artisan::call('db:seed', ['--class' => 'DemoSeeder', '--force' => true]); +}); + +test('email-log token enforces the mailable type (no cross-type disclosure)', function () { + $payment = Payment::factory()->create(); + + // Token issued for a Payment must not resolve a document on the invoice + // or estimate routes, even if the numeric id collides. + $emailLog = EmailLog::factory()->create([ + 'mailable_type' => Payment::class, + 'mailable_id' => $payment->id, + 'token' => 'tok-type-confusion', + ]); + + get('/customer/invoices/'.$emailLog->token)->assertNotFound(); + get('/customer/estimates/'.$emailLog->token)->assertNotFound(); +}); + +test('json invoice endpoint enforces link expiry', function () { + $invoice = Invoice::factory()->create(); + + CompanySetting::setSettings([ + 'automatically_expire_public_links' => 'YES', + 'link_expiry_days' => '1', + ], $invoice->company_id); + + $emailLog = EmailLog::factory()->create([ + 'mailable_type' => Invoice::class, + 'mailable_id' => $invoice->id, + 'token' => 'tok-expired', + 'created_at' => now()->subDays(5), + ]); + + get('/customer/invoices/'.$emailLog->token)->assertForbidden(); +}); + +test('json invoice endpoint returns the invoice for a valid token', function () { + $invoice = Invoice::factory()->create(); + + $emailLog = EmailLog::factory()->create([ + 'mailable_type' => Invoice::class, + 'mailable_id' => $invoice->id, + 'token' => 'tok-valid', + ]); + + get('/customer/invoices/'.$emailLog->token)->assertOk(); +});