Let a mailbox send without a password when it never needed one

An SMTP relay with a host and a from address but no username or password
- a trusted local or private-network relay - always used to be seeded as
a placeholder, because the takeover required a username to treat an
account as real, and Mailbox::isConfigured() always required a stored
password. Laravel's own SMTP mailer has supported unauthenticated relays
all along.

Add mailboxes.authenticates (default true, so every existing row keeps
its current behaviour) to tell "does not authenticate" apart from
"authenticates, but nobody has typed the password yet" - the same empty
password column used to mean both. The takeover migration now seeds the
real account off the host/address alone and marks it authenticates=false
only when neither a username nor a password resolved; isConfigured(),
MailboxTransport and MailboxTester all read it before requiring or
sending a password; the edit modal gets a checkbox for it with the
password field's hint updated to match, in both languages.
feat/mailboxes
nexxo 2026-07-28 05:54:21 +02:00
parent 40a6b3269d
commit 5d306fce31
17 changed files with 573 additions and 38 deletions

View File

@ -38,6 +38,9 @@ class EditMailbox extends ModalComponent
public bool $active = true; public bool $active = true;
/** Whether this mailbox logs in before it sends — Codex R15#4, P1b. */
public bool $authenticates = true;
public function mount(string $uuid): void public function mount(string $uuid): void
{ {
$this->authorize('mail.manage'); $this->authorize('mail.manage');
@ -50,6 +53,7 @@ class EditMailbox extends ModalComponent
$this->username = (string) $box->username; $this->username = (string) $box->username;
$this->noReply = $box->no_reply; $this->noReply = $box->no_reply;
$this->active = $box->active; $this->active = $box->active;
$this->authenticates = $box->authenticates;
} }
public function save(): void public function save(): void
@ -102,20 +106,23 @@ class EditMailbox extends ModalComponent
$oldAddress = $box->address; $oldAddress = $box->address;
$oldUsername = $box->username; $oldUsername = $box->username;
$oldAuthenticates = $box->authenticates;
$box->address = $this->address; $box->address = $this->address;
$box->display_name = $this->displayName ?: null; $box->display_name = $this->displayName ?: null;
$box->username = $this->username ?: null; $box->username = $this->username ?: null;
$box->no_reply = $this->noReply; $box->no_reply = $this->noReply;
$box->active = $this->active; $box->active = $this->active;
$box->authenticates = $this->authenticates;
// Either one changes the identity smtpUsername() authenticates as: // Either one changes the identity smtpUsername() authenticates as:
// username directly, or address as username's fallback when none is // username directly, or address as username's fallback when none is
// set (including username being CLEARED back to that fallback). // set (including username being CLEARED back to that fallback).
// Leaving last_verified_at standing after either would keep showing // authenticates itself is the fourth: whether that identity gets
// a successful verification for credentials nothing has actually // sent at all. Leaving last_verified_at standing after any of the
// tested against the new identity. // three would keep showing a successful verification for a
if ($box->address !== $oldAddress || $box->username !== $oldUsername) { // connection nothing has actually tested since.
if ($box->address !== $oldAddress || $box->username !== $oldUsername || $box->authenticates !== $oldAuthenticates) {
$box->last_verified_at = null; $box->last_verified_at = null;
} }

View File

@ -147,8 +147,16 @@ class MailboxTransport implements TransportInterface
throw new RuntimeException('Mail server port is not configured — cannot send mail.'); throw new RuntimeException('Mail server port is not configured — cannot send mail.');
} }
// authenticates is part of the fingerprint too, not only host/port/
// encryption/username/password: Mailbox::smtpUsername() always
// returns something non-empty (it falls back to the address), so an
// operator toggling authenticates off with everything else unchanged
// must still invalidate the cached delegate below — otherwise a
// long-running queue worker would keep the OLD delegate, which
// already has setUsername()/setPassword() called on it from before
// the toggle, still attempting AUTH the operator just turned off.
$fingerprint = md5(implode('|', [ $fingerprint = md5(implode('|', [
$host, $port, $encryption, $box->smtpUsername(), (string) $box->password, $host, $port, $encryption, (int) $box->authenticates, $box->smtpUsername(), (string) $box->password,
])); ]));
if ($this->delegate === null || $this->fingerprint !== $fingerprint) { if ($this->delegate === null || $this->fingerprint !== $fingerprint) {
@ -160,8 +168,17 @@ class MailboxTransport implements TransportInterface
$policy = MailTlsPolicy::for($encryption, $port); $policy = MailTlsPolicy::for($encryption, $port);
$transport = $policy->apply(new EsmtpTransport($host, $port, $policy->implicit)); $transport = $policy->apply(new EsmtpTransport($host, $port, $policy->implicit));
$transport->setUsername($box->smtpUsername());
$transport->setPassword((string) $box->password); // Codex R15#4, P1b: only called when this mailbox actually
// authenticates. smtpUsername()'s address fallback means it is
// NEVER empty, so calling setUsername() unconditionally would
// make Symfony attempt AUTH against any server that advertises
// it — exactly what a trusted, unauthenticated relay was never
// asked to answer.
if ($box->authenticates) {
$transport->setUsername($box->smtpUsername());
$transport->setPassword((string) $box->password);
}
$this->delegate = $transport; $this->delegate = $transport;
$this->fingerprint = $fingerprint; $this->fingerprint = $fingerprint;

View File

@ -20,7 +20,7 @@ class Mailbox extends Model
protected $fillable = [ protected $fillable = [
'key', 'address', 'display_name', 'username', 'key', 'address', 'display_name', 'username',
'password', 'no_reply', 'active', 'last_verified_at', 'password', 'no_reply', 'active', 'authenticates', 'last_verified_at',
]; ];
protected function casts(): array protected function casts(): array
@ -28,6 +28,7 @@ class Mailbox extends Model
return [ return [
'no_reply' => 'boolean', 'no_reply' => 'boolean',
'active' => 'boolean', 'active' => 'boolean',
'authenticates' => 'boolean',
'last_verified_at' => 'datetime', 'last_verified_at' => 'datetime',
]; ];
} }
@ -61,9 +62,18 @@ class Mailbox extends Model
: $this->address; : $this->address;
} }
/** Enough to send with: an address and a password. */ /**
* Enough to send with: an address, and a password ONLY when this mailbox
* actually authenticates.
*
* A trusted local or private-network relay can legitimately need no
* password at all (Codex R15#4, P1b) — requiring one unconditionally
* would refuse a relay that was never broken, just unauthenticated.
*/
public function isConfigured(): bool public function isConfigured(): bool
{ {
return $this->active && $this->address !== '' && $this->getRawOriginal('password') !== null; return $this->active
&& $this->address !== ''
&& (! $this->authenticates || $this->getRawOriginal('password') !== null);
} }
} }

View File

@ -38,13 +38,30 @@ final class MailboxTester
return ['ok' => false, 'error' => __('mail_settings.no_key')]; return ['ok' => false, 'error' => __('mail_settings.no_key')];
} }
if ($box->password === null) { // Codex R15#4, P1b: a password is only REQUIRED when this mailbox
// actually authenticates — a trusted local or private-network relay
// can legitimately need none at all, and refusing it here is exactly
// the "no password stored" outage the finding names, even though the
// relay was never broken.
if ($box->authenticates && $box->password === null) {
return ['ok' => false, 'error' => __('mail_settings.test_no_password')]; return ['ok' => false, 'error' => __('mail_settings.test_no_password')];
} }
$port = (int) Settings::get('mail.port', 587); $port = (int) Settings::get('mail.port', 587);
$policy = MailTlsPolicy::for((string) Settings::get('mail.encryption', 'tls'), $port); $policy = MailTlsPolicy::for((string) Settings::get('mail.encryption', 'tls'), $port);
// Omitted entirely, not merely blank, when this mailbox does not
// authenticate: Mailbox::smtpUsername() always returns something
// non-empty (it falls back to the address), so passing it through
// regardless would still hand Mail::build() a real 'username' —
// enough by itself to make Symfony attempt AUTH against any server
// that advertises it, which a trusted no-auth relay was never asked
// to answer. Mirrors MailboxTransport's own setUsername()/
// setPassword() guard for the real send path.
$credentials = $box->authenticates
? ['username' => $box->smtpUsername(), 'password' => $box->password]
: [];
try { try {
Mail::build([ Mail::build([
'transport' => 'smtp', 'transport' => 'smtp',
@ -66,8 +83,7 @@ final class MailboxTester
// silently prove a different policy than real sends enforce. // silently prove a different policy than real sends enforce.
'auto_tls' => $policy->autoTls, 'auto_tls' => $policy->autoTls,
'require_tls' => $policy->requireTls, 'require_tls' => $policy->requireTls,
'username' => $box->smtpUsername(), ...$credentials,
'password' => $box->password,
])->raw( ])->raw(
__('mail_settings.test_body', ['key' => $box->key]), __('mail_settings.test_body', ['key' => $box->key]),
function ($message) use ($box, $recipient) { function ($message) use ($box, $recipient) {

View File

@ -16,6 +16,7 @@ class MailboxFactory extends Factory
'display_name' => 'CluPilot', 'display_name' => 'CluPilot',
'username' => null, 'username' => null,
'password' => 'test-passwort', 'password' => 'test-passwort',
'authenticates' => true,
'no_reply' => false, 'no_reply' => false,
'active' => true, 'active' => true,
]; ];

View File

@ -0,0 +1,41 @@
<?php
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;
/**
* Whether this mailbox logs in before it sends.
*
* Distinct from "no password stored yet" Codex R15#4, P1b. A trusted local
* or private-network relay legitimately accepts mail with no AUTH at all, and
* Laravel's own SMTP mailer has always supported that (MAIL_USERNAME/
* MAIL_PASSWORD simply left unset). Before this column existed, "does not
* authenticate" and "authenticates, but nobody has typed the password yet"
* were the same empty password field to Mailbox::isConfigured() and to
* MailboxTransport an unauthenticated relay was refused exactly like a
* genuinely broken one.
*
* Defaults true: every mailbox row this table has ever held was either a real
* SMTP login (the takeover migration only ever wrote a username/password when
* one existed) or a placeholder awaiting one true reproduces both cases'
* actual behaviour unchanged, including for rows that already exist when this
* migration runs on a live install. The takeover migration is the one place
* that ever computes false, for the specific relay this column exists to fix.
*/
return new class extends Migration
{
public function up(): void
{
Schema::table('mailboxes', function (Blueprint $table) {
$table->boolean('authenticates')->default(true)->after('password');
});
}
public function down(): void
{
Schema::table('mailboxes', function (Blueprint $table) {
$table->dropColumn('authenticates');
});
}
};

View File

@ -246,21 +246,51 @@ return new class extends Migration
// without a password, exactly like the four addresses below it. // without a password, exactly like the four addresses below it.
$canEncrypt = app(SecretCipher::class)->isUsable(); $canEncrypt = app(SecretCipher::class)->isUsable();
// Codex R15#4, P1b: "neither a username nor a password" is what
// Laravel's SMTP mailer has always accepted as a legitimate
// unauthenticated relay — a trusted local or private-network server
// that authenticates by IP rather than by login. Read from the
// resolved SMTP config only (not from $storedPassword, the vault's
// own separate, legacy carry-over mechanism a few lines below this
// one — folding that in would mean a leftover vault row from a much
// older round could flip this even though today's .env states no
// credentials at all, a scope this fix deliberately leaves alone).
// A username ALONE, with no password anywhere, stays authenticates =
// true: that is an operator who started typing, not one who chose no
// auth, and isConfigured() must keep asking for a password rather
// than reading it as a deliberately open relay.
$authenticates = $configuredUser !== '' || $envPassword !== '';
foreach (self::SEED as $key => $meta) { foreach (self::SEED as $key => $meta) {
$isConfigured = $key === 'no-reply' && $configuredUser !== ''; // A real account exists when the takeover found somewhere to
// actually send through (the host — MailboxTransport's own first
// guard refuses to send without one too) AND someone to name it
// from ($resolvedAddress, which itself falls back through
// mail.from.address to the SMTP login). A username is
// deliberately NOT required here the way it used to be: gating
// on it exclusively is exactly what left an unauthenticated
// relay seeded as a placeholder, unable to send, until this fix.
$isConfigured = $key === 'no-reply' && $host !== '' && $resolvedAddress !== '';
$box = Mailbox::firstOrNew(['key' => $key]); $box = Mailbox::firstOrNew(['key' => $key]);
$box->fill([ $box->fill([
'address' => $isConfigured ? $resolvedAddress : $key.'@'.$domain, 'address' => $isConfigured ? $resolvedAddress : $key.'@'.$domain,
'display_name' => $displayName, 'display_name' => $displayName,
// Null when the login and address match: Mailbox:: // Null when the login and address match, OR when there is no
// smtpUsername() already falls back to address in that case, // login at all (an unauthenticated relay): Mailbox::
// so writing the identical string into both columns would // smtpUsername() already falls back to address in both
// just be a redundant copy — one more thing to keep in sync // cases, so writing '' into the column would just be a
// as either changes later through the console. // redundant, misleading copy of "no separate login".
'username' => $isConfigured && $configuredUser !== $resolvedAddress ? $configuredUser : null, 'username' => $isConfigured && $configuredUser !== '' && $configuredUser !== $resolvedAddress
? $configuredUser : null,
'no_reply' => $meta['no_reply'], 'no_reply' => $meta['no_reply'],
'active' => true, 'active' => true,
// Placeholders default true (the column's own default) the
// same way active does above — a fabricated support@/
// billing@/etc. address must keep asking for a password, not
// silently inherit "no auth needed" from the real account it
// sits next to in this loop.
'authenticates' => $isConfigured ? $authenticates : true,
]); ]);
// Only the account that actually exists gets credentials. // Only the account that actually exists gets credentials.

View File

@ -22,6 +22,9 @@ return [
'username_hint' => 'Leer lassen, wenn er der Adresse entspricht.', 'username_hint' => 'Leer lassen, wenn er der Adresse entspricht.',
'password' => 'Passwort', 'password' => 'Passwort',
'password_hint' => 'Wird verschlüsselt gespeichert und nie wieder vollständig angezeigt. Leer lassen, um das bisherige zu behalten.', 'password_hint' => 'Wird verschlüsselt gespeichert und nie wieder vollständig angezeigt. Leer lassen, um das bisherige zu behalten.',
'password_hint_unauthenticated' => 'Wird nicht verwendet — dieses Postfach ist auf Versand ohne Anmeldung eingestellt.',
'authenticates' => 'Passwort erforderlich',
'authenticates_hint' => 'Deaktivieren für einen vertrauenswürdigen Relay, der Mail ohne Anmeldung annimmt — etwa einen lokalen Server oder einen im eigenen Netz. Das Passwort oben wird ignoriert, solange dies deaktiviert ist.',
'no_reply' => 'Keine Antworten (kein Reply-To)', 'no_reply' => 'Keine Antworten (kein Reply-To)',
'no_reply_hint' => 'Ein „no-reply", auf das man antworten kann, ist eine Lüge im Absender.', 'no_reply_hint' => 'Ein „no-reply", auf das man antworten kann, ist eine Lüge im Absender.',
'active' => 'Aktiv', 'active' => 'Aktiv',

View File

@ -22,6 +22,9 @@ return [
'username_hint' => 'Leave empty if it matches the address.', 'username_hint' => 'Leave empty if it matches the address.',
'password' => 'Password', 'password' => 'Password',
'password_hint' => 'Stored encrypted and never shown in full again. Leave empty to keep the current one.', 'password_hint' => 'Stored encrypted and never shown in full again. Leave empty to keep the current one.',
'password_hint_unauthenticated' => 'Not used — this mailbox is set to send without logging in.',
'authenticates' => 'Requires a password',
'authenticates_hint' => 'Uncheck for a trusted relay that accepts mail without logging in — a local or private-network server, for example. The password above is ignored while this is unchecked.',
'no_reply' => 'No replies (no Reply-To)', 'no_reply' => 'No replies (no Reply-To)',
'no_reply_hint' => 'A "no-reply" that can be replied to is a lie in the sender.', 'no_reply_hint' => 'A "no-reply" that can be replied to is a lie in the sender.',
'active' => 'Active', 'active' => 'Active',

View File

@ -10,7 +10,16 @@
:label="__('mail_settings.username')" :hint="__('mail_settings.username_hint')" /> :label="__('mail_settings.username')" :hint="__('mail_settings.username_hint')" />
<x-ui.input name="password" type="password" autocomplete="off" wire:model="password" <x-ui.input name="password" type="password" autocomplete="off" wire:model="password"
:label="__('mail_settings.password')" :hint="__('mail_settings.password_hint')" /> :label="__('mail_settings.password')"
:hint="$authenticates ? __('mail_settings.password_hint') : __('mail_settings.password_hint_unauthenticated')" />
<div>
{{-- .live: the password hint above reads differently depending on
this, so toggling it has to reach the server and re-render
rather than sit deferred until the next unrelated action. --}}
<x-ui.checkbox name="authenticates" wire:model.live="authenticates" :label="__('mail_settings.authenticates')" />
<p class="pl-7 text-xs text-muted">{{ __('mail_settings.authenticates_hint') }}</p>
</div>
@if (! $passwordConfirmed) @if (! $passwordConfirmed)
{{-- The second gate, only reached if a new password is actually {{-- The second gate, only reached if a new password is actually

View File

@ -605,6 +605,7 @@ it('pre-fills the form from the existing record', function () {
'username' => 'smtp-support', 'username' => 'smtp-support',
'no_reply' => true, 'no_reply' => true,
'active' => false, 'active' => false,
'authenticates' => false,
]); ]);
Livewire::actingAs(User::factory()->operator('Owner')->create()) Livewire::actingAs(User::factory()->operator('Owner')->create())
@ -613,7 +614,85 @@ it('pre-fills the form from the existing record', function () {
->assertSet('displayName', 'Support-Team') ->assertSet('displayName', 'Support-Team')
->assertSet('username', 'smtp-support') ->assertSet('username', 'smtp-support')
->assertSet('noReply', true) ->assertSet('noReply', true)
->assertSet('active', false); ->assertSet('active', false)
->assertSet('authenticates', false);
});
// --- Codex R15#4, P1b: the modal's own checkbox for authenticates (R20 — no
// new modal, the existing one gets a field).
it('renders an authenticates checkbox in the modal, wired to the property', function () {
// A property the component accepts is not the same as a field the
// operator can actually reach — R20's own point.
$box = Mailbox::factory()->create(['authenticates' => true]);
Livewire::actingAs(User::factory()->operator('Owner')->create())
->test(EditMailbox::class, ['uuid' => $box->uuid])
->assertSeeHtml('wire:model.live="authenticates"')
->assertSee(__('mail_settings.authenticates'));
});
it('saves authenticates together with the other fields', function () {
$box = Mailbox::factory()->create(['authenticates' => true]);
Livewire::actingAs(User::factory()->operator('Owner')->create())
->test(EditMailbox::class, ['uuid' => $box->uuid])
->set('authenticates', false)
->call('save');
expect($box->fresh()->authenticates)->toBeFalse();
});
it('clears last_verified_at when authenticates changes, even though the password stays blank', function () {
// The fourth thing that invalidates a verification, alongside address
// and username (see the trio of tests above this block): a proof that
// credentials work means something different once whether to send them
// at all has flipped.
$box = Mailbox::factory()->create(['authenticates' => true, 'last_verified_at' => now()]);
Livewire::actingAs(User::factory()->operator('Owner')->create())
->test(EditMailbox::class, ['uuid' => $box->uuid])
->set('authenticates', false)
->call('save');
expect($box->fresh()->last_verified_at)->toBeNull();
});
it('does not clear last_verified_at when authenticates is saved unchanged', function () {
// The control case: the guard must react to an actual flip, not fire on
// every save regardless — the same shape as the existing address/
// username control test just above this block.
$box = Mailbox::factory()->create(['authenticates' => true, 'last_verified_at' => now()]);
Livewire::actingAs(User::factory()->operator('Owner')->create())
->test(EditMailbox::class, ['uuid' => $box->uuid])
->set('displayName', 'Neuer Anzeigename')
->call('save');
expect($box->fresh()->last_verified_at)->not->toBeNull();
});
it('does not report "no password stored" when testing a mailbox that does not authenticate', function () {
// The console-list-facing half of the finding: before this fix, the
// test-send button was the one place an unauthenticated mailbox actually
// read back as broken — MailboxTesterTest.php covers the service itself
// in depth (including that AUTH is never attempted); this proves the
// BUTTON reaches that fixed guard rather than a mock standing in for it.
// 127.0.0.1:1 refused in well under a millisecond (see "runs the real
// tester..." above) is what makes the failure real and fast rather than
// proving nothing by never actually attempting a connection.
Settings::set('mail.host', '127.0.0.1');
Settings::set('mail.port', 1);
$box = Mailbox::factory()->create([
'key' => 'support', 'address' => 'no-reply@clupilot.com',
'username' => null, 'password' => null, 'authenticates' => false,
]);
Livewire::actingAs(User::factory()->operator('Owner')->create())
->test(MailPage::class)
->set('testRecipient', 'ziel@example.com')
->call('test', $box->uuid)
->assertSet('testResult.error', fn ($error) => str_contains((string) $error, 'Connection refused'));
}); });
// --- Task 8: the test-send button. MailboxTesterTest.php covers // --- Task 8: the test-send button. MailboxTesterTest.php covers

View File

@ -48,6 +48,39 @@ it('falls back to the address when no separate username is given', function () {
expect($box->smtpUsername())->toBe('support@clupilot.com'); expect($box->smtpUsername())->toBe('support@clupilot.com');
}); });
// --- Codex R15#4, P1b: isConfigured() must require a password only when the
// mailbox actually authenticates — a trusted relay that needs no login is
// not the same state as one nobody has finished setting up yet.
it('is configured without a stored password when it does not authenticate', function () {
// ->fresh(), not the in-memory instance create() returns: authenticates
// needs its own boolean cast for this to mean anything (a raw DB read
// comes back "0"/"1", not a real PHP bool), the same gap
// "stores the password encrypted and never in the clear" above guards
// against for the password column.
$box = Mailbox::factory()->create(['authenticates' => false, 'password' => null]);
expect($box->fresh()->isConfigured())->toBeTrue();
});
it('is NOT configured without a stored password when it does authenticate', function () {
// The control case: the fix must not make isConfigured() stop requiring
// a password altogether — only when authenticates is explicitly false.
$box = Mailbox::factory()->create(['authenticates' => true, 'password' => null]);
expect($box->fresh()->isConfigured())->toBeFalse();
});
it('still requires active and a non-blank address regardless of authenticates', function () {
// authenticates=false must widen nothing except the password
// requirement — an inactive or addressless row stays unconfigured.
$inactive = Mailbox::factory()->create(['authenticates' => false, 'password' => null, 'active' => false]);
$blank = Mailbox::factory()->make(['authenticates' => false, 'password' => null, 'address' => '']);
expect($inactive->fresh()->isConfigured())->toBeFalse()
->and($blank->isConfigured())->toBeFalse();
});
it('finds a mailbox by its key', function () { it('finds a mailbox by its key', function () {
Mailbox::factory()->create(['key' => 'billing']); Mailbox::factory()->create(['key' => 'billing']);

View File

@ -311,6 +311,100 @@ it('still derives the placeholder domain from the SMTP login, not from mail.from
->and(Mailbox::findByKey('billing')->address)->toBe('billing@clupilot.com'); ->and(Mailbox::findByKey('billing')->address)->toBe('billing@clupilot.com');
}); });
// --- Codex R15#4, P1b: an existing SMTP relay with a host/from address but
// no username or password (a trusted local or private-network relay) must
// still be taken over for real — not seeded as a placeholder — with
// authenticates recording that it needs no password, distinct from a
// placeholder that simply has not had one typed in yet.
it('takes over a relay with a real host but no username or password, marking it as not authenticating', function () {
clearMailboxSeed();
config()->set('mail.mailers.smtp.host', 'relay.internal.example');
config()->set('mail.mailers.smtp.username', '');
config()->set('mail.mailers.smtp.password', '');
config()->set('mail.from.address', 'no-reply@clupilot.local');
loadMailboxSeedMigration()->up();
$box = Mailbox::findByKey('no-reply');
// The real From address, not the 'no-reply@clupilot.com' placeholder —
// before this fix, $isConfigured required a username and this relay has
// none, so it would have been seeded as a placeholder and the new
// transport would refuse to send for it, exactly the outage the finding
// describes.
expect($box->address)->toBe('no-reply@clupilot.local')
->and($box->authenticates)->toBeFalse()
->and($box->isConfigured())->toBeTrue()
// Null, not '': an empty username column must read as "no separate
// login" through Mailbox::smtpUsername()'s own fallback, not as a
// literal empty-string login distinct from having none at all.
->and($box->username)->toBeNull();
});
it('marks authenticates true, and the mailbox as needing a password, when only a username was resolved', function () {
// The other half of the distinction the brief draws: a username with no
// password is an operator who started typing, not one who chose no
// auth. authenticates must stay true so isConfigured() keeps asking for
// a password rather than reading this as a deliberately open relay.
clearMailboxSeed();
config()->set('mail.mailers.smtp.host', 'relay.internal.example');
config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com');
config()->set('mail.mailers.smtp.password', '');
loadMailboxSeedMigration()->up();
$box = Mailbox::findByKey('no-reply');
expect($box->authenticates)->toBeTrue()
->and($box->getRawOriginal('password'))->toBeNull()
->and($box->isConfigured())->toBeFalse();
});
it('leaves the four placeholder mailboxes authenticating by default, even when the real account does not', function () {
// authenticates=false must land ONLY on the account the takeover
// actually resolved — support@/billing@/office@/info@ are fabricated
// placeholder addresses with no real credentials behind them at all, and
// must keep asking for a password (authenticates=true, the column's own
// default) so they read as "not yet set up", not as "deliberately open".
// Getting this backwards would let a made-up placeholder address pass
// isConfigured() and be handed to MailboxTransport for real.
clearMailboxSeed();
config()->set('mail.mailers.smtp.host', 'relay.internal.example');
config()->set('mail.mailers.smtp.username', '');
config()->set('mail.mailers.smtp.password', '');
config()->set('mail.from.address', 'no-reply@clupilot.local');
loadMailboxSeedMigration()->up();
foreach (['support', 'billing', 'office', 'info'] as $key) {
$box = Mailbox::findByKey($key);
expect($box->authenticates)->toBeTrue("{$key} should still default to authenticates=true")
->and($box->isConfigured())->toBeFalse("{$key} should still read as unconfigured");
}
});
it('never seeds a blank address for the real account when neither a username nor a from address resolved', function () {
// A real host alone is not enough to know WHO mail should claim to be
// from — without this guard, a relay with a host but no username, no
// password, AND no MAIL_FROM_ADDRESS would resolve an empty address
// string. Falling back to the placeholder shape here (as if the account
// were not taken over at all) is safer than writing a blank address.
clearMailboxSeed();
config()->set('mail.mailers.smtp.host', 'relay.internal.example');
config()->set('mail.mailers.smtp.username', '');
config()->set('mail.mailers.smtp.password', '');
config()->set('mail.from.address', 'hello@example.com'); // config/mail.php's own unconfigured placeholder
loadMailboxSeedMigration()->up();
$box = Mailbox::findByKey('no-reply');
expect($box->address)->toBe('no-reply@clupilot.com')
->and($box->address)->not->toBe('');
});
it('carries the vault password across as ciphertext, never decrypting and re-encrypting it', function () { it('carries the vault password across as ciphertext, never decrypting and re-encrypting it', function () {
clearMailboxSeed(); clearMailboxSeed();
config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com'); config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com');
@ -347,11 +441,20 @@ it('leaves the vault row in the database even after a normal, fully successful c
expect(DB::table('app_secrets')->where('key', 'mail.password')->value('value'))->toBe($ciphertext); expect(DB::table('app_secrets')->where('key', 'mail.password')->value('value'))->toBe($ciphertext);
}); });
it('does not destroy a stored vault password when MAIL_USERNAME is empty, because it never lands anywhere', function () { it('does not destroy a stored vault password when nothing at all is configured, because it never lands anywhere', function () {
// Reproduces the Critical finding: $isConfigured is false for every key // Reproduces the Critical finding: $isConfigured is false for every key
// when MAIL_USERNAME is empty, so the carry-over never runs — the old // when nothing was resolved, so the carry-over never runs — the old code
// code deleted the vault row unconditionally regardless. // deleted the vault row unconditionally regardless.
//
// Host pinned to the placeholder explicitly, not left at this
// container's real .env value: Codex R15#4, P1b made $isConfigured
// sensitive to the HOST too (a real host with no username is now a
// legitimate unauthenticated relay, seeded for real — see the block of
// tests above this one), so this scenario has to say "nothing
// configured" on every signal itself rather than only blanking the
// username and relying on the rest to already be blank by accident.
clearMailboxSeed(); clearMailboxSeed();
config()->set('mail.mailers.smtp.host', '127.0.0.1');
config()->set('mail.mailers.smtp.username', ''); config()->set('mail.mailers.smtp.username', '');
$ciphertext = app(SecretCipher::class)->encrypt('nowhere-to-go'); $ciphertext = app(SecretCipher::class)->encrypt('nowhere-to-go');

View File

@ -61,7 +61,9 @@ it('passes the SMTP error through verbatim, because that is the whole diagnosis'
->and($result['error'])->toContain('Connection refused'); ->and($result['error'])->toContain('Connection refused');
}); });
it('refuses a mailbox without a password rather than pretending to try', function () { it('refuses a mailbox without a password rather than pretending to try — when it authenticates', function () {
// The control case for the block below: authenticates defaults to true
// on the factory, so this is what the guard must still refuse.
$box = Mailbox::factory()->create(['password' => null]); $box = Mailbox::factory()->create(['password' => null]);
$result = app(MailboxTester::class)->run($box, 'ziel@example.com'); $result = app(MailboxTester::class)->run($box, 'ziel@example.com');
@ -70,6 +72,78 @@ it('refuses a mailbox without a password rather than pretending to try', functio
->and($result['error'])->toContain('Passwort'); ->and($result['error'])->toContain('Passwort');
}); });
// --- Codex R15#4, P1b: an unauthenticated relay (a trusted local or
// private-network server) must not be refused for lacking a password —
// Laravel's previous SMTP configuration has always supported sending through
// one with none configured at all.
it('does not refuse an unauthenticated mailbox for lacking a password', 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', 'username' => null, 'password' => null, 'authenticates' => false,
]);
$result = app(MailboxTester::class)->run($box, 'ziel@example.com');
expect($result['ok'])->toBeTrue()
->and($result['error'])->toBeNull();
} finally {
$server->stop();
}
});
it('never attempts AUTH when the mailbox does not authenticate, even against a server that offers it', function () {
// succeeding() alone cannot prove this — it never advertises AUTH, so
// handleAuth() is unreachable regardless of what MailboxTester supplies.
// succeedingUnlessAuthAttempted() offers AUTH and rejects any attempt,
// so only a client that never sends "AUTH ..." at all can succeed here.
$server = FakeSmtpServer::succeedingUnlessAuthAttempted();
try {
Settings::set('mail.host', '127.0.0.1');
Settings::set('mail.port', $server->port);
$box = Mailbox::factory()->create([
'address' => 'no-reply@clupilot.com', 'username' => null, 'password' => null, 'authenticates' => false,
]);
$result = app(MailboxTester::class)->run($box, 'ziel@example.com');
expect($result['ok'])->toBeTrue()
->and($result['error'])->toBeNull();
} finally {
$server->stop();
}
});
it('still attempts AUTH when the mailbox does authenticate, against the same AUTH-offering server', function () {
// The control case for the test above: proves succeedingUnlessAuthAttempted()
// actually discriminates rather than always succeeding regardless of
// what the client does — an authenticating mailbox reaches handleAuth(),
// which sends AUTH LOGIN, which this server always rejects.
$server = FakeSmtpServer::succeedingUnlessAuthAttempted();
try {
Settings::set('mail.host', '127.0.0.1');
Settings::set('mail.port', $server->port);
$box = Mailbox::factory()->create([
'address' => 'no-reply@clupilot.com', 'password' => 'a-real-password', 'authenticates' => true,
]);
$result = app(MailboxTester::class)->run($box, 'ziel@example.com');
expect($result['ok'])->toBeFalse();
} finally {
$server->stop();
}
});
it('stamps last_verified_at only on success', function () { it('stamps last_verified_at only on success', function () {
$box = Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']); $box = Mailbox::factory()->create(['address' => 'no-reply@clupilot.com']);

View File

@ -178,6 +178,74 @@ it('reuses the same delegate until the mailbox credentials actually change', fun
expect($third)->not->toBe($first); // the operator's correction takes effect immediately expect($third)->not->toBe($first); // the operator's correction takes effect immediately
}); });
// --- Codex R15#4, P1b: an unauthenticated relay must not have credentials
// forced onto it. smtpUsername() always returns something non-empty (it
// falls back to the address), so a delegate built without checking
// authenticates first would still call setUsername() with a real value —
// which alone is enough to make Symfony attempt AUTH against any server
// that advertises it, something a trusted no-auth relay was never asked to
// answer.
it('never calls setUsername or setPassword on the delegate when the mailbox does not authenticate', function () {
Mailbox::factory()->create([
'key' => 'support', 'address' => 'support@clupilot.com',
'username' => null, 'password' => null, 'authenticates' => false,
]);
Settings::set(MailPurpose::settingKey(MailPurpose::SUPPORT), 'support');
config()->set('mail.default', 'smtp');
$transport = Mail::mailer('cp_support')->getSymfonyTransport();
$delegate = mailboxDelegate($transport);
expect($delegate)->toBeInstanceOf(EsmtpTransport::class)
->and($delegate->getUsername())->toBe('')
->and($delegate->getPassword())->toBe('');
});
it('still sets a real username and password on the delegate when the mailbox does authenticate', function () {
// The control case for the test above: the fix must not stop
// authenticated mailboxes from authenticating.
Mailbox::factory()->create([
'key' => 'support', 'address' => 'support@clupilot.com',
'username' => null, 'password' => 'a-real-password', 'authenticates' => true,
]);
Settings::set(MailPurpose::settingKey(MailPurpose::SUPPORT), 'support');
config()->set('mail.default', 'smtp');
$transport = Mail::mailer('cp_support')->getSymfonyTransport();
$delegate = mailboxDelegate($transport);
expect($delegate->getUsername())->toBe('support@clupilot.com') // smtpUsername()'s address fallback
->and($delegate->getPassword())->toBe('a-real-password');
});
it('rebuilds the delegate when authenticates toggles, even though host, port, encryption and password all stay the same', function () {
// reuses the same delegate...' above proves the fingerprint reacts to a
// PASSWORD change; this proves it also reacts to authenticates itself —
// without that, an operator unchecking "requires a password" on a
// long-running queue worker would keep sending through the OLD delegate,
// which already has setUsername()/setPassword() baked in from before the
// toggle, still attempting AUTH the operator just turned off.
$box = Mailbox::factory()->create([
'key' => 'support', 'address' => 'support@clupilot.com',
'username' => null, 'password' => 'unchanged-password', 'authenticates' => true,
]);
Settings::set(MailPurpose::settingKey(MailPurpose::SUPPORT), 'support');
config()->set('mail.default', 'smtp');
$transport = Mail::mailer('cp_support')->getSymfonyTransport();
$first = mailboxDelegate($transport);
expect($first->getUsername())->not->toBe('');
$box->update(['authenticates' => false]);
$second = mailboxDelegate($transport);
expect($second)->not->toBe($first)
->and($second->getUsername())->toBe('');
});
it('opens a plain connection with a STARTTLS upgrade for the submission port', function () { it('opens a plain connection with a STARTTLS upgrade for the submission port', function () {
Mailbox::factory()->create(['key' => 'support', 'address' => 'support@clupilot.com']); Mailbox::factory()->create(['key' => 'support', 'address' => 'support@clupilot.com']);
Settings::set(MailPurpose::settingKey(MailPurpose::SUPPORT), 'support'); Settings::set(MailPurpose::settingKey(MailPurpose::SUPPORT), 'support');

View File

@ -42,6 +42,25 @@ final class FakeSmtpServer
return self::spawn(['failAt' => null]); return self::spawn(['failAt' => null]);
} }
/**
* Accepts one connection, advertises AUTH LOGIN in its EHLO response,
* and completes a full successful send UNLESS the client attempts to
* authenticate, which is rejected outright.
*
* The point is proving a client deliberately did NOT attempt AUTH, not
* merely that it could succeed without it: succeeding() never advertises
* AUTH at all, so Symfony's EsmtpTransport::handleAuth() is never even
* reachable against it a mailbox that wrongly called setUsername()
* with a real value would pass every test that uses succeeding() too.
* Advertising the capability here is what makes the two cases tell
* apart: "chose not to authenticate" against a server that offered it,
* versus "had no opportunity to" against one that never did.
*/
public static function succeedingUnlessAuthAttempted(): self
{
return self::spawn(['failAt' => null, 'advertiseAuth' => true]);
}
/** /**
* Accepts one connection and rejects it with a real SMTP response at the * 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 * given stage. 'rcpt' is the simplest choice for a caller that just wants

View File

@ -5,23 +5,29 @@
* why a subprocess and not an in-process fake is necessary here). * why a subprocess and not an in-process fake is necessary here).
* *
* Configuration arrives as one JSON object on STDIN: {"failAt": string|null, * Configuration arrives as one JSON object on STDIN: {"failAt": string|null,
* "failResponse": string}. failAt names the command after which the server * "failResponse": string, "advertiseAuth": bool}. failAt names the command
* answers with failResponse instead of success 'mail', 'rcpt', or 'data'. * after which the server answers with failResponse instead of success
* null completes a normal, successful send. ('ehlo' is deliberately not a * 'mail', 'rcpt', or 'data'. null completes a normal, successful send.
* supported stage: EsmtpTransport retries a failed EHLO as plain HELO before * ('ehlo' is deliberately not a supported stage: EsmtpTransport retries a
* giving up, so a clean single-rejection test needs a later stage.) * failed EHLO as plain HELO before giving up, so a clean single-rejection
* test needs a later stage.)
* *
* Binds an ephemeral port on 127.0.0.1, prints it as the first line of * Binds an ephemeral port on 127.0.0.1, prints it as the first line of
* STDOUT so the parent process can connect, accepts exactly one connection, * STDOUT so the parent process can connect, accepts exactly one connection,
* and plays the minimal EHLO/MAIL FROM/RCPT TO/DATA dialogue. Neither * and plays the minimal EHLO/MAIL FROM/RCPT TO/DATA dialogue. STARTTLS is
* STARTTLS nor AUTH is advertised in the EHLO response, so * never advertised in the EHLO response, so EsmtpTransport auto_tls
* EsmtpTransport auto_tls defaults true, but only upgrades when the * defaults true, but only upgrades when the server actually offers
* server actually offers STARTTLS never attempts either, and the whole * STARTTLS never attempts it, and the whole exchange stays in plain text.
* exchange stays in plain text. *
* AUTH is advertised only when advertiseAuth is true (see
* FakeSmtpServer::succeedingUnlessAuthAttempted()) the base configuration
* never offers it, so EsmtpTransport::handleAuth() is never even reachable
* against it regardless of what credentials a client might have set.
*/ */
$config = json_decode((string) stream_get_contents(STDIN), true) ?: []; $config = json_decode((string) stream_get_contents(STDIN), true) ?: [];
$failAt = $config['failAt'] ?? null; $failAt = $config['failAt'] ?? null;
$failResponse = $config['failResponse'] ?? '550 Rejected'; $failResponse = $config['failResponse'] ?? '550 Rejected';
$advertiseAuth = $config['advertiseAuth'] ?? false;
$server = stream_socket_server('tcp://127.0.0.1:0', $errno, $errstr); $server = stream_socket_server('tcp://127.0.0.1:0', $errno, $errstr);
@ -59,9 +65,25 @@ function smtpRespond($conn, string $line): void
smtpRespond($conn, '220 fake.smtp ESMTP'); smtpRespond($conn, '220 fake.smtp ESMTP');
smtpReadLine($conn); // EHLO smtpReadLine($conn); // EHLO
smtpRespond($conn, '250 fake.smtp'); if ($advertiseAuth) {
// Multi-line EHLO: smtpRespond() only ever sends the final line, so the
// continuation is written directly.
fwrite($conn, "250-fake.smtp\r\n");
smtpRespond($conn, '250 AUTH LOGIN');
} else {
smtpRespond($conn, '250 fake.smtp');
}
smtpReadLine($conn); // MAIL FROM // AUTH, if the client attempts it, arrives here in place of MAIL FROM —
// immediately after EHLO and before the send dialogue proper, exactly where
// EsmtpTransport::handleAuth() sends it.
$line = smtpReadLine($conn);
if ($advertiseAuth && str_starts_with(strtoupper(ltrim($line)), 'AUTH')) {
smtpRespond($conn, '535 5.7.8 Authentication attempted but this relay does not require it');
exit(0);
}
// $line is MAIL FROM's own line when AUTH was not attempted.
if ($failAt === 'mail') { if ($failAt === 'mail') {
smtpRespond($conn, $failResponse); smtpRespond($conn, $failResponse);
exit(0); exit(0);