diff --git a/lib/Controller/DraftsController.php b/lib/Controller/DraftsController.php index 47a7f558c7..f6d95ef59e 100644 --- a/lib/Controller/DraftsController.php +++ b/lib/Controller/DraftsController.php @@ -99,6 +99,7 @@ public function create( array $cc = [], array $bcc = [], array $attachments = [], + bool $force = false, ?int $aliasId = null, ?string $inReplyToMessageId = null, ?int $smimeCertificateId = null, @@ -121,6 +122,7 @@ public function create( $message->setSendAt($sendAt); $message->setSmimeSign($smimeSign); $message->setSmimeEncrypt($smimeEncrypt); + $message->setForce($force); if (!empty($smimeCertificateId)) { $smimeCertificate = $this->smimeService->findCertificate($smimeCertificateId, $this->userId); @@ -165,6 +167,7 @@ public function update(int $id, array $cc = [], array $bcc = [], array $attachments = [], + bool $force = false, ?int $aliasId = null, ?string $inReplyToMessageId = null, ?int $smimeCertificateId = null, @@ -172,7 +175,6 @@ public function update(int $id, $message = $this->service->getMessage($id, $this->userId); $account = $this->accountService->find($this->userId, $accountId); - $message->setType(LocalMessage::TYPE_DRAFT); $message->setAccountId($accountId); $message->setAliasId($aliasId); @@ -186,6 +188,7 @@ public function update(int $id, $message->setUpdatedAt($this->timeFactory->getTime()); $message->setSmimeSign($smimeSign); $message->setSmimeEncrypt($smimeEncrypt); + $message->setForce($force); if (!empty($smimeCertificateId)) { $smimeCertificate = $this->smimeService->findCertificate($smimeCertificateId, $this->userId); diff --git a/lib/Controller/OutboxController.php b/lib/Controller/OutboxController.php index 0b1850ed8f..ff3f54df51 100644 --- a/lib/Controller/OutboxController.php +++ b/lib/Controller/OutboxController.php @@ -115,6 +115,7 @@ public function create( array $cc = [], array $bcc = [], array $attachments = [], + bool $force, ?int $draftId = null, ?int $aliasId = null, ?string $inReplyToMessageId = null, @@ -139,6 +140,7 @@ public function create( $message->setSendAt($sendAt); $message->setSmimeSign($smimeSign); $message->setSmimeEncrypt($smimeEncrypt); + $message->setForce($force); if (!empty($smimeCertificateId)) { $smimeCertificate = $this->smimeService->findCertificate($smimeCertificateId, $this->userId); @@ -198,11 +200,11 @@ public function update( bool $isHtml, bool $smimeSign, bool $smimeEncrypt, - bool $failed = false, array $to = [], array $cc = [], array $bcc = [], array $attachments = [], + bool $force, ?int $aliasId = null, ?string $inReplyToMessageId = null, ?int $smimeCertificateId = null, @@ -224,6 +226,7 @@ public function update( $message->setSendAt($sendAt); $message->setSmimeSign($smimeSign); $message->setSmimeEncrypt($smimeEncrypt); + $message->setForce($force); if (!empty($smimeCertificateId)) { $smimeCertificate = $this->smimeService->findCertificate($smimeCertificateId, $this->userId); diff --git a/lib/Db/LocalMessage.php b/lib/Db/LocalMessage.php index c71e910c25..62ef4a7ce7 100644 --- a/lib/Db/LocalMessage.php +++ b/lib/Db/LocalMessage.php @@ -64,6 +64,8 @@ * @method setStatus(?int $status); * @method string|null getRaw() * @method setRaw(string|null $raw) + * @method bool getForce() + * @method setForce(bool $force) */ class LocalMessage extends Entity implements JsonSerializable { public const TYPE_OUTGOING = 0; @@ -142,6 +144,9 @@ class LocalMessage extends Entity implements JsonSerializable { /** @var string|null */ protected $raw; + /** @var bool */ + protected $force; + public function __construct() { $this->addType('type', 'integer'); $this->addType('accountId', 'integer'); @@ -154,6 +159,8 @@ public function __construct() { $this->addType('smimeCertificateId', 'integer'); $this->addType('smimeEncrypt', 'boolean'); $this->addType('status', 'integer'); + $this->addType('force', 'boolean'); + } #[ReturnTypeWillChange] @@ -197,6 +204,7 @@ public function jsonSerialize() { 'smimeEncrypt' => $this->getSmimeEncrypt() === true, 'status' => $this->getStatus(), 'raw' => $this->getRaw(), + 'force' => $this->getForce(), ]; } diff --git a/lib/Migration/Version3600Date20240319181700.php b/lib/Migration/Version3600Date20240319181700.php new file mode 100644 index 0000000000..2bc3a44acb --- /dev/null +++ b/lib/Migration/Version3600Date20240319181700.php @@ -0,0 +1,57 @@ + + * + * FIXME @author Your name + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Mail\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version3600Date20240319181700 extends SimpleMigrationStep { + + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $localMessagesTable = $schema->getTable('mail_local_messages'); + if (!$localMessagesTable->hasColumn('force')) { + $localMessagesTable->addColumn('force', Types::BOOLEAN, [ + 'notnull' => false, + 'default' => false, + ]); + } + + return $schema; + } +} diff --git a/lib/Send/AntiAbuseHandler.php b/lib/Send/AntiAbuseHandler.php index ee2e5eaa21..c1cc54c695 100644 --- a/lib/Send/AntiAbuseHandler.php +++ b/lib/Send/AntiAbuseHandler.php @@ -25,13 +25,15 @@ use OCA\Mail\Account; use OCA\Mail\Db\LocalMessage; use OCA\Mail\Service\AntiAbuseService; +use OCA\Mail\Service\RecipientsService; use OCP\IUserManager; use Psr\Log\LoggerInterface; class AntiAbuseHandler extends AHandler { public function __construct(private IUserManager $userManager, - private AntiAbuseService $service, + private AntiAbuseService $antiAbuseService, + private RecipientsService $recipientsService, private LoggerInterface $logger) { parent::__construct(); } @@ -51,14 +53,16 @@ public function process(Account $account, LocalMessage $localMessage): LocalMess return $localMessage; } - $this->service->onBeforeMessageSent( + $this->recipientsService->checkNumberOfRecipients($account, $localMessage); + if($localMessage->getStatus() === LocalMessage::STATUS_TOO_MANY_RECIPIENTS && $localMessage->getForce() === false) { + return $localMessage; + } + + // We don't react to the ratelimit at the moment. + $this->antiAbuseService->onBeforeMessageSent( $user, $localMessage, ); - // We don't react to a ratelimited message / a message that has too many recipients - // at this point. - // Any future improvement from https://github.com/nextcloud/mail/issues/6461 - // should refactor the chain to stop at this point unless the force send option is true return $this->processNext($account, $localMessage); } } diff --git a/lib/Service/AntiAbuseService.php b/lib/Service/AntiAbuseService.php index 6c6876d780..98b7b3eae0 100644 --- a/lib/Service/AntiAbuseService.php +++ b/lib/Service/AntiAbuseService.php @@ -69,32 +69,9 @@ public function onBeforeMessageSent(IUser $user, LocalMessage $localMessage): vo return; } - $this->checkNumberOfRecipients($user, $localMessage); $this->checkRateLimits($user, $localMessage); } - private function checkNumberOfRecipients(IUser $user, LocalMessage $message): void { - $numberOfRecipientsThreshold = (int)$this->config->getAppValue( - Application::APP_ID, - 'abuse_number_of_recipients_per_message_threshold', - '0', - ); - if ($numberOfRecipientsThreshold <= 1) { - return; - } - - $actualNumberOfRecipients = count($message->getRecipients()); - - if ($actualNumberOfRecipients >= $numberOfRecipientsThreshold) { - $message->setStatus(LocalMessage::STATUS_TOO_MANY_RECIPIENTS); - $this->logger->alert('User {user} sends to a suspicious number of recipients. {expected} are allowed. {actual} are used', [ - 'user' => $user->getUID(), - 'expected' => $numberOfRecipientsThreshold, - 'actual' => $actualNumberOfRecipients, - ]); - } - } - private function checkRateLimits(IUser $user, LocalMessage $message): void { if (!$this->cacheFactory->isAvailable()) { // No cache, no rate limits diff --git a/lib/Service/DraftsService.php b/lib/Service/DraftsService.php index f3c9d596b4..6fe6ee3e90 100644 --- a/lib/Service/DraftsService.php +++ b/lib/Service/DraftsService.php @@ -34,6 +34,7 @@ use OCA\Mail\Db\Recipient; use OCA\Mail\Events\DraftMessageCreatedEvent; use OCA\Mail\Exception\ClientException; +use OCA\Mail\Exception\ManyRecipientsException; use OCA\Mail\Exception\ServiceException; use OCA\Mail\IMAP\IMAPClientFactory; use OCA\Mail\Service\Attachment\AttachmentService; @@ -62,7 +63,8 @@ public function __construct(IMailTransmission $transmission, IMailManager $mailManager, LoggerInterface $logger, AccountService $accountService, - ITimeFactory $time) { + ITimeFactory $time, + private RecipientsService $recipientsService) { $this->transmission = $transmission; $this->mapper = $mapper; $this->attachmentService = $attachmentService; @@ -109,6 +111,11 @@ public function deleteMessage(string $userId, LocalMessage $message): void { * @return LocalMessage */ public function saveMessage(Account $account, LocalMessage $message, array $to, array $cc, array $bcc, array $attachments = []): LocalMessage { + $this->recipientsService->checkNumberOfRecipients($account, $message); + if($message->getStatus() === LocalMessage::STATUS_TOO_MANY_RECIPIENTS && $message->getForce() === false) { + throw new ManyRecipientsException(); + } + $toRecipients = self::convertToRecipient($to, Recipient::TYPE_TO); $ccRecipients = self::convertToRecipient($cc, Recipient::TYPE_CC); $bccRecipients = self::convertToRecipient($bcc, Recipient::TYPE_BCC); @@ -147,11 +154,15 @@ public function saveMessage(Account $account, LocalMessage $message, array $to, * @return LocalMessage */ public function updateMessage(Account $account, LocalMessage $message, array $to, array $cc, array $bcc, array $attachments = []): LocalMessage { + $this->recipientsService->checkNumberOfRecipients($account, $message); + if($message->getStatus() === LocalMessage::STATUS_TOO_MANY_RECIPIENTS && $message->getForce() === false) { + throw new ManyRecipientsException(); + } + $toRecipients = self::convertToRecipient($to, Recipient::TYPE_TO); $ccRecipients = self::convertToRecipient($cc, Recipient::TYPE_CC); $bccRecipients = self::convertToRecipient($bcc, Recipient::TYPE_BCC); - $message = $this->mapper->updateWithRecipients($message, $toRecipients, $ccRecipients, $bccRecipients); if ($attachments === []) { diff --git a/lib/Service/RecipientsService.php b/lib/Service/RecipientsService.php new file mode 100644 index 0000000000..a5d887c098 --- /dev/null +++ b/lib/Service/RecipientsService.php @@ -0,0 +1,102 @@ + + * + * @author Anna Larch + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE + * License as published by the Free Software Foundation; either + * version 3 of the License, or any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU Affero General Public + * License along with this library. If not, see . + */ + +namespace OCA\Mail\Service; + +use OCA\Mail\Account; +use OCA\Mail\AppInfo\Application; +use OCA\Mail\Db\LocalMessage; +use OCA\Mail\Db\Recipient; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IConfig; +use Psr\Log\LoggerInterface; + +class RecipientsService { + + public function __construct( + private LoggerInterface $logger, + private IConfig $config, + private GroupsIntegration $groupsIntegration, + ) { + } + + /** + * @param Account $account + * @param LocalMessage $message + * @return LocalMessage + * @throws DoesNotExistException + */ + public function checkNumberOfRecipients(Account $account, LocalMessage $message): void { + if($message->getForce() === true) { + return; + } + + $numberOfRecipientsThreshold = (int)$this->config->getAppValue( + Application::APP_ID, + 'abuse_number_of_recipients_per_message_threshold', + '0', + ); + if ($numberOfRecipientsThreshold <= 1) { + return; + } + + $recipients = $this->groupsIntegration->expand($message->getRecipients()); + $to = count(array_filter($recipients, function ($recipient) { + return $recipient->getType() === Recipient::TYPE_TO; + })); + if ($to >= $numberOfRecipientsThreshold) { + $message->setStatus(LocalMessage::STATUS_TOO_MANY_RECIPIENTS); + $this->logger->alert('User {user} sends to a suspicious number of "TO" recipients. {expected} are allowed. {actual} are used', [ + 'user' => $account->getUserId(), + 'expected' => $numberOfRecipientsThreshold, + 'actual' => $to, + ]); + return; + } + + $cc = count(array_filter($recipients, function ($recipient) { + return $recipient->getType() === Recipient::TYPE_CC; + })); + if ($cc >= $numberOfRecipientsThreshold) { + $message->setStatus(LocalMessage::STATUS_TOO_MANY_RECIPIENTS); + $this->logger->alert('User {user} sends to a suspicious number of "CC" recipients. {expected} are allowed. {actual} are used', [ + 'user' => $account->getUserId(), + 'expected' => $numberOfRecipientsThreshold, + 'actual' => $cc, + ]); + return; + } + + $bcc = count(array_filter($recipients, function ($recipient) { + return $recipient->getType() === Recipient::TYPE_BCC; + })); + if ($bcc >= $numberOfRecipientsThreshold) { + $message->setStatus(LocalMessage::STATUS_TOO_MANY_RECIPIENTS); + $this->logger->alert('User {user} sends to a suspicious number of "BCC" recipients. {expected} are allowed. {actual} are used', [ + 'user' => $account->getUserId(), + 'expected' => $numberOfRecipientsThreshold, + 'actual' => $bcc, + ]); + return; + } + } +} diff --git a/tests/Unit/Send/AntiAbuseHandlerTest.php b/tests/Unit/Send/AntiAbuseHandlerTest.php index d837b3b713..5a313a2654 100644 --- a/tests/Unit/Send/AntiAbuseHandlerTest.php +++ b/tests/Unit/Send/AntiAbuseHandlerTest.php @@ -47,6 +47,7 @@ use OCA\Mail\Send\AntiAbuseHandler; use OCA\Mail\Send\SendHandler; use OCA\Mail\Service\AntiAbuseService; +use OCA\Mail\Service\RecipientsService; use OCP\IUser; use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; @@ -64,9 +65,11 @@ protected function setUp(): void { $this->antiAbuseService = $this->createMock(AntiAbuseService::class); $this->logger = $this->createMock(LoggerInterface::class); $this->sendHandler = $this->createMock(SendHandler::class); + $this->recipientsService = $this->createMock(RecipientsService::class); $this->handler = new AntiAbuseHandler( $this->userManager, $this->antiAbuseService, + $this->recipientsService, $this->logger, ); $this->handler->setNext($this->sendHandler);