Seed the mailbox address from mail.from.address, not the SMTP login
The takeover migration seeded `address` from MAIL_USERNAME and left `username` null unconditionally, conflating the SMTP login with the sender address. Whenever the two differ — an API-key username, a shared relay login — mail would go out From the login instead of the address the system was actually configured to send from, exactly like this installation: MAIL_USERNAME is no-reply@clupilot.com but MAIL_FROM_ADDRESS is hello@clupilot.local. `address` now reads config('mail.from.address'), falling back to the SMTP login only when that reads back config/mail.php's own unset placeholder (env('MAIL_FROM_ADDRESS', 'hello@example.com') — the same masking problem UNCONFIGURED_HOST already guards against for MAIL_HOST). `username` is set to the SMTP login only when it actually differs from the resolved address, since Mailbox::smtpUsername() already falls back to address when username is null. display_name now reads config('mail.from.name') instead of a hardcoded literal. The placeholder domain for the four non-configured mailboxes still comes from the SMTP login, not from mail.from.address — the two can name different domains, and only the login's is proven to accept mail for a given install. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feat/mailboxes
parent
3278f063ea
commit
e8a3b8d5be
|
|
@ -58,6 +58,20 @@ return new class extends Migration
|
|||
|
||||
private const UNCONFIGURED_PORT = 2525;
|
||||
|
||||
/**
|
||||
* config/mail.php's own fallback when MAIL_FROM_ADDRESS is unset.
|
||||
*
|
||||
* Same masking problem as UNCONFIGURED_HOST above, and the same fix:
|
||||
* env('MAIL_FROM_ADDRESS', 'hello@example.com') has already substituted
|
||||
* by the time this migration reads config('mail.from.address'), and
|
||||
* 'hello@example.com' is not '', so without this check it would be
|
||||
* seeded as a real From address instead of recognised as "nothing
|
||||
* configured, fall back to the SMTP login". Same trade-off as the host
|
||||
* placeholder too: an install that deliberately wants to send from the
|
||||
* literal address hello@example.com reads as unset — nobody does that.
|
||||
*/
|
||||
private const UNCONFIGURED_FROM_ADDRESS = 'hello@example.com';
|
||||
|
||||
public function up(): void
|
||||
{
|
||||
DB::transaction(function () {
|
||||
|
|
@ -88,10 +102,41 @@ return new class extends Migration
|
|||
Settings::set('mail.encryption', $port === 465 ? 'ssl' : 'tls');
|
||||
|
||||
$configuredUser = (string) config('mail.mailers.smtp.username', '');
|
||||
|
||||
// Placeholder domain for the four boxes below comes from the SMTP
|
||||
// LOGIN's domain, not from mail.from.address — even though address
|
||||
// itself (below) now prefers mail.from.address for the no-reply box.
|
||||
// The login and the From address can point at different domains (an
|
||||
// API-key login on a shared relay next to a From address on the
|
||||
// install's own domain, or the reverse), and only the login's domain
|
||||
// is proven to accept mail: it is the one .env's MAIL_HOST/
|
||||
// MAIL_USERNAME pair has actually been sending through. Building
|
||||
// support@/billing@/etc. from mail.from.address's domain instead
|
||||
// could seed a placeholder on a domain nobody could ever turn into a
|
||||
// working mailbox.
|
||||
$domain = str_contains($configuredUser, '@')
|
||||
? substr($configuredUser, strpos($configuredUser, '@') + 1)
|
||||
: 'clupilot.com';
|
||||
|
||||
// The From address actually in force today may not be the SMTP
|
||||
// login — a relay's own account name (an API-key username, a shared
|
||||
// login) is routinely not the address recipients see and can reply
|
||||
// to. A takeover migration's job is to reproduce what the system
|
||||
// already sends, not to improve on it, so address and username are
|
||||
// read and assigned separately rather than collapsing the login into
|
||||
// both.
|
||||
$configuredFromAddress = (string) config('mail.from.address', '');
|
||||
$fromAddress = $configuredFromAddress === self::UNCONFIGURED_FROM_ADDRESS ? '' : $configuredFromAddress;
|
||||
|
||||
// An install with no MAIL_FROM_ADDRESS has nothing better than the
|
||||
// SMTP login to send From — which is what Laravel's own mailer was
|
||||
// already falling back to before this migration, so this reproduces
|
||||
// that install's mail rather than changing it.
|
||||
$resolvedAddress = $fromAddress !== '' ? $fromAddress : $configuredUser;
|
||||
|
||||
$configuredFromName = (string) config('mail.from.name', '');
|
||||
$displayName = $configuredFromName !== '' ? $configuredFromName : 'CluPilot';
|
||||
|
||||
// A password may already be stored in the vault. It is CARRIED, not
|
||||
// moved: the app_secrets row is deliberately left in place below
|
||||
// rather than deleted, so this block only ever reads it.
|
||||
|
|
@ -111,8 +156,14 @@ return new class extends Migration
|
|||
|
||||
$box = Mailbox::firstOrNew(['key' => $key]);
|
||||
$box->fill([
|
||||
'address' => $isConfigured ? $configuredUser : $key.'@'.$domain,
|
||||
'display_name' => 'CluPilot',
|
||||
'address' => $isConfigured ? $resolvedAddress : $key.'@'.$domain,
|
||||
'display_name' => $displayName,
|
||||
// Null when the login and address match: Mailbox::
|
||||
// smtpUsername() already falls back to address in that case,
|
||||
// so writing the identical string into both columns would
|
||||
// just be a redundant copy — one more thing to keep in sync
|
||||
// as either changes later through the console.
|
||||
'username' => $isConfigured && $configuredUser !== $resolvedAddress ? $configuredUser : null,
|
||||
'no_reply' => $meta['no_reply'],
|
||||
'active' => true,
|
||||
]);
|
||||
|
|
|
|||
|
|
@ -68,6 +68,84 @@ it('still stores a deliberately configured host and port untouched', function ()
|
|||
->and(Settings::get('mail.port'))->toBe(587);
|
||||
});
|
||||
|
||||
it('seeds address from mail.from.address and keeps the SMTP login as username when they differ', function () {
|
||||
// The installation this reproduces: MAIL_USERNAME is a relay login,
|
||||
// MAIL_FROM_ADDRESS is the address actually in force today. Before this
|
||||
// fix the login was seeded as the From address and username was left
|
||||
// null — invalid mail, or mail sent from the relay login instead of the
|
||||
// sender the system was actually configured to use.
|
||||
clearMailboxSeed();
|
||||
config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com');
|
||||
config()->set('mail.from.address', 'hello@clupilot.local');
|
||||
|
||||
loadMailboxSeedMigration()->up();
|
||||
|
||||
$box = Mailbox::findByKey('no-reply');
|
||||
|
||||
expect($box->address)->toBe('hello@clupilot.local')
|
||||
->and($box->username)->toBe('no-reply@clupilot.com');
|
||||
});
|
||||
|
||||
it('leaves username null when the SMTP login and the From address are the same', function () {
|
||||
// Mailbox::smtpUsername() already falls back to address when username is
|
||||
// null — writing the identical string into both columns is a redundant
|
||||
// copy that only has to drift once (an operator changes one in the
|
||||
// console but not the other) to become a real, harder-to-spot bug.
|
||||
clearMailboxSeed();
|
||||
config()->set('mail.mailers.smtp.username', 'ops@clupilot.com');
|
||||
config()->set('mail.from.address', 'ops@clupilot.com');
|
||||
|
||||
loadMailboxSeedMigration()->up();
|
||||
|
||||
$box = Mailbox::findByKey('no-reply');
|
||||
|
||||
expect($box->address)->toBe('ops@clupilot.com')
|
||||
->and($box->username)->toBeNull();
|
||||
});
|
||||
|
||||
it('falls back to the SMTP login for address when MAIL_FROM_ADDRESS was never set', function () {
|
||||
// config/mail.php's own env('MAIL_FROM_ADDRESS', 'hello@example.com') has
|
||||
// already substituted by the time this migration reads config() — an
|
||||
// install that never set MAIL_FROM_ADDRESS reads back that placeholder,
|
||||
// not null, so it has to be recognised the same way UNCONFIGURED_HOST is.
|
||||
clearMailboxSeed();
|
||||
config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com');
|
||||
config()->set('mail.from.address', 'hello@example.com');
|
||||
|
||||
loadMailboxSeedMigration()->up();
|
||||
|
||||
$box = Mailbox::findByKey('no-reply');
|
||||
|
||||
expect($box->address)->toBe('no-reply@clupilot.com')
|
||||
->and($box->username)->toBeNull();
|
||||
});
|
||||
|
||||
it('seeds display_name from mail.from.name instead of a hardcoded CluPilot', function () {
|
||||
clearMailboxSeed();
|
||||
config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com');
|
||||
config()->set('mail.from.name', 'Example Org Support');
|
||||
|
||||
loadMailboxSeedMigration()->up();
|
||||
|
||||
expect(Mailbox::findByKey('no-reply')->display_name)->toBe('Example Org Support')
|
||||
->and(Mailbox::findByKey('support')->display_name)->toBe('Example Org Support');
|
||||
});
|
||||
|
||||
it('still derives the placeholder domain from the SMTP login, not from mail.from.address', function () {
|
||||
// The two can point at different domains. Only the login's domain is
|
||||
// proven to accept mail for this install — switching the placeholder
|
||||
// domain to mail.from.address's domain could seed support@/billing@/etc.
|
||||
// on a domain nobody could ever turn into a working mailbox.
|
||||
clearMailboxSeed();
|
||||
config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com');
|
||||
config()->set('mail.from.address', 'hello@clupilot.local');
|
||||
|
||||
loadMailboxSeedMigration()->up();
|
||||
|
||||
expect(Mailbox::findByKey('support')->address)->toBe('support@clupilot.com')
|
||||
->and(Mailbox::findByKey('billing')->address)->toBe('billing@clupilot.com');
|
||||
});
|
||||
|
||||
it('carries the vault password across as ciphertext, never decrypting and re-encrypting it', function () {
|
||||
clearMailboxSeed();
|
||||
config()->set('mail.mailers.smtp.username', 'no-reply@clupilot.com');
|
||||
|
|
|
|||
Loading…
Reference in New Issue