Merge pull request #11026 from nextcloud/bug/noid/enabeling-sieve-broken

fix enabeling sieve

Author: kesselb

lib/Controller/SieveController.php

@@ -10,7 +10,6 @@
 namespace OCA\Mail\Controller;
 
 use Horde\ManageSieve\Exception as ManagesieveException;
-use OCA\Mail\Account;
 use OCA\Mail\AppInfo\Application;
 use OCA\Mail\Db\MailAccountMapper;
 use OCA\Mail\Exception\ClientException;
@@ -142,36 +141,40 @@ public function updateAccount(int $id,
 			$mailAccount->setSieveEnabled(false);
 			$mailAccount->setSieveHost(null);
 			$mailAccount->setSievePort(null);
+			$mailAccount->setSieveSslMode(null);
 			$mailAccount->setSieveUser(null);
 			$mailAccount->setSievePassword(null);
-			$mailAccount->setSieveSslMode(null);
 
 			$this->mailAccountMapper->save($mailAccount);
 			return new JSONResponse(['sieveEnabled' => $mailAccount->isSieveEnabled()]);
 		}
 
-		if (empty($sieveUser)) {
+		if (empty($sieveUser) && empty($sievePassword)) {
+			$useImapCredentials = true;
 			$sieveUser = $mailAccount->getInboundUser();
-		}
-
-		if (empty($sievePassword)) {
-			$sievePassword = $mailAccount->getInboundPassword();
+			/** @psalm-suppress PossiblyNullArgument */
+			$sievePassword = $this->crypto->decrypt($mailAccount->getInboundPassword());
 		} else {
-			$sievePassword = $this->crypto->encrypt($sievePassword);
+			$useImapCredentials = false;
 		}
 
 		try {
-			$this->sieveClientFactory->getClient(new Account($mailAccount));
+			$this->sieveClientFactory->createClient($sieveHost, $sievePort, $sieveUser, $sievePassword, $sieveSslMode, null);
 		} catch (ManagesieveException $e) {
 			throw new CouldNotConnectException($e, 'ManageSieve', $sieveHost, $sievePort);
 		}
 
 		$mailAccount->setSieveEnabled(true);
 		$mailAccount->setSieveHost($sieveHost);
 		$mailAccount->setSievePort($sievePort);
-		$mailAccount->setSieveUser($mailAccount->getInboundUser() === $sieveUser ? null : $sieveUser);
-		$mailAccount->setSievePassword($mailAccount->getInboundPassword() === $sievePassword ? null : $sievePassword);
 		$mailAccount->setSieveSslMode($sieveSslMode);
+		if ($useImapCredentials) {
+			$mailAccount->setSieveUser(null);
+			$mailAccount->setSievePassword(null);
+		} else {
+			$mailAccount->setSieveUser($sieveUser);
+			$mailAccount->setSievePassword($this->crypto->encrypt($sievePassword));
+		}
 
 		$this->mailAccountMapper->save($mailAccount);
 		return new JSONResponse(['sieveEnabled' => $mailAccount->isSieveEnabled()]);

lib/Sieve/SieveClientFactory.php

@@ -15,18 +15,10 @@
 use OCP\Security\ICrypto;
 
 class SieveClientFactory {
-	/** @var ICrypto */
-	private $crypto;
+	private ICrypto $crypto;
+	private IConfig $config;
+	private array $cache = [];
 
-	/** @var IConfig */
-	private $config;
-
-	private $cache = [];
-
-	/**
-	 * @param ICrypto $crypto
-	 * @param IConfig $config
-	 */
 	public function __construct(ICrypto $crypto, IConfig $config) {
 		$this->crypto = $crypto;
 		$this->config = $config;
@@ -41,43 +33,62 @@ public function getClient(Account $account): ManageSieve {
 			if (empty($user)) {
 				$user = $account->getMailAccount()->getInboundUser();
 			}
-
 			$password = $account->getMailAccount()->getSievePassword();
 			if (empty($password)) {
 				$password = $account->getMailAccount()->getInboundPassword();
 			}
 
-			$sslMode = $account->getMailAccount()->getSieveSslMode();
-			if (empty($sslMode)) {
-				$sslMode = true;
-			} elseif ($sslMode === 'none') {
-				$sslMode = false;
-			}
-
-			$params = [
-				'host' => $account->getMailAccount()->getSieveHost(),
-				'port' => $account->getMailAccount()->getSievePort(),
-				'user' => $user,
-				'password' => $this->crypto->decrypt($password),
-				'secure' => $sslMode,
-				'timeout' => $this->config->getSystemValueInt('app.mail.sieve.timeout', 5),
-				'context' => [
-					'ssl' => [
-						'verify_peer' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
-						'verify_peer_name' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
-					]
-				],
-			];
-
 			if ($account->getDebug() || $this->config->getSystemValueBool('app.mail.debug')) {
-				$fn = 'mail-' . $account->getUserId() . '-' . $account->getId() . '-sieve.log';
-				$params['logger'] = new SieveLogger($this->config->getSystemValue('datadirectory') . '/' . $fn);
+				$logFile = $this->config->getSystemValue('datadirectory') . '/mail-' . $account->getUserId() . '-' . $account->getId() . '-sieve.log';
+			} else {
+				$logFile = null;
 			}
 
-			$this->cache[$account->getId()] = new ManageSieve($params);
+			$this->cache[$account->getId()] = $this->createClient(
+				$account->getMailAccount()->getSieveHost(),
+				$account->getMailAccount()->getSievePort(),
+				$user,
+				$this->crypto->decrypt($password),
+				$account->getMailAccount()->getSieveSslMode(),
+				$logFile,
+			);
 		}
 
 		return $this->cache[$account->getId()];
 	}
 
+	/**
+	 * @param string $sslMode possible values: '', 'none', 'ssl' or 'tls'
+	 * @param ?string $logFile absolute path for logFile or null to disable logging
+	 * @throws ManageSieve\Exception
+	 */
+	public function createClient(string $host, int $port, string $user, string $password, string $sslMode, ?string $logFile): ManageSieve {
+		if (empty($sslMode)) {
+			$sslMode = true;
+		} elseif ($sslMode === 'none') {
+			$sslMode = false;
+		}
+
+		$params = [
+			'host' => $host,
+			'port' => $port,
+			'user' => $user,
+			'password' => $password,
+			'secure' => $sslMode,
+			'timeout' => $this->config->getSystemValueInt('app.mail.sieve.timeout', 5),
+			'context' => [
+				'ssl' => [
+					'verify_peer' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
+					'verify_peer_name' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
+
+				]
+			],
+		];
+
+		if ($logFile !== null) {
+			$params['logger'] = new SieveLogger($logFile);
+		}
+
+		return new ManageSieve($params);
+	}
 }

tests/Unit/Controller/SieveControllerTest.php

@@ -107,7 +107,7 @@ public function testUpdateAccountEnableNoConnection(): void {
 
 		$sieveClientFactory = $this->serviceMock->getParameter('sieveClientFactory');
 		$sieveClientFactory->expects($this->once())
-			->method('getClient')
+			->method('createClient')
 			->willThrowException(new Exception('Computer says no'));
 
 		$response = $this->sieveController->updateAccount(2, true, 'localhost', 4190, 'user', 'password', '');