diff --git a/app/Notifications/Channels/EmailChannel.php b/app/Notifications/Channels/EmailChannel.php index 429b5a0bb..e255f5aa5 100644 --- a/app/Notifications/Channels/EmailChannel.php +++ b/app/Notifications/Channels/EmailChannel.php @@ -4,13 +4,13 @@ use App\Exceptions\NonReportableException; use App\Models\Team; +use App\Support\SmtpTransportFactory; 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 @@ -82,21 +82,7 @@ public function send(SendsEmail $notifiable, Notification $notification): void 'html' => (string) $mailMessage->render(), ]); } elseif ($isSmtpEnabled) { - $encryption = match (strtolower($settings->smtp_encryption)) { - 'starttls' => null, - 'tls' => 'tls', - 'none' => null, - default => null, - }; - - $transport = new EsmtpTransport( - $settings->smtp_host, - $settings->smtp_port, - $encryption - ); - $transport->setUsername($settings->smtp_username ?? ''); - $transport->setPassword($settings->smtp_password ?? ''); - + $transport = SmtpTransportFactory::fromSettings($settings); $mailer = new Mailer($transport); $email = (new Email) diff --git a/app/Services/ConfigurationRepository.php b/app/Services/ConfigurationRepository.php index 34d8a231f..422612e1e 100644 --- a/app/Services/ConfigurationRepository.php +++ b/app/Services/ConfigurationRepository.php @@ -2,6 +2,7 @@ namespace App\Services; +use App\Support\SmtpTransportFactory; use Illuminate\Config\Repository; use Illuminate\Support\Facades\Mail; @@ -27,25 +28,21 @@ public function updateMailConfig($settings): void } if ($settings->smtp_enabled) { - $encryption = match (strtolower($settings->smtp_encryption)) { - 'starttls' => null, - 'tls' => 'tls', - 'none' => null, - default => null, - }; + $mailerOptions = SmtpTransportFactory::mailerOptions($settings); $this->config->set('mail.default', 'smtp'); $this->applyMailFrom($from); $this->config->set('mail.mailers.smtp', [ 'transport' => 'smtp', + 'scheme' => $mailerOptions['scheme'], 'host' => $settings->smtp_host, 'port' => $settings->smtp_port, - 'encryption' => $encryption, + 'encryption' => $mailerOptions['encryption'], 'username' => $settings->smtp_username, 'password' => $settings->smtp_password, 'timeout' => $settings->smtp_timeout, 'local_domain' => null, - 'auto_tls' => $settings->smtp_encryption === 'none' ? '0' : '', + 'auto_tls' => $mailerOptions['auto_tls'], ]); } } diff --git a/app/Support/SmtpTransportFactory.php b/app/Support/SmtpTransportFactory.php new file mode 100644 index 000000000..1b4d4b09a --- /dev/null +++ b/app/Support/SmtpTransportFactory.php @@ -0,0 +1,63 @@ +smtp_host, + (int) $settings->smtp_port, + match ($mode) { + 'none' => false, + 'tls' => true, + default => null, + } + ); + + if ($mode === 'none') { + $transport->setAutoTls(false); + } + + $transport->setUsername($settings->smtp_username ?? ''); + $transport->setPassword($settings->smtp_password ?? ''); + + $stream = $transport->getStream(); + if (isset($settings->smtp_timeout) && $stream instanceof SocketStream) { + $stream->setTimeout((float) $settings->smtp_timeout); + } + + return $transport; + } + + /** + * @return array{scheme: ?string, encryption: ?string, auto_tls: string} + */ + public static function mailerOptions(object $settings): array + { + $mode = self::encryptionMode($settings); + + return [ + 'scheme' => match ($mode) { + 'none', 'starttls' => 'smtp', + 'tls' => 'smtps', + default => null, + }, + 'encryption' => $mode === 'tls' ? 'tls' : null, + 'auto_tls' => $mode === 'none' ? '0' : '', + ]; + } + + private static function encryptionMode(object $settings): ?string + { + return $settings->smtp_encryption === null + ? null + : strtolower((string) $settings->smtp_encryption); + } +} diff --git a/tests/Unit/ConfigurationRepositoryMailConfigTest.php b/tests/Unit/ConfigurationRepositoryMailConfigTest.php new file mode 100644 index 000000000..e5592b5e4 --- /dev/null +++ b/tests/Unit/ConfigurationRepositoryMailConfigTest.php @@ -0,0 +1,29 @@ +updateMailConfig((object) [ + 'resend_enabled' => false, + 'smtp_enabled' => true, + 'smtp_encryption' => $mode, + 'smtp_host' => 'smtp.example.com', + 'smtp_port' => $port, + 'smtp_username' => 'user', + 'smtp_password' => 'secret', + 'smtp_timeout' => null, + 'smtp_from_address' => 'from@example.com', + 'smtp_from_name' => 'Coolify', + ]); + + expect($config->get('mail.mailers.smtp.scheme'))->toBe($scheme) + ->and($config->get('mail.mailers.smtp.encryption'))->toBe($encryption) + ->and($config->get('mail.mailers.smtp.auto_tls'))->toBe($autoTls); +})->with([ + 'none on port 465' => ['none', 465, 'smtp', null, '0'], + 'tls on a non-465 port' => ['tls', 587, 'smtps', 'tls', ''], +]); diff --git a/tests/Unit/SmtpTransportFactoryTest.php b/tests/Unit/SmtpTransportFactoryTest.php new file mode 100644 index 000000000..168547661 --- /dev/null +++ b/tests/Unit/SmtpTransportFactoryTest.php @@ -0,0 +1,112 @@ + 'smtp.example.com', + 'smtp_port' => 25, + 'smtp_encryption' => 'none', + 'smtp_username' => 'user', + 'smtp_password' => 'secret', + 'smtp_timeout' => null, + ], $overrides); +} + +it('disables opportunistic STARTTLS when encryption is none', function () { + $transport = SmtpTransportFactory::fromSettings(smtpSettings([ + 'smtp_encryption' => 'none', + ])); + + expect($transport->isAutoTls())->toBeFalse() + ->and($transport->getStream())->toBeInstanceOf(SocketStream::class) + ->and($transport->getStream()->isTLS())->toBeFalse(); +}); + +it('does not issue STARTTLS for the issue 5877 anonymous port 25 relay', function () { + $transport = SmtpTransportFactory::fromSettings(smtpSettings([ + 'smtp_encryption' => 'none', + 'smtp_port' => 25, + 'smtp_username' => '', + 'smtp_password' => '', + ])); + + expect($transport->isAutoTls())->toBeFalse() + ->and($transport->getStream()->isTLS())->toBeFalse() + ->and($transport->getStream()->getPort())->toBe(25) + ->and($transport->getUsername())->toBe('') + ->and($transport->getPassword())->toBe(''); +}); + +it('does not enable implicit TLS on port 465 when encryption is none', function () { + $transport = SmtpTransportFactory::fromSettings(smtpSettings([ + 'smtp_encryption' => 'none', + 'smtp_port' => 465, + ])); + + expect($transport->isAutoTls())->toBeFalse() + ->and($transport->getStream()->isTLS())->toBeFalse(); +}); + +it('keeps opportunistic STARTTLS when encryption is starttls', function () { + $transport = SmtpTransportFactory::fromSettings(smtpSettings([ + 'smtp_encryption' => 'starttls', + ])); + + expect($transport->isAutoTls())->toBeTrue() + ->and($transport->getStream()->isTLS())->toBeFalse(); +}); + +it('uses implicit TLS when encryption is tls', function () { + $transport = SmtpTransportFactory::fromSettings(smtpSettings([ + 'smtp_encryption' => 'tls', + 'smtp_port' => 465, + ])); + + expect($transport->isAutoTls())->toBeTrue() + ->and($transport->getStream()->isTLS())->toBeTrue(); +}); + +it('infers implicit TLS on port 465 when encryption is null', function () { + $transport = SmtpTransportFactory::fromSettings(smtpSettings([ + 'smtp_encryption' => null, + 'smtp_port' => 465, + ])); + + expect($transport->getStream()->isTLS())->toBeTrue(); +}); + +it('applies a configured SMTP timeout to the transport stream', function () { + $transport = SmtpTransportFactory::fromSettings(smtpSettings([ + 'smtp_timeout' => 15, + ])); + + expect($transport->getStream()->getTimeout())->toBe(15.0); +}); + +it('maps none encryption to laravel mailer options that disable auto tls', function () { + expect(SmtpTransportFactory::mailerOptions(smtpSettings([ + 'smtp_encryption' => 'none', + ])))->toBe([ + 'scheme' => 'smtp', + 'encryption' => null, + 'auto_tls' => '0', + ]); +}); + +it('maps encryption to laravel mailer options', function (?string $mode, ?string $scheme, ?string $encryption) { + expect(SmtpTransportFactory::mailerOptions(smtpSettings([ + 'smtp_encryption' => $mode, + ])))->toBe([ + 'scheme' => $scheme, + 'encryption' => $encryption, + 'auto_tls' => $mode === 'none' ? '0' : '', + ]); +})->with([ + 'none' => ['none', 'smtp', null], + 'starttls' => ['starttls', 'smtp', null], + 'tls' => ['tls', 'smtps', 'tls'], + 'unset' => [null, null, null], +]);