diff --git a/lib/Provider/Command/MessageSend.php b/lib/Provider/Command/MessageSend.php index ed610fdb03..4e35a2e3c0 100644 --- a/lib/Provider/Command/MessageSend.php +++ b/lib/Provider/Command/MessageSend.php @@ -8,7 +8,6 @@ */ namespace OCA\Mail\Provider\Command; -use OCA\Mail\AppInfo\Application; use OCA\Mail\Db\LocalMessage; use OCA\Mail\Service\AccountService; use OCA\Mail\Service\Attachment\AttachmentService; @@ -46,7 +45,25 @@ public function __construct( * */ public function perform(string $userId, string $serviceId, IMessage $message, array $options = []): LocalMessage { - // find user mail account details + // validate that at least one To address is present + if (count($message->getTo()) === 0) { + throw new SendException('Invalid Message Parameter: MUST contain at least one TO address with a valid address'); + } + // validate that all To, CC and BCC have email address + $entries = array_merge($message->getTo(), $message->getCc(), $message->getBcc()); + array_walk($entries, function ($entry) { + if (empty($entry->getAddress())) { + throw new SendException('Invalid Message Parameter: All TO, CC and BCC addresses MUST contain at least an email address'); + } + }); + // validate that all attachments have a name, type, and contents + $entries = $message->getAttachments(); + array_walk($entries, function ($entry) { + if (empty($entry->getName()) || empty($entry->getType()) || empty($entry->getContents())) { + throw new SendException('Invalid Attachment Parameter: MUST contain values for Name, Type and Contents'); + } + }); + // retrieve user mail account details try { $account = $this->accountService->find($userId, (int)$serviceId); } catch (\Throwable $e) { @@ -67,10 +84,6 @@ public function perform(string $userId, string $serviceId, IMessage $message, ar if (count($message->getAttachments()) > 0) { // iterate attachments and save them foreach ($message->getAttachments() as $entry) { - // determine if required parameters are set - if (empty($entry->getName()) || empty($entry->getType()) || empty($entry->getContents())) { - throw new SendException('Invalid Attachment Parameter: MUST contain values for Name, Type and Contents'); - } // convert mail provider attachment to mail app attachment try { $attachments[] = $this->attachmentService->addFileFromString( @@ -84,10 +97,6 @@ public function perform(string $userId, string $serviceId, IMessage $message, ar } } } - // determine if required To address is set - if (empty($message->getTo()) || empty($message->getTo()[0]->getAddress())) { - throw new SendException('Invalid Message Parameter: MUST contain at least one TO address with a valid address'); - } // convert recipient addresses $to = $this->convertAddressArray($message->getTo()); $cc = $this->convertAddressArray($message->getCc()); @@ -105,13 +114,11 @@ public function perform(string $userId, string $serviceId, IMessage $message, ar } catch (\Throwable $e) { throw new SendException('Error: occurred while saving mail message', 0, $e); } - // evaluate if system messages are to be sent instantly - if ($this->config->getAppValue(Application::APP_ID, 'send_system_messages_instantly', 'yes') === 'yes') { - try { - $localMessage = $this->outboxService->sendMessage($localMessage, $account); - } catch (\Throwable $e) { - throw new SendException('Error: occurred while sending mail message', 0, $e); - } + // send message + try { + $localMessage = $this->outboxService->sendMessage($localMessage, $account); + } catch (\Throwable $e) { + throw new SendException('Error: occurred while sending mail message', 0, $e); } return $localMessage; @@ -124,7 +131,7 @@ public function perform(string $userId, string $serviceId, IMessage $message, ar * * @param array $addresses collection of IAddress objects * - * @return array + * @return array */ protected function convertAddressArray(array $addresses): array { return array_map(static function (IAddress $address) { diff --git a/lib/Provider/MailProvider.php b/lib/Provider/MailProvider.php index 37ae10cbf1..172ae5d8f1 100644 --- a/lib/Provider/MailProvider.php +++ b/lib/Provider/MailProvider.php @@ -135,7 +135,7 @@ public function findServiceByAddress(string $userId, string $address): IService| return null; } // evaluate if service details where found - if (isset($accounts[0])) { + if (count($accounts) > 0) { // return mail service object return $this->serviceFromAccount($userId, $accounts[0]); } diff --git a/lib/Service/AccountService.php b/lib/Service/AccountService.php index 8307fd0587..dea1e33985 100644 --- a/lib/Service/AccountService.php +++ b/lib/Service/AccountService.php @@ -87,8 +87,6 @@ public function findById(int $id): Account { * @param string $address mail address (e.g. test@example.com) * * @return Account[] - * - * @throws ClientException */ public function findByUserIdAndAddress(string $userId, string $address): array { // evaluate if cached accounts collection already exists @@ -101,21 +99,12 @@ public function findByUserIdAndAddress(string $userId, string $address): array { $list[] = $account; } } - // evaluate if any accounts where found and return them - if (count($list) > 0) { - return $list; - } - // if no accounts where found thrown an error - throw new ClientException("Account with address $address does not exist or you don\'t have permission to access it"); + return $list; } // if cached accounts collection did not exist retrieve account details directly from the data store - try { - return array_map(static function ($a) { - return new Account($a); - }, $this->mapper->findByUserIdAndAddress($userId, $address)); - } catch (DoesNotExistException $e) { - throw new ClientException("Account with address $address does not exist or you don\'t have permission to access it"); - } + return array_map(static function ($a) { + return new Account($a); + }, $this->mapper->findByUserIdAndAddress($userId, $address)); } /** diff --git a/tests/Unit/Provider/Command/MessageSendTest.php b/tests/Unit/Provider/Command/MessageSendTest.php index f60787f475..d78593905c 100644 --- a/tests/Unit/Provider/Command/MessageSendTest.php +++ b/tests/Unit/Provider/Command/MessageSendTest.php @@ -130,6 +130,11 @@ public function testPerformWithAttachment(): void { [], [$localAttachmentReturned->jsonSerialize()] )->willReturn($localMessageReturned); + $this->outboxService->expects($this->once())->method('sendMessage') + ->with( + $localMessageReturned, + $this->localAccount, + )->willReturn($localMessageReturned); // construct mail provider message with attachment $mailMessage = $this->mailMessage; $mailMessage->setAttachments($this->mailAttachment); @@ -165,6 +170,11 @@ public function testPerformWithOutAttachment(): void { [], [] )->willReturn($localMessageReturned); + $this->outboxService->expects($this->once())->method('sendMessage') + ->with( + $localMessageReturned, + $this->localAccount, + )->willReturn($localMessageReturned); // construct mail provider message $mailMessage = $this->mailMessage; // test send message @@ -172,14 +182,6 @@ public function testPerformWithOutAttachment(): void { } public function testPerformWithInvalidAttachment(): void { - // define time factory return - $this->time->method('getTime')->willReturn(1719792000); - // define account service returns - $this->accountService->method('find')->will( - $this->returnValueMap([ - ['user1', 100, $this->localAccount] - ]) - ); // construct mail provider message with attachment $mailMessage = $this->mailMessage; $mailMessage->setAttachments(new Attachment('This is the contents of our plan', 'plan.txt', '')); @@ -190,19 +192,34 @@ public function testPerformWithInvalidAttachment(): void { } public function testPerformWithInvalidTo(): void { - // define time factory return - $this->time->method('getTime')->willReturn(1719792000); - // define account service returns - $this->accountService->method('find')->will( - $this->returnValueMap([ - ['user1', 100, $this->localAccount] - ]) - ); // construct mail provider message $mailMessage = $this->mailMessage; $mailMessage->setTo(new Address('', 'User Two')); // define exception condition $this->expectException(SendException::class); + $this->expectExceptionMessage('Invalid Message Parameter: All TO, CC and BCC addresses MUST contain at least an email address'); + // test send message + $this->commandSend->perform('user1', '100', $mailMessage); + } + + public function testPerformWithInvalidCc(): void { + // construct mail provider message + $mailMessage = $this->mailMessage; + $mailMessage->setCc(new Address('', 'User Three')); + // define exception condition + $this->expectException(SendException::class); + $this->expectExceptionMessage('Invalid Message Parameter: All TO, CC and BCC addresses MUST contain at least an email address'); + // test send message + $this->commandSend->perform('user1', '100', $mailMessage); + } + + public function testPerformWithInvalidBcc(): void { + // construct mail provider message + $mailMessage = $this->mailMessage; + $mailMessage->setBcc(new Address('', 'User Three')); + // define exception condition + $this->expectException(SendException::class); + $this->expectExceptionMessage('Invalid Message Parameter: All TO, CC and BCC addresses MUST contain at least an email address'); // test send message $this->commandSend->perform('user1', '100', $mailMessage); } @@ -329,13 +346,6 @@ public function testPerformWithOutboxServiceSendFailure(): void { ); // construct mail provider message $mailMessage = $this->mailMessage; - // define configuration service returns - $this->config->expects($this->once()) - ->method('getAppValue') - ->with('mail', 'send_system_messages_instantly', 'yes') - ->willReturn('yes'); - // construct mail provider message - $mailMessage = $this->mailMessage; // define exception condition $this->expectException(SendException::class); $this->expectExceptionMessage('Error: occurred while sending mail message');