diff --git a/3rdparty b/3rdparty index 7d055b0645..b995ca8b8c 160000 --- a/3rdparty +++ b/3rdparty @@ -1 +1 @@ -Subproject commit 7d055b064564a2bc5c4109860b12b6b30202f508 +Subproject commit b995ca8b8c7f69a180ad6fb49989ad3e35b1367e diff --git a/config/config.sample.php b/config/config.sample.php index 68f27ed032..5c6ca58a64 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -328,24 +328,19 @@ $CONFIG = array( 'mail_smtpdebug' => false, /** - * Which mode to use for sending mail: ``sendmail``, ``smtp``, ``qmail`` or - * ``php``. + * Which mode to use for sending mail: ``sendmail``, ``smtp`` or ``qmail``. * * If you are using local or remote SMTP, set this to ``smtp``. * - * If you are using PHP mail you must have an installed and working email system - * on the server. The program used to send email is defined in the ``php.ini`` - * file. - * * For the ``sendmail`` option you need an installed and working email system on * the server, with ``/usr/sbin/sendmail`` installed on your Unix system. * * For ``qmail`` the binary is /var/qmail/bin/sendmail, and it must be installed * on your Unix system. * - * Defaults to ``php`` + * Defaults to ``smtp`` */ -'mail_smtpmode' => 'php', +'mail_smtpmode' => 'smtp', /** * This depends on ``mail_smtpmode``. Specify the IP address of your mail diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index eae0abae50..75d335043a 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -293,6 +293,18 @@ type: OC.SetupChecks.MESSAGE_TYPE_WARNING }) } + if (data.isPhpMailerUsed) { + messages.push({ + msg: t( + 'core', + 'Use of the the built in php mailer is no longer supported. Please update your email server settings ↗.', + { + docLink: data.mailSettingsDocumentation, + } + ), + type: OC.SetupChecks.MESSAGE_TYPE_WARNING + }); + } } else { messages.push({ msg: t('core', 'Error occurred while checking server setup'), diff --git a/lib/private/Mail/Mailer.php b/lib/private/Mail/Mailer.php index 001f7bd75c..6f148bc0c6 100644 --- a/lib/private/Mail/Mailer.php +++ b/lib/private/Mail/Mailer.php @@ -26,6 +26,8 @@ declare(strict_types=1); namespace OC\Mail; +use Egulias\EmailValidator\EmailValidator; +use Egulias\EmailValidator\Validation\RFCValidation; use OCP\Defaults; use OCP\IConfig; use OCP\IL10N; @@ -55,7 +57,7 @@ use OCP\Mail\IMessage; * @package OC\Mail */ class Mailer implements IMailer { - /** @var \Swift_SmtpTransport|\Swift_SendmailTransport|\Swift_MailTransport Cached transport */ + /** @var \Swift_Mailer Cached mailer */ private $instance = null; /** @var IConfig */ private $config; @@ -105,7 +107,7 @@ class Mailer implements IMailer { * @since 13.0.0 */ public function createAttachment($data = null, $filename = null, $contentType = null): IAttachment { - return new Attachment(\Swift_Attachment::newInstance($data, $filename, $contentType)); + return new Attachment(new \Swift_Attachment($data, $filename, $contentType)); } /** @@ -194,7 +196,10 @@ class Mailer implements IMailer { * @return bool True if the mail address is valid, false otherwise */ public function validateMailAddress(string $email): bool { - return \Swift_Validate::email($this->convertEmail($email)); + $validator = new EmailValidator(); + $validation = new RFCValidation(); + + return $validator->isValid($this->convertEmail($email), $validation); } /** @@ -215,32 +220,24 @@ class Mailer implements IMailer { return $name.'@'.$domain; } - /** - * Returns whatever transport is configured within the config - * - * @return \Swift_SmtpTransport|\Swift_SendmailTransport|\Swift_MailTransport - */ - protected function getInstance() { + protected function getInstance(): \Swift_Mailer { if (!is_null($this->instance)) { return $this->instance; } - switch ($this->config->getSystemValue('mail_smtpmode', 'php')) { - case 'smtp': - $this->instance = $this->getSmtpInstance(); - break; + $transport = null; + + switch ($this->config->getSystemValue('mail_smtpmode', 'smtp')) { case 'sendmail': - // FIXME: Move into the return statement but requires proper testing - // for SMTP and mail as well. Thus not really doable for a - // minor release. - $this->instance = \Swift_Mailer::newInstance($this->getSendMailInstance()); + $transport = $this->getSendMailInstance(); break; + case 'smtp': default: - $this->instance = $this->getMailInstance(); + $transport = $this->getSmtpInstance(); break; } - return $this->instance; + return new \Swift_Mailer($transport); } /** @@ -249,7 +246,7 @@ class Mailer implements IMailer { * @return \Swift_SmtpTransport */ protected function getSmtpInstance(): \Swift_SmtpTransport { - $transport = \Swift_SmtpTransport::newInstance(); + $transport = new \Swift_SmtpTransport(); $transport->setTimeout($this->config->getSystemValue('mail_smtptimeout', 10)); $transport->setHost($this->config->getSystemValue('mail_smtphost', '127.0.0.1')); $transport->setPort($this->config->getSystemValue('mail_smtpport', 25)); @@ -262,7 +259,7 @@ class Mailer implements IMailer { if (!empty($smtpSecurity)) { $transport->setEncryption($smtpSecurity); } - $transport->start(); + return $transport; } @@ -272,7 +269,7 @@ class Mailer implements IMailer { * @return \Swift_SendmailTransport */ protected function getSendMailInstance(): \Swift_SendmailTransport { - switch ($this->config->getSystemValue('mail_smtpmode', 'php')) { + switch ($this->config->getSystemValue('mail_smtpmode', 'smtp')) { case 'qmail': $binaryPath = '/var/qmail/bin/sendmail'; break; @@ -281,16 +278,6 @@ class Mailer implements IMailer { break; } - return \Swift_SendmailTransport::newInstance($binaryPath . ' -bs'); + return new \Swift_SendmailTransport($binaryPath . ' -bs'); } - - /** - * Returns the mail transport - * - * @return \Swift_MailTransport - */ - protected function getMailInstance(): \Swift_MailTransport { - return \Swift_MailTransport::newInstance(); - } - } diff --git a/lib/private/Settings/Admin/Mail.php b/lib/private/Settings/Admin/Mail.php index 74a94a4c7a..fc20b7eeb3 100644 --- a/lib/private/Settings/Admin/Mail.php +++ b/lib/private/Settings/Admin/Mail.php @@ -63,6 +63,10 @@ class Mail implements ISettings { $parameters['mail_smtppassword'] = '********'; } + if ($parameters['mail_smtpmode'] === '' || $parameters['mail_smtpmode'] === 'php') { + $parameters['mail_smtpmode'] = 'smtp'; + } + return new TemplateResponse('settings', 'settings/admin/additional-mail', $parameters, ''); } diff --git a/settings/Controller/CheckSetupController.php b/settings/Controller/CheckSetupController.php index ecbb9839c7..a301ecb1f6 100644 --- a/settings/Controller/CheckSetupController.php +++ b/settings/Controller/CheckSetupController.php @@ -523,6 +523,10 @@ Raw output return []; } + protected function isPhpMailerUsed(): bool { + return $this->config->getSystemValue('mail_smtpmode', 'php') === 'php'; + } + /** * @return DataResponse */ @@ -557,6 +561,8 @@ Raw output 'missingIndexes' => $this->hasMissingIndexes(), 'isSqliteUsed' => $this->isSqliteUsed(), 'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'), + 'isPhpMailerUsed' => $this->isPhpMailerUsed(), + 'mailSettingsDocumentation' => $this->urlGenerator->getAbsoluteURL('index.php/settings/admin') ] ); } diff --git a/settings/templates/settings/admin/additional-mail.php b/settings/templates/settings/admin/additional-mail.php index adcc5293ff..1e0b458f60 100644 --- a/settings/templates/settings/admin/additional-mail.php +++ b/settings/templates/settings/admin/additional-mail.php @@ -38,7 +38,6 @@ $mail_smtpsecure = [ ]; $mail_smtpmode = [ - ['php', 'PHP'], ['smtp', 'SMTP'], ]; if ($_['sendmail_is_available']) { diff --git a/tests/Settings/Controller/CheckSetupControllerTest.php b/tests/Settings/Controller/CheckSetupControllerTest.php index 470bc9cde6..057774a45b 100644 --- a/tests/Settings/Controller/CheckSetupControllerTest.php +++ b/tests/Settings/Controller/CheckSetupControllerTest.php @@ -119,7 +119,22 @@ class CheckSetupControllerTest extends TestCase { $this->lockingProvider, $this->dateTimeFormatter, ]) - ->setMethods(['isReadOnlyConfig', 'hasValidTransactionIsolationLevel', 'hasFileinfoInstalled', 'hasWorkingFileLocking', 'getLastCronInfo', 'getSuggestedOverwriteCliURL', 'getOutdatedCaches', 'getCurlVersion', 'isPhpOutdated', 'isOpcacheProperlySetup', 'hasFreeTypeSupport', 'hasMissingIndexes', 'isSqliteUsed'])->getMock(); + ->setMethods([ + 'isReadOnlyConfig', + 'hasValidTransactionIsolationLevel', + 'hasFileinfoInstalled', + 'hasWorkingFileLocking', + 'getLastCronInfo', + 'getSuggestedOverwriteCliURL', + 'getOutdatedCaches', + 'getCurlVersion', + 'isPhpOutdated', + 'isOpcacheProperlySetup', + 'hasFreeTypeSupport', + 'hasMissingIndexes', + 'isSqliteUsed', + 'isPhpMailerUsed', + ])->getMock(); } public function testIsInternetConnectionWorkingDisabledViaConfig() { @@ -352,6 +367,10 @@ class CheckSetupControllerTest extends TestCase { ->method('linkToDocs') ->with('admin-db-conversion') ->willReturn('http://docs.example.org/server/go.php?to=admin-db-conversion'); + $this->urlGenerator->expects($this->at(6)) + ->method('getAbsoluteURL') + ->with('index.php/settings/admin') + ->willReturn('https://server/index.php/settings/admin'); $this->checkSetupController ->method('hasFreeTypeSupport') ->willReturn(false); @@ -392,6 +411,10 @@ class CheckSetupControllerTest extends TestCase { 'relativeTime' => '2 hours ago', 'backgroundJobsUrl' => 'https://example.org', ]); + $this->checkSetupController + ->expects($this->once()) + ->method('isPhpMailerUsed') + ->willReturn(false); $this->checker ->expects($this->once()) ->method('hasPassedCheck') @@ -434,6 +457,8 @@ class CheckSetupControllerTest extends TestCase { 'isSqliteUsed' => false, 'databaseConversionDocumentation' => 'http://docs.example.org/server/go.php?to=admin-db-conversion', 'missingIndexes' => [], + 'isPhpMailerUsed' => false, + 'mailSettingsDocumentation' => 'https://server/index.php/settings/admin', ] ); $this->assertEquals($expected, $this->checkSetupController->check()); diff --git a/tests/lib/Mail/MailerTest.php b/tests/lib/Mail/MailerTest.php index 2dd4bca519..d724cd630d 100644 --- a/tests/lib/Mail/MailerTest.php +++ b/tests/lib/Mail/MailerTest.php @@ -48,50 +48,41 @@ class MailerTest extends TestCase { ); } - public function testGetMailInstance() { - $this->assertEquals(\Swift_MailTransport::newInstance(), self::invokePrivate($this->mailer, 'getMailinstance')); - } - public function testGetSendMailInstanceSendMail() { $this->config ->expects($this->once()) ->method('getSystemValue') - ->with('mail_smtpmode', 'php') + ->with('mail_smtpmode', 'smtp') ->will($this->returnValue('sendmail')); - $this->assertEquals(\Swift_SendmailTransport::newInstance('/usr/sbin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance')); + $this->assertEquals(new \Swift_SendmailTransport('/usr/sbin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance')); } public function testGetSendMailInstanceSendMailQmail() { $this->config ->expects($this->once()) ->method('getSystemValue') - ->with('mail_smtpmode', 'php') + ->with('mail_smtpmode', 'smtp') ->will($this->returnValue('qmail')); - $this->assertEquals(\Swift_SendmailTransport::newInstance('/var/qmail/bin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance')); + $this->assertEquals(new \Swift_SendmailTransport('/var/qmail/bin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance')); } public function testGetInstanceDefault() { - $this->assertInstanceOf('\Swift_MailTransport', self::invokePrivate($this->mailer, 'getInstance')); - } - - public function testGetInstancePhp() { - $this->config - ->expects($this->any()) - ->method('getSystemValue') - ->will($this->returnValue('php')); - - $this->assertInstanceOf('\Swift_MailTransport', self::invokePrivate($this->mailer, 'getInstance')); + $mailer = self::invokePrivate($this->mailer, 'getInstance'); + $this->assertInstanceOf(\Swift_Mailer::class, $mailer); + $this->assertInstanceOf(\Swift_SmtpTransport::class, $mailer->getTransport()); } public function testGetInstanceSendmail() { $this->config - ->expects($this->any()) ->method('getSystemValue') - ->will($this->returnValue('sendmail')); + ->with('mail_smtpmode', 'smtp') + ->willReturn('sendmail'); - $this->assertInstanceOf('\Swift_Mailer', self::invokePrivate($this->mailer, 'getInstance')); + $mailer = self::invokePrivate($this->mailer, 'getInstance'); + $this->assertInstanceOf(\Swift_Mailer::class, $mailer); + $this->assertInstanceOf(\Swift_SendmailTransport::class, $mailer->getTransport()); } public function testCreateMessage() { diff --git a/tests/lib/Settings/Admin/MailTest.php b/tests/lib/Settings/Admin/MailTest.php index 04c0e7fa92..436a795322 100644 --- a/tests/lib/Settings/Admin/MailTest.php +++ b/tests/lib/Settings/Admin/MailTest.php @@ -59,7 +59,7 @@ class MailTest extends TestCase { ->expects($this->at(2)) ->method('getSystemValue') ->with('mail_smtpmode', '') - ->willReturn('php'); + ->willReturn('smtp'); $this->config ->expects($this->at(3)) ->method('getSystemValue') @@ -103,7 +103,7 @@ class MailTest extends TestCase { 'sendmail_is_available' => (bool) \OC_Helper::findBinaryPath('sendmail'), 'mail_domain' => 'mx.nextcloud.com', 'mail_from_address' => 'no-reply@nextcloud.com', - 'mail_smtpmode' => 'php', + 'mail_smtpmode' => 'smtp', 'mail_smtpsecure' => true, 'mail_smtphost' => 'smtp.nextcloud.com', 'mail_smtpport' => 25,