Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable3.7] fix: setup check for mail transport php-mail #10576

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
use OCA\Mail\Service\Search\MailSearch;
use OCA\Mail\Service\TrustedSenderService;
use OCA\Mail\Service\UserPreferenceService;
use OCA\Mail\SetupChecks\MailTransport;
use OCA\Mail\Vendor\Favicon\Favicon;
use OCP\AppFramework\App;
use OCP\AppFramework\Bootstrap\IBootContext;
Expand Down Expand Up @@ -186,6 +187,11 @@

$context->registerNotifierService(Notifier::class);

if (method_exists($context, 'registerSetupCheck')) {
// registerSetupCheck needs Nextcloud 28 or newer
$context->registerSetupCheck(MailTransport::class);

Check failure on line 192 in lib/AppInfo/Application.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-stable27

MissingDependency

lib/AppInfo/Application.php:192:33: MissingDependency: OCA\Mail\SetupChecks\MailTransport depends on class or interface ocp\setupcheck\isetupcheck that does not exist (see https://psalm.dev/157)
}

// bypass Horde Translation system
Horde_Translation::setHandler('Horde_Imap_Client', new HordeTranslationHandler());
Horde_Translation::setHandler('Horde_Mime', new HordeTranslationHandler());
Expand Down
43 changes: 43 additions & 0 deletions lib/SetupChecks/MailTransport.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Mail\SetupChecks;

use OCP\IConfig;
use OCP\IL10N;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;

class MailTransport implements ISetupCheck {
public function __construct(
private IConfig $config,
private IL10N $l10n,
) {
}

public function getName(): string {
return $this->l10n->t('Mail Transport configuration');
}

public function getCategory(): string {
return 'mail';
}

public function run(): SetupResult {
$transport = $this->config->getSystemValueString('app.mail.transport', 'smtp');

if ($transport === 'smtp') {
return SetupResult::success();
}

return SetupResult::warning(
$this->l10n->t('The app.mail.transport setting is not set to smtp. This configuration can cause issues with modern email security measures such as SPF and DKIM because emails are sent directly from the web server, which is often not properly configured for this purpose. To address this, we have discontinued support for the mail transport. Please remove app.mail.transport from your configuration to use the SMTP transport and hide this message. A properly configured SMTP setup is required to ensure email delivery.')
);
}
}
1 change: 1 addition & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
<referencedClass name="OCP\User\Events\OutOfOfficeChangedEvent" /><!-- 28+ -->
<referencedClass name="OCP\User\Events\OutOfOfficeClearedEvent" /><!-- 28+ -->
<referencedClass name="OCP\User\IAvailabilityCoordinator" /><!-- 28+ -->
<referencedClass name="OCP\SetupCheck\ISetupCheck" /><!-- 28+ -->
</errorLevel>
</UndefinedClass>
<UndefinedDocblockClass>
Expand Down
66 changes: 66 additions & 0 deletions tests/Unit/SetupChecks/MailTransportTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace Unit\SetupChecks;

use ChristophWurst\Nextcloud\Testing\TestCase;
use OC\L10N\L10N;
use OCA\Mail\SetupChecks\MailTransport;
use OCP\IConfig;
use OCP\IL10N;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use PHPUnit\Framework\MockObject\MockObject;

class MailTransportTest extends TestCase {
private IConfig|MockObject $config;
private IL10N|MockObject $l10n;
private MailTransport $check;

protected function setUp(): void {
parent::setUp();

if (!interface_exists(ISetupCheck::class)) {
$this->markTestSkipped('ISetupCheck is not available.');
}

$this->config = $this->createMock(IConfig::class);
$this->l10n = $this->createMock(L10N::class);

$this->check = new MailTransport($this->config, $this->l10n);
}

public function testSuccess(): void {
$this->config->method('getSystemValueString')
->willReturn('smtp');

$result = $this->check->run();

$this->assertEquals(SetupResult::SUCCESS, $result->getSeverity());
}

public function testWarning(): void {
$this->config->method('getSystemValueString')
->willReturn('mail');

$result = $this->check->run();

$this->assertEquals(SetupResult::WARNING, $result->getSeverity());
}

public function testName(): void {
$this->l10n->method('t')
->willReturn('Translated Name');

$this->assertEquals('Translated Name', $this->check->getName());
}

public function testCategory(): void {
$this->assertEquals('mail', $this->check->getCategory());
}
}
Loading