From 41f7152441817d857d6b1d5ddd765eead63bd4e4 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:43:25 +0200 Subject: [PATCH] fix(email): apply configured sender identity (#11393) --- app/Livewire/SettingsEmail.php | 9 ++ app/Notifications/Channels/EmailChannel.php | 29 +++-- .../Channels/TransactionalEmailChannel.php | 2 + .../TransactionalEmails/ResetPassword.php | 2 + app/Services/ConfigurationRepository.php | 22 +++- bootstrap/helpers/notifications.php | 36 ++++++ .../TransactionalEmailFromNameTest.php | 121 ++++++++++++++++++ tests/Unit/MailFromIdentityTest.php | 52 ++++++++ 8 files changed, 255 insertions(+), 18 deletions(-) create mode 100644 tests/Feature/TransactionalEmailFromNameTest.php create mode 100644 tests/Unit/MailFromIdentityTest.php diff --git a/app/Livewire/SettingsEmail.php b/app/Livewire/SettingsEmail.php index 121fe92a48..cef2c254c2 100644 --- a/app/Livewire/SettingsEmail.php +++ b/app/Livewire/SettingsEmail.php @@ -234,11 +234,20 @@ class SettingsEmail extends Component $this->authorize('update', $this->settings); $this->validate([ 'testEmailAddress' => 'required|email', + 'smtpFromAddress' => 'required|email', + 'smtpFromName' => 'required|string', ], [ 'testEmailAddress.required' => 'Test email address is required.', 'testEmailAddress.email' => 'Please enter a valid email address.', + 'smtpFromAddress.required' => 'From Address is required.', + 'smtpFromAddress.email' => 'Please enter a valid email address.', + 'smtpFromName.required' => 'From Name is required.', ]); + $this->settings->smtp_from_address = $this->smtpFromAddress; + $this->settings->smtp_from_name = $this->smtpFromName; + $this->settings->save(); + $executed = RateLimiter::attempt( 'test-email:'.$this->team->id, $perMinute = 0, diff --git a/app/Notifications/Channels/EmailChannel.php b/app/Notifications/Channels/EmailChannel.php index abd1155502..429b5a0bb7 100644 --- a/app/Notifications/Channels/EmailChannel.php +++ b/app/Notifications/Channels/EmailChannel.php @@ -7,6 +7,11 @@ use App\Models\Team; use Exception; use Illuminate\Notifications\Notification; use Resend; +use Resend\Exceptions\ErrorException; +use Resend\Exceptions\TransporterException; +use Symfony\Component\Mailer\Mailer; +use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport; +use Symfony\Component\Mime\Email; class EmailChannel { @@ -70,9 +75,8 @@ class EmailChannel if ($isResendEnabled) { $resend = Resend::client($settings->resend_api_key); - $from = "{$settings->smtp_from_name} <{$settings->smtp_from_address}>"; $resend->emails->send([ - 'from' => $from, + 'from' => mail_from_formatted($settings), 'to' => $recipients, 'subject' => $mailMessage->subject, 'html' => (string) $mailMessage->render(), @@ -85,7 +89,7 @@ class EmailChannel default => null, }; - $transport = new \Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport( + $transport = new EsmtpTransport( $settings->smtp_host, $settings->smtp_port, $encryption @@ -93,20 +97,17 @@ class EmailChannel $transport->setUsername($settings->smtp_username ?? ''); $transport->setPassword($settings->smtp_password ?? ''); - $mailer = new \Symfony\Component\Mailer\Mailer($transport); + $mailer = new Mailer($transport); - $fromEmail = $settings->smtp_from_address ?? 'noreply@localhost'; - $fromName = $settings->smtp_from_name ?? 'System'; - $from = new \Symfony\Component\Mime\Address($fromEmail, $fromName); - $email = (new \Symfony\Component\Mime\Email) - ->from($from) + $email = (new Email) + ->from(mail_from_address($settings)) ->to(...$recipients) ->subject($mailMessage->subject) ->html((string) $mailMessage->render()); $mailer->send($email); } - } catch (\Resend\Exceptions\ErrorException $e) { + } catch (ErrorException $e) { // Map HTTP status codes to user-friendly messages $userMessage = match ($e->getErrorCode()) { 403 => 'Invalid Resend API key. Please verify your API key in the Resend dashboard and update it in settings.', @@ -131,13 +132,13 @@ class EmailChannel // Don't report expected errors (invalid keys, validation) to Sentry if (in_array($e->getErrorCode(), [403, 401, 400])) { - throw NonReportableException::fromException(new \Exception($userMessage, $e->getCode(), $e)); + throw NonReportableException::fromException(new Exception($userMessage, $e->getCode(), $e)); } - throw new \Exception($userMessage, $e->getCode(), $e); - } catch (\Resend\Exceptions\TransporterException $e) { + throw new Exception($userMessage, $e->getCode(), $e); + } catch (TransporterException $e) { send_internal_notification("Resend Transport Error: {$e->getMessage()}"); - throw new \Exception('Unable to connect to Resend API. Please check your internet connection and try again.'); + throw new Exception('Unable to connect to Resend API. Please check your internet connection and try again.'); } catch (\Throwable $e) { // Check if this is a Resend domain verification error on cloud instances if (isCloud() && str_contains($e->getMessage(), 'domain is not verified')) { diff --git a/app/Notifications/Channels/TransactionalEmailChannel.php b/app/Notifications/Channels/TransactionalEmailChannel.php index 8ab74a60b1..83c61f6d0f 100644 --- a/app/Notifications/Channels/TransactionalEmailChannel.php +++ b/app/Notifications/Channels/TransactionalEmailChannel.php @@ -27,10 +27,12 @@ class TransactionalEmailChannel } $this->bootConfigs(); $mailMessage = $notification->toMail($notifiable); + $from = mail_from_identity($settings); Mail::send( [], [], fn (Message $message) => $message + ->from($from['address'], $from['name']) ->to($email) ->subject($mailMessage->subject) ->html((string) $mailMessage->render()) diff --git a/app/Notifications/TransactionalEmails/ResetPassword.php b/app/Notifications/TransactionalEmails/ResetPassword.php index 511818e215..0e33bf495f 100644 --- a/app/Notifications/TransactionalEmails/ResetPassword.php +++ b/app/Notifications/TransactionalEmails/ResetPassword.php @@ -54,7 +54,9 @@ class ResetPassword extends Notification protected function buildMailMessage($url) { + $from = mail_from_identity($this->settings); $mail = new MailMessage; + $mail->from($from['address'], $from['name']); $mail->subject('Coolify: Reset Password'); $mail->view('emails.reset-password', ['url' => $url, 'count' => config('auth.passwords.'.config('auth.defaults.passwords').'.expire')]); diff --git a/app/Services/ConfigurationRepository.php b/app/Services/ConfigurationRepository.php index ff2e73eed6..34d8a231fb 100644 --- a/app/Services/ConfigurationRepository.php +++ b/app/Services/ConfigurationRepository.php @@ -3,6 +3,7 @@ namespace App\Services; use Illuminate\Config\Repository; +use Illuminate\Support\Facades\Mail; class ConfigurationRepository { @@ -15,10 +16,11 @@ class ConfigurationRepository public function updateMailConfig($settings): void { + $from = mail_from_identity($settings); + if ($settings->resend_enabled) { $this->config->set('mail.default', 'resend'); - $this->config->set('mail.from.address', $settings->smtp_from_address ?? 'test@example.com'); - $this->config->set('mail.from.name', $settings->smtp_from_name ?? 'Test'); + $this->applyMailFrom($from); $this->config->set('resend.api_key', $settings->resend_api_key); return; @@ -33,8 +35,7 @@ class ConfigurationRepository }; $this->config->set('mail.default', 'smtp'); - $this->config->set('mail.from.address', $settings->smtp_from_address ?? 'test@example.com'); - $this->config->set('mail.from.name', $settings->smtp_from_name ?? 'Test'); + $this->applyMailFrom($from); $this->config->set('mail.mailers.smtp', [ 'transport' => 'smtp', 'host' => $settings->smtp_host, @@ -49,6 +50,19 @@ class ConfigurationRepository } } + /** + * @param array{address: string, name: string} $from + */ + private function applyMailFrom(array $from): void + { + $this->config->set('mail.from.address', $from['address']); + $this->config->set('mail.from.name', $from['name']); + + if (app()->bound('mail.manager')) { + Mail::purge(); + } + } + public function disableSshMux(): void { $this->config->set('constants.ssh.mux_enabled', false); diff --git a/bootstrap/helpers/notifications.php b/bootstrap/helpers/notifications.php index f64535ea82..c5b4388807 100644 --- a/bootstrap/helpers/notifications.php +++ b/bootstrap/helpers/notifications.php @@ -5,6 +5,7 @@ use App\Notifications\Internal\GeneralNotification; use Illuminate\Mail\Message; use Illuminate\Notifications\Messages\MailMessage; use Illuminate\Support\Facades\Mail; +use Symfony\Component\Mime\Address; function is_transactional_emails_enabled(): bool { @@ -13,6 +14,37 @@ function is_transactional_emails_enabled(): bool return $settings->smtp_enabled || $settings->resend_enabled; } +/** + * @return array{address: string, name: string} + */ +function mail_from_identity(object $settings): array +{ + if (blank($settings->smtp_from_address ?? null)) { + throw new InvalidArgumentException('Transactional email sender address is not configured.'); + } + + $address = (string) $settings->smtp_from_address; + + $name = trim((string) ($settings->smtp_from_name ?? '')); + + return [ + 'address' => $address, + 'name' => $name !== '' ? $name : 'Coolify', + ]; +} + +function mail_from_address(object $settings): Address +{ + $identity = mail_from_identity($settings); + + return new Address($identity['address'], $identity['name']); +} + +function mail_from_formatted(object $settings): string +{ + return mail_from_address($settings)->toString(); +} + function send_internal_notification(string $message): void { try { @@ -29,11 +61,14 @@ function send_user_an_email(MailMessage $mail, string $email, ?string $cc = null if (blank($type)) { throw new Exception('No email settings found.'); } + $from = mail_from_identity($settings); + if ($cc) { Mail::send( [], [], fn (Message $message) => $message + ->from($from['address'], $from['name']) ->to($email) ->replyTo($email) ->cc($cc) @@ -45,6 +80,7 @@ function send_user_an_email(MailMessage $mail, string $email, ?string $cc = null [], [], fn (Message $message) => $message + ->from($from['address'], $from['name']) ->to($email) ->subject($mail->subject) ->html((string) $mail->render()) diff --git a/tests/Feature/TransactionalEmailFromNameTest.php b/tests/Feature/TransactionalEmailFromNameTest.php new file mode 100644 index 0000000000..2e5662c61f --- /dev/null +++ b/tests/Feature/TransactionalEmailFromNameTest.php @@ -0,0 +1,121 @@ +create(['id' => 0]); + InstanceSettings::forceCreate([ + 'id' => 0, + 'smtp_enabled' => true, + 'smtp_from_address' => 'admin@example.com', + 'smtp_from_name' => 'admin', + 'smtp_host' => 'coolify-mail', + 'smtp_port' => 1025, + ]); + Once::flush(); + + $user = User::factory()->create(); + $rootTeam->members()->attach($user->id, ['role' => 'admin']); + + return $user; +} + +test('saving transactional sender persists the configured from name', function () { + $user = setupTransactionalEmailFromNameAdmin(); + + $this->actingAs($user); + session(['currentTeam' => ['id' => 0]]); + + Livewire::test(SettingsEmail::class) + ->set('smtpFromName', 'Coolify') + ->set('smtpFromAddress', 'admin@example.com') + ->call('submit') + ->assertHasNoErrors(); + + Once::flush(); + + expect(instanceSettings()->smtp_from_name)->toBe('Coolify') + ->and(instanceSettings()->smtp_from_address)->toBe('admin@example.com'); +}); + +test('sending a test email persists the current from name before delivery', function () { + $user = setupTransactionalEmailFromNameAdmin(); + + $this->actingAs($user); + session(['currentTeam' => ['id' => 0]]); + + Notification::fake(); + + Livewire::test(SettingsEmail::class) + ->set('smtpFromName', 'Coolify') + ->set('smtpFromAddress', 'admin@example.com') + ->set('testEmailAddress', $user->email) + ->call('sendTestEmail') + ->assertHasNoErrors(); + + Once::flush(); + + expect(instanceSettings()->smtp_from_name)->toBe('Coolify'); +}); + +test('transactional emails send with the configured from name instead of the address local part', function () { + setupTransactionalEmailFromNameAdmin(); + + InstanceSettings::findOrFail(0)->update([ + 'smtp_from_name' => 'Coolify', + 'smtp_from_address' => 'admin@example.com', + ]); + Once::flush(); + + config([ + 'mail.default' => 'array', + 'mail.from.address' => 'hello@example.com', + 'mail.from.name' => 'Example', + ]); + Mail::purge(); + Mail::mailer(); + + $this->mock(ConfigurationRepository::class, function ($mock) { + $mock->shouldReceive('updateMailConfig')->andReturnUsing(function ($settings) { + config([ + 'mail.from.address' => $settings->smtp_from_address, + 'mail.from.name' => $settings->smtp_from_name, + ]); + }); + }); + + $user = User::factory()->create(['email' => 'recipient@example.com']); + $notification = new EmailChangeVerification( + $user, + '123456', + 'new@example.com', + now()->addMinutes(10), + ); + + $channel = new TransactionalEmailChannel; + $channel->send($user, $notification); + + $messages = app('mail.manager')->mailer('array')->getSymfonyTransport()->messages(); + + expect($messages)->not->toBeEmpty(); + + $from = $messages->first()->getOriginalMessage()->getFrom()[0]; + + expect($from->getAddress())->toBe('admin@example.com') + ->and($from->getName())->toBe('Coolify') + ->and($from->getName())->not->toBe('admin'); +}); diff --git a/tests/Unit/MailFromIdentityTest.php b/tests/Unit/MailFromIdentityTest.php new file mode 100644 index 0000000000..4a317c91ca --- /dev/null +++ b/tests/Unit/MailFromIdentityTest.php @@ -0,0 +1,52 @@ + 'admin@example.com', + 'smtp_from_name' => 'Coolify', + ]); + + expect($identity['address'])->toBe('admin@example.com') + ->and($identity['name'])->toBe('Coolify'); +}); + +it('does not fall back to the email local part when a from name is set', function () { + $address = mail_from_address((object) [ + 'smtp_from_address' => 'admin@example.com', + 'smtp_from_name' => 'Coolify', + ]); + + expect($address)->toBeInstanceOf(Address::class) + ->and($address->getAddress())->toBe('admin@example.com') + ->and($address->getName())->toBe('Coolify') + ->and($address->getName())->not->toBe('admin'); +}); + +it('formats the transactional sender for resend', function () { + $formattedAddress = mail_from_formatted((object) [ + 'smtp_from_address' => 'admin@example.com', + 'smtp_from_name' => 'Coolify', + ]); + + expect($formattedAddress)->toBe('"Coolify" '); +}); + +it('treats a blank from name as missing instead of sending an unnamed address', function () { + $identity = mail_from_identity((object) [ + 'smtp_from_address' => 'admin@example.com', + 'smtp_from_name' => ' ', + ]); + + expect($identity['name'])->toBe('Coolify') + ->and($identity['name'])->not->toBe('admin'); +}); + +it('rejects enabled email settings without a from address', function () { + mail_from_identity((object) [ + 'smtp_enabled' => true, + 'smtp_from_address' => null, + 'smtp_from_name' => 'Coolify', + ]); +})->throws(InvalidArgumentException::class, 'Transactional email sender address is not configured.');