Skip to content

Commit

Permalink
fix(outbox): add recipients check
Browse files Browse the repository at this point in the history
Signed-off-by: Anna Larch <anna@nextcloud.com>
  • Loading branch information
miaulalala committed Oct 9, 2023
1 parent fef0d04 commit f3ac92b
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 1 deletion.
7 changes: 6 additions & 1 deletion lib/Controller/OutboxController.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,13 @@ public function update(
* @return JsonResponse
*/
#[TrapError]
public function send(int $id): JsonResponse {
public function send(int $id, bool $force = false): JsonResponse {
$message = $this->service->getMessage($id, $this->userId);

if($force === false) {
$this->service->recipientCheck($message->getRecipients());
}

$account = $this->accountService->find($this->userId, $message->getAccountId());

$this->service->sendMessage($message, $account);
Expand Down
11 changes: 11 additions & 0 deletions lib/Service/OutboxService.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use OCA\Mail\Db\Recipient;
use OCA\Mail\Events\OutboxMessageCreatedEvent;
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;
Expand Down Expand Up @@ -284,4 +285,14 @@ public function convertDraft(LocalMessage $draftMessage, int $sendAt): LocalMess
$outboxMessage->setSendAt($sendAt);
return $this->mapper->update($outboxMessage);
}

/**
* @param array $recipients
* @return void
*/
public function recipientCheck(array $recipients): void {
if(count($recipients) > 10) {
throw new ManyRecipientsException();
}
}
}
65 changes: 65 additions & 0 deletions tests/Unit/Controller/OutboxControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
use OCA\Mail\Controller\OutboxController;
use OCA\Mail\Db\LocalMessage;
use OCA\Mail\Db\MailAccount;
use OCA\Mail\Db\Recipient;
use OCA\Mail\Exception\ClientException;
use OCA\Mail\Exception\ManyRecipientsException;
use OCA\Mail\Exception\ServiceException;
use OCA\Mail\Http\JsonResponse;
use OCA\Mail\Service\AccountService;
Expand All @@ -59,6 +61,7 @@ class OutboxControllerTest extends TestCase {

/** @var SmimeService&MockObject */
private $smimeService;
private array $recipients;

private OutboxController $controller;
protected function setUp(): void {
Expand All @@ -79,6 +82,19 @@ protected function setUp(): void {
$this->accountService,
$this->smimeService,
);

$this->recipients = [
Recipient::fromParams([
'email' => 'emily@stardewvalleypub.com',
'label' => 'Emily',
'type' => Recipient::TYPE_TO
]),
Recipient::fromParams([
'email' => 'gus@stardewvalleypub.com',
'label' => 'Gus',
'type' => Recipient::TYPE_TO
])
];
}

public function testIndex(): void {
Expand Down Expand Up @@ -144,12 +160,16 @@ public function testSend(): void {
$message = new LocalMessage();
$message->setId(1);
$message->setAccountId(1);
$message->setRecipients($this->recipients);
$account = new Account(new MailAccount());

$this->service->expects(self::once())
->method('getMessage')
->with($message->getId(), $this->userId)
->willReturn($message);
$this->service->expects(self::once())
->method('recipientCheck')
->with($this->recipients);
$this->accountService->expects(self::once())
->method('find')
->with($this->userId, $message->getAccountId())
Expand All @@ -164,15 +184,52 @@ public function testSend(): void {
$this->assertEquals($expected, $actual);
}

public function sendTooManyRecipients(): void {
$message = new LocalMessage();
$message->setId(1);
$message->setAccountId(1);
$recipients = [];
for ($i = 0; $i < 16; $i++) {
$recipients[] = Recipient::fromParams([
'email' => 'emily' . $i. '@stardewvalleypub.com',
'label' => 'Emily ' . $i,
'type' => Recipient::TYPE_TO
]);
}
$message->setRecipients($recipients);

$this->service->expects(self::once())
->method('getMessage')
->with($message->getId(), $this->userId)
->willReturn($message);
$this->service->expects(self::once())
->method('recipientCheck')
->with($recipients)
->willThrowException(new ManyRecipientsException());
$this->accountService->expects(self::never())
->method('find');
$this->service->expects(self::never())
->method('sendMessage');

$this->expectException(ManyRecipientsException::class);
$expected = JsonResponse::fail('', Http::STATUS_NOT_FOUND);
$actual = $this->controller->send($message->getId());

$this->assertEquals($expected, $actual);
}

public function testSendNoMessage(): void {
$message = new LocalMessage();
$message->setId(1);
$message->setAccountId(1);
$message->setRecipients($this->recipients);

$this->service->expects(self::once())
->method('getMessage')
->with($message->getId(), $this->userId)
->willThrowException(new DoesNotExistException(''));
$this->service->expects(self::never())
->method('recipientCheck');
$this->accountService->expects(self::never())
->method('find');
$this->service->expects(self::never())
Expand All @@ -189,11 +246,15 @@ public function testSendClientException(): void {
$message = new LocalMessage();
$message->setId(1);
$message->setAccountId(1);
$message->setRecipients($this->recipients);

$this->service->expects(self::once())
->method('getMessage')
->with($message->getId(), $this->userId)
->willReturn($message);
$this->service->expects(self::once())
->method('recipientCheck')
->with($this->recipients);
$this->accountService->expects(self::once())
->method('find')
->with($this->userId, $message->getAccountId())
Expand All @@ -209,12 +270,16 @@ public function testSendServiceException(): void {
$message = new LocalMessage();
$message->setId(1);
$message->setAccountId(1);
$message->setRecipients($this->recipients);
$account = new Account(new MailAccount());

$this->service->expects(self::once())
->method('getMessage')
->with($message->getId(), $this->userId)
->willReturn($message);
$this->service->expects(self::once())
->method('recipientCheck')
->with($this->recipients);
$this->accountService->expects(self::once())
->method('find')
->with($this->userId, $message->getAccountId())
Expand Down

0 comments on commit f3ac92b

Please sign in to comment.