From d7d116e9a59f48d0805d0fa34a3d0383c1e186a7 Mon Sep 17 00:00:00 2001 From: nexxo Date: Mon, 3 Aug 2026 20:36:20 +0200 Subject: [PATCH] Jede Mail faehrt wieder ueber ihren eigenen Mailer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mail::to(...) loest den Standard-Mailer auf, und dessen queue() schreibt danach `$view->mailer($this->name)`. Damit war der im Konstruktor gewaehlte Mailer (cp_mail_) durch 'smtp' ersetzt, bevor der Auftrag ueberhaupt gebaut war. Mit dem Mailer fiel alles weg, was an ihm haengt: MailboxTransport, die Postfachsuche samt passendem Absender — und der Notschalter aus App\Support\MailDelivery, der genau dort sitzt. Eine abgeschaltete Zustellung schaltete deshalb nichts ab, und die Mails meldeten sich mit den Zugangsdaten aus der Konfiguration an, waehrend der Absender aus der Mail kam: „553 Sender address rejected". Das lief hier im Minutentakt. Zwei Stellen hatten es schon richtig — MaintenanceNotifier und MailPreview, beide mit der Erklaerung an der Zeile. Neun andere nicht. Alle beginnen jetzt mit Mail::mailer($mail->mailer), sodass das Ueberschreiben denselben Wert zurueckschreibt. Die Attrappen in drei Tests zielten auf to() und trafen damit nicht mehr den Einstieg; drei von ihnen bestanden ohnehin nur zufaellig, weil ein Aufruf auf null ebenfalls warf. Sie zielen jetzt auf mailer(). Erzwungen durch tests/Feature/MailDispatchTest.php: Mail::to( ist in app/ verboten, und ein Gegentest haelt das Framework-Verhalten fest, damit die Regel fallen darf, wenn Laravel sie eines Tages unnoetig macht. Co-Authored-By: Claude Opus 5 --- app/Actions/IssueStripeInvoice.php | 3 +- app/Actions/StartCustomerProvisioning.php | 6 +- app/Console/Commands/PruneDormantAccounts.php | 3 +- app/Listeners/RecordSignInDevice.php | 5 +- app/Livewire/Admin/CustomerDetail.php | 5 +- app/Models/User.php | 8 +- app/Services/Billing/DunningMailer.php | 2 +- app/Services/Security/BlockAddress.php | 6 +- tests/Feature/Billing/DunningMailTest.php | 2 +- tests/Feature/Billing/RenewalInvoiceTest.php | 4 +- tests/Feature/MailDispatchTest.php | 94 +++++++++++++++++++ .../Security/SecurityBlockMailTest.php | 2 +- 12 files changed, 121 insertions(+), 19 deletions(-) create mode 100644 tests/Feature/MailDispatchTest.php diff --git a/app/Actions/IssueStripeInvoice.php b/app/Actions/IssueStripeInvoice.php index b46dc3d..5e0073e 100644 --- a/app/Actions/IssueStripeInvoice.php +++ b/app/Actions/IssueStripeInvoice.php @@ -299,7 +299,8 @@ class IssueStripeInvoice } try { - Mail::to($address)->queue(new InvoiceMail($invoice, (string) $customer->name)); + $mail = new InvoiceMail($invoice, (string) $customer->name); + Mail::mailer($mail->mailer)->to($address)->queue($mail); // Only once it is actually with the mailer. Stamped so a redelivery // does not send the same invoice a second time, and left unstamped diff --git a/app/Actions/StartCustomerProvisioning.php b/app/Actions/StartCustomerProvisioning.php index dd2f2aa..03fdc16 100644 --- a/app/Actions/StartCustomerProvisioning.php +++ b/app/Actions/StartCustomerProvisioning.php @@ -178,7 +178,8 @@ class StartCustomerProvisioning // already gone out, and Stripe redelivers until it gets a 2xx. The invoice // mail is the one a crash must not lose, and that one is stamped. try { - Mail::to($address)->queue(new OrderConfirmationMail($order, (string) $customer->name)); + $mail = new OrderConfirmationMail($order, (string) $customer->name); + Mail::mailer($mail->mailer)->to($address)->queue($mail); } catch (Throwable $e) { Log::warning('Could not queue the purchase confirmation', [ 'order' => $order->id, @@ -255,7 +256,8 @@ class StartCustomerProvisioning } try { - Mail::to($address)->queue(new InvoiceMail($invoice, (string) $customer->name)); + $mail = new InvoiceMail($invoice, (string) $customer->name); + Mail::mailer($mail->mailer)->to($address)->queue($mail); $invoice->update(['sent_at' => now()]); } catch (Throwable $e) { diff --git a/app/Console/Commands/PruneDormantAccounts.php b/app/Console/Commands/PruneDormantAccounts.php index 4a5362c..8037455 100644 --- a/app/Console/Commands/PruneDormantAccounts.php +++ b/app/Console/Commands/PruneDormantAccounts.php @@ -91,7 +91,8 @@ class PruneDormantAccounts extends Command } try { - Mail::to($user->email)->queue(new DormantAccountWarningMail($user, self::WARN_DAYS_BEFORE)); + $mail = new DormantAccountWarningMail($user, self::WARN_DAYS_BEFORE); + Mail::mailer($mail->mailer)->to($user->email)->queue($mail); } catch (Throwable $e) { // Not stamped: the stamp is what permits the deletion, and // stamping a mail that never went out would delete an account diff --git a/app/Listeners/RecordSignInDevice.php b/app/Listeners/RecordSignInDevice.php index 1449516..1461f69 100644 --- a/app/Listeners/RecordSignInDevice.php +++ b/app/Listeners/RecordSignInDevice.php @@ -51,11 +51,12 @@ class RecordSignInDevice // Queued: a sign-in must not wait on a mail server, and this one // runs while somebody is looking at a spinner on the login button. - Mail::to($address)->queue(new NewDeviceSignInMail( + $mail = new NewDeviceSignInMail( name: (string) ($event->user->getAttribute('name') ?? ''), device: $outcome->device, guard: $event->guard, - )); + ); + Mail::mailer($mail->mailer)->to($address)->queue($mail); } catch (Throwable $e) { Log::warning('Could not record the device for a sign-in', [ 'guard' => $event->guard, diff --git a/app/Livewire/Admin/CustomerDetail.php b/app/Livewire/Admin/CustomerDetail.php index aa02b1d..e5d9654 100644 --- a/app/Livewire/Admin/CustomerDetail.php +++ b/app/Livewire/Admin/CustomerDetail.php @@ -194,9 +194,8 @@ class CustomerDetail extends Component $data = $this->validate(); - Mail::to($this->customer->email)->send( - new OperatorMessageMail($this->customer, $data['subject'], $data['body']) - ); + $mail = new OperatorMessageMail($this->customer, $data['subject'], $data['body']); + Mail::mailer($mail->mailer)->to($this->customer->email)->send($mail); SentMail::query() ->where('customer_id', $this->customer->id) diff --git a/app/Models/User.php b/app/Models/User.php index 9ee8281..52e3001 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -30,11 +30,12 @@ class User extends Authenticatable implements MustVerifyEmail */ public function sendPasswordResetNotification(#[\SensitiveParameter] $token): void { - \Illuminate\Support\Facades\Mail::to($this->email)->send(new \App\Mail\ResetPasswordMail( + $mail = new \App\Mail\ResetPasswordMail( $this, route('password.reset', ['token' => $token, 'email' => $this->email]), (int) config('auth.passwords.users.expire', 60), - )); + ); + \Illuminate\Support\Facades\Mail::mailer($mail->mailer)->to($this->email)->send($mail); } /** @@ -52,7 +53,8 @@ class User extends Authenticatable implements MustVerifyEmail */ public function sendEmailVerificationNotification(): void { - Mail::to($this->getEmailForVerification())->queue(new VerifyEmailMail($this)); + $mail = new VerifyEmailMail($this); + Mail::mailer($mail->mailer)->to($this->getEmailForVerification())->queue($mail); } /** diff --git a/app/Services/Billing/DunningMailer.php b/app/Services/Billing/DunningMailer.php index a601af6..d22434b 100644 --- a/app/Services/Billing/DunningMailer.php +++ b/app/Services/Billing/DunningMailer.php @@ -102,7 +102,7 @@ class DunningMailer private function send(string $address, object $mailable): void { try { - Mail::to($address)->queue($mailable); + Mail::mailer($mailable->mailer)->to($address)->queue($mailable); } catch (Throwable $e) { // Siehe Kopfkommentar: laut, aber nicht kippend. Log::error('Eine Mahnungs-Mail liess sich nicht einreihen', [ diff --git a/app/Services/Security/BlockAddress.php b/app/Services/Security/BlockAddress.php index dd7801b..75008b4 100644 --- a/app/Services/Security/BlockAddress.php +++ b/app/Services/Security/BlockAddress.php @@ -119,7 +119,8 @@ class BlockAddress } try { - Mail::to($instance->customer->email)->queue(new SecurityBlockMail($block)); + $mail = new SecurityBlockMail($block); + Mail::mailer($mail->mailer)->to($instance->customer->email)->queue($mail); Settings::set($key, now()->toIso8601String()); } catch (Throwable $e) { report($e); @@ -177,7 +178,8 @@ class BlockAddress foreach ($recipients as $email) { try { - Mail::to($email)->queue(new SecurityBlockMail($block)); + $mail = new SecurityBlockMail($block); + Mail::mailer($mail->mailer)->to($email)->queue($mail); $queued = true; } catch (Throwable $e) { report($e); diff --git a/tests/Feature/Billing/DunningMailTest.php b/tests/Feature/Billing/DunningMailTest.php index ab80beb..746c1ed 100644 --- a/tests/Feature/Billing/DunningMailTest.php +++ b/tests/Feature/Billing/DunningMailTest.php @@ -169,7 +169,7 @@ it('advances the case even when the mail cannot be sent', function () { app()->instance(StripeClient::class, new FakeStripeClient); app()->instance(ProxmoxClient::class, new FakeProxmoxClient); - Mail::shouldReceive('to')->andThrow(new RuntimeException('kein Mailserver')); + Mail::shouldReceive('mailer')->andThrow(new RuntimeException('kein Mailserver')); $this->artisan('clupilot:advance-dunning')->assertSuccessful(); diff --git a/tests/Feature/Billing/RenewalInvoiceTest.php b/tests/Feature/Billing/RenewalInvoiceTest.php index 8d7a326..57b1bf4 100644 --- a/tests/Feature/Billing/RenewalInvoiceTest.php +++ b/tests/Feature/Billing/RenewalInvoiceTest.php @@ -143,7 +143,7 @@ it('does not fail the webhook when the mail cannot be sent, and leaves the invoi renewingContract(); // A mail server that is down, mid-send. - Mail::shouldReceive('to')->andThrow(new RuntimeException('Connection refused')); + Mail::shouldReceive('mailer')->andThrow(new RuntimeException('Connection refused')); $this->postJson(route('webhooks.stripe'), renewalPaid())->assertOk(); @@ -159,7 +159,7 @@ it('sends the invoice a second delivery finds unsent, without issuing a second d renewingContract(); $mailer = Mail::getFacadeRoot(); - Mail::shouldReceive('to')->once()->andThrow(new RuntimeException('Connection refused')); + Mail::shouldReceive('mailer')->once()->andThrow(new RuntimeException('Connection refused')); $this->postJson(route('webhooks.stripe'), renewalPaid())->assertOk(); // Stripe redelivers, and the mail server is back. The register entry is diff --git a/tests/Feature/MailDispatchTest.php b/tests/Feature/MailDispatchTest.php new file mode 100644 index 0000000..40633cc --- /dev/null +++ b/tests/Feature/MailDispatchTest.php @@ -0,0 +1,94 @@ +` + * oder `cp_`. Dahinter steht MailboxTransport: er sucht das Postfach, + * dessen Adresse im Absender steht, meldet sich mit genau dessen Zugangsdaten + * an, und er ist die Stelle, an der der Notschalter aus App\Support\MailDelivery + * greift. + * + * `Mail::to(…)` löst den STANDARD-Mailer auf. Dessen queue() schreibt dann + * `$view->mailer($this->name)` — der eigene Mailer der Mail wird durch 'smtp' + * ersetzt, bevor der Auftrag überhaupt gebaut ist. Was danach abgeschickt wird, + * kennt weder Postfach noch Schalter: es meldet sich mit den Zugangsdaten aus + * der Konfiguration an, setzt aber den Absender aus der Mail, und ein Server, + * der prüft, wem die Absenderadresse gehört, weist es ab („553 Sender address + * rejected"). Genau das lief hier tagelang im Minutentakt — und eine + * abgeschaltete Zustellung schaltete nichts ab. + * + * Deshalb beginnt jeder Versand mit `Mail::mailer($mail->mailer)`: dann + * schreibt das Überschreiben denselben Wert zurück, den die Mail schon hatte. + */ +it('keeps the mailable own mailer when it is queued through Mail::mailer', function () { + Queue::fake(); + + $mail = new DormantAccountWarningMail(User::factory()->create(), 30); + $chosen = $mail->mailer; + + expect($chosen)->not->toBeNull() + ->and($chosen)->not->toBe(config('mail.default')); + + Mail::mailer($mail->mailer)->to('empfaenger@example.com')->queue($mail); + + Queue::assertPushed( + PacedMail::class, + fn (PacedMail $job) => $job->mailable->mailer === $chosen, + ); +}); + +/** + * Der Gegenbeweis, damit die Regel oben nicht als Aberglaube gilt: dieselbe + * Mail, ohne den benannten Mailer eingereiht, verliert ihn. Schlägt dieser Test + * fehl, hat das Framework sein Verhalten geändert — dann darf die Regel weg, + * vorher nicht. + */ +it('proves that Mail::to alone overwrites the mailable own mailer', function () { + Queue::fake(); + + $mail = new DormantAccountWarningMail(User::factory()->create(), 30); + $chosen = $mail->mailer; + + Mail::to('empfaenger@example.com')->queue($mail); + + Queue::assertPushed( + PacedMail::class, + fn (PacedMail $job) => $job->mailable->mailer === config('mail.default') + && $job->mailable->mailer !== $chosen, + ); +}); + +it('never dispatches a mail through Mail::to without naming the mailer', function () { + $offenders = []; + + foreach (File::allFiles(app_path()) as $file) { + if ($file->getExtension() !== 'php') { + continue; + } + + foreach (explode("\n", $file->getContents()) as $number => $line) { + // Kommentare beschreiben die Falle — zwei Stellen im Repo tun genau + // das an der Zeile, an der sie umgangen wird. Sie sind der Grund + // für diese Regel, nicht ihr Verstoß. + if (preg_match('#^\s*(//|\*|/\*)#', $line) === 1) { + continue; + } + + // Trifft beide Schreibweisen: `Mail::to(` und die voll + // qualifizierte `\Illuminate\Support\Facades\Mail::to(`. + if (preg_match('/(?getPathname()).':'.($number + 1); + } + } + } + + expect($offenders)->toBe([]); +}); diff --git a/tests/Feature/Security/SecurityBlockMailTest.php b/tests/Feature/Security/SecurityBlockMailTest.php index 2d7c6c5..a6a27e4 100644 --- a/tests/Feature/Security/SecurityBlockMailTest.php +++ b/tests/Feature/Security/SecurityBlockMailTest.php @@ -46,7 +46,7 @@ it('laesst die Sperre stehen, wenn die Mail scheitert', function () { // Zustellung ist nicht die Bedingung fuer Schutz. Eine Sperre, die von einem // kaputten Postfach abhinge, waere genau dann weg, wenn ohnehin schon etwas // im Argen liegt. - Mail::shouldReceive('to')->andThrow(new RuntimeException('Postfach kaputt')); + Mail::shouldReceive('mailer')->andThrow(new RuntimeException('Postfach kaputt')); $instance = Instance::factory()->create(['status' => 'active', 'vmid' => 101]); $block = app(BlockAddress::class)->forInstance($instance, '203.0.113.7', 12);