diff --git a/app/Livewire/Admin/Mail.php b/app/Livewire/Admin/Mail.php index da8c2f8..a7190d9 100644 --- a/app/Livewire/Admin/Mail.php +++ b/app/Livewire/Admin/Mail.php @@ -3,6 +3,7 @@ namespace App\Livewire\Admin; use App\Models\Mailbox; +use App\Services\Mail\MailboxTester; use App\Services\Mail\MailPurpose; use App\Services\Secrets\SecretCipher; use App\Support\Settings; @@ -14,11 +15,9 @@ use Livewire\Component; * * The server sits at the top because there is one of it; the mailboxes are a * list because there are several; the mapping is last because it only makes - * sense once both exist. - * - * The test-send button is a later task's job. This page only shows and edits - * what already exists — a button that cannot actually test is a promise the - * page does not keep (the same reasoning the secrets page already applies). + * sense once both exist. The test-send button lives with the mailboxes: it + * proves one specific mailbox can actually send, which is the only thing + * that makes the rest of this page more than a form. */ #[Layout('layouts.admin')] class Mail extends Component @@ -32,6 +31,13 @@ class Mail extends Component /** @var array purpose => mailbox key */ public array $purposes = []; + public string $testRecipient = ''; + + /** @var array{ok: bool, error: ?string}|null */ + public ?array $testResult = null; + + public ?string $testedKey = null; + /** * Whether credentials can be stored at all on this installation. * @@ -92,6 +98,21 @@ class Mail extends Component $this->dispatch('notify', message: __('mail_settings.purposes_saved')); } + public function test(string $uuid): void + { + $this->authorize('mail.manage'); + + $this->validate( + ['testRecipient' => ['required', 'email']], + ['testRecipient.required' => __('mail_settings.test_recipient_required')], + ); + + $box = Mailbox::query()->where('uuid', $uuid)->firstOrFail(); + + $this->testedKey = $box->key; + $this->testResult = app(MailboxTester::class)->run($box, $this->testRecipient); + } + public function render() { $this->usable = app(SecretCipher::class)->isUsable(); diff --git a/app/Services/Mail/MailboxTester.php b/app/Services/Mail/MailboxTester.php new file mode 100644 index 0000000..3b9b3e2 --- /dev/null +++ b/app/Services/Mail/MailboxTester.php @@ -0,0 +1,78 @@ +password: that accessor + // decrypts under SECRETS_KEY, and it is not set on this installation + // at all (the mail settings page's own banner says so). Reading it + // first would let a RuntimeException out of a Livewire action — + // Task 7's EditMailbox::save() shipped a Critical fix for exactly + // this shape of bug. The tester touches the same cipher and gets the + // same guard, checked here rather than left for the property access + // below to throw through. + if (! $this->cipher->isUsable()) { + return ['ok' => false, 'error' => __('mail_settings.no_key')]; + } + + if ($box->password === null) { + return ['ok' => false, 'error' => __('mail_settings.test_no_password')]; + } + + $port = (int) Settings::get('mail.port', 587); + + try { + Mail::build([ + 'transport' => 'smtp', + 'host' => (string) Settings::get('mail.host', ''), + 'port' => $port, + // 'scheme', NOT 'encryption'. Laravel 13's MailManager reads + // scheme only (MailManager.php:196) — an 'encryption' key is + // silently ignored, and the connection would go out in the + // clear while the console reported TLS. + 'scheme' => (Settings::get('mail.encryption') === 'ssl' || $port === 465) ? 'smtps' : 'smtp', + 'username' => $box->smtpUsername(), + 'password' => $box->password, + ])->raw( + __('mail_settings.test_body', ['key' => $box->key]), + function ($message) use ($box, $recipient) { + $message->to($recipient) + ->from($box->address, $box->display_name ?: null) + ->subject(__('mail_settings.test_subject')); + }, + ); + } catch (Throwable $e) { + // Verbatim. With wrong credentials, or a server that rejects the + // message outright, the server's own sentence IS the entire + // diagnosis — "sending failed" would throw that away. + return ['ok' => false, 'error' => $e->getMessage()]; + } + + $box->forceFill(['last_verified_at' => now()])->save(); + + return ['ok' => true, 'error' => null]; + } +} diff --git a/lang/de/mail_settings.php b/lang/de/mail_settings.php index 486ddf1..dad22f2 100644 --- a/lang/de/mail_settings.php +++ b/lang/de/mail_settings.php @@ -41,4 +41,14 @@ return [ ], 'purposes_saved' => 'Zuordnung gespeichert.', 'system_required' => '„System" muss ein Postfach haben — es ist der Rückfall für alle anderen.', + + 'test' => 'Testmail senden', + 'test_recipient' => 'Testempfänger', + 'test_hint' => 'Der Testversand geht wirklich raus — auch auf dieser Installation, auf der gewöhnliche Mails nur protokolliert werden.', + 'test_recipient_required' => 'Bitte eine Empfängeradresse eingeben.', + 'test_subject' => 'CluPilot — Testnachricht', + 'test_body' => 'Diese Nachricht bestätigt, dass das Postfach :key verschicken kann.', + 'test_no_password' => 'Für dieses Postfach ist kein Passwort hinterlegt.', + 'test_ok' => 'Zugestellt. Das Postfach kann verschicken.', + 'test_failed' => 'Der Mailserver hat abgelehnt:', ]; diff --git a/lang/en/mail_settings.php b/lang/en/mail_settings.php index a91df3d..dadeaf2 100644 --- a/lang/en/mail_settings.php +++ b/lang/en/mail_settings.php @@ -41,4 +41,14 @@ return [ ], 'purposes_saved' => 'Mapping saved.', 'system_required' => '"System" must have a mailbox — it is the fallback for all the others.', + + 'test' => 'Send test mail', + 'test_recipient' => 'Test recipient', + 'test_hint' => 'The test send really goes out — even on this installation, where ordinary mail is only logged.', + 'test_recipient_required' => 'Please enter a recipient address.', + 'test_subject' => 'CluPilot — test message', + 'test_body' => 'This message confirms that the mailbox :key can send.', + 'test_no_password' => 'No password is stored for this mailbox.', + 'test_ok' => 'Delivered. The mailbox can send.', + 'test_failed' => 'The mail server rejected it:', ]; diff --git a/resources/views/components/ui/icon.blade.php b/resources/views/components/ui/icon.blade.php index 9ec707c..3a578e9 100644 --- a/resources/views/components/ui/icon.blade.php +++ b/resources/views/components/ui/icon.blade.php @@ -39,6 +39,7 @@ 'trash-2' => '', 'settings' => '', 'mail' => '', + 'send' => '', ]; $body = $icons[$name] ?? ''; diff --git a/resources/views/livewire/admin/mail.blade.php b/resources/views/livewire/admin/mail.blade.php index 2ed068b..e5a5bc9 100644 --- a/resources/views/livewire/admin/mail.blade.php +++ b/resources/views/livewire/admin/mail.blade.php @@ -37,6 +37,24 @@

{{ __('mail_settings.boxes_title') }}

+ {{-- One shared recipient field for every row's test button below — a + field ABOVE the table, not one growing inside a (R20). --}} +
+ +

{{ __('mail_settings.test_hint') }}

+
+ + @if ($testResult) + + {{ $testedKey }}: + @if ($testResult['ok']) + {{ __('mail_settings.test_ok') }} + @else + {{ __('mail_settings.test_failed') }} {{ $testResult['error'] }} + @endif + + @endif + @@ -55,11 +73,12 @@ -
{{ $box->last_verified_at?->local()->isoFormat('DD.MM.YYYY HH:mm') ?? __('mail_settings.never_verified') }} - {{-- Edit only (R18: icon beside its text, single line). - The test-send button belongs to a later task — - shipping one that cannot yet test anything is a - promise this page does not keep. --}} + + {{-- R18: icon beside its text, single line, size-4. --}} + + {{ __('mail_settings.test') }} + {{ __('mail_settings.edit') }} diff --git a/tests/Feature/Mail/MailSettingsPageTest.php b/tests/Feature/Mail/MailSettingsPageTest.php index 9eda5d0..715bc2a 100644 --- a/tests/Feature/Mail/MailSettingsPageTest.php +++ b/tests/Feature/Mail/MailSettingsPageTest.php @@ -297,3 +297,111 @@ it('pre-fills the form from the existing record', function () { ->assertSet('noReply', true) ->assertSet('active', false); }); + +// --- Task 8: the test-send button. MailboxTesterTest.php covers +// MailboxTester itself in depth (bypassing the log/array mailer, verbatim +// error passthrough, the SECRETS_KEY guard, a real success and a real SMTP +// rejection). What is left to prove here is the WIRING: does the button on +// this page reach that service with the right arguments, under the right +// authorization and validation, and does the page show what came back. + +it('refuses to run a mailbox test without mail.manage, re-checked independently of mount', function () { + $box = Mailbox::factory()->create(['key' => 'support']); + $owner = User::factory()->operator('Owner')->create(); + $billing = User::factory()->operator('Billing')->create(); + + $page = Livewire::actingAs($owner)->test(MailPage::class)->assertOk(); + + Livewire::actingAs($billing); + + $page->set('testRecipient', 'ziel@example.com') + ->call('test', $box->uuid) + ->assertForbidden(); +}); + +it('requires a recipient before sending a test', function () { + $box = Mailbox::factory()->create(['key' => 'support']); + + Livewire::actingAs(User::factory()->operator('Owner')->create()) + ->test(MailPage::class) + ->set('testRecipient', '') + ->call('test', $box->uuid) + ->assertHasErrors('testRecipient'); +}); + +it('requires the recipient to actually be an email address', function () { + $box = Mailbox::factory()->create(['key' => 'support']); + + Livewire::actingAs(User::factory()->operator('Owner')->create()) + ->test(MailPage::class) + ->set('testRecipient', 'not-an-email') + ->call('test', $box->uuid) + ->assertHasErrors('testRecipient'); +}); + +it('runs the real tester against the chosen mailbox and records the result', function () { + // 127.0.0.1:1 is refused in well under a millisecond (see + // MailboxTesterTest) — fast and deterministic enough to run through the + // REAL MailboxTester here rather than a mock, proving the button is + // wired to the thing that actually sends, not a stand-in for it. + Settings::set('mail.host', '127.0.0.1'); + Settings::set('mail.port', 1); + $box = Mailbox::factory()->create(['key' => 'support']); + + Livewire::actingAs(User::factory()->operator('Owner')->create()) + ->test(MailPage::class) + ->set('testRecipient', 'ziel@example.com') + ->call('test', $box->uuid) + ->assertSet('testedKey', 'support') + ->assertSet('testResult.ok', false) + ->assertSet('testResult.error', fn ($error) => str_contains((string) $error, 'Connection refused')); +}); + +it('shows the result against the mailbox it actually tested', function () { + // Not assertSee('support') alone: the mailboxes table already prints + // every box's own key in its row regardless of which one was tested, so + // that assertion would stay green even if $testedKey were wired to the + // wrong box (caught by mutation — see the task report). 'support:' with + // the colon is unique to the alert's ": " markup; the table + // row never follows a key with one. + Settings::set('mail.host', '127.0.0.1'); + Settings::set('mail.port', 1); + $box = Mailbox::factory()->create(['key' => 'support']); + Mailbox::factory()->create(['key' => 'billing']); // a second row: proves it's not just "any key" + + Livewire::actingAs(User::factory()->operator('Owner')->create()) + ->test(MailPage::class) + ->set('testRecipient', 'ziel@example.com') + ->call('test', $box->uuid) + ->assertSee('support:') + ->assertDontSee('billing:') + ->assertSee(__('mail_settings.test_failed')); +}); + +it('does not crash the test button when SECRETS_KEY is missing, and says so instead', function () { + // The gap the brief could not know about: $box->password decrypts on + // read, and this installation has no SECRETS_KEY at all. Proven at the + // MailboxTester unit level already — this proves the BUTTON reaches that + // guard rather than a mock standing in for it, the same distinction + // "re-checks mail.manage on every page action" draws for authorization. + $box = Mailbox::factory()->create(['key' => 'support']); + config()->set('admin_access.secrets_key', ''); + + Livewire::actingAs(User::factory()->operator('Owner')->create()) + ->test(MailPage::class) + ->set('testRecipient', 'ziel@example.com') + ->call('test', $box->uuid) + ->assertSet('testResult.error', __('mail_settings.no_key')); +}); + +it('offers a per-row test-send button, not only edit', function () { + $blade = file_get_contents(resource_path('views/livewire/admin/mail.blade.php')); + + expect($blade)->toContain('wire:click="test('); +}); + +it('tells the operator the test send really goes out, even though ordinary mail only logs here', function () { + Livewire::actingAs(User::factory()->operator('Owner')->create()) + ->test(MailPage::class) + ->assertSee(__('mail_settings.test_hint')); +}); diff --git a/tests/Feature/Mail/MailboxTesterTest.php b/tests/Feature/Mail/MailboxTesterTest.php new file mode 100644 index 0000000..36c1f2e --- /dev/null +++ b/tests/Feature/Mail/MailboxTesterTest.php @@ -0,0 +1,173 @@ +set('admin_access.secrets_key', 'base64:'.base64_encode(random_bytes(32))); + config()->set('mail.default', 'log'); // as on a development machine + Settings::set('mail.host', '127.0.0.1'); + Settings::set('mail.port', 1); + Settings::set('mail.encryption', 'tls'); +}); + +/* +| The brief points this beforeEach at 'mail.example.invalid' and asserts the +| error mentions it. Measured by hand in this container (php artisan tinker +| equivalent, three runs each): a lookup for 'mail.example.invalid' fails in +| 0.7-7ms via NXDOMAIN, but that number depends on a resolver being reachable +| at all — nothing here guarantees that in every environment this suite might +| run in. 127.0.0.1:1 (nothing listens on port 1) refused every one of those +| three runs in ~0.2ms with zero DNS involved, and it is the convention this +| codebase already settled on for the identical reason — see +| MailboxTransportTest's "LOAD-BEARING for the MAIL_MAILER=log guard" comment. +| Using it here keeps both tests fast AND deterministic instead of merely fast +| today. +*/ + +it('does NOT quietly succeed through the log mailer', function () { + // The whole point: on dev the default mailer is the log, and a tester + // that honoured it would report success while writing to a file — a + // test that proves nothing. The log "transport" never fails, so only a + // real attempt (which a closed port refuses) can make this assertion + // meaningful. + $result = app(MailboxTester::class)->run( + Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']), + 'ziel@example.com', + ); + + expect($result['ok'])->toBeFalse() + ->and($result['error'])->not->toBeNull(); +}); + +it('passes the SMTP error through verbatim, because that is the whole diagnosis', function () { + $result = app(MailboxTester::class)->run( + Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']), + 'ziel@example.com', + ); + + // Not "Fehler beim Senden" — the operating system's own sentence, naming + // the actual host:port and the actual reason. + expect($result['error'])->toContain('127.0.0.1:1') + ->and($result['error'])->toContain('Connection refused'); +}); + +it('refuses a mailbox without a password rather than pretending to try', function () { + $box = Mailbox::factory()->create(['password' => null]); + + $result = app(MailboxTester::class)->run($box, 'ziel@example.com'); + + expect($result['ok'])->toBeFalse() + ->and($result['error'])->toContain('Passwort'); +}); + +it('stamps last_verified_at only on success', function () { + $box = Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']); + + app(MailboxTester::class)->run($box, 'ziel@example.com'); + + expect($box->fresh()->last_verified_at)->toBeNull(); +}); + +it('gives a readable message instead of throwing when SECRETS_KEY is unusable', function () { + // Facts the brief could not know: SECRETS_KEY is not set on this + // installation at all (the mail settings page's own banner says so). + // $box->password decrypts on read, so touching it throws a + // RuntimeException out of SecretCipher — Task 7's EditMailbox::save() + // shipped a Critical fix for exactly this shape of bug. The tester + // touches the very same cipher and needs the very same guard. + $box = Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']); + + config()->set('admin_access.secrets_key', ''); + + $result = app(MailboxTester::class)->run($box, 'ziel@example.com'); + + expect($result['ok'])->toBeFalse() + ->and($result['error'])->toBe(__('mail_settings.no_key')); +}); + +/* +| LOAD-BEARING for the success path. Every test above only proves a FAILURE +| is reported correctly — none of them ever reaches +| `$box->forceFill(['last_verified_at' => now()])->save()`, so all of them +| would stay green even if that line were deleted outright. A real success +| needs something real to answer on the other end of the socket; see +| FakeSmtpServer for why that has to be a subprocess rather than an in-process +| fake. +*/ +it('actually delivers and stamps last_verified_at when the server accepts the message', function () { + $server = FakeSmtpServer::succeeding(); + + try { + Settings::set('mail.host', '127.0.0.1'); + Settings::set('mail.port', $server->port); + + $box = Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']); + + $result = app(MailboxTester::class)->run($box, 'ziel@example.com'); + + expect($result['ok'])->toBeTrue() + ->and($result['error'])->toBeNull() + ->and($box->fresh()->last_verified_at)->not->toBeNull(); + } finally { + $server->stop(); + } +}); + +it('passes a real SMTP rejection through verbatim too, not only a refused connection', function () { + // The strongest version of "the server's own sentence": an actual + // protocol-level 5xx from an actual SMTP dialogue, not merely the + // operating system's "connection refused". Proves MailboxTester's catch + // block does not paraphrase or replace what Symfony Mailer reports. + $server = FakeSmtpServer::rejectingAt('rcpt', '550 5.1.1 No such user ziel@example.com'); + + try { + Settings::set('mail.host', '127.0.0.1'); + Settings::set('mail.port', $server->port); + + $box = Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']); + + $result = app(MailboxTester::class)->run($box, 'ziel@example.com'); + + expect($result['ok'])->toBeFalse() + ->and($result['error'])->toContain('550 5.1.1 No such user ziel@example.com') + ->and($box->fresh()->last_verified_at)->toBeNull(); + } finally { + $server->stop(); + } +}); + +it('actually attempts TLS when SSL is configured on a non-standard port, instead of silently going out in the clear', function () { + // The exact trap this task names: Laravel 13's MailManager reads + // 'scheme', not 'encryption' (MailManager.php:196). An 'encryption' key + // is silently ignored, and MailManager falls back to choosing the scheme + // from the port number alone — smtps only for port 465, smtp otherwise. + // Configured for SSL on a non-465 port (a real EU hoster's submission + // port with implicit TLS, say), that bug would connect in PLAIN TEXT + // while the console still reported TLS. + // + // FakeSmtpServer only ever speaks plain text, so it can tell the two + // apart: a correct 'scheme' => 'smtps' opens an implicit-TLS connection + // this server cannot answer and the send FAILS; the buggy 'encryption' + // key would default to plain 'smtp' on this non-465 port and let it + // succeed instead. Verified by hand this fails in ~0.2s, not a hang — + // OpenSSL rejects the server's plain-text greeting as an invalid TLS + // record immediately. + $server = FakeSmtpServer::succeeding(); + + try { + Settings::set('mail.host', '127.0.0.1'); + Settings::set('mail.port', $server->port); // deliberately not 465 + Settings::set('mail.encryption', 'ssl'); + + $box = Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']); + + $result = app(MailboxTester::class)->run($box, 'ziel@example.com'); + + expect($result['ok'])->toBeFalse(); + } finally { + $server->stop(); + } +}); diff --git a/tests/Support/FakeSmtpServer.php b/tests/Support/FakeSmtpServer.php new file mode 100644 index 0000000..4c86ddc --- /dev/null +++ b/tests/Support/FakeSmtpServer.php @@ -0,0 +1,101 @@ + */ + private array $pipes; + + public readonly int $port; + + private function __construct($process, array $pipes, int $port) + { + $this->process = $process; + $this->pipes = $pipes; + $this->port = $port; + } + + /** Accepts one connection and completes a full, successful send. */ + public static function succeeding(): self + { + return self::spawn(['failAt' => null]); + } + + /** + * Accepts one connection and rejects it with a real SMTP response at the + * given stage. 'rcpt' is the simplest choice for a caller that just wants + * *a* rejection: MAIL FROM has already succeeded by then, so the failure + * is unambiguously the server's own — 'ehlo' is deliberately unsupported + * because EsmtpTransport retries a failed EHLO as plain HELO rather than + * giving up, which this one-shot server does not simulate. + */ + public static function rejectingAt(string $stage, string $response): self + { + return self::spawn(['failAt' => $stage, 'failResponse' => $response]); + } + + private static function spawn(array $config): self + { + $process = proc_open( + [PHP_BINARY, __DIR__.'/fake_smtp_server_process.php'], + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes, + ); + + if (! is_resource($process)) { + throw new RuntimeException('Could not start the fake SMTP server subprocess.'); + } + + fwrite($pipes[0], json_encode($config)); + fclose($pipes[0]); + + // A bound port comes back almost instantly; this timeout only + // matters if the subprocess never starts at all, so the test suite + // fails fast instead of hanging. + stream_set_timeout($pipes[1], 5); + $port = (int) trim((string) fgets($pipes[1])); + + if ($port <= 0) { + $stderr = stream_get_contents($pipes[2]); + proc_terminate($process); + proc_close($process); + + throw new RuntimeException("Fake SMTP server failed to bind a port. stderr: {$stderr}"); + } + + return new self($process, $pipes, $port); + } + + public function stop(): void + { + foreach ($this->pipes as $pipe) { + if (is_resource($pipe)) { + fclose($pipe); + } + } + + proc_terminate($this->process); + proc_close($this->process); + } +} diff --git a/tests/Support/fake_smtp_server_process.php b/tests/Support/fake_smtp_server_process.php new file mode 100644 index 0000000..b98e2e9 --- /dev/null +++ b/tests/Support/fake_smtp_server_process.php @@ -0,0 +1,99 @@ +