diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 3c7d5a5c0aba0..11900fad45b9e 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -572,6 +572,20 @@ protected function isReadOnlyConfig(): bool { return \OC_Helper::isReadOnlyConfigEnabled(); } + protected function wasEmailTestSuccessful(): bool { + // Handle the case that the configuration was set before the check was introduced or it was only set via command line and not from the UI + if ($this->config->getAppValue('core', 'emailTestSuccessful', '') === '' && $this->config->getSystemValue('mail_domain', '') === '') { + return false; + } + + // The mail test was unsuccessful or the config was changed using the UI without verifying with a testmail, hence return false + if ($this->config->getAppValue('core', 'emailTestSuccessful', '') === '0') { + return false; + } + + return true; + } + protected function hasValidTransactionIsolationLevel(): bool { try { if ($this->db->getDatabasePlatform() instanceof SqlitePlatform) { @@ -822,6 +836,7 @@ public function check() { 'isGetenvServerWorking' => !empty(getenv('PATH')), 'isReadOnlyConfig' => $this->isReadOnlyConfig(), 'hasValidTransactionIsolationLevel' => $this->hasValidTransactionIsolationLevel(), + 'wasEmailTestSuccessful' => $this->wasEmailTestSuccessful(), 'hasFileinfoInstalled' => $this->hasFileinfoInstalled(), 'hasWorkingFileLocking' => $this->hasWorkingFileLocking(), 'suggestedOverwriteCliURL' => $this->getSuggestedOverwriteCliURL(), diff --git a/apps/settings/lib/Controller/MailSettingsController.php b/apps/settings/lib/Controller/MailSettingsController.php index 04143cc7fa7d9..22c0622a0722d 100644 --- a/apps/settings/lib/Controller/MailSettingsController.php +++ b/apps/settings/lib/Controller/MailSettingsController.php @@ -33,6 +33,7 @@ use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\IUserSession; use OCP\Mail\IMailer; @@ -46,6 +47,8 @@ class MailSettingsController extends Controller { private $userSession; /** @var IMailer */ private $mailer; + /** @var IURLGenerator */ + private $urlGenerator; /** * @param string $appName @@ -53,6 +56,7 @@ class MailSettingsController extends Controller { * @param IL10N $l10n * @param IConfig $config * @param IUserSession $userSession + * @param IURLGenerator $urlGenerator, * @param IMailer $mailer */ public function __construct($appName, @@ -60,11 +64,13 @@ public function __construct($appName, IL10N $l10n, IConfig $config, IUserSession $userSession, + IURLGenerator $urlGenerator, IMailer $mailer) { parent::__construct($appName, $request); $this->l10n = $l10n; $this->config = $config; $this->userSession = $userSession; + $this->urlGenerator = $urlGenerator; $this->mailer = $mailer; } @@ -107,6 +113,8 @@ public function setMailSettings($mail_domain, $this->config->setSystemValues($configs); + $this->config->setAppValue('core', 'emailTestSuccessful', '0'); + return new DataResponse(); } @@ -130,6 +138,8 @@ public function storeCredentials($mail_smtpname, $mail_smtppassword) { 'mail_smtppassword' => $mail_smtppassword, ]); + $this->config->setAppValue('core', 'emailTestSuccessful', '0'); + return new DataResponse(); } @@ -159,14 +169,19 @@ public function sendTestMail() { $message->useTemplate($template); $errors = $this->mailer->send($message); if (!empty($errors)) { + $this->config->setAppValue('core', 'emailTestSuccessful', '0'); throw new \RuntimeException($this->l10n->t('Email could not be sent. Check your mail server log')); } + // Store the successful config in the app config + $this->config->setAppValue('core', 'emailTestSuccessful', '1'); return new DataResponse(); } catch (\Exception $e) { + $this->config->setAppValue('core', 'emailTestSuccessful', '0'); return new DataResponse($this->l10n->t('A problem occurred while sending the email. Please revise your settings. (Error: %s)', [$e->getMessage()]), Http::STATUS_BAD_REQUEST); } } - return new DataResponse($this->l10n->t('You need to set your user email before being able to send test emails.'), Http::STATUS_BAD_REQUEST); + $this->config->setAppValue('core', 'emailTestSuccessful', '0'); + return new DataResponse($this->l10n->t('You need to set your user email before being able to send test emails. Go to %s for that.', [$this->urlGenerator->linkToRouteAbsolute('settings.PersonalSettings.index')]), Http::STATUS_BAD_REQUEST); } } diff --git a/apps/settings/templates/settings/admin/additional-mail.php b/apps/settings/templates/settings/admin/additional-mail.php index 82cd9e09b134b..a7e8382de183c 100644 --- a/apps/settings/templates/settings/admin/additional-mail.php +++ b/apps/settings/templates/settings/admin/additional-mail.php @@ -158,7 +158,7 @@
- t('Test email settings')); ?> + t('Test and verify email settings')); ?> diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 478c4519b2f95..20cf2b0106984 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -182,6 +182,7 @@ protected function setUp(): void { ]) ->setMethods([ 'isReadOnlyConfig', + 'wasEmailTestSuccessful', 'hasValidTransactionIsolationLevel', 'hasFileinfoInstalled', 'hasWorkingFileLocking', @@ -506,6 +507,10 @@ public function testCheck() { ->expects($this->once()) ->method('isReadOnlyConfig') ->willReturn(false); + $this->checkSetupController + ->expects($this->once()) + ->method('wasEmailTestSuccessful') + ->willReturn(false); $this->checkSetupController ->expects($this->once()) ->method('hasValidTransactionIsolationLevel') @@ -604,6 +609,7 @@ public function testCheck() { [ 'isGetenvServerWorking' => true, 'isReadOnlyConfig' => false, + 'wasEmailTestSuccessful' => false, 'hasValidTransactionIsolationLevel' => true, 'hasFileinfoInstalled' => true, 'hasWorkingFileLocking' => true, diff --git a/apps/settings/tests/Controller/MailSettingsControllerTest.php b/apps/settings/tests/Controller/MailSettingsControllerTest.php index 93fffadcfe15e..54461201201a0 100644 --- a/apps/settings/tests/Controller/MailSettingsControllerTest.php +++ b/apps/settings/tests/Controller/MailSettingsControllerTest.php @@ -38,6 +38,7 @@ use OCP\IUserSession; use OCP\Mail\IEMailTemplate; use OCP\Mail\IMailer; +use OCP\IURLGenerator; /** * @package Tests\Settings\Controller @@ -52,6 +53,8 @@ class MailSettingsControllerTest extends \Test\TestCase { private $mailer; /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */ private $l; + /** @var IURLGenerator */ + private $urlGenerator; /** @var MailSettingsController */ private $mailController; @@ -63,6 +66,7 @@ protected function setUp(): void { $this->config = $this->createMock(IConfig::class); $this->userSession = $this->createMock(IUserSession::class); $this->mailer = $this->createMock(IMailer::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject $request */ $request = $this->createMock(IRequest::class); $this->mailController = new MailSettingsController( @@ -71,6 +75,7 @@ protected function setUp(): void { $this->l, $this->config, $this->userSession, + $this->urlGenerator, $this->mailer, 'no-reply@nextcloud.com' ); @@ -170,7 +175,7 @@ public function testSendTestMail() { // Ensure that it fails when no mail address has been specified $response = $this->mailController->sendTestMail(); $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); - $this->assertSame('You need to set your user email before being able to send test emails.', $response->getData()); + $this->assertSame('You need to set your user email before being able to send test emails. Go to for that.', $response->getData()); // If no exception is thrown it should work $this->config diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 8e6f17f07ede9..fdeed4897d091 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -195,6 +195,14 @@ type: OC.SetupChecks.MESSAGE_TYPE_INFO }); } + if (!data.wasEmailTestSuccessful) { + messages.push({ + msg: t('core', 'You have not set or verified your email server configuration, yet. Please head over to the {mailSettingsStart}Basic settings{mailSettingsEnd} in order to set them. Afterwards, use the "Send email" button below the form to verify your settings.',) + .replace('{mailSettingsStart}', '') + .replace('{mailSettingsEnd}', ''), + type: OC.SetupChecks.MESSAGE_TYPE_INFO + }); + } if (!data.hasValidTransactionIsolationLevel) { messages.push({ msg: t('core', 'Your database does not run with "READ COMMITTED" transaction isolation level. This can cause problems when multiple actions are executed in parallel.'), diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index c3e7fab14f19b..cfadcef674651 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -226,6 +226,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -283,6 +284,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -341,6 +343,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -396,6 +399,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -450,6 +454,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -492,6 +497,62 @@ describe('OC.SetupChecks tests', function() { }); }); + it('should return an info if the mail server config was not set or verified, yet', function(done) { + var async = OC.SetupChecks.checkSetup(); + + suite.server.requests[0].respond( + 200, + { + 'Content-Type': 'application/json' + }, + JSON.stringify({ + hasFileinfoInstalled: true, + isGetenvServerWorking: true, + isReadOnlyConfig: false, + wasEmailTestSuccessful: false, + hasWorkingFileLocking: true, + hasValidTransactionIsolationLevel: true, + suggestedOverwriteCliURL: '', + isRandomnessSecure: true, + securityDocs: 'https://docs.nextcloud.com/myDocs.html', + isFairUseOfFreePushService: true, + serverHasInternetConnectionProblems: false, + isMemcacheConfigured: true, + forwardedForHeadersWorking: true, + isCorrectMemcachedPHPModuleInstalled: true, + hasPassedCodeIntegrityCheck: true, + OpcacheSetupRecommendations: [], + phpOpcacheDocumentation: 'https://example.org/link/to/doc', + isSettimelimitAvailable: true, + hasFreeTypeSupport: true, + missingIndexes: [], + missingPrimaryKeys: [], + missingColumns: [], + cronErrors: [], + cronInfo: { + diffInSeconds: 0 + }, + isMemoryLimitSufficient: true, + appDirsWithDifferentOwner: [], + recommendedPHPModules: [], + pendingBigIntConversionColumns: [], + isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, + isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, + reverseProxyGeneratedURL: 'https://server', + temporaryDirectoryWritable: true, + }) + ); + + async.done(function( data, s, x ){ + expect(data).toEqual([{ + msg: 'You have not set or verified your email server configuration, yet. Please head over to the Basic settings in order to set them. Afterwards, use the "Send email" button below the form to verify your settings.', + type: OC.SetupChecks.MESSAGE_TYPE_INFO + }]); + done(); + }); + }); + it('should return a warning if there are app directories with wrong permissions', function(done) { var async = OC.SetupChecks.checkSetup(); @@ -504,6 +565,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -560,6 +622,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -614,6 +677,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -668,6 +732,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -742,6 +807,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -797,6 +863,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -852,6 +919,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -907,6 +975,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -965,6 +1034,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -1020,6 +1090,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -1072,6 +1143,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -1126,6 +1198,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '', @@ -1180,6 +1253,7 @@ describe('OC.SetupChecks tests', function() { hasFileinfoInstalled: true, isGetenvServerWorking: true, isReadOnlyConfig: false, + wasEmailTestSuccessful: true, hasWorkingFileLocking: true, hasValidTransactionIsolationLevel: true, suggestedOverwriteCliURL: '',