mail filters backend

REUSE.toml

@@ -101,6 +101,12 @@ precedence = "aggregate"
 SPDX-FileCopyrightText = "2024 Nextcloud GmbH and Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"
 
+[[annotations]]
+path = ["tests/data/mail-filter"]
+precedence = "aggregate"
+SPDX-FileCopyrightText = "2024 Nextcloud GmbH and Nextcloud contributors"
+SPDX-License-Identifier = "AGPL-3.0-or-later"
+
 [[annotations]]
 path = ".github/CODEOWNERS"
 precedence = "aggregate"

lib/Service/AllowedRecipientsService.php

@@ -14,7 +14,7 @@
 class AllowedRecipientsService {
 
 	public function __construct(
-		private readonly AliasesService $aliasesService,
+		private AliasesService $aliasesService,
 	) {
 	}
 

lib/Service/MailFilter/FilterBuilder.php

@@ -11,6 +11,7 @@
 
 use OCA\Mail\Exception\ImapFlagEncodingException;
 use OCA\Mail\IMAP\ImapFlag;
+use OCA\Mail\Sieve\SieveUtils;
 
 class FilterBuilder {
 	private const SEPARATOR = '### Nextcloud Mail: Filters ### DON\'T EDIT ###';
@@ -38,34 +39,46 @@ public function buildSieveScript(array $filters, string $untouchedScript): strin
 					$tests[] = sprintf(
 						'header :%s "Subject" %s',
 						$test['operator'],
-						$this->stringList($test['values'])
+						SieveUtils::stringList($test['values']),
 					);
 				}
 				if ($test['field'] === 'to') {
 					$tests[] = sprintf(
 						'address :%s :all "To" %s',
 						$test['operator'],
-						$this->stringList($test['values'])
+						SieveUtils::stringList($test['values']),
 					);
 				}
 				if ($test['field'] === 'from') {
 					$tests[] = sprintf(
 						'address :%s :all "From" %s',
 						$test['operator'],
-						$this->stringList($test['values'])
+						SieveUtils::stringList($test['values']),
 					);
 				}
 			}
 
+			if (count($tests) === 0) {
+				// skip filter without tests
+				$commands[] = '# No valid tests found';
+				continue;
+			}
+
 			$actions = [];
 			foreach ($filter['actions'] as $action) {
 				if ($action['type'] === 'fileinto') {
 					$extensions[] = 'fileinto';
-					$actions[] = sprintf('fileinto "%s";', $action['mailbox']);
+					$actions[] = sprintf(
+						'fileinto "%s";',
+						SieveUtils::escapeString($action['mailbox'])
+					);
 				}
 				if ($action['type'] === 'addflag') {
 					$extensions[] = 'imap4flags';
-					$actions[] = sprintf('addflag %s;', $this->stringList($this->sanitizeFlag($action['flag'])));
+					$actions[] = sprintf(
+						'addflag "%s";',
+						SieveUtils::escapeString($this->sanitizeFlag($action['flag']))
+					);
 				}
 				if ($action['type'] === 'keep') {
 					$actions[] = 'keep;';
@@ -82,7 +95,7 @@ public function buildSieveScript(array $filters, string $untouchedScript): strin
 			}
 
 			$ifBlock = sprintf(
-				"if %s {\r\n%s\r\n}\r\n",
+				"if %s {\r\n%s\r\n}",
 				$ifTest,
 				implode(self::SIEVE_NEWLINE, $actions)
 			);
@@ -95,20 +108,17 @@ public function buildSieveScript(array $filters, string $untouchedScript): strin
 
 		if (count($extensions) > 0) {
 			$requireSection[] = self::SEPARATOR;
-			$requireSection[] = 'require ' . $this->stringList($extensions) . ';';
+			$requireSection[] = 'require ' . SieveUtils::stringList($extensions) . ';';
 			$requireSection[] = self::SEPARATOR;
-			$requireSection[] = '';
 		}
 
 		$stateJsonString = json_encode($this->sanitizeDefinition($filters), JSON_THROW_ON_ERROR);
 
 		$filterSection = [
-			'',
 			self::SEPARATOR,
 			self::DATA_MARKER . $stateJsonString,
 			...$commands,
 			self::SEPARATOR,
-			'',
 		];
 
 		return implode(self::SIEVE_NEWLINE, array_merge(
@@ -118,22 +128,6 @@ public function buildSieveScript(array $filters, string $untouchedScript): strin
 		));
 	}
 
-	private function stringList(string|array $value): string {
-		if (is_string($value)) {
-			$items = explode(',', $value);
-		} else {
-			$items = $value;
-		}
-
-		$items = array_map([$this, 'quoteString'], $items);
-
-		return '[' . implode(', ', $items) . ']';
-	}
-
-	private function quoteString(string $value): string {
-		return '"' . $value . '"';
-	}
-
 	private function sanitizeFlag(string $flag): string {
 		try {
 			return $this->imapFlag->create($flag);
@@ -153,6 +147,7 @@ private function sanitizeDefinition(array $filters): array {
 				unset($action['id']);
 				return $action;
 			}, $filter['actions']);
+			$filter['priority'] = (int)$filter['priority'];
 			return $filter;
 		}, $filters);
 	}

lib/Service/OutOfOffice/OutOfOfficeParser.php

@@ -13,6 +13,7 @@
 use DateTimeZone;
 use JsonException;
 use OCA\Mail\Exception\OutOfOfficeParserException;
+use OCA\Mail\Sieve\SieveUtils;
 
 /**
  * Parses and builds out-of-office states from/to sieve scripts.
@@ -119,7 +120,7 @@ public function buildSieveScript(
 			$condition = "currentdate :value \"ge\" \"iso8601\" \"$formattedStart\"";
 		}
 
-		$escapedSubject = $this->escapeStringForSieve($state->getSubject());
+		$escapedSubject = SieveUtils::escapeString($state->getSubject());
 		$vacation = [
 			'vacation',
 			':days 4',
@@ -134,7 +135,7 @@ public function buildSieveScript(
 			$vacation[] = ":addresses [$joinedRecipients]";
 		}
 
-		$escapedMessage = $this->escapeStringForSieve($state->getMessage());
+		$escapedMessage = SieveUtils::escapeString($state->getMessage());
 		$vacation[] = "\"$escapedMessage\"";
 		$vacationCommand = implode(' ', $vacation);
 
@@ -183,9 +184,4 @@ public function buildSieveScript(
 	private function formatDateForSieve(DateTimeImmutable $date): string {
 		return $date->setTimezone($this->utc)->format('Y-m-d\TH:i:s\Z');
 	}
-
-	private function escapeStringForSieve(string $subject): string {
-		$subject = preg_replace('/\\\\/', '\\\\\\\\', $subject);
-		return preg_replace('/"/', '\\"', $subject);
-	}
 }

lib/Sieve/SieveUtils.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\Sieve;
+
+class SieveUtils {
+	/**
+	 * Escape a string for use in a Sieve script
+	 *
+	 * @see https://www.rfc-editor.org/rfc/rfc5228#section-2.4.2
+	 */
+	public static function escapeString(string $subject): string {
+		$subject = preg_replace(
+			['/\\\\/', '/"/'],
+			['\\\\\\\\', '\\"'],
+			$subject
+		);
+
+		return (string)$subject;
+	}
+
+	/**
+	 * Return a string list for use in a Sieve script
+	 *
+	 * @see https://www.rfc-editor.org/rfc/rfc5228#section-2.4.2.1
+	 */
+	public static function stringList(array $values): string {
+		$values = array_map([__CLASS__, 'escapeString'], $values);
+
+		return '["' . implode('", "', $values) . '"]';
+	}
+}

tests/Unit/Service/MailFilter/FilterBuilderTest.php

@@ -42,14 +42,17 @@ public function testBuild(string $testName): void {
 
 		$script = $this->builder->buildSieveScript($filters, $untouchedScript);
 
+		// the .sieve files have \r\n line endings
+		$script .= "\r\n";
+
 		$this->assertStringEqualsFile(
 			$this->testFolder . $testName . '.sieve',
 			$script
 		);
 	}
 
 	public function dataBuild(): array {
-		$files = glob($this->testFolder . 'test*.json');
+		$files = glob($this->testFolder . 'builder*.json');
 		$tests = [];
 
 		foreach($files as $file) {

tests/Unit/Service/MailFilter/FilterParserTest.php

@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Unit\Service\MailFilter;
+
+use ChristophWurst\Nextcloud\Testing\TestCase;
+use OCA\Mail\Service\MailFilter\FilterParser;
+
+class FilterParserTest extends TestCase {
+	private FilterParser $filterParser;
+
+	private string $testFolder;
+
+	public function __construct(?string $name = null, array $data = [], $dataName = '') {
+		parent::__construct($name, $data, $dataName);
+		$this->testFolder = __DIR__ . '/../../../data/mail-filter/';
+	}
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->filterParser = new FilterParser();
+	}
+
+	public function testParse1(): void {
+		$script = file_get_contents($this->testFolder . 'parser1.sieve');
+
+		$state = $this->filterParser->parseFilterState($script);
+		$filters = $state->getFilters();
+
+		$this->assertCount(1, $filters);
+		$this->assertSame('Test 1', $filters[0]['name']);
+		$this->assertTrue($filters[0]['enable']);
+		$this->assertSame('allof', $filters[0]['operator']);
+		$this->assertSame(10, $filters[0]['priority']);
+
+		$this->assertCount(1, $filters[0]['tests']);
+		$this->assertSame('from', $filters[0]['tests'][0]['field']);
+		$this->assertSame('is', $filters[0]['tests'][0]['operator']);
+		$this->assertEquals(['[email protected]', '[email protected]'], $filters[0]['tests'][0]['values']);
+
+		$this->assertCount(1, $filters[0]['actions']);
+		$this->assertSame('addflag', $filters[0]['actions'][0]['type']);
+		$this->assertSame('Alice and Bob', $filters[0]['actions'][0]['flag']);
+	}
+
+	public function testParse2(): void {
+		$script = file_get_contents($this->testFolder . 'parser2.sieve');
+
+		$state = $this->filterParser->parseFilterState($script);
+		$filters = $state->getFilters();
+
+		$this->assertCount(1, $filters);
+		$this->assertSame('Test 2', $filters[0]['name']);
+		$this->assertTrue($filters[0]['enable']);
+		$this->assertSame('anyof', $filters[0]['operator']);
+		$this->assertSame(20, $filters[0]['priority']);
+
+		$this->assertCount(2, $filters[0]['tests']);
+		$this->assertSame('subject', $filters[0]['tests'][0]['field']);
+		$this->assertSame('contains', $filters[0]['tests'][0]['operator']);
+		$this->assertEquals(['Project-A', 'Project-B'], $filters[0]['tests'][0]['values']);
+		$this->assertSame('from', $filters[0]['tests'][1]['field']);
+		$this->assertSame('is', $filters[0]['tests'][1]['operator']);
+		$this->assertEquals(['[email protected]'], $filters[0]['tests'][1]['values']);
+
+		$this->assertCount(1, $filters[0]['actions']);
+		$this->assertSame('fileinto', $filters[0]['actions'][0]['type']);
+		$this->assertSame('Test Data', $filters[0]['actions'][0]['mailbox']);
+	}
+}

tests/Unit/Service/OutOfOfficeServiceTest.php

@@ -151,11 +151,11 @@ public function testUpdateFromSystemWithEnabledOutOfOffice(?IOutOfOfficeData $da
 				['[email protected]'],
 			)
 			->willReturn('# new sieve script');
-		$aliasesService = $this->serviceMock->getParameter('aliasesService');
-		$aliasesService->expects(self::once())
-			->method('findAll')
-			->with(1, 'user')
-			->willReturn([]);
+		$allowedRecipientsService = $this->serviceMock->getParameter('allowedRecipientsService');
+		$allowedRecipientsService->expects(self::once())
+			->method('get')
+			->with($mailAccount)
+			->willReturn(['[email protected]']);
 		$sieveService->expects(self::once())
 			->method('updateActiveScript')
 			->with('user', 1, '# new sieve script');
@@ -229,11 +229,11 @@ public function testUpdateFromSystemWithDisabledOutOfOffice(?IOutOfOfficeData $d
 				['[email protected]'],
 			)
 			->willReturn('# new sieve script');
-		$aliasesService = $this->serviceMock->getParameter('aliasesService');
-		$aliasesService->expects(self::once())
-			->method('findAll')
-			->with(1, 'user')
-			->willReturn([]);
+		$allowedRecipientsService = $this->serviceMock->getParameter('allowedRecipientsService');
+		$allowedRecipientsService->expects(self::once())
+			->method('get')
+			->with($mailAccount)
+			->willReturn(['[email protected]']);
 		$sieveService->expects(self::once())
 			->method('updateActiveScript')
 			->with('user', 1, '# new sieve script');